feat(cli): persist /sandbox toggle across new sessions - #11912
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| if (!status.enabled && !status.available) return status | ||
| const next: Snapshot = { ...current, enabled: !status.enabled, version: status.version + 1 } | ||
| yield* Effect.promise(() => SandboxStore.write(directory, sessionID, next)) | ||
| yield* Effect.promise(() => SandboxPreference.write(directory, next.enabled)) |
There was a problem hiding this comment.
WARNING: New preference write can desync the in-memory snapshot cache from the persisted store on failure
snapshots.set(key(directory, sessionID), next) on line 211 only runs after this SandboxPreference.write call resolves. If the preference write throws (disk full, permission error, etc.) after the preceding SandboxStore.write(directory, sessionID, next) on line 209 already succeeded, change() fails and the cache update is skipped — but the session's persisted SandboxStore snapshot has already been updated to the new toggled value. Any subsequent read(directory, sessionID) call first checks snapshots.get(id) (policy.ts:143) and returns the stale cached value without re-reading disk, so the in-process state and the on-disk SandboxStore state can diverge for the remainder of the process's life, even though the toggle appeared to fail to the caller.
Consider updating snapshots.set(...) before the (best-effort) preference write, or treating a SandboxPreference.write failure as non-fatal (log and continue) so it can't leave the primary session snapshot cache out of sync with what was already durably persisted.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge The previously flagged WARNING (preference write could desync the in-memory snapshot cache from the persisted store) is fixed: Files Reviewed (1 file)
Previous Review Summary (commit d94328a)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit d94328a)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (5 files)
All changes are confined to Kilo-owned paths ( Reviewed by claude-sonnet-5-20260630 · Input: 32 · Output: 5.9K · Cached: 654.2K Review guidance: REVIEW.md from base branch |
The CLI /sandbox toggle was session-scoped: each new session reset to the static config default (or secure()-forced ON in authless mode), while the VS Code extension's sandbox button persisted the last choice. Add a per-directory persisted preference (SandboxPreference) that new sessions resolve after create-time metadata and before the config default, so /sandbox now remembers the last toggled state per project. secure()-by-default still applies when neither an explicit choice nor a preference exists. Add SandboxPreference.root to the sandbox denyWrite list so a sandboxed process cannot plant a false preference to disable confinement for later sessions.
d94328a to
1f80fdf
Compare
feat(cli): persist /sandbox toggle across new sessions
2 participants
What
The CLI
/sandboxtoggle was session-scoped: each new session reset to the staticexperimental.sandboxconfig default (orsecure()-forced ON in authless mode whereKILO_SERVER_PASSWORDis unset). The VS Code extension's sandbox button, by contrast, persists the last choice viakilo.sandbox.newSessionDefaultworkspace state, so new sessions inherit it. This PR makes the CLI match VS Code.Why
Users who toggle the sandbox off (or on) for a project expect that choice to stick the next time they start a session in the same directory. Forgetting the toggle on every new session is surprising and inconsistent with the extension, where the same control is persistent. The only existing "persistent" CLI option was the static config flag, which is a fixed default rather than "remember what I last chose."
How it works
A new per-directory persisted store (
SandboxPreference) records the last toggled boolean under~/.local/state/kilo-sandbox-preference/<sha256(directory)>.json(mirroring the existingSandboxStorelayout).SandboxPolicyresolves a new session's sandbox state with precedence:kilocode.sandboxsession metadata (existing)secure()(authless force-ON) only applying when neither explicit choice nor preference existstoggle()persists the new state to the per-directory preference on every toggle. Secure-by-default is preserved for directories with no prior toggle (the very first session still starts sandboxed in authless mode).Security tradeoff
In authless (local TUI) mode, once a user toggles the sandbox off, new sessions in that project stay off until toggled back, since
secure()no longer overrides an existing preference. This weakens the secure-by-default guarantee but matches VS Code's behavior, and the new preference store is protected by the sandbox profile'sdenyWritelist so a sandboxed process cannot plant afalsepreference to disable confinement for later sessions.Tests
Rewrote the two tests that encoded the old per-session isolation property to assert the new persistence, plus a round-trip test (toggle off, then on, new session inherits on). A third pre-existing test that exercises
retirewas adjusted sinceretirenow re-seeds from the directory preference. Added adenyWriteassertion forSandboxPreference.rootinpolicy.test.ts. All 64 sandbox tests pass.