fix(fs): preserve stored path spelling and app ACL identity (#4368)

This commit is contained in:
Daniel Salazar authored and GitHub committed 2026-10-26 22:04:36 -07:00
1 parent 7bccde40a8
commit 09da68424e
4 files changed
+185 -7

No files matched your search

@@ -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<string, unknown>,
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);
}
});
});
@@ -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<void> => {
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 });
@@ -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`,
);
});
});
+11 -5
View File
@@ -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<number, { id: number; uuid: string }>();
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;