fix(studio): resolve bare git on Windows sandbox PATH - #7323
Conversation
Sandboxed terminal tools rebuilt PATH as venv + System32 only, so user-installed Git under Program Files never resolved by bare name. Append absolute host PATH dirs after the curated prefix and inherit PATHEXT on Windows (unslothai#7317).
for more information, see https://pre-commit.ci
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4cf3265e2b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| entry = entry.strip().strip('"') | ||
| if not entry or entry in (".",) or not os.path.isabs(entry): | ||
| continue | ||
| path_entries.append(entry) |
There was a problem hiding this comment.
Keep auto-approved commands off inherited PATH
When the Studio process is launched with a user-writable absolute PATH entry (for example a venv, node_modules/.bin, or ~/bin) and the curated dirs lack an auto-safe command such as jq or rg, appending every host PATH directory lets the sandbox resolve and run that host executable. The terminal safety gate approves these commands by basename before execution, so this can turn an auto-approved read-only command into arbitrary code from the operator's PATH; restrict inheritance to trusted/tool-specific directories or gate it to the Windows Git case.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in f885a3d.
Host PATH inheritance is now gated to Windows Git install directories only (\Git\cmd, \Git\bin, \Git\mingw64\bin). User-writable absolute PATH entries (venv Scripts, node_modules/.bin, custom bins) are no longer appended, so they cannot shadow auto-safe terminal commands like jq/rg.
Linux/macOS curated PATH (/usr/bin, etc.) already covers system git, so no host inheritance there.
Tests: TestSandboxEnvIsolation (9 passed), including new test_user_writable_host_path_not_inherited.
…nslothai#7323) Only append Git-for-Windows install directories from the host PATH on Windows, instead of every absolute entry. This fixes bare `git` resolution (unslothai#7317) without letting user-writable dirs (venv, node_modules/.bin) shadow auto-safe terminal commands.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f885a3d036
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # list when present so bare tool names resolve like a normal shell. | ||
| pathext = os.environ.get("PATHEXT") | ||
| if pathext: | ||
| env["PATHEXT"] = pathext |
There was a problem hiding this comment.
Keep PATHEXT from enabling cwd command hijacks
On Windows, when the sandbox workdir already contains rg.bat/jq.cmd/another safe-command stub, inheriting the host PATHEXT lets cmd /c rg ... resolve that current-directory script before the curated PATH; Microsoft documents both PATHEXT-based extension lookup and that the current directory is searched before PATH, so an auto-approved read-only basename can become arbitrary code execution. Use a fixed minimal extension set plus NoDefaultCurrentDirectoryInExePath, or resolve trusted tool paths explicitly, instead of copying the operator's PATHEXT.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Completed in c51a7a4: sandbox env now also sets NoDefaultCurrentDirectoryInExePath=1 so cmd/CreateProcess skip cwd for bare names.
Use a fixed .EXE;.COM list instead of inheriting the host PATHEXT so cmd cannot resolve auto-approved bare names from workdir .BAT/.CMD stubs.
967a556 to
0bdf41e
Compare
|
Fixed in 0bdf41e. P2 — Keep PATHEXT from enabling cwd command hijacks: Valid. Sandbox env now pins |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0bdf41efa8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # without appending user-writable dirs (venv, node_modules/.bin) that could | ||
| # shadow auto-safe terminal commands (#7317). | ||
| _WINDOWS_GIT_PATH_RE = re.compile( | ||
| r"[\\/]git[\\/](?:cmd|bin|mingw64[\\/]bin)(?:[\\/]|$)", |
There was a problem hiding this comment.
Restrict inherited Git dirs to trusted installs
When Studio runs on Windows with a user-writable host PATH entry that merely ends in \Git\cmd or \Git\mingw64\bin (for example C:\Users\alice\scratch\Git\cmd), this suffix-only allowlist appends that directory to the sandbox PATH. If the curated dirs do not contain an auto-approved command such as rg, a malicious rg.exe in that user-controlled directory can still be selected after the terminal safety gate approves by basename. Fresh evidence relative to the earlier PATH comment is that the new allowlist still trusts any suffix-matching Git path rather than a trusted install root.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c51a7a4: the suffix regex is gone; we now append only the directory of the git the host shell resolves via shutil.which, so user-writable Git-named dirs are never trusted.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2408fad677
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| git_exe = shutil.which("git") | ||
| if git_exe: | ||
| git_dir = os.path.dirname(git_exe) | ||
| if os.path.isabs(git_dir): | ||
| path_entries.append(git_dir) |
There was a problem hiding this comment.
Preserve resolved Git wrapper extensions
When the host's working git is a .cmd or .bat wrapper/shim, shutil.which("git") succeeds using the host PATHEXT and this code appends its directory, but the sandbox then restricts PATHEXT to .EXE;.COM. The cmd /c terminal therefore cannot resolve bare git, so the Windows failure remains for those valid host setups. Either preserve the extension needed by the resolved executable or explicitly handle non-EXE Git launchers.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d96d12c: the resolved git launcher's extension (e.g. .CMD) is appended to the restricted PATHEXT; cwd lookup stays disabled via NoDefaultCurrentDirectoryInExePath.
|
Ready for review — undrafted after @danielhanchen's commits on ``. Maintainer changes incorporated; awaiting CI + review. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2408fad677
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if git_exe: | ||
| git_dir = os.path.dirname(git_exe) | ||
| if os.path.isabs(git_dir): | ||
| path_entries.append(git_dir) |
There was a problem hiding this comment.
Constrain the inherited Git directory to trusted installs
When Git is installed through a per-user manager such as Scoop, shutil.which("git") can resolve to a user-writable shims directory, and this appends that whole directory to the sandbox PATH. In auto mode, if the curated Windows dirs do not contain a safe basename like rg or jq, the safety gate still approves the basename but cmd can execute the copy from that inherited user-writable directory, re-opening arbitrary-code execution from outside the curated PATH. Fresh evidence relative to the earlier suffix-allowlist comment is that the current code trusts the shutil.which directory solely because it is absolute, with no trusted-root or ACL check.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
shutil.which returns the same git the operator's own shell runs. If that directory is attacker-writable, the host user's every git invocation is already compromised; the sandbox cannot meaningfully out-trust the host PATH for the tool the fix exists to expose. ACL checks here would add platform-specific complexity without changing that trust boundary.
|
@codex review |
for more information, see https://pre-commit.ci
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ad481c04b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| git_exe = shutil.which("git") | ||
| if git_exe: | ||
| git_dir = os.path.dirname(git_exe) | ||
| if os.path.isabs(git_dir): | ||
| path_entries.append(git_dir) |
There was a problem hiding this comment.
Do not trust Git resolved from arbitrary host PATH directories
shutil.which("git") can resolve a launcher from a user-writable host PATH entry (for example, the Scoop shim directory explicitly supported by the new test), but its entire directory is then added to the sandbox PATH. An attacker who can place rg.exe (or another auto-approved utility) beside that launcher can have an auto-approved command such as rg execute arbitrary code without confirmation; the restricted PATHEXT and disabled cwd lookup do not protect against an .exe already in this newly trusted directory. Restrict the inherited location to a trusted Git installation or validate that the resolved directory cannot be user-writable.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch, and correct that the auto-approve gate makes this reachable. Fixed in ccec25e1f: the inherited git directory is now accepted only when it sits under a system install root (Program Files / ProgramW6432 / SystemRoot). Per-user managers like Scoop shims are refused, so an attacker cannot drop rg.exe beside a user-writable git launcher; those users can still run git in full-access mode.
|
@codex review |
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98e6124c1e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| norm = os.path.normcase(os.path.normpath(path)) | ||
| for root in roots: | ||
| root_norm = os.path.normcase(os.path.normpath(root)) | ||
| if norm == root_norm or norm.startswith(root_norm + os.sep): |
There was a problem hiding this comment.
Limit trusted Windows roots to non-writable dirs
When the host PATH resolves git from a writable directory under %SystemRoot% such as C:\Windows\Temp, this prefix check treats that directory as trusted and appends it to the sandbox PATH. Standard users can create files in Windows\Temp, so a sibling rg.exe/jq.exe there can still be selected after the auto-approval gate approves the basename; the fresh case here is the recursive trust of all %SystemRoot% subdirectories rather than arbitrary user PATH suffixes. Restrict this to specific non-user-writable install locations or explicitly exclude writable Windows subdirectories.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 00ebff73e: SystemRoot is no longer a trusted root (Git does not install there and it holds world-writable Windows\Temp). Only the Program Files roots are trusted now.
| git_exe = shutil.which("git") | ||
| if git_exe: | ||
| git_dir = os.path.dirname(git_exe) | ||
| if os.path.isabs(git_dir) and _is_trusted_windows_program_dir(git_dir): |
There was a problem hiding this comment.
Expand short Program Files paths before trusting Git
On Windows hosts whose PATH contains Git through the DOS 8.3 alias, for example C:\PROGRA~1\Git\cmd, shutil.which("git") returns that short path but this trust check compares it textually against the long %ProgramFiles% roots. The directory is therefore refused and bare git remains unavailable in the sandbox even though it is the normal system Git install this change is meant to support; expand/canonicalize short names before applying the trusted-root check.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 00ebff73e: both the resolved git dir and the roots are canonicalized with realpath before comparison, so an 8.3 short alias like C:\PROGRA~1\Git\cmd matches Program Files and bare git stays available.
|
@codex review |
for more information, see https://pre-commit.ci
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 812ae238f9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for var in ("ProgramFiles", "ProgramFiles(x86)", "ProgramW6432"): | ||
| val = os.environ.get(var) | ||
| if val: | ||
| roots.append(val) |
There was a problem hiding this comment.
Do not trust overrideable ProgramFiles environment values
When Studio is launched with ProgramFiles (or the other checked variables) overridden to a user-writable directory, that directory is treated as a trusted install root. A fake git.exe there will be returned by shutil.which, its directory will be appended to the sandbox PATH, and an auto-approved bare command such as rg can then resolve to another attacker-controlled executable placed beside it. This defeats the user-writable-PATH exclusion; validate the actual directory ACL/known-folder location rather than trusting an inherited environment value.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5f1aa1ca3: the Program Files roots are now resolved via the Windows known-folder API (SHGetKnownFolderPath), so an overridden %ProgramFiles% env value no longer moves the trust boundary. Env vars are only a fallback when the API is unavailable.
| if os.path.isabs(git_dir) and _is_trusted_windows_program_dir(git_dir): | ||
| path_entries.append(git_dir) |
There was a problem hiding this comment.
Append the canonical Git directory to PATH
The trust check resolves symlinks with realpath, but the PATH entry retains the unresolved git_dir. A user-controlled junction in a host PATH can initially point at Program Files so it passes this check, then be retargeted to a writable directory before a sandboxed auto-approved command executes; commands such as rg will then resolve from that attacker-controlled directory. Append the canonical path used for the trust decision (or otherwise retain an immutable trusted target) instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5f1aa1ca3: the canonical realpath of the trusted dir is appended to PATH, not the raw entry, so a junction that passed the check cannot be retargeted to a writable dir before a sandboxed command runs.
|
@codex review |
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 080286bab5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if not roots: | ||
| for var in ("ProgramFiles", "ProgramFiles(x86)", "ProgramW6432"): | ||
| val = os.environ.get(var) | ||
| if val: | ||
| roots.append(val) |
There was a problem hiding this comment.
Include the native Program Files root for 32-bit Windows
On a 32-bit Python/Studio process running on 64-bit Windows, the known-folder API maps both FOLDERID_ProgramFiles and FOLDERID_ProgramFilesX86 to C:\Program Files (x86) (Microsoft table), so roots is non-empty and this env/fixed-root fallback never adds the native C:\Program Files root. In that environment a normal 64-bit Git install at C:\Program Files\Git\cmd is rejected by _is_trusted_windows_program_dir, leaving bare git unavailable in the sandbox; query a trusted native Program Files root as well instead of only falling back when no roots were found.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 58a650acb: FOLDERID_ProgramFilesX64 is now queried alongside the other two, so a 32-bit process still trusts the native C:\Program Files (where the first two ids both map to x86). The env fallback also already includes ProgramW6432.
| prog = tmp_path / "Program Files" | ||
| monkeypatch.setenv("ProgramFiles", str(prog)) | ||
| git_dir = prog / "Git" / "cmd" |
There was a problem hiding this comment.
Stub known-folder roots in Windows-specific env tests
When these tests run on an actual Windows runner, setting %ProgramFiles% here no longer makes tmp_path/Program Files trusted because _windows_program_roots() now uses SHGetKnownFolderPath before consulting the environment. That means the fake git.exe under tmp_path is rejected and the assertion below fails even though the production code is behaving as designed; monkeypatch _windows_program_roots in the positive tests the same way the later known-folder test does.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch. Fixed in 58a650acb: the positive Windows tests now monkeypatch _windows_program_roots instead of setting %ProgramFiles%, so they hold on a real Windows runner where SHGetKnownFolderPath is consulted first.
|
@codex review |
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0df5795ca3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| git_exe = shutil.which("git") | ||
| if git_exe: | ||
| git_dir = os.path.dirname(git_exe) | ||
| if os.path.isabs(git_dir) and _is_trusted_windows_program_dir(git_dir): |
There was a problem hiding this comment.
Continue past untrusted git shims
When a Windows PATH contains a user-writable git shim before a normal C:\Program Files\Git\cmd entry, shutil.which("git") returns only that first shim; this check rejects it and then stops, so the trusted Program Files Git later on PATH is never appended and cmd /c git ... remains not recognized. Fresh evidence versus the earlier trust-boundary comments is that the final code skips untrusted first matches without scanning for a later trusted match; iterate the PATH/PATHEXT candidates until a trusted Git dir is found.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in bf1eb19: git resolution now scans all PATH/PATHEXT candidates for a trusted Program Files launcher instead of stopping at shutil.which's first (possibly untrusted shim) match.
| if not roots: | ||
| for var in ("ProgramFiles", "ProgramFiles(x86)", "ProgramW6432"): | ||
| val = os.environ.get(var) | ||
| if val: | ||
| roots.append(val) |
There was a problem hiding this comment.
Add the native Program Files root after x86 matches
On a 32-bit Studio process running on 64-bit Windows, the first known-folder calls can populate roots with only C:\Program Files (x86), while Microsoft documents FOLDERID_ProgramFilesX64 as unsupported for 32-bit applications on 64-bit Windows, so this if not roots fallback never adds the native C:\Program Files root. Fresh evidence versus the earlier 32-bit-root thread is that the current fix relies on the unsupported X64 known-folder ID; derive and add a trusted native root even when the API returned only x86 roots, otherwise a normal 64-bit Git install is still rejected.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in bf1eb19: since FOLDERID_ProgramFilesX64 is unsupported for 32-bit apps, the native root is now derived by stripping the ' (x86)' suffix from the trusted x86 root, so a 64-bit Git install is trusted without relying on that id.
|
@codex review |
for more information, see https://pre-commit.ci
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1bb316460
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if not roots: | ||
| for var in ("ProgramFiles", "ProgramFiles(x86)", "ProgramW6432"): | ||
| val = os.environ.get(var) | ||
| if val: | ||
| roots.append(val) |
There was a problem hiding this comment.
Keep fallback Program Files roots out of the trust boundary
When the known-folder API is unavailable or raises, this fallback treats the parent-controlled ProgramFiles variables as admin-managed roots. A user who launches Studio with ProgramFiles pointing at a writable directory and a git.exe there can make that directory enter the sandbox PATH; subsequently, an auto-approved bare command such as rg can resolve to an attacker-controlled sibling in that directory. Fail closed or use a non-environment-derived trusted root in this path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in cbd15ce70: the known-folder-unavailable fallback no longer reads %ProgramFiles%/%ProgramW6432%; it uses fixed %SystemDrive%\Program Files roots (SystemDrive is not a viable override, since retargeting it breaks the OS), so an overridden ProgramFiles cannot relocate the trust boundary.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 877895dcc0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| drive = os.environ.get("SystemDrive", "C:") | ||
| roots = [drive + r"\Program Files", drive + r"\Program Files (x86)"] |
There was a problem hiding this comment.
Do not derive trusted Program Files from SystemDrive
When the known-folder API is unavailable, SystemDrive is still just an inherited, overrideable environment variable; setting it to a writable drive makes that drive’s Program Files directory a trusted root. If the parent PATH then resolves git from that location, this code appends the attacker-controlled directory to the sandbox PATH, allowing a colocated executable such as an auto-approved rg.exe to run. Use a non-environment-derived OS API/fixed system root for this fallback, or refuse to add Git when the trusted root cannot be resolved.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4bac1aa. Correct: %SystemDrive% is itself a caller-overrideable environment variable, so deriving a fallback trusted root from it still lets an attacker relocate the trust boundary onto a writable drive. The resolver now fails closed: when the known-folder API (SHGetKnownFolderPath) is unavailable it returns no roots at all, so Git is not appended to the sandbox PATH. On any real Windows host shell32 is present, so this only affects broken or non-Windows environments where the sandbox git-PATH feature is not needed.
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Reviewed and verified. In the studio sandbox, bare git on Windows was not resolvable because the sandbox PATH was stripped, so git backed tools failed. This resolves a trusted git into the sandbox PATH while keeping the trust boundary tight. Over review we hardened it so the trusted Program Files root is resolved only from the Windows known folder API (SHGetKnownFolderPath), never from caller overrideable environment variables including ProgramFiles, ProgramW6432, and SystemDrive, and it fails closed when that API is unavailable so no untrusted directory can ever enter the sandbox PATH. Covered with 145 sandbox tests including case sensitivity, PATHEXT, 8.3 short paths, and untrusted shim rejection. Staging CI confirmed the relevant Linux, Mac, and Windows jobs green. Codex review converged and approved. Good to merge. |
Fixes #7317
Problem
On Windows, Studio's sandboxed terminal tool fails with
'git' is not recognized as an internal or external commandeven when Git works in every normal cmd/PowerShell on the machine (including the one Studio was launched from). Baregitonly works after the model is told to use the 8.3 pathC:\PROGRA~1\Git\cmd\git.exe._build_safe_env()rebuildsPATHfrom scratch as Studio venv +System32only. User-installed Git lives underC:\Program Files\Git\cmd, so it never enters thatPATH. Unix sandboxes already include/usr/bin, which is why systemgitworked there.Fix
studio/backend/core/inference/tools.py: After the curated interpreter / venv / system dirs, append absolute entries from the parentPATH(strip quotes; skip empty /./ relative entries so cwd PATH hijacks stay blocked). On Windows, also inheritPATHEXTsogitmaps togit.exelike a normal shell.studio/backend/tests/test_sandbox_tools.py: Cover host PATH append order, relative-entry filtering, and quoted Windows-style PATH entries.Verification
PYTHONPATH=studio/backend python -m pytest studio/backend/tests/test_sandbox_tools.py::TestSandboxEnvIsolation -q PYTHONPATH=studio/backend python -m pytest studio/backend/tests/test_bypass_permissions.py -q -k 'safe_env or bypass_env or PATH or secret'8/8 sandbox env tests pass; 32 related bypass/env tests pass.
Compatibility
PATHprefix is unchanged: Studiopython/pipstill win over host copies.HF_TOKEN/USERPROFILE/ etc. in the sandbox env).PATH; unchanged.PATHappend; relative entries remain excluded.