From 2d6f146bb399d9c3a82f9104a8fa970f392f03d8 Mon Sep 17 00:00:00 2001 From: scuffi Date: Mon, 24 Aug 2026 15:31:15 +0100 Subject: [PATCH] Fix R2 copy source prefix scoping --- .changeset/secure-r2-copy-source-prefix.md | 5 + .../outbound/r2-egress-handler.ts | 28 ++--- .../sandbox/tests/r2-egress-handler.test.ts | 109 ++++++++++++++++++ 3 files changed, 128 insertions(+), 14 deletions(-) create mode 100644 .changeset/secure-r2-copy-source-prefix.md diff --git a/.changeset/secure-r2-copy-source-prefix.md b/.changeset/secure-r2-copy-source-prefix.md new file mode 100644 index 000000000..046f8df88 --- /dev/null +++ b/.changeset/secure-r2-copy-source-prefix.md @@ -0,0 +1,5 @@ +--- +'@cloudflare/sandbox': patch +--- + +Enforce source mount prefixes for server-side copies between R2 bindings. diff --git a/packages/sandbox/src/storage-mount/outbound/r2-egress-handler.ts b/packages/sandbox/src/storage-mount/outbound/r2-egress-handler.ts index 55c7a34bb..66950acec 100644 --- a/packages/sandbox/src/storage-mount/outbound/r2-egress-handler.ts +++ b/packages/sandbox/src/storage-mount/outbound/r2-egress-handler.ts @@ -68,6 +68,11 @@ function trimTrailingSlashes(s: string): string { return s.slice(0, end); } +function normalizeMountPrefix(prefix: string | undefined): string | undefined { + if (!prefix) return undefined; + return trimTrailingSlashes(normalizeObjectKey(prefix)) || undefined; +} + function parsePath(pathname: string): ParsedPath | null { const stripped = pathname.startsWith('/') ? pathname.slice(1) : pathname; if (!stripped) return null; @@ -490,8 +495,7 @@ async function handlePutObject( key: string, request: Request, env: Cloudflare.Env, - permitted: Set, - mountPrefix?: string + buckets: R2EgressParams['buckets'] ): Promise { const copySourceHeader = request.headers.get('x-amz-copy-source'); if (copySourceHeader) { @@ -502,7 +506,7 @@ async function handlePutObject( }); } - if (!permitted.has(copySource.bucket)) { + if (!Object.hasOwn(buckets, copySource.bucket)) { return new Response( `Access to R2 bucket "${copySource.bucket}" is not permitted. ` + 'Call mountBucket() with this bucket before accessing it.', @@ -510,6 +514,7 @@ async function handlePutObject( ); } + const sourceParams = buckets[copySource.bucket]; const sourceBucket = copySource.bucket === bucketName ? r2 @@ -522,10 +527,10 @@ async function handlePutObject( ); } - const sourceKey = - mountPrefix && copySource.bucket === bucketName - ? `${mountPrefix}/${copySource.key}` - : copySource.key; + const sourcePrefix = normalizeMountPrefix(sourceParams.prefix); + const sourceKey = sourcePrefix + ? `${sourcePrefix}/${copySource.key}` + : copySource.key; const sourceObject = await sourceBucket.get(sourceKey); if (!sourceObject) { return new Response(null, { status: 404 }); @@ -679,10 +684,7 @@ export const r2EgressHandler: OutboundHandler< } const bucketParams = ctx.params.buckets[bucketName]; - const rawPrefix = bucketParams.prefix; - const mountPrefix = rawPrefix - ? trimTrailingSlashes(normalizeObjectKey(rawPrefix)) - : undefined; + const mountPrefix = normalizeMountPrefix(bucketParams.prefix); const readOnly = bucketParams.readOnly ?? false; const r2 = resolveR2Bucket(env, bucketName); @@ -712,7 +714,6 @@ export const r2EgressHandler: OutboundHandler< } const fullKey = mountPrefix ? `${mountPrefix}/${key}` : key; - const permitted = new Set(Object.keys(ctx.params.buckets)); if ( readOnly && @@ -763,8 +764,7 @@ export const r2EgressHandler: OutboundHandler< fullKey, request, env, - permitted, - mountPrefix + ctx.params.buckets ); case 'DELETE': return handleDeleteObject(r2, fullKey); diff --git a/packages/sandbox/tests/r2-egress-handler.test.ts b/packages/sandbox/tests/r2-egress-handler.test.ts index c60c5077d..5dc5cc8dd 100644 --- a/packages/sandbox/tests/r2-egress-handler.test.ts +++ b/packages/sandbox/tests/r2-egress-handler.test.ts @@ -486,6 +486,115 @@ describe('r2EgressHandler', () => { }); }); + it('does not copy a raw physical key outside a cross-binding source prefix', async () => { + const victimKey = 'victim/secret.txt'; + const victimBody = 'VICTIM_CANARY_DO_NOT_COPY'; + const docsPrefix = 'tenant-a/ws/documents'; + const sessionPrefix = 'tenant-a/ws/sandboxes/sess-1'; + const stolenKey = `${sessionPrefix}/stolen.txt`; + const store = new Map([ + [victimKey, { body: victimBody }], + [`${docsPrefix}/readme.txt`, { body: 'hello from documents' }] + ]); + const r2 = createMockR2Bucket(store); + + const res = await r2EgressHandler( + new Request('http://r2.internal/SESSION/stolen.txt', { + method: 'PUT', + headers: { 'x-amz-copy-source': `/DOCS/${victimKey}` } + }), + { DOCS: r2, SESSION: r2 } as unknown as Cloudflare.Env, + makeCtx({ + buckets: { + DOCS: { prefix: `/${docsPrefix}/`, readOnly: true }, + SESSION: { prefix: `/${sessionPrefix}/` } + } + }) + ); + + expect(res.status).toBe(404); + expect(r2.get).toHaveBeenCalledWith(`${docsPrefix}/${victimKey}`); + expect(store.has(stolenKey)).toBe(false); + expect(store.get(victimKey)?.body).toBe(victimBody); + }); + + it('copies an in-prefix object between prefixed bindings', async () => { + const docsPrefix = 'tenant-a/ws/documents'; + const sessionPrefix = 'tenant-a/ws/sandboxes/sess-1'; + const store = new Map([ + [`${docsPrefix}/readme.txt`, { body: 'hello from documents' }] + ]); + const r2 = createMockR2Bucket(store); + + const res = await r2EgressHandler( + new Request('http://r2.internal/SESSION/copied.txt', { + method: 'PUT', + headers: { 'x-amz-copy-source': '/DOCS/readme.txt' } + }), + { DOCS: r2, SESSION: r2 } as unknown as Cloudflare.Env, + makeCtx({ + buckets: { + DOCS: { prefix: `/${docsPrefix}/`, readOnly: true }, + SESSION: { prefix: `/${sessionPrefix}/` } + } + }) + ); + + expect(res.status).toBe(200); + expect(r2.get).toHaveBeenCalledWith(`${docsPrefix}/readme.txt`); + expect(store.get(`${sessionPrefix}/copied.txt`)?.body).toBe( + 'hello from documents' + ); + }); + + it('copies an object within the same prefixed binding', async () => { + const prefix = 'tenant-a/ws/sandboxes/sess-1'; + const store = new Map([ + [`${prefix}/source.txt`, { body: 'same binding' }] + ]); + const r2 = createMockR2Bucket(store); + + const res = await r2EgressHandler( + new Request('http://r2.internal/SESSION/copied.txt', { + method: 'PUT', + headers: { 'x-amz-copy-source': '/SESSION/source.txt' } + }), + { SESSION: r2 } as unknown as Cloudflare.Env, + makeCtx({ buckets: { SESSION: { prefix: `/${prefix}/` } } }) + ); + + expect(res.status).toBe(200); + expect(r2.get).toHaveBeenCalledWith(`${prefix}/source.txt`); + expect(store.get(`${prefix}/copied.txt`)?.body).toBe('same binding'); + }); + + it('copies an object between unprefixed bindings', async () => { + const sourceStore = new Map([ + ['source.txt', { body: 'cross binding' }] + ]); + const destinationStore = new Map(); + const source = createMockR2Bucket(sourceStore); + const destination = createMockR2Bucket(destinationStore); + + const res = await r2EgressHandler( + new Request('http://r2.internal/SESSION/copied.txt', { + method: 'PUT', + headers: { 'x-amz-copy-source': '/DOCS/source.txt' } + }), + { DOCS: source, SESSION: destination } as unknown as Cloudflare.Env, + makeCtx({ + buckets: { + DOCS: { readOnly: true }, + SESSION: {} + } + }) + ); + + expect(res.status).toBe(200); + expect(source.get).toHaveBeenCalledWith('source.txt'); + expect(destinationStore.get('copied.txt')?.body).toBe('cross binding'); + }); + it('replaces metadata on copy when requested', async () => { const store = new Map([ ['source.txt', { body: 'copy me', contentType: 'text/plain' }]