From 226297578525156d5e2549ecaa3ef39e301cc8cc Mon Sep 17 00:00:00 2001 From: Daniel Salazar Date: Sat, 25 Jul 2026 18:06:50 -0700 Subject: [PATCH] fix: readdir response type (#3449) --- src/backend/clients/event/types.ts | 47 +-- .../controllers/fs/FSController.http.test.ts | 229 ++++++++++++++ .../controllers/fs/FSController.test.ts | 283 +++++++++++++++--- src/backend/controllers/fs/FSController.ts | 240 ++++++++++----- src/backend/controllers/fs/requestTypes.ts | 122 +++++++- 5 files changed, 776 insertions(+), 145 deletions(-) create mode 100644 src/backend/controllers/fs/FSController.http.test.ts diff --git a/src/backend/clients/event/types.ts b/src/backend/clients/event/types.ts index d17d0917d..dc0bfa1dc 100644 --- a/src/backend/clients/event/types.ts +++ b/src/backend/clients/event/types.ts @@ -3,15 +3,16 @@ * * Domain objects (User, Actor, FSEntry, Socket, ...) are typed as `unknown` * here on purpose — pulling their real types in would couple this module to - * most of the backend and risk import cycles. Refine at the listener if - * you need narrowed access. + * most of the backend and risk import cycles. Refine at the listener if you + * need narrowed access. * * Conventions: - * - `*.validate` events carry an `allow` flag listeners flip to reject; - * they tend to grow listener-specific fields, so they accept extras via - * an index signature. - * - `outer.gui.*` events ride the `{ user_id_list, response }` envelope - * the SocketService fans out to user-scoped channels. + * + * - `*.validate` events carry an `allow` flag listeners flip to reject; they tend + * to grow listener-specific fields, so they accept extras via an index + * signature. + * - `outer.gui.*` events ride the `{ user_id_list, response }` envelope the + * SocketService fans out to user-scoped channels. */ import type { @@ -148,13 +149,16 @@ export type EventMap = { trail?: Array; /** * Set by the abuse harness for flagged signups — the id under which the - * decision trail is persisted to KV (`abuse:trail:`), shared back on - * the request for log / support correlation. + * decision trail is persisted to KV (`abuse:trail:`), shared back + * on the request for log / support correlation. */ trail_id?: string; /** Device signal forwarded verbatim from the signup request body. */ fingerprint?: string | null; - /** Set by the abuse harness — require SMS phone verification post-signup. */ + /** + * Set by the abuse harness — require SMS phone verification + * post-signup. + */ requires_phone_verification?: boolean; /** Set by the abuse harness — require card verification post-signup. */ requires_card_verification?: boolean; @@ -334,9 +338,13 @@ export type EventMap = { }; // ---- Thumbnails ---- + // The listener rewrites `thumbnail` in place (an s3:// key or legacy + // https:// URL becomes a signed URL) and falls back to `uuid`/`uid` to + // migrate inline data-URL thumbnails. Declared to match: the previous + // `uri` member was never emitted nor read. 'thumbnail.read': { - uri: string; - size?: string; + uuid?: string; + uid?: string; thumbnail?: string | null; }; 'thumbnail.created': { url: string }; @@ -406,9 +414,9 @@ export type LifecyclePhase = 'before' | 'after' | 'error' | 'reject'; /** * Payload for `driver...` events. * - * One shape across all phases; read `phase` (or the key suffix) to - * branch. `allow`/`rejectReason` are only meaningful on the `before` phase - * (emitted via `emitAndWait`). + * One shape across all phases; read `phase` (or the key suffix) to branch. + * `allow`/`rejectReason` are only meaningful on the `before` phase (emitted via + * `emitAndWait`). */ export type DriverMethodLifecycleEvent = { phase: LifecyclePhase; @@ -474,11 +482,10 @@ export type EventKey = keyof EventMap & string; // Generates a wildcard for every non-final dot-separated prefix of K. export type WildcardPrefixes = K extends `${infer Head}.${infer Tail}` - ? - | `${Head}.*` - | (Tail extends `${string}.${string}` - ? `${Head}.${WildcardPrefixes}` - : never) + ? | `${Head}.*` + | (Tail extends `${string}.${string}` + ? `${Head}.${WildcardPrefixes}` + : never) : never; export type ListenKey = EventKey | WildcardPrefixes; diff --git a/src/backend/controllers/fs/FSController.http.test.ts b/src/backend/controllers/fs/FSController.http.test.ts new file mode 100644 index 000000000..7fe753604 --- /dev/null +++ b/src/backend/controllers/fs/FSController.http.test.ts @@ -0,0 +1,229 @@ +/** + * Copyright (C) 2024-present Puter Technologies Inc. + * + * This file is part of Puter. + * + * Puter is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { setupPuterTestEnv, type PuterTestEnv } from '../../testUtil.js'; + +/** + * Route-level coverage for `GET /fs/readdir`. The unit tests call the handler + * directly, which cannot catch a route that was never registered or a gate that + * rejects the request — so this suite drives real HTTP over a listening server, + * and authenticates purely through `?auth_token=` (no request headers), which is + * the whole point of offering the read as a GET. + */ +describe('GET /fs/readdir over HTTP', () => { + let env: PuterTestEnv; + + beforeAll(async () => { + env = await setupPuterTestEnv(); + }, 120_000); + + afterAll(async () => { + await env?.shutdown(); + }); + + const readdirUrl = (params: Record) => { + const url = new URL('/fs/readdir', env.apiOrigin); + for (const [key, value] of Object.entries(params)) { + url.searchParams.set(key, value); + } + return url; + }; + + it('lists a directory authenticated only by a query token', async () => { + const { username, token } = env.users.user; + const response = await fetch( + readdirUrl({ path: `/${username}`, auth_token: token }), + ); + expect(response.status).toBe(200); + const body = (await response.json()) as Array<{ + name: string; + uuid: string; + }>; + expect(Array.isArray(body)).toBe(true); + // Default provisioned home directories. + expect(body.map((e) => e.name)).toContain('Documents'); + for (const entry of body) { + expect(entry.uuid).toEqual(expect.any(String)); + } + }); + + it('paginates via query params', async () => { + const { username, token } = env.users.user; + const response = await fetch( + readdirUrl({ + path: `/${username}`, + auth_token: token, + limit: '2', + cursor: '', + includeTotal: 'true', + }), + ); + expect(response.status).toBe(200); + const page = (await response.json()) as { + items: unknown[]; + cursor?: string; + total?: number; + }; + expect(page.items.length).toBe(2); + expect(typeof page.total).toBe('number'); + expect(page.cursor).toEqual(expect.any(String)); + }); + + it('rejects an unauthenticated request', async () => { + const { username } = env.users.user; + const response = await fetch(readdirUrl({ path: `/${username}` })); + expect(response.status).toBeGreaterThanOrEqual(400); + expect(response.status).toBeLessThan(500); + }); + + it('does not leak internal ids or storage columns over the wire', async () => { + const { username, token } = env.users.user; + const response = await fetch( + readdirUrl({ path: `/${username}`, auth_token: token }), + ); + const raw = await response.text(); + const body = JSON.parse(raw) as Array>; + expect(body.length).toBeGreaterThan(0); + // Checked per-entry rather than by scanning the raw body: the nested + // `associatedApp` payload legitimately carries its own `id` (v1 has + // always exposed it), so a substring scan would false-alarm. + for (const entry of body) { + for (const field of [ + 'id', + 'parentId', + 'userId', + 'associatedAppId', + 'bucket', + 'bucketRegion', + 'publicToken', + 'fileRequestToken', + ]) { + expect(entry).not.toHaveProperty(field); + } + } + // No user-identifying data (emails, owner records) anywhere. + expect(raw).not.toContain('@'); + expect(raw).not.toMatch(/"(email|owner|user_id|userId)"/); + }); + + it('lists a nested subtree recursively over GET', async () => { + const { username, token } = env.users.user; + const base = `/${username}/Documents/http-recursive-${Date.now()}`; + for (const path of [base, `${base}/a`, `${base}/a/b`]) { + const created = await fetch(new URL('/fs/mkdir', env.apiOrigin), { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ path, auth_token: token }), + }); + expect(created.status).toBe(200); + } + + const response = await fetch( + readdirUrl({ + path: base, + auth_token: token, + recursive: 'true', + depth: '5', + }), + ); + expect(response.status).toBe(200); + const page = (await response.json()) as { + items: Array<{ path: string }>; + }; + expect( + page.items.map((e) => e.path.slice(base.length + 1)).sort(), + ).toEqual(['a', 'a/b']); + }); + + it('mkdir does not return internal ids, storage columns or tokens', async () => { + const { username, token } = env.users.user; + const response = await fetch(new URL('/fs/mkdir', env.apiOrigin), { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + path: `/${username}/Documents/http-mkdir-${Date.now()}`, + auth_token: token, + }), + }); + expect(response.status).toBe(200); + const entry = (await response.json()) as Record; + expect(entry.uuid).toEqual(expect.any(String)); + expect(entry.isDir).toBe(true); + for (const field of [ + 'id', + 'parentId', + 'userId', + 'associatedAppId', + 'bucket', + 'bucketRegion', + 'publicToken', + 'fileRequestToken', + ]) { + expect(entry).not.toHaveProperty(field); + } + }); + + it('startBatchWrite does not expose storage internals', async () => { + const { username, token } = env.users.user; + const response = await fetch( + new URL('/fs/startBatchWrite', env.apiOrigin), + { + method: 'POST', + // Array body, so there is no `auth_token` field for the auth + // probe to read — authenticate via the header instead. + headers: { + 'Content-Type': 'application/json', + Authorization: `Bearer ${token}`, + }, + body: JSON.stringify([ + { + fileMetadata: { + path: `/${username}/Documents/http-signed-${Date.now()}.bin`, + size: 4, + }, + }, + ]), + }, + ); + expect(response.status).toBe(200); + const [target] = (await response.json()) as Array< + Record + >; + // What a client actually needs to upload. + expect(target!.sessionId).toEqual(expect.any(String)); + expect(typeof target!.url).toBe('string'); + // What it does not: where the bytes physically live. + for (const field of ['bucket', 'bucketRegion', 'objectKey']) { + expect(target).not.toHaveProperty(field); + } + }); + + it('still serves the POST form for existing callers', async () => { + const { username, token } = env.users.user; + const response = await fetch(new URL('/fs/readdir', env.apiOrigin), { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ path: `/${username}`, auth_token: token }), + }); + expect(response.status).toBe(200); + const body = (await response.json()) as Array<{ name: string }>; + expect(body.map((e) => e.name)).toContain('Documents'); + }); +}); diff --git a/src/backend/controllers/fs/FSController.test.ts b/src/backend/controllers/fs/FSController.test.ts index 65ec1c30a..6c3f07abf 100644 --- a/src/backend/controllers/fs/FSController.test.ts +++ b/src/backend/controllers/fs/FSController.test.ts @@ -3,24 +3,25 @@ * * This file is part of Puter. * - * Puter is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published - * by the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. + * Puter is free software: you can redistribute it and/or modify it under the + * terms of the GNU Affero General Public License as published by the Free + * Software Foundation, either version 3 of the License, or (at your option) any + * later version. * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more + * details. * * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . + * along with this program. If not, see + * [https://www.gnu.org/licenses/](https://www.gnu.org/licenses/). */ import type { Request, Response } from 'express'; import type { Readable } from 'node:stream'; -import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; import { v4 as uuidv4 } from 'uuid'; +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; import type { Actor } from '../../core/actor.js'; import { runWithContext } from '../../core/context.js'; import { PuterServer } from '../../server.js'; @@ -28,9 +29,9 @@ import { setupTestServer } from '../../testUtil.js'; import { generateDefaultFsentries } from '../../util/userProvisioning.js'; import type { FSController } from './FSController.js'; import type { + ClientSignedWriteResponse, CompleteWriteRequest, SignedWriteRequest, - SignedWriteResponse, } from './requestTypes.js'; // ── Test harness ──────────────────────────────────────────────────── @@ -91,11 +92,15 @@ const makeReq = (init: { headers?: Record; actor: Actor; user?: { id: number; username: string }; + method?: string; }): Request => { return { body: init.body ?? ({} as B), query: init.query ?? {}, headers: init.headers ?? {}, + // Handlers that serve both verbs (readdir) read params from `query` on + // GET and `body` otherwise. + ...(init.method ? { method: init.method } : {}), actor: init.actor, // Some controller helpers fall back to `req.user` (set by the // session middleware) for id / username before reading `req.actor`. @@ -165,13 +170,16 @@ describe('FSController.startBatchWrites', () => { const req = makeReq({ body, actor }); await withActor(actor, () => controller.startBatchWrites(req, res)); - const responses = captured.body as SignedWriteResponse[]; + const responses = captured.body as ClientSignedWriteResponse[]; expect(responses).toHaveLength(2); for (const r of responses) { expect(r.sessionId).toEqual(expect.any(String)); - expect(r.objectKey).toEqual(expect.any(String)); - expect(r.bucket).toEqual(expect.any(String)); expect(r.uploadMode).toBe('single'); + // Storage internals stay server-side: the presigned `url` already + // encodes bucket and key. + for (const field of ['bucket', 'bucketRegion', 'objectKey']) { + expect(r).not.toHaveProperty(field); + } // In-memory mock S3 still returns a presigned-URL string for // single-mode uploads — verify it's there but don't assert // shape (varies by region/host config). @@ -203,7 +211,7 @@ describe('FSController.startBatchWrites', () => { const { res, captured } = makeRes(); const req = makeReq({ body, actor }); await withActor(actor, () => controller.startBatchWrites(req, res)); - const [response] = captured.body as SignedWriteResponse[]; + const [response] = captured.body as ClientSignedWriteResponse[]; const session = await server.stores.fsEntry.getPendingEntryBySessionId( response!.sessionId, ); @@ -329,7 +337,8 @@ describe('FSController.completeBatchWrites', () => { startRes.res, ), ); - const startResponses = startRes.captured.body as SignedWriteResponse[]; + const startResponses = startRes.captured + .body as ClientSignedWriteResponse[]; expect(startResponses).toHaveLength(2); // 2) Complete via the controller. Single-mode completion only @@ -352,16 +361,29 @@ describe('FSController.completeBatchWrites', () => { const responses = captured.body as Array<{ sessionId: string; wasOverwrite: boolean; - fsEntry: { path: string; userId: number; isDir: boolean }; + fsEntry: Record; }>; - expect(responses.map((r) => r.fsEntry.path).sort()).toEqual([ + expect(responses.map((r) => r.fsEntry.path as string).sort()).toEqual([ `/${username}/Documents/c.txt`, `/${username}/Documents/d.txt`, ]); for (const response of responses) { expect(response.wasOverwrite).toBe(false); - expect(response.fsEntry.userId).toBe(userId); expect(response.fsEntry.isDir).toBe(false); + // Ownership is asserted against the store below — the response + // itself must not carry the owner, storage columns, or the + // capability tokens. + for (const field of [ + 'userId', + 'id', + 'parentId', + 'bucket', + 'bucketRegion', + 'publicToken', + 'fileRequestToken', + ]) { + expect(response.fsEntry).not.toHaveProperty(field); + } } // The real fsentries were committed and are now resolvable. @@ -392,7 +414,7 @@ describe('FSController.completeBatchWrites', () => { ), ); const [firstResponse] = firstStart.captured - .body as SignedWriteResponse[]; + .body as ClientSignedWriteResponse[]; const firstComplete = makeRes(); await withActor(actor, () => controller.completeBatchWrites( @@ -424,7 +446,7 @@ describe('FSController.completeBatchWrites', () => { ), ); const [secondResponse] = secondStart.captured - .body as SignedWriteResponse[]; + .body as ClientSignedWriteResponse[]; const secondComplete = makeRes(); await withActor(actor, () => @@ -464,20 +486,25 @@ describe('FSController.completeBatchWrites', () => { start.res, ), ); - const [started] = start.captured.body as SignedWriteResponse[]; + const [started] = start.captured.body as ClientSignedWriteResponse[]; // Actually upload 4096 bytes to the session's object key (simulating - // a client that PUTs more than it declared via the signed URL). + // a client that PUTs more than it declared via the signed URL). The + // storage location isn't in the response any more, so read it off the + // upload session — a real client just PUTs to the presigned URL. + const session = await server.stores.fsEntry.getPendingEntryBySessionId( + started!.sessionId, + ); const realBytes = Buffer.alloc(4096, 0x41); await server.stores.s3Object.uploadFromServer( { - bucket: started!.bucket, - objectKey: started!.objectKey, + bucket: session!.bucket!, + objectKey: session!.objectKey, contentType: 'application/octet-stream', body: realBytes, contentLength: realBytes.byteLength, }, - started!.bucketRegion, + session!.bucketRegion!, ); const complete = makeRes(); @@ -518,7 +545,7 @@ describe('FSController.completeBatchWrites', () => { ), ); const [firstResponse] = firstStart.captured - .body as SignedWriteResponse[]; + .body as ClientSignedWriteResponse[]; await withActor(actor, () => controller.completeBatchWrites( makeReq({ @@ -555,7 +582,7 @@ describe('FSController.completeBatchWrites', () => { ), ); const [secondResponse] = secondStart.captured - .body as SignedWriteResponse[]; + .body as ClientSignedWriteResponse[]; const emitSpy = vi.spyOn(server.clients.event, 'emit'); let updatedCall: (typeof emitSpy.mock.calls)[number] | undefined; @@ -635,7 +662,7 @@ describe('FSController.completeBatchWrites', () => { startA.res, ), ); - const [aResponse] = startA.captured.body as SignedWriteResponse[]; + const [aResponse] = startA.captured.body as ClientSignedWriteResponse[]; const err = await withActor(b.actor, () => controller @@ -1788,13 +1815,12 @@ describe('FSController.readdirEntries recursive', () => { seen.push(...page.items.map((e) => e.path)); cursor = page.cursor; } while (cursor); - expect(rel(base, seen.map((path) => ({ path })))).toEqual([ - 'l1a', - 'l1a/l2a', - 'l1a/l2a/l3a', - 'l1a/l2a/l3a/l4a', - 'l1b', - ]); + expect( + rel( + base, + seen.map((path) => ({ path })), + ), + ).toEqual(['l1a', 'l1a/l2a', 'l1a/l2a/l3a', 'l1a/l2a/l3a/l4a', 'l1b']); }); it('counts the subtree with includeTotal', async () => { @@ -1918,6 +1944,178 @@ describe('FSController.readdirEntries recursive', () => { }); }); +// -- /readdir over GET (query params) -- + +describe('FSController.readdirEntriesViaGet', () => { + const seed = async (names: string[]) => { + const { actor } = await makeUser(); + const username = actor.user!.username!; + const dir = `/${username}/Documents/get-readdir`; + await withActor(actor, () => + controller.mkdirEntry( + makeReq({ body: { path: dir }, actor }), + makeRes().res, + ), + ); + for (const name of names) { + await withActor(actor, () => + controller.mkdirEntry( + makeReq({ body: { path: `${dir}/${name}` }, actor }), + makeRes().res, + ), + ); + } + return { actor, dir }; + }; + + // Query strings carry every value as a string — that is the whole risk + // surface of this route, so tests pass strings the way a real URL would. + const getReaddir = async ( + actor: Awaited>['actor'], + query: Record, + ) => { + const { res, captured } = makeRes(); + await withActor(actor, () => + controller.readdirEntriesViaGet( + makeReq({ query, actor, method: 'GET' }), + res, + ), + ); + return captured.body; + }; + + it('lists a directory from query params', async () => { + const { actor, dir } = await seed(['a', 'b', 'c']); + const body = await getReaddir(actor, { path: dir }); + expect(Array.isArray(body)).toBe(true); + expect((body as Array<{ name: string }>).map((e) => e.name)).toEqual([ + 'a', + 'b', + 'c', + ]); + }); + + it('honors string limit/offset like the POST form', async () => { + const { actor, dir } = await seed(['a', 'b', 'c']); + const body = (await getReaddir(actor, { + path: dir, + limit: '2', + offset: '1', + })) as Array<{ name: string }>; + expect(body.map((e) => e.name)).toEqual(['b', 'c']); + }); + + it('treats an empty cursor as the first page and returns the envelope', async () => { + const { actor, dir } = await seed(['a', 'b', 'c']); + const page = (await getReaddir(actor, { path: dir, cursor: '' })) as { + items: Array<{ name: string }>; + cursor?: string; + }; + expect(page.items.map((e) => e.name)).toEqual(['a', 'b', 'c']); + expect(page.cursor).toBeUndefined(); + }); + + it('pages through with a string cursor', async () => { + const { actor, dir } = await seed(['d1', 'd2', 'd3', 'd4', 'd5']); + const names: string[] = []; + let cursor: string | undefined = ''; + do { + const page = (await getReaddir(actor, { + path: dir, + limit: '2', + cursor, + })) as { items: Array<{ name: string }>; cursor?: string }; + names.push(...page.items.map((e) => e.name)); + cursor = page.cursor; + } while (cursor); + expect(names).toEqual(['d1', 'd2', 'd3', 'd4', 'd5']); + }); + + it('coerces string includeTotal=true', async () => { + const { actor, dir } = await seed(['a', 'b', 'c']); + const page = (await getReaddir(actor, { + path: dir, + limit: '1', + cursor: '', + includeTotal: 'true', + })) as { items: unknown[]; total?: number }; + expect(page.items.length).toBe(1); + expect(page.total).toBe(3); + }); + + it('coerces string recursive/depth and lists nested entries', async () => { + const { actor } = await makeUser(); + const username = actor.user!.username!; + const base = `/${username}/Documents/get-tree`; + for (const path of [base, `${base}/a`, `${base}/a/b`]) { + await withActor(actor, () => + controller.mkdirEntry( + makeReq({ body: { path }, actor }), + makeRes().res, + ), + ); + } + const page = (await getReaddir(actor, { + path: base, + recursive: 'true', + depth: '2', + })) as { items: Array<{ path: string }> }; + expect( + page.items.map((e) => e.path.slice(base.length + 1)).sort(), + ).toEqual(['a', 'a/b']); + }); + + it('rejects a non-directory target with the legacy code', async () => { + const { actor } = await makeUser(); + const username = actor.user!.username!; + const file = `/${username}/Documents/get-file.txt`; + await withActor(actor, () => + controller.touchEntry( + makeReq({ body: { path: file }, actor }), + makeRes().res, + ), + ); + await expect( + withActor(actor, () => + controller.readdirEntriesViaGet( + makeReq({ query: { path: file }, actor, method: 'GET' }), + makeRes().res, + ), + ), + ).rejects.toMatchObject({ + statusCode: 400, + legacyCode: 'dest_is_not_a_directory', + }); + }); + + it('does not expose internal ids, storage columns or tokens', async () => { + const { actor, dir } = await seed(['a']); + const body = (await getReaddir(actor, { path: dir })) as Array< + Record + >; + const entry = body[0]!; + // Numeric primary keys and storage/token columns must never ship in a + // listing — entries are addressed by uuid. + for (const field of [ + 'id', + 'parentId', + 'userId', + 'associatedAppId', + 'bucket', + 'bucketRegion', + 'publicToken', + 'fileRequestToken', + ]) { + expect(entry).not.toHaveProperty(field); + } + expect(entry.uuid).toEqual(expect.any(String)); + // No user-identifying data anywhere in the serialized payload. + const serialized = JSON.stringify(body); + expect(serialized).not.toContain('@'); + expect(serialized).not.toMatch(/"(email|owner|user_id|userId)"/); + }); +}); + // ── /touch additional branches ────────────────────────────────────── describe('FSController.touchEntry additional branches', () => { @@ -2430,10 +2628,9 @@ describe('FSController associatedAppId entitlement gate', () => { ], ); const row = ( - await server.clients.db.read( - 'SELECT id FROM apps WHERE uid = ?', - [uid], - ) + await server.clients.db.read('SELECT id FROM apps WHERE uid = ?', [ + uid, + ]) )[0] as { id: number }; return { id: row.id }; }; @@ -2449,13 +2646,15 @@ describe('FSController associatedAppId entitlement gate', () => { await withActor(actor, () => controller.startBatchWrites( makeReq({ - body: [{ fileMetadata: { path, size: 3, associatedAppId } }], + body: [ + { fileMetadata: { path, size: 3, associatedAppId } }, + ], actor, }), startRes.res, ), ); - const [started] = startRes.captured.body as SignedWriteResponse[]; + const [started] = startRes.captured.body as ClientSignedWriteResponse[]; const completeRes = makeRes(); await withActor(actor, () => controller.completeBatchWrites( diff --git a/src/backend/controllers/fs/FSController.ts b/src/backend/controllers/fs/FSController.ts index c8b0d7ca8..68915871a 100644 --- a/src/backend/controllers/fs/FSController.ts +++ b/src/backend/controllers/fs/FSController.ts @@ -41,13 +41,24 @@ import { import { applyInlineContentSecurity } from '../../util/inlineContentSecurity.js'; import { PuterController } from '../types.js'; import { FS_COSTS } from './costs.js'; +import { + assertAccess as assertLegacyAccess, + fsEntryMimeType, + loadLegacyAssociatedApps, + signEntryThumbnail, + toLegacyEntry, +} from './legacyFsHelpers.js'; import type { + ClientCompleteWriteResponse, + ClientFSEntry, + ClientReaddirEntry, + ClientSignedWriteResponse, + ClientSignMultipartPartsResponse, + ClientWriteResponse, CompleteWriteRequest, - CompleteWriteResponse, SignedWriteRequest, SignedWriteResponse, SignMultipartPartsRequest, - SignMultipartPartsResponse, WriteGuiMetadata, WriteRequest, WriteResponse, @@ -61,13 +72,6 @@ import type { ThumbnailUploadPrepareItem, ThumbnailUploadPreparePayload, } from './types.js'; -import { - toLegacyEntry, - loadLegacyAssociatedApps, - fsEntryMimeType, - signEntryThumbnail, - assertAccess as assertLegacyAccess, -} from './legacyFsHelpers.js'; class UploadProgressTracker implements UploadProgressTrackerLike { total = 0; progress = 0; @@ -115,7 +119,7 @@ export class FSController extends PuterController { @Post('/startWrite', { subdomain: 'api', requireVerified: true }) async startWrite( req: Request, - res: Response, + res: Response, ) { const userId = this.#getActorUserId(req); const storageAllowanceMax = this.#getStorageAllowanceMaxOverride(req); @@ -165,13 +169,15 @@ export class FSController extends PuterController { } }, 'emitStartWriteDirectoryEvents'); } - res.json(response); + res.json( + this.#withoutStorageInternals(this.#withClientFsEntry(response)), + ); } @Post('/startBatchWrite', { subdomain: 'api', requireVerified: true }) async startBatchWrites( req: Request, - res: Response, + res: Response, ) { const userId = this.#getActorUserId(req); const storageAllowanceMax = this.#getStorageAllowanceMaxOverride(req); @@ -261,13 +267,17 @@ export class FSController extends PuterController { } }, 'emitStartBatchWriteDirectoryEvents'); } - res.json(responses); + res.json( + responses.map((r) => + this.#withoutStorageInternals(this.#withClientFsEntry(r)), + ), + ); } @Post('/completeWrite', { subdomain: 'api', requireVerified: true }) async completeWrite( req: Request, - res: Response, + res: Response, ) { const userId = this.#getActorUserId(req); const requestBody = this.#withGuiMetadata(req.body, req.body); @@ -287,13 +297,18 @@ export class FSController extends PuterController { }, requestBody.guiMetadata, ); - res.json({ ...response, fsEntry: writeResponse.fsEntry }); + res.json( + this.#withRequiredClientFsEntry({ + ...response, + fsEntry: writeResponse.fsEntry, + }), + ); } @Post('/completeBatchWrite', { subdomain: 'api', requireVerified: true }) async completeBatchWrites( req: Request, - res: Response, + res: Response, ) { const userId = this.#getActorUserId(req); const requests = Array.isArray(req.body) @@ -328,7 +343,9 @@ export class FSController extends PuterController { return { ...writeResponse, fsEntry: withSideEffects.fsEntry }; }, ); - res.json(updatedResponse); + res.json( + updatedResponse.map((r) => this.#withRequiredClientFsEntry(r)), + ); } @Post('/abortWrite', { subdomain: 'api', requireVerified: true }) @@ -350,20 +367,20 @@ export class FSController extends PuterController { @Post('/signMultipartParts', { subdomain: 'api', requireVerified: true }) async signMultipartParts( req: Request, - res: Response, + res: Response, ) { const userId = this.#getActorUserId(req); const response = await this.services.fs.signMultipartParts( userId, req.body, ); - res.json(response); + res.json(this.#withoutStorageInternals(response)); } @Post('/write', { subdomain: 'api', requireVerified: true }) async write( req: Request, - res: Response, + res: Response, ) { const userId = this.#getActorUserId(req); const storageAllowanceMax = this.#getStorageAllowanceMaxOverride(req); @@ -403,13 +420,13 @@ export class FSController extends PuterController { response, requestBody.guiMetadata, ); - res.json(updatedResponse); + res.json(this.#withRequiredClientFsEntry(updatedResponse)); } @Post('/batchWrite', { subdomain: 'api', requireVerified: true }) async batchWrites( req: Request, - res: Response, + res: Response, ) { const userId = this.#getActorUserId(req); const storageAllowanceMax = this.#getStorageAllowanceMaxOverride(req); @@ -711,7 +728,9 @@ export class FSController extends PuterController { ); }, ); - res.json(updatedResponses); + res.json( + updatedResponses.map((r) => this.#withRequiredClientFsEntry(r)), + ); return; } @@ -831,7 +850,9 @@ export class FSController extends PuterController { ); }, ); - res.json(updatedResponses); + res.json( + updatedResponses.map((r) => this.#withRequiredClientFsEntry(r)), + ); } // -- Read-side routes ------------------------------------------------ @@ -867,30 +888,109 @@ export class FSController extends PuterController { * (with public folders enabled) any authenticated user. The legacy read * path already curates its output; this does the same for the v2 routes. */ - #toClientEntry(entry: object): Record { - const clone: Record = { ...entry }; - for (const field of [ - 'bucket', - 'bucketRegion', - 'userId', - 'publicToken', - 'fileRequestToken', - ]) - delete clone[field]; - return clone; + #toClientEntry(entry: FSEntry): ClientFSEntry { + // Allowlist, not a denylist: a denylist silently ships every column + // added to `fsentries` later. Omits the numeric primary keys (`id`, + // `parentId`, `associatedAppId`), the storage columns, the owning + // `userId`, and the `publicToken`/`fileRequestToken` capability tokens. + // + // Tolerant of partially-hydrated entries: write/mkdir paths return a + // freshly-built entry that hasn't been through a subdomain join. + const subdomains = entry.subdomains ?? []; + return { + uuid: entry.uuid, + uid: entry.uid ?? entry.uuid, + parentUid: entry.parentUid ?? null, + path: entry.path, + name: entry.name, + isDir: entry.isDir, + isShortcut: entry.isShortcut, + shortcutTo: entry.shortcutTo ?? null, + isSymlink: entry.isSymlink, + symlinkPath: entry.symlinkPath ?? null, + isPublic: entry.isPublic ?? null, + immutable: entry.immutable, + metadata: entry.metadata ?? null, + modified: entry.modified, + created: entry.created ?? null, + accessed: entry.accessed ?? null, + size: entry.size ?? null, + layout: entry.layout ?? null, + subdomains, + workers: entry.workers ?? [], + hasWebsite: entry.hasWebsite ?? subdomains.length > 0, + suggestedApps: entry.suggestedApps ?? [], + }; + } + + /** + * Sanitize the `fsEntry` a write response carries, leaving the rest of the + * envelope (session id, presigned upload targets) untouched — those fields + * are the point of the response. Applied at every `res.json` on the write + * paths, which previously serialized the raw database row. + * + * The return type is the sanitized counterpart, so a caller cannot keep + * treating the result as though it still held a full `FSEntry`. + */ + #withClientFsEntry( + response: T, + ): Omit & { fsEntry?: ClientFSEntry } { + const { fsEntry, ...rest } = response; + return { + ...rest, + ...(fsEntry ? { fsEntry: this.#toClientEntry(fsEntry) } : {}), + }; + } + + /** + * Drop the storage internals from a presigned-upload envelope. The client + * uploads to the presigned URLs, which already encode bucket and key, so + * naming the physical location of a user's bytes buys the caller nothing. + */ + #withoutStorageInternals< + T extends { bucket: string; bucketRegion: string; objectKey: string }, + >(response: T): Omit { + const { bucket, bucketRegion, objectKey, ...rest } = response; + return rest; + } + + /** Same, for the responses whose `fsEntry` is always present. */ + #withRequiredClientFsEntry( + response: T, + ): Omit & { fsEntry: ClientFSEntry } { + return { + ...response, + fsEntry: this.#toClientEntry(response.fsEntry), + }; + } + + /** + * `GET /fs/readdir` — same contract as the POST form, with parameters in + * the query string instead of the body. A read as a GET is cacheable and + * can authenticate via `?auth_token=`, which lets callers fetch a listing + * without a JSON body. Every value arrives as a string, so parameter + * parsing goes through the same coercion helpers the POST path uses. + */ + @Get('/readdir', { subdomain: 'api', requireVerified: true }) + async readdirEntriesViaGet(req: Request, res: Response) { + return this.readdirEntries(req, res); } @Post('/readdir', { subdomain: 'api', requireVerified: true }) async readdirEntries(req: Request, res: Response) { const actor = this.#requireActor(req); - const body = this.#toObjectRecord(req.body); + // GET carries its parameters in the query string; POST in the body. + const body = this.#toObjectRecord( + req.method === 'GET' ? req.query : req.body, + ); - // Presence of `cursor` (null means "first page") or `includeTotal` - // opts into the paginated `{items, cursor?, total?}` envelope. - // Legacy limit/offset requests keep the bare-array response. + // Presence of `cursor` (null/empty means "first page") or + // `includeTotal` opts into the paginated `{items, cursor?, total?}` + // envelope. Legacy limit/offset requests keep the bare-array response. + const includeTotal = this.#toBoolean(body.includeTotal) === true; const paginated = Object.prototype.hasOwnProperty.call(body, 'cursor') || - body.includeTotal === true; + includeTotal; // Undocumented: `recursive` lists descendants (prefix scan) up to // `depth` levels below the target. Always paginated; sorts by path. @@ -925,9 +1025,7 @@ export class FSController extends PuterController { if (paginated) { res.json({ items: rootItems, - ...(body.includeTotal === true - ? { total: rootItems.length } - : {}), + ...(includeTotal ? { total: rootItems.length } : {}), }); return; } @@ -988,14 +1086,13 @@ export class FSController extends PuterController { }, ); await this.#attachSuggestedApps(page.entries); - const total = - body.includeTotal === true - ? await this.services.fs.countDirectoryTree( - parent.userId, - parent.path, - maxDepth, - ) - : undefined; + const total = includeTotal + ? await this.services.fs.countDirectoryTree( + parent.userId, + parent.path, + maxDepth, + ) + : undefined; res.json({ items: await this.#toReaddirEntries(page.entries), ...(page.cursor ? { cursor: page.cursor } : {}), @@ -1013,10 +1110,9 @@ export class FSController extends PuterController { sortOrder, }); await this.#attachSuggestedApps(page.entries); - const total = - body.includeTotal === true - ? await this.services.fs.countDirectory(parent.uuid) - : undefined; + const total = includeTotal + ? await this.services.fs.countDirectory(parent.uuid) + : undefined; res.json({ items: await this.#toReaddirEntries(page.entries), ...(page.cursor ? { cursor: page.cursor } : {}), @@ -1040,28 +1136,26 @@ export class FSController extends PuterController { * the three fields the SDK cannot reconstruct on its own so it can rebuild * the v1 shape: `type` (MIME), a signed `thumbnail`, and `associatedApp`. */ - async #toReaddirEntries( - entries: FSEntry[], - ): Promise[]> { + async #toReaddirEntries(entries: FSEntry[]): Promise { const appsById = await loadLegacyAssociatedApps( this.stores.app, entries, ); return Promise.all( - entries.map(async (entry) => { - const shaped = this.#toClientEntry(entry); - shaped.type = fsEntryMimeType(entry); - shaped.thumbnail = await signEntryThumbnail( + entries.map(async (entry) => ({ + ...this.#toClientEntry(entry), + // Fields the client cannot derive on its own. + type: fsEntryMimeType(entry), + thumbnail: await signEntryThumbnail( this.clients.event, entry.uuid, entry.thumbnail, - ); - shaped.associatedApp = + ), + associatedApp: entry.associatedAppId !== null ? (appsById.get(entry.associatedAppId) ?? null) - : null; - return shaped; - }), + : null, + })), ); } @@ -1223,7 +1317,7 @@ export class FSController extends PuterController { ) ?? false, }); this.#emitGuiItemAdded(entry); - res.json(entry); + res.json(this.#toClientEntry(entry)); } @Post('/touch', { subdomain: 'api', requireVerified: true }) @@ -1259,7 +1353,7 @@ export class FSController extends PuterController { createMissingParents: this.#toBoolean(body.create_missing_parents) ?? false, }); - res.json(entry); + res.json(this.#toClientEntry(entry)); } @Post('/rename', { subdomain: 'api', requireVerified: true }) @@ -1277,7 +1371,7 @@ export class FSController extends PuterController { const renamed = await this.services.fs.rename(entry, newName); this.#emitGuiItemUpdated(renamed); - res.json(renamed); + res.json(this.#toClientEntry(renamed)); } @Post('/delete', { subdomain: 'api', requireVerified: true }) @@ -1323,7 +1417,7 @@ export class FSController extends PuterController { this.#toBoolean(body.dedupe_name ?? body.change_name) ?? false, }); this.#emitGuiItemMoved(source, moved); - res.json(moved); + res.json(this.#toClientEntry(moved)); } @Post('/copy', { subdomain: 'api', requireVerified: true }) @@ -1351,7 +1445,7 @@ export class FSController extends PuterController { this.#toBoolean(body.dedupe_name ?? body.change_name) ?? true, }); this.#emitGuiItemAdded(copy); - res.json(copy); + res.json(this.#toClientEntry(copy)); } @Post('/mkshortcut', { subdomain: 'api', requireVerified: true }) @@ -1380,7 +1474,7 @@ export class FSController extends PuterController { dedupeName: this.#toBoolean(body.dedupe_name) ?? true, }); this.#emitGuiItemAdded(shortcut); - res.json(shortcut); + res.json(this.#toClientEntry(shortcut)); } // -- Read-side helpers ----------------------------------------------- diff --git a/src/backend/controllers/fs/requestTypes.ts b/src/backend/controllers/fs/requestTypes.ts index dced0aa03..d3a40389e 100644 --- a/src/backend/controllers/fs/requestTypes.ts +++ b/src/backend/controllers/fs/requestTypes.ts @@ -3,22 +3,27 @@ * * This file is part of Puter. * - * Puter is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published - * by the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. + * Puter is free software: you can redistribute it and/or modify it under the + * terms of the GNU Affero General Public License as published by the Free + * Software Foundation, either version 3 of the License, or (at your option) any + * later version. * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more + * details. * * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . + * along with this program. If not, see + * [https://www.gnu.org/licenses/](https://www.gnu.org/licenses/). */ -import type { FSEntry, FSEntryWriteInput } from '../../stores/fs/FSEntry.js'; import type { Readable } from 'node:stream'; +import type { + FSEntry, + FSEntrySubdomain, + FSEntryWriteInput, +} from '../../stores/fs/FSEntry.js'; export type UploadMode = 'single' | 'multipart'; @@ -129,3 +134,100 @@ export interface WriteResponse { requestedThumbnail?: string | null; contentHashSha256?: string | null; } + +/** + * An `FSEntry` as it is safe to hand to a client. Built by an allowlist, so the + * `fsentries` primary key (`id`) and its `parentId`/`associatedAppId` + * references, the storage columns (`bucket`, `bucketRegion`), the owning + * `userId`, and the `publicToken`/`fileRequestToken` capability tokens never + * reach the wire. Entries are addressed by `uuid`. + * + * `shortcutTo` is the one numeric row reference that stays: the v1 contract has + * always exposed it as `shortcut_to` and the desktop resolves shortcuts through + * it, so dropping it would break them. + * + * The `?: never` members below are guards, not fields: they make a raw + * `FSEntry` fail to typecheck wherever a `ClientFSEntry` is expected. Without + * them this type is just a structural subset of `FSEntry`, so an unsanitized + * row would be silently assignable and the distinction would buy nothing. + * + * Checked by `tsc -p tsconfig.json` (the strict config). Note that + * `tsconfig.build.json` sets `noCheck: true` — it only transpiles, so it will + * not catch a violation here. + */ +export interface ClientFSEntry { + uuid: string; + uid: string; + parentUid: string | null; + path: string; + name: string; + isDir: boolean; + isShortcut: boolean; + shortcutTo: number | null; + isSymlink: boolean; + symlinkPath: string | null; + isPublic: boolean | null; + immutable: boolean; + metadata: string | null; + modified: number; + created: number | null; + accessed: number | null; + size: number | null; + layout: string | null; + subdomains: FSEntrySubdomain[]; + workers: FSEntrySubdomain[]; + hasWebsite: boolean; + suggestedApps: unknown[]; + + id?: never; + userId?: never; + parentId?: never; + associatedAppId?: never; + bucket?: never; + bucketRegion?: never; + publicToken?: never; + fileRequestToken?: never; +} + +/** + * Wire counterparts of the write responses. The bare `…Response` types describe + * what the service produces internally (a real `FSEntry`); these describe what + * the controller sends after sanitizing. Keeping them distinct is what makes + * "did this response get sanitized?" a question the compiler answers. + */ +/** + * The presigned-upload envelope minus the storage internals. A client uploads + * to the presigned `url` / `multipartPartUrls`, which already carry everything + * S3 needs, so `bucket`, `bucketRegion`, and `objectKey` are ours to keep — + * they name where a user's bytes physically live. + */ +export type ClientSignedWriteResponse = Omit< + SignedWriteResponse, + 'fsEntry' | 'bucket' | 'bucketRegion' | 'objectKey' +> & { fsEntry?: ClientFSEntry }; + +export type ClientSignMultipartPartsResponse = Omit< + SignMultipartPartsResponse, + 'bucket' | 'bucketRegion' | 'objectKey' +>; + +export type ClientCompleteWriteResponse = Omit< + CompleteWriteResponse, + 'fsEntry' +> & { fsEntry: ClientFSEntry }; + +export type ClientWriteResponse = Omit & { + fsEntry: ClientFSEntry; +}; + +/** + * A directory-listing entry: a sanitized entry plus the three fields a client + * cannot derive on its own — the MIME `type`, a _signed_ `thumbnail` URL (the + * stored value is an S3 key, which is useless to a client), and the resolved + * `associatedApp`. + */ +export type ClientReaddirEntry = ClientFSEntry & { + type: string | null; + thumbnail: string | null; + associatedApp: Record | null; +};