Skip to content

Commit 43f37cf

Browse files
icecrasher321claude
andcommitted
fix(file): clamp a declared mount cap, and demand identity not full metadata
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>
1 parent df197a0 commit 43f37cf

3 files changed

Lines changed: 80 additions & 54 deletions

File tree

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,13 @@ async function writeSandboxInputs(
269269
URL: file.url,
270270
DST: file.path,
271271
DIR: dir,
272-
MAX_BYTES: String(file.maxBytes ?? MAX_SANDBOX_URL_MOUNT_BYTES),
272+
// Clamped, not just defaulted: `sandboxFiles` reaches this layer from
273+
// the request body, so a declared ceiling is a caller's number. It may
274+
// lower the limit for its own mount but never raise it past the one
275+
// this layer guarantees.
276+
MAX_BYTES: String(
277+
Math.min(file.maxBytes ?? MAX_SANDBOX_URL_MOUNT_BYTES, MAX_SANDBOX_URL_MOUNT_BYTES)
278+
),
273279
},
274280
timeoutMs: Math.min(300_000, remainingSandboxBudgetMs(opts.signal)),
275281
maxOutputBytes: MAX_SANDBOX_PROCESS_OUTPUT_BYTES,

apps/sim/lib/function-execution/execute-request.ts

Lines changed: 53 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1955,54 +1955,62 @@ async function collectExecutionOutputFiles(args: {
19551955
}
19561956

19571957
const files: UserFile[] = []
1958-
for (const collected of args.collectedFiles) {
1959-
const buffer = Buffer.from(collected.contentBase64, 'base64')
1960-
const name = collectedFileName(collected.relativePath)
1961-
const mimeType = getMimeTypeFromExtension(getFileExtension(name))
1962-
1963-
// Scanned unconditionally — never gated on whether the bytes look textual.
1964-
// Both a filename check and a UTF-8 round-trip were trivially defeated: name
1965-
// the file `.png`, or append one invalid byte, and a plaintext secret sailed
1966-
// past. A lossy UTF-8 decode preserves ASCII runs, so a literal secret is
1967-
// findable in any buffer, textual or not.
1968-
//
1969-
// What stays out of reach is a secret carried in transformed form — deflated
1970-
// inside a PDF, re-encoded — which no substring scan can see. That is an
1971-
// inherent limit of scanning, not a hole in the gate, and it is why these
1972-
// files are execution-scoped rather than durable workspace files.
1973-
{
1974-
const provenance = await getOutputFileSecretProvenance(buffer, false, routeContext, {
1975-
userId: args.authUserId,
1976-
workspaceId: resolvedWorkspaceId,
1977-
})
1978-
// An execution-scoped file has nowhere to record a provenance envelope, so
1979-
// one carrying a resolved secret cannot ship under a lock the way a
1980-
// workspace file can — it is refused instead.
1981-
if (provenance.status !== 'exact' || provenance.entries.length > 0) {
1982-
await discardUploadedExecutionFiles(files)
1983-
return {
1984-
response: exportFailure(
1985-
`Sandbox output file "${name}" contains a resolved secret value and was not returned. Write the file without embedding secret values, or export it to a workspace file where its provenance can be recorded.`,
1986-
400,
1987-
args.stdout,
1988-
args.executionTime
1989-
),
1958+
// The harvest is all-or-nothing, so a throw partway through has to take the
1959+
// uploads that already succeeded with it. Without this they linger in storage
1960+
// with nothing referencing them, since the failure response carries no keys.
1961+
try {
1962+
for (const collected of args.collectedFiles) {
1963+
const buffer = Buffer.from(collected.contentBase64, 'base64')
1964+
const name = collectedFileName(collected.relativePath)
1965+
const mimeType = getMimeTypeFromExtension(getFileExtension(name))
1966+
1967+
// Scanned unconditionally — never gated on whether the bytes look textual.
1968+
// Both a filename check and a UTF-8 round-trip were trivially defeated: name
1969+
// the file `.png`, or append one invalid byte, and a plaintext secret sailed
1970+
// past. A lossy UTF-8 decode preserves ASCII runs, so a literal secret is
1971+
// findable in any buffer, textual or not.
1972+
//
1973+
// What stays out of reach is a secret carried in transformed form — deflated
1974+
// inside a PDF, re-encoded — which no substring scan can see. That is an
1975+
// inherent limit of scanning, not a hole in the gate, and it is why these
1976+
// files are execution-scoped rather than durable workspace files.
1977+
{
1978+
const provenance = await getOutputFileSecretProvenance(buffer, false, routeContext, {
1979+
userId: args.authUserId,
1980+
workspaceId: resolvedWorkspaceId,
1981+
})
1982+
// An execution-scoped file has nowhere to record a provenance envelope, so
1983+
// one carrying a resolved secret cannot ship under a lock the way a
1984+
// workspace file can — it is refused instead.
1985+
if (provenance.status !== 'exact' || provenance.entries.length > 0) {
1986+
await discardUploadedExecutionFiles(files)
1987+
return {
1988+
response: exportFailure(
1989+
`Sandbox output file "${name}" contains a resolved secret value and was not returned. Write the file without embedding secret values, or export it to a workspace file where its provenance can be recorded.`,
1990+
400,
1991+
args.stdout,
1992+
args.executionTime
1993+
),
1994+
}
19901995
}
19911996
}
1992-
}
19931997

1994-
const userFile = await uploadExecutionFile(
1995-
{
1996-
workspaceId: resolvedWorkspaceId,
1997-
workflowId: args.workflowId,
1998-
executionId: args.executionId,
1999-
},
2000-
buffer,
2001-
name,
2002-
mimeType,
2003-
args.authUserId
2004-
)
2005-
files.push(userFile)
1998+
const userFile = await uploadExecutionFile(
1999+
{
2000+
workspaceId: resolvedWorkspaceId,
2001+
workflowId: args.workflowId,
2002+
executionId: args.executionId,
2003+
},
2004+
buffer,
2005+
name,
2006+
mimeType,
2007+
args.authUserId
2008+
)
2009+
files.push(userFile)
2010+
}
2011+
} catch (error) {
2012+
await discardUploadedExecutionFiles(files)
2013+
throw error
20062014
}
20072015

20082016
// Registers the new keys on the execution so downstream blocks are authorized

apps/sim/lib/internal/file/operations.ts

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { acquireLock, releaseLock } from '@/lib/core/config/redis'
1212
import { OrchestrationError } from '@/lib/core/orchestration/types'
1313
import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
1414
import { ensureAbsoluteUrl } from '@/lib/core/utils/urls'
15-
import { isUserFileWithMetadata } from '@/lib/core/utils/user-file'
15+
import { isUserFile } from '@/lib/core/utils/user-file'
1616
import { durableSecretProvenanceFromPrivateBundle } from '@/lib/execution/durable-secret-provenance'
1717
import {
1818
inspectPrivateSecretProvenanceRequest,
@@ -791,13 +791,25 @@ export async function executeFileManageOperation(
791791
*/
792792
let inputProvenance: WorkspaceFileSecretProvenance | undefined
793793
if (fileInput !== undefined && fileInput !== null) {
794-
// Two shapes reach here and only one is already a UserFile. A block
795-
// reference or an agent-resolved id arrives complete; the file picker
796-
// stores `{name, path, key, size, type}` with no `id` or `url`, which
797-
// the shared normalizer turns into one — the same conversion every
798-
// other operation in this file applies to its own file input.
799-
const sourceFile: UserFile | null = isUserFileWithMetadata(fileInput)
800-
? fileInput
794+
/**
795+
* Two shapes reach here and only one already identifies a file. A block
796+
* reference, or an id the tool layer resolved through the execution
797+
* index or workspace metadata, arrives carrying `id`/`key`/`url`/`name`.
798+
* The file picker instead stores `{name, path, key, size, type}` with no
799+
* `id` or `url`, which the shared normalizer turns into one — the same
800+
* conversion every other operation in this file applies to its input.
801+
*
802+
* Identity is all that is demanded, deliberately. `size` is never read
803+
* before the download and the download reports the real content type, so
804+
* requiring them would reject an otherwise usable reference over two
805+
* fields nothing depends on.
806+
*/
807+
const sourceFile: UserFile | null = isUserFile(fileInput)
808+
? {
809+
...fileInput,
810+
size: fileInput.size ?? 0,
811+
type: fileInput.type ?? 'application/octet-stream',
812+
}
801813
: fileInputToUserFile(fileInput)
802814
if (!sourceFile) {
803815
return Response.json(

0 commit comments

Comments
 (0)