From 60ae7ad96d4df9a9b40e55ef7312d9f18fad227c Mon Sep 17 00:00:00 2001 From: alexcibotari Date: Mon, 26 Oct 2026 01:24:40 +0100 Subject: [PATCH 1/3] fix: stream asset passthrough responses so files over 32 MiB download Problem: /assets/:id/download and /original (and a bare /assets/:id on non-image types) downloaded the object to /tmp and sent it with res.sendFile(), which always sets Content-Length. Cloud Run rejects non-streamed responses over 32 MiB with a bare 500, so a 104 MiB .mov could not be downloaded. /tmp is tmpfs on Cloud Functions, so every request also held the whole object in RAM. Fix: add streamStoredObject(), which pipes file.createReadStream() straight to the response. Bodies up to 31 MiB keep Content-Length; larger ones are sent without it so Node uses chunked transfer encoding. Single byte ranges are handled manually (206 with Content-Range, 416 when unsatisfiable) so video seeking and resumed downloads still work, and the Storage read is destroyed if the client disconnects. --- functions/src/utils/stream-object.test.ts | 34 ++++++++ functions/src/utils/stream-object.ts | 94 +++++++++++++++++++++++ functions/src/v1/cdn-assets.test.ts | 67 +++++++++++++++- functions/src/v1/cdn.ts | 30 ++++---- 4 files changed, 207 insertions(+), 18 deletions(-) create mode 100644 functions/src/utils/stream-object.test.ts create mode 100644 functions/src/utils/stream-object.ts diff --git a/functions/src/utils/stream-object.test.ts b/functions/src/utils/stream-object.test.ts new file mode 100644 index 000000000..8d500fa9f --- /dev/null +++ b/functions/src/utils/stream-object.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it } from 'vitest'; +import { parseByteRange } from './stream-object'; + +describe('parseByteRange', () => { + it('returns undefined when there is nothing to honour', () => { + expect(parseByteRange(undefined, 100)).toBeUndefined(); + expect(parseByteRange('items=0-5', 100)).toBeUndefined(); + expect(parseByteRange('bytes=-', 100)).toBeUndefined(); + expect(parseByteRange('bytes=0-1,5-6', 100)).toBeUndefined(); + expect(parseByteRange('bytes=9-2', 100)).toBeUndefined(); + }); + + it('parses a closed range', () => { + expect(parseByteRange('bytes=0-9', 100)).toEqual({ start: 0, end: 9 }); + }); + + it('parses an open-ended range to the last byte', () => { + expect(parseByteRange('bytes=90-', 100)).toEqual({ start: 90, end: 99 }); + }); + + it('clamps an end past the object', () => { + expect(parseByteRange('bytes=90-500', 100)).toEqual({ start: 90, end: 99 }); + }); + + it('parses a suffix range, clamped to the object', () => { + expect(parseByteRange('bytes=-10', 100)).toEqual({ start: 90, end: 99 }); + expect(parseByteRange('bytes=-500', 100)).toEqual({ start: 0, end: 99 }); + }); + + it('reports a range wholly outside the object as unsatisfiable', () => { + expect(parseByteRange('bytes=100-', 100)).toBe('unsatisfiable'); + expect(parseByteRange('bytes=-0', 100)).toBe('unsatisfiable'); + }); +}); diff --git a/functions/src/utils/stream-object.ts b/functions/src/utils/stream-object.ts new file mode 100644 index 000000000..6c864b674 --- /dev/null +++ b/functions/src/utils/stream-object.ts @@ -0,0 +1,94 @@ +import type { Request, Response } from 'express'; +import { logger } from 'firebase-functions'; +import type { Readable } from 'stream'; + +/** + * Cloud Run rejects a response larger than 32 MiB with a bare `500` unless it is streamed with + * chunked transfer encoding — and a response carrying `Content-Length` is not streamed in that + * sense. Bodies at or under this size keep their `Content-Length` (so clients show progress); + * anything larger is sent without one, which is what makes Node fall back to chunked encoding. + * Kept a little under the hard limit to leave room for the headers. + */ +export const MAX_CONTENT_LENGTH_BYTES = 31 * 1024 * 1024; + +export type ByteRange = { start: number; end: number }; + +/** + * Parses a `Range` header against an object of `size` bytes. + * + * - `undefined`: no range to honour — serve the whole object. Covers a missing header, a unit + * other than `bytes`, malformed syntax and multi-range requests, all of which RFC 9110 lets a + * server answer with a plain `200`. + * - `'unsatisfiable'`: well-formed but entirely outside the object — answer `416`. + * - otherwise the inclusive byte range to send, clamped to the object. + */ +export function parseByteRange(header: string | undefined, size: number): ByteRange | 'unsatisfiable' | undefined { + if (!header) return undefined; + const match = /^bytes=(\d*)-(\d*)$/.exec(header.trim()); + if (!match) return undefined; + const [, first, last] = match; + if (first === '' && last === '') return undefined; + if (first === '') { + // Suffix range: the final `last` bytes. + const length = Number(last); + if (length === 0 || size === 0) return 'unsatisfiable'; + return { start: Math.max(0, size - length), end: size - 1 }; + } + const start = Number(first); + const end = last === '' ? size - 1 : Math.min(Number(last), size - 1); + if (last !== '' && Number(last) < start) return undefined; + if (start >= size) return 'unsatisfiable'; + return { start, end }; +} + +/** The slice of a Storage `File` this needs, so tests can hand in a fake. */ +export interface StreamableFile { + createReadStream(options?: { start?: number; end?: number; validation?: false }): Readable; +} + +/** + * Streams a stored object to the response, honouring a single `Range`. Headers other than + * length and range (type, disposition, cache) must already be set by the caller. + * + * Replaces `download()` to `/tmp` + `sendFile`: on Cloud Functions `/tmp` is RAM, so that held + * the whole object in memory, and `sendFile` always sets `Content-Length` — which is what made + * every object over 32 MiB fail. + */ +export function streamStoredObject(req: Request, res: Response, file: StreamableFile, size: number, tag: string): void { + res.header('Accept-Ranges', 'bytes'); + const range = parseByteRange(req.headers.range, size); + if (range === 'unsatisfiable') { + res.status(416).header('Content-Range', `bytes */${size}`).header('Cache-Control', 'no-cache').end(); + return; + } + const { start, end } = range ?? { start: 0, end: size - 1 }; + const length = size === 0 ? 0 : end - start + 1; + if (range) { + res.status(206).header('Content-Range', `bytes ${start}-${end}/${size}`); + } + if (length <= MAX_CONTENT_LENGTH_BYTES) { + res.header('Content-Length', String(length)); + } + if (req.method === 'HEAD' || length === 0) { + res.end(); + return; + } + // `validation: false` because a partial read cannot be checksummed, and a full one is + // already covered by the ETag the caller sent. + const stream = file.createReadStream(range ? { start, end, validation: false } : { validation: false }); + stream.on('error', error => { + logger.error(`${tag} stream failed`, error); + if (res.headersSent) { + // Mid-body: the status is gone, so the only honest signal left is a truncated response. + res.destroy(error); + } else { + res.status(500).removeHeader('Content-Length'); + res.removeHeader('Content-Range'); + res.removeHeader('Content-Disposition'); + res.header('Cache-Control', 'no-cache').end(); + } + }); + // A client that disconnects should stop the Storage read rather than let it run to the end. + res.on('close', () => stream.destroy()); + stream.pipe(res); +} diff --git a/functions/src/v1/cdn-assets.test.ts b/functions/src/v1/cdn-assets.test.ts index b3aec7c92..cc56e872e 100644 --- a/functions/src/v1/cdn-assets.test.ts +++ b/functions/src/v1/cdn-assets.test.ts @@ -1,6 +1,7 @@ import type { Express } from 'express'; import express from 'express'; import { writeFileSync } from 'fs'; +import { Readable } from 'stream'; import sharp from 'sharp'; import request from 'supertest'; import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; @@ -46,7 +47,9 @@ async function jpegFixture(quality = 95): Promise { raw[i + 2] = (x * y) % 255; } } - return sharp(raw, { raw: { width, height, channels: 3 } }).jpeg({ quality }).toBuffer(); + return sharp(raw, { raw: { width, height, channels: 3 } }) + .jpeg({ quality }) + .toBuffer(); } /** A genuinely multi-frame GIF; frames must differ or the encoder collapses them to one. */ @@ -81,9 +84,16 @@ interface StoredAsset { documentMissing?: boolean; } -function given(asset: StoredAsset): { download: ReturnType } { +function given(asset: StoredAsset): { download: ReturnType; createReadStream: ReturnType } { const file = { - getMetadata: asset.storageMissing ? vi.fn().mockRejectedValue(new Error('not found')) : vi.fn().mockResolvedValue([{ md5Hash: MD5 }]), + getMetadata: asset.storageMissing + ? vi.fn().mockRejectedValue(new Error('not found')) + : vi.fn().mockResolvedValue([{ md5Hash: MD5, size: String(asset.bytes.length) }]), + createReadStream: vi + .fn() + .mockImplementation((options?: { start?: number; end?: number }) => + Readable.from([asset.bytes.subarray(options?.start ?? 0, options?.end === undefined ? undefined : options.end + 1)]) + ), download: vi.fn().mockImplementation(async (options?: { destination: string }) => { if (options?.destination) { writeFileSync(options.destination, asset.bytes); @@ -111,6 +121,13 @@ function app(): Express { const URL = '/api/v1/spaces/s1/assets/a1'; +/** supertest only buffers known binary types; this keeps any body as raw bytes. */ +function binaryParser(res: NodeJS.ReadableStream, callback: (err: Error | null, body: Buffer) => void): void { + const chunks: Buffer[] = []; + res.on('data', (chunk: Buffer) => chunks.push(chunk)); + res.on('end', () => callback(null, Buffer.concat(chunks))); +} + beforeEach(() => { vi.restoreAllMocks(); }); @@ -138,6 +155,50 @@ describe('asset routes — the passthrough pair', () => { expect(res.body.length).toBe(bytes.length); }); + it('/download streams from Storage instead of staging the object in /tmp', async () => { + const bytes = Buffer.from('video bytes'); + const file = given({ bytes, type: 'video/quicktime', extension: '.mov' }); + + const res = await request(app()).get(`${URL}/download`).buffer(true).parse(binaryParser); + + expect(res.status).toBe(200); + expect(res.headers['accept-ranges']).toBe('bytes'); + expect(res.headers['content-length']).toBe(String(bytes.length)); + expect(Buffer.compare(res.body, bytes)).toBe(0); + expect(file.download).not.toHaveBeenCalled(); + }); + + it('/download honours a Range request with 206', async () => { + const bytes = Buffer.from('0123456789'); + given({ bytes, type: 'video/quicktime', extension: '.mov' }); + + const res = await request(app()).get(`${URL}/download`).set('Range', 'bytes=2-5').buffer(true).parse(binaryParser); + + expect(res.status).toBe(206); + expect(res.headers['content-range']).toBe('bytes 2-5/10'); + expect(res.body.toString()).toBe('2345'); + }); + + it('/download answers an out-of-bounds Range with 416', async () => { + given({ bytes: Buffer.from('0123456789'), type: 'video/quicktime', extension: '.mov' }); + + const res = await request(app()).get(`${URL}/download`).set('Range', 'bytes=50-'); + + expect(res.status).toBe(416); + expect(res.headers['content-range']).toBe('bytes */10'); + }); + + it('a bare transform-route URL on a video streams the stored bytes', async () => { + const bytes = Buffer.from('video bytes'); + const file = given({ bytes, type: 'video/mp4', extension: '.mp4' }); + + const res = await request(app()).get(URL).buffer(true).parse(binaryParser); + + expect(res.status).toBe(200); + expect(Buffer.compare(res.body, bytes)).toBe(0); + expect(file.download).not.toHaveBeenCalled(); + }); + it.each([['/original'], ['/download']])('%s rejects a transform parameter rather than ignoring it', async route => { given({ bytes: await jpegFixture(), type: 'image/jpeg', extension: '.jpg' }); diff --git a/functions/src/v1/cdn.ts b/functions/src/v1/cdn.ts index ceac0c994..4815928f4 100644 --- a/functions/src/v1/cdn.ts +++ b/functions/src/v1/cdn.ts @@ -55,6 +55,7 @@ import { assetResponsePolicy } from '../utils/asset-headers'; import { buildContentDisposition } from '../utils/content-disposition'; import { redactQuery } from '../utils/log-redact'; import { resolveLocaleFilePath } from '../utils/locale-utils'; +import { streamStoredObject } from '../utils/stream-object'; import { RequestWithToken, requireContentPermissions, @@ -535,9 +536,10 @@ CDN.get('/api/v1/spaces/:spaceId/assets/:assetId', async (req, res) => { // Set by the branches where sharp produces the response, so those never touch `/tmp`. // `/tmp` on Cloud Functions is tmpfs — RAM against the instance limit — so writing the // output there and reading it straight back with `sendFile` held every transformed image - // in memory twice. The passthrough branches below still use `tempFilePath`; converting - // those is the deferred video-streaming work. + // in memory twice. Only the SVG and video-thumbnail branches below still use `tempFilePath`; + // a plain passthrough streams from Storage. let output: Buffer | undefined; + let streamed = false; let filename = `${asset.name}${asset.extension}`; const formatMimeMap: Record = { webp: 'image/webp', jpeg: 'image/jpeg', png: 'image/png', avif: 'image/avif' }; const formatExtMap: Record = { webp: '.webp', jpeg: '.jpg', png: '.png', avif: '.avif' }; @@ -723,7 +725,9 @@ CDN.get('/api/v1/spaces/:spaceId/assets/:assetId', async (req, res) => { overwriteType = format ? formatMimeMap[format] : 'image/webp'; filename = `${asset.name}-${suffix || `w${width}`}-thumbnail${format ? formatExtMap[format] : '.webp'}`; } else { - await assetFile.download({ destination: tempFilePath }); + // Video, audio, PDF and the rest pass through untouched, so they are streamed rather than + // staged in `/tmp` — see the `/download` route for why that matters past 32 MiB. + streamed = true; } const policy = assetResponsePolicy(overwriteType || asset.type, false); res @@ -733,6 +737,8 @@ CDN.get('/api/v1/spaces/:spaceId/assets/:assetId', async (req, res) => { .contentType(policy.contentType); if (output) { res.send(output); + } else if (streamed) { + streamStoredObject(req, res, assetFile, Number(objectMetadata?.['size'] ?? 0), '[V1:AssetById]'); } else { res.sendFile(tempFilePath); } @@ -830,21 +836,15 @@ CDN.get(['/api/v1/spaces/:spaceId/assets/:assetId/original', '/api/v1/spaces/:sp } } - // A distinct path from the transform route's `assets-${assetId}`. That route writes - // *transformed* output there on the video-thumbnail branch, so sharing the path would let a - // concurrent request for the same asset serve the wrong bytes. - // - // `sendFile` rather than a stream because it implements Range requests, and a video here is - // exactly where a client resumes a partial transfer. Streaming is not the fix for the tmpfs - // cost — a redirect to Storage is, because GCS handles Range natively. - const tempFilePath = `${os.tmpdir()}/assets-stored-${assetId}`; - await assetFile.download({ destination: tempFilePath }); - + // Streamed straight from Storage rather than downloaded to `/tmp` and sent with `sendFile`: + // `/tmp` is RAM on Cloud Functions, and `sendFile` always sets `Content-Length`, which makes + // Cloud Run reject anything over 32 MiB — a 100 MB video failed with a bare `500`. The stream + // honours `Range` itself, so a client can still resume a partial transfer. const policy = assetResponsePolicy(asset.type, attachment); res .header('Cache-Control', `public, max-age=${CACHE_ASSET_MAX_AGE}, s-maxage=${CACHE_ASSET_MAX_AGE}`) .header('Content-Disposition', buildContentDisposition(`${asset.name}${asset.extension}`, policy.attachment)) .set(policy.headers) - .contentType(policy.contentType) - .sendFile(tempFilePath); + .contentType(policy.contentType); + streamStoredObject(req, res, assetFile, Number(objectMetadata?.['size'] ?? 0), tag); }); From 204cb40fa123d5dba32b337522877d6e37888f29 Mon Sep 17 00:00:00 2001 From: alexcibotari Date: Mon, 26 Oct 2026 01:33:05 +0100 Subject: [PATCH 2/3] fix: add JSDoc params to stream-object helpers for valid-jsdoc lint --- functions/src/utils/stream-object.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/functions/src/utils/stream-object.ts b/functions/src/utils/stream-object.ts index 6c864b674..01b5aecf2 100644 --- a/functions/src/utils/stream-object.ts +++ b/functions/src/utils/stream-object.ts @@ -21,6 +21,9 @@ export type ByteRange = { start: number; end: number }; * server answer with a plain `200`. * - `'unsatisfiable'`: well-formed but entirely outside the object — answer `416`. * - otherwise the inclusive byte range to send, clamped to the object. + * @param {string | undefined} header Raw `Range` request header + * @param {number} size Object size in bytes + * @return {ByteRange | 'unsatisfiable' | undefined} the range to serve, if any */ export function parseByteRange(header: string | undefined, size: number): ByteRange | 'unsatisfiable' | undefined { if (!header) return undefined; @@ -53,6 +56,11 @@ export interface StreamableFile { * Replaces `download()` to `/tmp` + `sendFile`: on Cloud Functions `/tmp` is RAM, so that held * the whole object in memory, and `sendFile` always sets `Content-Length` — which is what made * every object over 32 MiB fail. + * @param {Request} req Incoming request, read for `Range` and the method + * @param {Response} res Response to stream into + * @param {StreamableFile} file Storage object to read + * @param {number} size Object size in bytes, from its metadata + * @param {string} tag Log prefix of the calling route */ export function streamStoredObject(req: Request, res: Response, file: StreamableFile, size: number, tag: string): void { res.header('Accept-Ranges', 'bytes'); From 6f9c2b6bc4a28066b44fe8c405d60b11e3dcbcc0 Mon Sep 17 00:00:00 2001 From: alexcibotari Date: Mon, 26 Oct 2026 09:46:12 +0100 Subject: [PATCH 3/3] fix: redirect assets over 31 MiB to a signed Storage URL Problem: after streaming large assets, a 104 MiB .mov downloaded through localess-prod.web.app as only its first 33,553,834 bytes. Firebase Hosting buffers rewritten function responses and cuts them at 32 MiB, still answering 200 with a matching Content-Length, so the browser treated the truncated file as complete. The function itself served all 108,726,933 bytes when called directly on Cloud Run. Fix: serveStoredObject() keeps streaming objects up to 31 MiB through the function and answers anything larger with a 302 to a V4 signed Storage URL (valid 24h, redirect cached 1h) that carries the same Content-Type and Content-Disposition. If signing fails it returns an uncached 500 naming the repair, never a truncated body. Applies to /download, /original and bare /assets/:id on non-image types. The functions sign via IAM Credentials signBlob as the default compute service account, which needs roles/iam.serviceAccountTokenCreator on itself: - iamcredentials.googleapis.com added to REQUIRED_APIS and cloudbuild - localess:deploy grants the role after deploying functions - localess:check reports it (URL signing) and --fix grants it - cloudbuild grants it after firebase deploy --- cloudbuild.yaml | 15 ++++ docs/cdn-caching.md | 2 + docs/deployment/check.md | 15 ++++ docs/deployment/firebase-setup.md | 9 +- docs/v1-functions-api.md | 2 +- functions/src/utils/stream-object.ts | 116 ++++++++++++++++++++++---- functions/src/v1/cdn-assets.test.ts | 64 +++++++++++++- functions/src/v1/cdn.ts | 25 +++--- scripts/localess/apis.mjs | 3 + scripts/localess/apis.test.mjs | 3 +- scripts/localess/checks.mjs | 38 ++++++++- scripts/localess/checks.test.mjs | 30 ++++++- scripts/localess/commands/check.mjs | 45 +++++++--- scripts/localess/commands/deploy.mjs | 5 ++ scripts/localess/firebase-gaps.mjs | 63 ++++++++++++++ scripts/localess/url-signing.mjs | 78 +++++++++++++++++ scripts/localess/url-signing.test.mjs | 28 +++++++ 17 files changed, 494 insertions(+), 47 deletions(-) create mode 100644 scripts/localess/url-signing.mjs create mode 100644 scripts/localess/url-signing.test.mjs diff --git a/cloudbuild.yaml b/cloudbuild.yaml index 1a3d4ccd7..10e34daee 100644 --- a/cloudbuild.yaml +++ b/cloudbuild.yaml @@ -23,6 +23,7 @@ steps: pubsub.googleapis.com \ storage.googleapis.com \ translate.googleapis.com \ + iamcredentials.googleapis.com \ --project=$PROJECT_ID - name: 'node:24' id: 'install-deps' @@ -77,6 +78,20 @@ steps: - name: ghcr.io/lessify/firebase:edge id: 'firebase-deploy' args: [ 'deploy', '--config=firebase.$PROJECT_ID.json', '--project=$PROJECT_ID', '--only=hosting,functions,storage,firestore,auth', '--force' ] + # let the functions sign Storage URLs as themselves: assets over 31 MiB are redirected to one, + # because Hosting truncates function responses at 32 MiB (mirrors scripts/localess/url-signing.mjs) + - name: 'gcr.io/google.com/cloudsdktool/cloud-sdk' + id: 'grant-url-signing' + entrypoint: 'bash' + allowFailure: true + args: + - '-c' + - | + SA="$PROJECT_NUMBER-compute@developer.gserviceaccount.com" + gcloud iam service-accounts add-iam-policy-binding "$$SA" \ + --member="serviceAccount:$$SA" \ + --role=roles/iam.serviceAccountTokenCreator \ + --project=$PROJECT_ID options: logging: CLOUD_LOGGING_ONLY substitutions: diff --git a/docs/cdn-caching.md b/docs/cdn-caching.md index e4afc3810..7d7d6851d 100644 --- a/docs/cdn-caching.md +++ b/docs/cdn-caching.md @@ -37,6 +37,8 @@ Whenever the marker is rewritten, its generation increments. The content marker | Asset response (incl. `/original`, `/download`) | 365 days | 365 days | browser / CDN | | Asset `304 Not Modified` | 365 days | 365 days | browser / CDN | | Asset canonical-size redirect (`w`/`h` above source) | 365 days | 365 days | browser / CDN | +| Asset over 31 MiB — `302` to a signed Storage URL (valid 24 h) | 1 hour | 1 hour | browser / CDN | +| Asset over 31 MiB that could not be signed — `500` | no-cache | no-cache | — | | 400 — invalid asset transform param | 1 hour | 1 hour | browser / CDN | | 404 — space not found | 7 days | 7 days | browser / CDN | | 404 — content cache marker missing / content not found on disk (content routes only) | 10 min | 10 min | browser / CDN | diff --git a/docs/deployment/check.md b/docs/deployment/check.md index 0b354c69e..53c933e0c 100644 --- a/docs/deployment/check.md +++ b/docs/deployment/check.md @@ -136,6 +136,20 @@ invoked by Google's own service agents, so making them public would widen access --- +## URL signing + +Firebase Hosting buffers a rewritten function response and cuts it off at 32 MiB, still answering +`200` with a matching `Content-Length` — so a large video "downloads" as its first 32 MiB. The +asset API therefore redirects anything over 31 MiB to a short-lived signed Storage URL. The +functions have no key file, so they sign through the IAM Credentials `signBlob` API as their own +runtime service account (the default compute account, `-compute@developer.gserviceaccount.com`), +which needs `roles/iam.serviceAccountTokenCreator` **on itself**. Nothing grants that by default. + +`localess:deploy` grants it after each functions deploy; `--fix` grants it here. The account only +exists once functions have been deployed, so before that the row reads `?` and points at a deploy. + +--- + ## `--fix` ```bash @@ -151,6 +165,7 @@ Applies only the repairs that are **free and reversible**: | Project labels | Writes the three markers | | Local files / region | A full `syncLocalFiles`, propagating the live Firestore location everywhere | | Invoker bindings | Adds `allUsers` to `roles/run.invoker`, preserving every other binding | +| URL signing | Grants the runtime service account `roles/iam.serviceAccountTokenCreator` on itself | | Admin user | Prompts for credentials, then creates the account, its admin claim and its documents | Everything else is reported with the command that does it, because it is not safe to do as a side diff --git a/docs/deployment/firebase-setup.md b/docs/deployment/firebase-setup.md index d68a6827f..c25aa8cc4 100644 --- a/docs/deployment/firebase-setup.md +++ b/docs/deployment/firebase-setup.md @@ -92,7 +92,7 @@ In order: 1. Preflight firebase-tools present and >= 15.29.0, CLI authenticated 2. Project adopt --project, or pick from a list / create one 3. Billing link a billing account, then wait for it to become active -4. APIs check the 15 required APIs, enable whichever are off +4. APIs check the 16 required APIs, enable whichever are off 5. Firestore adopt the existing location, or ask and create in --region 6. Storage create and link the default bucket in --region 7. Storage CORS allow GET and HEAD from any origin, when the bucket has no rules @@ -116,8 +116,15 @@ firestore identitytoolkit firebasestorage firebaseextensions cloudfunctions cloudbuild artifactregistry run eventarc pubsub storage translate +iamcredentials ``` +`iamcredentials.googleapis.com` is what the functions sign Storage URLs through — assets over +31 MiB are redirected to a signed URL, because Firebase Hosting truncates function responses at +32 MiB. Signing also needs the runtime service account to hold +`roles/iam.serviceAccountTokenCreator` on itself; deploy grants that after the functions are up +(the account does not exist before), and `localess:check` reports it. + ### Authentication `firebase.json` carries an `auth` block, which the `auth` deploy target sends to Google's diff --git a/docs/v1-functions-api.md b/docs/v1-functions-api.md index 30d0b7edb..575dec2c0 100644 --- a/docs/v1-functions-api.md +++ b/docs/v1-functions-api.md @@ -37,7 +37,7 @@ Content delivery with cache-busting and asset transformation. All content/transl - **`thumbnail` param** — Collapses an animated WebP/GIF to its first frame, and extracts a video frame via FFmpeg (requires `w`). Has no effect on other image types. Since animations now resize with every frame intact, `thumbnail` is how you ask for a *still* rather than how you make resizing work. - **EXIF orientation is applied** — Sharp strips the orientation tag on re-encode, so a rotated source is baked into the pixels. Without it a portrait phone photo returned landscape with its aspect ratio transposed. - **Embedded colour profiles are carried through** — via `keepIccProfile()`. Sources without one gain nothing; tagging every response as sRGB would add ~506 bytes each. -- **`/original` and `/download`** — serve the stored bytes exactly as uploaded, inline and as an `attachment` respectively. Since a bare transform request re-encodes, these are the only way to retrieve the original file. Neither enters Sharp, and both reject `w`/`h`/`q`/`f`/`fit`/`thumbnail`/`download` with `400` rather than ignoring them. +- **`/original` and `/download`** — serve the stored bytes exactly as uploaded, inline and as an `attachment` respectively. Since a bare transform request re-encodes, these are the only way to retrieve the original file. Neither enters Sharp, and both reject `w`/`h`/`q`/`f`/`fit`/`thumbnail`/`download` with `400` rather than ignoring them. Objects over 31 MiB are answered with a `302` to a signed Storage URL (24 h validity, redirect cached 1 h) carrying the same `Content-Type` and `Content-Disposition`, because Firebase Hosting truncates function responses at 32 MiB; smaller ones are streamed from Storage with `Range` support. The same applies to a bare `/assets/:assetId` request for a non-image type (video, audio, PDF). Signing needs the runtime service account to hold `roles/iam.serviceAccountTokenCreator` on itself — see [check.md](deployment/check.md#url-signing). - **Removed in v4** — the `?download` flag and `f=original`. Both return `400` with a message naming the replacement route. Responses already cached under the old spellings keep serving for the remainder of their 365-day TTL. #### Asset resize combinations (`w` / `h`) diff --git a/functions/src/utils/stream-object.ts b/functions/src/utils/stream-object.ts index 01b5aecf2..a42d4d442 100644 --- a/functions/src/utils/stream-object.ts +++ b/functions/src/utils/stream-object.ts @@ -3,13 +3,26 @@ import { logger } from 'firebase-functions'; import type { Readable } from 'stream'; /** - * Cloud Run rejects a response larger than 32 MiB with a bare `500` unless it is streamed with - * chunked transfer encoding — and a response carrying `Content-Length` is not streamed in that - * sense. Bodies at or under this size keep their `Content-Length` (so clients show progress); - * anything larger is sent without one, which is what makes Node fall back to chunked encoding. - * Kept a little under the hard limit to leave room for the headers. + * Largest object served through the function; anything bigger is redirected to Storage. + * + * Firebase Hosting buffers a rewritten function response and cuts it off at 32 MiB — a 104 MiB + * video arrived as its first 32 MiB with a `200` and a matching `Content-Length`, so the browser + * reported a finished download. Chunked encoding does not help: it gets past Cloud Run's own + * 32 MiB limit, but not past Hosting's. Kept a little under the limit to leave room for headers. + */ +export const MAX_PROXIED_BYTES = 31 * 1024 * 1024; + +/** + * How long a signed Storage URL stays valid. Long enough that a video player can still seek + * (each seek is a fresh `Range` request against the signed URL) well after the page loaded. */ -export const MAX_CONTENT_LENGTH_BYTES = 31 * 1024 * 1024; +export const SIGNED_URL_TTL_SECONDS = 24 * 60 * 60; + +/** + * How long the redirect itself may be cached, by the browser and by the Hosting CDN. Well under + * the signed URL's lifetime, so a cached redirect never points at an expired signature. + */ +export const SIGNED_REDIRECT_MAX_AGE = 60 * 60; export type ByteRange = { start: number; end: number }; @@ -45,24 +58,99 @@ export function parseByteRange(header: string | undefined, size: number): ByteRa } /** The slice of a Storage `File` this needs, so tests can hand in a fake. */ -export interface StreamableFile { +export interface StoredFile { createReadStream(options?: { start?: number; end?: number; validation?: false }): Readable; + getSignedUrl(config: { + version: 'v4'; + action: 'read'; + expires: number; + responseDisposition?: string; + responseType?: string; + }): Promise<[string]>; +} + +/** The headers the stored object must be served with, wherever it is served from. */ +export interface StoredObjectHeaders { + contentType: string; + contentDisposition: string; +} + +/** + * Serves a stored object: streamed through the function when Hosting can carry it, otherwise a + * `302` to a short-lived signed Storage URL. The caller sets the type, disposition, cache and + * security headers first; the redirect overrides the cache header and repeats type and + * disposition as signed response overrides, so Storage serves the same filename. + * + * Signing calls the IAM Credentials `signBlob` API, so the runtime service account needs + * `iam.serviceAccounts.signBlob` on itself (`npm run localess:check -- --fix` grants it). When + * that is missing this answers an uncached `500` — never the truncated body Hosting would make + * of a streamed one. + * @param {Request} req Incoming request, read for `Range` and the method + * @param {Response} res Response to write + * @param {StoredFile} file Storage object to serve + * @param {number} size Object size in bytes, from its metadata + * @param {StoredObjectHeaders} headers Type and disposition to serve the object with + * @param {string} tag Log prefix of the calling route + * @return {Promise} resolves once the response is handed off + */ +export async function serveStoredObject( + req: Request, + res: Response, + file: StoredFile, + size: number, + headers: StoredObjectHeaders, + tag: string +): Promise { + if (size <= MAX_PROXIED_BYTES) { + streamStoredObject(req, res, file, size, tag); + return; + } + let url: string; + try { + [url] = await file.getSignedUrl({ + version: 'v4', + action: 'read', + expires: Date.now() + SIGNED_URL_TTL_SECONDS * 1000, + responseDisposition: headers.contentDisposition, + responseType: headers.contentType, + }); + } catch (error) { + logger.error(`${tag} could not sign a Storage URL for a ${size}-byte object`, error); + res.removeHeader('Content-Disposition'); + res + .status(500) + .header('Cache-Control', 'no-cache') + .contentType('text/plain') + .send( + 'This asset is too large to serve through Hosting, and the function could not sign a Storage URL for it. ' + + 'Grant the functions service account roles/iam.serviceAccountTokenCreator on itself: npm run localess:check -- --fix' + ); + return; + } + logger.info(`${tag} ${size} bytes, redirecting to Storage`); + res.removeHeader('Content-Disposition'); + res.header('Cache-Control', `public, max-age=${SIGNED_REDIRECT_MAX_AGE}, s-maxage=${SIGNED_REDIRECT_MAX_AGE}`).redirect(302, url); } /** * Streams a stored object to the response, honouring a single `Range`. Headers other than * length and range (type, disposition, cache) must already be set by the caller. * - * Replaces `download()` to `/tmp` + `sendFile`: on Cloud Functions `/tmp` is RAM, so that held - * the whole object in memory, and `sendFile` always sets `Content-Length` — which is what made - * every object over 32 MiB fail. + * Replaces `download()` to `/tmp` + `sendFile`, which held the whole object in memory: `/tmp` + * is RAM on Cloud Functions. * @param {Request} req Incoming request, read for `Range` and the method * @param {Response} res Response to stream into - * @param {StreamableFile} file Storage object to read + * @param {StoredFile} file Storage object to read * @param {number} size Object size in bytes, from its metadata * @param {string} tag Log prefix of the calling route */ -export function streamStoredObject(req: Request, res: Response, file: StreamableFile, size: number, tag: string): void { +export function streamStoredObject( + req: Request, + res: Response, + file: Pick, + size: number, + tag: string +): void { res.header('Accept-Ranges', 'bytes'); const range = parseByteRange(req.headers.range, size); if (range === 'unsatisfiable') { @@ -74,9 +162,7 @@ export function streamStoredObject(req: Request, res: Response, file: Streamable if (range) { res.status(206).header('Content-Range', `bytes ${start}-${end}/${size}`); } - if (length <= MAX_CONTENT_LENGTH_BYTES) { - res.header('Content-Length', String(length)); - } + res.header('Content-Length', String(length)); if (req.method === 'HEAD' || length === 0) { res.end(); return; diff --git a/functions/src/v1/cdn-assets.test.ts b/functions/src/v1/cdn-assets.test.ts index cc56e872e..330e265c6 100644 --- a/functions/src/v1/cdn-assets.test.ts +++ b/functions/src/v1/cdn-assets.test.ts @@ -19,6 +19,9 @@ import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; * The env has to be set before the module graph loads, which is why the imports are dynamic. */ const MD5 = 'abc123=='; +const SIGNED_URL = 'https://storage.googleapis.com/test-project.appspot.com/signed'; +/** Past what Firebase Hosting can carry, so the object is redirected rather than streamed. */ +const LARGE = 40 * 1024 * 1024; let CDN: express.Router; let bucket: { file: (path: string) => unknown }; @@ -82,13 +85,24 @@ interface StoredAsset { storageMissing?: boolean; /** Simulates a deleted asset still referenced by published content. */ documentMissing?: boolean; + /** Overrides the stored object's size, to exercise the large-object path without the bytes. */ + size?: number; + /** Simulates a runtime service account that is not allowed to sign URLs. */ + signingFails?: boolean; } -function given(asset: StoredAsset): { download: ReturnType; createReadStream: ReturnType } { +function given(asset: StoredAsset): { + download: ReturnType; + createReadStream: ReturnType; + getSignedUrl: ReturnType; +} { const file = { getMetadata: asset.storageMissing ? vi.fn().mockRejectedValue(new Error('not found')) - : vi.fn().mockResolvedValue([{ md5Hash: MD5, size: String(asset.bytes.length) }]), + : vi.fn().mockResolvedValue([{ md5Hash: MD5, size: String(asset.size ?? asset.bytes.length) }]), + getSignedUrl: asset.signingFails + ? vi.fn().mockRejectedValue(new Error('Permission iam.serviceAccounts.signBlob denied')) + : vi.fn().mockResolvedValue([SIGNED_URL]), createReadStream: vi .fn() .mockImplementation((options?: { start?: number; end?: number }) => @@ -188,6 +202,52 @@ describe('asset routes — the passthrough pair', () => { expect(res.headers['content-range']).toBe('bytes */10'); }); + it('/download redirects an object too large for Hosting to a signed Storage URL', async () => { + const file = given({ bytes: Buffer.from('x'), size: LARGE, type: 'video/quicktime', extension: '.mov' }); + + const res = await request(app()).get(`${URL}/download`); + + expect(res.status).toBe(302); + expect(res.headers['location']).toBe(SIGNED_URL); + expect(res.headers['cache-control']).toContain('max-age=3600'); + expect(res.headers['content-disposition']).toBeUndefined(); + expect(file.createReadStream).not.toHaveBeenCalled(); + // Storage must hand back the same filename and type the function would have sent. + const config = file.getSignedUrl.mock.calls[0][0]; + expect(config).toMatchObject({ version: 'v4', action: 'read', responseType: 'video/quicktime' }); + expect(config.responseDisposition).toContain('attachment'); + expect(config.responseDisposition).toContain('photo.mov'); + }); + + it('/original signs an inline disposition for a large object', async () => { + const file = given({ bytes: Buffer.from('x'), size: LARGE, type: 'video/mp4', extension: '.mp4' }); + + const res = await request(app()).get(`${URL}/original`); + + expect(res.status).toBe(302); + expect(file.getSignedUrl.mock.calls[0][0].responseDisposition).toContain('inline'); + }); + + it('a large object that cannot be signed fails loudly and uncached, never truncated', async () => { + const file = given({ bytes: Buffer.from('x'), size: LARGE, type: 'video/quicktime', extension: '.mov', signingFails: true }); + + const res = await request(app()).get(`${URL}/download`); + + expect(res.status).toBe(500); + expect(res.headers['cache-control']).toBe('no-cache'); + expect(res.text).toContain('localess:check'); + expect(file.createReadStream).not.toHaveBeenCalled(); + }); + + it('a bare transform-route URL on a large video redirects too', async () => { + given({ bytes: Buffer.from('x'), size: LARGE, type: 'video/mp4', extension: '.mp4' }); + + const res = await request(app()).get(URL); + + expect(res.status).toBe(302); + expect(res.headers['location']).toBe(SIGNED_URL); + }); + it('a bare transform-route URL on a video streams the stored bytes', async () => { const bytes = Buffer.from('video bytes'); const file = given({ bytes, type: 'video/mp4', extension: '.mp4' }); diff --git a/functions/src/v1/cdn.ts b/functions/src/v1/cdn.ts index 4815928f4..fc4b10dad 100644 --- a/functions/src/v1/cdn.ts +++ b/functions/src/v1/cdn.ts @@ -55,7 +55,7 @@ import { assetResponsePolicy } from '../utils/asset-headers'; import { buildContentDisposition } from '../utils/content-disposition'; import { redactQuery } from '../utils/log-redact'; import { resolveLocaleFilePath } from '../utils/locale-utils'; -import { streamStoredObject } from '../utils/stream-object'; +import { serveStoredObject } from '../utils/stream-object'; import { RequestWithToken, requireContentPermissions, @@ -725,20 +725,22 @@ CDN.get('/api/v1/spaces/:spaceId/assets/:assetId', async (req, res) => { overwriteType = format ? formatMimeMap[format] : 'image/webp'; filename = `${asset.name}-${suffix || `w${width}`}-thumbnail${format ? formatExtMap[format] : '.webp'}`; } else { - // Video, audio, PDF and the rest pass through untouched, so they are streamed rather than - // staged in `/tmp` — see the `/download` route for why that matters past 32 MiB. + // Video, audio, PDF and the rest pass through untouched, so they are streamed (or, past + // what Hosting can carry, redirected to Storage) rather than staged in `/tmp`. streamed = true; } const policy = assetResponsePolicy(overwriteType || asset.type, false); + const contentDisposition = buildContentDisposition(filename, policy.attachment); res .header('Cache-Control', `public, max-age=${CACHE_ASSET_MAX_AGE}, s-maxage=${CACHE_ASSET_MAX_AGE}`) - .header('Content-Disposition', buildContentDisposition(filename, policy.attachment)) + .header('Content-Disposition', contentDisposition) .set(policy.headers) .contentType(policy.contentType); if (output) { res.send(output); } else if (streamed) { - streamStoredObject(req, res, assetFile, Number(objectMetadata?.['size'] ?? 0), '[V1:AssetById]'); + const size = Number(objectMetadata?.['size'] ?? 0); + await serveStoredObject(req, res, assetFile, size, { contentType: policy.contentType, contentDisposition }, '[V1:AssetById]'); } else { res.sendFile(tempFilePath); } @@ -836,15 +838,16 @@ CDN.get(['/api/v1/spaces/:spaceId/assets/:assetId/original', '/api/v1/spaces/:sp } } - // Streamed straight from Storage rather than downloaded to `/tmp` and sent with `sendFile`: - // `/tmp` is RAM on Cloud Functions, and `sendFile` always sets `Content-Length`, which makes - // Cloud Run reject anything over 32 MiB — a 100 MB video failed with a bare `500`. The stream - // honours `Range` itself, so a client can still resume a partial transfer. + // Streamed straight from Storage rather than staged in `/tmp` (RAM on Cloud Functions), and + // redirected to a signed Storage URL past what Firebase Hosting can carry — see + // `serveStoredObject`. Either way `Range` works, so a client can resume a partial transfer. const policy = assetResponsePolicy(asset.type, attachment); + const contentDisposition = buildContentDisposition(`${asset.name}${asset.extension}`, policy.attachment); res .header('Cache-Control', `public, max-age=${CACHE_ASSET_MAX_AGE}, s-maxage=${CACHE_ASSET_MAX_AGE}`) - .header('Content-Disposition', buildContentDisposition(`${asset.name}${asset.extension}`, policy.attachment)) + .header('Content-Disposition', contentDisposition) .set(policy.headers) .contentType(policy.contentType); - streamStoredObject(req, res, assetFile, Number(objectMetadata?.['size'] ?? 0), tag); + const size = Number(objectMetadata?.['size'] ?? 0); + await serveStoredObject(req, res, assetFile, size, { contentType: policy.contentType, contentDisposition }, tag); }); diff --git a/scripts/localess/apis.mjs b/scripts/localess/apis.mjs index f66d18270..093cf21a4 100644 --- a/scripts/localess/apis.mjs +++ b/scripts/localess/apis.mjs @@ -32,6 +32,9 @@ export const REQUIRED_APIS = Object.freeze([ 'pubsub.googleapis.com', 'storage.googleapis.com', 'translate.googleapis.com', + // The functions sign Storage URLs for assets too large to proxy through Hosting, and + // signing without a key file goes through this API's `signBlob`. + 'iamcredentials.googleapis.com', ]); /** diff --git a/scripts/localess/apis.test.mjs b/scripts/localess/apis.test.mjs index 5c4bf56d4..a90dd836d 100644 --- a/scripts/localess/apis.test.mjs +++ b/scripts/localess/apis.test.mjs @@ -37,9 +37,10 @@ test('REQUIRED_APIS is frozen so a caller cannot mutate the shared list', () => }); test('missingApis returns a fresh array rather than the shared constant', () => { + const before = REQUIRED_APIS.length; const missing = missingApis(null); missing.pop(); - assert.equal(REQUIRED_APIS.length, 15); + assert.equal(REQUIRED_APIS.length, before); }); test('cloudbuild.yaml enables exactly the required APIs', () => { diff --git a/scripts/localess/checks.mjs b/scripts/localess/checks.mjs index 04b5c5d90..501ac8399 100644 --- a/scripts/localess/checks.mjs +++ b/scripts/localess/checks.mjs @@ -14,6 +14,7 @@ */ import { missingApis, REQUIRED_APIS } from './apis.mjs'; import { needsBucketCors } from './bucket-cors.mjs'; +import { SIGNER_ROLE } from './url-signing.mjs'; import { MANAGED_LABEL, REGION_LABEL, VERSION_LABEL, hasMarker, markerRegion, markerVersion } from './markers.mjs'; export const OK = 'ok'; @@ -38,6 +39,7 @@ export const FIX_CORS = 'cors'; export const FIX_LABELS = 'labels'; export const FIX_LOCAL_FILES = 'localFiles'; export const FIX_INVOKERS = 'invokers'; +export const FIX_SIGNING = 'signing'; export const FIX_ADMIN_USER = 'adminUser'; /** @@ -52,7 +54,15 @@ export const FIX_ADMIN_USER = 'adminUser'; * repair sorts to index -1 and would silently run first - which the FIX_ORDER coverage test * exists to prevent. */ -export const FIX_ORDER = Object.freeze([FIX_APIS, FIX_CORS, FIX_LABELS, FIX_LOCAL_FILES, FIX_INVOKERS, FIX_ADMIN_USER]); +export const FIX_ORDER = Object.freeze([ + FIX_APIS, + FIX_CORS, + FIX_LABELS, + FIX_LOCAL_FILES, + FIX_INVOKERS, + FIX_SIGNING, + FIX_ADMIN_USER, +]); const result = (id, group, title, status, detail, { fix = null, hint = null } = {}) => ({ id, @@ -321,6 +331,31 @@ export function checkInvokers(invokers, projectId) { ); } +/** + * Whether the functions can sign Storage URLs as their runtime service account. + * + * Firebase Hosting truncates function responses at 32 MiB, so larger assets are redirected + * to a signed Storage URL - and signing without a key file needs the account to hold + * `iam.serviceAccounts.signBlob` on itself. Without it every large asset download fails. + */ +export function checkUrlSigning(signing, projectId) { + const title = 'URL signing'; + if (!signing || signing.granted === null) { + return result('signing', DEPLOYMENT, title, UNKNOWN, 'could not read the runtime service account policy'); + } + if (signing.granted === undefined) { + return result('signing', DEPLOYMENT, title, UNKNOWN, `${signing.serviceAccount} does not exist yet`, { + hint: `npm run localess:deploy -- --project ${projectId} --only functions`, + }); + } + if (signing.granted) { + return result('signing', DEPLOYMENT, title, OK, `${signing.serviceAccount} can sign`); + } + return result('signing', DEPLOYMENT, title, MISSING, `${signing.serviceAccount} lacks ${SIGNER_ROLE}; assets over 31 MiB cannot be downloaded`, { + fix: FIX_SIGNING, + }); +} + /** Email/password is the provider Localess provisions; the rest are a console decision. */ export function checkEmailProvider(identityConfig, projectId) { if (identityConfig === null || identityConfig === undefined) { @@ -408,6 +443,7 @@ export function buildReport(facts) { checkLocalFiles(facts.absentLocalFiles, facts.localFileCount), checkFunctions(facts.deployedFunctions, facts.expectedGroups, projectId), checkInvokers(facts.invokers, projectId), + checkUrlSigning(facts.signing, projectId), checkEmailProvider(facts.identityConfig, projectId), checkAdminUser(facts.accounts, facts.accountLimit, projectId), ]; diff --git a/scripts/localess/checks.test.mjs b/scripts/localess/checks.test.mjs index a84d529bb..64ef8e2ff 100644 --- a/scripts/localess/checks.test.mjs +++ b/scripts/localess/checks.test.mjs @@ -14,6 +14,7 @@ import { FIX_LABELS, FIX_LOCAL_FILES, FIX_ORDER, + FIX_SIGNING, INFRASTRUCTURE, MISSING, OK, @@ -29,6 +30,7 @@ import { checkFunctions, checkHostingSite, checkInvokers, + checkUrlSigning, checkLabels, checkLocalFiles, checkRegionAgreement, @@ -401,6 +403,7 @@ const healthyFacts = { deployedFunctions: [{ id: 'setup', entryPoint: 'setup', callableTrigger: {} }], expectedGroups: ['setup'], invokers: [{ id: 'setup', hasInvoker: true }], + signing: { serviceAccount: '1-compute@developer.gserviceaccount.com', granted: true }, identityConfig: { signIn: { email: { enabled: true } } }, accounts: { recordsCount: 1, users: [{ email: 'a@example.com', customAttributes: '{"role":"admin"}' }] }, accountLimit: 500, @@ -411,7 +414,7 @@ const healthyFacts = { test('buildReport returns every check, grouped in install order', () => { const results = buildReport(healthyFacts); - assert.equal(results.length, 14); + assert.equal(results.length, 15); const groups = [...new Set(results.map(entry => entry.group))]; assert.deepEqual(groups, [INFRASTRUCTURE, DEPLOYMENT, APPLICATION]); }); @@ -430,7 +433,7 @@ test('a healthy project reports no problems and exits 0', () => { test('buildReport survives a project where nothing could be read', () => { const blind = { projectId: PROJECT, absentLocalFiles: [], localFileCount: 5, expectedGroups: [], regions: {} }; const results = buildReport(blind); - assert.equal(results.length, 14); + assert.equal(results.length, 15); // Nothing was established, so nothing is reported as broken. assert.equal(exitCodeFor(results), 0); }); @@ -477,9 +480,32 @@ test('FIX_ORDER covers every repair a check can ask for', () => { checkLabels({}), checkLocalFiles(['x'], 5), checkInvokers([{ id: 'setup', hasInvoker: false }], PROJECT), + checkUrlSigning({ serviceAccount: 'sa', granted: false }, PROJECT), checkAdminUser({ recordsCount: 0, users: [] }, 500, PROJECT), ].map(entry => entry.fix), ); for (const fix of everyFix) assert.ok(FIX_ORDER.includes(fix), `${fix} is absent from FIX_ORDER`); assert.equal(everyFix.size, FIX_ORDER.length); }); + +test('checkUrlSigning is ok when the runtime account can sign', () => { + assert.equal(checkUrlSigning({ serviceAccount: 'sa', granted: true }, PROJECT).status, OK); +}); + +test('checkUrlSigning offers the repair when the role is missing', () => { + const entry = checkUrlSigning({ serviceAccount: 'sa', granted: false }, PROJECT); + assert.equal(entry.status, MISSING); + assert.equal(entry.fix, FIX_SIGNING); + assert.match(entry.detail, /31 MiB/); +}); + +test('checkUrlSigning points at a functions deploy when the account does not exist yet', () => { + const entry = checkUrlSigning({ serviceAccount: 'sa', granted: undefined }, PROJECT); + assert.equal(entry.status, UNKNOWN); + assert.match(entry.hint, /--only functions/); +}); + +test('checkUrlSigning reports unknown when the policy could not be read', () => { + assert.equal(checkUrlSigning({ serviceAccount: 'sa', granted: null }, PROJECT).status, UNKNOWN); + assert.equal(checkUrlSigning(null, PROJECT).status, UNKNOWN); +}); diff --git a/scripts/localess/commands/check.mjs b/scripts/localess/commands/check.mjs index 6db041f85..dea185a13 100644 --- a/scripts/localess/commands/check.mjs +++ b/scripts/localess/commands/check.mjs @@ -39,6 +39,7 @@ import { } from '../firebase-gaps.mjs'; import { ensureRequiredApis } from '../apis.mjs'; import { ensureBucketCors } from '../bucket-cors.mjs'; +import { ensureUrlSigning, readUrlSigning } from '../url-signing.mjs'; import { PASSWORD_ENV, ensureFirstAdmin } from '../admin-user.mjs'; import { buildMarkerLabels, markerRegion } from '../markers.mjs'; import { @@ -50,6 +51,7 @@ import { FIX_INVOKERS, FIX_LABELS, FIX_LOCAL_FILES, + FIX_SIGNING, INFRASTRUCTURE, MISSING, OK, @@ -138,19 +140,31 @@ async function readInvokers(projectId, deployed) { async function gather(projectId) { log.step(`Inspecting ${projectId}`); - const [billingEnabled, enabledApis, databases, bucketName, webApps, hostingSites, labels, deployedFunctions, identityConfig, accounts] = - await Promise.all([ - attempt(isBillingEnabled(projectId)), - listEnabledApis(projectId), - attempt(cli.listFirestoreDatabases(projectId)), - attempt(getDefaultBucket(projectId), undefined), - attempt(cli.listWebApps(projectId)), - attempt(cli.listHostingSites(projectId)), - readProjectLabels(projectId), - attempt(cli.listFunctions(projectId)), - getIdentityConfig(projectId), - queryIdentityUsers(projectId, ACCOUNT_SCAN_LIMIT), - ]); + const [ + billingEnabled, + enabledApis, + databases, + bucketName, + webApps, + hostingSites, + labels, + deployedFunctions, + identityConfig, + accounts, + signing, + ] = await Promise.all([ + attempt(isBillingEnabled(projectId)), + listEnabledApis(projectId), + attempt(cli.listFirestoreDatabases(projectId)), + attempt(getDefaultBucket(projectId), undefined), + attempt(cli.listWebApps(projectId)), + attempt(cli.listHostingSites(projectId)), + readProjectLabels(projectId), + attempt(cli.listFunctions(projectId)), + getIdentityConfig(projectId), + queryIdentityUsers(projectId, ACCOUNT_SCAN_LIMIT), + attempt(readUrlSigning(projectId)), + ]); const [bucketCors, invokers, live] = await Promise.all([ bucketName ? getBucketCors(bucketName) : null, @@ -176,6 +190,7 @@ async function gather(projectId) { deployedFunctions, expectedGroups, invokers, + signing, identityConfig, accounts, accountLimit: ACCOUNT_SCAN_LIMIT, @@ -296,6 +311,10 @@ async function applyFix(fix, facts, options) { return; } + case FIX_SIGNING: + await ensureUrlSigning(projectId, log); + return; + case FIX_ADMIN_USER: // Talks to the Identity Platform API directly, so this works even on a project whose // functions failed to deploy - which is exactly the project this command diagnoses. diff --git a/scripts/localess/commands/deploy.mjs b/scripts/localess/commands/deploy.mjs index 1e91d418e..26f69438c 100644 --- a/scripts/localess/commands/deploy.mjs +++ b/scripts/localess/commands/deploy.mjs @@ -21,6 +21,7 @@ import { buildMarkerLabels } from '../markers.mjs'; import { mergeProjectLabels } from '../firebase-gaps.mjs'; import { ensureRequiredApis } from '../apis.mjs'; import { ensureBucketCors } from '../bucket-cors.mjs'; +import { ensureUrlSigning } from '../url-signing.mjs'; import { assertManaged, identifyProject } from '../projects.mjs'; import { confirmDeployPlan } from '../prompts.mjs'; import { ROOT, createLogger, rel } from '../log.mjs'; @@ -328,6 +329,10 @@ export async function run(argv) { log.step('Deploying'); await deployWithRetry(deployArgs, env); + // After the deploy, because the runtime service account only exists once functions have + // been deployed. Best-effort: it gates large asset downloads, not the deploy. + if (deployingFunctions) await ensureUrlSigning(projectId, log); + log.step('Refreshing the version marker'); try { await mergeProjectLabels(projectId, buildMarkerLabels(VERSION, region)); diff --git a/scripts/localess/firebase-gaps.mjs b/scripts/localess/firebase-gaps.mjs index 0a3f47a40..55392fb60 100644 --- a/scripts/localess/firebase-gaps.mjs +++ b/scripts/localess/firebase-gaps.mjs @@ -97,6 +97,12 @@ async function init() { urlPrefix: 'https://identitytoolkit.googleapis.com', apiVersion: 'admin/v2', }), + // Service-account IAM policies. The function runtime signs Storage URLs as itself, which + // needs a binding on the service account resource - not on the project. + iam: new Client({ + urlPrefix: 'https://iam.googleapis.com', + apiVersion: 'v1', + }), // Document writes. The Admin SDK is not available here, so the bootstrap writes its // documents through the same authenticated client everything else uses. firestore: new Client({ @@ -315,6 +321,63 @@ export async function readProjectLabels(projectId) { } } +/** + * The project number, or `null` when the project cannot be read. Needed to name the default + * compute service account, which is keyed by number rather than id. + */ +export async function getProjectNumber(projectId) { + try { + const { resourceManager } = await api(); + const response = await resourceManager.get(`/projects/${projectId}`); + return response.body?.name?.split('/')[1] ?? null; + } catch { + return null; + } +} + +/** + * The IAM policy *of* a service account (who may act as it), `undefined` when the account + * does not exist, or `null` when the policy cannot be read. + * + * Absent is its own answer because the default compute service account only appears once + * the Compute API is on, which the first functions deploy does - so before that there is + * nothing to grant on, rather than something wrong. + */ +export async function getServiceAccountIamPolicy(projectId, email) { + try { + const { iam } = await api(); + const res = await iam.post(`/projects/${projectId}/serviceAccounts/${email}:getIamPolicy`, {}, { resolveOnHTTPError: true }); + if (res.status === 404) return undefined; + if (res.status >= 400) return null; + return res.body ?? {}; + } catch { + return null; + } +} + +/** Adds `member` to `role` on a service account, preserving every other binding. */ +export async function addServiceAccountBinding(projectId, email, role, member) { + const { iam } = await api(); + const policy = await getServiceAccountIamPolicy(projectId, email); + if (!policy) throw new Error(`Could not read the IAM policy of ${email}`); + + const bindings = policy.bindings ?? []; + const existing = bindings.find(binding => binding.role === role); + const merged = existing + ? bindings.map(binding => + binding === existing ? { ...binding, members: [...new Set([...(binding.members ?? []), member])] } : binding, + ) + : [...bindings, { role, members: [member] }]; + + const res = await iam.post( + `/projects/${projectId}/serviceAccounts/${email}:setIamPolicy`, + { policy: { ...policy, bindings: merged } }, + { resolveOnHTTPError: true }, + ); + if (res.status >= 400) throw fail(`Could not grant ${role} on ${email}`, res); + return res.body; +} + /** * Merges labels into a project, leaving every other label untouched. * diff --git a/scripts/localess/url-signing.mjs b/scripts/localess/url-signing.mjs new file mode 100644 index 000000000..02a7d836b --- /dev/null +++ b/scripts/localess/url-signing.mjs @@ -0,0 +1,78 @@ +/** + * Lets the functions sign Storage URLs. + * + * Firebase Hosting cuts a rewritten function response off at 32 MiB, so the asset API + * redirects anything larger to a short-lived signed Storage URL (functions/src/utils/ + * stream-object.ts). The functions have no key file, so they sign through the IAM + * Credentials `signBlob` API as their own runtime service account - which needs + * `iam.serviceAccounts.signBlob` *on itself*. Nothing grants that by default, and without it + * every large asset download fails. + * + * Applied after each functions deploy and checked by `localess:check`, like the invoker + * bindings: the account only exists once functions have been deployed. + */ +import { addServiceAccountBinding, getProjectNumber, getServiceAccountIamPolicy } from './firebase-gaps.mjs'; + +/** The narrowest predefined role carrying `iam.serviceAccounts.signBlob`. */ +export const SIGNER_ROLE = 'roles/iam.serviceAccountTokenCreator'; + +/** Gen-2 functions run as the default compute service account unless told otherwise. */ +export function runtimeServiceAccount(projectNumber) { + return `${projectNumber}-compute@developer.gserviceaccount.com`; +} + +/** Whether `policy` lets `email` sign as itself. */ +export function canSign(policy, email) { + return (policy?.bindings ?? []).some( + binding => binding.role === SIGNER_ROLE && (binding.members ?? []).includes(`serviceAccount:${email}`), + ); +} + +/** + * The signing state: `{ serviceAccount, granted }`, where `granted` is `true`, `false`, or + * `null` when it could not be read. `serviceAccount` is `null` when the project number is + * unreadable, and `granted` is `undefined` when the account does not exist yet. + */ +export async function readUrlSigning(projectId) { + const projectNumber = await getProjectNumber(projectId); + if (!projectNumber) return { serviceAccount: null, granted: null }; + + const serviceAccount = runtimeServiceAccount(projectNumber); + const policy = await getServiceAccountIamPolicy(projectId, serviceAccount); + if (policy === undefined) return { serviceAccount, granted: undefined }; + if (policy === null) return { serviceAccount, granted: null }; + return { serviceAccount, granted: canSign(policy, serviceAccount) }; +} + +/** + * Grants the runtime service account the right to sign as itself, when it lacks it. + * + * Best-effort, like the bucket CORS: it needs `iam.serviceAccounts.setIamPolicy`, and an + * operator without it should still get a deploy. Only large asset downloads depend on it, + * and `localess:check` keeps reporting the gap until it is closed. + */ +export async function ensureUrlSigning(projectId, log) { + log.step('Checking that the functions can sign Storage URLs'); + + const { serviceAccount, granted } = await readUrlSigning(projectId); + if (granted === true) { + log.skip(`${serviceAccount} already holds ${SIGNER_ROLE} on itself`); + return; + } + if (granted === undefined) { + log.skip(`${serviceAccount} does not exist yet; it is created by the first functions deploy`); + return; + } + if (granted === null) { + log.warn(`could not read the IAM policy of ${serviceAccount ?? 'the runtime service account'}`); + return; + } + + try { + await addServiceAccountBinding(projectId, serviceAccount, SIGNER_ROLE, `serviceAccount:${serviceAccount}`); + log.done(`${serviceAccount} can now sign Storage URLs`); + } catch (error) { + log.warn(`could not grant ${SIGNER_ROLE} on ${serviceAccount} (${error.message})`); + log.warn('assets larger than 31 MiB will fail to download until it is granted'); + } +} diff --git a/scripts/localess/url-signing.test.mjs b/scripts/localess/url-signing.test.mjs new file mode 100644 index 000000000..2b4952891 --- /dev/null +++ b/scripts/localess/url-signing.test.mjs @@ -0,0 +1,28 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; + +import { SIGNER_ROLE, canSign, runtimeServiceAccount } from './url-signing.mjs'; + +const SA = runtimeServiceAccount('123456'); + +test('runtimeServiceAccount names the default compute service account', () => { + assert.equal(SA, '123456-compute@developer.gserviceaccount.com'); +}); + +test('canSign is true when the account holds the signer role on itself', () => { + assert.equal(canSign({ bindings: [{ role: SIGNER_ROLE, members: [`serviceAccount:${SA}`] }] }, SA), true); +}); + +test('canSign is false when the role is held by someone else', () => { + const policy = { bindings: [{ role: SIGNER_ROLE, members: ['serviceAccount:other@example.iam.gserviceaccount.com'] }] }; + assert.equal(canSign(policy, SA), false); +}); + +test('canSign is false when the account holds a different role', () => { + assert.equal(canSign({ bindings: [{ role: 'roles/iam.serviceAccountUser', members: [`serviceAccount:${SA}`] }] }, SA), false); +}); + +test('canSign is false for an empty policy', () => { + assert.equal(canSign({}, SA), false); + assert.equal(canSign(undefined, SA), false); +});