feat(function): mount referenced files into the code sandbox - #7251
Conversation
Reference a file's path in Function block code and it is mounted for you: `<block.file.path>` resolves to its location on the sandbox filesystem, so any language can open it. It is the counterpart to `.base64`, which inlines the bytes and only works in JavaScript, leaving Python and Shell with no way to read a referenced file at all. Resolution happens long before a sandbox exists, and mount paths are only assigned once the whole set is planned together, so the resolver leaves a marker that the function runtime swaps for the real path — the same shape as LargeValueRef. Files written to /tmp/sim/outputs come back as platform file objects, so they can be attached or uploaded with no intermediate step. Harvesting is automatic on runs already in the remote sandbox; isolate runs pay nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Greptile SummaryThe PR adds referenced-file mounts and harvested sandbox outputs while extending file tools to accept existing file objects.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains from the previously reported findings. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/function-execution/execute-request.ts | Materializes referenced-file mounts, harvests sandbox outputs, and unconditionally scans harvested bytes for resolved secret literals. |
| apps/sim/lib/execution/remote-sandbox/index.ts | Extends the provider-neutral sandbox lifecycle with output-directory initialization and bounded file harvesting. |
| apps/sim/executor/variables/resolver.ts | Converts file .path references into deferred mount markers and language-appropriate path expressions. |
| apps/sim/lib/function-execution/sandbox-mounts.ts | Centralizes inline and presigned-URL mount transport, byte ceilings, and aggregate budget accounting. |
| apps/sim/tools/file/write.ts | Extends File Write to store an existing execution file as an alternative to text content. |
Reviews (8): Last reviewed commit: "fix(file): answer null for a key a file ..." | Re-trigger Greptile
isolated-vm.test.ts mocks the spawn, so nothing proved the sim.* namespace was reachable from user code — only that the process plumbing was called. These run the real worker and assert values cross the boundary both ways. Pins the fast runtime's global surface, which turned out narrower than assumed: plain ECMAScript plus fetch, console and sim.*, with no Buffer, require, process, crypto, TextDecoder, atob or setTimeout. That list is exactly what decides whether a block needs an import and so moves to the remote sandbox, so it is asserted rather than described. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
14 issues found and verified against the latest diff
Confidence score: 2/5
apps/sim/tools/function/execute.tscan omit Function inputs files entirely, and advanced-mode JSON file lists can be treated as a single literal file ID, leaving referenced files unavailable to execution — forward and normalizeinputs.filesbefore resolution.apps/sim/executor/variables/resolver.tscan substitute literal__blockRef_Nvalues for.pathreferences in Shell or quoted code, while repeated references create duplicate mounts and transfers — resolve paths through the existing context and deduplicate byfile.key.apps/sim/lib/function-execution/execute-request.tsandapps/sim/lib/execution/remote-sandbox/index.tscan drop harvested outputs or count the same declared and harvested file twice, causing missing results or premature 50 MB-limit failures — merge output sources and exclude already-declared paths from harvesting.- File handling also has boundary and compatibility risks across
apps/sim/lib/execution/payloads/sandbox-file-mount-ref.ts,apps/sim/lib/api/contracts/hotspots.ts,apps/sim/lib/internal/file/operations.ts, andapps/sim/lib/workflows/types.ts:__proto__data can be lost, reference mounts bypass the 20-file limit, oversized downloads can allocate before rejection, and non-Function blocks advertise unsupported paths — harden property creation, enforce combined limits, cap input size, and scope path references to supported blocks.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/sim/tools/function/execute.ts">
<violation number="1" location="apps/sim/tools/function/execute.ts:70">
P1: When advanced-mode supplies `files` as a JSON string, the generic file resolver treats the entire JSON text as one file ID before `normalizeSandboxInputFiles` can parse it. Normalize the serialized array before generic `file[]` resolution or teach that resolver to accept serialized arrays.</violation>
<violation number="2" location="apps/sim/tools/function/execute.ts:170">
P1: When a Function block supplies `files`, `FunctionBlockHandler.execute` drops `inputs.files` while constructing `toolParams`, so the referenced files are never mounted. Forward `inputs.files` into the function tool call.</violation>
</file>
<file name="apps/sim/lib/execution/payloads/sandbox-file-mount-ref.ts">
<violation number="1" location="apps/sim/lib/execution/payloads/sandbox-file-mount-ref.ts:75">
P2: When a context value contains an own `__proto__` field, this assignment invokes `Object.prototype.__proto__` instead of creating a data property, so the field is dropped before the value reaches the sandbox. Define the property explicitly, as `setRecordValue` does, so arbitrary JSON keys survive.</violation>
</file>
<file name="apps/sim/lib/api/contracts/hotspots.ts">
<violation number="1" location="apps/sim/lib/api/contracts/hotspots.ts:196">
P2: When a Function references more than 20 files through `<block.file.path>`, this schema still allows every reference because those mounts bypass `body.files`. Enforce the limit on the combined explicit and reference-derived mount list before resolving or presigning files.</violation>
</file>
<file name="apps/sim/executor/variables/resolver.ts">
<violation number="1" location="apps/sim/executor/variables/resolver.ts:692">
P2: When code references the same file path multiple times, each occurrence creates a separate mount marker, so the runtime mounts and transfers duplicate copies. Deduplicate markers by `file.key` before planning to avoid wasting mount budget and rejecting executions that repeat a large input.</violation>
<violation number="2" location="apps/sim/executor/variables/resolver.ts:694">
P1: When a `.path` reference is used in Shell or inside a quoted JavaScript/Python string, this branch inserts `__blockRef_N` literally instead of reading the mounted path. Format the replacement with the existing context-aware formatter so Shell expands its environment variable and quoted code receives the resolved string.</violation>
</file>
<file name="apps/sim/lib/internal/file/operations.ts">
<violation number="1" location="apps/sim/lib/internal/file/operations.ts:229">
P2: When `fileInput` is 50–100 MiB, this cap allows a full download and base64 allocation before `createWorkspaceFile` rejects the file at its 50 MiB workspace limit. Cap the input at the destination limit so oversized files fail before the expensive transfer.</violation>
</file>
<file name="apps/docs/content/docs/workflows/blocks/function.mdx">
<violation number="1" location="apps/docs/content/docs/workflows/blocks/function.mdx:105">
P3: Adding `<function.files>` leaves the Function FAQ claiming there are only two outputs. Update the FAQ to include `files` and explain that it contains harvested files.</violation>
<violation number="2" location="apps/docs/content/docs/workflows/blocks/function.mdx:133">
P2: When an output file is nested under 12 directories, the sandbox rejects the harvest because depth 12 is treated as truncated. Document the actual maximum of 11 nested directories, or change the harvest depth check to accept the documented 12-level layout.</violation>
</file>
<file name="apps/sim/lib/workflows/types.ts">
<violation number="1" location="apps/sim/lib/workflows/types.ts:23">
P2: When a non-Function block consumes a file reference, this shared list now advertises `<file.path>` as valid, but only Function code converts it into a mount marker; ordinary resolution returns `undefined`. Scope `path` to Function references or reject it for other consumers instead of exposing it through the global file-property list.</violation>
</file>
<file name="apps/sim/lib/function-execution/execute-request.ts">
<violation number="1" location="apps/sim/lib/function-execution/execute-request.ts:2467">
P2: When a request also declares `outputSandboxPath` or `outputSandboxPaths`, files written under `/tmp/sim/outputs` disappear because the declared-export response returns first. Collect and merge harvested files before returning that response.</violation>
</file>
<file name="apps/sim/lib/execution/remote-sandbox/index.ts">
<violation number="1" location="apps/sim/lib/execution/remote-sandbox/index.ts:474">
P2: When user code deletes the output directory, this plain `Error` bypasses the harvest-shape classifier and returns HTTP 500. Raise an error carrying `sandbox_output_not_exportable` so the existing 400 response includes the remediation.</violation>
<violation number="2" location="apps/sim/lib/execution/remote-sandbox/index.ts:480">
P2: When user code writes a file named `.sim-keep`, the sentinel filter silently drops it from the harvested result. Use a collision-resistant internal marker or validate the sentinel before excluding it, and preserve a user-created file with that name.</violation>
<violation number="3" location="apps/sim/lib/execution/remote-sandbox/index.ts:551">
P2: When a declared sandbox output path is inside the harvested directory, this code counts and reads the same file twice. A single output larger than half the 50 MB limit is rejected as oversized; exclude declared paths from the discovered harvest or otherwise de-duplicate the byte accounting.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
Changing function_execute's params and adding file_write's fileInput made both generated artifacts stale, which check:audits catches. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- Classify harvested output by content, not by file name. Gating the provenance scan on a filename-derived MIME type let a resolved secret be written as plaintext under a binary-looking extension and skip the only guard before upload. Bytes that round-trip as UTF-8 are scannable whatever they are called. - Format the .path replacement through the shared context-aware helper. Returning a bare identifier inserted __blockRef_N literally in Shell and inside quoted strings instead of the mounted path. - Enforce the mount ceiling on the combined set. Marker-derived mounts bypassed the contract's max, which only bounded the explicit files param. - Reuse one marker per file key, so referencing a path twice mounts once. - Raise a coded error when the code deletes the output directory, so it reports as a 400 with remediation rather than an opaque 500. - Drop declared paths from the discovered set; counting a file in both rejected a single output larger than half the byte ceiling. - defineProperty when rebuilding context values, so an own __proto__ key survives instead of hitting Object.prototype's setter. - Give the directory sentinel a collision-resistant name, and cap file_write's fileInput at the destination's own limit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@cubic-dev-ai review |
@icecrasher321 I have started the AI code review. It will take a few minutes to complete. |
Adding <function.files> to the outputs table left the FAQ still saying the block returns two outputs, and the reference FAQ never mentioned that a file is read by referencing its path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Gating the scan on whether the bytes looked textual was defeatable twice over: name the file .png, or append a single invalid byte, and a plaintext secret skipped the only guard before upload. A lossy UTF-8 decode preserves ASCII runs, so a literal secret is findable in any buffer. The scan is now unconditional. What remains out of reach is a secret carried in transformed form, which no substring scan can see — an inherent limit of scanning rather than a hole in the gate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@cubic-dev-ai review |
@icecrasher321 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
15 issues found and verified against the latest diff
Confidence score: 2/5
apps/sim/lib/internal/file/operations.tsandapps/sim/tools/file/write.tscan copy a workspace file without its tracked secret provenance, weakening downstream handling of sensitive data — carry the source provenance forward or explicitly mark the destination as untracked.apps/sim/lib/execution/payloads/sandbox-file-mount-ref.ts,apps/sim/lib/api/contracts/hotspots.ts, andapps/sim/lib/function-execution/sandbox-mounts.tstrust caller-provided or stale sizes, allowing oversized cloud objects to bypass sandbox byte ceilings — validate sizes and resolve authoritative stored-object metadata before mounting.apps/sim/executor/variables/resolver.tsformats.pathreferences with literal JSON quotes inside quoted Python or JavaScript strings, producing invalid filenames or paths — use a path-specific formatter that inserts the raw value in the surrounding quotes.apps/sim/tools/index.tsloses the execution-scoped produced-file index across private-metadata calls and does not pass it through Copilot’soperationContext, so later same-turn references may fail; meanwhileapps/sim/lib/execution/remote-sandbox/types.tsandapps/sim/lib/execution/remote-sandbox/index.tscan over-materialize or return more than the output-file limit — use stable shared state and enforce bounded listing/counting before harvest.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/sim/lib/internal/file/operations.ts">
<violation number="1" location="apps/sim/lib/internal/file/operations.ts:783">
P1: When `fileInput` is a workspace file with tracked secret provenance, this copy drops the source provenance because only request `content` provenance reaches `createWorkspaceFile`. Read and carry the source file's bound provenance (or fail closed on unknown provenance) before creating the destination file.</violation>
</file>
<file name="apps/sim/lib/execution/remote-sandbox/types.ts">
<violation number="1" location="apps/sim/lib/execution/remote-sandbox/types.ts:235">
P2: When sandbox code creates many output files, this unbounded recursive listing materializes every entry before the 20-file limit is checked, allowing excessive metadata memory and provider traffic. Add a provider-side entry cap or paginated/streaming listing and stop once the export limit is exceeded.</violation>
</file>
<file name="apps/sim/lib/execution/remote-sandbox/index.ts">
<violation number="1" location="apps/sim/lib/execution/remote-sandbox/index.ts:551">
P2: When a run declares an output outside `outputSandboxDir` and harvests 20 files, this path returns 21 files despite the execution-wide limit. Count the readable declared paths plus the filtered discovered files before reading them.</violation>
</file>
<file name="apps/sim/lib/api/contracts/hotspots.ts">
<violation number="1" location="apps/sim/lib/api/contracts/hotspots.ts:195">
P2: When cloud storage is enabled, a caller can understate `userFile.size` to bypass the per-file and aggregate mount limits because the URL path trusts this metadata before fetching. Resolve the stored object size from the key, or verify the size before issuing the presigned mount URL.</violation>
</file>
<file name="apps/sim/lib/function-execution/sandbox-mounts.ts">
<violation number="1" location="apps/sim/lib/function-execution/sandbox-mounts.ts:114">
P2: When `files[].size` is stale or caller-controlled, this cloud branch can mount an object larger than both limits because the sandbox fetches the unrestricted presigned URL. Fetch authoritative object metadata before presigning, and budget that verified size.</violation>
</file>
<file name="apps/sim/lib/workflows/types.ts">
<violation number="1" location="apps/sim/lib/workflows/types.ts:23">
P2: When `.path` is referenced from a non-Function block, this shared allowlist makes it valid and visible, but no generic resolver materializes a sandbox path, so the reference resolves unsuccessfully. Restrict this property to Function references or add equivalent resolution for other consumers.</violation>
</file>
<file name="apps/docs/content/docs/workflows/blocks/function.mdx">
<violation number="1" location="apps/docs/content/docs/workflows/blocks/function.mdx:105">
P3: The new `files` output conflicts with the Function FAQ, which still says the block has only two outputs. Update the FAQ to include `files` so users can discover and consume harvested files.</violation>
</file>
<file name="apps/sim/lib/execution/payloads/sandbox-file-mount-ref.ts">
<violation number="1" location="apps/sim/lib/execution/payloads/sandbox-file-mount-ref.ts:42">
P1: When a caller supplies a marker with a negative or non-finite `file.size`, `isSandboxFileMountRef` accepts it and the mount layer trusts that value for its byte ceilings. Validate `size` as finite and non-negative before accepting the marker, so a malformed marker cannot cause an oversized object to be fetched directly into the sandbox.</violation>
</file>
<file name="apps/sim/tools/file/write.ts">
<violation number="1" location="apps/sim/tools/file/write.ts:54">
P1: When `fileInput` is a provenance-tracked workspace file, this path copies its bytes into a new workspace file without carrying the source file's secret provenance. Copy the source provenance, or mark the destination unknown before returning it, so materializing a file cannot bypass durable secret-output controls.</violation>
</file>
<file name="apps/sim/lib/function-execution/execute-request.ts">
<violation number="1" location="apps/sim/lib/function-execution/execute-request.ts:2478">
P2: When a remote run declares `outputSandboxPath` and also writes another file under the harvest directory, the declared-output branch returns before `collectExecutionOutputFiles` runs. Collect and include harvested files before returning the declared-output response so automatic harvesting does not silently lose them.</violation>
</file>
<file name="apps/sim/executor/variables/resolver.ts">
<violation number="1" location="apps/sim/executor/variables/resolver.ts:715">
P1: When a `.path` reference is used inside a quoted Python or JavaScript string, the generated filename includes literal JSON quotes. Use a path-specific formatter that inserts the raw context-variable value in quoted contexts, while retaining the existing shell quoting behavior.</violation>
</file>
<file name="apps/sim/providers/utils.ts">
<violation number="1" location="apps/sim/providers/utils.ts:860">
P2: When a mounted file has spaces, path separators, or a duplicate name, this lists the original name even though the sandbox uses a sanitized or suffixed filename. Tell the model to enumerate `SANDBOX_INPUT_DIR` for the exact paths, or provide the planned mount paths, so generated code does not open nonexistent paths.</violation>
</file>
<file name="apps/sim/tools/index.ts">
<violation number="1" location="apps/sim/tools/index.ts:1221">
P1: When a file-producing tool runs through the private-metadata path, the new id is recorded on a per-call context clone and is lost before the next tool call. Store the map on a stable run-shared object, or initialize and share it before these contexts are cloned.</violation>
<violation number="2" location="apps/sim/tools/index.ts:1740">
P1: When Copilot passes an id for a file produced earlier in the same turn, this resolver cannot see the execution-scoped index because Copilot supplies only `operationContext`. Thread the shared execution-file index through the Copilot tool context, or resolve it from the operation context before falling back to the workspace lookup.</violation>
</file>
<file name="apps/sim/lib/execution/remote-sandbox/sandbox-paths.ts">
<violation number="1" location="apps/sim/lib/execution/remote-sandbox/sandbox-paths.ts:44">
P2: When user code writes `.sim-keep-97f2c1a4` at the output root, harvesting silently drops that real file as the setup marker. Use a per-invocation marker whose identity is carried into harvesting, or explicitly reserve and document this filename.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
There was a problem hiding this comment.
16 issues found across 35 files
Confidence score: 1/5
- The highest-risk issue is secret leakage:
apps/sim/tools/file/write.ts,apps/sim/lib/internal/file/operations.ts, andapps/sim/lib/function-execution/execute-request.tscan drop provenance or misclassify transformed/binary secret-derived files, allowing them to reach model-facing output. Preserve source provenance, classify copied bytes correctly, and avoid treating unmatched unknown provenance as secret evidence. - Resource safeguards need tightening in
apps/sim/lib/function-execution/sandbox-mounts.tsandapps/sim/lib/execution/remote-sandbox/index.ts: stale caller sizes, unaccounted existing mounts, and unbounded depth-12 listings can bypass disk or file limits. Validate stored object sizes, share the mount budget, and bound provider listings before materializing entries. apps/sim/executor/variables/resolver.tscan generate literal shell paths inside single quotes and include JSON quote characters in Python or JavaScript string values, causing tool code to read or write the wrong location. Use language-aware escaping and runtime string conversion for mounted paths.apps/sim/executor/types.tslazily creates the tool-state map on per-call shallow clones, so consecutive tools may not share expected state. Initialize one map on the turn context and preserve its reference.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/sim/tools/file/write.ts">
<violation number="1" location="apps/sim/tools/file/write.ts:6">
P1: When `fileInput` references a workspace file containing secret-derived bytes, this new path drops the source file's durable provenance. The destination is therefore stored as exact-empty, allowing later model-facing reads to expose those bytes; resolve and pass through the source provenance, or fail closed as unknown, before storing.</violation>
</file>
<file name="apps/sim/lib/execution/remote-sandbox/output-limits.ts">
<violation number="1" location="apps/sim/lib/execution/remote-sandbox/output-limits.ts:192">
P3: This predicate also returns true for `SandboxOutputDirectoryMissingError`, but its declared union omits that class. Include it so callers cannot narrow a missing-directory failure to the wrong error types.</violation>
</file>
<file name="apps/docs/content/docs/integrations/file.mdx">
<violation number="1" location="apps/docs/content/docs/integrations/file.mdx:98">
P2: The File block cannot currently use this documented binary-file path: its `file_write` mapper drops `fileInput`, and its UI still requires `fileName` and `content`. Expose and map the new input in the block before publishing these optional/file-based requirements, or document this as an agent-tool-only parameter.</violation>
</file>
<file name="apps/sim/executor/types.ts">
<violation number="1" location="apps/sim/executor/types.ts:419">
P1: When Copilot executes consecutive tools, this optional map is lazily initialized on each per-call shallow clone rather than the turn context. Initialize one shared map before tool calls and preserve that reference, otherwise a later Function call cannot resolve a Gmail attachment fetched earlier in the same turn.</violation>
</file>
<file name="apps/sim/lib/workflows/types.ts">
<violation number="1" location="apps/sim/lib/workflows/types.ts:23">
P2: When a user selects `<file.path>` in a non-Function field, this global property list advertises a value that the ordinary resolver cannot materialize. Restrict `.path` to Function code references, or add equivalent runtime handling for every surface that exposes it.</violation>
</file>
<file name="apps/sim/lib/internal/file/operations.ts">
<violation number="1" location="apps/sim/lib/internal/file/operations.ts:783">
P1: When `fileInput` is an existing workspace file with secret provenance, this copy loses the source lineage and records the new file as empty provenance. Read and apply the source file's provenance, including the existing owner/unknown policy, before creating the destination file.</violation>
</file>
<file name="apps/sim/lib/function-execution/execute-request.ts">
<violation number="1" location="apps/sim/lib/function-execution/execute-request.ts:1935">
P1: When code has access to a protected secret and writes a transformed or binary copy under the harvest directory, this `false` argument classifies the bytes as text and allows the secret-bearing file to be returned. Pass `true` for harvested files so any execution with secret material fails closed for binary/unknown output, matching the existing declared-binary export policy.</violation>
<violation number="2" location="apps/sim/lib/function-execution/execute-request.ts:1942">
P2: Do not treat an unknown provenance result with no matched entries as proof that the output contains a secret. This currently rejects unrelated harvested files whenever all referenced secrets are shorter than the matcher threshold; reject only when the scan reports actual secret entries, or make the no-matcher case an explicit non-secret result.</violation>
</file>
<file name="apps/sim/lib/execution/payloads/sandbox-file-mount-ref.ts">
<violation number="1" location="apps/sim/lib/execution/payloads/sandbox-file-mount-ref.ts:65">
P2: When a mounted execution also carries a `Date`, `Buffer`, or other non-plain context value, this branch silently changes that value while rebuilding the context. Preserve non-plain objects or recurse only through arrays and plain records so adding a `.path` reference does not alter unrelated inputs.</violation>
</file>
<file name="apps/sim/executor/variables/resolver.ts">
<violation number="1" location="apps/sim/executor/variables/resolver.ts:715">
P2: When shell code places `<...file.path>` inside single quotes, the generated expansion remains literal because single quotes suppress variable expansion. Make the shell formatter break out of the single-quoted segment before expanding the mounted path.</violation>
<violation number="2" location="apps/sim/executor/variables/resolver.ts:715">
P2: When Python or JavaScript code places `<...file.path>` inside a quoted string, the generated expression includes JSON quote characters in the runtime value. Use runtime string conversion when splicing a mounted path into an existing string literal.</violation>
</file>
<file name="apps/sim/lib/function-execution/sandbox-mounts.ts">
<violation number="1" location="apps/sim/lib/function-execution/sandbox-mounts.ts:109">
P1: When cloud storage is enabled, this check trusts caller-provided file size metadata instead of the stored object's actual size. A small or stale size still presigns the full object, allowing sandbox disk usage to exceed both mount ceilings; verify the object size before presigning.</violation>
<violation number="2" location="apps/sim/lib/function-execution/sandbox-mounts.ts:234">
P2: When a request already has Copilot mounts in `_sandboxFiles`, this fresh budget ignores them and `execute-request.ts` merges both mount sets afterward. Pass a shared budget or account existing mounts before resolving user files so combined URL and inline mounts cannot exceed the advertised request limits.</violation>
</file>
<file name="apps/sim/tools/index.ts">
<violation number="1" location="apps/sim/tools/index.ts:311">
P2: When an execution-scoped file is passed as `{ id, base64 }`, this early return discards the supplied base64; preserve it so binary-consuming tools still receive inline content.</violation>
</file>
<file name="apps/sim/lib/execution/remote-sandbox/index.ts">
<violation number="1" location="apps/sim/lib/execution/remote-sandbox/index.ts:467">
P2: When user code creates many entries under `outputSandboxDir`, this call materializes the entire depth-12 tree before the 20-file limit is applied. Bound or paginate the provider listing, including directories, so untrusted code cannot exhaust provider or web-process memory before rejection.</violation>
</file>
<file name="apps/sim/lib/execution/remote-sandbox/sandbox-files.smoke.test.ts">
<violation number="1" location="apps/sim/lib/execution/remote-sandbox/sandbox-files.smoke.test.ts:149">
P3: The shell case hardcodes the marker `__SIM_RESULT__=` instead of using the imported `SIM_RESULT_PREFIX`, unlike both helpers in this same file (`pythonResult`, `javascriptResult`) and unlike the parser at remote-sandbox/index.ts which matches with `startsWith(SIM_RESULT_PREFIX)`. If the prefix constant ever changes, the Python/JS producers update automatically but this shell producer silently stops being recognized. Reuse the constant so producer and parser cannot drift, matching the file's own stated invariant.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
…charged The write card sent `content` unconditionally, and the contract counts any defined `content` as "text was provided" — so an untouched Content box, which serializes as an empty string, collided with a selected file and rejected every binary write. The mapper now emits only the source the card carries, leaving the contract to catch filling both or neither. A generated document that references other files needs a principal to resolve them; without one the resolver can only serve an already-published artifact and throws. The write path now passes it. URL mounts are granted exactly the byte count they were charged against the aggregate, rather than being charged a reported size while permitted the global per-file maximum — twenty mounts each claiming a byte could otherwise be allowed 500MB apiece. An honest size fetches normally and an understated one is refused. `--max-filesize` only refuses a transfer up front when the response declares a Content-Length, so a chunked reply slipped past it. The delivered file is now measured and removed if it overran, in the same command rather than a second round trip. Also documents that naming an explicit sandbox output path excludes the harvest directory, in both the block docs and the tool description the model reads. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@cubic-dev-ai review |
@icecrasher321 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 existing issue remains and 7 new issues found across 37 files
Confidence score: 2/5
apps/sim/lib/execution/remote-sandbox/index.tsandapps/sim/lib/execution/remote-sandbox/output-limits.tscan write an entire chunked or lengthless response before enforcing size limits, creating a resource-exhaustion risk; enforce the cap while streaming or reject responses without a usable length.apps/sim/blocks/blocks/file.tscan pass the new picker shape directly tofile_write, rejecting valid writes, and also treats an intentional empty string as missing input; convert the picker value toUserFileand track the selected source separately from its content.apps/sim/tools/function/execute.tscan resolve a JSON-encoded advancedfilesvalue as one file ID before parsing it, whileapps/sim/lib/function-execution/sandbox-mounts.tscan bypass the aggregate URL-byte limit for invalid sizes; parse before normalization and validate or normalizedeclaredSizebefore aggregation.apps/sim/lib/function-execution/execute-request.tsturns unauthorized or over-limit mounts into 500 responses, andapps/sim/executor/variables/resolver.tscan generate invalid Python for path references inside f-strings; classify mount input errors as 400s and handle f-string context explicitly.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/sim/lib/function-execution/execute-request.ts">
<violation number="1" location="apps/sim/lib/function-execution/execute-request.ts:2374">
P2: When a requested mount is unauthorized or exceeds a mount limit, this resolver throws and the Function route returns 500. Catch and classify these caller-input failures as a 400 response with actionable guidance.</violation>
</file>
<file name="apps/sim/lib/execution/remote-sandbox/output-limits.ts">
<violation number="1" location="apps/sim/lib/execution/remote-sandbox/output-limits.ts:13">
P2: When a URL response omits `Content-Length`, `curl --max-filesize` allows the entire response, and the sandbox writes it before `wc -c` removes it. Enforce the cap while streaming or reject lengthless responses, otherwise a mounted object can exhaust sandbox disk despite this limit.</violation>
</file>
<file name="apps/sim/executor/variables/resolver.ts">
<violation number="1" location="apps/sim/executor/variables/resolver.ts:746">
P2: When a Python path reference appears inside an f-string expression, this quote-context handling inserts string concatenation around the reference while leaving the `{}` delimiters, producing invalid Python. Detect f-string expression context and emit the `globals()` expression directly there.</violation>
</file>
<file name="apps/sim/lib/function-execution/sandbox-mounts.ts">
<violation number="1" location="apps/sim/lib/function-execution/sandbox-mounts.ts:115">
P2: When a URL mount has a non-finite or missing `declaredSize`, this aggregate check becomes `NaN > MAX_TOTAL_URL_BYTES`, so it never fires while the later fallback charges the per-file maximum. Normalize the size before this check, or reject malformed metadata, so the total URL budget remains enforced.</violation>
</file>
<file name="apps/sim/lib/execution/remote-sandbox/index.ts">
<violation number="1" location="apps/sim/lib/execution/remote-sandbox/index.ts:243">
P1: When a presigned response is chunked or omits `Content-Length`, `--max-filesize` does not stop it, so this command writes the entire object before `wc -c` rejects it. Stream through a byte-limited reader or provider API and abort after `MAX_BYTES + 1` bytes so a stale size cannot exhaust sandbox disk and timeout the run.</violation>
</file>
<file name="apps/sim/blocks/blocks/file.ts">
<violation number="1" location="apps/sim/blocks/blocks/file.ts:1238">
P2: When a workflow intentionally writes an empty text file, `hasText` becomes false and the request is rejected instead of creating the zero-byte file. Track the selected source explicitly so an empty string remains valid text while an untouched text field does not collide with `fileInput`.</violation>
<violation number="2" location="apps/sim/blocks/blocks/file.ts:1242">
P1: When Write uses the new basic file picker, this line forwards the picker shape unchanged, so `file_write` rejects it as not a file object before copying any bytes. Convert the picker value to the server’s `UserFile` shape or resolve it by storage key before sending `fileInput`.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Tip: instead of fixing issues one by one fix them all with cubic
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
The write path required an already-complete UserFile, but the file picker
stores {name, path, key, size, type} with no id or url — so selecting a
file in the new basic field was rejected as not a file object before any
bytes moved. It now runs through the same normalizer every other operation
in that file applies to its own input, while a block reference or an
agent-resolved id still passes through as-is.
Writing a deliberately empty text file was also rejected, because an empty
string read as "no text". The selected file is what disambiguates now: with
one present an empty Content box means unused, and with none, content
always goes through.
--max-filesize only refuses a transfer that declares a Content-Length, so
the previous size check ran after the whole object had already been written
to sandbox disk. The fetch now streams through a byte cap, so at most one
byte over the limit can ever land, and curl's status travels through a file
so a 403 on an expired URL is still distinguishable from an empty download.
A non-finite mount size made every comparison false, so the aggregate check
passed while the mount was charged the per-file maximum anyway; the size is
resolved once now, before either test.
Mount resolution failures are the caller's files — unreadable, oversized,
or over the aggregate — and now answer 400 with the message naming the
file, rather than a 500. Files already uploaded when a later one in the
same harvest is refused are removed, since the export is all-or-nothing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@cubic-dev-ai review |
@icecrasher321 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
2 existing issues remain and no new issues found across 37 files
Confidence score: 3/5
apps/sim/lib/execution/remote-sandbox/output-limits.tsmaterializes every output entry in both providers before enforcingfiles.length, so sandboxes with many files can produce oversized listing responses and unnecessary resource use; add provider-side limiting or pagination.apps/sim/lib/function-execution/sandbox-mounts.tsallocates separate mounts for duplicate file references but key-based replacement makes all references resolve to the last path, which can mount the wrong file; deduplicate repeated keys before allocating mount names.
Requires human review: Auto-approval blocked because this review re-detected 2 unresolved issues already reported by Cubic.
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Fix all with cubic | Re-trigger cubic
…etadata sandboxFiles reaches the sandbox layer from the request body, so a declared maxBytes is a caller's number. It may now lower its own mount's ceiling but never raise it past the one that layer guarantees. The write path required full UserFile metadata, but size is never read before the download and the download reports the real content type — so a reference carrying id, key, url and name was rejected over two fields nothing depends on. It now asks only for identity and fills the rest. Uploads already made are also discarded when a later upload throws, not only when a later file is refused for carrying a secret. Both exits leave the harvest all-or-nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@cubic-dev-ai review |
@icecrasher321 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
2 existing issues remain and no new issues found across 37 files
Confidence score: 3/5
- In
apps/sim/lib/execution/remote-sandbox/index.ts, recursive output listing materializes all entries before enforcing the 20-file limit, so a sandbox producing many files could cause excessive memory or provider load—add a provider-side cap or paginated/early-stop listing. - In
apps/sim/lib/execution/remote-sandbox/index.ts, user output named.sim-keep-97f2c1a4is silently omitted because it matches the setup sentinel, causing data loss—use a per-execution sentinel or distinguish setup metadata from harvested files.
Requires human review: Auto-approval blocked because this review re-detected 2 unresolved issues already reported by Cubic.
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Fix all with cubic | Re-trigger cubic
fileInputToUserFile normalizes caller-supplied file objects and returns null for anything it cannot use — but it classified the storage key with the throwing form, so a key without a recognized context prefix escaped as a 500 from every operation that normalizes a file input, not just write. Adds tryInferContextFromKey beside inferContextFromKey, sharing the one prefix list so a new context cannot be added to half of them. The throwing form stays right where an unclassifiable key means the platform built one wrong; the nullable form is for keys that arrived in a request, where an unrecognized prefix only means this is not a file we can use. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@cubic-dev-ai review |
@icecrasher321 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 39 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
What
Reference a file's path in Function block code and it is mounted for you:
.pathjoinsbase64inUSER_FILE_ACCESSIBLE_PROPERTIES, so it shows up in the reference dropdown next to.nameand.sizewith no extra wiring. Anything written to/tmp/sim/outputscomes back asfiles— platform file objects a Gmail attachment or an S3 upload accepts directly.Why this shape
.base64already inlines a file's contents, but it is gated on the JavaScript runtime helpers, so Python and Shell had no way to read a referenced file at all..pathis the counterpart: any language can open a path, which is what pandas, ffmpeg, or a CLI actually needs.Auto-mounting any referenced file was the obvious alternative and does not work: an existing plain-JS block doing
<gmail.attachments>[0].namewould silently move to the remote sandbox, and would hard-fail 422 where none is configured. Making it a property keeps it opt-in per reference, so nothing existing changes.Reference resolution happens long before a sandbox exists, and mount paths are assigned only once the whole set is planned together (they are sanitized and de-duplicated as a group). So the resolver leaves a marker — same shape as
LargeValueRef— that the function runtime swaps for the real path.There is deliberately no files subblock and no "return files" toggle. Output harvesting rides runs already in the remote sandbox, so isolate runs pay nothing.
Also here
file_writegained afileInputparam, so a binary result can be materialized into the workspace. Every file-producing tool benefits, not just Function.buildParameterSchemaemitted{"type": "file"}on the agent surface, which is not a JSON Schema type. Fixed; it now emits a reference string.Bugs found by testing against real providers
A real-provider smoke suite (
SANDBOX_FILES_SMOKE=1) surfaced four issues that mocks did not:execute-request. Output-directory creation had drifted into the caller's runtime prologue whileoutputSandboxDirwas a sandbox-layer field, so any direct caller gotENOENTon the first write. Ownership moved to the layer that owns the contract./etc/passwdinto the output directory itself. Both now follow links identically.Testing
check:api-validation, and lint clean..tar.gzand extension-less binaries, zero-byte files, mount-and-read from Python, JavaScript, and Shell..pathmount, writes two files) → Gmail with both attached.Not covered: the presigned-URL mount branch. Local dev has no cloud storage, so
hasCloudStorage()is false and mounts take the inline-buffered path; production uses the URL path. Worth exercising in an environment with S3/Blob/GCS before this carries real traffic.🤖 Generated with Claude Code