From 94d798ccce0db7c5f0d3c866a9164f83e2839609 Mon Sep 17 00:00:00 2001 From: Juan Castro Date: Fri, 28 Aug 2026 12:32:17 -0400 Subject: [PATCH] fix: close /share/blocks to app tokens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All three block routes set only `subdomain`, `requireVerified` and a rate limit, and the controller's actor check accepts anything carrying a user, so an app-under-user token passed. Unlike `share` and `revoke`, nothing else bounded them: an app the user authorized once could read exactly whom they had blocked, then clear the list — blanket switch and per-username — and reopen the channel. A block list is a personal safety control, not something an app can otherwise reach, so it falls outside the "an app can share what it can already reach" carve-out the other share routes rely on. `requireUserActor` on all three, and deliberately without `allowFullAccessToken`: this is security management, which stays closed to every access token, personal ones included. The only caller is the desktop's Blocked Senders window, which sends the user's own session token. Tests fail without the gate: an app token minted through /auth/get-user-app-token is refused on all three routes while the user's own session still succeeds, and a metadata check pins the gate on every block route so a fourth cannot quietly ship without it. --- .../share/ShareController.http.test.ts | 71 +++++++++++++++++++ .../controllers/share/ShareController.ts | 4 ++ 2 files changed, 75 insertions(+) diff --git a/src/backend/controllers/share/ShareController.http.test.ts b/src/backend/controllers/share/ShareController.http.test.ts index 45f9ceba2..7257fc727 100644 --- a/src/backend/controllers/share/ShareController.http.test.ts +++ b/src/backend/controllers/share/ShareController.http.test.ts @@ -19,6 +19,7 @@ import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; import { setupPuterTestEnv, type PuterTestEnv } from '../../testUtil.js'; +import { ShareController } from './ShareController.js'; /** * Route-level coverage for the sharing endpoints. The service unit tests drive @@ -578,4 +579,74 @@ describe('share endpoints over HTTP', () => { const body = (await res.json()) as { all: boolean; items: unknown[] }; expect(body).toEqual({ all: false, items: [] }); }); + + // Reading it says who the user avoids; clearing it puts them back in touch. + it('is closed to an app acting for the user', async () => { + const user = env.users.user; + const minted = await post('/auth/get-user-app-token', user.token, { + origin: 'https://blocks-probe.example', + }); + expect(minted.status).toBe(200); + const appToken = ((await minted.json()) as { token: string }).token; + expect(typeof appToken).toBe('string'); + + const asApp = [ + get('/share/blocks', appToken, {}), + post('/share/blocks', appToken, { all: true }), + fetch(new URL('/share/blocks', env.apiOrigin), { + method: 'DELETE', + headers: { + 'content-type': 'application/json', + authorization: `Bearer ${appToken}`, + }, + body: JSON.stringify({ all: true }), + }), + ]; + for (const res of await Promise.all(asApp)) { + expect(res.status).toBe(403); + expect(await res.json()).toMatchObject({ code: 'forbidden' }); + } + + // The same calls from the user's own session: the app is what is refused. + expect((await get('/share/blocks', user.token, {})).status).toBe(200); + expect( + (await post('/share/blocks', user.token, { all: true })).status, + ).toBe(200); + const lifted = await fetch(new URL('/share/blocks', env.apiOrigin), { + method: 'DELETE', + headers: { + 'content-type': 'application/json', + authorization: `Bearer ${user.token}`, + }, + body: JSON.stringify({ all: true }), + }); + expect(lifted.status).toBe(200); + }); + + // So a fourth block route cannot quietly ship without the gate. + it('gates every block route on a user session', () => { + const proto = ShareController.prototype as { + __puterRoutes?: Array<{ + method: string; + path: string; + options?: Record; + }>; + }; + const blocks = (proto.__puterRoutes ?? []).filter( + (r) => r.path === '/blocks', + ); + expect(blocks.map((r) => r.method.toLowerCase()).sort()).toEqual([ + 'delete', + 'get', + 'post', + ]); + for (const route of blocks) { + expect( + route.options?.requireUserActor, + `${route.method} /blocks`, + ).toBe(true); + // Security management stays closed to every access token. + expect(route.options?.allowFullAccessToken).toBeUndefined(); + } + }); }); diff --git a/src/backend/controllers/share/ShareController.ts b/src/backend/controllers/share/ShareController.ts index 8b2d2c441..0420cac59 100644 --- a/src/backend/controllers/share/ShareController.ts +++ b/src/backend/controllers/share/ShareController.ts @@ -311,6 +311,7 @@ export class ShareController extends PuterController { } // -- Blocking ----------------------------------------------------- + // User sessions only: a block list is a safety control, not an app's to touch. /** * GET /share/blocks — who the caller is refusing shares from, and whether @@ -318,6 +319,7 @@ export class ShareController extends PuterController { */ @Get('/blocks', { subdomain: 'api', + requireUserActor: true, requireVerified: true, rateLimit: SHARE_LIST_LIMIT, }) @@ -344,6 +346,7 @@ export class ShareController extends PuterController { */ @Post('/blocks', { subdomain: 'api', + requireUserActor: true, requireVerified: true, rateLimit: SHARE_LIMIT, }) @@ -371,6 +374,7 @@ export class ShareController extends PuterController { */ @Delete('/blocks', { subdomain: 'api', + requireUserActor: true, requireVerified: true, rateLimit: SHARE_LIMIT, })