fix: prevent sandbox mounts from exposing credentials - #4255
Conversation
![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: 87a4bf6788
ℹ️ 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 mount, mount_path in manifest.mount_targets(): | ||
| credential_fields = mount._configured_credential_fields() | ||
| if not credential_fields: | ||
| continue |
There was a problem hiding this comment.
When an opted-in in-container mount uses GCSMount.service_account_file or BoxMount.box_config_file pointing to a File entry in the same manifest, this check does not reject it because it only examines RcloneMountPattern.config_file_path. Serialization then clears the mount’s path field but leaves the credential-bearing File.content in the manifest, so the secret is still written to sandbox session and RunState JSON. Apply the manifest-entry rejection to every credential-file field, not only the rclone pattern field.
AGENTS.md reference: AGENTS.md:L137-L137
Useful? React with 👍 / 👎.
| paths = tuple( | ||
| entry_path | ||
| for entry_path, mount in _mounts_by_entry_path(manifest).items() | ||
| if mount._configured_credential_fields() | ||
| ) |
There was a problem hiding this comment.
A Docker-volume mount can supply credentials through mount_strategy.driver_options, for example {"s3-secret-access-key": "..."}; provider implementations merge this mapping into the actual driver configuration. Because the mount is selected for redaction only when _configured_credential_fields() finds a direct mount field, these option values remain verbatim in serialized session and RunState payloads—even when another direct credential causes the mount to be redacted, _clear_configured_credentials() leaves driver_options untouched. Treat credential-bearing driver options as trusted live configuration that must be removed and rebound.
AGENTS.md reference: AGENTS.md:L137-L137
Useful? React with 👍 / 👎.
| state_class = SandboxSessionState._subclass_registry.get(state_type) | ||
| if state_class is None: | ||
| raise ValueError("RunState sandbox session_state has an unknown type") |
There was a problem hiding this comment.
In a fresh process that calls RunState.from_json() before importing the concrete sandbox backend, even built-in states such as unix_local are absent from SandboxSessionState._subclass_registry, so this new check raises unknown type and prevents the durable RunState from being restored. Previously RunState deserialization preserved the raw sandbox payload and the configured client parsed it later, making provider import order irrelevant. Sanitize the raw manifest without requiring the provider state class, or otherwise defer provider-specific parsing until the client is available.
AGENTS.md reference: AGENTS.md:L136-L136
Useful? React with 👍 / 👎.
| credential_dir = _CREDENTIAL_DIRECTORY / session_id.hex | ||
| session.register_persist_workspace_skip_path(_CREDENTIAL_DIRECTORY) | ||
| await session.mkdir(credential_dir, parents=True) |
There was a problem hiding this comment.
If the process terminates after this workspace credential file is written but before its removal is confirmed, the surviving Blaxel sandbox can later be reconnected with _skip_start=True; the exclusion registered here exists only in the old session object’s _runtime_persist_workspace_skip_relpaths and is not serialized or reconstructed. A subsequent stop() in the new process can therefore archive .sandbox-blaxel-mount-credentials into the workspace snapshot. The new rclone config-file exclusion has the same in-memory-only lifecycle, so these secret-path exclusions need to be restored on reconnect or the files must remain outside the persisted workspace.
AGENTS.md reference: AGENTS.md:L137-L137
Useful? React with 👍 / 👎.
1 participant
This pull request fixes explicit cloud credentials being passed to mount helpers that execute inside model-controlled sandboxes. It classifies each built-in mount strategy by credential boundary and rejects secret-bearing in-container or unknown-boundary configurations before sandbox or manifest side effects, while preserving credentialless mounts, Docker volumes, and provider-native external mounts. A trusted application can explicitly acknowledge exposure for an exact mount path when no safer backend mechanism exists. Sandbox session and RunState persistence now strip credentials, reject untrusted serialized opt-ins, normalize resumed state through typed models, and restore secrets only after the complete credential-free mount configuration matches the current trusted manifest; the writer advances to schema 1.14 while supported older snapshots remain readable. It also protects rclone source files and Blaxel temporary credential files from snapshots, logs, command arguments, and incomplete cleanup, and updates examples to use safer strategies.