diff options
| author | Paul Buetow <paul@buetow.org> | 2026-04-17 08:45:05 +0300 |
|---|---|---|
| committer | Paul Buetow <paul@buetow.org> | 2026-04-17 08:45:05 +0300 |
| commit | b53e348d89046ea8de5b82c283fb56980b53cdd8 (patch) | |
| tree | f2115ccf556dddfc99ad129522069845c53ebd21 /internal/store | |
| parent | e4671dc15d224d944fe5c3715c95793c21167443 (diff) | |
feat: add WriteBack hook on store.Data for backend-specific edit round-trip (tasks i4+l4)
Add optional WriteBack func([]byte) error field to Data struct.
ReimportAfterExport calls WriteBack(newContent) when set, otherwise
falls back to Commit. applyAction now calls s.LoadData (exported) so
the geheim and KeePass edit paths are symmetric. Tests cover nil,
non-nil, and error-propagation paths.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Diffstat (limited to 'internal/store')
| -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 } |
