fix: owner-only session exports and surfaced token-store persist errors (#50)

* 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.

* 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.

Author: jkyberneees

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)
+	}
+}

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)
 }

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)
+	}
+}

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}