From cc30b205fe89158a55c9f47cf79eb3586d104f55 Mon Sep 17 00:00:00 2001 From: Akanksha Trehun Date: Fri, 6 Nov 2026 09:17:52 +0530 Subject: [PATCH] fix(security): Snyk Code findings; React 19.3.1 Snyk Code reported 14 findings on main: - the engine's input file name took its extension from the user's file name: every engine file name now uses a letters-and-digits extension; - the service worker accepted messages from any origin: it now ignores other origins; - strip-sourcemaps.mjs (which deletes files) and build-sw-manifest.mjs took a directory from the command line: both refuse one outside the repository; - check-deploy.mjs's summary had an object key named "pass", read as a hardcoded password: keys are the RESULT constants; - test-only code (unit and browser tests, the local test server) is excluded from Snyk Code in .snyk. The other two (Cache.match "ReDoS" in sw.js, the dedicated worker's message handler) are false positives, ignored in Snyk with reasons. React and react-dom go to 19.3.1 together, with @types/react and @types/react-dom 19.3.0, superseding the Snyk and Dependabot PRs that bumped them one at a time without updating bun.lock. --- .snyk | 9 +++++++++ bun.lock | 20 +++++++++---------- docs/SECURITY-PIPELINE.md | 30 +++++++++++++++++++++++++++++ package.json | 12 ++++++------ public/sw.js | 4 +++- scripts/build-sw-manifest.mjs | 9 +++++++-- scripts/deploy/check-deploy.mjs | 8 ++++---- scripts/strip-sourcemaps.mjs | 9 +++++++-- src/lib/ffmpeg.worker.ts | 17 ++++++++++++---- src/lib/tests/ffmpegWorker.test.ts | 17 ++++++++++++++++ src/lib/tests/serviceWorker.test.ts | 25 ++++++++++++++++++++++-- 11 files changed, 129 insertions(+), 31 deletions(-) diff --git a/.snyk b/.snyk index ac66b40b3..131f29966 100644 --- a/.snyk +++ b/.snyk @@ -17,3 +17,12 @@ version: v1.25.0 ignore: {} patch: {} +# Snyk Code skips test-only code: unit and browser tests (which start plain +# HTTP servers on 127.0.0.1 as fixtures) and the local server the tests and +# the deploy smoke check use. None of it ships to users. +exclude: + code: + - "tests/**" + - "__tests__/**" + - "test/**" + - scripts/serve-isolated.mjs diff --git a/bun.lock b/bun.lock index cbe066dac..ae3933b3d 100644 --- a/bun.lock +++ b/bun.lock @@ -15,8 +15,8 @@ "lottie-web": "^5.12.2", "lucide-react": "^0.577.0", "next": "16.4.1", - "react": "19.3.0", - "react-dom": "19.3.0", + "react": "19.3.1", + "react-dom": "19.3.1", "reframe": ".", "tailwind-merge": "^3.6.0", "wasm-feature-detect": "^1.8.0", @@ -32,8 +32,8 @@ "@testing-library/user-event": "^14.4.3", "@types/bun": "^1.3.14", "@types/node": "^25", - "@types/react": "19.2.18", - "@types/react-dom": "19.2.5", + "@types/react": "19.3.0", + "@types/react-dom": "19.3.0", "chromatic": "^18.5.0", "eslint": "9.39.5", "eslint-config-next": "16.4.1", @@ -49,8 +49,8 @@ }, }, "overrides": { - "@types/react": "19.2.18", - "@types/react-dom": "19.2.5", + "@types/react": "19.3.0", + "@types/react-dom": "19.3.0", "source-map-js": "^1.2.2", "undici": "^7.30.1", "vite": "^8.2.4", @@ -689,9 +689,9 @@ "@types/node": ["@types/node@25.9.5", "", { "dependencies": { "undici-types": ">=7.24.0 <7.24.7" } }, "sha512-OScDchr2fwuUmWdf4kZ9h7PcJiYDVInhJizG/biAq3cAvqwYktuy/TYGGdZNMtNTFUP7rnb0NU4TUdm82kt4Rg=="], - "@types/react": ["@types/react@19.2.18", "", { "dependencies": { "csstype": "^3.2.2" } }, "sha512-AnzbBERsrLKtk2XSfTbYRLjQPdy116Sty4q+T+Bp3IC4l6jNBvreVPAHmpq9qhXQM7CXZPjLVmGMw9sy+hxQ3w=="], + "@types/react": ["@types/react@19.3.0", "", { "dependencies": { "csstype": "^3.2.2" } }, "sha512-N0rFCuH9YoxG9/m61l9MfpJKfmLOVU0em7ipIz6TRgSSkvReLB9vL85GB+yr8Bs5leqpvg96JSwF4ZS1s4viQg=="], - "@types/react-dom": ["@types/react-dom@19.2.5", "", { "peerDependencies": { "@types/react": "^19.2.0" } }, "sha512-fMPwH9v7r/pp43yUd2/Mbiex5KouJwwR3dzHkhLREUC6764VyDsqxhAxv6OFEYR1RhjOyD1naqba8ECDBe7ZQg=="], + "@types/react-dom": ["@types/react-dom@19.3.0", "", { "peerDependencies": { "@types/react": "^19.3.0" } }, "sha512-ZI7bU42mZXXKHn/qNLEw2IrbiINU7X5+vfgdixBHkCNpYWXjKgfQ/P+uyGb5CjOLB9UcnTeg3rylQtV2hym44Q=="], "@types/resolve": ["@types/resolve@1.20.6", "", {}, "sha512-A4STmOXPhMUtHH+S6ymgE2GiBSMqf4oTvcQZMcHzokuTLVYzXTB8ttjcgxOVaAp2lGwEdzZ0J+cRbbeevQj1UQ=="], @@ -1693,13 +1693,13 @@ "range-parser": ["range-parser@1.3.0", "", {}, "sha512-hek2mFQpPuI4E1BBKrSto+BU3e3x4xuarsbiwr3+lf7p44juvFMV0XFWQAP3xUyqXA4RrXLIoaSUGbSt056ZMw=="], - "react": ["react@19.3.0", "", {}, "sha512-E8LUcbtBWt20bbl2YoHfx4ZDBdxVTfOKtCZn9cDSJ4l6/nuoApcpIBcj47t2wZoVX8g2ZHuMHbiShgCR1T5Sog=="], + "react": ["react@19.3.1", "", {}, "sha512-QNhWdt7nrgVWWQG58QuGsAR3jv3Hbe4mb4vz7w2csnHavciZJgda1CPNaldF6eVxv90ud3jUXpKlHL69gO38yw=="], "react-docgen": ["react-docgen@8.0.3", "", { "dependencies": { "@babel/core": "^7.28.0", "@babel/traverse": "^7.28.0", "@babel/types": "^7.28.2", "@types/babel__core": "^7.20.5", "@types/babel__traverse": "^7.20.7", "@types/doctrine": "^0.0.9", "@types/resolve": "^1.20.2", "doctrine": "^3.0.0", "resolve": "^1.22.1", "strip-indent": "^4.0.0" } }, "sha512-aEZ9qP+/M+58x2qgfSFEWH1BxLyHe5+qkLNJOZQb5iGS017jpbRnoKhNRrXPeA6RfBrZO5wZrT9DMC1UqE1f1w=="], "react-docgen-typescript": ["react-docgen-typescript@2.4.0", "", { "peerDependencies": { "typescript": ">= 4.3.x" } }, "sha512-ZtAp5XTO5HRzQctjPU0ybY0RRCQO19X/8fxn3w7y2VVTUbGHDKULPTL4ky3vB05euSgG5NpALhEhDPvQ56wvXg=="], - "react-dom": ["react-dom@19.3.0", "", { "dependencies": { "scheduler": "^0.28.0" }, "peerDependencies": { "react": "^19.3.0" } }, "sha512-JDk8dgif51OjFoDE70+OT9ICyYr+69HlmihNwp1+Nsfbna3t5sIiCa9ZJktDmQ4/1b/rn26hIAR2uYXDMr5r0Q=="], + "react-dom": ["react-dom@19.3.1", "", { "dependencies": { "scheduler": "^0.28.0" }, "peerDependencies": { "react": "^19.3.1" } }, "sha512-Y0bARz53lhEC1NMCASs0/j+faB/zqt3R8hpvg1e8+61ZVJ4UL0UdC4cBbXfZMcIuhrfgeILTLe+ZeBCavQPtKQ=="], "react-is": ["react-is@17.0.2", "", {}, "sha512-w2GsyukL62IJnlaff/nRegPQR94C/XXamvMWmSHRJ4y7Ts/4ocGRmTHvOs8PSE6pB3dWOrD/nueuU5sduBsQ4w=="], diff --git a/docs/SECURITY-PIPELINE.md b/docs/SECURITY-PIPELINE.md index 8abf1384d..8c1c2642e 100644 --- a/docs/SECURITY-PIPELINE.md +++ b/docs/SECURITY-PIPELINE.md @@ -62,6 +62,36 @@ versions: none has a high or critical advisory. After the fixes: 0 blocking, 5 excepted (the two packages above), 14 moderate or low. +## Snyk + +Snyk is connected through its GitHub integration (a webhook on the repo) +to the `akankshatrehun` Snyk organisation, where `reframe-oss/reframe` has +two projects: **package.json** (open-source dependencies) and **Code +analysis** (Snyk Code). It adds two checks to pull requests from this +repository, `security/snyk` and `license/snyk`, and `snyk.yml` runs the CLI +(`snyk test`, high and critical only) on pushes to main, on same-repo pull +requests and weekly. Snyk can't read `bun.lock`, so that job tests a +`package-lock.json` generated from `package.json`; the bun.lock policy above +is the exact check. + +`.snyk` holds the policy: open-source ignores (none) and the paths Snyk Code +skips (test-only code). + +### Snyk Code triage, 2026-11-06 + +14 findings on `d1ff303`: + +| Finding | Where | Outcome | +|---|---|---| +| Path traversal (high) | `ffmpeg.worker.ts`: the engine's input file name took its extension from the user's file name | **Fixed**: every engine file name uses a letters-and-digits extension (`safeExtension`); a test feeds `../` names | +| ReDoS (high) | `sw.js`: `cache.match(request.url)` | **False positive**: Cache Storage matching compares URLs, it builds no regular expression. Ignored in Snyk with this reason | +| Hardcoded password (medium) | `check-deploy.mjs`: an object key named `pass` | **Fixed**: keys are the `RESULT` constants | +| postMessage origin (low) | `sw.js` message handler | **Fixed**: messages from other origins are ignored | +| postMessage origin (low) | `ffmpeg.worker.ts` message handler | **False positive**: a dedicated worker only receives messages from the page that created it. Ignored in Snyk | +| Path traversal ×6 (low) | `build-sw-manifest.mjs`, `strip-sourcemaps.mjs`: a directory from the command line | **Fixed**: both refuse a directory outside the repository (one deletes files) | +| Cleartext HTTP, no rate limit (medium ×2) | `scripts/serve-isolated.mjs`: the local test server on 127.0.0.1 | **Excluded** from Snyk Code (test-only) | +| Cleartext HTTP (low) | `deployCheck.test.ts`: a fixture server | **Excluded** (test-only) | + ## Secrets On 2026-11-06, gitleaks 8.21.2 (redacted) found no secrets in the full diff --git a/package.json b/package.json index 9ce09af9e..e6a91cbf1 100644 --- a/package.json +++ b/package.json @@ -25,8 +25,8 @@ "lottie-web": "^5.12.2", "lucide-react": "^0.577.0", "next": "16.4.1", - "react": "19.3.0", - "react-dom": "19.3.0", + "react": "19.3.1", + "react-dom": "19.3.1", "reframe": ".", "tailwind-merge": "^3.6.0", "wasm-feature-detect": "^1.8.0" @@ -42,8 +42,8 @@ "@testing-library/user-event": "^14.4.3", "@types/bun": "^1.3.14", "@types/node": "^25", - "@types/react": "19.2.18", - "@types/react-dom": "19.2.5", + "@types/react": "19.3.0", + "@types/react-dom": "19.3.0", "chromatic": "^18.5.0", "eslint": "9.39.5", "eslint-config-next": "16.4.1", @@ -57,8 +57,8 @@ "vitest": "^4.1.12" }, "overrides": { - "@types/react": "19.2.18", - "@types/react-dom": "19.2.5", + "@types/react": "19.3.0", + "@types/react-dom": "19.3.0", "undici": "^7.30.1", "vite": "^8.2.4", "zod": "^4.6.6", diff --git a/public/sw.js b/public/sw.js index 41e3127a3..d2e34fc06 100644 --- a/public/sw.js +++ b/public/sw.js @@ -94,8 +94,10 @@ self.addEventListener("activate", (event) => { ); }); -// The page asks for this when the user chooses to load an update. +// The page asks for this when the user chooses to load an update. Only this +// site's own pages can control the worker. self.addEventListener("message", (event) => { + if (event.origin !== self.location.origin) return; if (event.data && event.data.type === "skip-waiting") self.skipWaiting(); }); diff --git a/scripts/build-sw-manifest.mjs b/scripts/build-sw-manifest.mjs index 025de3cf9..e45c99210 100644 --- a/scripts/build-sw-manifest.mjs +++ b/scripts/build-sw-manifest.mjs @@ -11,7 +11,7 @@ // Source maps, the headers file and sw.js itself are left out. import { createHash } from "node:crypto"; import { readdir, readFile, writeFile } from "node:fs/promises"; -import { join, relative, sep } from "node:path"; +import { join, relative, resolve, sep } from "node:path"; import { fileURLToPath } from "node:url"; const SKIP = [/\.map$/, /^_headers$/, /^sw\.js$/, /(^|\/)\.DS_Store$/]; @@ -68,7 +68,12 @@ export function injectManifest(source, manifest) { } if (process.argv[1] && fileURLToPath(import.meta.url) === process.argv[1]) { - const root = process.argv[2] ?? "out"; + // It rewrites out/sw.js, so it only works inside the repository it runs in. + const root = resolve(process.argv[2] ?? "out"); + if (!root.startsWith(process.cwd() + sep)) { + console.error(`build-sw-manifest: ${root} is outside ${process.cwd()}; refusing.`); + process.exit(2); + } const manifest = await buildManifest(root); const swPath = join(root, "sw.js"); await writeFile(swPath, injectManifest(await readFile(swPath, "utf8"), manifest)); diff --git a/scripts/deploy/check-deploy.mjs b/scripts/deploy/check-deploy.mjs index b86d3cfe3..2a065bb1b 100644 --- a/scripts/deploy/check-deploy.mjs +++ b/scripts/deploy/check-deploy.mjs @@ -121,10 +121,10 @@ if (page.outcome.status && page.outcome.status >= 500) results.push(RESULT.UNAVA const overall = overallResult(results); const summary = { - pass: "All checks passed.", - fail: "Some checks failed.", - protected: "Not checked: the deployment requires authentication (set VERCEL_AUTOMATION_BYPASS_SECRET for Vercel previews).", - unavailable: "Not checked: the deployment could not be reached.", + [RESULT.PASS]: "All checks passed.", + [RESULT.FAIL]: "Some checks failed.", + [RESULT.PROTECTED]: "Not checked: the deployment requires authentication (set VERCEL_AUTOMATION_BYPASS_SECRET for Vercel previews).", + [RESULT.UNAVAILABLE]: "Not checked: the deployment could not be reached.", }[overall]; if (json) console.log(JSON.stringify({ url: base.href, result: overall, commit, checks }, null, 2)); diff --git a/scripts/strip-sourcemaps.mjs b/scripts/strip-sourcemaps.mjs index c785c54f8..e26c4f4df 100644 --- a/scripts/strip-sourcemaps.mjs +++ b/scripts/strip-sourcemaps.mjs @@ -8,9 +8,14 @@ // // node scripts/strip-sourcemaps.mjs [dir] (default: out) import { readdir, unlink } from "node:fs/promises"; -import { join } from "node:path"; +import { join, resolve, sep } from "node:path"; -const root = process.argv[2] ?? "out"; +// It deletes files, so it only works inside the repository it is run from. +const root = resolve(process.argv[2] ?? "out"); +if (!root.startsWith(process.cwd() + sep)) { + console.error(`strip-sourcemaps: ${root} is outside ${process.cwd()}; refusing.`); + process.exit(2); +} async function* walk(dir) { for (const entry of await readdir(dir, { withFileTypes: true })) { diff --git a/src/lib/ffmpeg.worker.ts b/src/lib/ffmpeg.worker.ts index 84e6bac1e..7cd83c44d 100644 --- a/src/lib/ffmpeg.worker.ts +++ b/src/lib/ffmpeg.worker.ts @@ -207,6 +207,15 @@ function loadTextFont(): Promise { return textFontBytes; } +/** + * The extension of a user's file name, reduced to letters and digits, for the + * neutral names files get in FFmpeg's filesystem: the user's file name + * (which could hold "/" or "..") is never used as a path. + */ +function safeExtension(name: string, fallback: string): string { + return (name.split(".").pop() ?? "").replace(/[^a-z0-9]/gi, "").slice(0, 8) || fallback; +} + async function readFileBytes(file: SerializedFile): Promise { if (file.data) return new Uint8Array(file.data); if (file.blob) return new Uint8Array(await file.blob.arrayBuffer()); @@ -249,7 +258,7 @@ async function runExport(request: ExportRequest): Promise { const recipe = request.recipe; const { width: targetW, height: targetH } = resolveOutputSize(recipe); - const ext = request.file.name.split(".").pop() ?? "mp4"; + const ext = safeExtension(request.file.name, "mp4"); const inputName = `input_${sessionId}.${ext}`; const { filename: outputName, mimeType } = getOutputConfig(recipe.format, sessionId); @@ -276,7 +285,7 @@ async function runExport(request: ExportRequest): Promise { } const hasOverlay = !!request.overlayFile; - const overlayExt = request.overlayFile?.name.split(".").pop() ?? "png"; + const overlayExt = safeExtension(request.overlayFile?.name ?? "", "png"); const overlayInputName = `overlay_${sessionId}.${overlayExt}`; if (hasOverlay) { cleanupFiles.add(overlayInputName); @@ -478,7 +487,7 @@ async function runJoin(request: JoinRequest): Promise { // New Files over the same bytes (no copy) with neutral names: the user's // file names never enter FFmpeg's filesystem. const sources = request.clips.map(({ file }, i) => { - const ext = (file.name.split(".").pop() ?? "mp4").replace(/[^a-z0-9]/gi, "").slice(0, 8) || "mp4"; + const ext = safeExtension(file.name, "mp4"); return new File([file], `clip${i}.${ext}`, { type: file.type }); }); const outputName = `joined_${sessionId}.mp4`; @@ -590,7 +599,7 @@ async function runFrames(request: FramesRequest, signal: AbortSignal): Promise { ); expect(ffmpeg().execCalls).toHaveLength(1); }); + + it("never uses the user's file name as a path in FFmpeg's filesystem", async () => { + await startWorker({ isolated: false }); + send({ type: "load" }); + await waitFor("ready"); + send({ + ...exportRequest(), + file: { name: "../../etc/holiday.mp4/..", type: "video/mp4", data: new ArrayBuffer(16) }, + overlayFile: { name: "logo.p/ng", type: "image/png", data: new ArrayBuffer(4) }, + overlayOptions: { file: null, position: "bottom-right", size: 100, opacity: 100 }, + }); + await waitFor("result"); + const written = [...ffmpeg().writes.keys()]; + expect(written).toContain("input_job1.mp4"); + expect(written).toContain("overlay_job1.png"); + expect(written.some((name) => name.includes("/") || name.includes(".."))).toBe(false); + }); }); diff --git a/src/lib/tests/serviceWorker.test.ts b/src/lib/tests/serviceWorker.test.ts index 11ecf2d31..9e39a1447 100644 --- a/src/lib/tests/serviceWorker.test.ts +++ b/src/lib/tests/serviceWorker.test.ts @@ -11,13 +11,26 @@ type Route = (url: URL, request: { method: string; mode: string }) => string; /** Runs sw.js against a stand-in `self`, as the browser would, and returns its test hooks. */ function loadWorker(precache: unknown) { const listeners: string[] = []; + const handlers: Record void> = {}; + let skipped = 0; const self = { location: { origin: "https://reframe.test" }, - addEventListener: (type: string) => listeners.push(type), + addEventListener: (type: string, handler: (event: unknown) => void) => { + listeners.push(type); + handlers[type] = handler; + }, + skipWaiting: () => { + skipped++; + }, } as Record; const source = precache ? injectManifest(SOURCE, precache) : SOURCE; new Function("self", "caches", source)(self, {}); - return { ...(self.__reframeSw as { route: Route; pageKey: (path: string) => string }), listeners }; + return { + ...(self.__reframeSw as { route: Route; pageKey: (path: string) => string }), + listeners, + handlers, + skipped: () => skipped, + }; } const PRECACHE = { version: "abc", pages: ["/", "/contact", "/privacy"], assets: ["/_next/static/chunks/app.js"] }; @@ -58,6 +71,14 @@ describe("service worker routing (public/sw.js)", () => { expect(sw.listeners).toEqual(expect.arrayContaining(["install", "activate", "fetch", "message"])); }); + it("loads an update only when this site's own page asks", () => { + const worker = loadWorker(PRECACHE); + worker.handlers.message!({ origin: "https://evil.example", data: { type: "skip-waiting" } }); + expect(worker.skipped()).toBe(0); + worker.handlers.message!({ origin: "https://reframe.test", data: { type: "skip-waiting" } }); + expect(worker.skipped()).toBe(1); + }); + it("keeps every engine file the page loads, in either mode", () => { for (const url of [...engineFileUrls(true), ...engineFileUrls(false)]) { expect(sw.route(new URL(url), get())).toBe("engine");