From d202be10a96079fa4beb382fe441342c9dcf4547 Mon Sep 17 00:00:00 2001 From: Juan Fernando Castro Date: Sat, 8 Aug 2026 07:04:07 -0400 Subject: [PATCH] feat: let apps use another app's data with user consent (#3516) * feat(perms): add cross-app app-data permission vocabulary * feat(perms): sweep app grants by permission prefix * feat(perms): resolve and withdraw cross-app data grants * feat(kv): support an authorized namespace override and per-key privacy * feat(kv): gate cross-app KV access behind app-data grants * feat(fs): allow cross-app AppData access and require a scope to delete * feat(auth): accept permission lists and gate app-data grants * feat(perms): add requestAppData to the puter.js SDK * feat(gui): carry permission lists through the IPC and popup transports * feat(gui): describe cross-app data requests in the consent dialog * docs: document requestAppData and per-entry KV privacy * perf(perms): sweep cross-app grants only for origin-bootstrapped apps * fix(gui): stop double-encoding cross-app consent text * fix(perms): close three gaps in cross-app grant enforcement * fix(kv): meter and batch the per-entry privacy probe * fix(perms): resolve app identifiers and scopes more strictly in the SDK * test(perms): cover the cross-app consent flow end to end * fix: small missing token resolution for app also adds the same exclusion for the batchPut api, small change * fix: make resolved actor optional --------- Co-authored-by: Daniel Salazar --- .../controllers/auth/AuthController.test.ts | 250 ++++++ .../controllers/auth/AuthController.ts | 179 ++++- .../controllers/fs/FSController.test.ts | 12 +- src/backend/controllers/fs/FSController.ts | 6 +- .../fs/LegacyFSController.routes.test.ts | 4 +- .../controllers/fs/LegacyFSController.test.ts | 36 +- .../controllers/fs/LegacyFSController.ts | 21 +- .../controllers/peer/PeerController.test.ts | 1 + .../controllers/peer/PeerController.ts | 6 +- .../controllers/webdav/WebDAVController.ts | 6 +- src/backend/core/actor.test.ts | 104 +++ src/backend/core/actor.ts | 64 +- .../core/http/middleware/authProbe.test.ts | 17 +- src/backend/core/http/middleware/authProbe.ts | 3 +- .../core/http/middleware/gates.test.ts | 24 + src/backend/core/http/middleware/gates.ts | 5 +- src/backend/core/index.ts | 2 + src/backend/drivers/kv/KVStoreDriver.test.ts | 749 +++++++++++++++++- src/backend/drivers/kv/KVStoreDriver.ts | 181 ++++- src/backend/drivers/workers/WorkerDriver.ts | 4 +- src/backend/services/acl/ACLService.ts | 8 +- .../apps/AppPermissionService.test.ts | 557 ++++++++++++- .../services/apps/AppPermissionService.ts | 176 +++- src/backend/services/auth/AuthService.ts | 14 +- src/backend/services/fs/FSService.test.ts | 253 +++++- src/backend/services/fs/FSService.ts | 151 +++- .../localworker/LocalWorkerService.ts | 10 +- .../services/permission/PermissionService.ts | 19 +- .../services/permission/appDataScopes.ts | 167 ++++ .../stores/permission/PermissionStore.test.ts | 116 +++ .../stores/permission/PermissionStore.ts | 74 ++ .../stores/systemKv/SystemKVStore.test.ts | 111 ++- src/backend/stores/systemKv/SystemKVStore.ts | 259 ++++-- src/backend/testUtil.ts | 3 +- src/docs/src/KV/set.md | 24 +- src/docs/src/Perms.md | 34 +- src/docs/src/Perms/requestAppData.md | 146 ++++ .../examples/perms-request-app-data.html | 22 + src/gui/src/IPC.js | 25 +- src/gui/src/UI/UIPermissionDialog.js | 161 +++- .../UI/UIPermissionDialog.rendering.test.js | 91 +++ src/gui/src/UI/UIPermissionDialog.test.js | 106 +++ src/gui/src/i18n/translations/en.js | 7 + src/gui/src/initgui.js | 18 +- src/puter-js/src/modules/UI.js | 29 +- src/puter-js/src/modules/perms/appData.js | 178 +++++ .../src/modules/perms/appData.test.js | 195 +++++ src/puter-js/src/modules/perms/index.js | 5 + src/puter-js/tests/e2e/helpers/testApp.js | 45 ++ .../tests/e2e/specs/requestAppData.spec.js | 293 +++++++ src/puter-js/types/modules/kv.d.ts | 18 + src/puter-js/types/modules/perms.d.ts | 67 ++ 52 files changed, 4757 insertions(+), 299 deletions(-) create mode 100644 src/backend/core/actor.test.ts create mode 100644 src/backend/services/permission/appDataScopes.ts create mode 100644 src/docs/src/Perms/requestAppData.md create mode 100644 src/docs/src/playground/examples/perms-request-app-data.html create mode 100644 src/gui/src/UI/UIPermissionDialog.rendering.test.js create mode 100644 src/gui/src/UI/UIPermissionDialog.test.js create mode 100644 src/puter-js/src/modules/perms/appData.js create mode 100644 src/puter-js/src/modules/perms/appData.test.js create mode 100644 src/puter-js/tests/e2e/specs/requestAppData.spec.js diff --git a/src/backend/controllers/auth/AuthController.test.ts b/src/backend/controllers/auth/AuthController.test.ts index 1637f868a..9967f6b75 100644 --- a/src/backend/controllers/auth/AuthController.test.ts +++ b/src/backend/controllers/auth/AuthController.test.ts @@ -6907,3 +6907,253 @@ describe('AuthController.loginWait audience binding', () => { expect(res.body).toBeUndefined(); }); }); + +// -- Batch grant / revoke + cross-app data grants ----------------------- + +describe('AuthController — app-data grants', () => { + let issuer: { id: number; username: string }; + let issuerActor: Actor; + + beforeAll(async () => { + const name = `ad_${Math.random().toString(36).slice(2, 10)}`; + await controller.handleSignup( + makeReq({ + username: name, + email: `${name}@test.local`, + password: 'correct-horse-battery', + }), + makeRes(), + ); + const row = await server.stores.user.getByUsername(name); + await server.stores.user.update(row!.id, { email_confirmed: 1 }); + issuer = { id: row!.id, username: row!.username }; + issuerActor = { + user: { + id: row!.id, + uuid: row!.uuid, + username: row!.username, + email: row!.email, + email_confirmed: true, + }, + } as Actor; + }); + const makeAppRow = async (fields: Record = {}) => + (await server.stores.app.create( + { + name: `ad-${uuidv4()}`, + title: 'AppDataTest', + index_url: 'https://example.test/index.html', + ...fields, + }, + { ownerUserId: issuer.id }, + )) as { id: number; uid: string }; + + const grantedPermissions = async (appUid: string): Promise => { + const rows = (await server.clients.db.read( + 'SELECT p.`permission` FROM `user_to_app_permissions` p ' + + 'JOIN `apps` a ON a.`id` = p.`app_id` ' + + 'WHERE p.`user_id` = ? AND a.`uid` = ?', + [issuer.id, appUid], + )) as Array<{ permission: string }>; + return rows.map((r) => r.permission); + }; + + const post = ( + handler: 'handleGrantUserApp' | 'handleRevokeUserApp', + body: Record, + ) => { + const res = makeRes(); + return inCtx(issuerActor, () => + controller[handler](makeReq(body, { actor: issuerActor }), res), + ).then(() => res); + }; + + it('grants every permission in a list in one request', async () => { + const grantee = await makeAppRow(); + const target = await makeAppRow(); + const permissions = [ + `app-data:${target.uid}:kv:read`, + `app-data:${target.uid}:kv:delete`, + ]; + + await post('handleGrantUserApp', { + app_uid: grantee.uid, + permissions, + }); + + expect(await grantedPermissions(grantee.uid)).toEqual( + expect.arrayContaining(permissions), + ); + }); + + it('revokes every permission in a list in one request', async () => { + const grantee = await makeAppRow(); + const target = await makeAppRow(); + const permissions = [ + `app-data:${target.uid}:kv:read`, + `app-data:${target.uid}:fs:read`, + ]; + await post('handleGrantUserApp', { + app_uid: grantee.uid, + permissions, + }); + + await post('handleRevokeUserApp', { + app_uid: grantee.uid, + permissions, + }); + + const remaining = await grantedPermissions(grantee.uid); + for (const permission of permissions) { + expect(remaining).not.toContain(permission); + } + }); + + it('rejects the scalar and list forms together', async () => { + const grantee = await makeAppRow(); + const target = await makeAppRow(); + await expect( + post('handleGrantUserApp', { + app_uid: grantee.uid, + permission: `app-data:${target.uid}:kv:read`, + permissions: [`app-data:${target.uid}:kv:write`], + }), + ).rejects.toMatchObject({ statusCode: 400 }); + }); + + it('rejects an empty, oversized, or `*`-bearing list', async () => { + const grantee = await makeAppRow(); + const target = await makeAppRow(); + for (const permissions of [ + [], + Array.from( + { length: 17 }, + (_x, i) => `app-data:${target.uid}:kv:read${i}`, + ), + [`app-data:${target.uid}:kv:read`, '*'], + ]) { + await expect( + post('handleGrantUserApp', { + app_uid: grantee.uid, + permissions, + }), + ).rejects.toMatchObject({ statusCode: 400 }); + } + }); + + it('writes nothing when one entry in the list is invalid', async () => { + const grantee = await makeAppRow(); + const target = await makeAppRow(); + await expect( + post('handleGrantUserApp', { + app_uid: grantee.uid, + permissions: [ + `app-data:${target.uid}:kv:read`, + `app-data:app-does-not-exist:kv:read`, + ], + }), + ).rejects.toMatchObject({ statusCode: 404 }); + // The valid entry must not have landed: validation runs over the whole + // list before any row is written. + expect(await grantedPermissions(grantee.uid)).not.toContain( + `app-data:${target.uid}:kv:read`, + ); + }); + + it('writes nothing when an entry is too wide for the column it lands in', async () => { + // `#validateAppPermissionParams` allows 4096 chars but the column is 255, + // so this passes shape validation and fails inside the grant. Before the + // pre-flight, the first entry committed and the caller still got a 400 — + // and the dialog reads a 4xx as "nothing was written". + const grantee = await makeAppRow(); + const target = await makeAppRow(); + const good = `app-data:${target.uid}:kv:read`; + await expect( + post('handleGrantUserApp', { + app_uid: grantee.uid, + permissions: [good, 'x'.repeat(300)], + }), + ).rejects.toMatchObject({ statusCode: 400 }); + expect(await grantedPermissions(grantee.uid)).not.toContain(good); + }); + + it('404s when the target app does not exist', async () => { + const grantee = await makeAppRow(); + await expect( + post('handleGrantUserApp', { + app_uid: grantee.uid, + permission: 'app-data:app-nope/:kv:read', + }), + ).rejects.toMatchObject({ statusCode: 404 }); + }); + + it('rejects a bare `app-data` with no target app', async () => { + const grantee = await makeAppRow(); + for (const permission of ['app-data', 'app-data:', 'app-data::kv']) { + await expect( + post('handleGrantUserApp', { + app_uid: grantee.uid, + permission, + }), + ).rejects.toMatchObject({ statusCode: 400 }); + } + }); + + it('refuses a grant when the target opted out of sharing', async () => { + const grantee = await makeAppRow(); + const closed = await makeAppRow({ + metadata: JSON.stringify({ share_app_data: false }), + }); + await expect( + post('handleGrantUserApp', { + app_uid: grantee.uid, + permission: `app-data:${closed.uid}:kv:read`, + }), + ).rejects.toMatchObject({ statusCode: 403 }); + }); + + it('still allows revoking a grant after the target opts out', async () => { + const grantee = await makeAppRow(); + const target = await makeAppRow(); + const permission = `app-data:${target.uid}:kv:read`; + await post('handleGrantUserApp', { + app_uid: grantee.uid, + permission, + }); + + await server.stores.app.update(target.id, { + metadata: JSON.stringify({ share_app_data: false }), + }); + + // A user must always be able to withdraw consent, whatever the target + // now says about sharing. + await post('handleRevokeUserApp', { app_uid: grantee.uid, permission }); + expect(await grantedPermissions(grantee.uid)).not.toContain(permission); + }); + + it("creates the target's AppData directory for an fs grant", async () => { + const grantee = await makeAppRow(); + const target = await makeAppRow(); + const path = `/${issuer.username}/AppData/${target.uid}`; + expect(await server.stores.fsEntry.getEntryByPath(path)).toBeFalsy(); + + await post('handleGrantUserApp', { + app_uid: grantee.uid, + permission: `app-data:${target.uid}:fs:read`, + }); + + // Without this the grant is valid but every read 404s until the target + // app happens to run for the first time. + expect(await server.stores.fsEntry.getEntryByPath(path)).toBeTruthy(); + }); + + it('leaves unrelated permissions untouched by the new validation', async () => { + const grantee = await makeAppRow(); + const permission = 'service:unrelated:ii:read'; + await post('handleGrantUserApp', { + app_uid: grantee.uid, + permission, + }); + expect(await grantedPermissions(grantee.uid)).toContain(permission); + }); +}); diff --git a/src/backend/controllers/auth/AuthController.ts b/src/backend/controllers/auth/AuthController.ts index 7630b11f9..c5d2c0992 100644 --- a/src/backend/controllers/auth/AuthController.ts +++ b/src/backend/controllers/auth/AuthController.ts @@ -27,6 +27,7 @@ import type { HttpErrorOptions } from '../../core/http/HttpError.js'; import { HttpError } from '../../core/http/HttpError.js'; import { antiCsrf } from '../../core/http/middleware/antiCsrf.js'; import { generateCaptcha } from '../../core/http/middleware/captcha.js'; +import type { Actor } from '../../core/actor.js'; import { checkRateLimit } from '../../core/http/middleware/rateLimit.js'; import { signStepUpToken, @@ -60,11 +61,19 @@ import { generateDefaultFsentries, promoteToVerifiedGroup, } from '../../util/userProvisioning.js'; +import { + APP_DATA_PERMISSION_PREFIX, + appDataSharingAllowed, + parseAppDataPermission, +} from '../../services/permission/appDataScopes.js'; import { PuterController } from '../types.js'; const USERNAME_REGEX = /^\w{1,}$/; const USERNAME_MAX_LENGTH = 45; const FINGERPRINT_MAX_LENGTH = 128; +// One consent prompt covers a handful of scopes at most. The cap keeps a +// crafted request from turning a single grant call into a bulk write. +const MAX_PERMISSIONS_PER_REQUEST = 16; const DISPATCH_ID_MAX_LENGTH = 128; // Default SMS send attempts before the card fallback opens. const DEFAULT_CARD_FALLBACK_ATTEMPTS = 2; @@ -2827,13 +2836,113 @@ export class AuthController extends PuterController { return app.uid; } + /** + * Resolve the `permission` / `permissions` pair into the list to act on. + * + * One consent prompt can cover several scopes (read a store, write + * another), and a client looping the single form would have to invent its + * own partial-failure and rollback handling. Accepting the array keeps that + * in one request. + */ + #appPermissionList(body: { + permission?: unknown; + permissions?: unknown; + }): string[] { + const { permission, permissions } = body; + if (permissions !== undefined && permissions !== null) { + if (permission !== undefined && permission !== null) { + throw new HttpError( + 400, + 'Pass `permission` or `permissions`, not both', + { legacyCode: 'bad_request' }, + ); + } + if (!Array.isArray(permissions) || permissions.length === 0) { + throw new HttpError(400, 'Invalid `permissions`', { + legacyCode: 'bad_request', + }); + } + if (permissions.length > MAX_PERMISSIONS_PER_REQUEST) { + throw new HttpError(400, 'Too many `permissions`', { + legacyCode: 'bad_request', + }); + } + for (const entry of permissions) { + this.#validateAppPermissionParams({ permission: entry }); + // `*` means "revoke everything" in the scalar form only — + // inside a list it would silently widen a targeted request. + if (!entry || entry === '*') { + throw new HttpError(400, 'Invalid `permissions`', { + legacyCode: 'bad_request', + }); + } + } + return [...new Set(permissions as string[])]; + } + return typeof permission === 'string' && permission ? [permission] : []; + } + + /** + * Gate a cross-app data grant: the target must exist, must not have opted + * out of sharing, and must be named. Also creates the target's AppData + * directory for an `fs` scope, since it is only created lazily when the app + * first runs — without this a valid grant would 404 until then. + */ + async #prepareAppDataGrant( + actor: Actor, + permission: string, + ): Promise { + const parsed = parseAppDataPermission(permission); + if (!parsed) { + // A bare `app-data` (or one with an empty target) would cover every + // app the user has by prefix implication, which no prompt can + // describe. Reject rather than treat it as an unrelated permission. + if ( + permission === APP_DATA_PERMISSION_PREFIX || + permission.startsWith(`${APP_DATA_PERMISSION_PREFIX}:`) + ) { + throw new HttpError( + 400, + 'Invalid `app-data` permission: missing target app', + { legacyCode: 'bad_request' }, + ); + } + return; + } + + const target = await this.stores.app.getByUid(parsed.targetAppUid); + if (!target) { + throw new HttpError( + 404, + `entity_not_found: app:${parsed.targetAppUid}`, + { legacyCode: 'subject_does_not_exist' }, + ); + } + if (!appDataSharingAllowed(target)) { + throw new HttpError( + 403, + 'This app does not share its data with other apps', + { legacyCode: 'forbidden' }, + ); + } + + const username = actor.user?.username; + const userId = actor.user?.id; + if ((parsed.store === 'fs' || !parsed.store) && username && userId) { + await this.services.fs.mkdir(userId, { + path: `/${username}/AppData/${parsed.targetAppUid}`, + createMissingParents: true, + } as never); + } + } + @Post('/auth/grant-user-app', { subdomain: 'api', requireUserActor: true, }) async handleGrantUserApp(req: Request, res: Response): Promise { let { app_uid } = req.body; - const { origin, permission, extra, meta } = req.body; + const { origin, permission, permissions, extra, meta } = req.body; this.#validateAppPermissionParams({ app_uid, origin, @@ -2841,21 +2950,36 @@ export class AuthController extends PuterController { extra, meta, }); + const list = this.#appPermissionList({ permission, permissions }); if (origin) { app_uid = await this.#registeredAppUidFromOrigin(origin); } - if (!app_uid || !permission) { + if (!app_uid || list.length === 0) { throw new HttpError(400, 'Missing `app_uid` or `permission`', { legacyCode: 'bad_request', }); } - await this.services.permission.grantUserAppPermission( - req.actor!, - app_uid, - permission, - extra ?? undefined, - meta ?? undefined, - ); + + // Validate every entry before writing any, so a bad one in the list + // cannot leave a partially-granted set behind: the dialog reads a 4xx as + // "nothing was written" and skips its withdrawal, so a partial commit + // leaves live access the user was told they refused. The rewrite running + // twice is cheaper than splitting the grant into prepare/commit. + for (const entry of list) { + await this.services.permission.assertUserAppPermissionWritable( + entry, + ); + await this.#prepareAppDataGrant(req.actor!, entry); + } + for (const entry of list) { + await this.services.permission.grantUserAppPermission( + req.actor!, + app_uid, + entry, + extra ?? undefined, + meta ?? undefined, + ); + } res.json({}); } @@ -2915,21 +3039,24 @@ export class AuthController extends PuterController { }) async handleRevokeUserApp(req: Request, res: Response): Promise { let { app_uid } = req.body; - const { origin, permission, meta } = req.body; + const { origin, permission, permissions, meta } = req.body; this.#validateAppPermissionParams({ app_uid, origin, permission, meta, }); + const list = this.#appPermissionList({ permission, permissions }); if (origin) { app_uid = await this.#registeredAppUidFromOrigin(origin); } - if (!app_uid || !permission) { + if (!app_uid || list.length === 0) { throw new HttpError(400, 'Missing `app_uid` or `permission`', { legacyCode: 'bad_request', }); } + // Deliberately not gated by the target's sharing flag: a user must + // always be able to withdraw a grant, whatever the target now says. if (permission === '*') { await this.services.permission.revokeUserAppAll( req.actor!, @@ -2937,12 +3064,14 @@ export class AuthController extends PuterController { meta ?? undefined, ); } else { - await this.services.permission.revokeUserAppPermission( - req.actor!, - app_uid, - permission, - meta ?? undefined, - ); + for (const entry of list) { + await this.services.permission.revokeUserAppPermission( + req.actor!, + app_uid, + entry, + meta ?? undefined, + ); + } } res.json({}); } @@ -3250,6 +3379,22 @@ export class AuthController extends PuterController { app = await this.stores.app.createFromOrigin(app_uid, origin, { ownerUserId, }); + // An origin's uid is a deterministic uuidv5, so a deleted app + // reappears here under the identical uid. Withdraw any cross-app + // data grants left pointing at it before this new row can inherit + // consent the user gave its predecessor. Only *this* path can reuse + // a uid: `AppStore.create` mints a random uuid4, which no deleted + // app can ever hold again. + // + // Called directly rather than through `app.changed`: the token is + // issued below, so this has to be able to stop that, and + // `emitAndWait` swallows listener errors. Letting it throw is the + // point — a sweep that failed leaves the old grants live against an + // app whoever controls the origin now has just claimed. + await this.services.appPermission.withdrawAppDataGrants( + app_uid, + 'uid reused by a new app', + ); } if (!app) { throw new HttpError(404, `App ${app_uid} does not exist`, { diff --git a/src/backend/controllers/fs/FSController.test.ts b/src/backend/controllers/fs/FSController.test.ts index c056dee2b..d4420b0af 100644 --- a/src/backend/controllers/fs/FSController.test.ts +++ b/src/backend/controllers/fs/FSController.test.ts @@ -21,7 +21,7 @@ import type { Request, Response } from 'express'; import type { Readable } from 'node:stream'; import { v4 as uuidv4 } from 'uuid'; import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; -import type { Actor } from '../../core/actor.js'; +import { makeActor, type Actor } from '../../core/actor.js'; import { runWithContext } from '../../core/context.js'; import { PuterServer } from '../../server.js'; import { setupTestServer } from '../../testUtil.js'; @@ -927,7 +927,7 @@ describe('FSController.searchEntries', () => { const { actor: userActor } = await makeUser(); const username = userActor.user!.username!; const appUid = `app-search-${uuidv4()}`; - const appActor: Actor = { ...userActor, app: { uid: appUid } }; + const appActor = makeActor({ ...userActor, app: { uid: appUid } }); const needle = `appneedle-${Math.random().toString(36).slice(2, 8)}`; // User-owned entry outside AppData — must NOT appear for the app. @@ -978,7 +978,7 @@ describe('FSController.searchEntries', () => { const { actor: userActor } = await makeUser(); const username = userActor.user!.username!; const appUid = `app-search-${uuidv4()}`; - const appActor: Actor = { ...userActor, app: { uid: appUid } }; + const appActor = makeActor({ ...userActor, app: { uid: appUid } }); const needle = `appneedle-${Math.random().toString(36).slice(2, 8)}`; // Only seed outside AppData — the app must not be able to find it. @@ -1127,7 +1127,7 @@ describe('FSController.mkdirEntry', () => { const { actor: userActor } = await makeUser(); const username = userActor.user!.username!; const appUid = `app-mkdir-${uuidv4()}`; - const appActor: Actor = { ...userActor, app: { uid: appUid } }; + const appActor = makeActor({ ...userActor, app: { uid: appUid } }); const target = `/${username}/AppData/${appUid}`; await withActor(userActor, () => @@ -1889,10 +1889,10 @@ describe('FSController.readdirEntries recursive', () => { it('masks denials for app-under-user actors as a 404 (legacy parity)', async () => { const { actor: userActor } = await makeUser(); const username = userActor.user!.username!; - const appActor: Actor = { + const appActor = makeActor({ ...userActor, app: { uid: `app-readdir-${uuidv4()}` }, - }; + }); // The user's Documents is outside the app's AppData subtree, so the // app can't list it. Legacy `/readdir` masks this as a 404 // subject_does_not_exist rather than leaking a 403. diff --git a/src/backend/controllers/fs/FSController.ts b/src/backend/controllers/fs/FSController.ts index eb947a708..f9d2ac1d2 100644 --- a/src/backend/controllers/fs/FSController.ts +++ b/src/backend/controllers/fs/FSController.ts @@ -22,7 +22,6 @@ import type { Request, Response } from 'express'; import { posix as pathPosix } from 'node:path'; import { pipeline } from 'node:stream/promises'; import type { Actor } from '../../core/actor.js'; -import { effectiveActorApp } from '../../core/actor.js'; import { Context } from '../../core/context.js'; import { HttpError } from '../../core/http/HttpError.js'; import { Controller, Get, Post } from '../../core/http/decorators.js'; @@ -2091,8 +2090,7 @@ export class FSController extends PuterController { // the ActorUser type. Access via the escape hatch until a proper // storage-quota mechanism is in place. const actorUser = req.actor?.user as - | Record - | undefined; + Record | undefined; const candidates = [ this.#toStorageCapacityCandidate(actorUser?.free_storage), @@ -2437,7 +2435,7 @@ export class FSController extends PuterController { // search results to this path so app actors can't see entries outside // their AppData via `/fs/search`. #appDataScopeForActor(actor: Actor): string | undefined { - const app = effectiveActorApp(actor); + const app = actor.effectiveApp; if (!app) return undefined; const username = actor.user?.username; if (typeof username !== 'string' || username.length === 0) diff --git a/src/backend/controllers/fs/LegacyFSController.routes.test.ts b/src/backend/controllers/fs/LegacyFSController.routes.test.ts index 13e695b0a..e72054312 100644 --- a/src/backend/controllers/fs/LegacyFSController.routes.test.ts +++ b/src/backend/controllers/fs/LegacyFSController.routes.test.ts @@ -21,7 +21,7 @@ import type { Request, RequestHandler, Response } from 'express'; import { Readable, Writable } from 'node:stream'; import { v4 as uuidv4 } from 'uuid'; import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; -import type { Actor } from '../../core/actor.js'; +import { makeActor, type Actor } from '../../core/actor.js'; import { runWithContext } from '../../core/context.js'; import { PuterRouter } from '../../core/http/PuterRouter.js'; import { PuterServer } from '../../server.js'; @@ -377,7 +377,7 @@ describe('LegacyFSController.readdirSubdomains', () => { 'INSERT INTO `subdomains` (`uuid`, `subdomain`, `user_id`) VALUES (?, ?, ?)', [uuidv4(), `sd-${Math.random().toString(36).slice(2, 8)}`, userId], ); - const appActor: Actor = { ...actor, app: { uid: `app-${uuidv4()}` } }; + const appActor = makeActor({ ...actor, app: { uid: `app-${uuidv4()}` } }); const { res, captured } = makeRes(); await withActor(appActor, () => controller.readdirSubdomains(makeReq({ actor: appActor }), res), diff --git a/src/backend/controllers/fs/LegacyFSController.test.ts b/src/backend/controllers/fs/LegacyFSController.test.ts index 0b02dd530..fb38aa8c0 100644 --- a/src/backend/controllers/fs/LegacyFSController.test.ts +++ b/src/backend/controllers/fs/LegacyFSController.test.ts @@ -21,7 +21,7 @@ import type { Request, Response } from 'express'; import { Readable } from 'node:stream'; import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; import { v4 as uuidv4 } from 'uuid'; -import type { Actor } from '../../core/actor.js'; +import { makeActor, type Actor } from '../../core/actor.js'; import { runWithContext } from '../../core/context.js'; import { PuterRouter } from '../../core/http/PuterRouter.js'; import { PuterServer } from '../../server.js'; @@ -250,7 +250,7 @@ describe('LegacyFSController.mkdir', () => { const { actor: userActor } = await makeUser(); const username = userActor.user!.username!; const appUid = `app-legacy-mkdir-${uuidv4()}`; - const appActor: Actor = { ...userActor, app: { uid: appUid } }; + const appActor = makeActor({ ...userActor, app: { uid: appUid } }); const parent = `/${username}/AppData`; await withActor(userActor, () => @@ -1214,7 +1214,7 @@ describe('LegacyFSController.search', () => { const { actor: userActor } = await makeUser(); const username = userActor.user!.username!; const appUid = `app-legacy-search-${uuidv4()}`; - const appActor: Actor = { ...userActor, app: { uid: appUid } }; + const appActor = makeActor({ ...userActor, app: { uid: appUid } }); const needle = `appneedle-${Math.random().toString(36).slice(2, 8)}`; await withActor(userActor, () => @@ -1419,10 +1419,10 @@ describe('LegacyFSController.sign', () => { }, { ownerUserId: userActor.user!.id! }, ); - const attackerActor: Actor = { + const attackerActor = makeActor({ ...userActor, app: { uid: `attacker-${uuidv4()}` }, - }; + }); const { res } = makeRes(); await expect( @@ -1456,7 +1456,7 @@ describe('LegacyFSController.sign', () => { }, { ownerUserId: userActor.user!.id! }, ); - const appActor: Actor = { ...userActor, app: { uid: ownApp.uid } }; + const appActor = makeActor({ ...userActor, app: { uid: ownApp.uid } }); const { res, captured } = makeRes(); await withActor(appActor, () => @@ -1766,10 +1766,10 @@ describe('LegacyFSController.requestAppRootDir', () => { it('rejects with 403 when the actor.app.uid differs from the requested app_uid', async () => { const { actor } = await makeUser(); const { res } = makeRes(); - const appActor = { + const appActor = makeActor({ ...actor, app: { uid: 'app-mismatch' }, - } as unknown as Actor; + }); await expect( withActor(appActor, () => controller.requestAppRootDir( @@ -1787,10 +1787,10 @@ describe('LegacyFSController.requestAppRootDir', () => { const { actor } = await makeUser(); const username = actor.user!.username!; const appUid = 'app-self'; - const appActor = { + const appActor = makeActor({ ...actor, app: { uid: appUid }, - } as unknown as Actor; + }); const { res, captured } = makeRes(); await withActor(appActor, () => controller.requestAppRootDir( @@ -2463,10 +2463,10 @@ describe('LegacyFSController.sign app sandbox + write downgrade', () => { const { actor } = await makeUser(); const username = actor.user!.username!; // Build an app-under-user actor whose AppData root is the test app. - const appActor = { + const appActor = makeActor({ ...actor, app: { uid: 'sandbox-app' }, - } as unknown as Actor; + }); // Create a file *outside* /Documents (anywhere outside AppData/). const target = `/${username}/Documents/forbidden.txt`; @@ -2730,10 +2730,10 @@ describe('LegacyFSController.suggestApps', () => { it('refuses to look up entries an app actor cannot see', async () => { const { actor: userActor } = await makeUser(); const username = userActor.user!.username!; - const appActor: Actor = { + const appActor = makeActor({ ...userActor, app: { uid: `app-suggest-${uuidv4()}` }, - }; + }); const target = `/${username}/Documents/probe.txt`; await withActor(userActor, () => controller.touch( @@ -2801,10 +2801,10 @@ describe('LegacyFSController.suggestApps', () => { describe('LegacyFSController.readdirSubdomains', () => { it('returns an empty array for app-under-user actors', async () => { const { actor: userActor, userId } = await makeUser(); - const appActor: Actor = { + const appActor = makeActor({ ...userActor, app: { uid: `app-subd-${uuidv4()}` }, - }; + }); await server.clients.db.write( 'INSERT INTO `subdomains` (`uuid`, `subdomain`, `user_id`) VALUES (?, ?, ?)', [uuidv4(), `sd-${uuidv4().slice(0, 8)}`, userId], @@ -2843,10 +2843,10 @@ describe('LegacyFSController.updateFsentryThumbnail', () => { it('rejects an app actor probing entries outside AppData with 404', async () => { const { actor: userActor } = await makeUser(); const username = userActor.user!.username!; - const appActor: Actor = { + const appActor = makeActor({ ...userActor, app: { uid: `app-thumb-${uuidv4()}` }, - }; + }); const target = `/${username}/Documents/thumbme.txt`; await withActor(userActor, () => controller.touch( diff --git a/src/backend/controllers/fs/LegacyFSController.ts b/src/backend/controllers/fs/LegacyFSController.ts index b7cb346ea..562bd555a 100644 --- a/src/backend/controllers/fs/LegacyFSController.ts +++ b/src/backend/controllers/fs/LegacyFSController.ts @@ -21,15 +21,18 @@ import Busboy from 'busboy'; import type { NextFunction, Request, RequestHandler, Response } from 'express'; import { contentType as contentTypeFromMime } from 'mime-types'; import { posix as pathPosix } from 'node:path'; -import type { Actor } from '../../core/actor.js'; -import { effectiveActorApp, isAccessTokenActor } from '../../core/actor.js'; +import { + assertResolvedActor, + isAccessTokenActor, + makeActor, +} from '../../core/actor.js'; import { Context } from '../../core/context.js'; import { HttpError } from '../../core/http/HttpError.js'; +import { RouteOptions } from '../../core/http/index.js'; import { assertNotSuspended, assertVerifiedAccount, } from '../../core/http/middleware/gates.js'; -import { RouteOptions } from '../../core/http/index.js'; import type { PuterRouter } from '../../core/http/PuterRouter.js'; import type { ACLService } from '../../services/acl/ACLService.js'; import type { SignedFile } from '../../util/fileSigning.js'; @@ -908,7 +911,7 @@ export class LegacyFSController extends PuterController { const actor = this.#requireActor(req); // Subdomain enumeration is a user-level concern; app actors would // otherwise see root_dir uids pointing outside their AppData scope. - if (effectiveActorApp(actor)) { + if (actor.effectiveApp) { res.json([]); return; } @@ -985,7 +988,7 @@ export class LegacyFSController extends PuterController { // App-under-user actors only see entries within their AppData root; // user actors are unscoped. Mirrors the ACL short-circuit in // ACLService.check. - const app = effectiveActorApp(actor); + const app = actor.effectiveApp; const username = actor.user?.username; const pathScope = app && typeof username === 'string' && username.length > 0 @@ -1125,7 +1128,7 @@ export class LegacyFSController extends PuterController { assertNotSuspended(actor!.user); assertVerifiedAccount(actor!.user); - req.actor = actor!; + req.actor = assertResolvedActor(actor!); Context.set('actor', actor); // Forward back to regular read after setting actor @@ -1727,10 +1730,10 @@ export class LegacyFSController extends PuterController { }); // Build an actor-under-user shape for the check. - const actorForApp = { - user: (req.actor as { user?: unknown }).user, + const actorForApp = makeActor({ + user: req.actor!.user, app: { uid: (app as { uid: string }).uid }, - } as unknown as Actor; + }); const descriptor = { path: subject.path, resolveAncestors: () => diff --git a/src/backend/controllers/peer/PeerController.test.ts b/src/backend/controllers/peer/PeerController.test.ts index 54961772d..abbdcace0 100644 --- a/src/backend/controllers/peer/PeerController.test.ts +++ b/src/backend/controllers/peer/PeerController.test.ts @@ -438,6 +438,7 @@ describe('PeerController TURN', () => { id: created.id, username: created.username, }, + effectiveApp: null, }); } finally { meterSpy.mockRestore(); diff --git a/src/backend/controllers/peer/PeerController.ts b/src/backend/controllers/peer/PeerController.ts index e6628f7c6..b35ecb53e 100644 --- a/src/backend/controllers/peer/PeerController.ts +++ b/src/backend/controllers/peer/PeerController.ts @@ -19,7 +19,7 @@ import { createHmac, randomBytes, timingSafeEqual } from 'node:crypto'; import type { Request, Response } from 'express'; -import type { Actor } from '../../core/actor.js'; +import { makeActor, type Actor } from '../../core/actor.js'; import { HttpError } from '../../core/http/HttpError.js'; import type { PuterRouter } from '../../core/http/PuterRouter.js'; import { PuterController } from '../types.js'; @@ -217,13 +217,13 @@ export class PeerController extends PuterController { if (!user) continue; const costInMicrocents = egressBytes * PEER_COSTS['turn:egress-bytes']; - const actor = { + const actor = makeActor({ user: { uuid: user.uuid, id: user.id, username: user.username, }, - }; + }); await this.services.metering.incrementUsage( actor, 'turn:egress-bytes', diff --git a/src/backend/controllers/webdav/WebDAVController.ts b/src/backend/controllers/webdav/WebDAVController.ts index 63a4a8da0..7f072a530 100644 --- a/src/backend/controllers/webdav/WebDAVController.ts +++ b/src/backend/controllers/webdav/WebDAVController.ts @@ -21,7 +21,7 @@ import { compare as bcryptCompare } from 'bcrypt'; import type { Request, Response } from 'express'; import { posix as pathPosix } from 'node:path'; import { EventMap } from '../../clients/event/types.js'; -import type { Actor } from '../../core/actor.js'; +import { makeActor, type Actor } from '../../core/actor.js'; import { HttpError } from '../../core/http/HttpError.js'; import { assertNotSuspended, @@ -238,7 +238,7 @@ export class WebDAVController extends PuterController { } // Build a session-less actor for the user - return { + return makeActor({ user: { id: user.id, uuid: user.uuid, @@ -258,7 +258,7 @@ export class WebDAVController extends PuterController { requires_card_verification: user.requires_card_verification ?? false, }, - }; + }); } // -- OPTIONS ------------------------------------------------------ diff --git a/src/backend/core/actor.test.ts b/src/backend/core/actor.test.ts new file mode 100644 index 000000000..c9fb58702 --- /dev/null +++ b/src/backend/core/actor.test.ts @@ -0,0 +1,104 @@ +/* + * Copyright (C) 2024-present Puter Technologies Inc. + * + * This file is part of Puter. + * + * Puter is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { describe, expect, it } from 'vitest'; +import { + assertResolvedActor, + makeActor, + SYSTEM_ACTOR, + userRelatedActor, + type Actor, +} from './actor'; + +describe('makeActor / effectiveApp', () => { + const user = { uuid: 'u-1', id: 1, username: 'u' }; + + it('resolves a plain user actor to no app', () => { + expect(makeActor({ user }).effectiveApp).toBeNull(); + }); + + it("resolves an app-under-user actor to its own app", () => { + expect(makeActor({ user, app: { uid: 'app-1' } }).effectiveApp).toEqual({ + uid: 'app-1', + }); + }); + + it("resolves a token to the app that issued it", () => { + // The whole point: a token actor carries no `app` of its own, so + // anything reading `app` sees a bare user token and skips app gating. + const issuer = makeActor({ user, app: { uid: 'app-1' } }); + const token = makeActor({ + user, + accessToken: { uid: 'tok-1', issuer }, + }); + expect(token.app).toBeUndefined(); + expect(token.effectiveApp).toEqual({ uid: 'app-1' }); + }); + + it('collapses a chain of tokens in one hop', () => { + const issuer = makeActor({ user, app: { uid: 'app-1' } }); + const inner = makeActor({ user, accessToken: { uid: 't1', issuer } }); + const outer = makeActor({ + user, + accessToken: { uid: 't2', issuer: inner }, + }); + expect(outer.effectiveApp).toEqual({ uid: 'app-1' }); + }); + + it('resolves a user-issued token to no app', () => { + const issuer = makeActor({ user }); + const token = makeActor({ + user, + accessToken: { uid: 'tok-1', issuer, fullAccess: true }, + }); + expect(token.effectiveApp).toBeNull(); + }); + + it('drops the app when narrowing to the underlying user', () => { + const app = makeActor({ user, app: { uid: 'app-1' } }); + expect(userRelatedActor(app).effectiveApp).toBeNull(); + }); + + it('resolves the system actor', () => { + expect(SYSTEM_ACTOR.effectiveApp).toBeNull(); + }); +}); + +describe('assertResolvedActor', () => { + it('passes a resolved actor through unchanged', () => { + const actor = makeActor({ user: { uuid: 'u-1' } }); + expect(assertResolvedActor(actor)).toBe(actor); + // `null` is a resolved answer, not a missing one. + expect(assertResolvedActor({ user: {}, effectiveApp: null })).toEqual({ + user: {}, + effectiveApp: null, + }); + }); + + it('throws on an actor that skipped makeActor', () => { + // The field is optional so pre-existing literals still compile, which + // means an unresolved one can reach a gate. Fail loudly at the edge + // rather than let a gate read `undefined` as "no app" and wave it + // through — an app-under-user actor is the dangerous case. + const unresolved = { user: { uuid: 'u-1' }, app: { uid: 'app-1' } }; + expect(() => assertResolvedActor(unresolved as Actor)).toThrow( + /effectiveApp/, + ); + }); +}); diff --git a/src/backend/core/actor.ts b/src/backend/core/actor.ts index 3d61760dd..de3a45aa8 100644 --- a/src/backend/core/actor.ts +++ b/src/backend/core/actor.ts @@ -47,6 +47,26 @@ export interface ActorAccessToken { export interface Actor { user: Partial; app?: ActorApp | null; + /** + * The app this actor ultimately acts as: its own `app`, or failing that the + * app of whoever issued its access token. + * + * Read this — not `app` — in any gate asking "which app is doing this?". An + * access-token actor carries no `app` of its own, so `app` alone reads as + * "no app" even for a token an app minted, and a gate keyed off it fails + * open exactly where it must not. + * + * `null` and `undefined` are not the same thing here: + * + * - `null` — resolved, and this actor is not acting as any app. + * - Absent — never resolved, because the actor skipped `makeActor`. + * + * A gate must not read the second as the first: that is the fail-open this + * field exists to prevent. Optional only so an actor literal that predates + * the field still compiles; `assertResolvedActor` is what keeps the request + * path honest, and `makeActor` is the one place the derivation lives. + */ + effectiveApp?: ActorApp | null; /** True for the system actor; skips metering / quota tracking. */ system?: boolean; accessToken?: ActorAccessToken | null; @@ -67,9 +87,41 @@ export const SYSTEM_ACTOR_UUID = '5d4adce0-a381-4982-9c02-6e2540026238'; /** The default system actor used when no actor is supplied. */ export const SYSTEM_ACTOR: Actor = { user: { uuid: SYSTEM_ACTOR_UUID, username: 'system' }, + effectiveApp: null, system: true, }; +/** + * Build an actor, deriving `effectiveApp` from its own app and, failing that, + * from the app of whoever issued its access token. + * + * The issuer was itself built here, so its chain is already collapsed — one hop + * is enough, and no caller has to walk anything. + */ +export const makeActor = (actor: Omit): Actor => ({ + ...actor, + effectiveApp: actor.app ?? actor.accessToken?.issuer.effectiveApp ?? null, +}); + +/** + * Fail closed on an actor whose `effectiveApp` was never derived. + * + * Every actor on the request path is built by `AuthService` through + * `makeActor`, so this cannot fire in production — which is the point. It turns + * a future actor literal that skips the builder into a loud 500 at the edge + * rather than a silent bypass deep inside a gate that read `undefined` as "no + * app". Call it once, where the request actor is established. + */ +export const assertResolvedActor = (actor: Actor): Actor => { + if (actor.effectiveApp === undefined) { + throw new Error( + 'actor was built without `makeActor`: `effectiveApp` is unresolved, ' + + 'and app-scoped gates would read that as "no app"', + ); + } + return actor; +}; + export const isSystemActor = (actor: Actor | undefined | null): boolean => { return !!actor?.system || actor?.user?.uuid === SYSTEM_ACTOR_UUID; }; @@ -106,15 +158,5 @@ export const actorUid = (actor: Actor): string => { */ export const userRelatedActor = (actor: Actor): Actor => { if (!actor.app && !actor.accessToken) return actor; - return { user: actor.user }; -}; - -/** - * Walk the access-token issuer chain and return the first app identity found, - * or null if no app appears anywhere in the chain. - */ -export const effectiveActorApp = (actor: Actor): ActorApp | null => { - if (actor.app) return actor.app; - if (actor.accessToken) return effectiveActorApp(actor.accessToken.issuer); - return null; + return { user: actor.user, effectiveApp: null }; }; diff --git a/src/backend/core/http/middleware/authProbe.test.ts b/src/backend/core/http/middleware/authProbe.test.ts index e089a493c..a6dc116c8 100644 --- a/src/backend/core/http/middleware/authProbe.test.ts +++ b/src/backend/core/http/middleware/authProbe.test.ts @@ -21,7 +21,7 @@ import type { Request, Response } from 'express'; import jwt from 'jsonwebtoken'; import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; import { v4 as uuidv4 } from 'uuid'; -import type { Actor } from '../../actor'; +import { makeActor, type Actor } from '../../actor'; import type { AuthService } from '../../../services/auth/AuthService'; import { PuterServer } from '../../../server'; import { setupTestServer } from '../../../testUtil'; @@ -59,7 +59,16 @@ const makeStubAuth = (defaultActor: Actor | null = null): StubAuth => { authenticate: async (token: string) => { seenTokens.push(token); if (nextResult === 'throw') throw new Error('verify failed'); - return nextResult; + // The real service builds every actor through `makeActor`, and the + // probe asserts that contract — so the stub has to honour it too, + // rather than handing back a literal the probe rejects. + if (!('actor' in nextResult)) return nextResult; + // Resolve only if the fixture didn't: the probe asserts the + // `makeActor` contract the real service satisfies, but tests that + // check actor identity need the same object back. + return nextResult.actor.effectiveApp === undefined + ? { ...nextResult, actor: makeActor(nextResult.actor) } + : nextResult; }, // Back-compat wrapper for callers that still want Actor | null. authenticateFromToken: async (token: string) => { @@ -273,7 +282,7 @@ describe('createAuthProbe — tokenSource', () => { ['handshake', { handshakeQuery: { auth_token: 'tok' } }], ]; - const actor: Actor = { user: { uuid: 'u-1' } }; + const actor: Actor = makeActor({ user: { uuid: 'u-1' } }); it.each(cases)('records %s', async (expected, init) => { const stub = makeStubAuth(actor); @@ -523,7 +532,7 @@ describe('createAuthProbe — cookie reading', () => { describe('createAuthProbe — actor attachment + failure tracking', () => { it('attaches actor + token on a successful authenticate', async () => { - const actor: Actor = { user: { uuid: 'u-1' } }; + const actor: Actor = makeActor({ user: { uuid: 'u-1' } }); const stub = makeStubAuth(actor); const probe = createAuthProbe({ authService: stub.service }); const { req } = await runProbe( diff --git a/src/backend/core/http/middleware/authProbe.ts b/src/backend/core/http/middleware/authProbe.ts index c8ec7e845..b79ca4705 100644 --- a/src/backend/core/http/middleware/authProbe.ts +++ b/src/backend/core/http/middleware/authProbe.ts @@ -22,6 +22,7 @@ import type { AuthService, ReauthReason, } from '../../../services/auth/AuthService'; +import { assertResolvedActor } from '../../actor'; import type { TokenSource } from '../types'; // Ensure the `Request.actor` / `Request.token` augmentation is in scope @@ -155,7 +156,7 @@ export const createAuthProbe = (opts: AuthProbeOptions): RequestHandler => { } if (result.actor) { - req.actor = result.actor; + req.actor = assertResolvedActor(result.actor); req.token = token; req.tokenSource = source; } else if (result.invalid) { diff --git a/src/backend/core/http/middleware/gates.test.ts b/src/backend/core/http/middleware/gates.test.ts index f4e1b6a40..a3e795b92 100644 --- a/src/backend/core/http/middleware/gates.test.ts +++ b/src/backend/core/http/middleware/gates.test.ts @@ -19,6 +19,7 @@ import type { Request, Response } from 'express'; import { describe, expect, it } from 'vitest'; +import { makeActor, type Actor } from '../../actor'; import { HttpError, isHttpError } from '../HttpError'; import { DEFAULT_ADMIN_USERNAMES, @@ -42,6 +43,23 @@ import { type NextArg = undefined | 'route' | HttpError | unknown; +/** + * Rebuild an actor literal through `makeActor`, issuer-first, so the derived + * `effectiveApp` is present at every level of the token chain. + */ +const reviveActor = (actor: Actor): Actor => + makeActor({ + ...actor, + ...(actor.accessToken + ? { + accessToken: { + ...actor.accessToken, + issuer: reviveActor(actor.accessToken.issuer), + }, + } + : {}), + }); + const runGate = ( gate: ( req: Request, @@ -50,6 +68,12 @@ const runGate = ( ) => unknown, req: Partial, ): NextArg => { + // Normalise the actor the way AuthService does before any gate sees one, + // so `effectiveApp` is derived here rather than spelled out on every + // literal below. Issuers too: a gate reading the chain reads the derived + // field, not `issuer.app`. + if (req.actor) req = { ...req, actor: reviveActor(req.actor) }; + let captured: NextArg = undefined; let called = false; gate(req as Request, {} as Response, (arg?: unknown) => { diff --git a/src/backend/core/http/middleware/gates.ts b/src/backend/core/http/middleware/gates.ts index a84d03de0..29d5dc31f 100644 --- a/src/backend/core/http/middleware/gates.ts +++ b/src/backend/core/http/middleware/gates.ts @@ -18,7 +18,6 @@ */ import type { Request, RequestHandler } from 'express'; -import { effectiveActorApp } from '../../actor'; import type { Actor } from '../../actor'; import { HttpError } from '../HttpError'; import { assertVerifiedEmail } from '../verifiedEmail'; @@ -238,7 +237,7 @@ export const DEFAULT_ADMIN_USERNAMES = ['admin', 'system'] as const; * pair, not a replacement for it. * * Also requires a _root token_ — an actor with no app anywhere in its token - * chain (see `effectiveActorApp`) — so a third-party app an admin has + * chain (see `Actor.effectiveApp`) — so a third-party app an admin has * authorized can't reach admin endpoints on the admin's behalf. The one * exception is `appGated`: on a route that is also appId-gated * (`allowedAppIds`), a direct app-under-user actor is deferred to @@ -278,7 +277,7 @@ export const adminOnlyGate = ( // through an app. A direct app-under-user actor is deferred to // `allowedAppIdsGate` when the route is appId-gated; chain-only apps // are rejected even then, since that gate can't see them. - const chainApp = req.actor ? effectiveActorApp(req.actor) : null; + const chainApp = req.actor?.effectiveApp ?? null; if (chainApp && !(opts.appGated && req.actor?.app?.uid)) { next( new HttpError(403, 'Only admins may request this resource', { diff --git a/src/backend/core/index.ts b/src/backend/core/index.ts index ccba97e13..d753e3333 100644 --- a/src/backend/core/index.ts +++ b/src/backend/core/index.ts @@ -28,5 +28,7 @@ export { isAppActor, isAccessTokenActor, actorUid, + assertResolvedActor, + makeActor, userRelatedActor, } from './actor'; diff --git a/src/backend/drivers/kv/KVStoreDriver.test.ts b/src/backend/drivers/kv/KVStoreDriver.test.ts index 807c70d7f..6b82f5e9b 100644 --- a/src/backend/drivers/kv/KVStoreDriver.test.ts +++ b/src/backend/drivers/kv/KVStoreDriver.test.ts @@ -5,11 +5,16 @@ import { describe, expect, it, + vi, } from 'vitest'; -import { Actor } from '../../core/actor.ts'; +import { Actor, makeActor as buildActor } from '../../core/actor.ts'; import { runWithContext } from '../../core/context.ts'; import { PuterServer } from '../../server.ts'; -import { setupTestServer } from '../../testUtil.ts'; +import { + APP_DATA_KV_METHOD_OPS, + appDataPermission, +} from '../../services/permission/appDataScopes.ts'; +import { createTestUser, setupTestServer } from '../../testUtil.ts'; import { KV_COSTS } from './costs.ts'; import type { KVStoreDriver } from './KVStoreDriver.ts'; @@ -29,17 +34,18 @@ describe('KVStoreDriver', () => { // Each test runs against a unique actor namespace so state from one test // never leaks into another. Mirrors the pattern used by SystemKVStore.test. let actor: Actor; - const makeActor = (overrides: Partial = {}): Actor => ({ - user: { - uuid: `test-user-${Math.random().toString(36).slice(2)}`, - id: 1, - username: 'test-user', - email: 'test@test.com', - email_confirmed: true, - }, - app: { uid: 'test-app', id: 1 }, - ...overrides, - }); + const makeActor = (overrides: Partial = {}): Actor => + buildActor({ + user: { + uuid: `test-user-${Math.random().toString(36).slice(2)}`, + id: 1, + username: 'test-user', + email: 'test@test.com', + email_confirmed: true, + }, + app: { uid: 'test-app', id: 1 }, + ...overrides, + }); beforeEach(() => { actor = makeActor(); }); @@ -726,14 +732,14 @@ describe('KVStoreDriver', () => { it('isolates values between two app actors with the same user but different apps', async () => { const baseUser = `user-${Math.random().toString(36).slice(2)}`; - const appA: Actor = { + const appA = buildActor({ user: { uuid: baseUser }, app: { uid: 'app-A', id: 100 }, - }; - const appB: Actor = { + }); + const appB = buildActor({ user: { uuid: baseUser }, app: { uid: 'app-B', id: 200 }, - }; + }); await inCtx(() => target.set({ key: 'k', value: 'A' }), appA); const fromB = await inCtx(() => target.get({ key: 'k' }), appB); @@ -743,27 +749,39 @@ describe('KVStoreDriver', () => { expect(fromA).toBe('A'); }); - it('ignores optConfig.appUuid when the actor already has an app uid', async () => { - // App-actor sets a value, then tries to read with an appUuid override - // pointing somewhere else — driver must scrub the override. + it('refuses a foreign optConfig.appUuid rather than silently scrubbing it', async () => { + // The override used to be dropped for an app actor, which returned + // the app's *own* value — a success answering a different question + // than the caller asked. Cross-app access is now a real capability, + // so an override the caller cannot justify fails closed instead: an + // unknown target app is a 404, and a real target with no grant is a + // 403 (covered under cross-app access). const baseUser = `user-${Math.random().toString(36).slice(2)}`; - const appActor: Actor = { + const appActor = buildActor({ user: { uuid: baseUser }, app: { uid: 'real-app', id: 1 }, - }; + }); await inCtx( () => target.set({ key: 'k', value: 'real' }), appActor, ); - const res = await inCtx( - () => - target.get({ - key: 'k', - optConfig: { appUuid: 'spoof-app' }, - }), - appActor, + + await expect( + inCtx( + () => + target.get({ + key: 'k', + optConfig: { appUuid: 'spoof-app' }, + }), + appActor, + ), + ).rejects.toMatchObject({ statusCode: 404 }); + + // The app's own entry is untouched and still reachable with no + // override — the refusal is about the override, not the namespace. + expect(await inCtx(() => target.get({ key: 'k' }), appActor)).toBe( + 'real', ); - expect(res).toBe('real'); }); it('uses optConfig.appUuid for a user-only (root) actor', async () => { @@ -771,11 +789,11 @@ describe('KVStoreDriver', () => { // app namespace via optConfig.appUuid. Verify by reading the same // entry via a real app-actor for that app. const baseUser = `user-${Math.random().toString(36).slice(2)}`; - const userOnly: Actor = { user: { uuid: baseUser } }; - const asApp: Actor = { + const userOnly = buildActor({ user: { uuid: baseUser } }); + const asApp = buildActor({ user: { uuid: baseUser }, app: { uid: 'target-app', id: 1 }, - }; + }); await inCtx( () => @@ -813,4 +831,669 @@ describe('KVStoreDriver', () => { expect(rows.length).toBe(Object.keys(KV_COSTS).length); }); }); + + // -- Cross-app access (app-data::kv:) --------------------- + // + // An app may reach another app's KV namespace under the same user once the + // user has granted it. These tests use real user and app rows, because the + // grant lands in `user_to_app_permissions` and the driver resolves the + // target app row for its existence and sharing checks. + describe('cross-app access', () => { + const permissions = () => server.services.permission; + + const makeOwner = async (): Promise => { + const username = `kvx${Math.random().toString(36).slice(2, 10)}`; + const created = await createTestUser(server, { + username, + password: 'kv-cross-app-password', + }); + const row = await server.stores.user.getByUsername( + created.username, + ); + return buildActor({ + user: { + id: row!.id, + uuid: row!.uuid, + username: row!.username, + email: row!.email ?? null, + }, + }); + }; + + const makeRealApp = async ( + ownerUserId: number, + fields: Record = {}, + ): Promise<{ id: number; uid: string }> => { + const name = `kvx-${Math.random().toString(36).slice(2)}`; + return (await server.stores.app.create( + { + name, + title: 'KV cross-app test', + index_url: `https://${name}.test/`, + ...fields, + }, + { ownerUserId }, + )) as { id: number; uid: string }; + }; + + const asApp = ( + owner: Actor, + app: { id: number; uid: string }, + ): Actor => + buildActor({ + user: owner.user, + app: { uid: app.uid, id: app.id }, + }); + + /** An access token that `app` minted, as AuthService builds one. */ + const asTokenOf = (owner: Actor, actorForApp: Actor): Actor => + buildActor({ + user: owner.user, + accessToken: { + uid: `tok-${Math.random().toString(36).slice(2)}`, + issuer: actorForApp, + authorized: null, + }, + }); + + const grant = ( + owner: Actor, + granteeAppUid: string, + permission: string, + ) => + runWithContext({ actor: owner }, () => + permissions().grantUserAppPermission( + owner, + granteeAppUid, + permission, + ), + ); + + /** + * The common fixture: an owner, a calendar app asking for access, a + * contacts app holding the data, and one seeded entry in contacts' + * namespace written by contacts itself. + */ + const setup = async (targetFields: Record = {}) => { + const owner = await makeOwner(); + const calendar = await makeRealApp(owner.user.id!); + const contacts = await makeRealApp(owner.user.id!, targetFields); + const calendarActor = asApp(owner, calendar); + const contactsActor = asApp(owner, contacts); + await inCtx( + () => target.set({ key: 'entry', value: 'contacts-value' }), + contactsActor, + ); + return { owner, calendar, contacts, calendarActor, contactsActor }; + }; + + const crossApp = ( + actorForCall: Actor, + targetAppUid: string, + fn: (optConfig: { appUuid: string }) => T | Promise, + ) => inCtx(() => fn({ appUuid: targetAppUid }), actorForCall); + + it("reads another app's entry with a matching grant", async () => { + const { owner, calendar, contacts, calendarActor } = await setup(); + await grant( + owner, + calendar.uid, + appDataPermission(contacts.uid, 'kv', 'get'), + ); + + // Also the positive control for the `not.toHaveBeenCalled()` + // assertions below: it proves the spy is attached to the same + // service instance the driver consults, so those negatives mean + // "no check happened" rather than "the spy saw nothing". + const spy = vi.spyOn(permissions(), 'check'); + try { + const res = await crossApp( + calendarActor, + contacts.uid, + (optConfig) => target.get({ key: 'entry', optConfig }), + ); + expect(res).toBe('contacts-value'); + expect(spy).toHaveBeenCalledWith( + expect.anything(), + appDataPermission(contacts.uid, 'kv', 'get'), + ); + } finally { + spy.mockRestore(); + } + }); + + it('refuses without a grant', async () => { + const { contacts, calendarActor } = await setup(); + await expect( + crossApp(calendarActor, contacts.uid, (optConfig) => + target.get({ key: 'entry', optConfig }), + ), + ).rejects.toMatchObject({ statusCode: 403 }); + }); + + it('404s when the target uid names no app', async () => { + const { calendarActor } = await setup(); + await expect( + crossApp(calendarActor, 'app-does-not-exist', (optConfig) => + target.get({ key: 'entry', optConfig }), + ), + ).rejects.toMatchObject({ statusCode: 404 }); + }); + + it('refuses when the target app has opted out of sharing', async () => { + const { owner, calendar, contacts, calendarActor } = await setup({ + metadata: JSON.stringify({ share_app_data: false }), + }); + await grant( + owner, + calendar.uid, + appDataPermission(contacts.uid, 'kv', 'get'), + ); + await expect( + crossApp(calendarActor, contacts.uid, (optConfig) => + target.get({ key: 'entry', optConfig }), + ), + ).rejects.toMatchObject({ statusCode: 403 }); + }); + + it('keeps read and write distinct', async () => { + const { owner, calendar, contacts, calendarActor } = await setup(); + await grant( + owner, + calendar.uid, + appDataPermission(contacts.uid, 'kv', 'read'), + ); + expect( + await crossApp(calendarActor, contacts.uid, (optConfig) => + target.get({ key: 'entry', optConfig }), + ), + ).toBe('contacts-value'); + await expect( + crossApp(calendarActor, contacts.uid, (optConfig) => + target.set({ + key: 'entry', + value: 'overwritten', + optConfig, + }), + ), + ).rejects.toMatchObject({ statusCode: 403 }); + }); + + it("writes into the target's namespace, where the target app sees it", async () => { + const { owner, calendar, contacts, calendarActor, contactsActor } = + await setup(); + await grant( + owner, + calendar.uid, + appDataPermission(contacts.uid, 'kv', 'write'), + ); + await crossApp(calendarActor, contacts.uid, (optConfig) => + target.set({ + key: 'invite', + value: 'from-calendar', + optConfig, + }), + ); + // Read back as contacts itself — proves the write landed in the + // target namespace rather than the caller's own. + expect( + await inCtx(() => target.get({ key: 'invite' }), contactsActor), + ).toBe('from-calendar'); + }); + + it('keeps delete orthogonal to write', async () => { + const write = await setup(); + await grant( + write.owner, + write.calendar.uid, + appDataPermission(write.contacts.uid, 'kv', 'write'), + ); + await expect( + crossApp(write.calendarActor, write.contacts.uid, (optConfig) => + target.del({ key: 'entry', optConfig }), + ), + ).rejects.toMatchObject({ statusCode: 403 }); + + const del = await setup(); + await grant( + del.owner, + del.calendar.uid, + appDataPermission(del.contacts.uid, 'kv', 'delete'), + ); + await expect( + crossApp(del.calendarActor, del.contacts.uid, (optConfig) => + target.set({ key: 'entry', value: 'nope', optConfig }), + ), + ).rejects.toMatchObject({ statusCode: 403 }); + }); + + it('permits every delete op with the delete class', async () => { + const { owner, calendar, contacts, calendarActor } = await setup(); + await grant( + owner, + calendar.uid, + appDataPermission(contacts.uid, 'kv', 'delete'), + ); + const uid = contacts.uid; + await crossApp(calendarActor, uid, (optConfig) => + target.expire({ key: 'entry', ttl: 60, optConfig }), + ); + await crossApp(calendarActor, uid, (optConfig) => + target.expireAt({ + key: 'entry', + timestamp: 4_000_000_000, + optConfig, + }), + ); + await crossApp(calendarActor, uid, (optConfig) => + target.del({ key: 'entry', optConfig }), + ); + // `remove` needs an object value to strip a path from. + await crossApp(calendarActor, uid, (optConfig) => + target.remove({ key: 'entry', paths: ['x'], optConfig }), + ); + }); + + it('refuses flush at any scope, without consulting permissions', async () => { + const { owner, calendar, contacts, calendarActor } = await setup(); + // App-wide grant: the widest scope that exists. + await grant(owner, calendar.uid, appDataPermission(contacts.uid)); + const spy = vi.spyOn(permissions(), 'check'); + try { + await expect( + crossApp(calendarActor, contacts.uid, (optConfig) => + target.flush({ optConfig }), + ), + ).rejects.toMatchObject({ statusCode: 403 }); + expect(spy).not.toHaveBeenCalled(); + } finally { + spy.mockRestore(); + } + }); + + it('requires the delete class for an expiry on a write', async () => { + const { owner, calendar, contacts, calendarActor } = await setup(); + await grant( + owner, + calendar.uid, + appDataPermission(contacts.uid, 'kv', 'write'), + ); + const uid = contacts.uid; + + // An expiry deletes the entry once it lapses, so `write` alone is + // not enough — on set, on update's ttl, or per item in batchPut. + await expect( + crossApp(calendarActor, uid, (optConfig) => + target.set({ + key: 'entry', + value: 'v', + expireAt: 4_000_000_000, + optConfig, + }), + ), + ).rejects.toMatchObject({ statusCode: 403 }); + await expect( + crossApp(calendarActor, uid, (optConfig) => + target.update({ + key: 'doc', + pathAndValueMap: { a: 1 }, + ttl: 60, + optConfig, + }), + ), + ).rejects.toMatchObject({ statusCode: 403 }); + await expect( + crossApp(calendarActor, uid, (optConfig) => + target.batchPut({ + items: [ + { key: 'a', value: 1 }, + { key: 'b', value: 2, expireAt: 4_000_000_000 }, + ], + optConfig, + }), + ), + ).rejects.toMatchObject({ statusCode: 403 }); + + // The same write with no expiry is fine. + await crossApp(calendarActor, uid, (optConfig) => + target.set({ key: 'entry', value: 'v', optConfig }), + ); + }); + + it('allows an expiry once the delete class is granted too', async () => { + const { owner, calendar, contacts, calendarActor } = await setup(); + await grant( + owner, + calendar.uid, + appDataPermission(contacts.uid, 'kv', 'write'), + ); + await grant( + owner, + calendar.uid, + appDataPermission(contacts.uid, 'kv', 'delete'), + ); + await crossApp(calendarActor, contacts.uid, (optConfig) => + target.set({ + key: 'entry', + value: 'v', + expireAt: 4_000_000_000, + optConfig, + }), + ); + }); + + // -- Compatibility ------------------------------------------------- + + it('does not consult permissions for an own-namespace call', async () => { + const { contactsActor } = await setup(); + const spy = vi.spyOn(permissions(), 'check'); + try { + expect( + await inCtx( + () => target.get({ key: 'entry' }), + contactsActor, + ), + ).toBe('contacts-value'); + expect(spy).not.toHaveBeenCalled(); + } finally { + spy.mockRestore(); + } + }); + + it('does not consult permissions when an app names itself', async () => { + const { contacts, contactsActor } = await setup(); + const spy = vi.spyOn(permissions(), 'check'); + try { + expect( + await crossApp(contactsActor, contacts.uid, (optConfig) => + target.get({ key: 'entry', optConfig }), + ), + ).toBe('contacts-value'); + expect(spy).not.toHaveBeenCalled(); + } finally { + spy.mockRestore(); + } + }); + + it('still honours appUuid for a user-only actor with no grant', async () => { + const { owner, contacts } = await setup(); + // The user owns the data in every one of their app namespaces, so + // this path is deliberately ungated — tightening it would break + // existing dashboard and API-token callers. + const spy = vi.spyOn(permissions(), 'check'); + try { + expect( + await crossApp(owner, contacts.uid, (optConfig) => + target.get({ key: 'entry', optConfig }), + ), + ).toBe('contacts-value'); + expect(spy).not.toHaveBeenCalled(); + } finally { + spy.mockRestore(); + } + }); + + // -- Per-key privacy ---------------------------------------------- + + it('hides an entry the owning app marked private', async () => { + const { owner, calendar, contacts, calendarActor, contactsActor } = + await setup(); + await inCtx( + () => + target.set({ + key: 'token', + value: 'oauth-secret', + optConfig: { disableSharing: true }, + }), + contactsActor, + ); + // The widest possible grant still does not reach it. + await grant(owner, calendar.uid, appDataPermission(contacts.uid)); + + // Absent, not refused: the flag must not confirm what is stored. + expect( + await crossApp(calendarActor, contacts.uid, (optConfig) => + target.get({ key: 'token', optConfig }), + ), + ).toBeNull(); + // Its unflagged neighbour is still visible. + expect( + await crossApp(calendarActor, contacts.uid, (optConfig) => + target.get({ key: 'entry', optConfig }), + ), + ).toBe('contacts-value'); + // And the owning app sees its own entry normally. + expect( + await inCtx(() => target.get({ key: 'token' }), contactsActor), + ).toBe('oauth-secret'); + }); + + it('omits private entries from a cross-app list but not the owner’s', async () => { + const { owner, calendar, contacts, calendarActor, contactsActor } = + await setup(); + await inCtx( + () => + target.set({ + key: 'token', + value: 'oauth-secret', + optConfig: { disableSharing: true }, + }), + contactsActor, + ); + await grant(owner, calendar.uid, appDataPermission(contacts.uid)); + + const seen = (await crossApp( + calendarActor, + contacts.uid, + (optConfig) => target.list({ as: 'keys', optConfig }), + )) as string[]; + expect(seen).toContain('entry'); + expect(seen).not.toContain('token'); + + const own = (await inCtx( + () => target.list({ as: 'keys' }), + contactsActor, + )) as string[]; + expect(own).toContain('token'); + }); + + it('refuses cross-app writes and deletes against a private entry', async () => { + const { owner, calendar, contacts, calendarActor, contactsActor } = + await setup(); + await inCtx( + () => + target.set({ + key: 'token', + value: 'oauth-secret', + optConfig: { disableSharing: true }, + }), + contactsActor, + ); + await grant(owner, calendar.uid, appDataPermission(contacts.uid)); + + // A write must refuse rather than behave as absent — treating it as + // missing would overwrite the value the flag exists to protect. + await expect( + crossApp(calendarActor, contacts.uid, (optConfig) => + target.set({ key: 'token', value: 'clobbered', optConfig }), + ), + ).rejects.toMatchObject({ statusCode: 403 }); + await expect( + crossApp(calendarActor, contacts.uid, (optConfig) => + target.del({ key: 'token', optConfig }), + ), + ).rejects.toMatchObject({ statusCode: 403 }); + + // Still intact for its owner. + expect( + await inCtx(() => target.get({ key: 'token' }), contactsActor), + ).toBe('oauth-secret'); + }); + + it('gates a token an app minted, not just the app itself', async () => { + const { owner, contacts, calendarActor } = await setup(); + // An access-token actor carries no `app` of its own — the app is on + // `accessToken.issuer`. Reading `actor.app` here would take the + // ungated user-token branch and hand the token the whole namespace. + const token = asTokenOf(owner, calendarActor); + await expect( + crossApp(token, contacts.uid, (optConfig) => + target.get({ key: 'entry', optConfig }), + ), + ).rejects.toMatchObject({ statusCode: 403 }); + }); + + it('resolves a token to its minting app, not to a bare user', async () => { + const { owner, contacts, calendarActor } = await setup(); + const token = asTokenOf(owner, calendarActor); + const spy = vi.spyOn(permissions(), 'check'); + try { + await expect( + crossApp(token, contacts.uid, (optConfig) => + target.get({ key: 'entry', optConfig }), + ), + ).rejects.toMatchObject({ statusCode: 403 }); + // The ungated user-token branch never consults permissions at + // all, so the call itself is the evidence the token was read as + // app-scoped. + expect(spy).toHaveBeenCalledWith( + expect.anything(), + appDataPermission(contacts.uid, 'kv', 'get'), + ); + } finally { + spy.mockRestore(); + } + }); + + it('reads through a token that carries the scope itself', async () => { + const { owner, calendar, contacts, calendarActor } = await setup(); + const permission = appDataPermission(contacts.uid, 'kv', 'read'); + await grant(owner, calendar.uid, permission); + + // A scoped token does not inherit its issuer's grants — it needs + // the row too. Both halves have to line up for the read to land. + const token = asTokenOf(owner, calendarActor); + await server.clients.db.write( + 'INSERT INTO `access_token_permissions` (`token_uid`, `permission`) VALUES (?, ?)', + [token.accessToken!.uid, permission], + ); + + expect( + await crossApp(token, contacts.uid, (optConfig) => + target.get({ key: 'entry', optConfig }), + ), + ).toBe('contacts-value'); + }); + + it("files a token's own writes under the minting app's namespace", async () => { + const owner = await makeOwner(); + const calendar = await makeRealApp(owner.user.id!); + const calendarActor = asApp(owner, calendar); + const token = asTokenOf(owner, calendarActor); + + await inCtx(() => target.set({ key: 'own', value: 'v' }), token); + // Not the shared global namespace: the gate reads the token as + // app-scoped, so the store has to file it the same way. + expect( + await inCtx(() => target.get({ key: 'own' }), calendarActor), + ).toBe('v'); + }); + + it('honours disableSharing on a batch write', async () => { + const { owner, calendar, contacts, calendarActor, contactsActor } = + await setup(); + await inCtx( + () => + target.batchPut({ + items: [ + { key: 'b1', value: 'v1' }, + { key: 'b2', value: 'v2' }, + ], + optConfig: { disableSharing: true }, + }), + contactsActor, + ); + await grant(owner, calendar.uid, appDataPermission(contacts.uid)); + + // Silently dropping the flag here would hand a granted app entries + // the owner asked to keep to itself. + expect( + await crossApp(calendarActor, contacts.uid, (optConfig) => + target.get({ key: ['b1', 'b2'], optConfig }), + ), + ).toEqual([null, null]); + expect( + await inCtx( + () => target.get({ key: ['b1', 'b2'] }), + contactsActor, + ), + ).toEqual(['v1', 'v2']); + }); + + it('refuses disableSharing on a cross-app write', async () => { + const { owner, calendar, contacts, calendarActor } = await setup(); + await grant(owner, calendar.uid, appDataPermission(contacts.uid)); + // Otherwise one app could hide data inside another app's namespace. + await expect( + crossApp(calendarActor, contacts.uid, (optConfig) => + target.set({ + key: 'sneaky', + value: 'v', + optConfig: { ...optConfig, disableSharing: true }, + }), + ), + ).rejects.toMatchObject({ statusCode: 400 }); + }); + + it('lets the owning app clear the flag by rewriting the entry', async () => { + const { owner, calendar, contacts, calendarActor, contactsActor } = + await setup(); + await inCtx( + () => + target.set({ + key: 'token', + value: 'secret', + optConfig: { disableSharing: true }, + }), + contactsActor, + ); + await grant(owner, calendar.uid, appDataPermission(contacts.uid)); + expect( + await crossApp(calendarActor, contacts.uid, (optConfig) => + target.get({ key: 'token', optConfig }), + ), + ).toBeNull(); + + // `put` replaces the item, so a write without the flag re-shares it. + await inCtx( + () => target.set({ key: 'token', value: 'now-public' }), + contactsActor, + ); + expect( + await crossApp(calendarActor, contacts.uid, (optConfig) => + target.get({ key: 'token', optConfig }), + ), + ).toBe('now-public'); + }); + + it('maps every public driver method to an op', () => { + // A method added without a mapping resolves to `undefined` and fails + // closed at the call site — correct, but silently unreachable across + // apps. This fails instead, so the omission is a decision. + const methods = Object.getOwnPropertyNames( + Object.getPrototypeOf(target) as object, + ).filter( + (name) => + name !== 'constructor' && + name !== 'getReportedCosts' && + typeof (target as unknown as Record)[ + name + ] === 'function', + ); + expect(methods.length).toBeGreaterThan(0); + for (const name of methods) { + expect(APP_DATA_KV_METHOD_OPS).toHaveProperty(name); + } + }); + }); }); diff --git a/src/backend/drivers/kv/KVStoreDriver.ts b/src/backend/drivers/kv/KVStoreDriver.ts index 6f0992cb1..bc642a3c0 100644 --- a/src/backend/drivers/kv/KVStoreDriver.ts +++ b/src/backend/drivers/kv/KVStoreDriver.ts @@ -26,9 +26,25 @@ import { import { PuterDriver } from '../types.js'; import type { Actor } from '../../core/actor.js'; import type { DriverRateLimitConfig } from '../meta.js'; -import type { KVUsage } from '../../stores/systemKv/SystemKVStore.js'; +import { + APP_DATA_KV_METHOD_OPS, + APP_DATA_KV_TTL_PARAMS, + appDataPermission, + appDataSharingAllowed, +} from '../../services/permission/appDataScopes.js'; +import type { KVOpts, KVUsage } from '../../stores/systemKv/SystemKVStore.js'; import { KV_COSTS } from './costs.js'; +/** + * Every KV method's argument object, as far as option resolution cares: the + * namespace override, plus the expiry parameters that can delete an entry. + */ +type KvCallArgs = { + optConfig?: { appUuid?: string; disableSharing?: boolean }; + expireAt?: unknown; + ttl?: unknown; +}; + /** * KV store driver implementing the `puter-kvstore` interface. * @@ -83,13 +99,106 @@ export class KVStoreDriver extends PuterDriver { return str; } - #opts(appUuid?: string): { actor: Actor | undefined; appUuid?: string } { + async #opts(method: string, args: KvCallArgs): Promise { const actor = Context.get('actor') as Actor | undefined; - if (actor?.app?.uid) { - // force appUuid to be the one from the actor if it exists, only root tokens allowed to override appUuid - appUuid = undefined; + const appUuid = args.optConfig?.appUuid; + // Through the issuer chain, not `actor.app`: an access-token actor + // carries no app of its own, so keying off `app` would read a token an + // app minted as a bare user token and hand it the ungated branch below + // — no permission check, and no private-entry filtering either, since + // the store keys that off `namespaceAppUuid`. + const ownAppUid = actor?.effectiveApp?.uid; + + // A user or API token acting on its own data: ungated, as before. + if (!ownAppUid) return { actor, appUuid }; + + // Self-access is implicit, so it never reaches a permission lookup. + if (!appUuid || appUuid === ownAppUid) return { actor }; + + // Only an entry's owner may mark it private; otherwise one app could + // hide data inside another's namespace. + if (args.optConfig?.disableSharing) { + throw new HttpError( + 400, + "kv: `disableSharing` cannot be set on another app's data", + { legacyCode: 'bad_request' }, + ); + } + + await this.#assertCrossAppKvAccess(actor!, appUuid, method, args); + return { actor, namespaceAppUuid: appUuid }; + } + + async #assertCrossAppKvAccess( + actor: Actor, + targetAppUid: string, + method: string, + args: KvCallArgs, + ): Promise { + // `null` = no scope reaches it (`flush` is namespace-wide, not an + // entry op); `undefined` = unmapped method. Both fail closed. + const op = APP_DATA_KV_METHOD_OPS[method]; + if (!op) { + throw new HttpError( + 403, + `kv: \`${method}\` is not available on another app's data`, + { legacyCode: 'forbidden' }, + ); + } + + const target = await this.stores.app.getByUid(targetAppUid); + if (!target) { + throw new HttpError(404, `entity_not_found: app:${targetAppUid}`, { + legacyCode: 'subject_does_not_exist', + }); + } + if (!appDataSharingAllowed(target)) { + throw new HttpError( + 403, + 'kv: this app does not share its data with other apps', + { legacyCode: 'forbidden' }, + ); + } + + if ( + !(await this.services.permission.check( + actor, + appDataPermission(targetAppUid, 'kv', op), + )) + ) { + throw new HttpError(403, 'Permission denied', { + legacyCode: 'forbidden', + }); + } + + const raw = args as Record; + if ( + APP_DATA_KV_TTL_PARAMS.some( + (p) => raw[p] !== undefined && raw[p] !== null, + ) + ) { + await this.#assertCrossAppExpiry(actor, targetAppUid); + } + } + + /** + * An expiry destroys the entry once it lapses, so carrying one needs the + * delete class on top of the write. The class, not an op: `kv:del` alone + * means "may remove keys", not "may attach expiries". + */ + async #assertCrossAppExpiry( + actor: Actor, + targetAppUid: string, + ): Promise { + const granted = await this.services.permission.check( + actor, + appDataPermission(targetAppUid, 'kv', 'delete'), + ); + if (!granted) { + throw new HttpError(403, 'Permission denied', { + legacyCode: 'forbidden', + }); } - return { actor: Context.get('actor') as Actor | undefined, appUuid }; } #meter(actor: Actor | undefined, usage: KVUsage): void { @@ -131,14 +240,14 @@ export class KVStoreDriver extends PuterDriver { key: unknown; optConfig?: { appUuid?: string }; }): Promise { - const { key, optConfig } = args; + const { key } = args; if (key === undefined || key === null) { throw new HttpError(400, 'Missing `key`', { legacyCode: 'bad_request', }); // legacyCode for backward compatibility with old error handling in controllers } - const opts = this.#opts(optConfig?.appUuid); + const opts = await this.#opts('get', args); if (Array.isArray(key)) { if (key.length === 0) return []; @@ -163,18 +272,23 @@ export class KVStoreDriver extends PuterDriver { key: unknown; value: unknown; expireAt?: number; - optConfig?: { appUuid?: string }; + optConfig?: { appUuid?: string; disableSharing?: boolean }; }): Promise { - const { key, value, expireAt, optConfig } = args; + const { key, value, expireAt } = args; const coerced = this.#coerceKey(key); if (value === undefined) throw new HttpError(400, 'Missing `value`', { legacyCode: 'bad_request', }); // legacyCode for backward compatibility with old error handling in controllers - const opts = this.#opts(optConfig?.appUuid); + const opts = await this.#opts('set', args); const { res, usage } = await this.stores.kv.set( - { key: coerced, value, expireAt }, + { + key: coerced, + value, + expireAt, + disableSharing: args.optConfig?.disableSharing, + }, opts, ); this.#meter(opts.actor, usage); @@ -183,9 +297,9 @@ export class KVStoreDriver extends PuterDriver { async batchPut(args: { items: Array<{ key: string; value: unknown; expireAt?: number }>; - optConfig?: { appUuid?: string }; + optConfig?: { appUuid?: string; disableSharing?: boolean }; }): Promise { - const { items, optConfig } = args; + const { items } = args; if (!Array.isArray(items) || items.length === 0) { throw new HttpError(400, 'Missing or empty `items`', { legacyCode: 'bad_request', @@ -198,9 +312,24 @@ export class KVStoreDriver extends PuterDriver { expireAt: item.expireAt, })); - const opts = this.#opts(optConfig?.appUuid); + const opts = await this.#opts('batchPut', args); + // Per-item expiry, which the top-level scan in `#opts` cannot see. + if ( + opts.namespaceAppUuid && + coerced.some( + (item) => item.expireAt !== undefined && item.expireAt !== null, + ) + ) { + await this.#assertCrossAppExpiry( + opts.actor!, + opts.namespaceAppUuid, + ); + } const { res, usage } = await this.stores.kv.batchPut( - { items: coerced }, + { + items: coerced, + disableSharing: args.optConfig?.disableSharing, + }, opts, ); this.#meter(opts.actor, usage); @@ -212,7 +341,7 @@ export class KVStoreDriver extends PuterDriver { optConfig?: { appUuid?: string }; }): Promise { const coerced = this.#coerceKey(args.key); - const opts = this.#opts(args.optConfig?.appUuid); + const opts = await this.#opts('del', args); const { res, usage } = await this.stores.kv.del({ key: coerced }, opts); this.#meter(opts.actor, usage); return res; @@ -228,7 +357,7 @@ export class KVStoreDriver extends PuterDriver { fetchUntilFull?: boolean; optConfig?: { appUuid?: string }; }): Promise { - const opts = this.#opts(args.optConfig?.appUuid); + const opts = await this.#opts('list', args); const { res, usage } = await this.stores.kv.list( { as: args.as, @@ -246,7 +375,7 @@ export class KVStoreDriver extends PuterDriver { } async flush(args: { optConfig?: { appUuid?: string } }): Promise { - const opts = this.#opts(args.optConfig?.appUuid); + const opts = await this.#opts('flush', args); const { res, usage } = await this.stores.kv.flush(opts); this.#meter(opts.actor, usage); return res; @@ -266,7 +395,7 @@ export class KVStoreDriver extends PuterDriver { legacyCode: 'bad_request', }); } - const opts = this.#opts(args.optConfig?.appUuid); + const opts = await this.#opts('incr', args); const { res, usage } = await this.stores.kv.incr( { key: coerced, pathAndAmountMap: args.pathAndAmountMap }, opts, @@ -289,7 +418,7 @@ export class KVStoreDriver extends PuterDriver { legacyCode: 'bad_request', }); } - const opts = this.#opts(args.optConfig?.appUuid); + const opts = await this.#opts('decr', args); const { res, usage } = await this.stores.kv.decr( { key: coerced, pathAndAmountMap: args.pathAndAmountMap }, opts, @@ -309,7 +438,7 @@ export class KVStoreDriver extends PuterDriver { legacyCode: 'bad_request', }); } - const opts = this.#opts(args.optConfig?.appUuid); + const opts = await this.#opts('expireAt', args); const { usage } = await this.stores.kv.expireAt( { key: coerced, timestamp: args.timestamp }, opts, @@ -328,7 +457,7 @@ export class KVStoreDriver extends PuterDriver { legacyCode: 'bad_request', }); } - const opts = this.#opts(args.optConfig?.appUuid); + const opts = await this.#opts('expire', args); const { usage } = await this.stores.kv.expire( { key: coerced, ttl: args.ttl }, opts, @@ -348,7 +477,7 @@ export class KVStoreDriver extends PuterDriver { legacyCode: 'bad_request', }); } - const opts = this.#opts(args.optConfig?.appUuid); + const opts = await this.#opts('update', args); const { res, usage } = await this.stores.kv.update( { key: coerced, @@ -372,7 +501,7 @@ export class KVStoreDriver extends PuterDriver { legacyCode: 'bad_request', }); } - const opts = this.#opts(args.optConfig?.appUuid); + const opts = await this.#opts('add', args); const { res, usage } = await this.stores.kv.add( { key: coerced, pathAndValueMap: args.pathAndValueMap }, opts, @@ -392,7 +521,7 @@ export class KVStoreDriver extends PuterDriver { legacyCode: 'bad_request', }); } - const opts = this.#opts(args.optConfig?.appUuid); + const opts = await this.#opts('remove', args); const { res, usage } = await this.stores.kv.remove( { key: coerced, paths: args.paths }, opts, diff --git a/src/backend/drivers/workers/WorkerDriver.ts b/src/backend/drivers/workers/WorkerDriver.ts index 7a7b1910d..03fea6d31 100644 --- a/src/backend/drivers/workers/WorkerDriver.ts +++ b/src/backend/drivers/workers/WorkerDriver.ts @@ -21,7 +21,7 @@ import { existsSync, readFileSync } from 'node:fs'; import path from 'node:path'; import type { EventMetadata } from '../../clients/event/types.js'; -import type { Actor } from '../../core/actor.js'; +import { makeActor, type Actor } from '../../core/actor.js'; import { Context } from '../../core/context.js'; import { HttpError, type LegacyErrorCodes } from '../../core/http/HttpError.js'; import { assertVerifiedEmail } from '../../core/http/verifiedEmail.js'; @@ -809,7 +809,7 @@ export class WorkerDriver extends PuterDriver { try { const ownerUser = await this.stores.user.getById(entry.userId); if (!ownerUser) continue; - const ownerActor = { user: ownerUser } as Actor; + const ownerActor = makeActor({ user: ownerUser }); // Read the updated file content. `ownerActor` is the file's // owner from the originating write event, so the read-ACL diff --git a/src/backend/services/acl/ACLService.ts b/src/backend/services/acl/ACLService.ts index 74be3d576..58d5fba40 100644 --- a/src/backend/services/acl/ACLService.ts +++ b/src/backend/services/acl/ACLService.ts @@ -46,11 +46,7 @@ export interface ResourceDescriptor { } export type AclMode = - | 'see' - | 'list' - | 'read' - | 'write' - | typeof MANAGE_PERM_PREFIX; + 'see' | 'list' | 'read' | 'write' | typeof MANAGE_PERM_PREFIX; /** Duck-typed error shape compatible with APIError consumers (fsv2). */ export interface AclError { @@ -207,7 +203,7 @@ export class ACLService extends PuterService { // App-under-user: underlying user must also hold the permission. if (actor.app) { - const userActor: Actor = { user: actor.user }; + const userActor: Actor = { user: actor.user, effectiveApp: null }; if (!(await this.check(userActor, resource, mode))) return false; // Shared-appdata rule: an app accessing its AppData under a diff --git a/src/backend/services/apps/AppPermissionService.test.ts b/src/backend/services/apps/AppPermissionService.test.ts index 93ce30b48..fa04afe38 100644 --- a/src/backend/services/apps/AppPermissionService.test.ts +++ b/src/backend/services/apps/AppPermissionService.test.ts @@ -18,11 +18,12 @@ */ import { v4 as uuidv4 } from 'uuid'; -import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; import type { Actor } from '../../core/actor.js'; import { runWithContext } from '../../core/context.js'; import type { PuterServer } from '../../server.js'; import { createTestUser, setupTestServer } from '../../testUtil.js'; +import { appDataPermission } from '../permission/appDataScopes.js'; import { PERMISSION_FOR_NOTHING_IN_PARTICULAR } from '../permission/consts.js'; import type { PermissionService } from '../permission/PermissionService.js'; @@ -422,3 +423,557 @@ describe('AppPermissionService — app-root-dir rewriter', () => { ).rejects.toMatchObject({ statusCode: 404 }); }); }); + +// -- app-data::: ----------------------------------- + +describe('AppPermissionService — app-data cross-app permissions', () => { + /** + * A grantee app (the one asking — think a calendar) plus the target app + * whose data it names (a contacts app), and an app-under-user actor for the + * grantee. `targetOwner` defaults to the acting user, but the data + * namespace belongs to the actor either way. + */ + const makeGranteeAndTarget = async ( + owner: Actor, + targetOwner: Actor = owner, + ) => { + const grantee = await makeApp(owner.user.id!); + const target = await makeApp(targetOwner.user.id!); + const granteeActor: Actor = { + user: owner.user, + app: { uid: grantee.uid, id: grantee.id }, + }; + return { grantee, target, granteeActor }; + }; + + const grant = (owner: Actor, granteeAppUid: string, permission: string) => + runWithContext({ actor: owner }, () => + permissions.grantUserAppPermission( + owner, + granteeAppUid, + permission, + ), + ); + + it('lets a user act on any app-data namespace under their own account', async () => { + const owner = await makeUser(); + const target = await makeApp(owner.user.id!); + expect( + await permissions.check( + owner, + appDataPermission(target.uid, 'kv', 'get'), + ), + ).toBe(true); + expect( + await permissions.check( + owner, + appDataPermission(target.uid, 'fs', 'read'), + ), + ).toBe(true); + }); + + it('gives an app no reach into another app by default', async () => { + const owner = await makeUser(); + const { target, granteeActor } = await makeGranteeAndTarget(owner); + expect( + await permissions.check( + granteeActor, + appDataPermission(target.uid, 'kv', 'get'), + ), + ).toBe(false); + expect( + await permissions.check( + granteeActor, + appDataPermission(target.uid, 'fs', 'read'), + ), + ).toBe(false); + }); + + it('resolves an exact op grant through the issuing user', async () => { + const owner = await makeUser(); + const { grantee, target, granteeActor } = + await makeGranteeAndTarget(owner); + await grant( + owner, + grantee.uid, + appDataPermission(target.uid, 'kv', 'get'), + ); + expect( + await permissions.check( + granteeActor, + appDataPermission(target.uid, 'kv', 'get'), + ), + ).toBe(true); + }); + + it('treats a class grant as covering its ops and nothing wider', async () => { + const owner = await makeUser(); + const { grantee, target, granteeActor } = + await makeGranteeAndTarget(owner); + await grant( + owner, + grantee.uid, + appDataPermission(target.uid, 'kv', 'read'), + ); + for (const op of ['get', 'list'] as const) { + expect( + await permissions.check( + granteeActor, + appDataPermission(target.uid, 'kv', op), + ), + ).toBe(true); + } + // A read class must not reach a mutating op, and must not reach a + // delete either — `delete` is its own class. + for (const op of ['set', 'incr', 'update', 'del'] as const) { + expect( + await permissions.check( + granteeActor, + appDataPermission(target.uid, 'kv', op), + ), + ).toBe(false); + } + }); + + it('treats a write grant as covering the matching read', async () => { + const owner = await makeUser(); + const { grantee, target, granteeActor } = + await makeGranteeAndTarget(owner); + await grant( + owner, + grantee.uid, + appDataPermission(target.uid, 'kv', 'write'), + ); + for (const op of ['set', 'get'] as const) { + expect( + await permissions.check( + granteeActor, + appDataPermission(target.uid, 'kv', op), + ), + ).toBe(true); + } + + const second = await makeGranteeAndTarget(owner); + await grant( + owner, + second.grantee.uid, + appDataPermission(second.target.uid, 'fs', 'write'), + ); + expect( + await permissions.check( + second.granteeActor, + appDataPermission(second.target.uid, 'fs', 'read'), + ), + ).toBe(true); + }); + + it('honours store-level and app-level grants via prefix implication', async () => { + const owner = await makeUser(); + const storeWide = await makeGranteeAndTarget(owner); + await grant( + owner, + storeWide.grantee.uid, + appDataPermission(storeWide.target.uid, 'kv'), + ); + for (const op of ['get', 'set'] as const) { + expect( + await permissions.check( + storeWide.granteeActor, + appDataPermission(storeWide.target.uid, 'kv', op), + ), + ).toBe(true); + } + // Store-level for one store says nothing about the other. + expect( + await permissions.check( + storeWide.granteeActor, + appDataPermission(storeWide.target.uid, 'fs', 'read'), + ), + ).toBe(false); + + const appWide = await makeGranteeAndTarget(owner); + await grant( + owner, + appWide.grantee.uid, + appDataPermission(appWide.target.uid), + ); + expect( + await permissions.check( + appWide.granteeActor, + appDataPermission(appWide.target.uid, 'kv', 'get'), + ), + ).toBe(true); + expect( + await permissions.check( + appWide.granteeActor, + appDataPermission(appWide.target.uid, 'fs', 'write'), + ), + ).toBe(true); + }); + + it('does not let a grant naming one app satisfy another', async () => { + const owner = await makeUser(); + const { grantee, target, granteeActor } = + await makeGranteeAndTarget(owner); + const unrelated = await makeApp(owner.user.id!); + await grant( + owner, + grantee.uid, + appDataPermission(target.uid, 'kv', 'get'), + ); + expect( + await permissions.check( + granteeActor, + appDataPermission(unrelated.uid, 'kv', 'get'), + ), + ).toBe(false); + }); + + it('resolves for a target app owned by another user', async () => { + const owner = await makeUser(); + const stranger = await makeUser(); + const { grantee, target, granteeActor } = await makeGranteeAndTarget( + owner, + stranger, + ); + // The KV namespace and AppData directory belong to the acting user, + // so who wrote the target app is irrelevant. + await grant( + owner, + grantee.uid, + appDataPermission(target.uid, 'kv', 'get'), + ); + expect( + await permissions.check( + granteeActor, + appDataPermission(target.uid, 'kv', 'get'), + ), + ).toBe(true); + }); + + it('denies an access-token actor the implicit user hold', async () => { + const owner = await makeUser(); + const target = await makeApp(owner.user.id!); + const tokenActor: Actor = { + user: owner.user, + accessToken: { uid: 'tok-1', issuer: owner, fullAccess: false }, + }; + expect( + await permissions.check( + tokenActor, + appDataPermission(target.uid, 'kv', 'get'), + ), + ).toBe(false); + }); + + it('has no `manage:` form, so it cannot be delegated without a prompt', async () => { + const owner = await makeUser(); + const holder = await makeUser(); + const { grantee, target } = await makeGranteeAndTarget(owner); + const permission = appDataPermission(target.uid, 'kv', 'get'); + + expect(await permissions.canManagePermission(owner, permission)).toBe( + false, + ); + // A developer's any-user grant and a user-to-user grant both gate on + // the manage form, so neither can hand out cross-app data access. + await expect( + runWithContext({ actor: owner }, () => + permissions.grantDevAppPermission( + owner, + grantee.uid, + permission, + ), + ), + ).rejects.toMatchObject({ statusCode: 403 }); + await expect( + runWithContext({ actor: owner }, () => + permissions.grantUserUserPermission( + owner, + holder.user.username!, + permission, + ), + ), + ).rejects.toMatchObject({ statusCode: 403 }); + }); + + it('ignores the bare namespace and lookalike prefixes', async () => { + const owner = await makeUser(); + expect(await permissions.check(owner, 'app-data')).toBe(false); + expect(await permissions.check(owner, 'app-database:x:read')).toBe( + false, + ); + }); + + it('keeps delete orthogonal to write', async () => { + const owner = await makeUser(); + const DELETE_OPS = ['del', 'remove', 'expire', 'expireAt'] as const; + + // A write grant must not reach any deletion, or "may add invites" + // would silently mean "may remove anything". + const w = await makeGranteeAndTarget(owner); + await grant( + owner, + w.grantee.uid, + appDataPermission(w.target.uid, 'kv', 'write'), + ); + for (const op of DELETE_OPS) { + expect( + await permissions.check( + w.granteeActor, + appDataPermission(w.target.uid, 'kv', op), + ), + ).toBe(false); + } + + // ...and a delete grant covers every deletion without conferring + // write, so cancelling an entry doesn't imply rewriting the rest. + const d = await makeGranteeAndTarget(owner); + await grant( + owner, + d.grantee.uid, + appDataPermission(d.target.uid, 'kv', 'delete'), + ); + for (const op of DELETE_OPS) { + expect( + await permissions.check( + d.granteeActor, + appDataPermission(d.target.uid, 'kv', op), + ), + ).toBe(true); + } + expect( + await permissions.check( + d.granteeActor, + appDataPermission(d.target.uid, 'kv', 'set'), + ), + ).toBe(false); + }); +}); + +// -- Withdrawing grants when the target app changes --------------------- + +describe('AppPermissionService — cross-app grant withdrawal', () => { + /** + * Emit the same event `AppDriver` emits, since these tests exercise the + * listener rather than the driver that triggers it. + */ + const emitAppChanged = async (payload: { + app_uid: string; + action: string; + app?: unknown; + old_app?: unknown; + }) => { + // `emitAndWait`, not `emit`: the listener is async and a fire-and-forget + // emit would race the assertions. + await server.clients.event.emitAndWait('app.changed', payload, {}); + }; + + const hasGrant = ( + owner: Actor, + granteeAppId: number, + permission: string, + ) => + server.stores.permission.hasUserAppPerm( + owner.user.id!, + granteeAppId, + permission, + ); + + const setupGrant = async (targetFields: Record = {}) => { + const owner = await makeUser(); + const grantee = await makeApp(owner.user.id!); + const target = await makeApp(owner.user.id!, targetFields); + const permission = appDataPermission(target.uid, 'kv', 'get'); + await runWithContext({ actor: owner }, () => + permissions.grantUserAppPermission(owner, grantee.uid, permission), + ); + expect(await hasGrant(owner, grantee.id, permission)).toBe(true); + return { owner, grantee, target, permission }; + }; + + it('withdraws grants naming an app that was deleted', async () => { + const { owner, grantee, target, permission } = await setupGrant(); + await emitAppChanged({ app_uid: target.uid, action: 'deleted' }); + expect(await hasGrant(owner, grantee.id, permission)).toBe(false); + }); + + it('withdraws grants when an origin bootstrap reuses a uid', async () => { + // An origin-derived uid is regenerated verbatim, so a recreated app + // must not inherit consent the user gave to its predecessor. Driven + // directly by the auth controller rather than through `app.changed`, + // because that path has to be able to refuse the token when the sweep + // fails and `emitAndWait` swallows listener errors. + const { owner, grantee, target, permission } = await setupGrant(); + await server.services.appPermission.withdrawAppDataGrants( + target.uid, + 'uid reused by a new app', + ); + expect(await hasGrant(owner, grantee.id, permission)).toBe(false); + }); + + it('propagates a sweep failure so the caller can refuse to proceed', async () => { + // Swallowing this is what would let a recreated app come up with the + // old grants still live. + const { target } = await setupGrant(); + const spy = vi + .spyOn( + server.stores.permission, + 'deleteAppGrantsByPermissionPrefix', + ) + .mockRejectedValue(new Error('db down')); + const alarm = vi.spyOn(server.clients.alarm, 'create'); + try { + await expect( + server.services.appPermission.withdrawAppDataGrants( + target.uid, + 'uid reused by a new app', + ), + ).rejects.toThrow('db down'); + expect(alarm).toHaveBeenCalledWith( + expect.stringContaining('app_data_grant_withdrawal_failed'), + expect.any(String), + expect.objectContaining({ targetAppUid: target.uid }), + 'warning', + ); + } finally { + spy.mockRestore(); + alarm.mockRestore(); + } + }); + + it('keeps an app.changed sweep best-effort so a delete still succeeds', async () => { + const { target } = await setupGrant(); + const spy = vi + .spyOn( + server.stores.permission, + 'deleteAppGrantsByPermissionPrefix', + ) + .mockRejectedValue(new Error('db down')); + try { + // Deleting an app must not fail because the sweep did — the alarm + // is what carries the failure, not an exception at the emit site. + await expect( + emitAppChanged({ app_uid: target.uid, action: 'deleted' }), + ).resolves.not.toThrow(); + } finally { + spy.mockRestore(); + } + }); + + it('does not sweep on an ordinary app creation', async () => { + // `AppStore.create` mints a random uuid4, so a fresh app cannot hold a + // uid a deleted one had. Scanning the grant tables here would be work + // that can never find anything. + const { owner, grantee, target, permission } = await setupGrant(); + await emitAppChanged({ app_uid: target.uid, action: 'created' }); + expect(await hasGrant(owner, grantee.id, permission)).toBe(true); + }); + + it('withdraws grants when the target stops sharing its data', async () => { + const { owner, grantee, target, permission } = await setupGrant(); + await emitAppChanged({ + app_uid: target.uid, + action: 'updated', + old_app: { metadata: null }, + app: { metadata: { share_app_data: false } }, + }); + expect(await hasGrant(owner, grantee.id, permission)).toBe(false); + }); + + it('leaves grants alone on an unrelated update', async () => { + const { owner, grantee, target, permission } = await setupGrant(); + await emitAppChanged({ + app_uid: target.uid, + action: 'updated', + old_app: { metadata: null }, + app: { metadata: { title: 'renamed' } }, + }); + expect(await hasGrant(owner, grantee.id, permission)).toBe(true); + }); + + it('withdraws every level of the namespace, and nothing outside it', async () => { + const owner = await makeUser(); + const grantee = await makeApp(owner.user.id!); + const target = await makeApp(owner.user.id!); + const other = await makeApp(owner.user.id!); + + const doomed = [ + appDataPermission(target.uid), + appDataPermission(target.uid, 'kv'), + appDataPermission(target.uid, 'fs', 'read'), + ]; + const survivors = [ + appDataPermission(other.uid, 'kv', 'get'), + `fs:${uuidv4()}:read`, + ]; + for (const permission of [...doomed, ...survivors]) { + await runWithContext({ actor: owner }, () => + permissions.grantUserAppPermission( + owner, + grantee.uid, + permission, + ), + ); + } + + await emitAppChanged({ app_uid: target.uid, action: 'deleted' }); + + for (const permission of doomed) { + expect(await hasGrant(owner, grantee.id, permission)).toBe(false); + } + for (const permission of survivors) { + expect(await hasGrant(owner, grantee.id, permission)).toBe(true); + } + }); + + it('does not withdraw a grant for a uid that merely shares a prefix', async () => { + const owner = await makeUser(); + const grantee = await makeApp(owner.user.id!); + const target = await makeApp(owner.user.id!); + // `` must not match `-extra`: the sweep anchors on a segment + // boundary, not a bare string prefix. + const lookalike = appDataPermission(`${target.uid}-extra`, 'kv', 'get'); + await runWithContext({ actor: owner }, () => + permissions.grantUserAppPermission(owner, grantee.uid, lookalike), + ); + + await emitAppChanged({ app_uid: target.uid, action: 'deleted' }); + expect(await hasGrant(owner, grantee.id, lookalike)).toBe(true); + }); + + it('makes the withdrawal effective immediately, not after the cache TTL', async () => { + const { owner, grantee, target, permission } = await setupGrant(); + const granteeActor = { + user: owner.user, + app: { uid: grantee.uid, id: grantee.id }, + } as Actor; + // Warm the scan cache with an allow. + expect(await permissions.check(granteeActor, permission)).toBe(true); + + await emitAppChanged({ app_uid: target.uid, action: 'deleted' }); + expect(await permissions.check(granteeActor, permission)).toBe(false); + }); + + it('withdraws dev-app grants too', async () => { + const owner = await makeUser(); + const grantee = await makeApp(owner.user.id!); + const target = await makeApp(owner.user.id!); + const permission = appDataPermission(target.uid, 'kv', 'get'); + // Dev-app grants gate on the manage form, which `app-data` has none of, + // so write the row directly — the point here is the sweep, not the gate. + await server.stores.permission.upsertDevAppPerm( + owner.user.id!, + grantee.id, + permission, + {}, + ); + + await emitAppChanged({ app_uid: target.uid, action: 'deleted' }); + const rows = await server.stores.permission.readDevAppPerms( + grantee.id, + [permission], + ); + expect(rows).toHaveLength(0); + }); +}); diff --git a/src/backend/services/apps/AppPermissionService.ts b/src/backend/services/apps/AppPermissionService.ts index 7c29b6e2b..ae30decb6 100644 --- a/src/backend/services/apps/AppPermissionService.ts +++ b/src/backend/services/apps/AppPermissionService.ts @@ -19,14 +19,22 @@ import { Context } from '../../core/context.js'; import { HttpError } from '../../core/http/HttpError.js'; +import type { puterStores } from '../../stores/index.js'; +import type { LayerInstances } from '../../types.js'; +import type { puterServices } from '../index.js'; +import { + APP_DATA_KV_OP_CLASSES, + APP_DATA_PERMISSION_PREFIX, + appDataPermission, + appDataSharingAllowed, + type AppDataKvOp, + type AppDataStore, +} from '../permission/appDataScopes.js'; import { MANAGE_PERM_PREFIX, PERMISSION_FOR_NOTHING_IN_PARTICULAR, } from '../permission/consts.js'; import { PermissionUtil } from '../permission/permissionUtil.js'; -import type { LayerInstances } from '../../types.js'; -import type { puterStores } from '../../stores/index.js'; -import type { puterServices } from '../index.js'; import { PuterService } from '../types.js'; /** @@ -202,6 +210,168 @@ export class AppPermissionService extends PuterService { return PermissionUtil.join('fs', entry.uuid, access, ...rest); }, }); + + // -- app-data::: ---------------------- + // One app reaching another's per-user state (KV namespace, AppData + // directory). Both live under the granting user, so the user holds them + // implicitly and `#scanUserApp` resolves a grant through here. + // + // No `manage:` form, deliberately: `canManagePermission` gates + // user-to-user and dev-app grants, so neither can hand out cross-app + // access without the user answering a prompt. + permissions.registerImplicator({ + id: 'user-holds-own-app-data', + matches: (permission: string) => + permission.startsWith(`${APP_DATA_PERMISSION_PREFIX}:`), + check: async ({ actor }): Promise => { + if (actor.app || actor.accessToken) return undefined; + if (!actor.user?.id) return undefined; + return {}; + }, + }); + + // Prefix implication already covers `…:kv` and `…:` grants; this + // covers the class level, so a check for a concrete op also accepts the + // class containing it. Mirrors `fs-access-levels` in FSService. + permissions.registerExploder({ + id: 'app-data-op-classes', + matches: (permission: string) => + permission.startsWith(`${APP_DATA_PERMISSION_PREFIX}:`), + explode: ({ permission }) => { + const parts = PermissionUtil.split(permission); + if (parts.length < 4) return [permission]; + const [, targetAppUid, store, op] = parts; + const out = [permission]; + const push = (cls: string) => { + out.push( + appDataPermission( + targetAppUid, + store as AppDataStore, + cls, + ), + ); + }; + if (store === 'kv') { + for (const cls of APP_DATA_KV_OP_CLASSES[ + op as AppDataKvOp + ] ?? []) { + push(cls); + } + } + // fs:read is satisfied by fs:write. `delete` is orthogonal — + // it implies neither, and neither implies it. + if (store === 'fs' && op === 'read') push('write'); + return out; + }, + }); + + // A cross-app grant names its target inside the permission string, so no + // foreign key withdraws it when that app goes away — and an + // origin-derived uid is regenerated verbatim if the app comes back, which + // would silently reattach the old consent to whoever controls the origin + // now. Sweep on both edges, and when an app stops sharing. + this.clients.event.on( + 'app.changed', + async (_key: string, data: unknown) => { + const d = data as + | { + app_uid?: string; + action?: string; + app?: unknown; + old_app?: unknown; + } + | undefined; + if (!d?.app_uid) return; + + let reason: string | null = null; + if (d.action === 'deleted') { + reason = 'target app deleted'; + } else if ( + d.action === 'updated' && + appDataSharingAllowed( + (d.old_app ?? {}) as { metadata?: unknown }, + ) && + !appDataSharingAllowed( + (d.app ?? {}) as { metadata?: unknown }, + ) + ) { + reason = 'target app stopped sharing its data'; + } + if (!reason) return; + + // Best-effort here, unlike the origin-bootstrap sweep the auth + // controller drives: deleting or updating an app must not fail + // because this did. `withdrawAppDataGrants` has already raised + // the alarm, so the failure is not silent. + try { + await this.withdrawAppDataGrants(d.app_uid, reason); + } catch { + // Already alarmed and logged. + } + }, + ); + } + + /** + * Withdraw every cross-app grant naming `targetAppUid`, audit each removal, + * and bust the holders' permission caches so the change is effective at + * once rather than after the scan TTL. + * + * Throws on failure. Every caller decides for itself whether a failed sweep + * is fatal: the event listener treats it as best-effort, because deleting + * an app must not fail because this did, while the origin-bootstrap path + * refuses to issue a token rather than let a new app inherit the consent + * its predecessor was given. + */ + async withdrawAppDataGrants( + targetAppUid: string, + reason: string, + ): Promise { + try { + const removed = + await this.stores.permission.deleteAppGrantsByPermissionPrefix( + appDataPermission(targetAppUid), + ); + if (removed.length === 0) return; + + const usernames = new Set(); + for (const row of removed) { + const audit = { + user_id: row.user_id, + app_id: row.app_id, + permission: row.permission, + action: 'revoke', + reason, + }; + if (row.table === 'user_to_app_permissions') { + await this.stores.permission.auditUserAppPerm(audit); + const user = await this.stores.user.getById(row.user_id); + if (user?.username) usernames.add(user.username); + } else { + await this.stores.permission.auditDevAppPerm(audit); + } + } + + if (usernames.size > 0) { + // A user-level bump also orphans that user's app actors, which + // is where these grants were being read. + await this.services.permission.bumpPermissionCacheForUsernames([ + ...usernames, + ]); + } + } catch (e) { + // A sweep that fails leaves consent live for an app the user can no + // longer see, so it is worth a human looking today — but nobody + // needs waking, since the callers that cannot tolerate it fail the + // request outright. + this.clients.alarm.create( + `app_data_grant_withdrawal_failed:${targetAppUid}`, + 'Failed to withdraw cross-app data grants', + { targetAppUid, reason, error: e as Error }, + 'warning', + ); + throw e; + } } /** diff --git a/src/backend/services/auth/AuthService.ts b/src/backend/services/auth/AuthService.ts index 95d2a8a0e..35fdc34bc 100644 --- a/src/backend/services/auth/AuthService.ts +++ b/src/backend/services/auth/AuthService.ts @@ -18,7 +18,7 @@ */ import { v4 as uuidv4, v5 as uuidv5 } from 'uuid'; -import type { Actor } from '../../core/actor'; +import { makeActor, type Actor } from '../../core/actor'; import { HttpError } from '../../core/http/HttpError.js'; import { ASSET_WINDOW_SECONDS, @@ -1802,7 +1802,7 @@ export class AuthService extends PuterService { } return { - actor: { + actor: makeActor({ user: this.#actorUserFromRow(user), accessToken: { uid: decoded.token_uid, @@ -1815,7 +1815,7 @@ export class AuthService extends PuterService { fullAccess: !decoded.app_uid && decoded.full_access === true, }, - }, + }), }; } @@ -1842,12 +1842,12 @@ export class AuthService extends PuterService { } #buildUserActor(user: UserRow, session: SessionRow | null): Actor { - return { + return makeActor({ user: this.#actorUserFromRow(user), session: session ? { uid: session.uuid, kind: session.kind ?? null } : null, - }; + }); } #buildAppUnderUserActor( @@ -1855,7 +1855,7 @@ export class AuthService extends PuterService { app: { uid: string; id: number }, session: SessionRow | null, ): Actor { - return { + return makeActor({ user: this.#actorUserFromRow(user), app: { uid: app.uid, @@ -1864,6 +1864,6 @@ export class AuthService extends PuterService { session: session ? { uid: session.uuid, kind: session.kind ?? null } : null, - }; + }); } } diff --git a/src/backend/services/fs/FSService.test.ts b/src/backend/services/fs/FSService.test.ts index 4c4f0d9a4..c47e64346 100644 --- a/src/backend/services/fs/FSService.test.ts +++ b/src/backend/services/fs/FSService.test.ts @@ -19,9 +19,19 @@ import { Readable } from 'node:stream'; import { v4 as uuidv4 } from 'uuid'; -import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; -import type { Actor } from '../../core/actor.js'; +import { + afterAll, + beforeAll, + beforeEach, + describe, + expect, + it, + vi, +} from 'vitest'; +import { makeActor, type Actor } from '../../core/actor.js'; +import { runWithContext } from '../../core/context.js'; import { HttpError } from '../../core/http/HttpError.js'; +import { appDataPermission } from '../permission/appDataScopes.js'; import { PuterServer } from '../../server.js'; import type { FSEntry } from '../../stores/fs/FSEntry.js'; import { toPendingUploadSessionKey } from '../../stores/fs/pendingUploadSessionHelpers.js'; @@ -2686,7 +2696,7 @@ describe('FSService permission rules', () => { `${user.home}/Documents/outside.json`, '{}', ); - const appActor: Actor = { user: user.actor.user, app: { uid: appUid } }; + const appActor = makeActor({ user: user.actor.user, app: { uid: appUid } }); await expect( server.services.permission.check( @@ -2731,3 +2741,240 @@ describe('FSService permission rules', () => { expect(higher).not.toContain(`fs:${file.uuid}:read`); }); }); + +// -- Cross-app AppData (app-data::fs:) ---------------------- + +describe('FSService — cross-app AppData access', () => { + let owner: TestUser; + let calendar: { id: number; uid: string }; + let contacts: { id: number; uid: string }; + let calendarActor: Actor; + let contactsFile: FSEntry; + let contactsRoot: FSEntry; + + const makeRealApp = async ( + ownerUserId: number, + fields: Record = {}, + ): Promise<{ id: number; uid: string }> => { + const name = `fsx-${uuidv4()}`; + return (await server.stores.app.create( + { + name, + title: 'FS cross-app test', + index_url: `https://${name}.test/`, + ...fields, + }, + { ownerUserId }, + )) as { id: number; uid: string }; + }; + + const grant = (permission: string) => + runWithContext({ actor: owner.actor }, () => + server.services.permission.grantUserAppPermission( + owner.actor, + calendar.uid, + permission, + ), + ); + + const asCalendar = (fn: () => T | Promise) => + runWithContext({ actor: calendarActor }, fn); + + /** An AppData subtree with one file in it, as opening the app would leave. */ + const seedAppData = async ( + appUid: string, + name = 'state.json', + ): Promise => { + await fs.mkdir(owner.userId, { + path: `${owner.home}/AppData/${appUid}`, + createMissingParents: true, + }); + return writeFile( + owner, + `${owner.home}/AppData/${appUid}/${name}`, + '{}', + ); + }; + + beforeEach(async () => { + owner = await makeUser(); + calendar = await makeRealApp(owner.userId); + contacts = await makeRealApp(owner.userId); + calendarActor = makeActor({ + user: owner.actor.user, + app: { uid: calendar.uid, id: calendar.id }, + }); + contactsRoot = await fs.mkdir(owner.userId, { + path: `${owner.home}/AppData/${contacts.uid}`, + createMissingParents: true, + }); + contactsFile = await writeFile( + owner, + `${owner.home}/AppData/${contacts.uid}/state.json`, + '{"a":1}', + ); + }); + + it('gives no access without a grant', async () => { + await expect( + server.services.permission.check( + calendarActor, + `fs:${contactsFile.uuid}:read`, + ), + ).resolves.toBe(false); + }); + + it('reads another app’s AppData with the read class', async () => { + await grant(appDataPermission(contacts.uid, 'fs', 'read')); + await expect( + server.services.permission.check( + calendarActor, + `fs:${contactsFile.uuid}:read`, + ), + ).resolves.toBe(true); + // Read does not carry write. + await expect( + server.services.permission.check( + calendarActor, + `fs:${contactsFile.uuid}:write`, + ), + ).resolves.toBe(false); + }); + + it('covers the subtree root and its descendants', async () => { + await grant(appDataPermission(contacts.uid, 'fs', 'read')); + for (const uuid of [contactsRoot.uuid, contactsFile.uuid]) { + await expect( + server.services.permission.check( + calendarActor, + `fs:${uuid}:read`, + ), + ).resolves.toBe(true); + } + }); + + it('refuses when the target app has opted out of sharing', async () => { + const closed = await makeRealApp(owner.userId, { + metadata: JSON.stringify({ share_app_data: false }), + }); + const closedFile = await seedAppData(closed.uid); + await grant(appDataPermission(closed.uid, 'fs', 'read')); + await expect( + server.services.permission.check( + calendarActor, + `fs:${closedFile.uuid}:read`, + ), + ).resolves.toBe(false); + }); + + it('does not let a grant for one app reach another', async () => { + const third = await makeRealApp(owner.userId); + const thirdFile = await seedAppData(third.uid); + await grant(appDataPermission(contacts.uid, 'fs', 'read')); + await expect( + server.services.permission.check( + calendarActor, + `fs:${thirdFile.uuid}:read`, + ), + ).resolves.toBe(false); + }); + + // -- The delete guard ------------------------------------------------ + + it('refuses delete, move, and rename with only the write class', async () => { + await grant(appDataPermission(contacts.uid, 'fs', 'write')); + // ACL would allow all three: they ask for `fs:write`, which the grant + // satisfies. The guard is what separates them. + await expect( + asCalendar(() => + fs.remove(owner.userId, { entry: contactsFile }), + ), + ).rejects.toMatchObject({ statusCode: 403 }); + await expect( + asCalendar(() => fs.rename(contactsFile, 'renamed.json')), + ).rejects.toMatchObject({ statusCode: 403 }); + + const desktop = (await server.stores.fsEntry.getEntryByPath( + `${owner.home}/Desktop`, + ))!; + await expect( + asCalendar(() => + fs.move(owner.userId, { + source: contactsFile, + destinationParent: desktop as unknown as FSEntry, + }), + ), + ).rejects.toMatchObject({ statusCode: 403 }); + }); + + it('allows delete once the delete class is granted', async () => { + await grant(appDataPermission(contacts.uid, 'fs', 'delete')); + await asCalendar(() => fs.remove(owner.userId, { entry: contactsFile })); + expect( + await server.stores.fsEntry.getEntryByPath(contactsFile.path), + ).toBeFalsy(); + }); + + it('allows rename once the delete class is granted', async () => { + await grant(appDataPermission(contacts.uid, 'fs', 'delete')); + const renamed = await asCalendar(() => + fs.rename(contactsFile, 'renamed.json'), + ); + expect(renamed.name).toBe('renamed.json'); + }); + + it('leaves an app’s own AppData deletable', async () => { + // The guard must only fire on a *foreign* subtree, or every app loses + // the ability to clean up after itself. + const ownFile = await seedAppData(calendar.uid, 'own.json'); + await asCalendar(() => fs.remove(owner.userId, { entry: ownFile })); + expect( + await server.stores.fsEntry.getEntryByPath(ownFile.path), + ).toBeFalsy(); + }); + + it('leaves the owning user unaffected by the guard', async () => { + // No app actor in context at all — the plain user path must not change. + await fs.remove(owner.userId, { entry: contactsFile }); + expect( + await server.stores.fsEntry.getEntryByPath(contactsFile.path), + ).toBeFalsy(); + }); + + it('refuses an access-token actor whose issuer is the granted app', async () => { + // The token carries no `app` of its own, so a guard keyed on `actor.app` + // would skip entirely — failing open where the read/write implicator + // fails closed. + await grant(appDataPermission(contacts.uid, 'fs', 'write')); + const tokenActor = makeActor({ + user: owner.actor.user, + accessToken: { + uid: 'tok-cross-app', + issuer: calendarActor, + fullAccess: false, + }, + }); + + await expect( + runWithContext({ actor: tokenActor }, () => + fs.remove(owner.userId, { entry: contactsFile }), + ), + ).rejects.toMatchObject({ statusCode: 403 }); + }); + + it('lets system-initiated repair through the guard', async () => { + // Ghost-fsentry cleanup runs during an unrelated caller's *read*, so it + // is not that caller's action. Without the opt-out the repair is refused + // and the orphaned row is never reaped. + await grant(appDataPermission(contacts.uid, 'fs', 'read')); + await asCalendar(() => + fs.remove(owner.userId, { + entry: contactsFile, + systemInitiated: true, + }), + ); + expect( + await server.stores.fsEntry.getEntryByPath(contactsFile.path), + ).toBeFalsy(); + }); +}); diff --git a/src/backend/services/fs/FSService.ts b/src/backend/services/fs/FSService.ts index 607e0988d..502c59563 100644 --- a/src/backend/services/fs/FSService.ts +++ b/src/backend/services/fs/FSService.ts @@ -17,16 +17,26 @@ * along with this program. If not, see . */ -import { posix as pathPosix } from 'node:path'; -import { assertNormalized } from './resolveNode.js'; import { createHash } from 'node:crypto'; -import { Readable, Transform } from 'node:stream'; +import { posix as pathPosix } from 'node:path'; import type { TransformCallback } from 'node:stream'; +import { Readable, Transform } from 'node:stream'; import { v4 as uuidv4 } from 'uuid'; -import type { - MultipartCompletePart, - SignedUploadResult, -} from '../../stores/fs/s3Types.js'; +import { + BinaryPayload, + CompleteWriteRequest, + CompleteWriteResponse, + SignedWriteRequest, + SignedWriteResponse, + SignMultipartPartsRequest, + SignMultipartPartsResponse, + UploadMode, + WriteRequest, + WriteResponse, +} from '../../controllers/fs/requestTypes.js'; +import { Actor } from '../../core/actor.js'; +import { Context } from '../../core/context.js'; +import { HttpError } from '../../core/http/HttpError.js'; import { FSEntry, FSEntryCreateInput, @@ -35,18 +45,25 @@ import { PendingUploadCreateInput, PendingUploadSession, } from '../../stores/fs/FSEntry.js'; +import type { + MultipartCompletePart, + SignedUploadResult, +} from '../../stores/fs/s3Types.js'; +import type { puterStores } from '../../stores/index.js'; +import type { LayerInstances } from '../../types.js'; +import { runWithConcurrencyLimitSettled } from '../../util/concurrency.js'; +import { AclMode } from '../acl/ACLService.js'; +import type { puterServices } from '../index.js'; import { - BinaryPayload, - CompleteWriteRequest, - CompleteWriteResponse, - SignMultipartPartsRequest, - SignMultipartPartsResponse, - SignedWriteRequest, - SignedWriteResponse, - UploadMode, - WriteRequest, - WriteResponse, -} from '../../controllers/fs/requestTypes.js'; + APP_DATA_FS_MODE_CLASSES, + appDataPermission, + appDataSharingAllowed, +} from '../permission/appDataScopes.js'; +import { MANAGE_PERM_PREFIX } from '../permission/consts.js'; +import { PermissionUtil } from '../permission/permissionUtil.js'; +import { PuterService } from '../types.js'; +import { FSEntryCacheInvalidationEventHandler } from './cacheInvalidation.js'; +import { assertNormalized } from './resolveNode.js'; import type { BatchWritePrepareRequest, NormalizedWriteInput, @@ -56,23 +73,28 @@ import type { UploadPreparedBatchItemInput, UploadProgressTrackerLike, } from './types.js'; -import { runWithConcurrencyLimitSettled } from '../../util/concurrency.js'; -import { HttpError } from '../../core/http/HttpError.js'; -import { PuterService } from '../types.js'; -import type { LayerInstances } from '../../types.js'; -import type { puterStores } from '../../stores/index.js'; -import type { puterServices } from '../index.js'; -import { FSEntryCacheInvalidationEventHandler } from './cacheInvalidation.js'; -import { MANAGE_PERM_PREFIX } from '../permission/consts.js'; -import { PermissionUtil } from '../permission/permissionUtil.js'; -import { Actor } from '../../core/actor.js'; -import { AclMode } from '../acl/ACLService.js'; const DEFAULT_CONTENT_TYPE = 'application/octet-stream'; const DEFAULT_SIGNED_UPLOAD_EXPIRY_SECONDS = 60 * 15; const RESERVED_METADATA_KEYS: readonly string[] = ['objectKey']; +/** + * The app whose `AppData` subtree `path` sits in, when that app is not + * `ownAppUid` — i.e. the target of a cross-app access. Null for anything else. + */ +const foreignAppDataOwner = ( + path: string, + username: string, + ownAppUid: string, +): string | null => { + const prefix = `/${username}/AppData/`; + if (!path.startsWith(prefix)) return null; + const appUid = path.slice(prefix.length).split('/')[0]; + if (!appUid || appUid === ownAppUid) return null; + return appUid; +}; + const isNoSuchKeyError = (err: unknown): boolean => { if (!err || typeof err !== 'object') return false; const e = err as { name?: unknown; Code?: unknown }; @@ -248,7 +270,28 @@ export class FSService extends PuterService { if (entry.path === root || entry.path.startsWith(`${root}/`)) { return {}; } - return undefined; + + // Another app's AppData, reachable once the user grants it. + // Same entry lookup, so this costs nothing extra. + const targetAppUid = foreignAppDataOwner( + entry.path, + username, + appUid, + ); + if (!targetAppUid) return undefined; + const mode = PermissionUtil.split(stripped)[2]; + const cls = + APP_DATA_FS_MODE_CLASSES[ + mode as keyof typeof APP_DATA_FS_MODE_CLASSES + ]; + if (!cls) return undefined; + const target = await this.stores.app.getByUid(targetAppUid); + if (!target || !appDataSharingAllowed(target)) return undefined; + const granted = await permissions.check( + actor, + appDataPermission(targetAppUid, 'fs', cls), + ); + return granted ? {} : undefined; }, }); @@ -2915,7 +2958,7 @@ export class FSService extends PuterService { objectKey, }); try { - await this.remove(entry.userId, { entry }); + await this.remove(entry.userId, { entry, systemInitiated: true }); } catch (cleanupErr) { console.error( 'prodfsv2 ghost fsentry cleanup failed', @@ -3134,6 +3177,7 @@ export class FSService extends PuterService { legacyCode: 'bad_request', }); if (entry.name === newName) return entry; + await this.#assertCrossAppDeleteAllowed(entry.path); const parentPath = pathPosix.dirname(entry.path); const newPath = @@ -3218,15 +3262,57 @@ export class FSService extends PuterService { * * Does NOT enforce ACL — caller (controller) performs the `write` check. */ + /** + * Delete, move, and rename all ask ACL for `fs:write`, which cannot tell + * them apart from an ordinary write — so the delete class is enforced here + * instead, at the only choke point both FS controllers and the `/batch` + * dispatcher go through. + * + * Reads the actor from context: these methods take a `userId`, not an + * actor, and a caller with no actor (provisioning, internal mkdir, the + * system actor) is unaffected. + */ + async #assertCrossAppDeleteAllowed(path: string): Promise { + const actor = Context.get('actor') as Actor | undefined; + if (!actor) return; + // Through the issuer chain: a token actor has no `app` of its own, so + // keying off `actor.app` would skip the guard — failing open where the + // paired implicator fails closed. + const app = actor.effectiveApp; + if (!app) return; + const username = actor.user?.username; + if (!username) return; + + const targetAppUid = foreignAppDataOwner(path, username, app.uid); + if (!targetAppUid) return; + + const granted = await this.services.permission.check( + actor, + appDataPermission(targetAppUid, 'fs', 'delete'), + ); + if (!granted) { + throw new HttpError(403, 'Forbidden', { legacyCode: 'forbidden' }); + } + } + async remove( userId: number, input: { entry: FSEntry; recursive?: boolean; descendantsOnly?: boolean; + /** + * Set by internal repair (ghost-fsentry cleanup), which runs during + * an unrelated caller's read and is not that caller's action — + * otherwise the guard refuses it and the orphan is never reaped. + */ + systemInitiated?: boolean; }, ): Promise { const { entry } = input; + if (!input.systemInitiated) { + await this.#assertCrossAppDeleteAllowed(entry.path); + } if (entry.userId !== userId) { // Defensive — only the owner should be hitting this path; higher // layers grant access via ACL, not raw ownership, but we still @@ -3459,6 +3545,9 @@ export class FSService extends PuterService { }, ): Promise { const { source, destinationParent } = input; + // The source only: moving *into* another app's AppData is a write, and + // ACL plus the fs:write class already cover that. + await this.#assertCrossAppDeleteAllowed(source.path); if (source.userId !== userId) { throw new HttpError( 403, diff --git a/src/backend/services/localworker/LocalWorkerService.ts b/src/backend/services/localworker/LocalWorkerService.ts index c790202f3..dfbeea744 100644 --- a/src/backend/services/localworker/LocalWorkerService.ts +++ b/src/backend/services/localworker/LocalWorkerService.ts @@ -1,6 +1,6 @@ import { Miniflare, RequestInit as MiniflareRequestInit } from 'miniflare'; import { puterServices } from '..'; -import { Actor } from '../../core'; +import { makeActor } from '../../core'; import { loadFileInput } from '../../drivers/util/fileInput'; import { getWorkerPreamble } from '../../drivers/workers/WorkerDriver'; import { puterStores } from '../../stores'; @@ -53,9 +53,9 @@ export class LocalWorkerService extends PuterService { } /** - * Local analogue of the production worker URL, matching the host the - * local worker proxy dispatches on (`.workers.puter.localhost`). - * Clients rely on `create` returning a usable `url`. + * Local analogue of the production worker URL, matching the host the local + * worker proxy dispatches on (`.workers.puter.localhost`). Clients + * rely on `create` returning a usable `url`. */ #localWorkerUrl(workerName: string): string { const port = this.config.port ? `:${this.config.port}` : ''; @@ -159,7 +159,7 @@ export class LocalWorkerService extends PuterService { let authorization: string; const ownerUser = await this.stores.user.getById(row.user_id); if (!ownerUser) throw new Error('Owner seems to not exist'); - const ownerActor = { user: ownerUser } as Actor; + const ownerActor = makeActor({ user: ownerUser }); if (appOwnerId) { const app = await this.stores.app.getById(appOwnerId); diff --git a/src/backend/services/permission/PermissionService.ts b/src/backend/services/permission/PermissionService.ts index c6c0ae7ec..30ba34e1e 100644 --- a/src/backend/services/permission/PermissionService.ts +++ b/src/backend/services/permission/PermissionService.ts @@ -1058,6 +1058,23 @@ export class PermissionService extends PuterService { } } + /** + * What `grantUserAppPermission` checks before it writes: the rewrite that + * decides what the row stores, and the width of the column it lands in. + * + * Exposed so a caller granting several at once can reject the whole set + * before committing any of it — a half-written set reads to the caller as a + * refusal while some access is live. + */ + async assertUserAppPermissionWritable(permission: string): Promise { + const rewritten = await this.#rewriteForUserAppWrite(permission); + if (rewritten.length > PERMISSION_MAX_LEN) { + throw new HttpError(400, 'Invalid `permission`', { + legacyCode: 'bad_request', + }); + } + } + async grantUserAppPermission( actor: Actor, appIdentifier: string, @@ -1574,7 +1591,7 @@ export class PermissionService extends PuterService { username: user.username, email: user.email ?? null, }; - return { user: actorUser }; + return { user: actorUser, effectiveApp: null }; } } diff --git a/src/backend/services/permission/appDataScopes.ts b/src/backend/services/permission/appDataScopes.ts new file mode 100644 index 000000000..066bac4aa --- /dev/null +++ b/src/backend/services/permission/appDataScopes.ts @@ -0,0 +1,167 @@ +/* + * Copyright (C) 2024-present Puter Technologies Inc. + * + * This file is part of Puter. + * + * Puter is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { PermissionUtil } from './permissionUtil'; + +/** Root of the cross-app application-data permission namespace. */ +export const APP_DATA_PERMISSION_PREFIX = 'app-data'; +export type AppDataStore = 'kv' | 'fs'; + +/** + * Access classes a grant may be written at. Coarser than a concrete op, so a + * single `…:kv:read` row covers `get` and `list`. + * + * `delete` is orthogonal: `write` does not imply it and it does not imply + * `write`. That is what lets a grant say "may add invites but not remove them", + * and conversely "may cancel an invite" without handing over the ability to + * rewrite everything. Coarser grants (`app-data:X:kv`, `app-data:X`) still + * cover all three by prefix implication, which is why the consent dialog has to + * name deletion whenever it prompts for one. + */ +export const APP_DATA_CLASSES = ['read', 'write', 'delete'] as const; +export type AppDataClass = (typeof APP_DATA_CLASSES)[number]; + +/** KV operations another app may be granted. */ +export const APP_DATA_KV_OPS = [ + 'get', + 'list', + 'set', + 'add', + 'incr', + 'decr', + 'update', + 'del', + 'remove', + 'expire', + 'expireAt', +] as const; +export type AppDataKvOp = (typeof APP_DATA_KV_OPS)[number]; + +/** + * Parameters that turn a write into a deletion: both set an expiry, and an + * expiry in the past makes the key vanish (the store filters it out on read and + * DynamoDB reaps it later). A cross-app call carrying either one therefore + * needs the `delete` class on top of `write` — otherwise `kv:set` alone would + * be a delete capability under another name. + */ +export const APP_DATA_KV_TTL_PARAMS = ['expireAt', 'ttl'] as const; + +/** Classes that satisfy a concrete op. Drives the exploder. */ +export const APP_DATA_KV_OP_CLASSES: Record< + AppDataKvOp, + readonly AppDataClass[] +> = { + get: ['read', 'write'], + list: ['read', 'write'], + set: ['write'], + add: ['write'], + incr: ['write'], + decr: ['write'], + update: ['write'], + del: ['delete'], + remove: ['delete'], + expire: ['delete'], + expireAt: ['delete'], +}; + +/** + * ACL fs mode → the class that covers it. + * + * `delete` is not an ACL mode: delete, move, and rename all ask ACL for + * `write`, so it cannot tell them apart. That class is supplied by the + * destructive guard on `remove`/`move`/`rename` instead. + */ +export const APP_DATA_FS_MODE_CLASSES = { + see: 'read', + list: 'read', + read: 'read', + write: 'write', + delete: 'delete', +} as const; + +/** + * Driver method → the op it is checked as, or `null` for methods that must + * never reach another app's namespace. Every public method on KVStoreDriver + * appears here; a method missing from this map fails closed at the call site. + */ +export const APP_DATA_KV_METHOD_OPS: Record = { + get: 'get', + list: 'list', + set: 'set', + batchPut: 'set', + add: 'add', + incr: 'incr', + decr: 'decr', + update: 'update', + del: 'del', + remove: 'remove', + expire: 'expire', + expireAt: 'expireAt', + flush: null, +}; + +/** + * Builds a permission string, omitting trailing components so callers can name + * a whole store (`app-data::kv`) or the whole app (`app-data:`). + */ +export const appDataPermission = ( + targetAppUid: string, + store?: AppDataStore, + op?: string, +): string => + PermissionUtil.join( + ...[APP_DATA_PERMISSION_PREFIX, targetAppUid, store, op].filter( + (part): part is string => Boolean(part), + ), + ); + +/** + * Parse a cross-app data permission, or `null` if it isn't one. + * + * Rejects a bare `app-data` (no target): prefix implication would make that one + * row cover every app the user has, which no consent prompt can describe. + */ +export const parseAppDataPermission = ( + permission: string, +): { targetAppUid: string; store?: string; op?: string } | null => { + if (!permission.startsWith(`${APP_DATA_PERMISSION_PREFIX}:`)) return null; + const [, targetAppUid, store, op] = PermissionUtil.split(permission); + if (!targetAppUid) return null; + return { targetAppUid, store, op }; +}; + +/** + * Whether `app` lets other apps reach its per-user data. + * + * Opt-**out**: only an explicit `share_app_data: false` closes it, so an app + * with no metadata — which is every app written before this existed — stays + * shareable. An app that keeps third-party tokens or entitlements in its KV + * namespace or AppData directory sets the flag to exclude itself; user consent + * alone is otherwise the gate. + * + * `AppStore` parses the `metadata` column into an object, or `null` when the + * stored JSON is malformed. Absent, `null`, and non-object metadata all read as + * allowed: this decides whether a feature is available, not whether access is + * authorized, so it must not fail closed on a row it cannot interpret. + */ +export const appDataSharingAllowed = (app: { metadata?: unknown }): boolean => { + const metadata = app.metadata; + if (!metadata || typeof metadata !== 'object') return true; + return (metadata as { share_app_data?: unknown }).share_app_data !== false; +}; diff --git a/src/backend/stores/permission/PermissionStore.test.ts b/src/backend/stores/permission/PermissionStore.test.ts index 77c38b8d4..a9fe1d158 100644 --- a/src/backend/stores/permission/PermissionStore.test.ts +++ b/src/backend/stores/permission/PermissionStore.test.ts @@ -17,6 +17,7 @@ * along with this program. If not, see . */ +import { readFileSync } from 'fs'; import { v4 as uuidv4 } from 'uuid'; import { afterAll, beforeAll, describe, expect, it } from 'vitest'; import { PuterServer } from '../../server.ts'; @@ -203,6 +204,121 @@ describe('PermissionStore', () => { }); }); + // -- prefix deletion ----------------------------------------------- + + describe('deleteAppGrantsByPermissionPrefix', () => { + it('removes the exact permission and its subtree, across both tables', async () => { + const user = await makeUser(); + const app = await makeApp(user.id); + + await store.upsertUserAppPerm(user.id, app.id, 'app-data:x', {}); + await store.upsertUserAppPerm( + user.id, + app.id, + 'app-data:x:kv:get', + {}, + ); + await store.upsertUserAppPerm(user.id, app.id, 'app-data:y', {}); + await store.upsertDevAppPerm(user.id, app.id, 'app-data:x:fs', {}); + + const removed = + await store.deleteAppGrantsByPermissionPrefix('app-data:x'); + + expect(removed.map((r) => r.permission).sort()).toEqual([ + 'app-data:x', + 'app-data:x:fs', + 'app-data:x:kv:get', + ]); + expect( + await store.hasUserAppPerm(user.id, app.id, 'app-data:y'), + ).toBe(true); + }); + + it('treats LIKE wildcards in the permission as literal text', async () => { + const user = await makeUser(); + const app = await makeApp(user.id); + + // `_` matches any single character unescaped, so an unescaped + // pattern for `app-data:a_c` would also delete `app-data:abc`. + await store.upsertUserAppPerm( + user.id, + app.id, + 'app-data:a_c:kv', + {}, + ); + await store.upsertUserAppPerm( + user.id, + app.id, + 'app-data:abc:kv', + {}, + ); + await store.upsertUserAppPerm(user.id, app.id, 'app-data:100%', {}); + await store.upsertUserAppPerm( + user.id, + app.id, + 'app-data:100pct', + {}, + ); + + const removed = + await store.deleteAppGrantsByPermissionPrefix('app-data:a_c'); + expect(removed.map((r) => r.permission)).toEqual([ + 'app-data:a_c:kv', + ]); + expect( + await store.hasUserAppPerm(user.id, app.id, 'app-data:abc:kv'), + ).toBe(true); + + await store.deleteAppGrantsByPermissionPrefix('app-data:100%'); + expect( + await store.hasUserAppPerm(user.id, app.id, 'app-data:100pct'), + ).toBe(true); + }); + + it('escapes the escape character itself', async () => { + const user = await makeUser(); + const app = await makeApp(user.id); + + await store.upsertUserAppPerm(user.id, app.id, 'app-data:a!b', {}); + await store.upsertUserAppPerm( + user.id, + app.id, + 'app-data:a!b:kv', + {}, + ); + + const removed = + await store.deleteAppGrantsByPermissionPrefix('app-data:a!b'); + expect(removed.map((r) => r.permission).sort()).toEqual([ + 'app-data:a!b', + 'app-data:a!b:kv', + ]); + }); + + it('does not use a backslash as the LIKE escape character', () => { + // MySQL processes backslash escapes inside string literals, so the + // `'\'` that a JS `'\\'` produces reads as an escaped quote and + // leaves the literal unterminated. That is a parse error on MySQL + // only — no SQLite or Postgres run, including this suite, would + // ever surface it, so the SQL text itself is what gets pinned. + const source = readFileSync( + new URL('./PermissionStore.ts', import.meta.url), + 'utf8', + ); + const escapeClauses = source + .split('\n') + .filter( + (line) => + line.includes('LIKE ?') && line.includes('ESCAPE'), + ) + .map((line) => line.trim()); + expect(escapeClauses.length).toBeGreaterThan(0); + for (const clause of escapeClauses) { + expect(clause).toContain("ESCAPE '!'"); + } + }); + }); + // -- dev → app ----------------------------------------------------- describe('dev-to-app permissions', () => { diff --git a/src/backend/stores/permission/PermissionStore.ts b/src/backend/stores/permission/PermissionStore.ts index d23e88bb5..2f7326204 100644 --- a/src/backend/stores/permission/PermissionStore.ts +++ b/src/backend/stores/permission/PermissionStore.ts @@ -317,6 +317,80 @@ export class PermissionStore extends PuterStore { }); } + /** + * Delete every app grant whose permission is exactly `permission` or sits + * beneath it, across both the user-to-app and dev-to-app tables, returning + * the rows removed so the caller can audit them and bust their caches. + * + * Kept generic — the caller owns what the prefix means. Used to withdraw + * cross-app data grants when their _target_ app goes away: the target lives + * in the permission text rather than a column, so no foreign key can + * cascade it. `permission` has no index, making this a table scan — + * acceptable on app deletion, never on a hot path. + */ + async deleteAppGrantsByPermissionPrefix(permission: string): Promise< + Array<{ + table: 'user_to_app_permissions' | 'dev_to_app_permissions'; + user_id: number; + app_id: number; + permission: string; + }> + > { + // `_` and `%` are LIKE wildcards, so an unescaped one would widen the + // match beyond the intended subtree. + // + // `!` as the escape character, matching FSEntryStore: a backslash one + // would have to be written `ESCAPE '\\'` in the SQL text, and MySQL + // processes backslash escapes inside string literals, so the `'\'` a + // JS `'\\'` produces reads as an escaped quote and leaves the literal + // unterminated. SQLite and Postgres accept it, which is why only MySQL + // would have seen the parse error. + const escaped = permission.replace(/([!%_])/g, '!$1'); + const exact = permission; + const prefix = `${escaped}:%`; + + const removed: Array<{ + table: 'user_to_app_permissions' | 'dev_to_app_permissions'; + user_id: number; + app_id: number; + permission: string; + }> = []; + + for (const table of [ + 'user_to_app_permissions', + 'dev_to_app_permissions', + ] as const) { + const rows = (await this.clients.db.read( + `SELECT \`user_id\`, \`app_id\`, \`permission\` FROM \`${table}\` ` + + "WHERE `permission` = ? OR `permission` LIKE ? ESCAPE '!'", + [exact, prefix], + )) as Array<{ + user_id: number; + app_id: number; + permission: string; + }>; + if (rows.length === 0) continue; + + await this.clients.db.write( + `DELETE FROM \`${table}\` ` + + "WHERE `permission` = ? OR `permission` LIKE ? ESCAPE '!'", + [exact, prefix], + ); + for (const row of rows) removed.push({ table, ...row }); + } + + const keys = [ + ...new Set( + removed + .filter((r) => r.table === 'user_to_app_permissions') + .map((r) => this.#u2aCacheKey(r.user_id, r.app_id)), + ), + ]; + if (keys.length > 0) await this.publishCacheKeys({ keys }); + + return removed; + } + async auditUserAppPerm( entry: AuditEntry & { user_id: number; diff --git a/src/backend/stores/systemKv/SystemKVStore.test.ts b/src/backend/stores/systemKv/SystemKVStore.test.ts index 876684103..a1eca3971 100644 --- a/src/backend/stores/systemKv/SystemKVStore.test.ts +++ b/src/backend/stores/systemKv/SystemKVStore.test.ts @@ -6,6 +6,7 @@ import { describe, expect, it, + vi, } from 'vitest'; import { setupTestServer } from '../../testUtil.ts'; import type { SystemKVStore } from './SystemKVStore.ts'; @@ -255,9 +256,9 @@ describe('SystemKVStore', () => { }); it('rejects a non-positive limit', async () => { - await expect( - target.list({ limit: 0 }, opts), - ).rejects.toMatchObject({ statusCode: 400 }); + await expect(target.list({ limit: 0 }, opts)).rejects.toMatchObject( + { statusCode: 400 }, + ); }); it('rejects a malformed cursor', async () => { @@ -280,7 +281,8 @@ describe('SystemKVStore', () => { }); it('skips ahead with offset', async () => { - const all = (await target.list({ as: 'keys' }, opts)).res as string[]; + const all = (await target.list({ as: 'keys' }, opts)) + .res as string[]; const result = await target.list( { as: 'keys', offset: 1, limit: 5 }, opts, @@ -385,10 +387,7 @@ describe('SystemKVStore', () => { let cursor: string | undefined; do { const page = ( - await target.list( - { as: 'keys', limit: 1, cursor }, - opts, - ) + await target.list({ as: 'keys', limit: 1, cursor }, opts) ).res as { items: string[]; cursor?: string }; keys.push(...page.items); cursor = page.cursor; @@ -500,7 +499,11 @@ describe('SystemKVStore', () => { // First bump creates the counter and stamps the (already-elapsed) // ttl in the same write — no separate expireAt call. await target.incr( - { key: 'ttlCounter', pathAndAmountMap: { hits: 1 }, expireAt: past }, + { + key: 'ttlCounter', + pathAndAmountMap: { hits: 1 }, + expireAt: past, + }, opts, ); const result = await target.get({ key: 'ttlCounter' }, opts); @@ -510,14 +513,22 @@ describe('SystemKVStore', () => { it('keeps the first expireAt stamp across later bumps (if_not_exists)', async () => { const future = Math.floor(Date.now() / 1000) + 3600; await target.incr( - { key: 'ttlKeep', pathAndAmountMap: { hits: 1 }, expireAt: future }, + { + key: 'ttlKeep', + pathAndAmountMap: { hits: 1 }, + expireAt: future, + }, opts, ); // A later bump passing an already-elapsed ttl must NOT override the // first stamp, so the counter stays visible. const past = Math.floor(Date.now() / 1000) - 10; await target.incr( - { key: 'ttlKeep', pathAndAmountMap: { hits: 1 }, expireAt: past }, + { + key: 'ttlKeep', + pathAndAmountMap: { hits: 1 }, + expireAt: past, + }, opts, ); const result = await target.get({ key: 'ttlKeep' }, opts); @@ -726,9 +737,9 @@ describe('SystemKVStore', () => { afterEach(() => { // Nothing a caller sends may end up on the shared prototype. - expect( - Object.getOwnPropertyNames(Object.prototype), - ).not.toContain('polluted'); + expect(Object.getOwnPropertyNames(Object.prototype)).not.toContain( + 'polluted', + ); expect(({} as Record).polluted).toBeUndefined(); }); @@ -848,7 +859,10 @@ describe('SystemKVStore', () => { it('rejects an unsafe key in a batchPut item', async () => { const value = JSON.parse('{"__proto__":{"polluted":true}}'); await expect( - target.batchPut({ items: [{ key: 'proto-batch', value }] }, opts), + target.batchPut( + { items: [{ key: 'proto-batch', value }] }, + opts, + ), ).rejects.toMatchObject({ statusCode: 400 }); }); @@ -882,4 +896,71 @@ describe('SystemKVStore', () => { expect(getRes.usage.write).toBe(0); }); }); + + // -- Cross-app privacy probe --------------------------------------- + // + // Reached only through `namespaceAppUuid`, which the KV driver sets after + // its permission check. Asserted at the store so no permission machinery + // (which reads flat perms through KV) is in the measurement. + describe('cross-app mutations', () => { + let crossOpts: { actor: Actor; namespaceAppUuid: string }; + beforeEach(() => { + crossOpts = { actor, namespaceAppUuid: 'app-other' }; + }); + + it('probes a whole batch in one read rather than one per key', async () => { + const batchGet = vi.spyOn(server.clients.dynamo, 'batchGet'); + const get = vi.spyOn(server.clients.dynamo, 'get'); + try { + await target.batchPut( + { + items: [ + { key: 'b1', value: 1 }, + { key: 'b2', value: 2 }, + { key: 'b3', value: 3 }, + ], + }, + crossOpts, + ); + // A per-key probe would make this N single-key gets. + expect(batchGet).toHaveBeenCalledTimes(1); + expect(get).not.toHaveBeenCalled(); + } finally { + batchGet.mockRestore(); + get.mockRestore(); + } + }); + + it('bills the probe as a read', async () => { + // The probe is a real round trip; metering runs off the usage the + // store reports, so swallowing it under-bills the caller. + const { usage } = await target.set( + { key: 'metered', value: 'v' }, + crossOpts, + ); + expect(usage.read).toBeGreaterThan(0); + expect(usage.write).toBeGreaterThan(0); + }); + + it('still refuses a private entry through the batch probe', async () => { + // Into the *target* namespace: a plain user actor may address any of + // its own app namespaces via `appUuid`, which is how the owning app's + // data is seeded here. + await target.set( + { key: 'secret', value: 's', disableSharing: true }, + { actor, appUuid: 'app-other' }, + ); + await expect( + target.batchPut( + { + items: [ + { key: 'ok', value: 1 }, + { key: 'secret', value: 2 }, + ], + }, + crossOpts, + ), + ).rejects.toMatchObject({ statusCode: 403 }); + }); + }); }); diff --git a/src/backend/stores/systemKv/SystemKVStore.ts b/src/backend/stores/systemKv/SystemKVStore.ts index be65acd2f..f3f986906 100644 --- a/src/backend/stores/systemKv/SystemKVStore.ts +++ b/src/backend/stores/systemKv/SystemKVStore.ts @@ -55,6 +55,12 @@ export interface KVOpts { actor?: Actor; /** Optional app uuid override for non-app-scoped actors. */ appUuid?: string; + /** + * Namespace override that wins over the actor's own app — how an app + * addresses a _different_ app's namespace. Set only by the KV driver, and + * only after its cross-app permission check has passed. + */ + namespaceAppUuid?: string; } export interface RecursiveRecord { @@ -74,12 +80,33 @@ const PATH_CLEANER_REGEX = /[^A-Za-z0-9_]/g; const MAX_LIST_OFFSET = 5000; const MAX_FILL_PAGES = 10; +/** + * Marks an entry private to the app that wrote it. Beside `value`/`ttl`, so + * caller data can neither collide with it nor set it. + */ +export const KV_PRIVATE_ATTR = 'noShare'; + const ttlFilter = (now: number) => ({ expression: 'attribute_not_exists(#ttlAttr) OR #ttlAttr > :nowTs', names: { '#ttlAttr': 'ttl' }, values: { ':nowTs': now }, }); +/** + * Expired entries, plus private ones for a cross-app caller. Filtered in the + * query rather than afterwards so `includeTotal`'s COUNT excludes them too — a + * total counting rows the caller can't see would leak what the flag hides. + */ +const listFilter = (now: number, crossApp: boolean) => { + const ttl = ttlFilter(now); + if (!crossApp) return ttl; + return { + expression: `(${ttl.expression}) AND attribute_not_exists(#privAttr)`, + names: { ...ttl.names, '#privAttr': KV_PRIVATE_ATTR }, + values: { ...ttl.values }, + }; +}; + const emptyUsage = (): KVUsage => ({ read: 0, write: 0 }); const readUsage = (units: number | undefined): KVUsage => ({ @@ -99,9 +126,19 @@ const addUsage = (a: KVUsage, b: KVUsage): KVUsage => ({ const ensureActor = (opts?: KVOpts): Actor => opts?.actor ?? SYSTEM_ACTOR; -const getNamespace = (actor: Actor, appUuidOverride?: string): string => { +const isCrossApp = (opts?: KVOpts): boolean => Boolean(opts?.namespaceAppUuid); + +// `effectiveApp`, not `app`, so the namespace agrees with the gate the driver +// applied: both read an access-token actor as the app that issued it, rather +// than the driver treating it as app-scoped while the store files its data +// under the shared global key. +const getNamespace = (actor: Actor, opts?: KVOpts): string => { if (isSystemActor(actor)) return SYSTEM_NAMESPACE; - const appUuid = actor.app?.uid ?? appUuidOverride ?? GLOBAL_APP_KEY; + const appUuid = + opts?.namespaceAppUuid ?? + actor.effectiveApp?.uid ?? + opts?.appUuid ?? + GLOBAL_APP_KEY; return `v1:${actor.user.uuid}:${appUuid}`; }; @@ -266,6 +303,59 @@ export class SystemKVStore extends PuterStore { // -- Public API --------------------------------------------------- + /** + * Refuse a cross-app mutation against a private entry. Not atomic with the + * write that follows, so one in-flight write can still land on an entry the + * owner flags in between. + */ + async #assertNotPrivate( + namespace: string, + key: string, + opts?: KVOpts, + ): Promise { + if (!isCrossApp(opts)) return emptyUsage(); + const response = await this.clients.dynamo.get(this.tableName, { + namespace, + key, + }); + if (response.Item?.[KV_PRIVATE_ATTR]) { + throw new HttpError( + 403, + 'kv: this entry is private to the app that wrote it', + { legacyCode: 'forbidden' }, + ); + } + // Returned rather than swallowed: the probe is a real read, and a caller + // that did not pay for it is under-billed for the operation. + return readUsage( + response.ConsumedCapacity?.CapacityUnits as number | undefined, + ); + } + + /** + * The batch form: one batched read for every key rather than a round trip + * each, which would turn a single batched write into N+1 calls. + */ + async #assertNonePrivate( + namespace: string, + keys: string[], + opts?: KVOpts, + ): Promise { + if (!isCrossApp(opts) || keys.length === 0) return emptyUsage(); + const { entries, usage } = await this.getBatches(namespace, keys); + const isPrivate = (entries as Array<{ noShare?: boolean }>).some( + (entry) => entry?.noShare, + ); + if (isPrivate) { + throw new HttpError( + 403, + 'kv: this entry is private to the app that wrote it', + { legacyCode: 'forbidden' }, + ); + } + return usage; + } + async get( { key, @@ -274,14 +364,19 @@ export class SystemKVStore extends PuterStore { opts?: KVOpts, ): Promise> { const actor = ensureActor(opts); - const namespace = getNamespace(actor, opts?.appUuid); + const namespace = getNamespace(actor, opts); + const crossApp = isCrossApp(opts); const multi = Array.isArray(key); const keys = multi ? key : [key]; for (const k of keys) assertKey(k); - let kvEntries: Array<{ key: string; value?: unknown; ttl?: number }> = - []; + let kvEntries: Array<{ + key: string; + value?: unknown; + ttl?: number; + noShare?: boolean; + }> = []; let usage = emptyUsage(); if (multi) { @@ -310,6 +405,8 @@ export class SystemKVStore extends PuterStore { const entry = kvEntries.find((e) => e.key === k); if (!entry) return null; if (entry.ttl && entry.ttl <= now) return null; + // Absent rather than refused: the flag must not confirm the key. + if (crossApp && entry.noShare) return null; return entry.value ?? null; }); @@ -321,25 +418,41 @@ export class SystemKVStore extends PuterStore { key, value, expireAt, - }: { key: string; value: unknown; expireAt?: number }, + disableSharing, + }: { + key: string; + value: unknown; + expireAt?: number; + /** + * Mark the entry private. `put` replaces the item, so omitting it + * on a later write is how the owner re-shares the entry. + */ + disableSharing?: boolean; + }, opts?: KVOpts, ): Promise> { assertKey(key); assertValue(value); const actor = ensureActor(opts); - const namespace = getNamespace(actor, opts?.appUuid); + const namespace = getNamespace(actor, opts); + const probeUsage = await this.#assertNotPrivate(namespace, key, opts); const response = await this.clients.dynamo.put(this.tableName, { namespace, key, value, ttl: expireAt, + ...(disableSharing ? { [KV_PRIVATE_ATTR]: true } : {}), }); return { res: true, - usage: writeUsage( - response.ConsumedCapacity?.CapacityUnits as number | undefined, + usage: addUsage( + probeUsage, + writeUsage( + response.ConsumedCapacity?.CapacityUnits as + number | undefined, + ), ), }; } @@ -347,7 +460,16 @@ export class SystemKVStore extends PuterStore { async batchPut( { items, - }: { items: Array<{ key: string; value: unknown; expireAt?: number }> }, + disableSharing, + }: { + items: Array<{ key: string; value: unknown; expireAt?: number }>; + /** + * Marks every entry in the batch private, as `set` does for one. + * Batch-wide rather than per-item: it arrives on the same trailing + * options object the single form takes. + */ + disableSharing?: boolean; + }, opts?: KVOpts, ): Promise> { if (!Array.isArray(items) || items.length === 0) { @@ -370,7 +492,14 @@ export class SystemKVStore extends PuterStore { } const actor = ensureActor(opts); - const namespace = getNamespace(actor, opts?.appUuid); + const namespace = getNamespace(actor, opts); + + // One private key refuses the batch — no partial success to probe with. + const probeUsage = await this.#assertNonePrivate( + namespace, + [...byKey.keys()], + opts, + ); const putParams = Array.from(byKey.values()).map((item) => ({ table: this.tableName, @@ -379,6 +508,7 @@ export class SystemKVStore extends PuterStore { key: item.key, value: item.value, ttl: item.expireAt, + ...(disableSharing ? { [KV_PRIVATE_ATTR]: true } : {}), }, })); @@ -389,7 +519,10 @@ export class SystemKVStore extends PuterStore { 0, ) ?? byKey.size; - return { res: true, usage: writeUsage(units || byKey.size) }; + return { + res: true, + usage: addUsage(probeUsage, writeUsage(units || byKey.size)), + }; } async del( @@ -397,7 +530,8 @@ export class SystemKVStore extends PuterStore { opts?: KVOpts, ): Promise> { const actor = ensureActor(opts); - const namespace = getNamespace(actor, opts?.appUuid); + const namespace = getNamespace(actor, opts); + const probeUsage = await this.#assertNotPrivate(namespace, key, opts); const response = await this.clients.dynamo.del(this.tableName, { namespace, @@ -405,10 +539,12 @@ export class SystemKVStore extends PuterStore { }); return { res: true, - usage: writeUsage( - (response.ConsumedCapacity?.CapacityUnits as - | number - | undefined) ?? 1, + usage: addUsage( + probeUsage, + writeUsage( + (response.ConsumedCapacity?.CapacityUnits as + number | undefined) ?? 1, + ), ), }; } @@ -439,16 +575,14 @@ export class SystemKVStore extends PuterStore { | { key: string; value: unknown }[] | { items: - | string[] - | unknown[] - | { key: string; value: unknown }[]; + string[] | unknown[] | { key: string; value: unknown }[]; cursor?: string; total?: number; } > > { const actor = ensureActor(opts); - const namespace = getNamespace(actor, opts?.appUuid); + const namespace = getNamespace(actor, opts); const normalizedLimit = normalizeLimit(limit, { label: 'kv: limit' }); const normalizedOffset = normalizeOffset(offset, { @@ -512,7 +646,7 @@ export class SystemKVStore extends PuterStore { }, } : {}), - filter: ttlFilter(now), + filter: listFilter(now, isCrossApp(opts)), ...(select ? { select } : {}), }, ); @@ -520,8 +654,7 @@ export class SystemKVStore extends PuterStore { usage, readUsage( (response.ConsumedCapacity?.CapacityUnits as - | number - | undefined) ?? 1, + number | undefined) ?? 1, ), ); return response; @@ -538,8 +671,7 @@ export class SystemKVStore extends PuterStore { const skip = await runQuery(remaining, startKey, 'COUNT'); remaining -= Number(skip.Count ?? 0); startKey = skip.LastEvaluatedKey as - | Record - | undefined; + Record | undefined; if (!startKey) { exhausted = remaining > 0; break; @@ -562,8 +694,7 @@ export class SystemKVStore extends PuterStore { >), ); nextKey = response.LastEvaluatedKey as - | Record - | undefined; + Record | undefined; pages++; if (normalizedLimit === undefined) { // Legacy full listing: follow continuation pages so the @@ -599,8 +730,7 @@ export class SystemKVStore extends PuterStore { const counted = await runQuery(0, countKey, 'COUNT'); total += Number(counted.Count ?? 0); countKey = counted.LastEvaluatedKey as - | Record - | undefined; + Record | undefined; } while (countKey); } @@ -617,7 +747,7 @@ export class SystemKVStore extends PuterStore { async flush(opts?: KVOpts): Promise> { const actor = ensureActor(opts); - const namespace = getNamespace(actor, opts?.appUuid); + const namespace = getNamespace(actor, opts); const response = await this.clients.dynamo.query(this.tableName, { namespace, @@ -658,9 +788,10 @@ export class SystemKVStore extends PuterStore { ): Promise> { assertKey(key); const actor = ensureActor(opts); - const namespace = getNamespace(actor, opts?.appUuid); + const namespace = getNamespace(actor, opts); + const probeUsage = await this.#assertNotPrivate(namespace, key, opts); const usage = await this.rawExpireAt(namespace, key, Number(timestamp)); - return { res: undefined, usage }; + return { res: undefined, usage: addUsage(probeUsage, usage) }; } async expire( @@ -669,10 +800,11 @@ export class SystemKVStore extends PuterStore { ): Promise> { assertKey(key); const actor = ensureActor(opts); - const namespace = getNamespace(actor, opts?.appUuid); + const namespace = getNamespace(actor, opts); + const probeUsage = await this.#assertNotPrivate(namespace, key, opts); const timestamp = Math.floor(Date.now() / 1000) + Number(ttl); const usage = await this.rawExpireAt(namespace, key, timestamp); - return { res: undefined, usage }; + return { res: undefined, usage: addUsage(probeUsage, usage) }; } async incr>( @@ -702,7 +834,9 @@ export class SystemKVStore extends PuterStore { assertPaths(Object.keys(pathAndAmountMap)); const actor = ensureActor(opts); - const namespace = getNamespace(actor, opts?.appUuid); + const namespace = getNamespace(actor, opts); + + const probeUsage = await this.#assertNotPrivate(namespace, key, opts); const setStatements = Object.entries(pathAndAmountMap).map( ([valPath, _amt], idx) => { @@ -779,9 +913,12 @@ export class SystemKVStore extends PuterStore { response = await runUpdate(); } - const usage = writeUsage( - Number(response.ConsumedCapacity?.CapacityUnits ?? 0) + - createPathsUsage, + const usage = addUsage( + probeUsage, + writeUsage( + Number(response.ConsumedCapacity?.CapacityUnits ?? 0) + + createPathsUsage, + ), ); return { res: response.Attributes?.value, usage }; @@ -818,7 +955,9 @@ export class SystemKVStore extends PuterStore { assertPaths(Object.keys(pathAndValueMap)); const actor = ensureActor(opts); - const namespace = getNamespace(actor, opts?.appUuid); + const namespace = getNamespace(actor, opts); + + const probeUsage = await this.#assertNotPrivate(namespace, key, opts); const createPathsUsage = await this.createPaths( namespace, @@ -864,9 +1003,12 @@ export class SystemKVStore extends PuterStore { { ...valueAttributeNames, '#value': 'value' }, ); - const usage = writeUsage( - Number(response.ConsumedCapacity?.CapacityUnits ?? 0) + - createPathsUsage, + const usage = addUsage( + probeUsage, + writeUsage( + Number(response.ConsumedCapacity?.CapacityUnits ?? 0) + + createPathsUsage, + ), ); return { res: response.Attributes?.value, usage }; @@ -885,7 +1027,9 @@ export class SystemKVStore extends PuterStore { assertPaths(paths); const actor = ensureActor(opts); - const namespace = getNamespace(actor, opts?.appUuid); + const namespace = getNamespace(actor, opts); + + const probeUsage = await this.#assertNotPrivate(namespace, key, opts); const removeStatements = paths.map((valPath) => { return ['value', ...valPath.split('.')] @@ -920,10 +1064,12 @@ export class SystemKVStore extends PuterStore { ); return { res: response.Attributes?.value, - usage: writeUsage( - (response.ConsumedCapacity?.CapacityUnits as - | number - | undefined) ?? 1, + usage: addUsage( + probeUsage, + writeUsage( + (response.ConsumedCapacity?.CapacityUnits as + number | undefined) ?? 1, + ), ), }; } catch (e) { @@ -936,7 +1082,10 @@ export class SystemKVStore extends PuterStore { const fallback = await this.get({ key }, opts); return { res: fallback.res, - usage: addUsage(fallback.usage, writeUsage(1)), + usage: addUsage( + probeUsage, + addUsage(fallback.usage, writeUsage(1)), + ), }; } throw e; @@ -967,7 +1116,8 @@ export class SystemKVStore extends PuterStore { assertPaths(Object.keys(pathAndValueMap)); const actor = ensureActor(opts); - const namespace = getNamespace(actor, opts?.appUuid); + const namespace = getNamespace(actor, opts); + const probeUsage = await this.#assertNotPrivate(namespace, key, opts); const createPathsUsage = await this.createPaths( namespace, @@ -1024,9 +1174,12 @@ export class SystemKVStore extends PuterStore { { ...valueAttributeNames, '#value': 'value' }, ); - const usage = writeUsage( - Number(response.ConsumedCapacity?.CapacityUnits ?? 0) + - createPathsUsage, + const usage = addUsage( + probeUsage, + writeUsage( + Number(response.ConsumedCapacity?.CapacityUnits ?? 0) + + createPathsUsage, + ), ); return { res: response.Attributes?.value, usage }; diff --git a/src/backend/testUtil.ts b/src/backend/testUtil.ts index b86bffb30..4b73b10bc 100644 --- a/src/backend/testUtil.ts +++ b/src/backend/testUtil.ts @@ -4,6 +4,7 @@ import type { AddressInfo } from 'node:net'; import { fileURLToPath } from 'node:url'; import { v4 as uuidv4 } from 'uuid'; import { deepMerge } from '../../tools/lib/configMigration.mjs'; +import { makeActor } from './core/actor'; import { PuterServer } from './server'; import { IConfig } from './types'; import { puterClients } from './clients'; @@ -268,7 +269,7 @@ export const createTestUser = async ( // "API Token" flow does (POST /auth/create-access-token with the // full-api-access sentinel). const apiToken = await server.services.auth.createAccessToken( - { user }, + makeActor({ user }), [[FULL_API_ACCESS]], { label: 'puter-test-env' }, ); diff --git a/src/docs/src/KV/set.md b/src/docs/src/KV/set.md index 83227b167..c45505b0b 100755 --- a/src/docs/src/KV/set.md +++ b/src/docs/src/KV/set.md @@ -6,7 +6,7 @@ platforms: [websites, apps, nodejs, workers] When passed a key and a value, will add it to the user's key-value store, or update that key's value if it already exists. -
Each app has its own private key-value store within each user's account. Apps cannot access the key-value stores of other apps - only their own.
+
Each app has its own key-value store within each user's account. Another app can only reach it if the user explicitly grants that with puter.perms.requestAppData() — and never for entries you write with disableSharing.
## Syntax @@ -32,6 +32,14 @@ A string containing the value you want to give the key you are creating/updating A number containing when the key should expire in timestamp seconds. +#### `disableSharing` (Boolean) (optional) + +Pass inside the trailing options object — `set(key, value, { disableSharing: true })` — to mark this entry private to your app. A private entry cannot be read, listed, changed, or deleted by any other app, even one the user has granted access to your app's data with [`puter.perms.requestAppData()`](/Perms/requestAppData/). Use it for anything another app should never see, such as a cached access token: a user approving a request cannot see what your store holds. + +The batch form takes it too — `set([...items], { disableSharing: true })` marks every entry in the batch. + +Your own app reads and writes the entry normally. Writing the same key again without the flag makes it shareable once more, since `set` replaces the whole entry. + #### `items` (Array) (batch only) An array of `{ key, value, expireAt? }` objects, set in a single request. Each `key` is required and follows the same **1 KB** key / **400 KB** value limits. You can pass the array directly (`set([...])`) or wrapped in an object (`set({ items: [...] })`). @@ -44,6 +52,20 @@ A `Promise` that will resolves to `true` when the key-value pair has been create ## Examples +Store a value no other app can ever read + +```html + + + + + + +``` + Create a new key-value pair ```html;kv-set diff --git a/src/docs/src/Perms.md b/src/docs/src/Perms.md index 56de30d76..9ce3c8d5c 100644 --- a/src/docs/src/Perms.md +++ b/src/docs/src/Perms.md @@ -4,7 +4,7 @@ description: Request permissions to access user data and resources with Puter.js platforms: [apps] --- -The Permissions API enables your application to request access to user data and resources such as email addresses, special folders (Desktop, Documents, Pictures, Videos), apps, and subdomains. +The Permissions API enables your application to request access to user data and resources such as email addresses, special folders (Desktop, Documents, Pictures, Videos), apps, subdomains, and other apps' saved data. When requesting permissions, users will be prompted to grant or deny access. If a permission has already been granted, the user will not be prompted again. This provides a seamless experience while maintaining user privacy and control. @@ -15,6 +15,7 @@ When requesting permissions, users will be prompted to grant or deny access. If
Request Desktop Access
Request Documents Access
Request Apps Access
+
Use Another App's Data
@@ -123,6 +124,33 @@ When requesting permissions, users will be prompted to grant or deny access. If
+
+ +#### Use another app's saved data + +```html + + + + + + + +``` + +
+ ## Functions These permission features are supported out of the box when using Puter.js: @@ -160,6 +188,10 @@ These permission features are supported out of the box when using Puter.js: - **[`puter.perms.requestReadApps()`](/Perms/requestReadApps/)** - Request read access to the user's apps - **[`puter.perms.requestManageApps()`](/Perms/requestManageApps/)** - Request write (manage) access to the user's apps +### Other Apps' Data + +- **[`puter.perms.requestAppData()`](/Perms/requestAppData/)** - Request permission to use another app's key-value data and `AppData` files + ### Subdomains Management - **[`puter.perms.requestReadSubdomains()`](/Perms/requestReadSubdomains/)** - Request read access to the user's subdomains diff --git a/src/docs/src/Perms/requestAppData.md b/src/docs/src/Perms/requestAppData.md new file mode 100644 index 000000000..5a5ce6206 --- /dev/null +++ b/src/docs/src/Perms/requestAppData.md @@ -0,0 +1,146 @@ +--- +title: puter.perms.requestAppData() +description: Request permission to use another app's data — its key-value store and its AppData files. +platforms: [websites, apps] +--- + +Request permission for your app to use another app's data belonging to the signed-in user: that app's key-value namespace, its `AppData` directory, or both. A calendar might read a contacts app's entries to show birthdays, and add an invite the user can later cancel from either app. + +The user is prompted once and sees exactly which apps and which kinds of access are involved. If the permission has already been granted the user is not prompted and `true` is returned. If the user declines, `false` is returned. + +On a website, sign the user in to your site first with [`puter.auth.signIn()`](/Auth/signIn/). This method reads the signed-in user's identity before it can prompt, so for a signed-out visitor it rejects with `Unauthorized` and no prompt is shown. Answering a permission prompt does not by itself sign the user in to your site, so guard the call: + +```js +if (!puter.authToken) await puter.auth.signIn(); +``` + +## Syntax + +```js +puter.perms.requestAppData(appIdentifier, scopes) +``` + +## Parameters + +#### `appIdentifier` (String | Object) (required) +The app whose data you want to use. Either its uid (`app-…`), its registered name, or an object carrying one: `{ uid: 'app-…' }` or `{ name: 'contacts' }`. + +#### `scopes` (String | Array | Object) (required) +What access to ask for. Three equivalent forms: + +- **A single word** applied to both stores: `'read'`, `'write'`, or `'delete'`. +- **An array of `store:name` pairs**: `['kv:get', 'fs:read']`. +- **An object per store**: `{ kv: ['get', 'set'], fs: 'read' }`. + +`store` is `kv` (the app's key-value data) or `fs` (its files under `AppData`). + +`name` is either an access class or a single key-value operation: + +| Class | Covers | +| --- | --- | +| `read` | `get`, `list` | +| `write` | `set`, `add`, `incr`, `decr`, `update` | +| `delete` | `del`, `remove`, `expire`, `expireAt` | + +**`delete` is separate from `write`.** An app granted `write` can add and change entries but cannot remove any — ask for `delete` explicitly when it needs to. Emptying another app's whole key-value store is never available at any scope. + +## Return value + +A `Promise` that resolves to: +- `true` - If your app may now use that data +- `false` - If the user declined + +The promise rejects if the named app does not exist, or if a scope is misspelled. + +## Examples + +Read another app's data + +```html + + + + + + + +``` + +Add an entry, and be able to remove it later + +```html + + + + + + + +``` + +Read another app's files + +```html + + + + + + + +``` + +## Keeping your own data private + +Another app can only reach your data if the user grants it, but the user cannot see what a key-value namespace holds before answering. If your app stores something no other app should ever read — a cached OAuth token, a licence key — mark it private when you write it: + +```js +await puter.kv.set('googleRefreshToken', token, { disableSharing: true }); +``` + +A private entry is invisible to every other app: reads return nothing, listings omit it, and writes and deletes are refused — regardless of what the user has granted. Your own app reads and writes it normally, and writing the key again without the flag makes it shareable once more. + +To keep *all* of your app's data out of this feature, set `share_app_data` to `false` in your app's metadata. Requests naming your app are then refused and the user is never prompted. + +## Notes + +Granted access is scoped to the user who granted it, and only to the two stores above — it does not extend to that app's source, settings, or anything outside their per-user data. + +Access ends automatically when the target app is deleted. Grants are also withdrawn if the app is later re-created under the same identifier, so a new owner of that identifier does not inherit consent the user gave its predecessor. diff --git a/src/docs/src/playground/examples/perms-request-app-data.html b/src/docs/src/playground/examples/perms-request-app-data.html new file mode 100644 index 000000000..06b8340d6 --- /dev/null +++ b/src/docs/src/playground/examples/perms-request-app-data.html @@ -0,0 +1,22 @@ + + + + + + + diff --git a/src/gui/src/IPC.js b/src/gui/src/IPC.js index fc5531bb4..1ff6f9beb 100644 --- a/src/gui/src/IPC.js +++ b/src/gui/src/IPC.js @@ -1348,16 +1348,33 @@ const ipc_listener = async (event, handled) => { event.data.options = {}; } - // options.permission must be provided and be a string - if ( !event.data.options.permission || typeof event.data.options.permission !== 'string' ) + // One of `permission` (a string) or `permissions` (a non-empty list of + // strings) must be provided. The dialog validates the strings + // themselves; this only rejects a shape it cannot read. + const requested_permissions = Array.isArray(event.data.options.permissions) + ? event.data.options.permissions + : [event.data.options.permission]; + // Capped to match the server: otherwise the dialog renders every row and + // the grant 400s after Allow — consent for something ungrantable. + const MAX_REQUESTED_PERMISSIONS = 16; + if ( requested_permissions.length === 0 + || requested_permissions.length > MAX_REQUESTED_PERMISSIONS + || requested_permissions.some(p => !p || typeof p !== 'string') ) { - console.error('IPC requestPermission requires parameter { permission }', event.data); + console.error('IPC requestPermission requires parameter { permission } or { permissions }', event.data); respond(false); return; } let granted = await UIPermissionDialog({ - permission: event.data.options.permission, + // Both forms: the dialog reads `permissions`, and `permission` keeps + // the single-scope path working for callers (and dialog versions) + // that only know the scalar. A multi-scope request with no list + // support is refused rather than partially granted. + permissions: requested_permissions, + permission: requested_permissions.length === 1 + ? requested_permissions[0] + : undefined, app_uid: app_uuid, app_name: app_name, }); diff --git a/src/gui/src/UI/UIPermissionDialog.js b/src/gui/src/UI/UIPermissionDialog.js index 664789904..7aac5e58f 100644 --- a/src/gui/src/UI/UIPermissionDialog.js +++ b/src/gui/src/UI/UIPermissionDialog.js @@ -49,7 +49,8 @@ const LOOKUP_TIMEOUT_MS = 10000; * written against, so without one there is nothing to prompt about. * * @param {Object} options - * @param {string} options.permission - The permission string being requested. + * @param {string} [options.permission] - A single permission string. + * @param {string[]} [options.permissions] - Several, answered as one decision. * @param {string} [options.app_uid] - UID of the requesting app, if known. * @param {string} [options.app_name] - Registered name of the requesting app; * used for display, never as the grant target. @@ -59,9 +60,18 @@ const LOOKUP_TIMEOUT_MS = 10000; async function UIPermissionDialog (options) { options = options ?? {}; - if ( ! options.permission || typeof options.permission !== 'string' ) { + // One decision can cover several scopes. Normalised here so everything + // downstream — the pending key, the dialog body, the grant — sees a list. + const permissions = Array.isArray(options.permissions) + ? options.permissions + : [options.permission]; + if ( permissions.length === 0 + || permissions.some(p => ! p || typeof p !== 'string') ) { return false; } + // Sorted so two requests for the same set share one in-flight prompt + // regardless of the order the caller listed them in. + options = { ...options, permissions: [...permissions].sort() }; // Never prompt the user on behalf of a requester the grant can't name. // Only `app_uid` and `origin` are sent to /auth/grant-user-app, so an @@ -86,7 +96,7 @@ async function UIPermissionDialog (options) { // `||`, not `??`: the gate above treats an empty uid as absent, so the key // has to fall through to the origin too — otherwise two different origins // arriving with a blank uid would share one decision. - const pending_key = `${options.app_uid || options.origin || ''}\n${options.permission}`; + const pending_key = `${options.app_uid || options.origin || ''}\n${options.permissions.join('\n')}`; if ( pending_dialogs.has(pending_key) ) { return pending_dialogs.get(pending_key); } @@ -106,7 +116,7 @@ async function UIPermissionDialog (options) { // Callers treat this as the user's decision and some of them (the // IPC bridge) have no way to answer a rejection, which would leave // the requesting app waiting forever. Fail closed instead. - console.error('Permission dialog failed', options.permission, e); + console.error('Permission dialog failed', options.permissions, e); return false; } finally { release_turn(); @@ -121,30 +131,36 @@ async function UIPermissionDialog (options) { } async function show_permission_dialog (options) { - let permission_description; + let descriptions; try { - permission_description = await with_timeout( - get_permission_description(options.permission), + descriptions = await with_timeout( + Promise.all(options.permissions.map( + permission => get_permission_description(permission, options), + )), LOOKUP_TIMEOUT_MS, null, ); } catch (e) { // Description lookup needs auth/whoami; treat failures as unsupported. - console.error('Failed to describe permission', options.permission, e); + console.error('Failed to describe permission', options.permissions, e); return false; } // Unsupported permission strings are denied silently (existing contract). // A lookup that timed out lands here too: nothing to describe, so nothing // to prompt about. - if ( ! permission_description ) { + // + // One undescribable scope denies the whole prompt rather than being dropped + // from it: a scope the user was never shown must not ride along on an Allow + // that described the others. + if ( ! descriptions || descriptions.some(d => ! d) ) { return false; } const entity = await resolve_requesting_entity(options); return new Promise((resolve) => { - const el_dialog = create_dialog_element(entity, permission_description); + const el_dialog = create_dialog_element(entity, descriptions); document.body.appendChild(el_dialog); // Set once a grant request has been sent without the server definitively @@ -253,7 +269,11 @@ async function show_permission_dialog (options) { body: JSON.stringify({ app_uid: options.app_uid, origin: options.origin, - permission: options.permission, + // One request for the whole set: the server validates + // every entry before writing any, so a partial grant + // can't survive a rejected scope — and the + // uncertain-commit handling below stays single-flight. + permissions: options.permissions, }), method: 'POST', ...(controller ? { signal: controller.signal } : {}), @@ -325,7 +345,7 @@ async function undo_uncertain_grant (options) { body: JSON.stringify({ app_uid: options.app_uid, origin: options.origin, - permission: options.permission, + permissions: options.permissions, }), method: 'POST', // In the popup flow the answer is posted and the window closed @@ -334,7 +354,7 @@ async function undo_uncertain_grant (options) { keepalive: true, }); } catch (e) { - console.error('Failed to withdraw an uncertain permission grant', e); + console.error('Failed to withdraw uncertain permission grants', e); } } @@ -377,7 +397,7 @@ function with_timeout (promise, ms, fallback) { * Builds the dialog DOM from the requesting entity and the permission * description. Returns a detached element. */ -function create_dialog_element (entity, permission_description) { +function create_dialog_element (entity, descriptions) { let h = ''; h += '
'; @@ -398,10 +418,12 @@ function create_dialog_element (entity, permission_description) { // what is being requested h += `

${i18n('perm_dialog_wants_to')}

`; - h += '
'; - h += `
${permission_icon_svg(permission_description.icon)}
`; - h += `
${permission_description.html}
`; - h += '
'; + for ( const description of descriptions ) { + h += '
'; + h += `
${permission_icon_svg(description.icon)}
`; + h += `
${description.html}
`; + h += '
'; + } // error message (hidden until a grant attempt fails) h += ''; @@ -527,10 +549,12 @@ function permission_icon_svg (icon) { * Generates a user-friendly description of a permission string. * * @param {string} permission - The permission string to describe + * @param {Object} [options] - The dialog options, for checks that depend on who + * is asking (a request for the requester's own data, for instance). * @returns {Promise<{html: string, icon: string} | null>} Description (HTML) * and icon key, or null if the permission cannot be requested interactively. */ -async function get_permission_description (permission) { +async function get_permission_description (permission, options = {}) { const parts = split_permission(permission); if ( ['fs', 'thread', 'service', 'driver'].includes(parts[0]) ) { @@ -618,6 +642,10 @@ async function get_permission_description (permission) { } } + if ( parts[0] === 'app-data' ) { + return await get_app_data_description(parts, options); + } + if ( parts[0] === 'app-root-dir' ) { // Format: app-root-dir:: if ( parts[2] === 'read' ) { @@ -631,6 +659,101 @@ async function get_permission_description (permission) { return null; } +/** + * KV operation, or access class, → the verb the user sees. Deletion is named + * explicitly: a scope that can remove another app's entries must not read as + * "change". + */ +// `write` satisfies a read check via the exploder, so its wording names +// reading too — "change" alone would understate the grant. +const APP_DATA_VERBS = { + read: 'read', get: 'read', list: 'read', + write: 'change', set: 'change', add: 'change', + incr: 'change', decr: 'change', update: 'change', + delete: 'delete', del: 'delete', remove: 'delete', + expire: 'delete', expireAt: 'delete', +}; + +/** + * Look up the app a cross-app request names. Returns null when it does not + * exist, so an unresolvable target never reaches the prompt. + */ +async function get_app_by_uid (uid) { + try { + const res = await fetch(`${window.api_origin}/drivers/call`, { + headers: { + 'Content-Type': 'application/json', + 'Authorization': `Bearer ${window.auth_token}`, + }, + body: JSON.stringify({ + interface: 'puter-apps', + driver: 'es:app', + method: 'read', + args: { uid }, + }), + method: 'POST', + }); + if ( ! res.ok ) return null; + const body = await res.json(); + return body?.result ?? null; + } catch (e) { + console.error('Failed to look up app', uid, e); + return null; + } +} + +/** + * Describes `app-data:[:[:]]` — one app using another's data. + * + * Returns null (which denies without prompting) when there is nothing to ask: + * the target is the requester itself, the target does not exist, or its + * developer has opted out of sharing. In each case the grant would be refused or + * redundant, so a prompt would only mislead. + */ +export async function get_app_data_description (parts, options) { + const [, target_uid, store, op] = parts; + if ( ! target_uid ) return null; + + // An app already reaches its own data; approving that would mean nothing. + if ( options.app_uid && target_uid === options.app_uid ) return null; + + const app = await get_app_by_uid(target_uid); + if ( ! app ) return null; + if ( app.metadata?.share_app_data === false ) return null; + + const app_label = app.title || app.name || target_uid; + + // No op named means every op under it, by prefix implication — so the copy + // has to cover deletion too, not just reading and changing. + const verb = op ? APP_DATA_VERBS[op] : null; + if ( op && ! verb ) return null; + + if ( ! store ) { + return { + html: i18n('perm_app_data_all', { app: app_label }), + icon: 'shield', + }; + } + if ( store !== 'kv' && store !== 'fs' ) return null; + + // `false` so only the outer `i18n` encodes: it encodes the whole + // interpolated string, so two levels show a literal `'`. + const subject = store === 'fs' + ? i18n('perm_app_data_subject_files', { app: app_label }, false) + : i18n('perm_app_data_subject_data', { app: app_label }, false); + + if ( ! verb ) { + return { + html: i18n('perm_app_data_store_all', { subject }), + icon: store === 'fs' ? 'folder' : 'shield', + }; + } + return { + html: i18n(`perm_app_data_${verb}`, { subject }), + icon: store === 'fs' ? 'folder' : 'shield', + }; +} + /** * Returns a user-friendly description for standard folder permissions. * Uses whoami().directories to verify the path/UUID belongs to the current user. diff --git a/src/gui/src/UI/UIPermissionDialog.rendering.test.js b/src/gui/src/UI/UIPermissionDialog.rendering.test.js new file mode 100644 index 000000000..4f1f01bcf --- /dev/null +++ b/src/gui/src/UI/UIPermissionDialog.rendering.test.js @@ -0,0 +1,91 @@ +import { beforeAll, describe, expect, it, vi } from 'vitest'; +import { decode, encode } from 'html-entities'; + +// What the user actually sees: these run the *real* `i18n` and the real encoder, +// unlike UIPermissionDialog.test.js which stubs `i18n` to echo keys. That stub is +// right for asserting which wording was chosen, but it cannot catch an encoding +// mistake — `i18n()` HTML-encodes the whole interpolated string, so composing two +// calls double-encodes and the stub renders both identically. +globalThis.window = globalThis.window ?? {}; +window.api_origin = 'https://api.test'; +window.auth_token = 'tok'; +globalThis.html_encode = (str) => encode(str); + +let get_app_data_description; + +beforeAll(async () => { + await import('../i18n/i18n.js'); // installs window.i18n + globalThis.i18n = window.i18n; + ({ get_app_data_description } = await import('./UIPermissionDialog.js')); +}); + +const CONTACTS = 'app-contacts'; + +const stubApp = (app) => { + globalThis.fetch = vi.fn(async () => ({ + ok: true, + json: async () => ({ success: true, result: app }), + })); +}; + +const render = async (permission) => { + const d = await get_app_data_description(permission.split(':'), { + app_uid: 'app-calendar', + }); + return d.html; +}; + +describe('UIPermissionDialog app-data rendering', () => { + beforeAll(() => stubApp({ uid: CONTACTS, title: 'Contacts' })); + + it('encodes the apostrophe exactly once', async () => { + const html = await render(`app-data:${CONTACTS}:kv:get`); + // Decoding once must yield readable text. If both `i18n` levels encoded, + // the user would see a literal `'` in the prompt instead. + expect(decode(html)).toContain("Contacts's saved data"); + expect(html).not.toContain('&'); + }); + + it('reads correctly for every scope shape', async () => { + for (const permission of [ + `app-data:${CONTACTS}:kv:get`, + `app-data:${CONTACTS}:kv:set`, + `app-data:${CONTACTS}:kv:del`, + `app-data:${CONTACTS}:kv`, + `app-data:${CONTACTS}:fs:read`, + `app-data:${CONTACTS}`, + ]) { + const html = await render(permission); + expect(html).not.toContain('&'); + expect(decode(html)).toContain('Contacts'); + } + }); + + it('names deletion for scopes that imply it', async () => { + expect(decode(await render(`app-data:${CONTACTS}:kv:del`))).toContain( + 'delete', + ); + // Store- and app-wide scopes cover deletion by prefix implication, so + // the wording has to say so. + expect(decode(await render(`app-data:${CONTACTS}:kv`))).toContain( + 'delete', + ); + expect(decode(await render(`app-data:${CONTACTS}`))).toContain('delete'); + }); + + it('names reading for a write scope, which confers it', async () => { + // `write` satisfies `get`/`list` through the exploder, so a prompt that + // said only "change" would understate what the user is approving. + expect(decode(await render(`app-data:${CONTACTS}:kv:set`))).toContain( + 'read', + ); + }); + + it('escapes a hostile app title exactly once', async () => { + stubApp({ uid: CONTACTS, title: '' }); + const html = await render(`app-data:${CONTACTS}:kv:get`); + expect(html).not.toContain(' + `${key}(${Object.entries(params).map(([k, v]) => `${k}=${v}`).join(',')})`; + +const { get_app_data_description } = await import('./UIPermissionDialog.js'); + +const CONTACTS = 'app-contacts'; +const CALENDAR = 'app-calendar'; + +/** Stub the app lookup the describer performs. */ +const stubApp = (app) => { + globalThis.fetch = vi.fn(async () => ({ + ok: app !== null, + json: async () => ({ success: true, result: app }), + })); +}; + +const describeScope = (permission, options = { app_uid: CALENDAR }) => + get_app_data_description(permission.split(':'), options); + +describe('UIPermissionDialog app-data descriptions', () => { + beforeEach(() => { + stubApp({ uid: CONTACTS, name: 'contacts', title: 'Contacts' }); + }); + + it('names the target app and the read verb', async () => { + const d = await describeScope(`app-data:${CONTACTS}:kv:get`); + expect(d.html).toContain('perm_app_data_read'); + expect(d.html).toContain('Contacts'); + }); + + it('says "change" for a write and "delete" for a deletion', async () => { + expect((await describeScope(`app-data:${CONTACTS}:kv:set`)).html) + .toContain('perm_app_data_change'); + // Deletion must be named, not folded into "change". + expect((await describeScope(`app-data:${CONTACTS}:kv:del`)).html) + .toContain('perm_app_data_delete'); + expect((await describeScope(`app-data:${CONTACTS}:kv:delete`)).html) + .toContain('perm_app_data_delete'); + }); + + it('distinguishes files from saved data', async () => { + expect((await describeScope(`app-data:${CONTACTS}:fs:read`)).html) + .toContain('perm_app_data_subject_files'); + expect((await describeScope(`app-data:${CONTACTS}:kv:read`)).html) + .toContain('perm_app_data_subject_data'); + }); + + it('names deletion for a store-wide scope, which implies it', async () => { + const d = await describeScope(`app-data:${CONTACTS}:kv`); + expect(d.html).toContain('perm_app_data_store_all'); + }); + + it('names deletion for an app-wide scope too', async () => { + const d = await describeScope(`app-data:${CONTACTS}`); + expect(d.html).toContain('perm_app_data_all'); + expect(d.html).toContain('Contacts'); + }); + + // -- the cases that must never prompt --------------------------------- + + it('refuses to describe a request for the requester’s own data', async () => { + // Already implicit, so a prompt would ask the user to approve nothing. + expect( + await describeScope(`app-data:${CALENDAR}:kv:get`, { + app_uid: CALENDAR, + }), + ).toBeNull(); + }); + + it('refuses when the target app does not exist', async () => { + stubApp(null); + expect(await describeScope(`app-data:${CONTACTS}:kv:get`)).toBeNull(); + }); + + it('refuses when the target app opted out of sharing', async () => { + stubApp({ + uid: CONTACTS, + title: 'Contacts', + metadata: { share_app_data: false }, + }); + expect(await describeScope(`app-data:${CONTACTS}:kv:get`)).toBeNull(); + }); + + it('refuses a missing target, unknown store, or unknown op', async () => { + expect(await describeScope('app-data')).toBeNull(); + expect(await describeScope('app-data:')).toBeNull(); + expect(await describeScope(`app-data:${CONTACTS}:sql:read`)).toBeNull(); + expect(await describeScope(`app-data:${CONTACTS}:kv:flush`)).toBeNull(); + expect(await describeScope(`app-data:${CONTACTS}:kv:bogus`)).toBeNull(); + }); + + it('treats a failed lookup as undescribable rather than throwing', async () => { + globalThis.fetch = vi.fn(async () => { + throw new Error('network down'); + }); + expect(await describeScope(`app-data:${CONTACTS}:kv:get`)).toBeNull(); + }); +}); diff --git a/src/gui/src/i18n/translations/en.js b/src/gui/src/i18n/translations/en.js index 3470dbc61..8a069f6d6 100644 --- a/src/gui/src/i18n/translations/en.js +++ b/src/gui/src/i18n/translations/en.js @@ -644,6 +644,13 @@ const en = { 'perm_subdomains_write': 'manage your subdomains', 'perm_app_root_dir_read': 'read the root directory of one of your apps', 'perm_app_root_dir_write': 'read and write to the root directory of one of your apps', + 'perm_app_data_subject_data': "{{app}}'s saved data", + 'perm_app_data_subject_files': "{{app}}'s files", + 'perm_app_data_read': 'read {{subject}}.', + 'perm_app_data_change': 'read and change {{subject}}.', + 'perm_app_data_delete': 'delete entries from {{subject}}.', + 'perm_app_data_store_all': 'read, change and delete {{subject}}.', + 'perm_app_data_all': "read, change and delete everything {{app}} has saved for you, including any saved logins.", 'perm_dialog_wants_to': 'wants permission to', 'perm_dialog_footnote': 'You can change this anytime in Settings.', 'perm_dialog_error': 'Something went wrong. Please try again.', diff --git a/src/gui/src/initgui.js b/src/gui/src/initgui.js index 09d43b1dd..af929ae08 100644 --- a/src/gui/src/initgui.js +++ b/src/gui/src/initgui.js @@ -648,7 +648,18 @@ const postAuthActions = async (action) => { // Runs post-auth so signed-out users go through sign-in/signup first. // ------------------------------------------------------------------------------------- if ( action === 'request-permission' ) { - const permission = window.url_query_params.get('permission'); + // Repeated `permission=` params: one prompt can cover several scopes. + // Capped because this URL is supplied by whoever opened the popup, and an + // unbounded list would put an unreadable consent prompt in front of the + // user. Over the cap we drop the request rather than truncate it, since a + // silently shortened list would grant less than the dialog described. + const MAX_REQUESTED_PERMISSIONS = 16; + const requested_permissions = window.url_query_params + .getAll('permission') + .filter(Boolean); + const permissions = requested_permissions.length <= MAX_REQUESTED_PERMISSIONS + ? requested_permissions + : []; const msg_id = window.url_query_params.get('msg_id'); // Browser-attested only: `openerOrigin` is the referrer, or the opener's // own reply to the `requestOrigin` handshake. There is deliberately no @@ -683,7 +694,10 @@ const postAuthActions = async (action) => { // same origin the dialog displayed, and reject it outright // unless it names an app that really exists. granted = await UIPermissionDialog({ - permission: permission, + // See IPC.js: both forms, so a single scope still works with a + // dialog that only understands the scalar. + permissions, + permission: permissions.length === 1 ? permissions[0] : undefined, origin: origin, }); } catch (e) { diff --git a/src/puter-js/src/modules/UI.js b/src/puter-js/src/modules/UI.js index bc8ed926e..5a52c8f1a 100644 --- a/src/puter-js/src/modules/UI.js +++ b/src/puter-js/src/modules/UI.js @@ -30,6 +30,10 @@ const createDeferred = () => { const FILE_SAVE_CANCELLED = Symbol('FILE_SAVE_CANCELLED'); const FILE_OPEN_CANCELLED = Symbol('FILE_OPEN_CANCELLED'); +// A consent prompt covers a handful of scopes at most, and the popup carries +// them in its URL. +const MAX_REQUESTED_PERMISSIONS = 16; + // AppConnection provides an API for interacting with another app. // It's returned by UI methods, and cannot be constructed directly by user code. // For basic usage: @@ -1371,7 +1375,10 @@ class UI extends EventListener { * the request is relayed to the desktop; on the web the permission * dialog is shown in a popup window on the Puter origin. * - * @param {{ permission: string }} options + * One prompt may cover several scopes: pass `permissions` instead of + * `permission` and the user answers for the whole list at once. + * + * @param {{ permission?: string, permissions?: string[] }} options * @returns {Promise} `true` only if the permission was granted. */ async requestPermission (options) { @@ -1392,8 +1399,17 @@ class UI extends EventListener { if ( ! globalThis.open || ! globalThis.document ) { return false; } - const permission = options?.permission; - if ( typeof permission !== 'string' || permission === '' ) { + // The popup carries the request in its URL, so cap the list: a link is + // attacker-supplied, and an unbounded one could stack an unreadable + // consent prompt in front of the user (and overflow the URL). + const requested = Array.isArray(options?.permissions) + ? options.permissions + : [options?.permission]; + if ( + requested.length === 0 || + requested.length > MAX_REQUESTED_PERMISSIONS || + requested.some(p => typeof p !== 'string' || p === '') + ) { return false; } @@ -1430,7 +1446,12 @@ class UI extends EventListener { // two impossible to confuse. The GUI echoes the value back verbatim as // a string, which the loose `!=` below compares correctly. const msg_id = `${this.#messageID++}-${Math.random().toString(36).slice(2, 10)}`; - const url = `${gui_origin}/action/request-permission?embedded_in_popup=true&msg_id=${encodeURIComponent(msg_id)}&permission=${encodeURIComponent(permission)}`; + // Repeated `permission=` params rather than a JSON blob, so the GUI + // reads a list of plain strings with no parsing step to get wrong. + const query = requested + .map(p => `permission=${encodeURIComponent(p)}`) + .join('&'); + const url = `${gui_origin}/action/request-permission?embedded_in_popup=true&msg_id=${encodeURIComponent(msg_id)}&${query}`; // Guards against settling more than once across the message, // popup-closed, and dialog-cancel code paths. diff --git a/src/puter-js/src/modules/perms/appData.js b/src/puter-js/src/modules/perms/appData.js new file mode 100644 index 000000000..2d1ea53e5 --- /dev/null +++ b/src/puter-js/src/modules/perms/appData.js @@ -0,0 +1,178 @@ +import { PuterJSError } from '../../lib/PuterJSError.js'; + +/** @typedef {import('./index.js').PermsModule} PermsModule */ +/** @typedef {import('../../../types/modules/perms').AppDataScopes} AppDataScopes */ +/** @typedef {import('../../../types/modules/perms').AppDataKvScope} AppDataKvScope */ +/** @typedef {import('../../../types/modules/perms').AppDataFsScope} AppDataFsScope */ + +// Mirrors `services/permission/appDataScopes.ts`, which stays authoritative. +// This copy only turns a typo into a useful error instead of an opaque 403. +const KV_CLASS_OPS = { + read: ['get', 'list'], + write: ['set', 'add', 'incr', 'decr', 'update'], + delete: ['del', 'remove', 'expire', 'expireAt'], +}; + +const FS_CLASSES = ['read', 'write', 'delete']; + +const KV_OPS = Object.values(KV_CLASS_OPS).flat(); + +/** Never grantable: it empties a whole namespace rather than touching entries. */ +const KV_FORBIDDEN_OPS = ['flush']; + +const APP_UID_RE = + /^app-[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +const err = (message) => new PuterJSError(message, 'invalid_argument'); + +/** + * Normalise one store's requested scopes into a deduped list of op or class + * names, accepting a single string or an array. + */ +const toList = (value, store) => { + if (value === undefined || value === null) return []; + const list = Array.isArray(value) ? value : [value]; + for (const entry of list) { + if (typeof entry !== 'string' || entry === '') { + throw err(`${store} scopes must be non-empty strings`); + } + if (store === 'kv' && KV_FORBIDDEN_OPS.includes(entry)) { + throw err(`kv:${entry} cannot be granted to another app`); + } + const known = + store === 'kv' + ? [...KV_OPS, ...Object.keys(KV_CLASS_OPS)] + : FS_CLASSES; + if (!known.includes(entry)) { + throw err(`unknown ${store} scope: ${entry}`); + } + } + return [...new Set(list)]; +}; + +/** + * Collapse a KV request to the fewest names covering it, but only when the + * collapse is lossless — the requested ops must be exactly a class's full set. + * + * Collapsing `['get']` up to `read` would ask the user to approve `list` as + * well, so a partial set stays spelled out: fewer dialog lines are not worth + * granting more than the app asked for. + */ +const collapseKv = (requested) => { + const wanted = new Set(requested); + const classes = []; + + for (const [cls, ops] of Object.entries(KV_CLASS_OPS)) { + if (wanted.has(cls)) { + classes.push(cls); + for (const op of ops) wanted.delete(op); + wanted.delete(cls); + continue; + } + if (ops.every((op) => wanted.has(op))) { + classes.push(cls); + for (const op of ops) wanted.delete(op); + } + } + + return [...classes, ...wanted]; +}; + +/** + * Build the permission strings for a request. Exported for the unit tests, which + * assert the mapping without going near the IPC layer. + * + * @param {string} appUid + * @param {{ kv?: AppDataKvScope | AppDataKvScope[], fs?: AppDataFsScope | AppDataFsScope[] }} scopes + * @returns {string[]} + */ +export function appDataPermissions (appUid, scopes) { + const kv = collapseKv(toList(scopes.kv, 'kv')); + const fs = toList(scopes.fs, 'fs'); + if (kv.length === 0 && fs.length === 0) { + throw err('at least one `kv` or `fs` scope is required'); + } + + // Both stores complete: one row covers the lot. + const allKv = Object.keys(KV_CLASS_OPS).every((c) => kv.includes(c)); + const allFs = FS_CLASSES.every((c) => fs.includes(c)); + if (allKv && allFs) return [`app-data:${appUid}`]; + + const out = []; + if (allKv) out.push(`app-data:${appUid}:kv`); + else for (const name of kv) out.push(`app-data:${appUid}:kv:${name}`); + if (allFs) out.push(`app-data:${appUid}:fs`); + else for (const name of fs) out.push(`app-data:${appUid}:fs:${name}`); + + // Sorted so an identical request yields an identical list — the dialog + // de-duplicates concurrent prompts on it. + return out.sort(); +} + +/** Expand the `'read' | 'write' | 'delete'` shorthand to both stores. */ +const normaliseScopes = (scopes) => { + if (typeof scopes === 'string') return { kv: scopes, fs: scopes }; + if (Array.isArray(scopes)) { + const out = { kv: [], fs: [] }; + for (const entry of scopes) { + if (typeof entry !== 'string' || !entry.includes(':')) { + throw err(`scope must look like "kv:get" or "fs:read": ${entry}`); + } + const [store, name] = entry.split(':'); + // Explicit names, not `out[store]`: `toString` and friends are + // truthy, so the push below would throw a TypeError instead. + if (store !== 'kv' && store !== 'fs') { + throw err(`unknown store: ${store}`); + } + out[store].push(name); + } + return out; + } + if (scopes && typeof scopes === 'object') return scopes; + throw err('scopes must be a string, an array, or an object'); +}; + +/** + * Ask the user to let this app use another app's data — its KV namespace and its + * AppData directory. + * + * The target may be named by uid or by registered app name. Scopes accept a + * shorthand applying to both stores, explicit `store:name` pairs, or a per-store + * object: + * + * await puter.perms.requestAppData('contacts', 'read'); + * await puter.perms.requestAppData('contacts', ['kv:get', 'fs:read']); + * await puter.perms.requestAppData('contacts', { kv: ['get', 'set'], fs: 'read' }); + * + * Deleting entries is a separate scope from writing them, so an app that only + * adds data cannot remove any: request `delete` explicitly when it needs to. + * + * @this {PermsModule} + * @param {string | { uid: string } | { name: string }} appIdentifier + * @param {AppDataScopes} scopes + * @returns {Promise} `true` if the app may now use that data. + */ +export async function requestAppData (appIdentifier, scopes) { + const identifier = + typeof appIdentifier === 'object' && appIdentifier !== null + ? (appIdentifier.uid ?? appIdentifier.name) + : appIdentifier; + if (typeof identifier !== 'string' || identifier === '') { + throw err('parameter appIdentifier must be a non-empty string'); + } + + // A uid is `app-` plus a UUID. The bare prefix would read an app *named* + // `app-store` as a uid, and `puter.apps.get` resolves names only. + const appUid = APP_UID_RE.test(identifier) + ? identifier + : (await this.puter.apps.get(identifier))?.uid; + if (typeof appUid !== 'string' || appUid === '') { + throw new PuterJSError(`app not found: ${identifier}`, 'not_found'); + } + + // Already true for its own data, so prompting would ask for nothing. + if (appUid === this.puter.appID) return true; + + const permissions = appDataPermissions(appUid, normaliseScopes(scopes)); + return await this.puter.ui.requestPermission({ permissions }); +} diff --git a/src/puter-js/src/modules/perms/appData.test.js b/src/puter-js/src/modules/perms/appData.test.js new file mode 100644 index 000000000..52cee8960 --- /dev/null +++ b/src/puter-js/src/modules/perms/appData.test.js @@ -0,0 +1,195 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { appDataPermissions, requestAppData } from './appData.js'; +import { PuterJSError } from '../../lib/PuterJSError.js'; + +const CONTACTS = 'app-11111111-2222-3333-4444-555555555555'; + +const makeModule = ({ granted = true, appID = 'app-99999999-8888-7777-6666-555555555555', get } = {}) => ({ + puter: { + appID, + apps: { get: vi.fn(get ?? (async () => ({ uid: CONTACTS }))) }, + ui: { requestPermission: vi.fn(async () => granted) }, + }, +}); + +describe('perms appDataPermissions', () => { + it('names one permission per requested op', () => { + expect(appDataPermissions(CONTACTS, { kv: ['get'] })).toEqual([ + `app-data:${CONTACTS}:kv:get`, + ]); + }); + + it('accepts a class name directly', () => { + expect(appDataPermissions(CONTACTS, { kv: 'read', fs: 'write' })).toEqual([ + `app-data:${CONTACTS}:fs:write`, + `app-data:${CONTACTS}:kv:read`, + ]); + }); + + it('collapses a complete class into the class name', () => { + // get + list is exactly the read class, so this loses nothing. + expect(appDataPermissions(CONTACTS, { kv: ['get', 'list'] })).toEqual([ + `app-data:${CONTACTS}:kv:read`, + ]); + }); + + it('does not collapse a partial class', () => { + // Collapsing ['set'] to `write` would also grant add/incr/decr/update — + // more than the app asked for, so it stays spelled out. + expect(appDataPermissions(CONTACTS, { kv: ['set'] })).toEqual([ + `app-data:${CONTACTS}:kv:set`, + ]); + }); + + it('collapses every class of a store into the store name', () => { + expect( + appDataPermissions(CONTACTS, { kv: ['read', 'write', 'delete'] }), + ).toEqual([`app-data:${CONTACTS}:kv`]); + }); + + it('collapses both stores into the app-level name', () => { + expect( + appDataPermissions(CONTACTS, { + kv: ['read', 'write', 'delete'], + fs: ['read', 'write', 'delete'], + }), + ).toEqual([`app-data:${CONTACTS}`]); + }); + + it('keeps a mixed request per store', () => { + expect( + appDataPermissions(CONTACTS, { kv: ['get', 'set'], fs: 'read' }), + ).toEqual([ + `app-data:${CONTACTS}:fs:read`, + `app-data:${CONTACTS}:kv:get`, + `app-data:${CONTACTS}:kv:set`, + ]); + }); + + it('dedupes and sorts so the same request is always identical', () => { + const a = appDataPermissions(CONTACTS, { kv: ['set', 'get', 'set'] }); + const b = appDataPermissions(CONTACTS, { kv: ['get', 'set'] }); + expect(a).toEqual(b); + }); + + it('refuses flush, which no scope may reach', () => { + expect(() => appDataPermissions(CONTACTS, { kv: ['flush'] })).toThrow( + PuterJSError, + ); + }); + + it('refuses an unknown scope rather than sending it', () => { + expect(() => appDataPermissions(CONTACTS, { kv: ['nope'] })).toThrow( + /unknown kv scope/, + ); + expect(() => appDataPermissions(CONTACTS, { fs: ['append'] })).toThrow( + /unknown fs scope/, + ); + }); + + it('requires at least one scope', () => { + expect(() => appDataPermissions(CONTACTS, {})).toThrow(/at least one/); + }); +}); + +describe('perms requestAppData', () => { + it('resolves a name to a uid and prompts with the permission list', async () => { + const mod = makeModule(); + + const result = await requestAppData.call(mod, 'contacts', { + kv: ['get'], + }); + + expect(result).toBe(true); + expect(mod.puter.apps.get).toHaveBeenCalledWith('contacts'); + expect(mod.puter.ui.requestPermission).toHaveBeenCalledWith({ + permissions: [`app-data:${CONTACTS}:kv:get`], + }); + }); + + it('passes an app- prefixed identifier through without a lookup', async () => { + const mod = makeModule(); + await requestAppData.call(mod, CONTACTS, 'read'); + expect(mod.puter.apps.get).not.toHaveBeenCalled(); + }); + + it('accepts an object identifier', async () => { + const mod = makeModule(); + await requestAppData.call(mod, { uid: CONTACTS }, 'read'); + expect(mod.puter.ui.requestPermission).toHaveBeenCalled(); + }); + + it('expands a shorthand string to both stores', async () => { + const mod = makeModule(); + await requestAppData.call(mod, CONTACTS, 'read'); + expect(mod.puter.ui.requestPermission).toHaveBeenCalledWith({ + permissions: [ + `app-data:${CONTACTS}:fs:read`, + `app-data:${CONTACTS}:kv:read`, + ], + }); + }); + + it('accepts explicit store:name pairs', async () => { + const mod = makeModule(); + await requestAppData.call(mod, CONTACTS, ['kv:get', 'fs:write']); + expect(mod.puter.ui.requestPermission).toHaveBeenCalledWith({ + permissions: [ + `app-data:${CONTACTS}:fs:write`, + `app-data:${CONTACTS}:kv:get`, + ], + }); + }); + + it('short-circuits a request for its own data without prompting', async () => { + // An app already reaches its own namespace, so a prompt here would ask + // the user to approve something already true. + const mod = makeModule({ appID: CONTACTS }); + expect(await requestAppData.call(mod, CONTACTS, 'read')).toBe(true); + expect(mod.puter.ui.requestPermission).not.toHaveBeenCalled(); + }); + + it('returns false when the user denies', async () => { + const mod = makeModule({ granted: false }); + expect(await requestAppData.call(mod, CONTACTS, 'read')).toBe(false); + }); + + it('throws when the named app does not exist', async () => { + const mod = makeModule({ get: async () => null }); + await expect( + requestAppData.call(mod, 'no-such-app', 'read'), + ).rejects.toThrow(/app not found/); + }); + + it('treats an app *named* like a uid prefix as a name, not a uid', async () => { + // `puter.apps.get` looks up by name only, so a bare `app-` prefix test + // would send `app-store` to a uid lookup that can never match. + const mod = makeModule(); + await requestAppData.call(mod, 'app-store', 'read'); + expect(mod.puter.apps.get).toHaveBeenCalledWith('app-store'); + }); + + it('rejects a prototype key as an unknown store', async () => { + const mod = makeModule(); + // `out['toString']` is truthy, so a truthiness guard would fall through + // and throw a TypeError instead of a clean error. + await expect( + requestAppData.call(mod, CONTACTS, ['toString:read']), + ).rejects.toThrow(/unknown store/); + await expect( + requestAppData.call(mod, CONTACTS, ['constructor:read']), + ).rejects.toThrow(/unknown store/); + }); + + it('rejects a bad identifier and a bad scope before any IPC', async () => { + const mod = makeModule(); + await expect(requestAppData.call(mod, '', 'read')).rejects.toThrow( + PuterJSError, + ); + await expect( + requestAppData.call(mod, CONTACTS, { kv: ['flush'] }), + ).rejects.toThrow(PuterJSError); + expect(mod.puter.ui.requestPermission).not.toHaveBeenCalled(); + }); +}); diff --git a/src/puter-js/src/modules/perms/index.js b/src/puter-js/src/modules/perms/index.js index 3d7ac6f3d..6fee13c9a 100644 --- a/src/puter-js/src/modules/perms/index.js +++ b/src/puter-js/src/modules/perms/index.js @@ -1,4 +1,5 @@ import { PuterModule } from '../../lib/PuterModule.js'; +import { requestAppData } from './appData.js'; import { requestReadAppRootDir, requestWriteAppRootDir } from './appRootDir.js'; import { requestFolder_, @@ -35,6 +36,7 @@ const METHODS = [ 'requestReadPictures', 'requestWritePictures', 'requestReadVideos', 'requestWriteVideos', 'requestReadAppRootDir', 'requestWriteAppRootDir', + 'requestAppData', ]; /** @@ -87,6 +89,9 @@ export class PermsModule extends PuterModule { requestReadAppRootDir = requestReadAppRootDir; requestWriteAppRootDir = requestWriteAppRootDir; + // Another app's data (KV namespace + AppData directory) + requestAppData = requestAppData; + /** @param {Puter} puter */ constructor (puter) { super(puter); diff --git a/src/puter-js/tests/e2e/helpers/testApp.js b/src/puter-js/tests/e2e/helpers/testApp.js index d5ac0e6cc..9d4fbcedb 100644 --- a/src/puter-js/tests/e2e/helpers/testApp.js +++ b/src/puter-js/tests/e2e/helpers/testApp.js @@ -98,6 +98,51 @@ export async function registerTestApp (page, { fixtureURL = FIXTURE_URL } = {}) return appName; } +/** + * Registers an app that only exists to *own data* — the target of a cross-app + * request. Its `index_url` is never loaded, so no fixture is needed. + * + * `seed` entries are written into the target's own KV namespace from the GUI + * page, using the user-token override that is deliberately ungated. + * + * @returns {Promise<{ name: string, uid: string, title: string }>} + */ +export async function registerTargetApp (page, { title = 'Contacts', seed = {} } = {}) { + await page.goto('/'); + await waitForPuterReady(page); + + const appName = `puter-js-target-${randomUUID().slice(0, 8)}`; + const result = await page.evaluate( + async ({ name, appTitle, entries }) => { + try { + const app = await window.puter.apps.create( + name, + 'https://target.example.test/', + appTitle, + ); + for ( const [key, spec] of Object.entries(entries) ) { + await window.puter.kv.set(key, spec.value, { + appUuid: app.uid, + ...(spec.private ? { disableSharing: true } : {}), + }); + } + return { ok: true, app }; + } catch (e) { + return { ok: false, error: String(e?.message ?? e) }; + } + }, + { name: appName, appTitle: title, entries: seed }, + ); + if ( ! result.ok ) { + throw new Error(`registerTargetApp failed: ${result.error}`); + } + // The dialog labels the target by title, so the tests depend on it landing. + if ( result.app.title !== title ) { + throw new Error(`target app title is "${result.app.title}", expected "${title}"`); + } + return { name: appName, uid: result.app.uid, title }; +} + export async function deleteTestApp (page, appName) { if ( ! appName ) return; try { diff --git a/src/puter-js/tests/e2e/specs/requestAppData.spec.js b/src/puter-js/tests/e2e/specs/requestAppData.spec.js new file mode 100644 index 000000000..bab408072 --- /dev/null +++ b/src/puter-js/tests/e2e/specs/requestAppData.spec.js @@ -0,0 +1,293 @@ +import { test, expect } from '@playwright/test'; +import { + registerTestApp, + registerTargetApp, + deleteTestApp, + gotoTestApp, + FIXTURE_URL, +} from '../helpers/testApp.js'; + +const PERMISSION_FIXTURE_URL = FIXTURE_URL.replace( + 'menubar-contextmenu.html', + 'request-permission.html', +); + +const DIALOG = 'dialog.perm-dialog'; +const ROW = '.perm-dialog-permission'; + +/** + * Kick off a `requestAppData` call inside the app without awaiting it, so the + * test can act on the dialog it raises. Resolve with `settle()`. + */ +async function ask (appFrame, target, scopes) { + await appFrame.locator('body').evaluate( + (_el, { target: t, scopes: s }) => { + window.__appData = puter.perms.requestAppData(t, s).then( + v => ({ ok: true, value: v }), + e => ({ ok: false, error: String(e?.message ?? e) }), + ); + }, + { target, scopes }, + ); +} + +const settle = (appFrame) => + appFrame.locator('body').evaluate(() => window.__appData); + +/** Run a KV call in the app against the target's namespace. */ +const kv = (appFrame, target, method, args = {}) => + appFrame.locator('body').evaluate( + async (_el, { target: t, method: m, args: a }) => { + try { + const res = await puter.kv[m]({ + ...a, + optConfig: { appUuid: t }, + }); + return { ok: true, res }; + } catch (e) { + return { ok: false, error: String(e?.message ?? e) }; + } + }, + { target, method, args }, + ); + +test.describe('puter.perms.requestAppData (env=app)', () => { + test('deny then allow, and the grant reaches the target namespace', async ({ + page, + }) => { + const target = await registerTargetApp(page, { + title: 'Contacts', + seed: { + birthday: { value: 'March 3' }, + phone: { value: '555-0101' }, + oauthToken: { value: 'SECRET', private: true }, + }, + }); + const appName = await registerTestApp(page, { + fixtureURL: PERMISSION_FIXTURE_URL, + }); + try { + const appFrame = await gotoTestApp(page, appName); + const dialog = page.locator(DIALOG); + + // -- the prompt itself -- + await ask(appFrame, target.uid, { kv: ['get', 'list'] }); + await expect(dialog).toBeVisible(); + // Names the requesting app and the *target*, by title rather than uid. + await expect(dialog.locator('.perm-dialog-entity-name')).toContainText( + appName, + ); + await expect(dialog).toContainText('Contacts'); + await expect(dialog.locator(ROW)).toHaveCount(1); + // Encoded exactly once: `i18n()` encodes the whole interpolated + // string, so composing two calls renders a literal `'` here. + await expect(dialog).toContainText("'s saved data"); + await expect(dialog).not.toContainText('''); + + // -- deny -- + await dialog.locator('.perm-dialog-deny').click(); + expect(await settle(appFrame)).toEqual({ ok: true, value: false }); + await expect(dialog).toBeHidden(); + + // -- allow -- + await ask(appFrame, target.uid, { kv: ['get', 'list'] }); + await expect(dialog).toBeVisible(); + await dialog.locator('.perm-dialog-allow').click(); + expect(await settle(appFrame)).toEqual({ ok: true, value: true }); + + // -- the grant works, and per-entry privacy holds under it -- + expect(await kv(appFrame, target.uid, 'get', { key: 'birthday' })) + .toEqual({ ok: true, res: 'March 3' }); + // Private entries read as absent, not as a refusal. + expect(await kv(appFrame, target.uid, 'get', { key: 'oauthToken' })) + .toEqual({ ok: true, res: null }); + const listed = await kv(appFrame, target.uid, 'list', { as: 'keys' }); + expect(listed.res).toContain('birthday'); + expect(listed.res).not.toContain('oauthToken'); + + // -- a repeat request prompts again -- + // `requestAppData` does not consult existing grants before + // prompting, unlike `requestEmail` (checks whoami) and the folder + // helpers (stat first). Pinned as current behaviour: an app calling + // this on every launch re-asks the user. + await ask(appFrame, target.uid, { kv: ['get'] }); + await expect(dialog).toBeVisible(); + await dialog.locator('.perm-dialog-allow').click(); + expect(await settle(appFrame)).toEqual({ ok: true, value: true }); + } finally { + await deleteTestApp(page, appName); + await deleteTestApp(page, target.name); + } + }); + + test('one row per scope, and the wording matches the scope', async ({ + page, + }) => { + const target = await registerTargetApp(page, { title: 'Contacts' }); + const appName = await registerTestApp(page, { + fixtureURL: PERMISSION_FIXTURE_URL, + }); + try { + const appFrame = await gotoTestApp(page, appName); + const dialog = page.locator(DIALOG); + + // Both stores in one decision. + await ask(appFrame, target.uid, 'read'); + await expect(dialog.locator(ROW)).toHaveCount(2); + await expect(dialog).toContainText('saved data'); + await expect(dialog).toContainText('files'); + await dialog.locator('.perm-dialog-deny').click(); + await settle(appFrame); + + // A delete scope must say so — "change" would misdescribe it. + await ask(appFrame, target.uid, { kv: ['del'] }); + await expect(dialog).toContainText('delete'); + await dialog.locator('.perm-dialog-deny').click(); + await settle(appFrame); + + // `write` satisfies a read check via the exploder, so its wording + // names reading too. + await ask(appFrame, target.uid, { kv: ['set'] }); + await expect(dialog).toContainText('read and change'); + await dialog.locator('.perm-dialog-deny').click(); + await settle(appFrame); + + // Every class of a store collapses to one store-wide row, which + // covers deletion by prefix implication and must say so. + await ask(appFrame, target.uid, { kv: ['read', 'write', 'delete'] }); + await expect(dialog.locator(ROW)).toHaveCount(1); + await expect(dialog).toContainText('delete'); + await dialog.locator('.perm-dialog-deny').click(); + await settle(appFrame); + } finally { + await deleteTestApp(page, appName); + await deleteTestApp(page, target.name); + } + }); + + test('deleting an entry needs its own scope', async ({ page }) => { + const target = await registerTargetApp(page, { + title: 'Contacts', + seed: { phone: { value: '555-0101' } }, + }); + const appName = await registerTestApp(page, { + fixtureURL: PERMISSION_FIXTURE_URL, + }); + try { + const appFrame = await gotoTestApp(page, appName); + const dialog = page.locator(DIALOG); + + await ask(appFrame, target.uid, { kv: ['get', 'list'] }); + await dialog.locator('.perm-dialog-allow').click(); + await settle(appFrame); + + const refused = await kv(appFrame, target.uid, 'del', { + key: 'phone', + }); + expect(refused.ok).toBe(false); + + await ask(appFrame, target.uid, { kv: ['del'] }); + await dialog.locator('.perm-dialog-allow').click(); + await settle(appFrame); + + expect((await kv(appFrame, target.uid, 'del', { key: 'phone' })).ok) + .toBe(true); + } finally { + await deleteTestApp(page, appName); + await deleteTestApp(page, target.name); + } + }); + + test('requests that must never prompt resolve without a dialog', async ({ + page, + }) => { + const target = await registerTargetApp(page, { title: 'Contacts' }); + const appName = await registerTestApp(page, { + fixtureURL: PERMISSION_FIXTURE_URL, + }); + try { + const appFrame = await gotoTestApp(page, appName); + + // Its own data: already implicit, so approving it would mean nothing. + await appFrame.locator('body').evaluate(() => { + window.__appData = puter.perms + .requestAppData(puter.appID, 'read') + .then(v => ({ ok: true, value: v })); + }); + expect(await settle(appFrame)).toEqual({ ok: true, value: true }); + await expect(page.locator(DIALOG)).toHaveCount(0); + + // Over the transport cap. Asserted through `requestPermission` + // rather than `requestAppData`: the scope vocabulary has only 14 + // names and complete classes collapse, so the SDK helper can never + // produce a list this long. The cap protects the transport. + const overCap = await appFrame.locator('body').evaluate((_el, t) => + puter.ui.requestPermission({ + permissions: Array.from( + { length: 17 }, + (_x, i) => `app-data:${t}:kv:get${i}`, + ), + }), + target.uid); + expect(overCap).toBe(false); + await expect(page.locator(DIALOG)).toHaveCount(0); + + // Target opted out of sharing: no prompt, and the grant would 403. + await page.evaluate(async (name) => { + await window.puter.apps.update(name, { + metadata: { share_app_data: false }, + }); + }, target.name); + const appFrame2 = await gotoTestApp(page, appName); + await ask(appFrame2, target.uid, { kv: ['get'] }); + expect(await settle(appFrame2)).toEqual({ ok: true, value: false }); + await expect(page.locator(DIALOG)).toHaveCount(0); + } finally { + await deleteTestApp(page, appName); + await deleteTestApp(page, target.name); + } + }); + + test('invalid scopes are refused in the SDK before any prompt', async ({ + page, + }) => { + const target = await registerTargetApp(page, { title: 'Contacts' }); + const appName = await registerTestApp(page, { + fixtureURL: PERMISSION_FIXTURE_URL, + }); + try { + const appFrame = await gotoTestApp(page, appName); + + const outcomes = await appFrame.locator('body').evaluate( + async (_el, t) => { + const attempt = (scopes, id) => + puter.perms.requestAppData(t, scopes).then( + v => `${id}:resolved:${v}`, + e => `${id}:rejected`, + ); + return Promise.all([ + // Emptying a whole namespace is not grantable. + attempt({ kv: ['flush'] }, 'flush'), + // `out['toString']` is truthy, so a truthiness guard + // would throw a TypeError instead of a clean error. + attempt(['toString:read'], 'proto'), + attempt({ kv: ['nope'] }, 'unknown'), + attempt({}, 'empty'), + ]); + }, + target.uid, + ); + + expect(outcomes).toEqual([ + 'flush:rejected', + 'proto:rejected', + 'unknown:rejected', + 'empty:rejected', + ]); + await expect(page.locator(DIALOG)).toHaveCount(0); + } finally { + await deleteTestApp(page, appName); + await deleteTestApp(page, target.name); + } + }); +}); diff --git a/src/puter-js/types/modules/kv.d.ts b/src/puter-js/types/modules/kv.d.ts index 945373fa2..b3bbb7db5 100644 --- a/src/puter-js/types/modules/kv.d.ts +++ b/src/puter-js/types/modules/kv.d.ts @@ -143,7 +143,25 @@ export interface KVListPage { } export interface KVOptConfig { + /** + * Address another app's namespace instead of this app's own. Requires an + * `app-data::kv:` permission, which `puter.perms.requestAppData()` + * asks the user for. + */ appUuid?: string; + /** + * Mark the entry private to this app: invisible and untouchable to any other + * app the user later grants access to this namespace. + * + * Honoured by both forms of `set()` — one key, or a batch, where it marks + * every entry in the batch. `set` writes the whole entry, so writing the + * key again without the flag makes it shareable. Rejected when combined + * with `appUuid`, since only an entry's owner may mark it private. + * + * Use it for anything another app should never read, such as a cached OAuth + * token, since a user granting access cannot see what a namespace holds. + */ + disableSharing?: boolean; } /** diff --git a/src/puter-js/types/modules/perms.d.ts b/src/puter-js/types/modules/perms.d.ts index 98f962aec..7d2fbb50b 100644 --- a/src/puter-js/types/modules/perms.d.ts +++ b/src/puter-js/types/modules/perms.d.ts @@ -103,4 +103,71 @@ export class Perms { * @returns `true` if manage access was granted, `false` otherwise. */ requestManageSubdomains (): Promise; + + /** + * Request permission to use another app's data: its key-value namespace and + * its AppData directory, both scoped to the current user. + * + * Deleting entries is a separate scope from writing them, so request + * `delete` explicitly when the app needs to remove data it did not write. + * + * @param appIdentifier - The target app's uid, registered name, or an object + * carrying either. + * @param scopes - An access class applied to both stores, an array of + * `':'` pairs, or a per-store object. + * @returns `true` if the app may now use that data, `false` if denied. + */ + requestAppData ( + appIdentifier: string | { uid: string } | { name: string }, + scopes: AppDataScopes, + ): Promise; } + +/** The stores an `app-data` scope can name. */ +export type AppDataStore = 'kv' | 'fs'; + +/** + * The three access classes. `delete` is orthogonal to `write`: neither implies + * the other, so an app that only adds data cannot remove any. + */ +export type AppDataClass = 'read' | 'write' | 'delete'; + +/** + * A key-value scope: an access class, or one concrete operation. Classes are + * the coarser form — `read` covers `get`/`list`, `write` covers + * `set`/`add`/`incr`/`decr`/`update`, and `delete` covers + * `del`/`remove`/`expire`/`expireAt`. + * + * `flush` is deliberately absent — it empties a whole namespace and no scope + * reaches it. + */ +export type AppDataKvScope = + | AppDataClass + | 'get' | 'list' + | 'set' | 'add' | 'incr' | 'decr' | 'update' + | 'del' | 'remove' | 'expire' | 'expireAt'; + +/** + * A file scope. Classes only, with no per-operation form: ACL checks a mode, + * not an operation, so there is nothing finer to name. + */ +export type AppDataFsScope = AppDataClass; + +/** One `':'` pair, as the array form takes them. */ +export type AppDataScopePair = + | `kv:${AppDataKvScope}` + | `fs:${AppDataFsScope}`; + +/** + * What `requestAppData` accepts. A bare class applies to both stores; the array + * form spells out the store on every entry; the object form groups by store. + * There is no bare-name array — an entry with no store would be ambiguous + * between the two. + */ +export type AppDataScopes = + | AppDataClass + | AppDataScopePair[] + | { + kv?: AppDataKvScope | AppDataKvScope[], + fs?: AppDataFsScope | AppDataFsScope[], + };