From 09da68424efcf8a2cf3edf631a8e9d77f84f0a0d Mon Sep 17 00:00:00 2001 From: Daniel Salazar Date: Mon, 26 Oct 2026 22:04:36 -0700 Subject: [PATCH] fix(fs): preserve stored path spelling and app ACL identity (#4368) --- .../fs/LegacyFSController.appAcl.http.test.ts | 114 ++++++++++++++++++ .../controllers/fs/LegacyFSController.ts | 10 +- src/backend/stores/fs/FSEntryStore.test.ts | 52 ++++++++ src/backend/stores/fs/FSEntryStore.ts | 16 ++- 4 files changed, 185 insertions(+), 7 deletions(-) create mode 100644 src/backend/controllers/fs/LegacyFSController.appAcl.http.test.ts diff --git a/src/backend/controllers/fs/LegacyFSController.appAcl.http.test.ts b/src/backend/controllers/fs/LegacyFSController.appAcl.http.test.ts new file mode 100644 index 000000000..4d39b68c9 --- /dev/null +++ b/src/backend/controllers/fs/LegacyFSController.appAcl.http.test.ts @@ -0,0 +1,114 @@ +import { v4 as uuidv4 } from 'uuid'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { makeActor } from '../../core/actor.js'; +import { setupPuterTestEnv, type PuterTestEnv } from '../../testUtil.js'; + +describe('/auth/check-app-acl', () => { + let env: PuterTestEnv; + beforeAll(async () => { + env = await setupPuterTestEnv(); + }, 120_000); + afterAll(async () => { + await env?.shutdown(); + }); + + const post = (path: string, body: unknown, token: string) => + fetch(new URL(path, env.apiOrigin), { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Authorization: `Bearer ${token}`, + }, + body: JSON.stringify(body), + }); + + const makeDirAndApp = async () => { + const { username, token } = env.users.user; + const server = env.server; + const user = (await server.stores.user.getByUsername(username))!; + const path = `/${username}/Documents/cacl-${uuidv4()}`; + expect((await post('/fs/mkdir', { path }, token)).status).toBe(200); + const entry = (await server.stores.fsEntry.getEntryByPath(path))!; + + const app = await ( + server.stores.app.create as unknown as ( + f: Record, + o: { ownerUserId: number }, + ) => Promise<{ uid: string; id: number }> + )( + { + name: `cacl-${uuidv4()}`, + title: 'Check app acl', + index_url: `https://cacl-${uuidv4()}.test/`, + }, + { ownerUserId: user.id }, + ); + return { user, path, entry, app }; + }; + + it('allows a granted file without poisoning the app permission cache', async () => { + const { token } = env.users.user; + const server = env.server; + const { user, path, entry, app } = await makeDirAndApp(); + + const grant = await post( + '/auth/grant-user-app', + { app_uid: app.uid, permission: `fs:${entry.uuid}:read` }, + token, + ); + expect(grant.status).toBe(200); + + const res = await post( + '/auth/check-app-acl', + { subject: { path }, app: app.uid, mode: 'read' }, + token, + ); + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ allowed: true }); + + const asApp = makeActor({ user, app: { uid: app.uid, id: app.id } }); + const descriptor = () => server.services.acl.fsDescriptor(path); + expect( + await server.services.acl.check(asApp, descriptor(), 'read'), + ).toBe(true); + }); + + it('denies an app with no grant', async () => { + const { path, app } = await makeDirAndApp(); + const res = await post( + '/auth/check-app-acl', + { subject: { path }, app: app.uid, mode: 'read' }, + env.users.user.token, + ); + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ allowed: false }); + }); + + it('refuses app and access tokens', async () => { + const { token, apiToken } = env.users.user; + const { path, entry, app } = await makeDirAndApp(); + const grant = await post( + '/auth/grant-user-app', + { app_uid: app.uid, permission: `fs:${entry.uuid}:read` }, + token, + ); + expect(grant.status).toBe(200); + + const minted = await post( + '/auth/get-user-app-token', + { app_uid: app.uid }, + token, + ); + expect(minted.status).toBe(200); + const appToken = ((await minted.json()) as { token: string }).token; + + for (const credential of [appToken, apiToken]) { + const res = await post( + '/auth/check-app-acl', + { subject: { path }, app: app.uid, mode: 'read' }, + credential, + ); + expect(res.status).toBe(403); + } + }); +}); diff --git a/src/backend/controllers/fs/LegacyFSController.ts b/src/backend/controllers/fs/LegacyFSController.ts index eebb4d720..ca32b09df 100644 --- a/src/backend/controllers/fs/LegacyFSController.ts +++ b/src/backend/controllers/fs/LegacyFSController.ts @@ -26,6 +26,7 @@ import { assertResolvedActor, isAccessTokenActor, isAppActor, + isPlainUserActor, makeActor, } from '../../core/actor.js'; import { Context } from '../../core/context.js'; @@ -2041,6 +2042,11 @@ export class LegacyFSController extends PuterController { */ checkAppAcl = async (req: Request, res: Response): Promise => { const actor = this.#requireActor(req); + // Sessions only: for an app, this would be a window onto its neighbours' grants. + if (!isPlainUserActor(actor)) + throw new HttpError(403, 'actor must be a user', { + legacyCode: 'forbidden', + }); const body = asRecord(req.body); const subjectRef = body.subject; @@ -2071,7 +2077,7 @@ export class LegacyFSController extends PuterController { legacyCode: 'subject_does_not_exist', }); - let app: { uid: string } | null = null; + let app: { uid: string; id: number } | null = null; if (typeof appRef === 'string') { app = (await this.stores.app.getByUid(appRef)) ?? @@ -2085,7 +2091,7 @@ export class LegacyFSController extends PuterController { // Build an actor-under-user shape for the check. const actorForApp = makeActor({ user: actor.user, - app: { uid: (app as { uid: string }).uid }, + app: { uid: app.uid, id: app.id }, }); const allowed = await acl.check(actorForApp, descriptor, mode); res.json({ allowed }); diff --git a/src/backend/stores/fs/FSEntryStore.test.ts b/src/backend/stores/fs/FSEntryStore.test.ts index 4a8c7e9f4..383d763c5 100644 --- a/src/backend/stores/fs/FSEntryStore.test.ts +++ b/src/backend/stores/fs/FSEntryStore.test.ts @@ -3959,3 +3959,55 @@ describe('FSEntryStore subtree walks', () => { }); }); }); + +describe('FSEntryStore lineage backfill under a lenient collation', () => { + // MySQL's `name` collation matches some names the `path` collation + // doesn't (`ß` = `s` by name, not by path). SQLite compares both + // exactly, so the lineage read is handed the stored spelling instead. + const matchNamesLeniently = (alias: string, stored: string) => { + const db = server.clients.db; + const originalRead = (Object.getPrototypeOf(db) as typeof db).read; + return vi + .spyOn(db, 'read') + .mockImplementation(async (sql: string, params?: unknown[]) => { + const lenient = sql.includes('segments(depth, name)') + ? params?.map((p) => (p === alias ? stored : p)) + : params; + return originalRead.call(db, sql, lenient); + }); + }; + + it('writes the stored path, not the requested spelling', async () => { + const user = await makeUser(); + const desktop = (await store.getEntryByPath(`${user.home}/Desktop`))!; + await server.clients.redis.flushall?.(); + + const spy = matchNamesLeniently('Deßktop', 'Desktop'); + try { + const resolved = await store.getEntryByPath(`${user.home}/Deßktop`); + expect(resolved?.uuid).toBe(desktop.uuid); + expect(resolved?.path).toBe(`${user.home}/Desktop`); + } finally { + spy.mockRestore(); + } + await expect(pathOf(desktop.uuid)).resolves.toBe( + `${user.home}/Desktop`, + ); + }); + + it('heals a path-less row with its stored names', async () => { + const user = await makeUser(); + const desktop = (await store.getEntryByPath(`${user.home}/Desktop`))!; + await blankPath(desktop.uuid); + + const spy = matchNamesLeniently('Deßktop', 'Desktop'); + try { + await store.getEntryByPath(`${user.home}/Deßktop`); + } finally { + spy.mockRestore(); + } + await expect(pathOf(desktop.uuid)).resolves.toBe( + `${user.home}/Desktop`, + ); + }); +}); diff --git a/src/backend/stores/fs/FSEntryStore.ts b/src/backend/stores/fs/FSEntryStore.ts index 24c4677ea..4296e075d 100644 --- a/src/backend/stores/fs/FSEntryStore.ts +++ b/src/backend/stores/fs/FSEntryStore.ts @@ -1229,43 +1229,49 @@ export class FSEntryStore extends PuterStore { segments(depth, name) AS ( ${segmentRows} ), - lineage(id, uuid, depth) AS ( - SELECT f.id, f.uuid, 1 + lineage(id, uuid, name, depth) AS ( + SELECT f.id, f.uuid, f.name, 1 FROM fsentries f INNER JOIN segments s ON s.depth = 1 WHERE f.parent_uid IS NULL AND f.name = s.name UNION ALL - SELECT f.id, f.uuid, l.depth + 1 + SELECT f.id, f.uuid, f.name, l.depth + 1 FROM fsentries f INNER JOIN lineage l ON f.parent_uid = l.uuid INNER JOIN segments s ON s.depth = l.depth + 1 WHERE f.name = s.name ) - SELECT id, uuid, depth FROM lineage ORDER BY depth ASC, id ASC + SELECT id, uuid, name, depth FROM lineage ORDER BY depth ASC, id ASC `; const rows = (await this.clients.db.read(sql, segments)) as { id: number | string; uuid: string; + name: string; depth: number | string; }[]; // Take the lowest-id row at each depth (deterministic pick that // matches v1's `LIMIT 1` per-segment behaviour). const byDepth = new Map(); + const storedNames: string[] = []; for (const r of rows) { const depth = Number(r.depth); if (!byDepth.has(depth)) { byDepth.set(depth, { id: Number(r.id), uuid: r.uuid }); + storedNames[depth - 1] = r.name; } } if (byDepth.size !== segments.length) { return null; } - await this.#backfillLineagePaths(byDepth, segments); + // The stored names, not the caller's spelling: a collation can match + // a segment to a name spelled differently, and this runs before any + // access check. + await this.#backfillLineagePaths(byDepth, storedNames); const target = byDepth.get(segments.length); if (!target) return null;