diff options
| -rw-r--r-- | internal/store/data.go | 26 | ||||
| -rw-r--r-- | internal/store/data_test.go | 140 | ||||
| -rw-r--r-- | internal/store/store.go | 29 |
3 files changed, 189 insertions, 6 deletions
diff --git a/internal/store/data.go b/internal/store/data.go index 8ab8429..1b9c562 100644 --- a/internal/store/data.go +++ b/internal/store/data.go @@ -13,12 +13,21 @@ import ( // Data holds a decrypted secret blob and the paths used to persist it. // DataPath is the absolute path to the on-disk .data file. // ExportedPath is populated by Export() and consumed by ReimportAfterExport(). +// +// WriteBack is an optional hook for backend-specific persistence. When set, +// ReimportAfterExport calls WriteBack(newContent) instead of the default +// encrypt-and-git-stage path (Commit). The KeePass backend will populate it +// with parse-fields+upsert+save. This keeps the edit command identical +// across backends. type Data struct { Content []byte DataPath string // absolute path to .data file ExportedPath string // set by Export(), used by ReimportAfterExport() - encryptor Encryptor - committer Committer + // WriteBack, when non-nil, is called by ReimportAfterExport with the + // newly read content instead of using the default Commit path. + WriteBack func([]byte) error + encryptor Encryptor + committer Committer } // loadData decrypts a .data file and returns a Data struct with Content populated. @@ -71,8 +80,12 @@ func (d *Data) Export(ctx context.Context, exportDir, destinationFile string) er } // ReimportAfterExport reads the (possibly edited) file from ExportedPath back -// into Content and then commits it. This is used by the edit workflow: export → -// user edits in external editor → reimport. +// into Content and persists the new content. This is used by the edit workflow: +// export → user edits in external editor → reimport. +// +// If WriteBack is set, it is called with the new content — allowing backends +// (e.g. KeePass) to supply their own persistence logic. Otherwise, the default +// geheim path is used: encrypt and git-stage via Commit. func (d *Data) ReimportAfterExport(ctx context.Context) error { content, err := os.ReadFile(d.ExportedPath) if err != nil { @@ -80,6 +93,11 @@ func (d *Data) ReimportAfterExport(ctx context.Context) error { } d.Content = content + + if d.WriteBack != nil { + return d.WriteBack(content) + } + return d.Commit(ctx, true) } diff --git a/internal/store/data_test.go b/internal/store/data_test.go index 1c05a42..351618d 100644 --- a/internal/store/data_test.go +++ b/internal/store/data_test.go @@ -4,6 +4,7 @@ package store import ( "context" + "errors" "os" "path/filepath" "strings" @@ -275,3 +276,142 @@ func TestDataCommitMissingCommitter(t *testing.T) { t.Fatalf("Commit error = %q; want missing committer", err.Error()) } } + +// --- TestReimportAfterExportWriteBack ---------------------------------------- + +// writeBackMode selects which sub-case of TestReimportAfterExportWriteBack runs. +type writeBackMode int + +const ( + modeNoWriteBack writeBackMode = iota // nil WriteBack → falls back to Commit + modeWriteBack // non-nil WriteBack → hook is called + modeWriteBackError // non-nil WriteBack that returns an error +) + +// TestReimportAfterExportWriteBack exercises three paths through ReimportAfterExport: +// - nil WriteBack: falls back to Commit (verified via "missing committer" error) +// - non-nil WriteBack: hook is called with the new content +// - non-nil WriteBack that errors: error is propagated to the caller +// +// Table-driven so new cases can be added without duplicating setup logic. +func TestReimportAfterExportWriteBack(t *testing.T) { + cases := []struct { + name string + editedContent string + mode writeBackMode + }{ + { + name: "nil WriteBack uses Commit path", + editedContent: "updated via commit path\n", + mode: modeNoWriteBack, + }, + { + name: "non-nil WriteBack is called with new content", + editedContent: "updated via WriteBack hook\n", + mode: modeWriteBack, + }, + { + name: "WriteBack error is propagated", + editedContent: "updated content that triggers hook error\n", + mode: modeWriteBackError, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + dir := t.TempDir() + + // Write the "edited" file that ReimportAfterExport will read. + exportedPath := filepath.Join(dir, "secret.txt") + if err := os.WriteFile(exportedPath, []byte(tc.editedContent), 0o600); err != nil { + t.Fatalf("writing exported file: %v", err) + } + + switch tc.mode { + case modeWriteBack: + testReimportWithWriteBack(t, ctx, exportedPath, tc.editedContent) + case modeWriteBackError: + testReimportWithWriteBackError(t, ctx, exportedPath) + case modeNoWriteBack: + testReimportWithoutWriteBack(t, ctx, dir, exportedPath, tc.editedContent) + } + }) + } +} + +// testReimportWithWriteBack verifies that ReimportAfterExport calls WriteBack +// with the file's content when WriteBack is non-nil. +func testReimportWithWriteBack(t *testing.T, ctx context.Context, exportedPath, wantContent string) { + t.Helper() + + var capturedContent []byte + d := &Data{ + ExportedPath: exportedPath, + WriteBack: func(newContent []byte) error { + capturedContent = newContent + return nil + }, + } + + if err := d.ReimportAfterExport(ctx); err != nil { + t.Fatalf("ReimportAfterExport with WriteBack: %v", err) + } + if string(capturedContent) != wantContent { + t.Errorf("WriteBack received %q; want %q", capturedContent, wantContent) + } + // Content field should also be updated. + if string(d.Content) != wantContent { + t.Errorf("d.Content = %q; want %q", d.Content, wantContent) + } +} + +// testReimportWithWriteBackError verifies that when WriteBack returns a non-nil +// error, ReimportAfterExport propagates that error to the caller unchanged. +func testReimportWithWriteBackError(t *testing.T, ctx context.Context, exportedPath string) { + t.Helper() + + sentinelErr := errors.New("backend write failed") + d := &Data{ + ExportedPath: exportedPath, + WriteBack: func(newContent []byte) error { + return sentinelErr + }, + } + + err := d.ReimportAfterExport(ctx) + if err == nil { + t.Fatal("ReimportAfterExport with failing WriteBack: expected error, got nil") + } + if !errors.Is(err, sentinelErr) { + t.Errorf("error = %v; want sentinel error %v", err, sentinelErr) + } +} + +// testReimportWithoutWriteBack verifies that ReimportAfterExport falls back to +// Commit (encrypt+git-stage) when WriteBack is nil. We stub out git by leaving +// committer nil and expect the "missing committer" error, which confirms the +// Commit path was reached (not a hook). +func testReimportWithoutWriteBack(t *testing.T, ctx context.Context, dir, exportedPath, editedContent string) { + t.Helper() + + c := newTestCipher(t) + dataPath := filepath.Join(dir, "entry.data") + + d := &Data{ + ExportedPath: exportedPath, + DataPath: dataPath, + encryptor: c, + // WriteBack intentionally left nil — must use Commit path. + // committer left nil so Commit returns "missing committer" error, + // confirming we reached Commit rather than any WriteBack hook. + } + + err := d.ReimportAfterExport(ctx) + if err == nil { + t.Fatal("expected missing committer error from nil-WriteBack path, got nil") + } + if !strings.Contains(err.Error(), "missing committer") { + t.Errorf("error = %q; want missing committer (confirms Commit path was taken)", err.Error()) + } +} diff --git a/internal/store/store.go b/internal/store/store.go index c91798f..9d661c5 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -150,11 +150,33 @@ func (s *Store) processIndexFile(ctx context.Context, path, searchTerm string, r } // LoadData decrypts and returns the .data payload for the given index entry. +// The returned Data.WriteBack is populated with the geheim re-encrypt path so +// that ReimportAfterExport encrypts and git-stages the file via Commit, exactly +// as before the WriteBack hook was introduced. func (s *Store) LoadData(ctx context.Context, idx *Index) (*Data, error) { if idx == nil { return nil, fmt.Errorf("loading data: nil index") } - return loadData(ctx, filepath.Join(s.cfg.DataDir, idx.DataFile), s.cipher, s.git) + + d, err := loadData(ctx, filepath.Join(s.cfg.DataDir, idx.DataFile), s.cipher, s.git) + if err != nil { + return nil, err + } + + // Populate WriteBack so that ReimportAfterExport uses the standard + // encrypt-and-git-stage path rather than relying on ReimportAfterExport's + // built-in Commit fallback. This makes the hook explicit and keeps + // backend-specific reimport logic out of the Data struct itself + // (Open/Closed principle). d.Content is already set by ReimportAfterExport + // before calling WriteBack, so no assignment is needed here. + d.WriteBack = func(newContent []byte) error { + // newContent is intentionally ignored here: ReimportAfterExport already + // assigned it to d.Content before calling WriteBack, and Commit reads + // d.Content directly. + return d.Commit(ctx, true) + } + + return d, nil } // Search collects all indexes matching searchTerm, sorts them by Description, @@ -210,8 +232,11 @@ func (s *Store) applyAction(ctx context.Context, idx *Index, action Action, acti default: // ActionPaste, ActionOpen, ActionEdit — require external tools; // delegate to the caller-supplied callback. + // Use s.LoadData (exported) rather than the bare loadData so that + // WriteBack is populated, enabling ReimportAfterExport (ActionEdit) + // to encrypt and git-stage changes via the standard hook path. if actionFn != nil { - d, err := loadData(ctx, filepath.Join(s.cfg.DataDir, idx.DataFile), s.cipher, s.git) + d, err := s.LoadData(ctx, idx) if err != nil { return err } |