Studio: Bypass Permissions (skip confirmation, disable tool sandbox)#5895
Conversation
for more information, see https://pre-commit.ci
The Allow / Always allow / Deny controls only lived in the fallback tool
card, but the built-in tools (web search, python, terminal, code
execution, image generation) render with their own components and so
never showed the buttons. Those calls paused after tool_start with no way
to approve them, hanging until the 1 hour timeout. Only MCP tools, which
use the fallback renderer, actually worked.
Render the controls for every tool card by wrapping each registered tool
component (and the fallback) in thread.tsx with a shared
ToolConfirmationControls, so the gate applies uniformly.
Also make the handshake robust:
- The gate keys on a per-call approval_id minted by the backend and
echoed in tool_start, instead of session_id alone, so a stale or
concurrent confirmation can no longer resolve the wrong call.
- The approval slot is registered before tool_start is yielded, closing
the race where a fast click or an auto "Always allow" could reach the
backend before the waiter existed.
- The frontend resolves with the same session id the request was sent
with (plus the approval_id), fixing the new-thread mismatch where the
confirmation targeted a different session than the blocked stream.
- The confirm endpoint returns {resolved}; the UI keeps the buttons and
shows a retry hint until the backend confirms a match, instead of
hiding them on a failed or mistargeted post.
- The gate runs after the disabled-tool and duplicate-call checks, so a
call that will not execute is not put up for approval. A denied call is
still excluded from duplicate detection, so re-issuing and approving it
works.
- "Always allow" is scoped per session to match the backend gate.
Add backend tests for the approval registry, the SSE no-deadlock
handshake, and the loop integration (allow, deny, disabled, duplicate,
re-issue after deny).
for more information, see https://pre-commit.ci
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a 'Bypass Permissions' feature that allows users to skip tool-call confirmations and disable the execution sandbox for Python and terminal tools, while still stripping sensitive environment variables. The changes span backend execution logic, API routes, and frontend settings, along with a comprehensive suite of tests. Feedback on these changes highlights two key improvements: explicitly defining the bypass_permissions field on the Anthropic request model to prevent potential validation or attribute errors, and ensuring Windows compatibility by repointing the TEMP and TMP environment variables to the sandbox working directory.
| auto_heal_tool_calls = True, | ||
| tool_call_timeout = 300, | ||
| session_id = payload.session_id, | ||
| bypass_permissions = bool(payload.bypass_permissions), |
There was a problem hiding this comment.
Instead of using getattr and checking model_extra to safely retrieve bypass_permissions, define bypass_permissions explicitly as an Optional field with a default value (e.g., None or False) on the AnthropicMessagesRequest Pydantic model. This prevents potential validation issues or AttributeError crashes and ensures the model accurately reflects the fields that can be sent by the frontend.
| bypass_permissions = bool(payload.bypass_permissions), | |
| bypass_permissions = payload.bypass_permissions or False, |
References
- Ensure Pydantic models for API requests mark fields as Optional with a default of None if they can be omitted by the frontend, to prevent validation errors and avoid relying on model_extra workarounds.
| "PRIVATE_KEY", | ||
| ) | ||
|
|
There was a problem hiding this comment.
On Windows, Python's tempfile module and other SDKs/tools respect TEMP and TMP environment variables instead of TMPDIR. Since _build_bypass_env inherits the host's environment, the host's real TEMP and TMP variables will remain in the environment and take precedence over TMPDIR. This causes temporary files to be written to the host's real temp directory instead of the sandbox workdir. Repoint TEMP and TMP to workdir as well.
| "PRIVATE_KEY", | |
| ) | |
| env = {k: v for k, v in os.environ.items() if not _is_secret_env_name(k)} | |
| env["HOME"] = workdir | |
| env["TMPDIR"] = workdir | |
| env["TEMP"] = workdir | |
| env["TMP"] = workdir |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d2c8de0ed
ℹ️ 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".
| auto_heal_tool_calls = True, | ||
| tool_call_timeout = 300, | ||
| session_id = payload.session_id, | ||
| bypass_permissions = bool(payload.bypass_permissions), |
There was a problem hiding this comment.
When the Anthropic-compatible /v1/messages path runs server-side tools and the client does not send the new extension, this access raises AttributeError because AnthropicMessagesRequest does not declare bypass_permissions (I verified an omitted field is not present on the model despite extra='allow'). That makes ordinary tool-enabled Anthropic requests 500 unless they explicitly include this new nonstandard field; use getattr(payload, 'bypass_permissions', False) or add the field to the request model.
Useful? React with 👍 / 👎.
danielhanchen
force-pushed
the
bypass-permissions
branch
from
f2ce5f0 to
3dc3c83
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9dee0e8865
ℹ️ 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".
| """Env for bypass exec: full host env (unrestricted) minus credential vars, | ||
| with HOME/TMPDIR repointed at the workdir so SDKs cannot read cached creds. | ||
| """ | ||
| env = {k: v for k, v in os.environ.items() if not _is_secret_env_name(k)} |
There was a problem hiding this comment.
When bypass mode is used on a host with an SSH agent, this copies SSH_AUTH_SOCK because the name is not matched by _is_secret_env_name, so a model-run terminal command such as git/ssh can authenticate with the operator's loaded SSH keys even though the feature promises credential-bearing environment access is stripped. Please explicitly drop broker/location variables such as SSH_AUTH_SOCK before exposing the host environment in bypass mode.
Useful? React with 👍 / 👎.
| imageToolsEnabled: loadBool(CHAT_IMAGE_TOOLS_ENABLED_KEY, false), | ||
| mcpEnabledForChat: loadBool(CHAT_MCP_ENABLED_KEY, false), | ||
| confirmToolCalls: loadBool(CHAT_CONFIRM_TOOL_CALLS_KEY, false), | ||
| bypassPermissions: loadBool(CHAT_BYPASS_PERMISSIONS_KEY, false), |
There was a problem hiding this comment.
Because bypassPermissions is loaded directly from localStorage at startup, once a user enables it a browser reload or later Studio session silently starts with the sandbox disabled and tool confirmations bypassed, without showing the warning dialog again. For this high-risk setting, avoid persisting the enabled state or gate the restored value behind the same acceptance flow before requests can send bypass_permissions: true.
Useful? React with 👍 / 👎.
danielhanchen
force-pushed
the
bypass-permissions
branch
from
9dee0e8 to
df14066
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df140669c5
ℹ️ 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".
| """Env for bypass exec: full host env (unrestricted) minus credential vars, | ||
| with HOME/TMPDIR repointed at the workdir so SDKs cannot read cached creds. | ||
| """ | ||
| env = {k: v for k, v in os.environ.items() if not _is_secret_env_name(k)} |
There was a problem hiding this comment.
When bypass mode runs on hosts that use credentialed package indexes or proxies, variables such as PIP_INDEX_URL, PIP_EXTRA_INDEX_URL, or HTTP_PROXY are copied here because their names do not match _is_secret_env_name; those values commonly embed user:token@..., so a bypassed python/terminal tool can simply print the environment and recover credentials despite the feature promising secrets are stripped. Please switch to an allowlist or also scrub URL userinfo / known credential-bearing URL variables before exposing the host env.
Useful? React with 👍 / 👎.
Adds an opt-in Bypass Permissions toggle next to Confirm tool calls. When on, no tool call shows a confirmation prompt and the python/terminal sandbox is disabled: safety checks, command blocklist, and resource limits are skipped. Secret env vars are still stripped and HOME stays repointed at the session workdir. Default off keeps current behavior, and it takes precedence over Confirm tool calls. Enabling it requires accepting a warning each time.
danielhanchen
force-pushed
the
bypass-permissions
branch
from
df14066 to
57f8456
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57f84569f3
ℹ️ 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".
| """Env for bypass exec: full host env (unrestricted) minus credential vars, | ||
| with HOME/TMPDIR repointed at the workdir so SDKs cannot read cached creds. | ||
| """ | ||
| env = {k: v for k, v in os.environ.items() if not _is_secret_env_name(k)} |
There was a problem hiding this comment.
On Linux hosts with the default same-user /proc visibility, stripping secrets only from the child env is not enough once bypass disables the code/file safety checks: a bypassed python/terminal call can read /proc/<Studio pid>/environ (for example using its parent PID) and recover the original Studio process variables such as HF_TOKEN or cloud keys. Since the UI/API promise that API keys stay hidden while this path explicitly runs unsandboxed, this leaks exactly the credentials this filter is trying to remove; avoid claiming/enabling secret protection here unless the parent environment is also protected or the child is isolated from the parent process environment.
Useful? React with 👍 / 👎.
…ool path Follow-up to the Bypass Permissions feature. Addresses the review findings: - Anthropic /v1/messages 500: declare bypass_permissions on AnthropicMessagesRequest so tool requests that omit the field default to False instead of raising AttributeError (extra='allow' does not set absent attributes). - /proc parent-env leak: stripping the child env did not stop a same-uid bypassed child from reading /proc/<parent>/environ to recover the tool-executing process's unfiltered secrets. Clear PR_SET_DUMPABLE on that process before the first bypass exec so its /proc entries become root-owned. Hardening is fail-closed: if prctl is denied, bypass execution is refused rather than run with the parent environ still readable. Mitigation, not a full boundary; documented in the code. - Broker/capability vars: strip SSH_AUTH_SOCK, SSH_AGENT_PID, GPG_AGENT_INFO, GNUPGHOME, KUBECONFIG, DOCKER_HOST so a bypassed tool cannot use the operator's live agents. - Credential-bearing URL values: drop any env var whose value embeds URL userinfo (scheme://user:pass@ and token-only scheme://token@) regardless of the variable name. Benign proxy/index URLs without credentials are kept, so proxy-only and internal-index setups still work in bypass mode. - Windows temp isolation: repoint TEMP and TMP (not just TMPDIR) at the per-session sandbox dir. - Frontend: stop persisting bypassPermissions; a reload now starts with the sandbox/confirmation bypass off and requires re-accepting the warning dialog. Adds regression tests for each finding in test_bypass_permissions.py.
for more information, see https://pre-commit.ci
|
Reviewed this end to end and reproduced each flagged issue locally. They are all real; I pushed a commit on top ( Findings (all confirmed) and fixes1. Anthropic Fix: declare 2. Secret stripping defeated via Fix: clear 3. 4. Credential-bearing URL vars not stripped (codex P1). 5. Windows 6. Persisted bypass reactivates on reload without re-consent (codex P2). The store rehydrated Validation
One follow-up out of scope here: the Anthropic path still does not thread |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d906139661
ℹ️ 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".
| env = { | ||
| k: v | ||
| for k, v in os.environ.items() | ||
| if not _is_secret_env_name(k) and not _is_secret_env_value(v) | ||
| } |
There was a problem hiding this comment.
When the Studio process was started with HF_HOME (or similar Hugging Face cache-location variables) pointing at the operator's real cache, this copies that path into the bypassed child because the name is not classified as secret. Repointing HOME then does not prevent huggingface_hub/Transformers from auto-reading the cached token under $HF_HOME, so a bypassed python/terminal tool can recover the credential despite the bypass env promising SDK cached credentials are hidden; strip or repoint those cache/config location variables before inheriting the host environment.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d906139661
ℹ️ 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".
| env = { | ||
| k: v | ||
| for k, v in os.environ.items() | ||
| if not _is_secret_env_name(k) and not _is_secret_env_value(v) |
There was a problem hiding this comment.
When the Studio process has HF_HOME/HF_HUB_CACHE in its environment (startup sets these cache paths, and without_hf_auth already treats HF_HOME as auth-bearing), this bypass env copies them because their names do not match the secret filters. A bypassed python/terminal call can then read $HF_HOME/token or let huggingface_hub load it, so repointing only HOME does not actually prevent cached Hugging Face credentials from reaching unsandboxed tools; strip or repoint the HF cache/auth path variables here as well.
Useful? React with 👍 / 👎.
| for k, v in os.environ.items() | ||
| if not _is_secret_env_name(k) and not _is_secret_env_value(v) | ||
| } | ||
| env["HOME"] = workdir |
There was a problem hiding this comment.
On Windows, overriding only HOME here does not move the profile/config locations most SDKs use: USERPROFILE, HOMEDRIVE/HOMEPATH, APPDATA, and LOCALAPPDATA are still copied from the host by the env comprehension above. In bypass mode a python/terminal tool can therefore read cached credentials from the operator's real profile (for example .aws, Hugging Face, or wandb config) despite the intended HOME repointing, so these Windows profile variables need to be stripped or redirected to the session workdir too.
Useful? React with 👍 / 👎.
| env = { | ||
| k: v | ||
| for k, v in os.environ.items() | ||
| if not _is_secret_env_name(k) and not _is_secret_env_value(v) |
There was a problem hiding this comment.
If the operator has path override variables such as NETRC, BOTO_CONFIG, or PIP_CONFIG_FILE pointing at real credential/config files, this comprehension preserves them because neither the names nor path values match the new secret filters. Those variables are honored by common HTTP/cloud/package clients before $HOME, so a bypassed python/terminal tool can still auto-auth through the host's cached credentials even though HOME is repointed; strip or rewrite credential-bearing config path variables here too.
Useful? React with 👍 / 👎.
…ions Repointing HOME did not stop SDKs auto-reading cached creds via vars that point at the real home/cache/config: HF_HOME (startup always sets it; token lives under $HF_HOME/token), HF/XDG cache roots, NETRC/BOTO_CONFIG/ PIP_CONFIG_FILE, and Windows HOMEDRIVE/HOMEPATH. Drop those, and repoint USERPROFILE/APPDATA/LOCALAPPDATA at the per-session workdir. Adds regression tests.
for more information, see https://pre-commit.ci
|
Pushed The clearest case is Fix in
Same boundary as before: bypass is unsandboxed by design, so a tool can still read on-disk credentials by absolute path or walk Added regression tests in |
The drop-based fix relies on the whole HF_HOME/XDG fallback chain being removed so huggingface_hub resolves under the repointed HOME. Add a test that sets HF_HOME and XDG_CACHE_HOME at a real cache and asserts the resolved token path lands under the workdir, not the operator's cache.
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: b58752f006
ℹ️ 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".
| _BYPASS_ENV_SECRET_MARKERS = ( | ||
| "TOKEN", | ||
| "API_KEY", | ||
| "APIKEY", | ||
| "SECRET", |
There was a problem hiding this comment.
When an operator supplies npm registry credentials through NPM_CONFIG__AUTH (npm maps this env var to the protected _auth config; npm config get _auth reports the option is protected), this marker list does not match AUTH and the base64 value has no URL userinfo, so _build_bypass_env passes it through. In bypass mode a terminal/python tool can simply print that env var and recover registry credentials, contrary to the secret-stripping guarantee; add _AUTH/AUTH or explicit npm auth names to the scrubber.
Useful? React with 👍 / 👎.
| if not _is_secret_env_name(k) | ||
| and not _is_secret_env_value(v) | ||
| and not _is_cred_location_env_name(k) |
There was a problem hiding this comment.
For terminal calls on POSIX, _get_shell_cmd runs bash -c, and bash reads the file named by BASH_ENV for non-interactive shells; because this filter keeps BASH_ENV, an operator/CI shell startup file that re-exports stripped values like HF_TOKEN or cloud keys is sourced before the requested command. In that environment, a bypassed terminal tool can recover secrets just by printing variables that were removed from the child env, so shell startup hook variables such as BASH_ENV should be stripped or repointed to an empty file.
Useful? React with 👍 / 👎.
| "PASSWORD", | ||
| "PASSWD", |
There was a problem hiding this comment.
When the Studio process is started with the MySQL client password in MYSQL_PWD, this classifier keeps it because the markers include PASSWORD/PASSWD but not the exact MYSQL_PWD spelling, and the value is not a URL. A bypassed python or terminal tool can therefore print the inherited variable and recover the database password despite the secret-stripping guarantee; add the exact MySQL password env name rather than relying on the generic markers.
Useful? React with 👍 / 👎.
Three more credential vectors dodged the bypass scrubber: NPM_CONFIG__AUTH (npm _auth, base64 so no URL userinfo and no AUTH marker), MYSQL_PWD (markers use PASSWD, not PWD, since PWD is the cwd var), and BASH_ENV (bash -c sources it for non-interactive shells, so a startup file can re-export stripped secrets). Add an AUTH marker, the exact MYSQL_PWD name, and drop BASH_ENV plus PGPASSFILE. Adds regression tests incl. an end-to-end check that a bypass terminal call does not source BASH_ENV.
for more information, see https://pre-commit.ci
|
Addressed the three credential vectors from the latest Codex review (all confirmed and reproduced) in
Added regression tests, including an end-to-end check that a real bypass terminal call does not source Worth stating plainly: in bypass mode the secret stripping is best-effort by design. The env is otherwise inherited so tools work, so a name-based denylist cannot be exhaustive, and a bypassed tool can still read on-disk credentials by absolute path or reach a same-uid ancestor through |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1cb8e37b6
ℹ️ 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".
| auto_heal_tool_calls = True, | ||
| tool_call_timeout = 300, | ||
| session_id = payload.session_id, | ||
| bypass_permissions = bool(payload.bypass_permissions), |
There was a problem hiding this comment.
When an Anthropic request uses the server-tool path with enable_tools: true, enabled_tools: [], and bypass_permissions: true, _select_anthropic_server_tools returns an empty openai_tools list but this path still enters generate_chat_completion_with_tools. The GGUF tool loop treats an empty allow-list as unrestricted (bool(_allowed) is false), so a prompt-emitted XML terminal/python call can run with disable_sandbox=True even though the request enabled no tools; the OpenAI GGUF/safetensors paths already skip the loop when the selected tool list is empty, so mirror that guard here before forwarding bypass.
Useful? React with 👍 / 👎.
| try: | ||
| os.setsid() | ||
| except OSError: |
There was a problem hiding this comment.
On Linux bypassed python/terminal calls now use this preexec path instead of _sandbox_preexec, but it drops the existing PR_SET_PDEATHSIG = SIGKILL setup. If the Studio process is killed or crashes while a bypassed command is running, a no-output or long-running child can survive as an orphan with the sandbox disabled and no resource caps; keeping the parent-death signal here would preserve cleanup without re-enabling the sandbox checks.
Useful? React with 👍 / 👎.
…loops From a parallel review pass over the bypass changes: - Drop more credential-location vars in _build_bypass_env: npm/yarn/git/cargo/ rclone config pointers (NPM_CONFIG_USERCONFIG, NPM_CONFIG_GLOBALCONFIG, YARN_RC_FILENAME, GIT_CONFIG_GLOBAL, GIT_CONFIG_SYSTEM, CARGO_HOME, RCLONE_CONFIG) and the GIT_ASKPASS/SSH_ASKPASS auth helpers. - Enforce confirm_tool_calls AND NOT bypass_permissions inside the safetensors and GGUF tool loops, not just at the route, so a direct internal caller passing both flags never prompts. - Soften the toggle hint: environment secrets are stripped, but bypassed code can still read files and credentials on the machine (no overclaim that keys stay hidden). Adds regression tests for the new names and the loop-level precedence.
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: 3c8d719691
ℹ️ 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 sys.platform != "linux": | ||
| return True # no /proc/<pid>/environ same-UID leak to close |
There was a problem hiding this comment.
On macOS/Windows hosts, this branch treats parent-environment hardening as successful even though the bypassed child is unsandboxed and can still query the Studio process environment via native process-inspection APIs (for example Darwin ps -E for same-user processes). Fresh evidence versus the earlier Linux /proc finding is that this version fail-closes only on Linux and explicitly returns True here, so _python_exec/_bash_exec proceed with HF_TOKEN/cloud keys still recoverable from the parent; fail closed or implement equivalent hardening on unsupported platforms.
Useful? React with 👍 / 👎.
| # auth-helper scripts that hand creds to git/ssh | ||
| "GIT_ASKPASS", | ||
| "SSH_ASKPASS", |
There was a problem hiding this comment.
When the operator has GIT_SSH_COMMAND or GIT_SSH set to select a deploy key, this scrub list drops GIT_ASKPASS/SSH_ASKPASS but leaves those SSH command overrides in the inherited bypass environment. Git documents GIT_SSH_COMMAND as the SSH command it runs and gives ssh -i ... as the intended pattern, so a bypassed terminal/python tool can print the key path or run git with the operator's key despite stripping other broker variables; add these Git SSH overrides to the credential-location/capability filter. Source checked: https://git-scm.com/book/en/v2/Git-Internals-Environment-Variables
Useful? React with 👍 / 👎.
| # package-manager / git / cloud config pointers to real cred files | ||
| "NPM_CONFIG_USERCONFIG", | ||
| "NPM_CONFIG_GLOBALCONFIG", | ||
| "YARN_RC_FILENAME", | ||
| "GIT_CONFIG_GLOBAL", | ||
| "GIT_CONFIG_SYSTEM", | ||
| "CARGO_HOME", | ||
| "RCLONE_CONFIG", |
There was a problem hiding this comment.
When Studio is launched from CI or a wrapper that injects Git credentials through inline config, e.g. GIT_CONFIG_COUNT=1, GIT_CONFIG_KEY_0=http.extraHeader, and GIT_CONFIG_VALUE_0=Authorization: Bearer ..., this filter only drops file-based Git config paths and leaves the GIT_CONFIG_* triplet untouched. Those names do not match the secret markers and the value is not a URL, so a bypassed terminal/python tool can print the bearer header or have git use it; Git documents these env pairs as runtime configuration that overrides config files. Strip GIT_CONFIG_COUNT, GIT_CONFIG_KEY_*, and GIT_CONFIG_VALUE_* before inheriting the host env. Source checked: https://git-scm.com/docs/git-config
Useful? React with 👍 / 👎.
The safetensors loop precedence is covered behaviorally; the GGUF loop needs a live llama-server so add an AST guard asserting its _needs_confirm gate references both confirm_tool_calls and bypass_permissions, matching the other llama_cpp source-inspection tests.
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: 23344c8f68
ℹ️ 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".
| "PASSWD", | ||
| "CREDENTIAL", | ||
| "PRIVATE_KEY", | ||
| "AUTH", # e.g. NPM_CONFIG__AUTH (npm _auth), REDISCLI_AUTH |
There was a problem hiding this comment.
When Studio is launched with capability URLs such as SLACK_WEBHOOK_URL or DISCORD_WEBHOOK_URL, _build_bypass_env still passes them through because the name does not contain any of these markers and the current value check only catches URL userinfo, not tokens embedded in the path. In bypass mode a python/terminal tool can print or use that inherited webhook despite the secret-stripping guarantee, so add a WEBHOOK marker/explicit names or move this path to an allowlist.
Useful? React with 👍 / 👎.
When Bypass Permissions is on, show a persistent red pill in the composer tool-pill row (like the Search/Code pills), matching Claude Code's always- visible bypass indicator. Clicking it turns bypass off, mirroring the other composer toggles. Enabling still goes through the settings toggle + warning dialog. Adds a data-variant=danger style for the destructive-colored pill.
The empty-state and active Thread render their own composer (thread.tsx), not shared-composer, so the badge only appeared in the split layout. Mirror the red dismissible pill in ComposerAction so it shows in every composer.
Bring the Bypass Permissions feature up to date with main (262 commits). main landed the tool-call-confirmation base (#5869) and evolved it, so the conflicts were resolved by taking main's version of every touched file and re-applying the bypass deltas on top: - tools.py: bypass env scrubber (_build_bypass_env, secret/cred-location classifiers), _bypass_preexec, /proc fail-closed hardening, and the disable_sandbox path in execute_tool/_python_exec/_bash_exec. - safetensors_agentic.py / llama_cpp.py: thread bypass_permissions and gate needs_confirm with 'and not bypass_permissions' (main simplified the gate to bool(confirm_tool_calls)). - routes/inference.py, models/inference.py, orchestrator.py: precedence (confirm and not bypass) plus the request fields and loop wiring. - frontend: bypassPermissions store state, chat-adapter payload, the settings toggle + warning dialog, and the red composer badge in both composers. - tests: fakes accept rag_scope (main added it); GGUF AST test matches main's needs_confirm name.
danielhanchen
requested review from
Datta0,
Etherll and
pluesclues
as code owners
for more information, see https://pre-commit.ci
…s collapsed The Thread composer only renders the pill row when expanded, so the active-mode badge vanished on the default (collapsed) empty state. Render it before the expand gate (it returns null when bypass is off) so the red indicator always shows while bypass is on.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd81d9762d
ℹ️ 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".
| const token = getAuthToken(); | ||
| if (token) { | ||
| query.set("allow_network", "1"); | ||
| query.set("token", token); |
There was a problem hiding this comment.
When allowArtifactNetworkAccess is enabled, the generated HTML runs as script in the preview frame and can read its own location.href, which includes this token query parameter; because the network CSP for that mode also allows outbound http(s)/image requests, any model-generated artifact can exfiltrate the Studio bearer token and then use it against the API until it expires. Use a frame-only capability or an HttpOnly authentication mechanism instead of exposing the main auth token to the untrusted document.
Useful? React with 👍 / 👎.
|
|
||
|
|
||
| _MAX_BODY_BYTES = int(os.environ.get("UNSLOTH_STUDIO_MAX_BODY_MB", "500")) * 1024 * 1024 | ||
| _BODY_PROTECTED_PREFIXES = ( |
There was a problem hiding this comment.
The new RAG document upload routes live under /api/rag/.../documents, but this protected-prefix list does not include /api/rag, so MaxBodyMiddleware skips those requests entirely and routes/rag.py streams the uploaded file until EOF. An authenticated client can therefore write arbitrarily large RAG documents to disk despite the Studio upload cap; include /api/rag here and add the same upload passthrough sizing used by the other multipart upload routes.
Useful? React with 👍 / 👎.
The destructive button variant is a subtle 10% tint that read as bare red text next to the outlined Cancel. Force the solid destructive fill (the variant's class loses to the tint through AlertDialogAction's Slot merge, so use the ! override the codebase already uses for this case) and shorten the label to 'I understand' so it fits the small dialog's two-column footer.
Adds a 'Bypass Permissions' entry to the composer plus-menu (under More by default) in both composers, so it can be toggled without opening Run settings. Enabling routes through the same danger warning dialog; disabling is immediate. A shared BypassPermissionsMenuItem keeps the two composers in sync.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf0f0c8029
ℹ️ 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".
| confirm_tool_calls = bool(payload.confirm_tool_calls) | ||
| and not bool(payload.bypass_permissions), | ||
| bypass_permissions = bool(payload.bypass_permissions), |
There was a problem hiding this comment.
When an API client sends confirm_tool_calls: true together with bypass_permissions: true on a local tool request with stream: false, the earlier stream guard still checks the raw payload.confirm_tool_calls and rejects before this effective confirm && !bypass value is used (the safetensors path has the same ordering). That contradicts the new precedence rule documented here and forces clients that have both flags set to know to clear confirm_tool_calls themselves; compute the effective confirm flag before validation, or gate the validation on confirm_tool_calls && !bypass_permissions.
Useful? React with 👍 / 👎.
| "DOCKER_HOST", | ||
| } | ||
| ) | ||
| _BYPASS_ENV_SECRET_PREFIXES = ("AWS_", "AZURE_", "GOOGLE_", "GCP_", "GCLOUD_", "DYLD_") |
There was a problem hiding this comment.
When Studio runs on EC2 with AWS_EC2_METADATA_DISABLED=true, this broad AWS_ prefix drops that non-secret guard from the bypass child; AWS documents that the flag disables SDK/CLI credential lookups from IMDS (https://docs.aws.amazon.com/sdkref/latest/guide/feature-imds-credentials.html). With env/file credentials stripped, a bypassed python/terminal call that uses boto or the AWS CLI can now fall back to the instance role and obtain cloud credentials even though the operator explicitly disabled that path, so keep this safety flag while stripping the actual AWS credential variables.
Useful? React with 👍 / 👎.
| "PASSWORD", | ||
| "PASSWD", | ||
| "CREDENTIAL", | ||
| "PRIVATE_KEY", | ||
| "AUTH", # e.g. NPM_CONFIG__AUTH (npm _auth), REDISCLI_AUTH |
There was a problem hiding this comment.
When Studio is launched in environments that expose connection strings, this marker set misses names such as SQLCONNSTR_*, CUSTOMCONNSTR_*, or WEBSITE_CONTENTAZUREFILECONNECTIONSTRING, and _is_secret_env_value only catches URL userinfo, not values like AccountKey=... or Password=...; Azure documents these App Service variables as connection strings (https://learn.microsoft.com/en-us/azure/app-service/reference-app-settings). A bypassed python/terminal tool can therefore print database or storage credentials despite the secret-stripping guarantee, so add connection-string name/value markers or switch this path to an allowlist.
Useful? React with 👍 / 👎.
…rings Two gaps in the Bypass Permissions secret scrubber: - The broad AWS_ prefix also dropped AWS_EC2_METADATA_DISABLED, a non-secret opt-out. Removing it re-opens the IMDS instance-role credential path that the operator explicitly disabled, so a bypassed boto/AWS-CLI call could recover cloud creds. Keep that flag (and AWS_EC2_METADATA_V1_DISABLED) via a keep-list while still stripping the real AWS credential vars. - Azure App Service connection strings (SQLCONNSTR_/CUSTOMCONNSTR_/..., WEBSITE_CONTENTAZUREFILECONNECTIONSTRING) and values like Password=/AccountKey= /SharedAccessKey= slipped past the name and URL-only value classifiers. Add CONNSTR/CONNECTIONSTRING name markers and a connection-string value matcher.
The confirm-vs-bypass precedence (confirm and not bypass) was applied at the loop call sites but not at the earlier request guards, so a client sending confirm_tool_calls + bypass_permissions together was rejected (stream=true required / unsupported for external or Anthropic tools) before the precedence took effect. Gate all four confirm guards on not bypass_permissions so both flags together proceed with the gate suppressed, matching the documented rule.
|
Thanks, all three are valid and now fixed in 78db58a and 0033c84. Bypass env scrubber (P1 x2)
Confirm vs bypass precedence (P2)
Added regression tests for the keep-list, the connection-string name and value cases, and non-credential connection strings (full suite green). |
for more information, see https://pre-commit.ci
2 participants
Builds on #5869 (the Confirm tool calls gate). This PR is based on the
tool-call-confirmationbranch so the diff shows only the Bypass Permissions changes. Merge #5869 first, then I will retarget this tomain.What this adds
An opt-in "Bypass Permissions" toggle in chat settings, next to "Confirm tool calls". When on:
Default off, so current behavior is unchanged.
Scope
Secrets stay hidden
Even in bypass mode, credential-bearing env vars (HF_TOKEN, AWS_*, OPENAI_API_KEY, and similar) are stripped from the tool environment, and HOME is repointed at the per-session sandbox folder so the HF, aws, and wandb SDKs do not auto-read cached credentials. On-disk files stay reachable because the filesystem is unrestricted by design.
Safety detail
Bypass keeps a minimal
setsid-only pre-exec. This is required, not optional:_kill_process_treesends SIGKILL to the child process group, so without a fresh session a timeout or cancel would kill the Studio server itself. Per-call timeouts and cancellation are kept too.UX
Enabling the toggle opens a warning that must be accepted every time:
While on, "Confirm tool calls" is disabled and shown as overridden.
Backend flow
bypass_permissionsis a per-request field on the chat completion request. The route enforces precedence (confirm_tool_calls = confirm and not bypass) and threadsbypass_permissionsinto the GGUF and safetensors tool loops, which passdisable_sandboxto the tool executor.Tests
test_bypass_permissions.py: the secret-name classifier, both env builders, thedisable_sandboxbranch of python and terminal exec (env, pre-exec, skipped safety checks and blocklist), a real subprocess check that the host env is visible while secrets are stripped, the request-model default, the precedence rule, and that the loop forwardsdisable_sandboxwithout gating.execute_toolkeyword.