fix(sandbox): mask the session's CLI capability in returned sandbox output - #8498
waleedlatif1 wants to merge 2 commits into
Conversation
…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.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 5 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
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
|
… 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.
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
|
Closing in favour of #8503, which fixes the same finding by feeding the session's callback credentials into the existing model-output redaction registry. |
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
| * 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') |
There was a problem hiding this comment.
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
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 printSIM_API_KEYand the token-bearingSIM_ENDPOINT, and the values came back unmasked to the model and the transcript.Root cause
buildMothershipSandboxSessionexported the per-call CLI capability (SIM_API_KEYand the scopedSIM_ENDPOINT) throughsession.envs, whichexecuteInSandbox/executeShellInSandboxspread into the process environment. Nothing in the output path knew those values were sensitive, soenv,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
SandboxSessionRequestgainssecretEnvs: variables present on every execution whose values are masked in its output.SIM_API_KEYandSIM_ENDPOINTinsecretEnvs;SIM_WORKSPACE/SIM_ORGANIZATION_IDstay inenvs.redactKnownSensitiveValues) in stdout, stderr, the__SIM_RESULT__payload and error text, before anything is parsed or returned.SIM_OUTPUT_DIRover their raw bytes (latin1 round trip, so every other byte survives unchanged;byteLengthis recomputed). Non-session executions skip this entirely.Behaviour changes
[REDACTED]in its place. Programs still receive the real values in their environment, so the CLI works unchanged.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
finallybefore 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
SIM_API_KEY_FILEwas considered and left out: it changes the CLI contract, and masking already covers what is returned.Test plan
session-sandbox.test.tscase, 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 visiblesandbox-session.test.tsupdated for theenvs/secretEnvssplitvitest run lib/execution lib/mothership/toolsandlib/function-executionbun run lint,bun run type-check,bun run check:audits