Backport R2 copy source prefix fix

.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.

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<string>,
-  mountPrefix?: string
+  buckets: R2EgressParams['buckets']
 ): Promise<Response> {
   const copySourceHeader = request.headers.get('x-amz-copy-source');
   if (copySourceHeader) {
@@ -502,14 +506,15 @@ 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.',
         { status: 403 }
       );
     }
 
+    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);

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<string, MockObject>([
+        [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<string, MockObject>([
+        [`${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<string, MockObject>([
+        [`${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<string, MockObject>([
+        ['source.txt', { body: 'cross binding' }]
+      ]);
+      const destinationStore = new Map<string, MockObject>();
+      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<string, MockObject>([
         ['source.txt', { body: 'copy me', contentType: 'text/plain' }]