From 73ebe3f47afa0223d903bfcfd0831e90ea753c00 Mon Sep 17 00:00:00 2001 From: Daniel Salazar Date: Sun, 4 Oct 2026 00:47:37 -0700 Subject: [PATCH] fix(metering): decide app-usage access in MeteringService only (PUT-2015) (#4061) The /metering/usage/:app handler repeated the "an app reads only its own usage" check while the service still let any app read the global bucket. The service now refuses an app actor anything but its own app, and the handler only resolves names. Status codes for app actors are unchanged; a user reading os-global gets their global usage instead of a name-lookup 404. whoami no longer sets taskbar_items to undefined for app actors, and only keys with a value count as built, so a listener can't fill one in for an app. getMonthlyUsage's JSDoc notes allowanceInfo is account-wide. --- extensions/metering.test.ts | 13 +++++++ extensions/metering.ts | 21 ++--------- extensions/whoami.test.ts | 24 +++++++++++-- extensions/whoami.ts | 35 +++++++++++-------- .../services/metering/MeteringService.test.ts | 15 ++++---- .../services/metering/MeteringService.ts | 8 ++--- src/puter-js/src/modules/Auth.js | 3 +- 7 files changed, 68 insertions(+), 51 deletions(-) diff --git a/extensions/metering.test.ts b/extensions/metering.test.ts index dfa8a1562..2586c96a6 100644 --- a/extensions/metering.test.ts +++ b/extensions/metering.test.ts @@ -283,6 +283,19 @@ describe('metering extension — handleMeteringUsageForApp', () => { expect((await appUsageAs(asUser, mine)).total).toBe(10); expect((await appUsageAs(asUser, other)).total).toBe(20); }); + + it('lets the user read their usage outside any app', async () => { + const { owner } = await seedApps(); + const asUser = makeActor({ user: owner }); + await server.services.metering.incrementUsage( + asUser, + 'kv:read', + 1, + 5, + ); + await server.stores.meteringBuffer.flushCycle(); + expect((await appUsageAs(asUser, 'os-global')).total).toBe(5); + }); }); }); diff --git a/extensions/metering.ts b/extensions/metering.ts index 7d4cd64dd..cbc7248ff 100644 --- a/extensions/metering.ts +++ b/extensions/metering.ts @@ -152,23 +152,9 @@ export const handleMeteringUsageForApp = async ( let appId = String(req.params.appIdOrName ?? ''); if (!appId) throw new HttpError(400, 'appId parameter is required'); - // An app may read its own usage only; refuse a uid before any lookup. - const ownAppId = actor.effectiveApp?.uid; - const refuseOtherApp = (uid: string) => { - if (ownAppId && uid !== ownAppId) { - throw new HttpError( - 403, - 'An app can only get usage details for itself', - { legacyCode: 'forbidden' }, - ); - } - }; - if (appId.startsWith('app-') || appId === GLOBAL_APP_KEY) { - refuseOtherApp(appId); - } - - // If not a UUID-shaped app UID, look up by name - if (!appId.startsWith('app-')) { + // If not a UUID-shaped app UID or the global sentinel, look up by name. + // Which apps an actor may read is MeteringService's call, not this route's. + if (!appId.startsWith('app-') && appId !== GLOBAL_APP_KEY) { const appRows = (await clients.db.read( 'SELECT `uid` FROM `apps` WHERE `name` = ? LIMIT 1', [appId], @@ -178,7 +164,6 @@ export const handleMeteringUsageForApp = async ( } else { throw new HttpError(404, 'App not found'); } - refuseOtherApp(appId); } const appUsage = diff --git a/extensions/whoami.test.ts b/extensions/whoami.test.ts index 4b7d58b73..3f42cf77e 100644 --- a/extensions/whoami.test.ts +++ b/extensions/whoami.test.ts @@ -298,15 +298,18 @@ describe('whoami extension — handleWhoami', () => { }); }; - const whoamiWithListener = async (actor: Actor) => { + const whoamiWithListener = async ( + actor: Actor, + listener: typeof addAccountDetails = addAccountDetails, + ) => { const { res, captured } = makeRes(); - server.clients.event.on('whoami.details', addAccountDetails); + server.clients.event.on('whoami.details', listener); try { await runWithContext({ actor }, () => handleWhoami(makeReq(), res), ); } finally { - server.clients.event.off('whoami.details', addAccountDetails); + server.clients.event.off('whoami.details', listener); } return captured.body as Record; }; @@ -363,6 +366,21 @@ describe('whoami extension — handleWhoami', () => { accountEligible: true, }); }); + + it('cannot add taskbar_items for an app actor, which core omits', async () => { + const user = await seedUser(); + const body = await whoamiWithListener( + makeActor({ + user: { uuid: user.uuid, id: user.id as number }, + app: { uid: 'app-test-actor' }, + }), + (_key, event) => { + event.details.taskbar_items = [{ name: 'injected' }]; + }, + ); + + expect(body).not.toHaveProperty('taskbar_items'); + }); }); it('redacts tmp_password from metadata for user actors', async () => { diff --git a/extensions/whoami.ts b/extensions/whoami.ts index 9fd7bdd26..7bd0b265f 100644 --- a/extensions/whoami.ts +++ b/extensions/whoami.ts @@ -193,19 +193,6 @@ export const handleWhoami = async ( // this endpoint is polled, and a mint is a write. referral_code: user.referral_code, oidc_only: oidcOnly, - taskbar_items: isUser - ? await getTaskbarItems( - user, - { - clients, - stores, - services, - apiBaseUrl: String(extension.config.api_base_url ?? ''), - config: extension.config, - }, - { iconSize, noIcons }, - ) - : undefined, otp: !!user.otp_enabled, feature_flags, created_ts: toUnixSeconds(user.timestamp), @@ -234,6 +221,21 @@ export const handleWhoami = async ( } } + // Taskbar items — only sent to user actors + if (isUser) { + details.taskbar_items = await getTaskbarItems( + user, + { + clients, + stores, + services, + apiBaseUrl: String(extension.config.api_base_url ?? ''), + config: extension.config, + }, + { iconSize, noIcons }, + ); + } + // Directories — only sent to user actors if (isUser) { const directories: Record = {}; @@ -299,7 +301,12 @@ export const handleWhoami = async ( details.app_name = app.uid; } - const builtKeys = isUser ? null : new Set(Object.keys(details)); + // A key core left undefined isn't built, so a listener can't fill it for an app. + const builtKeys = isUser + ? null + : new Set( + Object.keys(details).filter((key) => details[key] !== undefined), + ); try { await clients.event.emitAndWait( 'whoami.details', diff --git a/src/backend/services/metering/MeteringService.test.ts b/src/backend/services/metering/MeteringService.test.ts index 86a073df8..360c1c122 100644 --- a/src/backend/services/metering/MeteringService.test.ts +++ b/src/backend/services/metering/MeteringService.test.ts @@ -1364,20 +1364,17 @@ describe('MeteringService', () => { }); }); - it('allows an app actor to query the global namespace', async () => { - const userOnly: Actor = { user: makeUser() }; - await target.incrementUsage(userOnly, 'kv:read', 1, 60); + it('forbids an app actor from querying the global namespace', async () => { const appActor: Actor = resolveActor({ - user: userOnly.user, + user: makeUser(), app: { uid: 'my-app', id: 1 }, }); - await waitFor(async () => { - const r = await target.getActorCurrentMonthAppUsageDetails( + await expect( + target.getActorCurrentMonthAppUsageDetails( appActor, GLOBAL_APP_KEY, - ); - expect(r.total).toBe(60); - }); + ), + ).rejects.toMatchObject({ statusCode: 403 }); }); it('forbids an app actor from querying another app', async () => { diff --git a/src/backend/services/metering/MeteringService.ts b/src/backend/services/metering/MeteringService.ts index 1dc5c8c37..a257ae381 100644 --- a/src/backend/services/metering/MeteringService.ts +++ b/src/backend/services/metering/MeteringService.ts @@ -1511,14 +1511,10 @@ export class MeteringService extends PuterService { appId || actor.effectiveApp?.uid || GLOBAL_APP_KEY; const actorAppId = actor.effectiveApp?.uid; - if ( - actorAppId && - actorAppId !== resolvedAppId && - resolvedAppId !== GLOBAL_APP_KEY - ) { + if (actorAppId && actorAppId !== resolvedAppId) { throw new HttpError( 403, - 'Actor can only get usage details for their own app or global app', + 'Actor can only get usage details for their own app', { legacyCode: 'forbidden' }, ); } diff --git a/src/puter-js/src/modules/Auth.js b/src/puter-js/src/modules/Auth.js index 3a9f100e8..727c76646 100644 --- a/src/puter-js/src/modules/Auth.js +++ b/src/puter-js/src/modules/Auth.js @@ -468,7 +468,8 @@ export class AuthModule extends PuterModule { /** * The user's resource usage for the current month, scoped to the calling - * app. Amounts are in microcents ($0.01 = 1,000,000). + * app. `allowanceInfo` covers the whole account, not just the app. + * Amounts are in microcents ($0.01 = 1,000,000). * * @returns {Promise} */