Skip to content

fix(sandbox): mask the session's CLI capability in returned sandbox output - #8498

Closed
waleedlatif1 wants to merge 2 commits into
stagingfrom
fix/sandbox-redact-session-capability
Closed

waleedlatif1 wants to merge 2 commits into
stagingfrom
fix/sandbox-redact-session-capability

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Addresses the release review finding on apps/sim/lib/execution/remote-sandbox/index.ts (the provenance check ignores the session's environment): code running in a Chat sandbox session could print SIM_API_KEY and the token-bearing SIM_ENDPOINT, and the values came back unmasked to the model and the transcript.

Root cause

buildMothershipSandboxSession exported the per-call CLI capability (SIM_API_KEY and the scoped SIM_ENDPOINT) through session.envs, which executeInSandbox / executeShellInSandbox spread into the process environment. Nothing in the output path knew those values were sensitive, so env, echo $SIM_API_KEY, a traceback, or a returned result carried them verbatim. This is pre-existing (it predates the previous release), not a regression from the release diff.

Fix

  • SandboxSessionRequest gains secretEnvs: variables present on every execution whose values are masked in its output.
  • The Chat session builder puts SIM_API_KEY and SIM_ENDPOINT in secretEnvs; SIM_WORKSPACE / SIM_ORGANIZATION_ID stay in envs.
  • Both sandbox entry points mask those values (plus their URI/form-encoded forms, via the existing redactKnownSensitiveValues) in stdout, stderr, the __SIM_RESULT__ payload and error text, before anything is parsed or returned.
  • Exported files are masked too: declared text exports like stdout, and binary declared files plus everything harvested from SIM_OUTPUT_DIR over their raw bytes (latin1 round trip, so every other byte survives unchanged; byteLength is recomputed). Non-session executions skip this entirely.

Behaviour changes

  • Sandbox output that contains the session capability now shows [REDACTED] in its place. Programs still receive the real values in their environment, so the CLI works unchanged.
  • Secret provenance recording is unchanged: the capability is not a user secret, so it does not taint the machine.

Severity context

The capability is short-lived: a fresh key per tool call, only its hash stored, accepted only while the tool call's Redis scope and the run's ownership lease are both live, and revoked in the scope's finally before the result reaches the model. This change is defense in depth so a printed copy never lands in the transcript or logs at all.

Not changed

  • Moving the key out of the process environment into a SIM_API_KEY_FILE was considered and left out: it changes the CLI contract, and masking already covers what is returned.

Test plan

  • New session-sandbox.test.ts case, for code and shell, completing and failing: the program receives the real values, and the returned result contains neither the key, the endpoint, nor its URI-encoded form, while non-secret session envs stay visible
  • New file-export case, for code and shell: a declared text file, a declared binary file and a harvested file carrying the key and the URI-encoded endpoint come back masked, with non-UTF-8 bytes intact
  • Confirmed each masking branch goes red when reverted on its own, and green with it
  • sandbox-session.test.ts updated for the envs / secretEnvs split
  • vitest run lib/execution lib/mothership/tools and lib/function-execution
  • bun run lint, bun run type-check, bun run check:audits

…utput

The Mothership session sandbox exports SIM_API_KEY and the token-bearing
SIM_ENDPOINT to every execution, but nothing masked their values, so code
that printed its environment returned them to the model and transcript.
The session request now carries them as secretEnvs, and the sandbox masks
their values in stdout, stderr, the result payload, and error text before
any of it is parsed or returned. Provenance recording is unchanged.
@vercel

vercel Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
docs Skipped Skipped Oct 1, 2026 1:39am UTC

Request Review

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 5 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread apps/sim/lib/execution/remote-sandbox/index.ts

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 5 files

Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.

Re-trigger cubic

@greptile-apps

greptile-apps Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Separates sensitive environment variables from regular ones in sandbox execution.

The PR appears safe to merge, with a non-blocking masking gap for non-ASCII configured endpoints in binary exports.

Findings

  1. P2 Security Non-ASCII endpoints escape masking ▶

Summary

The PR separates the Chat sandbox CLI capability from ordinary session environment variables and masks it in process output and exported files.

  • Code and shell executions continue to receive the real capability.
  • A configured non-ASCII endpoint can remain visible in a binary or harvested export.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Session capability] --> B[Sandbox process environment]
  B --> C[Process output]
  B --> D[Exported files]
  C --> E[String mask]
  D --> F[Binary byte mask]
  E --> G[Returned result]
  F --> G
Loading

Reviews (3) · Last reviewed commit: "fix(sandbox): mask the session's capabil..."

Comment thread apps/sim/lib/execution/remote-sandbox/index.ts
… files

Code could write SIM_API_KEY or SIM_ENDPOINT to a declared output file or
into SIM_OUTPUT_DIR, and those bytes were returned unmasked. Text exports
are now masked like stdout; binary and harvested files are masked over
their raw bytes through a latin1 round trip, so every other byte survives
unchanged and the reported byteLength matches the masked content.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@waleedlatif1
waleedlatif1 marked this pull request as draft October 1, 2026 01:41
Comment thread apps/sim/lib/execution/remote-sandbox/index.ts
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

Closing in favour of #8503, which fixes the same finding by feeding the session's callback credentials into the existing model-output redaction registry.

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 5 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

* unit and back, so every byte outside a masked value, including non-UTF-8 binary, survives as-is.
*/
function maskFileBytes(contentBase64: string, mask: (output: string) => string): string {
const bytes = Buffer.from(contentBase64, 'base64').toString('latin1')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 security Non-ASCII endpoints escape masking

If the configured sandbox CLI endpoint contains a non-ASCII character and a program writes its UTF-8 bytes to a binary or harvested file, this code reads those bytes as Latin-1 before masking. The endpoint string no longer matches, so the returned file can retain the token-bearing URL. This leaves a copy in the exported output despite the session mask.

How this was verified: Binary file bytes are compared as Latin-1 text against the configured endpoint string, which is not Unicode-normalized.

Knowledge Base Used: Agent execution and sandbox tasks

This branch was previously deployed

1 inactive deployment
Preview — 6cc8341c Deployed Oct 1, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant