From b451d05d1004407408dca0b364918062aaa9958f Mon Sep 17 00:00:00 2001 From: Daniel Salazar Date: Thu, 3 Sep 2026 19:43:27 -0700 Subject: [PATCH] feat: app-minted kv share handles (PUT-1688) (#3692) --- .../controllers/auth/AuthController.test.ts | 44 ++ .../controllers/auth/AuthController.ts | 9 +- .../controllers/events/EventsController.ts | 8 +- src/backend/services/events/EventsService.ts | 124 ++++- .../kvHandleDelegation.integration.test.ts | 430 ++++++++++++++++++ .../events/kvHandles.integration.test.ts | 4 +- src/backend/services/events/kvShares.test.ts | 40 ++ src/backend/services/events/kvShares.ts | 47 ++ .../services/permission/PermissionService.ts | 10 +- src/gui/src/UI/UIPermissionDialog.js | 36 ++ .../UI/UIPermissionDialog.rendering.test.js | 41 +- src/gui/src/UI/UIPermissionDialog.test.js | 56 ++- src/gui/src/i18n/translations/en.js | 1 + 13 files changed, 829 insertions(+), 21 deletions(-) create mode 100644 src/backend/services/events/kvHandleDelegation.integration.test.ts diff --git a/src/backend/controllers/auth/AuthController.test.ts b/src/backend/controllers/auth/AuthController.test.ts index ffd92854a..faba1e1f8 100644 --- a/src/backend/controllers/auth/AuthController.test.ts +++ b/src/backend/controllers/auth/AuthController.test.ts @@ -2053,6 +2053,50 @@ describe('AuthController grant flows', () => { expect(res.body).toEqual({}); }); + it('grant-user-app: refuses a key-value delegation over a whole namespace', async () => { + // The prompt for it would read "let this app show your app data to + // whoever it picks", which describes no bounded capability — so the + // request is refused where it is made, not after the user answers it. + const appName = `tkv-${uuidv4()}`; + const app = await server.stores.app.create( + { + name: appName, + title: 'TestKvShareApp', + index_url: `https://${appName}.example.test/index.html`, + }, + { ownerUserId: issuer.id }, + ); + const namespace = `manage:kv-share:${issuer.uuid}:${app.uid}`; + + await expect( + inCtx(issuerActor, () => + controller.handleGrantUserApp( + makeReq( + { app_uid: app.uid, permission: namespace }, + { actor: issuerActor }, + ), + makeRes(), + ), + ), + ).rejects.toMatchObject({ legacyCode: 'invalid_kv_share_prefix' }); + + // A region under it is exactly what the flow is for. + const res = makeRes(); + await inCtx(issuerActor, () => + controller.handleGrantUserApp( + makeReq( + { + app_uid: app.uid, + permission: `${namespace}:workspace:abc`, + }, + { actor: issuerActor }, + ), + res, + ), + ); + expect(res.body).toEqual({}); + }); + it('grant-user-app: 400 on a `permission` wider than the column it lands in', async () => { // 300 chars is under the 4096 input cap but over `varchar(255)`, so it // used to reach the INSERT and fault on MySQL/Postgres. The check runs diff --git a/src/backend/controllers/auth/AuthController.ts b/src/backend/controllers/auth/AuthController.ts index b90955cda..8c43883e4 100644 --- a/src/backend/controllers/auth/AuthController.ts +++ b/src/backend/controllers/auth/AuthController.ts @@ -74,7 +74,10 @@ import { generateDefaultFsentries, promoteToVerifiedGroup, } from '../../util/userProvisioning.js'; -import { isKvSharePermission } from '../../services/events/kvShares.js'; +import { + assertBoundedManageGrant, + isKvSharePermission, +} from '../../services/events/kvShares.js'; import { APP_DATA_PERMISSION_PREFIX, appDataSharingAllowed, @@ -3241,6 +3244,10 @@ export class AuthController extends PuterController { await this.services.permission.assertUserAppPermissionWritable( entry, ); + // A delegation over a whole key-value namespace has no bounded + // description, so it is refused where it is asked for rather than + // prompted for and then refused at use. + assertBoundedManageGrant(entry); await this.#prepareAppDataGrant(req.actor!, entry); } for (const entry of list) { diff --git a/src/backend/controllers/events/EventsController.ts b/src/backend/controllers/events/EventsController.ts index 9ccacffb3..bcbf03218 100644 --- a/src/backend/controllers/events/EventsController.ts +++ b/src/backend/controllers/events/EventsController.ts @@ -141,9 +141,11 @@ export class EventsController extends PuterController { * POST /events/kv-handles — hand another user a watchable region of this * account's key-value data. * - * An account session only: minting on behalf of an app is delegation, and - * the service refuses an app-context actor rather than the gate doing it, - * because `effectiveApp` is where app-ness actually lives. + * An account session, or an app session holding a `manage:` delegation on + * the region — the service tells those apart and gates each rather than the + * route doing it, because `effectiveApp` is where app-ness actually lives. + * A token an app minted is refused there too: a delegation is the app's to + * hold, not to pass on. A user's own token still acts for the user. */ @Post('/kv-handles', { subdomain: 'api', diff --git a/src/backend/services/events/EventsService.ts b/src/backend/services/events/EventsService.ts index a2fd779b7..9b823d81c 100644 --- a/src/backend/services/events/EventsService.ts +++ b/src/backend/services/events/EventsService.ts @@ -40,7 +40,12 @@ import { SUSPENDED_ROW_TTL_DAYS, type SubscriptionQuota, } from '../../controllers/events/limits.js'; -import { makeActor, type Actor } from '../../core/actor.js'; +import { + isAccessTokenActor, + makeActor, + userRelatedActor, + type Actor, +} from '../../core/actor.js'; import { HttpError, isHttpError } from '../../core/http/HttpError.js'; import { checkRateLimit } from '../../core/http/middleware/rateLimit.js'; import type { @@ -170,10 +175,13 @@ import { } from './registry.js'; import { SubscriptionCache } from './subscriptionCache.js'; import { + assertBoundedManageGrant, assertShareableAppUid, assertShareablePermission, assertShareablePrefix, kvShareGrantCovers, + kvShareManageNamespaceRoot, + kvShareManagePermission, kvShareOwnerImplicator, kvSharePermission, relativeToKvShareRoot, @@ -666,15 +674,46 @@ const handlerAppRequired = (): HttpError => * there: which apps exist is not this surface's to disclose. */ /** - * Minting is the user disposing of a region of their own data. An app doing it - * on their behalf is delegation, which is a `manage:` grant's job and a - * separate consent. + * Listing and revoking are the user's own view of what they have shared. An app + * doing it on their behalf has no surface of its own yet. */ const handleOwnerOnly = (): HttpError => new HttpError(403, 'Only an account session may manage share handles', { legacyCode: 'events_kv_handle_owner_only', }); +/** + * An app minting is delegation, and the `manage:` grant on the region is the + * consent for it. Without one there is nothing authorizing the app to dispose + * of its user's data, whatever the user themselves could do here. + */ +const handleNotDelegated = (): HttpError => + new HttpError( + 403, + 'This app has not been given permission to share this data', + { legacyCode: 'events_kv_handle_not_delegated' }, + ); + +/** + * A delegation is the app's to hold, not to pass on: a token it minted may + * carry the `manage:` permission and still not mint through it, the same way an + * access token is refused a socket of its own (`SocketService`). The user's own + * token is not this case — it acts for the user, who needs no delegation. + */ +const handleAccessTokenForbidden = (): HttpError => + new HttpError(403, 'An access token may not mint a share handle', { + legacyCode: 'forbidden', + }); + +/** + * An app addresses exactly one key-value namespace — its own, under its user — + * so that is the only one it can hand a region of out. + */ +const handleOutsideNamespace = (): HttpError => + new HttpError(403, 'An app may only share its own key-value data', { + legacyCode: 'events_kv_handle_outside_namespace', + }); + const handlerAppForbidden = (): HttpError => new HttpError(403, 'Only the app owner may publish its handlers', { legacyCode: 'events_handler_forbidden', @@ -1691,6 +1730,13 @@ export class EventsService extends PuterService { * never landed would be a name for nothing; a grant whose handle never * landed is unaddressable, since a handle is the only thing that can name * this family. + * + * An app may mint too, bounded the way sharing bounds an app handing out + * its user's files: authority is the user's, asked of the user behind the + * actor and never of the app, and reach is what the credential itself + * holds. Here reach is structural — an app addresses one namespace — so it + * is asserted rather than worked out, and the app's own consent to delegate + * is a `manage:` grant on the region. */ async mintKvHandle( actor: Actor, @@ -1702,28 +1748,34 @@ export class EventsService extends PuterService { const owner = actor.user; if (!owner?.uuid || owner.id === undefined) throw disabled(); // `undefined` is an app that could not be resolved, not the absence of - // one — reading it as an account session is what would hand an app the - // surface this refuses it. - if (actor.effectiveApp !== null) throw handleOwnerOnly(); + // one — reading it as an account session is what would hand an app a + // surface bounded on the app it is acting as. + const app = actor.effectiveApp; + if (app === undefined) throw handleOwnerOnly(); + // The budget is the user's, so an app spends its user's slots rather + // than a machine-rate allowance of its own. await this.#spendHandleBudget(owner.id); await this.#assertHandleCeiling(owner.id); const keyPrefix = assertShareablePrefix(request?.prefix); - const appUid = assertShareableAppUid( - parseAppUid(request?.appUid) ?? KV_GLOBAL_APP_KEY, - ); + const appUid = this.#mintNamespace(app, request); const grantee = await this.#resolveGrantee(request); const permission = assertShareablePermission( kvSharePermission(owner.uuid, appUid, keyPrefix), ); + if (app) await this.#assertKvShareDelegated(actor, permission); + + // The user behind the app, so the authority checked is theirs and the + // issuer recorded is them. Which app acted is carried separately: it + // belongs on the audit row, not in the grant's authority. await this.services.permission.grantUserUserPermission( - actor, + userRelatedActor(actor), grantee.username, permission, {}, - { reason: 'kv share handle' }, + { reason: 'kv share handle', appUid: app?.uid }, ); const row = await this.stores.kvShareHandle.mint({ @@ -1736,6 +1788,54 @@ export class EventsService extends PuterService { return { handle: row.handle, prefix: row.keyPrefix }; } + /** + * Which namespace a mint may name. An app reaches `v1::` + * and nothing else, so a request naming another one is refused rather than + * quietly minted somewhere the app cannot even write. + */ + #mintNamespace( + app: Actor['effectiveApp'], + request: MintKvHandleRequest, + ): string { + const named = parseAppUid(request?.appUid); + if (!app) return assertShareableAppUid(named ?? KV_GLOBAL_APP_KEY); + if (named !== null && named !== app.uid) throw handleOutsideNamespace(); + return app.uid; + } + + /** + * Whether this app was given the region to hand out. The `manage:` grant is + * the user's consent, and prefix implication makes one taken on a region + * cover the keys beneath it — so a handle deeper than the consent is still + * inside it, and one above it is not. + */ + async #assertKvShareDelegated( + actor: Actor, + permission: string, + ): Promise { + // Consequential enough to require the app's own session, whatever a + // token it minted happens to carry — the same posture already taken + // for an access token wanting a socket of its own (SocketService). + if (isAccessTokenActor(actor)) throw handleAccessTokenForbidden(); + // The consent surface refuses a namespace-root delegation because no + // prompt can describe it; refused here too, so a row written any other + // way cannot authorize a mint. + assertBoundedManageGrant(kvShareManagePermission(permission)); + const delegated = await this.services.permission.canManagePermission( + actor, + permission, + ); + if (!delegated) throw handleNotDelegated(); + // `canManagePermission` walks ancestors, so a namespace-root grant + // written by any path other than the consent surface would otherwise + // authorize a mint anywhere in the namespace, bounded region or not. + const unbounded = await this.services.permission.check( + actor, + kvShareManageNamespaceRoot(permission), + ); + if (unbounded) throw handleNotDelegated(); + } + /** * Take a shared region back. * diff --git a/src/backend/services/events/kvHandleDelegation.integration.test.ts b/src/backend/services/events/kvHandleDelegation.integration.test.ts new file mode 100644 index 000000000..a0750e00e --- /dev/null +++ b/src/backend/services/events/kvHandleDelegation.integration.test.ts @@ -0,0 +1,430 @@ +/* + * 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 . + */ + +/** + * An app minting a share handle on its user's data. + * + * The bounds are the ones sharing already puts on an app handing out its + * user's files: the authority is the user's, the consent is a `manage:` grant + * the user gave this app on this region, and the reach is whatever the + * credential structurally holds — for key-value that is one namespace. What + * these cases pin is that each of those is actually load-bearing, and that a + * handle minted this way is in every other respect an ordinary one. + */ + +import { v4 as uuidv4 } from 'uuid'; +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; +import { EVENTS_COALESCE_WINDOW_MS } from '../../controllers/events/limits.js'; +import { makeActor, type Actor } from '../../core/actor.js'; +import { runWithContext } from '../../core/context.js'; +import { setupPuterTestEnv, type PuterTestEnv } from '../../testUtil.js'; +import type { IConfig } from '../../types.js'; +import type { DeliveryEnvelope } from './EventsService.js'; +import { kvShareManagePermission, kvSharePermission } from './kvShares.js'; + +const BOOT_TIMEOUT_MS = 120_000; +const SOCKET_ID = 'kv-delegate-socket'; +const TABLE = 'event_subscriptions'; +const PREFIX = 'workspace:abc:'; + +interface TestUser { + actor: Actor; + username: string; + id: number; + uuid: string; +} + +let env: PuterTestEnv; +let owner: TestUser; +let guest: TestUser; +let appUid: string; +let appActor: Actor; + +const events = () => env.server.services.events; +const permissions = () => env.server.services.permission; + +const settled = (count = 1) => + vi.waitFor(() => expect(delivered.length).toBeGreaterThanOrEqual(count), { + timeout: EVENTS_COALESCE_WINDOW_MS * 12, + interval: 25, + }); + +const quiet = () => + new Promise((resolve) => + setTimeout(resolve, EVENTS_COALESCE_WINDOW_MS * 3), + ); + +let delivered: DeliveryEnvelope[]; + +const userFor = async (username: string): Promise => { + const row = await env.server.stores.user.getByUsername(username); + return { + actor: makeActor({ user: row as never }), + username, + id: row!.id, + uuid: row!.uuid as string, + }; +}; + +/** `kv.set` as the app makes it: its own namespace, under its user. */ +const appWrites = (key: string, value: unknown): Promise => + runWithContext({ actor: appActor }, () => + env.server.drivers.kvStore.set({ key, value }), + ); + +/** The consent: the user lets this app hand out one region of its data. */ +const delegate = (prefix = PREFIX) => + permissions().grantUserAppPermission( + owner.actor, + appUid, + kvShareManagePermission( + kvSharePermission(owner.uuid, appUid, prefix), + ), + ); + +const undelegate = (prefix = PREFIX) => + permissions().revokeUserAppPermission( + owner.actor, + appUid, + kvShareManagePermission( + kvSharePermission(owner.uuid, appUid, prefix), + ), + ); + +const mint = (request: Record = {}, actor = appActor) => + events().mintKvHandle(actor, { + granteeUsername: guest.username, + prefix: PREFIX, + ...request, + }); + +const clearRows = async () => { + await env.server.clients.db.write(`DELETE FROM \`${TABLE}\``, []); + for (const id of [owner.id, guest.id]) { + await events().reapSocket(id, SOCKET_ID); + events().invalidateUser(id); + await env.server.stores.eventSubscription.markRegionCold(id); + await env.server.stores.durableSubscription.warmRegion(id); + } + delivered.length = 0; +}; + +beforeAll(async () => { + env = await setupPuterTestEnv({ + events: { enabled: true, kvHandles: true }, + // Seeded accounts carry no email, which the plan machinery reads as a + // temporary account — and a temporary account holds no durable rows. + unlimitedMetering: true, + } as IConfig); + + owner = await userFor(env.users.user.username); + guest = await userFor(env.users.other.username); + + const name = `kv-delegate-${uuidv4().slice(0, 8)}`; + const app = await env.server.stores.app.create( + { + name, + title: 'Delegating App', + index_url: `https://${name}.example.test/index.html`, + }, + { ownerUserId: owner.id }, + ); + appUid = app.uid; + appActor = makeActor({ + user: owner.actor.user as never, + app: { uid: app.uid, id: app.id }, + }); + + delivered = []; + events().onDelivered = (envelope) => delivered.push(envelope); +}, BOOT_TIMEOUT_MS); + +afterAll(async () => { + await env?.shutdown(); +}); + +describe('an app minting without consent', () => { + it('is refused, however much its user could do here themselves', async () => { + await undelegate(); + await expect(mint()).rejects.toMatchObject({ + legacyCode: 'events_kv_handle_not_delegated', + }); + // The same call as the user is allowed: what is missing is the + // delegation, not the authority behind it. On its own region, since a + // session mints in the account's namespace rather than the app's. + await expect( + mint({ prefix: 'session-probe:' }, owner.actor), + ).resolves.toMatchObject({ prefix: 'session-probe:' }); + }); + + it('is refused for a region the consent does not cover', async () => { + await delegate('workspace:abc:'); + // A sibling region, and the parent the consent sits under: coverage + // only ever runs downward. + await expect(mint({ prefix: 'workspace:other:' })).rejects.toMatchObject( + { legacyCode: 'events_kv_handle_not_delegated' }, + ); + await expect(mint({ prefix: 'workspace:' })).rejects.toMatchObject({ + legacyCode: 'events_kv_handle_not_delegated', + }); + }); + + it('is refused when only a namespace-root grant exists, written some other way than the consent surface', async () => { + // The consent surface (grant-user-app) refuses to write this row; write + // it directly to prove the mint path does not trust one that reaches it + // by a different route. + await undelegate('workspace:abc:'); + await permissions().grantUserAppPermission( + owner.actor, + appUid, + kvShareManagePermission(kvSharePermission(owner.uuid, appUid, '')), + ); + try { + await expect( + mint({ prefix: 'totally-unrelated-region:' }), + ).rejects.toMatchObject({ + legacyCode: 'events_kv_handle_not_delegated', + }); + } finally { + await undelegate(''); + } + }); +}); + +describe('an access token', () => { + beforeAll(async () => { + await delegate(); + }); + + it('may not mint even when issued by an app the user has delegated to', async () => { + const tokenActor = makeActor({ + user: owner.actor.user as never, + accessToken: { + uid: `tok-${uuidv4()}`, + issuer: appActor, + authorized: null, + fullAccess: false, + }, + }); + + await expect(mint({}, tokenActor)).rejects.toMatchObject({ + legacyCode: 'forbidden', + }); + }); + + it('does not block a full-access token acting for its own user, on their own namespace', async () => { + const patActor = makeActor({ + user: owner.actor.user as never, + accessToken: { + uid: `tok-${uuidv4()}`, + issuer: owner.actor, + authorized: null, + fullAccess: true, + }, + }); + + await expect( + mint({ prefix: 'pat-probe:' }, patActor), + ).resolves.toMatchObject({ prefix: 'pat-probe:' }); + }); +}); + +describe('an app minting with consent', () => { + beforeAll(async () => { + await delegate(); + }); + + it('mints inside the region it was given, and beneath it', async () => { + await expect(mint()).resolves.toMatchObject({ prefix: PREFIX }); + // Prefix implication: one consent covers the keys under it. + await expect( + mint({ prefix: `${PREFIX}messages:` }), + ).resolves.toMatchObject({ prefix: `${PREFIX}messages:` }); + }); + + it('cannot name a namespace other than its own', async () => { + // The reach cap is structural — this app addresses `v1::` + // and nothing else — so naming another namespace is refused rather + // than minted somewhere the app cannot even write. + await expect( + mint({ appUid: 'os-global' }), + ).rejects.toMatchObject({ + legacyCode: 'events_kv_handle_outside_namespace', + }); + await expect( + mint({ appUid: `app-${uuidv4()}` }), + ).rejects.toMatchObject({ + legacyCode: 'events_kv_handle_outside_namespace', + }); + }); + + it('ignores a fabricated owner field — the owner is always the caller behind the app', async () => { + // The request has no field an owner could even be read from — `owner` + // is always `actor.user` — so a body naming someone else changes + // nothing about whose namespace or issuer identity the grant carries. + await mint({ + owner: guest.uuid, + ownerUuid: guest.uuid, + ownerUserUuid: guest.uuid, + userUuid: guest.uuid, + }); + + const rows = (await env.server.clients.db.pread( + 'SELECT `issuer_user_id` FROM `user_to_user_permissions` WHERE `permission` = ?', + [kvSharePermission(owner.uuid, appUid, PREFIX)], + )) as Array<{ issuer_user_id: number }>; + + expect(rows).toHaveLength(1); + expect(rows[0].issuer_user_id).toBe(owner.id); + + // No grant was ever written under the fabricated owner's namespace — + // checked as a row count, not `permissions().check()`, since a user + // asking about their own uuid's namespace always reads as "owns it" + // by construction (the owner shortcut), independent of any grant row. + const fabricated = (await env.server.clients.db.pread( + 'SELECT `holder_user_id` FROM `user_to_user_permissions` WHERE `permission` = ?', + [kvSharePermission(guest.uuid, appUid, PREFIX)], + )) as Array<{ holder_user_id: number }>; + expect(fabricated).toHaveLength(0); + }); + + it('grants in its own namespace, so the owner`s own region is untouched', async () => { + await mint(); + + await expect( + permissions().check( + guest.actor, + kvSharePermission(owner.uuid, appUid, PREFIX), + ), + ).resolves.toBe(true); + await expect( + permissions().check( + guest.actor, + kvSharePermission(owner.uuid, 'os-global', PREFIX), + ), + ).resolves.toBe(false); + }); + + it('records the owner as the issuer, not the app', async () => { + await mint(); + const rows = (await env.server.clients.db.pread( + 'SELECT `issuer_user_id`, `holder_user_id` FROM `user_to_user_permissions` WHERE `permission` = ?', + [kvSharePermission(owner.uuid, appUid, PREFIX)], + )) as Array<{ issuer_user_id: number; holder_user_id: number }>; + + expect(rows).toHaveLength(1); + expect(rows[0].issuer_user_id).toBe(owner.id); + expect(rows[0].holder_user_id).toBe(guest.id); + }); + + it('leaves an audit row naming the user and the app that acted', async () => { + const prefix = `audit-probe-${uuidv4().slice(0, 8)}:`; + await delegate(prefix); + await mint({ prefix }); + + const row = await vi.waitFor(async () => { + const [found] = (await env.server.clients.db.pread( + 'SELECT `issuer_user_id`, `holder_user_id`, `extra` FROM `audit_user_to_user_permissions` ' + + 'WHERE `permission` = ? AND `action` = ?', + [kvSharePermission(owner.uuid, appUid, prefix), 'grant'], + )) as Array<{ + issuer_user_id: number; + holder_user_id: number; + extra: unknown; + }>; + expect(found).toBeDefined(); + return found; + }); + + expect(row.issuer_user_id).toBe(owner.id); + expect(row.holder_user_id).toBe(guest.id); + const extra = + typeof row.extra === 'string' ? JSON.parse(row.extra) : row.extra; + expect(extra).toMatchObject({ appUid }); + }); + + it('shows up in the owner`s listing of what they have shared out', async () => { + const { handle } = await mint(); + const page = await events().listKvHandles(owner.actor, { limit: 100 }); + + expect(page.items).toContainEqual( + expect.objectContaining({ + handle, + prefix: PREFIX, + appUid, + granteeUsername: guest.username, + revokedAt: null, + }), + ); + }); +}); + +describe('a handle an app minted', () => { + beforeAll(async () => { + await delegate(); + }); + + it('delivers the app`s writes to the grantee', async () => { + await clearRows(); + const { handle } = await mint(); + const { sub } = await events().subscribe(guest.actor, SOCKET_ID, { + subject: `kv:${handle}:*`, + }); + delivered.length = 0; + + await appWrites(`${PREFIX}messages:1`, { body: 'hello' }); + await settled(); + + expect(delivered).toHaveLength(1); + expect(delivered[0].subId).toBe(sub.subId); + // Relative to the handle, the same as an owner-minted one. + expect(delivered[0].event).toMatchObject({ + subject: `kv:${handle}:messages:1`, + key: 'messages:1', + op: 'set', + }); + }); + + it('settles on revocation exactly as an owner-minted one does', async () => { + await clearRows(); + const { handle } = await mint(); + const { sub } = await events().subscribeDurable(guest.actor, { + subject: `kv:${handle}:*`, + delivery: 'single', + handlerName: 'onChange', + }); + + await events().revokeKvHandle(owner.actor, handle); + await vi.waitFor( + async () => { + const [row] = (await env.server.clients.db.pread( + `SELECT \`suspended_reason\` FROM \`${TABLE}\` WHERE \`sub_id\` = ?`, + [sub.subId], + )) as Array<{ suspended_reason: unknown }>; + expect(row?.suspended_reason).toBe('permission_revoked'); + }, + { timeout: 5_000, interval: 25 }, + ); + + delivered.length = 0; + await appWrites(`${PREFIX}messages:2`, { body: 'after' }); + await quiet(); + expect(delivered).toEqual([]); + }); +}); diff --git a/src/backend/services/events/kvHandles.integration.test.ts b/src/backend/services/events/kvHandles.integration.test.ts index aac35d66b..062478cd2 100644 --- a/src/backend/services/events/kvHandles.integration.test.ts +++ b/src/backend/services/events/kvHandles.integration.test.ts @@ -268,7 +268,7 @@ describe('minting a handle', () => { } }); - it('refuses an app minting on its user`s behalf', async () => { + it('refuses an app its user has not delegated the region to', async () => { const uid = `app-${uuidv4()}`; await env.server.clients.db.write( 'INSERT INTO `apps` (`uid`, `name`, `title`, `index_url`, `owner_user_id`) VALUES (?, ?, ?, ?, ?)', @@ -281,7 +281,7 @@ describe('minting a handle', () => { }); await expect(mint({}, appActor)).rejects.toMatchObject({ - legacyCode: 'events_kv_handle_owner_only', + legacyCode: 'events_kv_handle_not_delegated', }); }); }); diff --git a/src/backend/services/events/kvShares.test.ts b/src/backend/services/events/kvShares.test.ts index 996471c52..49a0dae2b 100644 --- a/src/backend/services/events/kvShares.test.ts +++ b/src/backend/services/events/kvShares.test.ts @@ -22,11 +22,13 @@ import type { Actor } from '../../core/actor.js'; import { HttpError } from '../../core/http/HttpError.js'; import { PermissionUtil } from '../permission/permissionUtil.js'; import { + assertBoundedManageGrant, assertShareableAppUid, assertShareablePermission, assertShareablePrefix, keyPrefixSegments, kvShareGrantCovers, + kvShareManagePermission, kvShareOwnerImplicator, kvSharePermission, mintKvHandleId, @@ -195,6 +197,44 @@ describe('keys inside a granted region', () => { }); }); +describe('a delegation request', () => { + const region = kvShareManagePermission( + kvSharePermission(OWNER, APP, 'workspace:abc:'), + ); + + it('is the manage arm of the grant it would let an app issue', () => { + expect(region).toBe(`manage:kv-share:${OWNER}:${APP}:workspace:abc`); + expect(assertBoundedManageGrant(region)).toBe(region); + }); + + it.each([ + ['manage:kv-share', 'the family itself'], + [`manage:kv-share:${OWNER}`, 'an owner and no namespace'], + [`manage:kv-share:${OWNER}:${APP}`, 'a whole namespace'], + ])('refuses %s (%s)', (permission) => { + let thrown: unknown; + try { + assertBoundedManageGrant(permission); + } catch (err) { + thrown = err; + } + expect(thrown).toBeInstanceOf(HttpError); + expect((thrown as HttpError).legacyCode).toBe( + 'invalid_kv_share_prefix', + ); + }); + + it('leaves permissions from other families alone', () => { + const unrelated = `manage:fs:${OWNER}:write`; + expect(assertBoundedManageGrant(unrelated)).toBe(unrelated); + // The read arm of this family is not a delegation, so it is not this + // check's to bound. + expect(assertBoundedManageGrant(`kv-share:${OWNER}:${APP}`)).toBeTypeOf( + 'string', + ); + }); +}); + describe('handle ids', () => { it('are told apart from an app uid in the same slot', () => { const handle = mintKvHandleId(); diff --git a/src/backend/services/events/kvShares.ts b/src/backend/services/events/kvShares.ts index dfaf76e86..75b035938 100644 --- a/src/backend/services/events/kvShares.ts +++ b/src/backend/services/events/kvShares.ts @@ -184,6 +184,53 @@ export const assertShareablePermission = (permission: string): string => { return permission; }; +// -- Delegation ------------------------------------------------------- + +/** The delegation arm of a share grant: authority to hand the region out. */ +export const kvShareManagePermission = (permission: string): string => + PermissionUtil.join( + MANAGE_PERM_PREFIX, + ...PermissionUtil.split(permission), + ); + +export const KV_SHARE_MANAGE_PREFIX = `${MANAGE_PERM_PREFIX}:${KV_SHARE_PERMISSION_PREFIX}`; + +export const isKvShareManagePermission = (permission: string): boolean => + permission === KV_SHARE_MANAGE_PREFIX || + permission.startsWith(`${KV_SHARE_MANAGE_PREFIX}:`); + +/** + * A delegation must name a subtree. Consent on the namespace root reads as "let + * this app hand out anything it has stored for you", which is not a bounded + * capability and so is not something a prompt can put to a user. + */ +export const assertBoundedManageGrant = (permission: string): string => { + if (!isKvShareManagePermission(permission)) return permission; + const [, , owner, appUid, ...segments] = PermissionUtil.split(permission); + if (!owner || !appUid || segments.length === 0) + throw invalidPrefix( + 'A share delegation must name a region, not the whole namespace', + ); + return permission; +}; + +/** + * The unbounded `manage:` grant a bounded delegation's own permission descends + * from. The consent surface refuses to ever write this row, but + * `canManagePermission` walks ancestors to decide "may I delegate this", so if + * it exists by any other path it would silently authorize a mint anywhere in + * the namespace — this is what lets the mint path notice it regardless. + */ +export const kvShareManageNamespaceRoot = (permission: string): string => { + const [, owner, appUid] = PermissionUtil.split(permission); + return PermissionUtil.join( + MANAGE_PERM_PREFIX, + KV_SHARE_PERMISSION_PREFIX, + owner, + appUid, + ); +}; + // -- Permission rules ------------------------------------------------- /** diff --git a/src/backend/services/permission/PermissionService.ts b/src/backend/services/permission/PermissionService.ts index 10d2b7d44..806d3a5f6 100644 --- a/src/backend/services/permission/PermissionService.ts +++ b/src/backend/services/permission/PermissionService.ts @@ -59,6 +59,12 @@ export interface ScanState { export interface GrantMeta { reason?: string; + /** + * The app that acted, when a grant is issued programmatically on its user's + * behalf. The issuer is still the user — this is what makes the audit trail + * able to say which app it was. + */ + appUid?: string | null; } /** @@ -864,7 +870,9 @@ export class PermissionService extends PuterService { permission, action: 'grant', reason: meta.reason ?? 'granted via PermissionService', - extra: this.#auditActorContext(actor), + extra: meta.appUid + ? { appUid: meta.appUid } + : this.#auditActorContext(actor), }) .catch((err) => { console.warn( diff --git a/src/gui/src/UI/UIPermissionDialog.js b/src/gui/src/UI/UIPermissionDialog.js index bc4ac639f..a3b6e0df9 100644 --- a/src/gui/src/UI/UIPermissionDialog.js +++ b/src/gui/src/UI/UIPermissionDialog.js @@ -646,6 +646,10 @@ async function get_permission_description (permission, options = {}) { return await get_app_data_description(parts, options); } + if ( parts[0] === 'manage' && parts[1] === 'kv-share' ) { + return await get_kv_share_description(parts, options); + } + if ( parts[0] === 'events' && parts[1] === 'background' ) { return { html: i18n('perm_events_background'), icon: 'zap' }; } @@ -758,6 +762,38 @@ export async function get_app_data_description (parts, options) { }; } +/** + * Describes `manage:kv-share:::` — the app handing a + * region of the data it keeps for this user to other people it picks. + * + * Returns null (which denies without prompting) for anything this copy cannot + * honestly bound: another user's data, a namespace that is not the requester's + * own, or a request naming no region — that last one is the whole of the app's + * data, which is a different decision and not one a prompt can put in a line. + */ +export async function get_kv_share_description (parts, options) { + const [, , owner_uuid, namespace_app_uid, ...segments] = parts; + if ( ! owner_uuid || ! namespace_app_uid || segments.length === 0 ) return null; + + // An app reaches its own namespace and no other, so a request naming + // another one describes access it could not use. + if ( ! options.app_uid || namespace_app_uid !== options.app_uid ) return null; + + const whoami = await puter.auth.whoami(); + if ( whoami.uuid !== owner_uuid ) return null; + + const app = await get_app_by_uid(namespace_app_uid); + if ( ! app ) return null; + + return { + html: i18n('perm_kv_share_manage', { + app: app.title || app.name || options.app_name, + region: `${segments.join(':')}:`, + }), + icon: '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 index 4f1f01bcf..eb259858c 100644 --- a/src/gui/src/UI/UIPermissionDialog.rendering.test.js +++ b/src/gui/src/UI/UIPermissionDialog.rendering.test.js @@ -12,11 +12,14 @@ window.auth_token = 'tok'; globalThis.html_encode = (str) => encode(str); let get_app_data_description; +let get_kv_share_description; beforeAll(async () => { await import('../i18n/i18n.js'); // installs window.i18n globalThis.i18n = window.i18n; - ({ get_app_data_description } = await import('./UIPermissionDialog.js')); + ({ get_app_data_description, get_kv_share_description } = await import( + './UIPermissionDialog.js' + )); }); const CONTACTS = 'app-contacts'; @@ -89,3 +92,39 @@ describe('UIPermissionDialog app-data rendering', () => { stubApp({ uid: CONTACTS, title: 'Contacts' }); }); }); + +describe('UIPermissionDialog key-value delegation rendering', () => { + const OWNER = '2a1b0c9d-0000-4000-8000-000000000001'; + + const renderShare = async (permission) => { + const d = await get_kv_share_description(permission.split(':'), { + app_uid: CONTACTS, + }); + return d.html; + }; + + beforeAll(() => { + globalThis.puter = { auth: { whoami: async () => ({ uuid: OWNER }) } }; + }); + + // The region is a key prefix an app names in its own `grant-user-app` + // request — attacker-controlled the same way an app title is. + it('escapes a hostile region exactly once', async () => { + stubApp({ uid: CONTACTS, title: 'Contacts' }); + const html = await renderShare( + `manage:kv-share:${OWNER}:${CONTACTS}:`, + ); + expect(html).not.toContain(' { + stubApp({ uid: CONTACTS, title: '' }); + const html = await renderShare( + `manage:kv-share:${OWNER}:${CONTACTS}:workspace:abc`, + ); + expect(html).not.toContain(' `${key}(${Object.entries(params).map(([k, v]) => `${k}=${v}`).join(',')})`; -const { get_app_data_description } = await import('./UIPermissionDialog.js'); +const { get_app_data_description, get_kv_share_description } = + await import('./UIPermissionDialog.js'); const CONTACTS = 'app-contacts'; const CALENDAR = 'app-calendar'; +const OWNER = '2a1b0c9d-0000-4000-8000-000000000001'; /** Stub the app lookup the describer performs. */ const stubApp = (app) => { @@ -104,3 +106,55 @@ describe('UIPermissionDialog app-data descriptions', () => { expect(await describeScope(`app-data:${CONTACTS}:kv:get`)).toBeNull(); }); }); + +describe('UIPermissionDialog key-value delegation descriptions', () => { + beforeEach(() => { + stubApp({ uid: CALENDAR, name: 'calendar', title: 'Calendar' }); + globalThis.puter = { auth: { whoami: async () => ({ uuid: OWNER }) } }; + }); + + const describeShare = (permission, options = { app_uid: CALENDAR }) => + get_kv_share_description(permission.split(':'), options); + + it('names the app and the region, never the namespace', async () => { + const d = await describeShare( + `manage:kv-share:${OWNER}:${CALENDAR}:workspace:abc`, + ); + expect(d.html).toContain('perm_kv_share_manage'); + expect(d.html).toContain('Calendar'); + expect(d.html).toContain('region=workspace:abc:'); + }); + + it('refuses a delegation naming no region', async () => { + // The whole of the app's data: a different decision, and not one this + // line can put to the user. + expect( + await describeShare(`manage:kv-share:${OWNER}:${CALENDAR}`), + ).toBeNull(); + expect(await describeShare(`manage:kv-share:${OWNER}`)).toBeNull(); + expect(await describeShare('manage:kv-share')).toBeNull(); + }); + + it('refuses a namespace that is not the requester’s own', async () => { + expect( + await describeShare( + `manage:kv-share:${OWNER}:${CONTACTS}:workspace:abc`, + ), + ).toBeNull(); + // No requesting app at all — the popup flow — cannot be bounded either. + expect( + await describeShare( + `manage:kv-share:${OWNER}:${CALENDAR}:workspace:abc`, + { origin: 'https://site.example' }, + ), + ).toBeNull(); + }); + + it('refuses another user’s data', async () => { + expect( + await describeShare( + `manage:kv-share:someone-else:${CALENDAR}:workspace:abc`, + ), + ).toBeNull(); + }); +}); diff --git a/src/gui/src/i18n/translations/en.js b/src/gui/src/i18n/translations/en.js index 97ed37c88..1602f1e1f 100644 --- a/src/gui/src/i18n/translations/en.js +++ b/src/gui/src/i18n/translations/en.js @@ -772,6 +772,7 @@ const en = { '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_kv_share_manage': "share the {{region}} section of {{app}}'s data in your account with other Puter users it picks.", 'perm_events_background': 'run in the background when your files or data change, even while it is closed', 'perm_dialog_wants_to': 'wants permission to', 'perm_dialog_footnote': 'You can change this anytime in Settings.',