From ec968179ec2bf5154a9fadb1333803c9b84f8f9e Mon Sep 17 00:00:00 2001 From: Daniel Salazar Date: Mon, 20 Jul 2026 23:49:41 -0700 Subject: [PATCH] fix: misc hardening --- .../controllers/auth/AuthController.test.ts | 23 +++++++++++ .../controllers/auth/AuthController.ts | 11 ++++-- .../controllers/fs/FSController.test.ts | 35 +++++++++++++++++ src/backend/controllers/fs/FSController.ts | 39 ++++++++++++------- .../controllers/fs/LegacyFSController.ts | 25 ++++++------ src/backend/drivers/ai-ocr/OCRDriver.test.ts | 17 ++++++++ src/backend/drivers/ai-ocr/OCRDriver.ts | 18 +++++++-- src/backend/services/auth/OIDCService.test.ts | 35 +++++++++++++++++ src/backend/services/auth/OIDCService.ts | 6 ++- 9 files changed, 177 insertions(+), 32 deletions(-) diff --git a/src/backend/controllers/auth/AuthController.test.ts b/src/backend/controllers/auth/AuthController.test.ts index 5473a8e313..d4e8d6abcf 100644 --- a/src/backend/controllers/auth/AuthController.test.ts +++ b/src/backend/controllers/auth/AuthController.test.ts @@ -3221,6 +3221,29 @@ describe('AuthController.handleConfirmEmail', () => { original_client_socket_id: 's', }); }); + + it('rejects "null" as a code when no confirmation code is stored', async () => { + const { user, actor } = await makeUserAndActor(); + // A row with no stored code must never be confirmable: String(null) + // would otherwise equal a submitted "null" and confirm the email. + await server.stores.user.update(user.id, { email_confirm_code: null }); + const res = makeRes(); + await controller.handleConfirmEmail( + makeReq( + { code: 'null', original_client_socket_id: 's' }, + { actor }, + ), + res, + ); + expect(res.body).toEqual({ + email_confirmed: false, + original_client_socket_id: 's', + }); + const after = await server.stores.user.getById(user.id, { + force: true, + }); + expect(after!.email_confirmed).toBeFalsy(); + }); }); // ── Password recovery flow ────────────────────────────────────────── diff --git a/src/backend/controllers/auth/AuthController.ts b/src/backend/controllers/auth/AuthController.ts index c7b551663a..5f1ededd46 100644 --- a/src/backend/controllers/auth/AuthController.ts +++ b/src/backend/controllers/auth/AuthController.ts @@ -924,8 +924,7 @@ export class AuthController extends PuterController { is_temp: user!.password === null && user!.email === null, ip: (req?.headers?.['x-forwarded-for'] as - | string - | undefined) || + string | undefined) || ( req as unknown as { connection?: { remoteAddress?: string }; @@ -1078,7 +1077,13 @@ export class AuthController extends PuterController { }); return; } - if (String(user.email_confirm_code) !== String(code)) { + // Reject before comparing when no code is stored: `String(null)` would + // otherwise equal a submitted `"null"` and confirm the email without + // the real code. + if ( + !user.email_confirm_code || + String(user.email_confirm_code) !== String(code) + ) { res.json({ email_confirmed: false, original_client_socket_id, diff --git a/src/backend/controllers/fs/FSController.test.ts b/src/backend/controllers/fs/FSController.test.ts index 70ddfbfe76..dc08d2ac60 100644 --- a/src/backend/controllers/fs/FSController.test.ts +++ b/src/backend/controllers/fs/FSController.test.ts @@ -696,6 +696,41 @@ describe('FSController.statEntry', () => { expect(body.name).toBe('stat-me'); }); + it('does not leak backend-internal fields to the client', async () => { + const { actor } = await makeUser(); + const username = actor.user!.username!; + await withActor(actor, () => + controller.mkdirEntry( + makeReq({ + body: { path: `/${username}/Documents/no-leak` }, + actor, + }), + makeRes().res, + ), + ); + + const { res, captured } = makeRes(); + await withActor(actor, () => + controller.statEntry( + makeReq({ + body: { path: `/${username}/Documents/no-leak` }, + actor, + }), + res, + ), + ); + const body = captured.body as Record; + for (const field of [ + 'bucket', + 'bucketRegion', + 'userId', + 'publicToken', + 'fileRequestToken', + ]) { + expect(body).not.toHaveProperty(field); + } + }); + it('includes the subtree size when return_size is set on a directory', async () => { const { actor } = await makeUser(); const username = actor.user!.username!; diff --git a/src/backend/controllers/fs/FSController.ts b/src/backend/controllers/fs/FSController.ts index f1327e6999..0047cd376e 100644 --- a/src/backend/controllers/fs/FSController.ts +++ b/src/backend/controllers/fs/FSController.ts @@ -845,11 +845,32 @@ export class FSController extends PuterController { await this.services.suggestedApps.getSuggestedApps(entry); res.json({ - ...entry, + ...this.#toClientEntry(entry), ...(subtreeSize !== undefined ? { size: subtreeSize } : {}), }); } + /** + * Strip backend-internal fields before returning an entry to a client. + * Storage location (bucket/region), the owner's numeric id, and the + * capability-token columns are never used by clients and must not leak to + * callers who only hold `see`/`list` on the entry — a share recipient, or + * (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; + } + @Post('/readdir', { subdomain: 'api', requireVerified: true }) async readdirEntries(req: Request, res: Response) { const actor = this.#requireActor(req); @@ -880,16 +901,7 @@ export class FSController extends PuterController { child.suggestedApps = rootSuggestions[index] ?? []; } } - if (paginated) { - res.json({ - items: rootChildren, - ...(body.includeTotal === true - ? { total: rootChildren.length } - : {}), - }); - return; - } - res.json(rootChildren); + res.json(rootChildren.map((child) => this.#toClientEntry(child))); return; } @@ -960,6 +972,8 @@ export class FSController extends PuterController { child.suggestedApps = suggestions[index] ?? []; } } + + res.json(children.map((child) => this.#toClientEntry(child))); } @Post('/search', { subdomain: 'api', requireVerified: true }) @@ -1849,8 +1863,7 @@ export class FSController extends PuterController { // the ActorUser type. Access via the escape hatch until a proper // storage-quota mechanism is in place. const actorUser = req.actor?.user as - | Record - | undefined; + Record | undefined; const candidates = [ this.#toStorageCapacityCandidate(actorUser?.free_storage), diff --git a/src/backend/controllers/fs/LegacyFSController.ts b/src/backend/controllers/fs/LegacyFSController.ts index 2726b2b94e..7b8aac512e 100644 --- a/src/backend/controllers/fs/LegacyFSController.ts +++ b/src/backend/controllers/fs/LegacyFSController.ts @@ -25,6 +25,10 @@ import type { Actor } from '../../core/actor.js'; import { effectiveActorApp, isAccessTokenActor } from '../../core/actor.js'; import { Context } from '../../core/context.js'; import { HttpError } from '../../core/http/HttpError.js'; +import { + assertNotSuspended, + assertVerifiedAccount, +} from '../../core/http/middleware/gates.js'; import { RouteOptions } from '../../core/http/index.js'; import type { PuterRouter } from '../../core/http/PuterRouter.js'; import type { ACLService } from '../../services/acl/ACLService.js'; @@ -158,8 +162,7 @@ export class LegacyFSController extends PuterController { router.get('/get-launch-apps', apiOptions, async (req, res) => { const recommendedSvc = this.services.recommendedApps as unknown as - | { getRecommendedApps?: () => Promise } - | undefined; + { getRecommendedApps?: () => Promise } | undefined; const recommended = recommendedSvc?.getRecommendedApps ? await recommendedSvc.getRecommendedApps() : []; @@ -616,9 +619,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, { @@ -1001,6 +1002,12 @@ export class LegacyFSController extends PuterController { }); } + // This endpoint authenticates the token by hand and never runs the + // route gate chain, so the suspension and pending-verification checks + // that guard every other authenticated FS route have to run here. + assertNotSuspended(actor!.user); + assertVerifiedAccount(actor!.user); + req.actor = actor!; Context.set('actor', actor); @@ -1042,8 +1049,7 @@ export class LegacyFSController extends PuterController { } type SignedOrEmpty = - | (SignedFile & { path?: string }) - | Record; + (SignedFile & { path?: string }) | Record; const result: { signatures: SignedOrEmpty[]; token?: string } = { signatures: [], }; @@ -1584,10 +1590,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', diff --git a/src/backend/drivers/ai-ocr/OCRDriver.test.ts b/src/backend/drivers/ai-ocr/OCRDriver.test.ts index 9b05a91c54..d94d7a8536 100644 --- a/src/backend/drivers/ai-ocr/OCRDriver.test.ts +++ b/src/backend/drivers/ai-ocr/OCRDriver.test.ts @@ -374,6 +374,23 @@ it('meters one usage line per detected page at the per-page rate from costs.ts', // ── Mistral OCR ───────────────────────────────────────────────────── describe('OCRDriver.recognize (mistral)', () => { + it('throws 402 when the actor does not have enough credits', async () => { + hasCreditsSpy.mockResolvedValueOnce(false); + const { actor } = await makeUser(); + + await expect( + withActor(actor, () => + driver.recognize({ + source: dataUrl(Buffer.from('img'), 'image/png'), + provider: 'mistral', + }), + ), + ).rejects.toMatchObject({ statusCode: 402 }); + + // The paid Mistral call must not happen when credits are short. + expect(mistralOcrProcessMock).not.toHaveBeenCalled(); + }); + it('packages an image as an image_url chunk with a base64 data URL', async () => { const { actor } = await makeUser(); mistralOcrProcessMock.mockResolvedValueOnce({ diff --git a/src/backend/drivers/ai-ocr/OCRDriver.ts b/src/backend/drivers/ai-ocr/OCRDriver.ts index 5131a06519..c27ec13500 100644 --- a/src/backend/drivers/ai-ocr/OCRDriver.ts +++ b/src/backend/drivers/ai-ocr/OCRDriver.ts @@ -117,11 +117,9 @@ export class OCRDriver extends PuterDriver { const providers = this.config.providers ?? {}; const textract = providers['aws-textract'] as - | Record - | undefined; + Record | undefined; const textractAws = (textract?.aws ?? textract) as - | Record - | undefined; + Record | undefined; const textractAccessKey = textractAws?.access_key as string | undefined; const textractSecretKey = textractAws?.secret_key as string | undefined; const textractRegion = @@ -325,6 +323,18 @@ export class OCRDriver extends PuterDriver { args: RecognizeArgs, actor: Actor, ) { + // Gate on credits before the paid upstream call, mirroring the + // Textract branch. Page count isn't known until Mistral responds, so + // pre-flight one page's cost and meter the real total afterward. + const hasCredits = await this.services.metering.hasEnoughCredits( + actor, + OCR_COSTS['mistral-ocr:ocr:page'], + ); + if (!hasCredits) + throw new HttpError(402, 'Insufficient credits', { + legacyCode: 'insufficient_funds', + }); + const model = args.model ?? 'mistral-ocr-latest'; const chunk = this.#mistralBuildChunk(loaded); const payload: Record = { model, document: chunk }; diff --git a/src/backend/services/auth/OIDCService.test.ts b/src/backend/services/auth/OIDCService.test.ts index 931821cbde..e15c2880c7 100644 --- a/src/backend/services/auth/OIDCService.test.ts +++ b/src/backend/services/auth/OIDCService.test.ts @@ -233,3 +233,38 @@ describe('OIDCService.createUserFromOIDC', () => { } }); }); + +describe('OIDCService.linkProviderToUser', () => { + const makeConfirmedUser = async (): Promise => { + const username = `oidc-link-${crypto.randomBytes(4).toString('hex')}`; + const created = await server.stores.user.create({ + username, + uuid: crypto.randomUUID(), + password: null, + email: `${username}@corp.example`, + requires_email_confirmation: false, + }); + await server.stores.user.update(created.id, { email_confirmed: 1 }); + return created.id; + }; + + it('refuses to link to an existing account when the provider omits email_verified', async () => { + const userId = await makeConfirmedUser(); + const result = await oidc().linkProviderToUser(userId, 'custom-idp', { + sub: `attacker-${crypto.randomBytes(4).toString('hex')}`, + email: 'anything@corp.example', + }); + expect(result.success).toBe(false); + expect(result.error).toMatch(/verify/i); + }); + + it('links when the provider attests email_verified: true', async () => { + const userId = await makeConfirmedUser(); + const result = await oidc().linkProviderToUser(userId, 'custom-idp', { + sub: `legit-${crypto.randomBytes(4).toString('hex')}`, + email: 'anything@corp.example', + email_verified: true, + }); + expect(result.success).toBe(true); + }); +}); diff --git a/src/backend/services/auth/OIDCService.ts b/src/backend/services/auth/OIDCService.ts index dcefe35892..6a9c65c2f5 100644 --- a/src/backend/services/auth/OIDCService.ts +++ b/src/backend/services/auth/OIDCService.ts @@ -391,7 +391,11 @@ export class OIDCService extends PuterService { providerId: string, claims: OIDCUserInfo, ): Promise<{ success: boolean; error?: string }> { - if (claims.email_verified === false) { + // Fail closed: linking an OIDC identity to an EXISTING account hands + // login control to whoever holds that identity, so an absent + // `email_verified` claim (from a lax/custom provider) must not be + // treated as verified. Built-in providers always send it as `true`. + if (claims.email_verified !== true) { return { success: false, error: 'Provider did not verify this email address.',