From 18098dd71bea93fb850ddae6311007948d7a57c6 Mon Sep 17 00:00:00 2001 From: Daniel Salazar Date: Fri, 21 Aug 2026 01:30:19 -0400 Subject: [PATCH] fix(perms): answer the app-root-dir check without provisioning it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `/auth/request-app-root-dir` conflates two questions: may the caller claim its root directory, and where is it. The second provisions `AppData/` on first ask, so a caller that only wanted the first — `puter.perms.check('appRootDir')` — created a directory by asking about it. Adds `check: true`, which runs the same actor guard and stops at the answer. A caller that may not claim it still gets the 403, so the flag can't widen anything. Existing callers are unaffected: without it the route behaves exactly as before. Co-Authored-By: Claude Opus 5 (1M context) --- .../controllers/fs/LegacyFSController.test.ts | 44 +++++++++++++++++++ .../controllers/fs/LegacyFSController.ts | 23 ++++++---- 2 files changed, 58 insertions(+), 9 deletions(-) diff --git a/src/backend/controllers/fs/LegacyFSController.test.ts b/src/backend/controllers/fs/LegacyFSController.test.ts index 8969e7ac6..fc58d9fd3 100644 --- a/src/backend/controllers/fs/LegacyFSController.test.ts +++ b/src/backend/controllers/fs/LegacyFSController.test.ts @@ -2094,6 +2094,50 @@ describe('LegacyFSController.requestAppRootDir', () => { expect(body.path).toBe(`/${username}/AppData/${appUid}`); expect(body.is_dir).toBe(true); }); + + // `check: true` answers the question without acting on the answer, so that + // asking whether the access is held is not itself a filesystem write. + it('answers check:true without creating the directory', async () => { + const { actor } = await makeUser(); + const username = actor.user!.username!; + const appUid = 'app-check-only'; + const appActor = makeActor({ ...actor, app: { uid: appUid } }); + const rootPath = `/${username}/AppData/${appUid}`; + const { res, captured } = makeRes(); + + await withActor(appActor, () => + controller.requestAppRootDir( + makeReq({ + body: { app_uid: appUid, access: 'read', check: true }, + actor: appActor, + }), + res, + ), + ); + + expect(captured.body).toEqual({ allowed: true }); + expect( + await server.stores.fsEntry.getEntryByPath(rootPath), + ).toBeFalsy(); + }); + + // The guard runs first either way, so a caller that may not claim it is + // refused rather than told it is allowed. + it('still refuses check:true from a caller that is not the app', async () => { + const { actor } = await makeUser(); + const { res } = makeRes(); + await expect( + withActor(actor, () => + controller.requestAppRootDir( + makeReq({ + body: { app_uid: 'app-xyz', check: true }, + actor, + }), + res, + ), + ), + ).rejects.toMatchObject({ statusCode: 403 }); + }); }); // ── checkAppAcl ───────────────────────────────────────────────────── diff --git a/src/backend/controllers/fs/LegacyFSController.ts b/src/backend/controllers/fs/LegacyFSController.ts index 884ea350c..12c7025d5 100644 --- a/src/backend/controllers/fs/LegacyFSController.ts +++ b/src/backend/controllers/fs/LegacyFSController.ts @@ -832,9 +832,7 @@ export class LegacyFSController extends PuterController { // Trash, and `null`/`{}` when restoring. See // `src/gui/src/helpers.js` → `window.move_items`. newMetadata: (body.new_metadata ?? undefined) as - | Record - | null - | undefined, + Record | null | undefined, }); const oldPath = source.path; await this.#emitGuiEvent('outer.gui.item.moved', moved, { @@ -1281,8 +1279,7 @@ export class LegacyFSController extends PuterController { } type SignedOrEmpty = - | (SignedFile & { path?: string }) - | Record; + (SignedFile & { path?: string }) | Record; const result: { signatures: SignedOrEmpty[]; token?: string } = { signatures: [], }; @@ -1846,6 +1843,12 @@ export class LegacyFSController extends PuterController { /** * POST /auth/request-app-root-dir — an app-under-user requests stat on its * own app root directory. The app must own itself. + * + * `check: true` answers whether the caller may claim it and stops there, + * for callers asking the question rather than acting on the answer — + * otherwise `puter.perms.check('appRootDir', …)` would provision a + * directory just by being asked. Answering is the whole response: a caller + * that may not claim it gets the 403 below either way. */ requestAppRootDir = async (req: Request, res: Response): Promise => { const actor = this.#requireActor(req); @@ -1872,6 +1875,11 @@ export class LegacyFSController extends PuterController { legacyCode: 'unauthorized', }); + if (getBoolean(body, 'check')) { + res.json({ allowed: true }); + return; + } + const rootPath = `/${username}/AppData/${appUid}`; // Auto-create the AppData/ tree on first call. const entry = await this.services.fs.mkdir(userId, { @@ -1892,10 +1900,7 @@ export class LegacyFSController extends PuterController { const subjectRef = body.subject; const appRef = body.app; const mode = (getString(body, 'mode') ?? 'read') as - | 'see' - | 'list' - | 'read' - | 'write'; + 'see' | 'list' | 'read' | 'write'; if (!subjectRef || !appRef) throw new HttpError(400, '`subject` and `app` are required', { legacyCode: 'bad_request',