fix(cli): apply sandbox settings to existing sessions - #12600
fix(cli): apply sandbox settings to existing sessions#12600marius-kilocode wants to merge 7 commits into
Conversation
| Each initialized session snapshots its network mode, allowed destinations, and additional writable paths. Changing config affects new sessions. The prompt control or `/sandbox` can change the current session's enabled state, but it cannot change these authority lists, and they never expand during an active session. | ||
|
|
||
| {% callout type="info" %} | ||
| Restarting VS Code or the extension does not refresh an existing session's sandbox policy. After changing network mode, allowed destinations, or writable paths, start a new session to pick up the updated settings. |
There was a problem hiding this comment.
I thought disabling/re-enabling also refreshes it?
Code Review SummaryStatus: No Issues Found | Recommendation: Merge OverviewIncremental diff since the last review ( The two previously-flagged WARNINGs ( Files Reviewed (1 file)
Previous Review Summaries (5 snapshots, latest commit 1f53963)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 1f53963)Status: No Issues Found | Recommendation: Merge OverviewIncremental diff since the last review ( The two previously-flagged WARNINGs ( Files Reviewed (1 file)
Previous review (commit 17af2a8)Status: No Issues Found | Recommendation: Merge OverviewBoth previously flagged WARNINGs remain resolved at current HEAD:
The rest of the diff (docs wording update describing the new live-refresh timing, i18n string cleanup removing the now-stale "changes apply to new sessions" caveat across 21 locales, a Files Reviewed (33 files)
Previous review (commit 33f93b2)Status: No Issues Found | Recommendation: Merge OverviewBoth previously flagged WARNINGs are resolved in this update:
The remaining diff (docs wording tweak, removal of the now-redundant Files Reviewed (27 files, incremental since 5b37f18)
Previous review (commit 5b37f18)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (33 files, incremental since 700b539)
This commit does two things: (1) makes sandbox network/writable-path/allowed-host settings refresh existing sessions from trusted global config instead of only affecting new sessions (resolves the previously-flagged doc question about disable/re-enable refresh), and (2) strips Fix these issues in Kilo Cloud Previous review (commit 700b539)Status: No Issues Found | Recommendation: Merge Files Reviewed (24 files)
All 20 locale dictionaries were updated consistently (new Reviewed by claude-sonnet-5 · Input: 42 · Output: 10.6K · Cached: 1.2M Review guidance: REVIEW.md from base branch |
marius-kilocode
changed the title
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| ), | ||
| ) | ||
| delete env.KILO_SERVER_PASSWORD | ||
| delete env.KILO_SERVER_USERNAME |
There was a problem hiding this comment.
WARNING: model() only strips KILO_SERVER_PASSWORD/KILO_SERVER_USERNAME, not KILO_CONFIG_CONTENT
This helper is now used to build the env for formatters (format/index.ts), LSP servers (lsp/launch.ts), MCP servers (mcp/index.ts), and model shell commands (tool/shell.ts) — all non-sandboxed child processes. The sandbox's own environment.deny list in policy.ts treats KILO_CONFIG, KILO_CONFIG_CONTENT, and KILO_CONFIG_DIR as equally sensitive alongside the server credentials, since KILO_CONFIG_CONTENT can carry inline config (including provider apiKey values, see config-injector.ts).
As written, any formatter, LSP server, MCP server, or model-run shell command will still inherit KILO_CONFIG_CONTENT/KILO_CONFIG/KILO_CONFIG_DIR from the parent process even after this fix, leaving a similar credential-leak path open for that variable.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| if (event.payload?.type === "global.config.updated") revision++ | ||
| }) | ||
|
|
||
| function externalConfig() { |
There was a problem hiding this comment.
WARNING: Raw file polling widens the trust boundary for mid-session policy refresh
externalConfig() re-reads configFiles under Global.Path.config every second and bumps revision whenever their raw content changes, independent of the global.config.updated GlobalBus event that the trusted /config/overlay route emits (per the new config-overlay.test.ts case). That means any process with filesystem write access to the global config directory — not just the settings API — can now cause every active sandboxed session across all directories to pick up new network/writable-path limits before its next tool call, per current().
The dedicated test does not let project config weaken an initialized policy shows project-level config intentionally can't do this, which suggests the mid-session widening is meant to be scoped to genuinely trusted, user-initiated global settings changes. Given the PR's own stated invariant ("a confined agent must not be able to widen its own escape hatch mid-session"), it's worth confirming that nothing with a plausible write path to Global.Path.config (e.g. an agent that briefly runs unsandboxed, or a sibling session sharing the same backend) can use this raw-file channel to widen confinement for other, already-running sessions before their next tool execution.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
33f93b2 to
17af2a8
Compare
43ed4b2 to
1f53963
Compare
2 participants
Sandbox network mode, allowed destinations, and writable paths were previously fixed when a session initialized. Saving a corrected path or network policy left that session on stale authority, so users had to abandon otherwise valid conversations and start over.
The user workflow is now: change the sandbox settings, click Save, and continue in the same session. The next tool execution automatically uses the updated network mode, allowed destinations, and writable paths. No new session, VS Code restart, extension restart, or sandbox toggle is required.
If sandboxing is disabled, changing settings and then enabling it also uses the latest settings.
This change applies trusted sandbox setting changes to existing sessions before their next tool execution:
sandbox.enabledvalue remains the default for new sessions.Creation-time inheritance still gives child sessions the stricter parent/child policy. Later trusted user settings intentionally replace those network and writable-path limits across existing sessions.
To keep this authority update user-controlled, model-launched shell, local MCP, formatter, and language-server processes do not inherit backend server credentials or Kilo config environment variables. They cannot use authenticated configuration endpoints or injected config content to change their own sandbox policy.
The sandboxing documentation describes the live behavior and timing.