diff --git a/cmd/gnoma/main.go b/cmd/gnoma/main.go index f477652..fee70e1 100644 --- a/cmd/gnoma/main.go +++ b/cmd/gnoma/main.go @@ -354,7 +354,11 @@ func main() { // Restore QualityTracker data from disk (best-effort). Per-profile // path avoids bandit cross-contamination between work/private/etc. - { + // Skipped under --incognito to keep prior learned quality out of the + // session's selection biases. (TUI-runtime incognito can't fire this + // early — the firewall doesn't exist yet — so the CLI flag is the + // only source of truth at restore time.) + if !*incognito { qualityPath := profile.QualityFile(gnomacfg.GlobalConfigDir()) if data, err := os.ReadFile(qualityPath); err == nil { var snap router.QualitySnapshot @@ -369,18 +373,27 @@ func main() { // incognito). Lifted to a named closure so the /profile switch path // can fire it explicitly before syscall.Exec, since defers don't run // after a successful exec. + // + // Two gates: the CLI flag is the unconditional first check (covers + // the window between `defer saveQuality()` here and `fwRef.Set(fw)` + // downstream, where fwRef.Get() still returns nil). The firewall + // state is the second check so TUI-runtime Ctrl+X correctly + // suppresses the snapshot on exit. saveQuality := func() { if *incognito { return } + if fw := fwRef.Get(); fw != nil && !fw.Incognito().ShouldLearn() { + return + } snap := rtr.QualityTracker().Snapshot() data, err := json.Marshal(snap) if err != nil { return } qualityPath := profile.QualityFile(gnomacfg.GlobalConfigDir()) - _ = os.MkdirAll(filepath.Dir(qualityPath), 0o755) - _ = os.WriteFile(qualityPath, data, 0o644) + _ = os.MkdirAll(filepath.Dir(qualityPath), 0o700) + _ = os.WriteFile(qualityPath, data, 0o600) } defer saveQuality() var armID router.ArmID @@ -536,9 +549,25 @@ func main() { } } - // Incognito mode + // Incognito mode. Both flags must move together — the firewall owns + // the intent flag, the router owns the local-only enforcement, and + // they go out of sync if either is set in isolation. We also reject + // the --incognito + --provider combination here rather + // than silently routing to a forced cloud arm under an "incognito" + // badge (audit finding W2-1). if *incognito { + if forced := rtr.ForcedArm(); forced != "" { + if arm, ok := rtr.LookupArm(forced); ok && !arm.IsLocal { + fmt.Fprintf(os.Stderr, + "error: --incognito conflicts with --provider %s (non-local arm). "+ + "Clear the provider pin or drop --incognito.\n", + forced, + ) + os.Exit(1) + } + } fw.Incognito().Activate() + rtr.SetLocalOnly(true) logger.Debug("incognito mode enabled") } @@ -572,7 +601,10 @@ func main() { time.Now().Format("20060102-150405"), mrand.Int63()&0xffffff, ) - store := persist.New(sessionID) + // Pass the firewall's incognito mode so Save no-ops while incognito + // is active. Mode is consulted on every Save (dynamic), so TUI + // runtime toggles take effect without reconstructing the store. + store := persist.New(sessionID, fw.Incognito()) logger.Debug("session store initialized", "dir", store.Dir()) // Create elf manager and register agent tools. diff --git a/docs/superpowers/plans/2026-05-19-security-wave2-incognito.md b/docs/superpowers/plans/2026-05-19-security-wave2-incognito.md new file mode 100644 index 0000000..db05c91 --- /dev/null +++ b/docs/superpowers/plans/2026-05-19-security-wave2-incognito.md @@ -0,0 +1,240 @@ +# Security Hardening Wave 2 — Incognito Coherence — 2026-05-19 + +Follow-up to +[`2026-05-19-security-wave1-safeprovider.md`](2026-05-19-security-wave1-safeprovider.md) +(merged via PR #1). Addresses audit findings 5, 6, 7 — the cluster +where gnoma's incognito promise leaks because state is duplicated +across the CLI flag, the firewall's `IncognitoMode`, the router's +`localOnly` flag, and the TUI's local `m.incognito` field, and these +four go out of sync on the realistic paths. + +The audit's UI string is "🔒 incognito ON — no persistence, no +learning, local-only routing." Today, depending on which path the +user took, any of those three promises can be silently false. Wave 2 +makes the promise hold. + +--- + +## Findings to close + +### 1. Forced cloud arm beats local-only + +`internal/router/router.go:67-73` short-circuits on `forcedArm` before +the `localOnly` filter at line 81. Concretely: launch with +`--provider anthropic`, then `Ctrl+X` to enter incognito. The TUI +says "local-only routing"; the next turn streams to Anthropic. + +### 2. Tool-result store persists in incognito + +`cmd/gnoma/main.go:575` calls `persist.New(sessionID)` unconditionally. +The engine's `Save` path at `internal/engine/loop.go:709-714` only +checks `e.cfg.Store != nil`. With `--incognito` *or* with +`Ctrl+X`-activated incognito, tool results ≥1 KiB still land in +`/tmp/gnoma-/tool-results/`. Audit also flags 0o755 / 0o644 +perms. + +### 3. TUI `m.incognito` not initialized from firewall + +`internal/tui/app.go:133` declares `incognito bool` but `tui.New(...)` +doesn't seed it from `fw.Incognito().Active()`. Launch with +`--incognito`: the firewall is on, but `m.incognito = false`. First +`Ctrl+X` calls `Toggle()`, flips the firewall off, and now incognito +is silently disabled. + +### 4. `saveQuality` and `ReportOutcome` gate on the wrong source + +`cmd/gnoma/main.go:373` checks `*incognito` (the CLI flag). If the +user entered incognito via TUI at runtime, the bandit snapshot still +writes on exit. Likewise `internal/engine/loop.go:85` calls +`Router.ReportOutcome` unconditionally — every successful turn during +incognito updates the per-arm × per-task EMA quality. + +### 5. Session files are 0o644 + +`internal/session/store.go:167` writes session JSON at world-readable +0o644. Not a network leak, but inconsistent with the audit's "0600 +everywhere for sensitive local files" recommendation. + +### 6. `IncognitoMode.LocalOnly` is dead weight + +`internal/security/incognito.go:12` declares `LocalOnly bool` as a +public field. Nothing reads it; the router uses its own `localOnly` +state set via `SetLocalOnly`. Two state pointers for the same idea. + +--- + +## Approach + +The pattern across all six findings is the same: there's no single +source of truth for "is the session in incognito?" Wave 2 makes +`security.IncognitoMode` that source. + +- All gates (persist, learn, route, log) read from + `fw.Incognito().Should*()` rather than the CLI flag or local state. +- The router rejects forced non-local arms when `localOnly` is on + (rather than silently bypassing). +- The TUI seeds its display flag from the firewall on startup; the + firewall stays canonical thereafter. +- File modes harden to 0o600 / 0o700. +- The unused `IncognitoMode.LocalOnly` field gets removed. + +### Why not move `localOnly` onto `IncognitoMode`? + +Tempting — it would centralise everything on one type. But the router +already has the access pattern (mutex around arm selection) and the +firewall has no business carrying routing state. Wave 2 keeps the +existing split: firewall owns the *intent* flag (`Active()`); router +owns the *enforcement* flag (`localOnly`). What changes is that they +must agree, which is a TUI/CLI concern, not a router/firewall concern. + +--- + +## Tasks + +### W2-1 — Router rejects forced non-local arms under local-only + +- [ ] `internal/router/router.go:Select` — when `r.forcedArm != ""` + and the forced arm `IsLocal == false` and `r.localOnly == true`, + return `RoutingDecision{Error: ...}` with an actionable message + ("incognito requires a local arm; current force is %s — clear the + --provider pin or disable incognito"). +- [ ] Test: forced cloud arm + `SetLocalOnly(true)` → `Select` error. +- [ ] Test: forced local arm + `SetLocalOnly(true)` → `Select` + returns that arm. +- [ ] `internal/tui/app.go` — incognito toggle path must check + `rtr.ForcedArm()` before flipping. If a non-local arm is pinned, + refuse with a message ("clear `--provider` to enter incognito"). +- [ ] CLI: when both `--incognito` and `--provider ` are set + at launch, fail fast in `main.go` with the same message rather than + starting a broken session. + +### W2-2 — Persist store consults IncognitoMode + +- [ ] `internal/tool/persist/store.go` — `New(sessionID, *security.IncognitoMode)` + signature. `Save` returns `("", false)` when + `mode != nil && !mode.ShouldPersist()`. Existing + `len(content) < minPersistSize` check stays. +- [ ] Permission tightening: `MkdirAll(..., 0o700)`, + `WriteFile(..., 0o600)`. No reason these need group/world read. +- [ ] `cmd/gnoma/main.go:575` passes `fw.Incognito()` to `persist.New`. +- [ ] `internal/persist` tests cover (a) incognito-active store + returns false from `Save`, (b) directory mode is 0o700 when created, + (c) file mode is 0o600. +- [ ] Audit note: persist mode is *dynamic* (consults + `ShouldPersist()` on every call), not frozen at construction. So + TUI-runtime toggles work without needing to swap the store. + +### W2-3 — TUI seeds incognito from firewall + +- [ ] `internal/tui/app.go:tui.New` (or wherever the model is + constructed) — `m.incognito = cfg.Firewall.Incognito().Active()`. +- [ ] Render the status-bar badge from the firewall, not from local + state, on every render. `m.incognito` becomes a UI cache that's + re-read from the firewall on each `View()` (or kept in sync + explicitly). +- [ ] Test: launch with `--incognito`, hit `Ctrl+X` once → firewall + is off, badge gone. Currently fails because first toggle ON->OFF + reads from wrong state. + +### W2-4 — Quality + outcome gates read firewall, not CLI flag + +- [ ] `cmd/gnoma/main.go:saveQuality` — replace `if *incognito` with + `if !fw.Incognito().ShouldLearn()`. +- [ ] Quality *restore* path at `main.go:351-360` — when + `fw.Incognito().Active()` at startup, don't read the snapshot at + all. Avoids leaking learned quality into an incognito session. +- [ ] `internal/engine/loop.go:reportOutcome` (the inner closure) — + gate on `e.cfg.Firewall != nil && e.cfg.Firewall.Incognito().ShouldLearn()`. +- [ ] Elf result reporting (`internal/elf/manager.go:ReportResult`) + needs the same gate. Pass `fw` into the manager or thread the + predicate. +- [ ] Tests: confirm `Router.ReportOutcome` is *not* called from the + engine path when incognito is active; quality snapshot not written + on exit; quality snapshot not loaded on startup under + `--incognito`. + +### W2-5 — Session file perms + +- [ ] `internal/session/store.go:167` — `os.WriteFile(tmp, data, 0o600)`. +- [ ] Any `MkdirAll` in the session path → 0o700. +- [ ] Test: session file written from a default-umask process has + mode `0o600`. + +### W2-6 — Remove dead `IncognitoMode.LocalOnly` field + +- [ ] `internal/security/incognito.go:12` — drop `LocalOnly bool`. + Audit grep confirms no readers. +- [ ] If any tests reference it, remove those lines. + +--- + +## Exit criteria + +- Launching with `--incognito` and `--provider anthropic` fails fast + with an actionable error (not a silent cloud routing under an + "incognito" badge). +- During an incognito session — entered either via `--incognito` or + `Ctrl+X` — no files appear under `/tmp/gnoma-/`, no + bandit snapshot is written on exit, no `Router.ReportOutcome` calls + fire from the engine or elves. +- `Ctrl+X` on a `--incognito` launch flips the firewall *off* on + first press (matches the badge state at launch). +- All persisted files (sessions, tool-results) are mode 0o600, + directories 0o700. +- `IncognitoMode` no longer has a `LocalOnly` field. +- `make test`, `make lint`, `go test -race ./...` green. + +--- + +## Out of scope (Wave 3 or beyond) + +- **Permission mode** under incognito (audit didn't flag; current + behaviour is intentional — incognito doesn't change *what tools can + run*, only what persists / where it routes). +- **Provider request body logging** — slog output today may include + redacted-but-still-context-rich data. If we want + `ShouldLogContent` to actually gate something, that's its own + finding; current `ShouldLogContent` has zero readers and is part + of the same dead-weight pattern as `LocalOnly`. Decision: keep + `ShouldLogContent` because it's the natural extension point if we + later add structured request logging; remove only `LocalOnly` + (which is structurally wrong, not just unused). +- **Re-exec carrying incognito state** — `argsWithProfileReplaced` + preserves `--incognito` through profile switches via `os.Args`. + Verified, no work needed. + +--- + +## Effort estimate + +- W2-1 (router gate + UI refuse): ~80 LOC + ~50 LOC tests. +- W2-2 (persist incognito): ~50 LOC + ~80 LOC tests. +- W2-3 (TUI seed): ~30 LOC + ~40 LOC test. +- W2-4 (gates on firewall state): ~60 LOC across 4 files + ~80 LOC tests. +- W2-5 (file modes): ~10 LOC + ~30 LOC tests. +- W2-6 (dead field): ~5 LOC. + +Total: ~400 LOC including tests. One PR. + +--- + +## Suggested execution order + +1. **W2-6** first — tiny cleanup, surfaces any test that was + relying on the dead field. +2. **W2-1** next — the router gate is independently useful and + doesn't depend on TUI work. +3. **W2-2** — persist gate. Wave 2's biggest individual change. +4. **W2-4** — outcome/quality gates. Touches engine + elf manager; + easier with a clean baseline from W2-2. +5. **W2-3** — TUI seed. Last because TUI tests are slowest to write + and easiest to land once the underlying state is coherent. +6. **W2-5** — file modes. Trivial, but adding a test that asserts + `Stat().Mode().Perm() == 0o600` should land together with the + change. + +--- + +## Changelog + +- 2026-05-19: Initial. diff --git a/internal/elf/elf_test.go b/internal/elf/elf_test.go index f0ad84d..965201c 100644 --- a/internal/elf/elf_test.go +++ b/internal/elf/elf_test.go @@ -11,6 +11,7 @@ import ( "somegit.dev/Owlibou/gnoma/internal/message" "somegit.dev/Owlibou/gnoma/internal/provider" "somegit.dev/Owlibou/gnoma/internal/router" + "somegit.dev/Owlibou/gnoma/internal/security" "somegit.dev/Owlibou/gnoma/internal/stream" "somegit.dev/Owlibou/gnoma/internal/tool" ) @@ -310,6 +311,43 @@ func TestManager_CleanupRemovesMeta(t *testing.T) { } } +func TestManager_ReportResultSuppressedWhenIncognito(t *testing.T) { + // Incognito sessions must not leave bandit signal behind, even for + // background elf turns. ReportOutcome should be skipped; pool + // reservations must still commit so capacity accounting stays sane. + mp := &mockProvider{ + name: "test", + streams: []stream.Stream{newEventStream("result")}, + } + + rtr := router.New(router.Config{}) + armID := router.ArmID("test/mock") + rtr.RegisterArm(&router.Arm{ + ID: armID, Provider: mp, ModelName: "mock", + Capabilities: provider.Capabilities{ToolUse: true}, + }) + + fw := security.NewFirewall(security.FirewallConfig{ScanOutgoing: true}) + fw.Incognito().Activate() + + mgr := NewManager(ManagerConfig{ + Router: rtr, + Tools: tool.NewRegistry(), + Firewall: fw, + }) + + e, err := mgr.Spawn(context.Background(), router.TaskGeneration, "task", "", 30) + if err != nil { + t.Fatalf("Spawn: %v", err) + } + result := e.Wait() + mgr.ReportResult(result) + + if _, hasData := rtr.QualityTracker().Quality(armID, router.TaskGeneration); hasData { + t.Error("quality tracker received outcome despite incognito — gating not effective") + } +} + // slowEventStream blocks until context cancelled type slowEventStream struct { done bool diff --git a/internal/elf/manager.go b/internal/elf/manager.go index 190b9ec..be15d50 100644 --- a/internal/elf/manager.go +++ b/internal/elf/manager.go @@ -134,6 +134,12 @@ func (m *Manager) ReportResult(result Result) { // safe — it just moves reserved tokens to used at rate 0. meta.decision.Commit(int(result.Usage.TotalTokens())) + // Suppress quality feedback while incognito is active — bandit + // learning would otherwise persist signal about the session. + if m.firewall != nil && !m.firewall.Incognito().ShouldLearn() { + return + } + m.router.ReportOutcome(router.Outcome{ ArmID: meta.armID, TaskType: meta.taskType, diff --git a/internal/engine/engine_test.go b/internal/engine/engine_test.go index 3d0993c..7efab4f 100644 --- a/internal/engine/engine_test.go +++ b/internal/engine/engine_test.go @@ -11,6 +11,7 @@ import ( "somegit.dev/Owlibou/gnoma/internal/message" "somegit.dev/Owlibou/gnoma/internal/provider" "somegit.dev/Owlibou/gnoma/internal/router" + "somegit.dev/Owlibou/gnoma/internal/security" "somegit.dev/Owlibou/gnoma/internal/stream" "somegit.dev/Owlibou/gnoma/internal/tool" ) @@ -714,3 +715,48 @@ func TestSubmit_ReportsOutcomeToRouter(t *testing.T) { t.Errorf("quality score = %f, want ≥0.9 for all successful turns", score) } } + +func TestSubmit_SuppressesOutcomeWhenIncognito(t *testing.T) { + // Incognito sessions must not leave bandit signal behind. + rtr := router.New(router.Config{}) + armID := router.NewArmID("test", "mock-model") + + makeStream := func() stream.Stream { + return newEventStream(message.StopEndTurn, "mock-model", + stream.Event{Type: stream.EventTextDelta, Text: "hi"}, + stream.Event{Type: stream.EventUsage, Usage: &message.Usage{InputTokens: 10, OutputTokens: 5}}, + ) + } + mp := &mockProvider{name: "test", streams: []stream.Stream{makeStream(), makeStream()}} + rtr.RegisterArm(&router.Arm{ + ID: armID, + Provider: mp, + ModelName: "mock-model", + Capabilities: provider.Capabilities{ToolUse: true}, + }) + rtr.ForceArm(armID) + + fw := security.NewFirewall(security.FirewallConfig{ScanOutgoing: true}) + fw.Incognito().Activate() + + e, err := New(Config{ + Provider: mp, + Router: rtr, + Tools: tool.NewRegistry(), + Firewall: fw, + }) + if err != nil { + t.Fatalf("New: %v", err) + } + + for i := 0; i < 2; i++ { + if _, err := e.Submit(context.Background(), "hello", nil); err != nil { + t.Fatalf("Submit %d: %v", i, err) + } + } + + taskType := router.ClassifyTask("hello").Type + if _, hasData := rtr.QualityTracker().Quality(armID, taskType); hasData { + t.Error("quality tracker received outcomes despite incognito — gating not effective") + } +} diff --git a/internal/engine/loop.go b/internal/engine/loop.go index c520051..79205a1 100644 --- a/internal/engine/loop.go +++ b/internal/engine/loop.go @@ -82,6 +82,11 @@ func (e *Engine) runLoop(ctx context.Context, cb Callback) (*Turn, error) { if e.cfg.Router == nil || lastArmID == "" { return } + // Suppress quality feedback while incognito is active — bandit + // learning would otherwise persist signal about the session. + if e.cfg.Firewall != nil && !e.cfg.Firewall.Incognito().ShouldLearn() { + return + } e.cfg.Router.ReportOutcome(router.Outcome{ ArmID: lastArmID, TaskType: lastTaskType, diff --git a/internal/router/router.go b/internal/router/router.go index edaf9e4..4ddb400 100644 --- a/internal/router/router.go +++ b/internal/router/router.go @@ -63,12 +63,23 @@ func (r *Router) Select(task Task) RoutingDecision { r.mu.RLock() defer r.mu.RUnlock() - // If an arm is forced, use it directly + // If an arm is forced, use it directly — except when local-only + // routing is on and the forced arm isn't local. The earlier + // short-circuit silently bypassed the local-only filter, which + // broke the incognito badge's "local-only routing" promise when + // a cloud arm was pinned via --provider. Reject explicitly so the + // TUI/CLI can surface an actionable error instead. if r.forcedArm != "" { arm, ok := r.arms[r.forcedArm] if !ok { return RoutingDecision{Error: fmt.Errorf("forced arm %q not found", r.forcedArm)} } + if r.localOnly && !arm.IsLocal { + return RoutingDecision{Error: fmt.Errorf( + "forced arm %q is non-local but routing is local-only (incognito); clear the --provider pin or disable incognito", + r.forcedArm, + )} + } return RoutingDecision{Strategy: StrategySingleArm, Arm: arm} } diff --git a/internal/router/router_test.go b/internal/router/router_test.go index 9f82984..8bdee78 100644 --- a/internal/router/router_test.go +++ b/internal/router/router_test.go @@ -345,6 +345,40 @@ func TestRouter_SelectForcedNotFound(t *testing.T) { } } +func TestRouter_SelectForcedNonLocalUnderLocalOnlyErrors(t *testing.T) { + // Audit finding: --provider anthropic pins a cloud arm, then Ctrl+X + // enables local-only. Select used to short-circuit on forcedArm and + // return the cloud arm anyway, breaking the "local-only routing" + // promise the UI badge makes. Must now error out. + r := New(Config{}) + r.RegisterArm(&Arm{ID: "anthropic/sonnet", IsLocal: false, Capabilities: provider.Capabilities{ToolUse: true}}) + r.ForceArm("anthropic/sonnet") + r.SetLocalOnly(true) + + decision := r.Select(Task{Type: TaskGeneration}) + if decision.Error == nil { + t.Fatal("expected error: forced cloud arm under local-only must not select") + } + if decision.Arm != nil { + t.Errorf("decision.Arm = %v, want nil", decision.Arm) + } +} + +func TestRouter_SelectForcedLocalUnderLocalOnlyAllowed(t *testing.T) { + r := New(Config{}) + r.RegisterArm(&Arm{ID: "ollama/qwen", IsLocal: true, Capabilities: provider.Capabilities{ToolUse: true}}) + r.ForceArm("ollama/qwen") + r.SetLocalOnly(true) + + decision := r.Select(Task{Type: TaskGeneration}) + if decision.Error != nil { + t.Fatalf("forced local arm under local-only should select: %v", decision.Error) + } + if decision.Arm == nil || decision.Arm.ID != "ollama/qwen" { + t.Errorf("decision.Arm = %v, want ollama/qwen", decision.Arm) + } +} + // --- Gap A: Pool Reservations --- func TestRoutingDecision_CommitReleasesReservation(t *testing.T) { diff --git a/internal/security/incognito.go b/internal/security/incognito.go index 6827af5..4656d50 100644 --- a/internal/security/incognito.go +++ b/internal/security/incognito.go @@ -4,12 +4,15 @@ import "sync" // IncognitoMode controls privacy-sensitive behavior. // When active: no persistence, no learning, no content logging. +// +// Routing constraint (local-only) is enforced by the router, not here — +// see router.SetLocalOnly. The two states must agree, but they live on +// different types because the router owns enforcement (mutex around arm +// selection) and the firewall owns intent. TUI/CLI bootstrap is +// responsible for keeping them in sync. type IncognitoMode struct { mu sync.RWMutex active bool - - // Options - LocalOnly bool // only route to local arms when incognito } func NewIncognitoMode() *IncognitoMode { diff --git a/internal/session/store.go b/internal/session/store.go index d42792a..520da74 100644 --- a/internal/session/store.go +++ b/internal/session/store.go @@ -54,7 +54,7 @@ func (s *SessionStore) Save(snap Snapshot) error { if err != nil { return fmt.Errorf("session save: %w", err) } - if err := os.MkdirAll(dir, 0o755); err != nil { + if err := os.MkdirAll(dir, 0o700); err != nil { return fmt.Errorf("session %q: create dir: %w", snap.ID, err) } @@ -164,7 +164,7 @@ func atomicWrite(path string, v any) error { return fmt.Errorf("marshal: %w", err) } tmp := path + ".tmp" - if err := os.WriteFile(tmp, data, 0o644); err != nil { + if err := os.WriteFile(tmp, data, 0o600); err != nil { return fmt.Errorf("write tmp: %w", err) } if err := os.Rename(tmp, path); err != nil { diff --git a/internal/session/store_test.go b/internal/session/store_test.go index c237ef1..074a329 100644 --- a/internal/session/store_test.go +++ b/internal/session/store_test.go @@ -125,6 +125,37 @@ func TestSessionStore_Save_RejectsPathTraversal(t *testing.T) { } } +func TestSessionStore_Save_FilesArePrivate(t *testing.T) { + // Session files contain conversation history including raw user + // input — keep them 0o600 / 0o700 so other local users on shared + // hosts can't read them. + root := t.TempDir() + store := session.NewSessionStore(root, 3, slog.Default()) + snap := makeSnap("sess-perms", time.Now().UTC()) + if err := store.Save(snap); err != nil { + t.Fatal(err) + } + + dir := filepath.Join(root, ".gnoma", "sessions", "sess-perms") + dirInfo, err := os.Stat(dir) + if err != nil { + t.Fatalf("stat session dir: %v", err) + } + if dirInfo.Mode().Perm() != 0o700 { + t.Errorf("session dir mode = %o, want 0700", dirInfo.Mode().Perm()) + } + + for _, name := range []string{"metadata.json", "messages.json"} { + info, err := os.Stat(filepath.Join(dir, name)) + if err != nil { + t.Fatalf("stat %s: %v", name, err) + } + if info.Mode().Perm() != 0o600 { + t.Errorf("%s mode = %o, want 0600", name, info.Mode().Perm()) + } + } +} + func TestSessionStore_Prune_RemovesOldest(t *testing.T) { store := makeStore(t) // maxKeep = 3 now := time.Now().UTC() diff --git a/internal/tool/agent/coordinator_test.go b/internal/tool/agent/coordinator_test.go index 8b00e1c..67690b3 100644 --- a/internal/tool/agent/coordinator_test.go +++ b/internal/tool/agent/coordinator_test.go @@ -13,7 +13,7 @@ import ( func makeTestStore(t *testing.T) *persist.Store { t.Helper() - s := persist.New("test-coord-" + t.Name()) + s := persist.New("test-coord-"+t.Name(), nil) t.Cleanup(func() { _ = os.RemoveAll(s.Dir()) }) return s } diff --git a/internal/tool/persist/store.go b/internal/tool/persist/store.go index fa383d9..d242642 100644 --- a/internal/tool/persist/store.go +++ b/internal/tool/persist/store.go @@ -23,29 +23,50 @@ type ResultFile struct { ModTime time.Time } -// Store persists tool results to /tmp for cross-tool session sharing. -type Store struct { - dir string // /tmp/gnoma-/tool-results +// IncognitoGate is the minimal contract Store needs from +// security.IncognitoMode. Defined locally so the persist package keeps +// a stdlib-only dependency surface; *security.IncognitoMode satisfies +// this interface naturally. +// +// A nil IncognitoGate means "no gate" — Save runs unconditionally. +// A non-nil gate is consulted on every Save call (dynamic), so TUI +// runtime toggles take effect without reconstructing the Store. +type IncognitoGate interface { + ShouldPersist() bool } -// New creates a Store for the given session ID. +// Store persists tool results to /tmp for cross-tool session sharing. +type Store struct { + dir string // /tmp/gnoma-/tool-results + mode IncognitoGate // nil = always persist; non-nil = consult on Save +} + +// New creates a Store for the given session ID. Pass mode=nil for the +// pre-W2-2 behaviour (always persist when content is large enough). +// Pass fw.Incognito() to block persistence whenever incognito is active. // The directory is created on first Save. -func New(sessionID string) *Store { +func New(sessionID string, mode IncognitoGate) *Store { return &Store{ - dir: filepath.Join("/tmp", "gnoma-"+sessionID, "tool-results"), + dir: filepath.Join("/tmp", "gnoma-"+sessionID, "tool-results"), + mode: mode, } } // Dir returns the absolute path to the tool-results directory. func (s *Store) Dir() string { return s.dir } -// Save writes content to disk if len(content) >= minPersistSize. -// Returns (filePath, true) on persistence, ("", false) if content is too small. +// Save writes content to disk if len(content) >= minPersistSize and +// the configured incognito gate (if any) permits persistence. +// Returns (filePath, true) on persistence; ("", false) if content is too +// small, if incognito is active, or if a filesystem error occurred. func (s *Store) Save(toolName, callID, content string) (string, bool) { if len(content) < minPersistSize { return "", false } - if err := os.MkdirAll(s.dir, 0o755); err != nil { + if s.mode != nil && !s.mode.ShouldPersist() { + return "", false + } + if err := os.MkdirAll(s.dir, 0o700); err != nil { slog.Warn("persist: failed to create session directory", "dir", s.dir, "error", err) return "", false } @@ -54,7 +75,7 @@ func (s *Store) Save(toolName, callID, content string) (string, bool) { safeCallID := strings.NewReplacer("/", "_", "..", "_").Replace(callID) filename := safeName + "-" + safeCallID + ".txt" path := filepath.Join(s.dir, filename) - if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { slog.Warn("persist: failed to write tool result", "path", path, "error", err) return "", false } diff --git a/internal/tool/persist/store_incognito_test.go b/internal/tool/persist/store_incognito_test.go new file mode 100644 index 0000000..7b97369 --- /dev/null +++ b/internal/tool/persist/store_incognito_test.go @@ -0,0 +1,95 @@ +package persist_test + +import ( + "os" + "strings" + "testing" + + "somegit.dev/Owlibou/gnoma/internal/tool/persist" +) + +// stubMode implements the incognito-gate interface persist depends on. +type stubMode struct { + persist bool +} + +func (m *stubMode) ShouldPersist() bool { return m.persist } + +func TestStore_NilModeStillPersists(t *testing.T) { + // Existing callers that pass nil for the mode (tests, legacy paths) + // must behave exactly like the pre-W2-2 store. nil = no gate. + s := persist.New("test-nil-mode", nil) + t.Cleanup(func() { _ = os.RemoveAll(s.Dir()) }) + + content := strings.Repeat("x", 1024) + _, ok := s.Save("bash", "call-001", content) + if !ok { + t.Error("nil mode should not block persistence") + } +} + +func TestStore_IncognitoActiveSkipsSave(t *testing.T) { + mode := &stubMode{persist: false} + s := persist.New("test-incognito-active", mode) + t.Cleanup(func() { _ = os.RemoveAll(s.Dir()) }) + + content := strings.Repeat("x", 1024) + path, ok := s.Save("bash", "call-001", content) + if ok { + t.Errorf("incognito-active mode must block Save, got path %q", path) + } + if _, err := os.Stat(s.Dir()); !os.IsNotExist(err) { + t.Errorf("directory should not exist when persistence is blocked: stat err=%v", err) + } +} + +func TestStore_IncognitoInactiveStillSaves(t *testing.T) { + mode := &stubMode{persist: true} + s := persist.New("test-incognito-inactive", mode) + t.Cleanup(func() { _ = os.RemoveAll(s.Dir()) }) + + content := strings.Repeat("x", 1024) + _, ok := s.Save("bash", "call-001", content) + if !ok { + t.Error("inactive incognito mode must not block persistence") + } +} + +func TestStore_FilePermissionsAre0600(t *testing.T) { + s := persist.New("test-file-perms", nil) + t.Cleanup(func() { _ = os.RemoveAll(s.Dir()) }) + + content := strings.Repeat("x", 1024) + path, ok := s.Save("bash", "call-001", content) + if !ok { + t.Fatal("expected persistence to succeed") + } + info, err := os.Stat(path) + if err != nil { + t.Fatalf("stat persisted file: %v", err) + } + // Tool-result files contain post-redaction output but may still carry + // project context. 0o600 prevents other local users from reading + // session artefacts on multi-user hosts. + if info.Mode().Perm() != 0o600 { + t.Errorf("file perm = %o, want 0600", info.Mode().Perm()) + } +} + +func TestStore_DirPermissionsAre0700(t *testing.T) { + s := persist.New("test-dir-perms", nil) + t.Cleanup(func() { _ = os.RemoveAll(s.Dir()) }) + + // Trigger directory creation. + content := strings.Repeat("x", 1024) + if _, ok := s.Save("bash", "call-001", content); !ok { + t.Fatal("expected persistence to succeed") + } + info, err := os.Stat(s.Dir()) + if err != nil { + t.Fatalf("stat session dir: %v", err) + } + if info.Mode().Perm() != 0o700 { + t.Errorf("dir perm = %o, want 0700", info.Mode().Perm()) + } +} diff --git a/internal/tool/persist/store_test.go b/internal/tool/persist/store_test.go index 87cf56f..1c4807b 100644 --- a/internal/tool/persist/store_test.go +++ b/internal/tool/persist/store_test.go @@ -10,7 +10,7 @@ import ( ) func TestStore_SaveSkipsSmallContent(t *testing.T) { - s := persist.New("test-session-001") + s := persist.New("test-session-001", nil) t.Cleanup(func() { _ = os.RemoveAll(s.Dir()) }) path, ok := s.Save("bash", "call-001", "small output") @@ -23,7 +23,7 @@ func TestStore_SaveSkipsSmallContent(t *testing.T) { } func TestStore_SavePersistsLargeContent(t *testing.T) { - s := persist.New("test-session-002") + s := persist.New("test-session-002", nil) t.Cleanup(func() { _ = os.RemoveAll(s.Dir()) }) content := strings.Repeat("x", 1024) @@ -44,7 +44,7 @@ func TestStore_SavePersistsLargeContent(t *testing.T) { } func TestStore_ListFilters(t *testing.T) { - s := persist.New("test-session-003") + s := persist.New("test-session-003", nil) t.Cleanup(func() { _ = os.RemoveAll(s.Dir()) }) bigContent := strings.Repeat("y", 1024) @@ -70,7 +70,7 @@ func TestStore_ListFilters(t *testing.T) { } func TestStore_ReadValidatesPath(t *testing.T) { - s := persist.New("test-session-004") + s := persist.New("test-session-004", nil) t.Cleanup(func() { _ = os.RemoveAll(s.Dir()) }) // Path outside session dir must be rejected diff --git a/internal/tui/app.go b/internal/tui/app.go index 171bad2..9e3e8dc 100644 --- a/internal/tui/app.go +++ b/internal/tui/app.go @@ -198,10 +198,19 @@ func New(sess session.Session, cfg Config) Model { glamour.WithWordWrap(74), ) + // Seed incognito state from the firewall so a launch with + // --incognito starts the TUI with the badge ON, and Ctrl+X first- + // press correctly toggles OFF (audit finding W2-3). + var initialIncognito bool + if cfg.Firewall != nil { + initialIncognito = cfg.Firewall.Incognito().Active() + } + return Model{ session: sess, config: cfg, input: ti, + incognito: initialIncognito, completionSrc: completionSource(cfg.Skills), mdRenderer: mdRenderer, elfStates: make(map[string]*elf.Progress), @@ -463,21 +472,17 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { switch msg.String() { case "ctrl+x": // Toggle incognito - if m.config.Firewall != nil { - m.incognito = m.config.Firewall.Incognito().Toggle() - if m.config.Router != nil { - m.config.Router.SetLocalOnly(m.incognito) - } - var msg string - if m.incognito { - msg = "🔒 incognito ON — no persistence, no learning, local-only routing" - } else { - msg = "🔓 incognito OFF" - } - m.messages = append(m.messages, chatMessage{role: "system", content: msg}) - m.injectSystemContext(msg) - m.scrollOffset = 0 + newM, statusMsg, refused := m.attemptIncognitoToggle() + m = newM + role := "system" + if refused { + role = "error" } + m.messages = append(m.messages, chatMessage{role: role, content: statusMsg}) + if !refused { + m.injectSystemContext(statusMsg) + } + m.scrollOffset = 0 return m, nil case "shift+tab": // Cycle permission mode: bypass → default → plan → bypass @@ -1027,22 +1032,13 @@ func (m Model) handleCommand(cmd string) (tea.Model, tea.Cmd) { return m, nil case "/incognito": - if m.config.Firewall != nil { - m.incognito = m.config.Firewall.Incognito().Toggle() - if m.config.Router != nil { - m.config.Router.SetLocalOnly(m.incognito) - } - if m.incognito { - m.messages = append(m.messages, chatMessage{role: "system", - content: "🔒 incognito mode ON — no persistence, no learning, local-only routing"}) - } else { - m.messages = append(m.messages, chatMessage{role: "system", - content: "🔓 incognito mode OFF"}) - } - } else { - m.messages = append(m.messages, chatMessage{role: "error", - content: "firewall not configured"}) + newM, statusMsg, refused := m.attemptIncognitoToggle() + m = newM + role := "system" + if refused { + role = "error" } + m.messages = append(m.messages, chatMessage{role: role, content: statusMsg}) return m, nil case "/model": @@ -1602,6 +1598,42 @@ func (m Model) injectSystemContext(text string) { m.config.Engine.InjectMessage(message.NewAssistantText("Understood.")) } +// attemptIncognitoToggle flips incognito state subject to the local-only +// constraint: if a non-local arm is currently forced, turning incognito +// ON is refused with an actionable message. Returns the new model, a +// user-facing status string, and whether the toggle was refused. +// +// The firewall (intent) and the router's local-only flag (enforcement) +// are toggled together — they must agree, otherwise the incognito badge +// lies about routing. See plan W2-1. +func (m Model) attemptIncognitoToggle() (Model, string, bool) { + if m.config.Firewall == nil { + return m, "firewall not configured", true + } + currentlyOn := m.config.Firewall.Incognito().Active() + if !currentlyOn && m.config.Router != nil { + if forced := m.config.Router.ForcedArm(); forced != "" { + if arm, ok := m.config.Router.LookupArm(forced); ok && !arm.IsLocal { + return m, fmt.Sprintf( + "⚠ cannot enable incognito: --provider %s is non-local; clear the pin first", + forced, + ), true + } + } + } + m.incognito = m.config.Firewall.Incognito().Toggle() + if m.config.Router != nil { + m.config.Router.SetLocalOnly(m.incognito) + } + var status string + if m.incognito { + status = "🔒 incognito ON — no persistence, no learning, local-only routing" + } else { + status = "🔓 incognito OFF" + } + return m, status, false +} + // updateInputHeight recalculates and sets the textarea viewport height based on // isKnownModel returns true if modelName matches a ModelName in the provided arms slice. func isKnownModel(arms []*router.Arm, modelName string) bool { @@ -1783,13 +1815,9 @@ func (m Model) applyConfigSetting() Model { } m.config.Permissions.SetMode(next) - case 2: // Incognito — toggle - if m.config.Firewall != nil { - m.incognito = m.config.Firewall.Incognito().Toggle() - if m.config.Router != nil { - m.config.Router.SetLocalOnly(m.incognito) - } - } + case 2: // Incognito — toggle (silent; config panel has no status line) + newM, _, _ := m.attemptIncognitoToggle() + m = newM } return m } diff --git a/internal/tui/incognito_test.go b/internal/tui/incognito_test.go new file mode 100644 index 0000000..4956010 --- /dev/null +++ b/internal/tui/incognito_test.go @@ -0,0 +1,159 @@ +package tui + +import ( + "strings" + "testing" + + "somegit.dev/Owlibou/gnoma/internal/provider" + "somegit.dev/Owlibou/gnoma/internal/router" + "somegit.dev/Owlibou/gnoma/internal/security" +) + +func newToggleTestModel(rtr *router.Router, fw *security.Firewall) Model { + return Model{ + config: Config{ + Firewall: fw, + Router: rtr, + }, + } +} + +func TestAttemptIncognitoToggle_NilFirewallReturnsRefused(t *testing.T) { + m := newToggleTestModel(nil, nil) + _, status, refused := m.attemptIncognitoToggle() + if !refused { + t.Error("expected refused=true when firewall is nil") + } + if !strings.Contains(status, "firewall") { + t.Errorf("status = %q, want mention of firewall", status) + } +} + +func TestAttemptIncognitoToggle_NoForcedArmFlipsOn(t *testing.T) { + rtr := router.New(router.Config{}) + fw := security.NewFirewall(security.FirewallConfig{ScanOutgoing: true}) + m := newToggleTestModel(rtr, fw) + + newM, status, refused := m.attemptIncognitoToggle() + if refused { + t.Fatalf("expected refused=false, got refused; status=%q", status) + } + if !newM.incognito { + t.Error("expected newM.incognito = true after toggle") + } + if !fw.Incognito().Active() { + t.Error("firewall incognito should be active after toggle") + } + if !rtr.LocalOnly() { + t.Error("router localOnly should be true after toggle") + } + if !strings.Contains(status, "incognito ON") { + t.Errorf("status = %q, want incognito ON marker", status) + } +} + +func TestAttemptIncognitoToggle_ForcedLocalArmAllowed(t *testing.T) { + rtr := router.New(router.Config{}) + rtr.RegisterArm(&router.Arm{ + ID: router.NewArmID("ollama", "qwen"), + IsLocal: true, + Capabilities: provider.Capabilities{ToolUse: true}, + }) + rtr.ForceArm(router.NewArmID("ollama", "qwen")) + + fw := security.NewFirewall(security.FirewallConfig{ScanOutgoing: true}) + m := newToggleTestModel(rtr, fw) + + _, _, refused := m.attemptIncognitoToggle() + if refused { + t.Error("forced LOCAL arm + incognito should NOT be refused") + } +} + +func TestAttemptIncognitoToggle_ForcedCloudArmRefused(t *testing.T) { + rtr := router.New(router.Config{}) + rtr.RegisterArm(&router.Arm{ + ID: router.NewArmID("anthropic", "sonnet"), + IsLocal: false, + Capabilities: provider.Capabilities{ToolUse: true}, + }) + rtr.ForceArm(router.NewArmID("anthropic", "sonnet")) + + fw := security.NewFirewall(security.FirewallConfig{ScanOutgoing: true}) + m := newToggleTestModel(rtr, fw) + + _, status, refused := m.attemptIncognitoToggle() + if !refused { + t.Fatalf("forced CLOUD arm + incognito should be refused; status=%q", status) + } + if fw.Incognito().Active() { + t.Error("firewall must NOT activate when toggle is refused") + } + if rtr.LocalOnly() { + t.Error("router localOnly must NOT flip when toggle is refused") + } + if !strings.Contains(status, "non-local") && !strings.Contains(status, "pin") { + t.Errorf("status should explain the refusal; got %q", status) + } +} + +func TestNew_SeedsIncognitoFromActiveFirewall(t *testing.T) { + fw := security.NewFirewall(security.FirewallConfig{ScanOutgoing: true}) + fw.Incognito().Activate() + + m := New(nil, Config{Firewall: fw}) + if !m.incognito { + t.Error("New() should seed m.incognito=true when firewall already active") + } +} + +func TestNew_SeedsIncognitoFalseWhenFirewallInactive(t *testing.T) { + fw := security.NewFirewall(security.FirewallConfig{ScanOutgoing: true}) + + m := New(nil, Config{Firewall: fw}) + if m.incognito { + t.Error("New() should seed m.incognito=false when firewall inactive") + } +} + +func TestNew_SeedsIncognitoFalseWhenNoFirewall(t *testing.T) { + m := New(nil, Config{}) + if m.incognito { + t.Error("New() should seed m.incognito=false when no firewall") + } +} + +func TestAttemptIncognitoToggle_TurningOffNotBlockedByForcedCloud(t *testing.T) { + // Once incognito is ON, the user must always be able to turn it OFF + // regardless of the forced-arm state. Otherwise they're trapped. + rtr := router.New(router.Config{}) + rtr.RegisterArm(&router.Arm{ + ID: router.NewArmID("anthropic", "sonnet"), + IsLocal: false, + Capabilities: provider.Capabilities{ToolUse: true}, + }) + // Note: not forcing the arm yet — start incognito on a clean state, + // then pretend a forced cloud arm appears (which shouldn't happen in + // practice, but the toggle-off path must be robust). + fw := security.NewFirewall(security.FirewallConfig{ScanOutgoing: true}) + fw.Incognito().Activate() + rtr.SetLocalOnly(true) + rtr.ForceArm(router.NewArmID("anthropic", "sonnet")) + + m := newToggleTestModel(rtr, fw) + m.incognito = true + + newM, _, refused := m.attemptIncognitoToggle() + if refused { + t.Fatal("turning incognito OFF must never be refused") + } + if newM.incognito { + t.Error("incognito should be false after toggle-off") + } + if fw.Incognito().Active() { + t.Error("firewall incognito should be off") + } + if rtr.LocalOnly() { + t.Error("router localOnly should be off") + } +}