diff --git a/src/backend/drivers/subdomain/SubdomainDriver.edges.test.ts b/src/backend/drivers/subdomain/SubdomainDriver.edges.test.ts index 88c811e1b..a4b6f0644 100644 --- a/src/backend/drivers/subdomain/SubdomainDriver.edges.test.ts +++ b/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; + // `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; + 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); diff --git a/src/backend/drivers/subdomain/SubdomainDriver.ts b/src/backend/drivers/subdomain/SubdomainDriver.ts index 8ec5bc547..5669c0bda 100644 --- a/src/backend/drivers/subdomain/SubdomainDriver.ts +++ b/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): Promise { 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, actor: Actor, ): Promise { - // 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' });