PUT-1432 (#3539)

src/backend/drivers/subdomain/SubdomainDriver.edges.test.ts

@@ -28,6 +28,7 @@
 
 import { v4 as uuidv4 } from 'uuid';
 import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest';
+import { makeActor } from '../../core/actor.js';
 import type { Actor } from '../../core/actor.js';
 import { runWithContext } from '../../core/context.js';
 import { PuterServer } from '../../server.js';
@@ -281,10 +282,17 @@ describe('SubdomainDriver app-actor scoping', () => {
             appOwner,
         });
 
+    // Built through `makeActor` so `effectiveApp` is derived the way the auth
+    // path derives it. An actor literal carrying only `app` leaves it
+    // undefined, which the read gate reads as "no app acting" — such an actor
+    // would sail past the very scoping these tests exist to pin.
+    const asApp = (base: Actor, app: { uid: string; id: number }): Actor =>
+        makeActor({ ...base, app: { uid: app.uid, id: app.id } });
+
     it('lets the owning app read and update the row', async () => {
         const { actor, userId } = await makeUser();
         const app = await seedApp(userId, 'owner-app');
-        const appActor: Actor = { ...actor, app: { uid: app.uid, id: app.id } };
+        const appActor: Actor = asApp(actor, app);
         const name = uniqueName('appscope');
         const row = await seedRow(userId, name, app.id);
 
@@ -310,9 +318,8 @@ describe('SubdomainDriver app-actor scoping', () => {
         const row = await seedRow(userId, uniqueName('appdeny'), ownerApp.id);
 
         await expect(
-            withActor(
-                { ...actor, app: { uid: otherApp.uid, id: otherApp.id } },
-                () => driver.delete({ uid: row.uuid }),
+            withActor(asApp(actor, otherApp), () =>
+                driver.delete({ uid: row.uuid }),
             ),
         ).rejects.toMatchObject({ statusCode: 403, legacyCode: 'forbidden' });
     });
@@ -324,32 +331,88 @@ describe('SubdomainDriver app-actor scoping', () => {
         const row = await seedRow(owner.userId, uniqueName('appmix'), app.id);
 
         await expect(
-            withActor(
-                { ...stranger.actor, app: { uid: app.uid, id: app.id } },
-                () => driver.delete({ uid: row.uuid }),
+            withActor(asApp(stranger.actor, app), () =>
+                driver.delete({ uid: row.uuid }),
             ),
         ).rejects.toMatchObject({ statusCode: 403, legacyCode: 'forbidden' });
     });
 
-    it('lets an app read a row it owns even when the caller is a different user', async () => {
+    it('refuses an app reading a row of a different user that shares its app_owner', async () => {
         const owner = await makeUser();
         const stranger = await makeUser();
         const app = await seedApp(owner.userId, 'reader-app');
         const name = uniqueName('appread');
         const row = await seedRow(owner.userId, name, app.id);
 
-        const read = (await withActor(
-            { ...stranger.actor, app: { uid: app.uid, id: app.id } },
-            () => driver.read({ uid: row.uuid }),
-        )) as Record<string, unknown>;
+        // `app_owner` is a global app id, so every user of an app shares it.
+        // Granting on that alone would hand the owner's username, uuid and
+        // home path to anyone else acting under the same app.
+        await expect(
+            withActor(asApp(stranger.actor, app), () =>
+                driver.read({ uid: row.uuid }),
+            ),
+        ).rejects.toMatchObject({ statusCode: 403, legacyCode: 'forbidden' });
 
-        expect(read.subdomain).toBe(name);
+        await expect(
+            withActor(asApp(stranger.actor, app), () =>
+                driver.read({ id: { subdomain: name } }),
+            ),
+        ).rejects.toMatchObject({ statusCode: 403, legacyCode: 'forbidden' });
+    });
+
+    it('refuses an app reading its own user rows that belong to another app', async () => {
+        const { actor, userId } = await makeUser();
+        const ownerApp = await seedApp(userId, 'creator-app');
+        const otherApp = await seedApp(userId, 'nosy-app');
+        const scoped = uniqueName('appscoped');
+        const loose = uniqueName('apploose');
+        const scopedRow = await seedRow(userId, scoped, ownerApp.id);
+        const looseRow = await seedRow(userId, loose, null);
+
+        // Same scoping `select` applies: an app sees what it created, not
+        // everything its user owns. Rows with no owning app included.
+        for (const uid of [scopedRow.uuid, looseRow.uuid]) {
+            await expect(
+                withActor(asApp(actor, otherApp), () => driver.read({ uid })),
+            ).rejects.toMatchObject({
+                statusCode: 403,
+                legacyCode: 'forbidden',
+            });
+        }
+
+        // The user acting directly still reads both.
+        for (const uid of [scopedRow.uuid, looseRow.uuid]) {
+            const read = (await withActor(actor, () =>
+                driver.read({ uid }),
+            )) as Record<string, unknown>;
+            expect(read.uid).toBe(uid);
+        }
+    });
+
+    it('does not answer for worker rows, which belong to the workers driver', async () => {
+        const { actor, userId } = await makeUser();
+        const name = uniqueName('wk');
+        const row = await seedRow(userId, `workers.puter.${name}`, null);
+
+        // Same 404 as a miss: the name resolves globally, so a distinct
+        // refusal would confirm the row exists.
+        for (const args of [
+            { uid: row.uuid },
+            { id: { subdomain: `workers.puter.${name}` } },
+        ]) {
+            await expect(
+                withActor(actor, () => driver.read(args)),
+            ).rejects.toMatchObject({
+                statusCode: 404,
+                legacyCode: 'not_found',
+            });
+        }
     });
 
     it('scopes select to the acting app', async () => {
         const { actor, userId } = await makeUser();
         const app = await seedApp(userId, 'listing-app');
-        const appActor: Actor = { ...actor, app: { uid: app.uid, id: app.id } };
+        const appActor: Actor = asApp(actor, app);
         const appOwned = uniqueName('applist');
         const userOwned = uniqueName('userlist');
         await seedRow(userId, appOwned, app.id);

src/backend/drivers/subdomain/SubdomainDriver.ts

@@ -73,9 +73,11 @@ const RESERVED_SUBDOMAINS = new Set([
  * Permission model:
  *
  * - Owner (user_id) can read/write their own subdomains
- * - App actor matching app_owner can read/write scoped subdomains
+ * - An app actor is further scoped to the rows it created (app_owner), for reads
+ *   as well as writes — never widened past its own user
  * - `system:es:write-all-owners` grants blanket write
- * - `read-all-subdomains` grants cross-user reads
+ * - `read-all-subdomains` grants cross-user reads, and is the only thing that
+ *   does
  */
 export class SubdomainDriver extends PuterDriver {
     readonly driverInterface = 'puter-subdomains';
@@ -236,7 +238,16 @@ export class SubdomainDriver extends PuterDriver {
     async read(args: Record<string, unknown>): Promise<unknown> {
         const actor = this.#requireActor();
         const row = await this.#resolve(args);
-        if (!row)
+        // Worker deployments live in this table but aren't sites. `select`
+        // excludes them and the workers driver serves them under its own
+        // scoping, so answering for them here would make the hosting API a
+        // by-name lookup for objects it doesn't manage. Same 404 as a miss:
+        // the name is resolved globally, so a distinct refusal would confirm
+        // the row exists.
+        const isWorkerRow =
+            typeof row?.subdomain === 'string' &&
+            row.subdomain.startsWith(WORKER_SUBDOMAIN_PREFIX);
+        if (!row || isWorkerRow)
             throw new HttpError(404, 'Subdomain not found', {
                 legacyCode: 'not_found',
             });
@@ -527,14 +538,31 @@ export class SubdomainDriver extends PuterDriver {
         }
     }
 
+    /**
+     * Both grants are nested under "the caller owns this row" on purpose.
+     *
+     * Held flat, an `app_owner` match reads as a grant in its own right — and
+     * since the owner check above it has already returned for every row the
+     * caller owns, it is only ever reached for a row owned by somebody else.
+     * `app_owner` is a global app id shared by every user of that app, so that
+     * hands one user's owner name, uuid and home path to any other user acting
+     * under the same app. `#checkWriteAccess` requires the owner match in both
+     * of its branches; this is the same rule, written as nesting.
+     *
+     * The inner check is the predicate `select` applies in SQL: an app sees
+     * what it created, not everything its user owns. Read `effectiveApp`, not
+     * `app` — an app-minted access token carries no `app` of its own and would
+     * otherwise slip past as though no app were involved.
+     */
     async #checkReadAccess(
         row: Record<string, unknown>,
         actor: Actor,
     ): Promise<void> {
-        // Owner
-        if (actor.user?.id === row.user_id) return;
-        // App actor matching app_owner
-        if (actor.app?.id && actor.app.id === row.app_owner) return;
+        if (actor.user?.id === row.user_id) {
+            const app = actor.effectiveApp;
+            if (!app?.id) return;
+            if (app.id === row.app_owner) return;
+        }
         // Cross-user read permission
         if (await this.#hasPermission(actor, 'read-all-subdomains')) return;
         throw new HttpError(403, 'Access denied', { legacyCode: 'forbidden' });