chore: land PRs #139/#148/#149 with original authorship - #156
Merged
Conversation
Three JSON-writing paths computed
ensure_ascii = not output_config.get("ensure_ascii", False)
and passed it to json.dump. The stray `not` inverts the configured flag: the
sibling option on the line above (json_indent) is a direct pass-through, the
config field name equals the json.dump parameter, and the shipped default in
indexer_config.yaml is `ensure_ascii: false`. So with the default config the
code sent ensure_ascii=True, escaping every non-ASCII char (Chinese text,
accented identifiers) as \uXXXX in the index/statistics/summary JSON. Drop
the `not` so the flag is passed through as configured.
Covers the three JSON-writing paths fixed in the preceding commit. Without that fix 7 of these 8 tests fail, so the inversion cannot silently return. Also pins the default runtime path: CodeIndexer does not load the shipped tools/indexer_config.yaml unless indexer_config_path is passed, so the in-code False fallback — not the YAML — is what most callers hit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Native Windows previously reported backend 'none', so shell and code-mode commands ran without any isolation. This adds a real Windows isolation primitive — a Job Object with JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE: - When the wrapper process exits (normally or killed), the entire child process tree is terminated: no orphaned shells/compilers survive. - The inner command is spawned suspended (CreateProcessW), assigned to the job, then resumed; every descendant joins the same job automatically (Windows jobs are hierarchical). - Exit code is forwarded so callers see the inner command's result. Wiring mirrors the seatbelt/bwrap backends: sandbox_backend() returns 'job' on Windows and wrap_argv_command prefixes the inner argv with 'python -m core.harness.windows_sandbox --'. Pure ctypes, no third-party dependency; degrades to a subprocess passthrough if any Win32 call fails. Honest boundary documented in the module: this provides process-tree isolation and lifetime guarantees, not a filesystem write-fence (which on Windows requires disk quotas or AppContainer/SILO privileges). Tests: exit-code forwarding, wrapper wiring via build_exec_command, and a KILL_ON_JOB_CLOSE descendant-kill smoke test (Windows only). seatbelt profile tests are now correctly skipped off-Darwin.
On Windows, POSIX mode bits are ignored, so credentials.json and other private state inherited the permissive profile ACL (Authenticated Users typically gets Modify). An attacker with a foothold could read API keys from a file that POSIX would have kept at 0600. Remove inheritance and grant the current user exclusive access via icacls, matching the 0700/0600 intent on NTFS. Best-effort: failures are swallowed like POSIX chmod. Rebased onto main, which gained open_existing_private_file() after this patch was written. The three-way merge attached the ACL call to that new function instead of open_private_file(); both now restrict the ACL, which matches the POSIX path where each repairs the mode on its own descriptor. Co-authored-by: raymondginger <raymondginger2018@gmail.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The windows-2022 job ran four named lifecycle files; the ubuntu job runs everything but can only skip Windows-gated cases. So the NTFS ACL and Job Object tests added alongside had no enforcement point anywhere — six cases that never executed in CI. Point the Windows job at them too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The workspace log line hardcoded "(writes fenced to workspace)" regardless of backend. With the Job Object backend now returning "job" on Windows, it would report process-tree isolation and a write-fence in the same breath — but that backend deliberately leaves the filesystem open. The "none" case was already mislabelled the same way. Derive the claim from the backend via fences_writes() instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ruff-format on the file introduced by the Job Object backend. Kept separate so the cherry-picked commit stays byte-identical to what was submitted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 6, 2026
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
3 participants
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.
Description
Lands the three open PRs that still apply to
mainafter the v2.0 refactor, with original authorship preserved via cherry-pick.8f9999c9e4b2c8576ed69The remaining commits are follow-on work found while verifying the above.
Changes
#139 —
ensure_asciiconfig was inverted (tools/code_indexer.py)Three JSON-writing paths computed
ensure_ascii = not output_config.get("ensure_ascii", False).tools/indexer_config.yamlshipsensure_ascii: false, andcodebase_index_workflow.load_or_create_indexer_config()loads it — so every index/statistics/summary JSON escaped non-ASCII to\uXXXXagainst the config's explicit intent. Added 8 regression tests pinning the direction; reverting the fix fails 7 of them.#149 — Windows had no sandbox backend (
core/harness/sandbox.py,core/harness/windows_sandbox.py)sandbox_backend()returned"none"on Windows, andwrap_argv_command()returned the argv unwrapped — socore/harness/tools/shell.pyandcore/harness/code_mode/tool.pyran with no isolation. Adds a Job Object backend withJOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSEfor process-tree isolation. It does not fake a write-fence; that boundary is documented.#148 — Windows private files inherited the profile ACL (
core/private_storage.py)Every Windows path in this module was a no-op, so
credentials.jsonand other private state inherited the profile ACL (Authenticated Userstypically gets Modify) instead of the POSIX0600equivalent. Now removes inheritance and grants the current user exclusive access viaicacls, best-effort like POSIXchmod.Rebase note:
maingainedopen_existing_private_file()after this patch was written, and the three-way merge attached the ACL call to that new function instead ofopen_private_file(). Both now restrict the ACL, matching the POSIX path where each repairs the mode on its own descriptor.CI — the Windows-gated suites never ran
windows-lifecycleran four named lifecycle files; the ubuntu job runs everything but can only skip Windows-gated cases. The NTFS ACL and Job Object tests had no enforcement point anywhere — six cases that never executed. The Windows job now runs them.Sandbox log — claimed a write-fence the backend does not provide
tools/code_implementation_server.pyhardcoded"(writes fenced to workspace)"regardless of backend. Derived fromfences_writes()instead.Testing
mainbaseline: 1119 passed)pre-commit run --from-ref origin/main --to-ref HEAD— clean, includingactionlintNot verified locally: the Windows ACL behaviour — all three of its cases skip off Windows. The
windows-2022job added here is their first real execution.Related Issues
Supersedes #139, #148, #149 — cherry-picked, so the SHAs differ and GitHub will not auto-close them. They should be closed manually referencing the commits above.