Repository navigation
fix(tui): apply per-call execution policy to Python and JavaScript tools - #6820
Merged
Hmbown merged 3 commits intoOct 3, 2026
Merged
Conversation
Request a maintainer decision on removing duplicate Python/JavaScript entrypoints before extending their permission handling. Document the current policy-dispatch gap, existing shared launcher, and platform limitations. Documentation only; no runtime implementation or confinement tests.
Pass effective tool context to Python and Node and reuse sandboxed_runner_command. Extend exact-call sandbox elevation to both tools, deliver source on stdin, preserve Node proxy environment, and document shared-launcher platform limits. Windows verification: code_execution 7/0, js_execution 8/0, interpreter execution policy 2/0, sandbox escalation 4/0; standalone feature registry source 6/0. Bypassing the shared launcher makes both policy regressions fail (0 passed, 2 failed); restoring the fix passes 2/0. Formatting and diff checks passed. No native Linux/macOS confinement or full workspace-suite claim.
Replace the RFC-only proposal in the final PR diff with the verified interpreter-policy implementation. Preserve published ancestry without rewriting history; the resulting tree matches the already tested repair commit.
Guan0923
marked this pull request as ready for review
October 3, 2026 12:17
Hmbown
pushed a commit
that referenced
this pull request
Oct 4, 2026
Windows Native hosts: on 02fcc3e the LPAC token check and the data-directory share conflict were fixed, and the isolation probe ran inside the LPAC for the first time. Node then aborted at startup (WSAStartup 10107): an LPAC cannot open any registry key without the registryRead capability, and Winsock reads its catalog from the registry. Native hosts now receive exactly that one capability (derived by name), and the token check requires exactly it; it grants no file, network or COM access, and socket creation stays denied, which the probe checks. Bun failed opening the NUL device (EPERM) for --config=NUL: the plain Bun host now reads an empty config from the granted runtime-copy directory, and the Windows compiled host relies on --no-compile-autoload-bunfig instead of baking --config=NUL. Unprotected file ACL edits now pass an explicit-only DACL to SetSecurityInfo with UNPROTECTED_DACL_SECURITY_INFORMATION so Windows re-derives inherited entries from the parent (a file has no children to propagate to); the kernel write had turned a file's inherited entries explicit. ACL pointer reads use checked NonNull access (CodeQL rust/access-invalid-pointer on 02fcc3e). Windows atomic writes: Fleet's WorkspaceFile/WindowsDirectory pins, new in 0.10.1, keep every ancestor directory of a Fleet file open with read-only sharing. MoveFileExW (tempfile::persist) reopens the destination directory with FILE_ADD_FILE, so while a Fleet ledger is alive (a whole `codewhale fleet` run, or a Runtime API request) Codewhale's own atomic writes into the workspace root or its ancestors failed with os error 32. That includes file-tool edits; hosted Windows showed it as a bare 500 writing mcp.json. On Windows, write_atomic now creates its temp with DELETE access and renames through that handle with a bare file name (the rename Fleet publication already uses), so the parent is never reopened and no pin is relaxed. Names Windows would rewrite (trailing dot or space, device names) are refused before any temp exists, and every failure path closes the delete-denying handle before removing the temp. MCP config write errors now name the file. A Windows test writes and replaces a file beside live Fleet pins and checks refused names leave no temp behind. Wire JSON: the extension-host `isJson` guard used Array.every and Object.values, so it accepted sparse arrays, extra array properties, accessors and symbol keys that serialize differently from what was checked (Copilot review on #6815). It now accepts only dense arrays and plain objects whose own keys are enumerable string data properties, the rule storage.ts already enforced; storage's duplicate check is removed. A new test fails on the old guard and passes on the new one. Bundles that include the guard were rebuilt with the pinned esbuild; the harness builtin digest pin in tier.rs moves with it. Release notes: credits and entries for #6820, #6829, #6830 and #6831, which landed in 0.10.1 without CHANGELOG coverage, and for #6827 (Windows node.exe kills are held by the built-in safety floor even in Full Access), crediting @jayanthvee in the CHANGELOG, docs/CONTRIBUTORS.md, website credits and public-surface facts. Fixes #6827 Source15 verification (3,792 rows; owning HEAD cf03333 and index 5983aa54 unchanged): - npm test and npm run check:web: 1,185 passed / 0 failed / 7 explicit skips (the website credit-parity test caught the first draft missing the new reporter credit); extension-host suite 390 tests, 383 passed, 0 failed, 7 skipped. - macOS, compiled host rebuilt from the new bundle: extension_host 177/0, plugins::install::dsh 28/0, mcp 407/0, utils 38/0, tools::file 190/0, runtime_store_convergence 14/0, builtin digest table 2/0. - Windows: cargo check --target x86_64-pc-windows-gnu of codewhale-tui lib and tests clean under deny(warnings); write_atomic change reviewed adversarially (FIX_FIRST findings applied: early-error temp cleanup, single retry layer, rewritten-name refusal). The LPAC capability, Bun config and file-ACL changes are type-checked only; their runtime proof is this head's hosted Windows run. Hosted 02fcc3e: Safety, Lint, npm wrapper smoke and Test (ubuntu-latest) passed. Windows 17,952 passed / 91 failed: LPAC runtime startup (Node WSAStartup, Bun NUL config), one file-ACL hygiene test, the mcp.json write conflict and one DSH HTTP case behind the probe, all addressed above. CodeQL flagged five pointer reads, addressed above. Not proven here: Windows runtime of these repairs. This is source integration, not release, deploy, provider or install approval. Evidence: artifacts/gpt61-0101-takeover-20261002/parent-source15-application, parent-source15-local, parent-source15-npm-web-actual, windows-gnu-check. Signed-off-by: CodeWhale Bot <bot@codewhale.net> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
Keep
code_executionandjs_execution, but route their interpreter processes through the existing permission-aware launcher.Previously, interpreter dispatch passed only the tool input and workspace path to the Python and Node runners. After approval, both runners launched local interpreters directly without applying the effective session sandbox policy.
A working directory is not filesystem confinement. Environment sanitization, timeouts, and process-tree cleanup are useful protections, but they do not enforce filesystem or network restrictions.
This change connects both tools to the existing execution-policy mechanism. It does not introduce another sandbox implementation or require ordinary interpreter calls to be rewritten as Shell calls.
Explicitly authorized wider execution remains supported: ordinary approval retains the current policy, while a separately approved elevation applies only to the corresponding call.
Changes
ToolContext, including per-call policy overrides, to both interpreter runners.sandboxed_runner_command.sandbox_permissionsandjustificationfields to both tool schemas./tmp.Authorization behavior
The two decisions remain distinct:
The tools retain their names and their required
codeinput. Callers that need a wider policy can request it through the same interpreter tool instead of switching to Shell solely to obtain approval.Scope and known limitations
workspace-writeretains the existing unenforced behavior. This change does not add Windows filesystem or network isolation.How I tested it
Local verification was performed on Windows.
Focused TUI tests
7 passed; 0 failed2 passed; 0 failed8 passed; 0 failed4 passed; 0 failedThese are 21 distinct passing TUI tests, not a full-suite run.
Cargo invocations included:
The
js_executionandsandbox_escalationgroups were run directly from the compiled TUI unit-test binary.The runtime regressions cover both interpreter entrypoints:
The existing engine approval journeys were extended to cover Python and JavaScript, including approval before execution and refusal in postures that cannot provide the required human decision.
Negative control
Temporarily bypassed the shared launcher and restored direct local command construction while keeping the same source-delivery path.
Both permission regressions failed:
After restoring the fixed sources and rebuilding, both passed again:
Feature registry and formatting
6 passed; 0 failed.cargo fmt --all -- --checkpassed.git diff --checkpassed.Not run locally:
Issue
No-Issue: This PR replaces its original design-only proposal with the implemented interpreter-policy repair; no separate issue is linked.
The original proposal commits remain in the branch history. The RFC-only document is no longer part of the final diff, which contains the verified repair instead of an unimplemented direction discussion.
Checklist
origin/mainbefore implementation.