From afd9ab3ed0892bc4bd0b678b943b1b6cc17cd98b Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Sun, 30 Aug 2026 21:34:51 +0200 Subject: [PATCH 1/2] fix(tui): write session exports owner-only (0600) Transcripts can carry sensitive tool output; the exported artifact now matches the tokens/settings store standard instead of being world-readable. --- internal/tui/export_test.go | 36 ++++++++++++++++++++++++++++++++++++ internal/tui/panels.go | 4 +++- 2 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 internal/tui/export_test.go diff --git a/internal/tui/export_test.go b/internal/tui/export_test.go new file mode 100644 index 0000000..83a3b74 --- /dev/null +++ b/internal/tui/export_test.go @@ -0,0 +1,36 @@ +package tui + +import ( + "os" + "testing" + + "github.com/BackendStack21/bodek/internal/client" +) + +// TestExportWritesOwnerOnlyFile pins the export file mode: transcripts can +// carry sensitive tool output, so the artifact must be readable by its owner +// only (0600), matching the tokens/settings store standard. +func TestExportWritesOwnerOnlyFile(t *testing.T) { + m := wired(t) + t.Chdir(t.TempDir()) // exports land in the process CWD + + m.panel = panelSessions + m.sessions = []client.Session{{ID: "s1"}} + m.panelSel = 0 + + msg := exec(m.exportSelected("md")) + em, ok := msg.(sessionExportedMsg) + if !ok { + t.Fatalf("export cmd yielded %#v, want sessionExportedMsg", msg) + } + if em.err != nil { + t.Fatalf("export failed: %v", em.err) + } + info, err := os.Stat(em.path) + if err != nil { + t.Fatalf("stat exported file: %v", err) + } + if got := info.Mode().Perm(); got != 0o600 { + t.Errorf("export %s mode = %o, want 600", em.path, got) + } +} diff --git a/internal/tui/panels.go b/internal/tui/panels.go index f78703c..6a440ab 100644 --- a/internal/tui/panels.go +++ b/internal/tui/panels.go @@ -731,7 +731,9 @@ func (m *Model) exportSelected(format string) tea.Cmd { return sessionExportedMsg{id: id, err: err} } path := fmt.Sprintf("bodek-%s.%s", id, format) - if err := os.WriteFile(path, data, 0o644); err != nil { + // 0600: transcripts can carry sensitive tool output — owner-only, + // matching the tokens/settings store standard. + if err := os.WriteFile(path, data, 0o600); err != nil { return sessionExportedMsg{id: id, err: err} } return sessionExportedMsg{id: id, path: path} From 7f6ccfed9044703d44f91ded4d02c424006c62b6 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Sun, 30 Aug 2026 21:34:51 +0200 Subject: [PATCH 2/2] fix(tokens): surface persist errors and remove orphaned tmp persist() now returns an error instead of silently swallowing every failure; Set/Delete report it on stderr while keeping best-effort semantics. A failed rename no longer leaves the staged .tmp behind. --- internal/tokens/persist_errors_test.go | 43 ++++++++++++++++++++++++++ internal/tokens/tokens.go | 36 +++++++++++++++------ 2 files changed, 70 insertions(+), 9 deletions(-) create mode 100644 internal/tokens/persist_errors_test.go diff --git a/internal/tokens/persist_errors_test.go b/internal/tokens/persist_errors_test.go new file mode 100644 index 0000000..076c56e --- /dev/null +++ b/internal/tokens/persist_errors_test.go @@ -0,0 +1,43 @@ +package tokens + +import ( + "os" + "path/filepath" + "testing" +) + +// TestPersistSurfacesErrors pins that persist reports failures instead of +// silently dropping them: a store that fails to save must be observable, +// otherwise session resume breaks with no diagnostic. +func TestPersistSurfacesErrors(t *testing.T) { + dir := t.TempDir() + // A regular file where the store directory should be makes MkdirAll fail. + blocker := filepath.Join(dir, "blocker") + if err := os.WriteFile(blocker, []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + err := persist(filepath.Join(blocker, "sessions.json"), map[string]string{"id": "tok"}) + if err == nil { + t.Error("persist against an unwritable path should report an error") + } +} + +// TestPersistNoTmpLeftOnRenameFailure pins the tmp cleanup: when the final +// rename fails, the staged .tmp file must not be left behind. +func TestPersistNoTmpLeftOnRenameFailure(t *testing.T) { + dir := t.TempDir() + store := filepath.Join(dir, "sessions.json") + // A non-empty directory at the store path makes the final rename fail. + if err := os.Mkdir(store, 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(store, "occupied"), []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + if err := persist(store, map[string]string{"id": "tok"}); err == nil { + t.Error("persist onto a directory path should report an error") + } + if _, err := os.Stat(store + ".tmp"); !os.IsNotExist(err) { + t.Errorf("failed persist left the .tmp file behind: %v", err) + } +} diff --git a/internal/tokens/tokens.go b/internal/tokens/tokens.go index 7ddbef1..1795ab5 100644 --- a/internal/tokens/tokens.go +++ b/internal/tokens/tokens.go @@ -5,11 +5,12 @@ // auth_token) and requires it on the cancel/detail/delete endpoints. The Web // UI keeps these in localStorage; bodek keeps them in ~/.bodek/sessions.json. // Persistence is best-effort — a Store with no writable path still works as an -// in-memory cache for the current run. +// in-memory cache for the current run; failures are reported on stderr. package tokens import ( "encoding/json" + "fmt" "os" "path/filepath" "sync" @@ -65,7 +66,9 @@ func (s *Store) Set(id, token string) { } path := s.path s.mu.Unlock() - persist(path, snapshot) + if err := persist(path, snapshot); err != nil { + warnPersist(err) + } } // Delete removes a session's token and persists the store (best-effort). @@ -85,23 +88,38 @@ func (s *Store) Delete(id string) { } path := s.path s.mu.Unlock() - persist(path, snapshot) + if err := persist(path, snapshot); err != nil { + warnPersist(err) + } } -func persist(path string, m map[string]string) { +// persist atomically writes the store (staged .tmp + rename) so a crash +// mid-write never corrupts the previous snapshot. +func persist(path string, m map[string]string) error { if path == "" { - return + return nil } if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { - return + return fmt.Errorf("create store dir: %w", err) } data, err := json.MarshalIndent(m, "", " ") if err != nil { - return + return fmt.Errorf("encode store: %w", err) } tmp := path + ".tmp" if err := os.WriteFile(tmp, data, 0o600); err != nil { - return + return fmt.Errorf("write store: %w", err) + } + if err := os.Rename(tmp, path); err != nil { + _ = os.Remove(tmp) // don't leave the staged copy behind + return fmt.Errorf("replace store: %w", err) } - _ = os.Rename(tmp, path) + return nil +} + +// warnPersist reports a failed best-effort save without aborting the +// operation: the store stays a working in-memory cache, but a silent failure +// would break session resume with no diagnostic. +func warnPersist(err error) { + fmt.Fprintf(os.Stderr, "bodek: warning: session token store not saved: %v\n", err) }