From f7c6ce7a1f76a246bb2aa0bd13e2689dbea53940 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sat, 20 Jun 2026 02:09:16 -0400 Subject: [PATCH] fix(#1461): loader never crashes on a malformed overlay (skip-with-warning); bound the source fetch Co-Authored-By: Claude Opus 4.8 --- .changeset/fix-1461-overlay-crash-dos.md | 7 + src/capability-loader.cts | 142 +++++- src/capability-source.cts | 427 ++++++++++++++--- tests/capability-loader.test.cjs | 285 ++++++++++- tests/capability-source.test.cjs | 586 +++++++++++++++++++++++ 5 files changed, 1370 insertions(+), 77 deletions(-) create mode 100644 .changeset/fix-1461-overlay-crash-dos.md diff --git a/.changeset/fix-1461-overlay-crash-dos.md b/.changeset/fix-1461-overlay-crash-dos.md new file mode 100644 index 000000000..422ac4b15 --- /dev/null +++ b/.changeset/fix-1461-overlay-crash-dos.md @@ -0,0 +1,7 @@ +--- +type: Fixed +pr: 1475 +--- +**The capability loader never crashes on a single malformed overlay, and untrusted manifest/tar reads are size-bounded (ADR-1244 D2 invariant)** — `loadRegistry` now makes the WHOLE per-candidate overlay-processing body total: ANY throw from ANY validator or step (including the committed `validateCapability`, which dereferences a malformed array entry such as `gates: [null]` / `steps: [null]` / `contributions: [null]` before its shape check) drops just that one overlay with a skip-warning instead of escaping the loader, while `gatePointsOf` is hardened to be total over null/non-array/malformed gates. The final canonical `buildRegistry` compose stays guarded: a throw on the merged set falls back to the frozen first-party registry plus a warning, records each dropped gate-declaring overlay's declared gate as blocked (`incompatibleGateCapIds` / `blockedGates`) so a dropped blocking gate FAILS CLOSED, AND now clears `_overlay.commandRoots` in the fallback so no dropped overlay retains a stale command root. Previously a malformed-array throw or a compose throw escaped the loader and crashed every consumer (loop-resolver, config-loader, surface, capability-state, gsd-tools). On the source side, the capability resolver/staging now reads every untrusted `capability.json` (tarball / npm / git / local) via the shared bounded fd-reader (regular-file + 8 MiB cap) instead of a raw `fs.readFileSync`, so an oversized or FIFO/non-regular extracted-or-local manifest can no longer OOM or hang the resolver; the fetch (`realHttpsGet`) bounds the downloaded response to 64 MiB; and `stageValidated` now enforces ONE uniform aggregate byte-budget (`MAX_STAGED_BUNDLE_BYTES`, 128 MiB) over the STAGED bundle directory via a bounded streaming walk (cumulative byte + entry counters; symlink / non-regular entries rejected) at the common staging chokepoint — AFTER staging and BEFORE validation/promotion — so a huge source tree, git repo, npm package, or gzip/tar bomb is rejected (and its staging dir cleaned up) before promotion, uniformly bounding the RESULT of `copyDirRecursive` / `git clone` / `npm pack` / `tar -x` that were previously only timeout-bounded. `copyDirRecursive` itself is now STREAMING and BUDGETED: it enumerates each directory via `fs.opendirSync` + `dir.readSync()` (one entry at a time) and threads CUMULATIVE entry (`MAX_STAGED_BUNDLE_ENTRIES`, 100k) and byte (`MAX_STAGED_BUNDLE_BYTES`) counters through the recursion, failing closed the MOMENT either cap is exceeded DURING the copy — closing a residual where the former `fs.readdirSync(src, { withFileTypes: true })` materialized the ENTIRE directory-entry array into memory at staging time (BEFORE the post-copy budget walk), so a hostile source whose tree held a directory of millions of tiny files (fetch < 64 MiB, but a colossal dirent array) could OOM the process during the copy before the budget could fail closed; the post-copy walk is retained as a cheap belt-and-suspenders re-verification on what actually landed in staging. The spoofable per-member `tar`-header size parse (`parseTarMemberSize`, which mis-anchored on BSD `tar -tv` owner/group columns such as a `Jan` group → fail-open) was REMOVED in favor of that non-spoofable staged-dir budget; `assertSafeTarMembers` keeps its unambiguous traversal / symlink / hardlink NAME and TYPE guards. (#1461) + + diff --git a/src/capability-loader.cts b/src/capability-loader.cts index b23514747..1a1049dc3 100644 --- a/src/capability-loader.cts +++ b/src/capability-loader.cts @@ -170,6 +170,26 @@ function errMessage(e: unknown): string { return e instanceof Error ? e.message : String(e); } +// --------------------------------------------------------------------------- +// Test seams (#1461). The validator and generator are normally `require()`d +// fresh inside loadRegistry. These optional overrides let a test inject a +// validator whose cross-capability check THROWS (OVL-1) or a generator whose +// buildRegistry THROWS (OVL-2), to prove the loader still NEVER crashes the +// loop — it skips the offending overlay with a warning / falls back to the +// frozen first-party registry. Pass null to restore the real module. +// --------------------------------------------------------------------------- +let _validatorOverride: ValidatorModule | null = null; +let _generatorOverride: GeneratorModule | null = null; + +/** Test seam: override the capability validator module. Pass null to restore. */ +function _setValidatorForTest(v: ValidatorModule | null): void { + _validatorOverride = v; +} +/** Test seam: override the registry generator module. Pass null to restore. */ +function _setGeneratorForTest(g: GeneratorModule | null): void { + _generatorOverride = g; +} + /** Resolve the running GSD version; fail-closed to '0.0.0' if it cannot be read. */ function readHostVersion(): string { try { @@ -406,6 +426,32 @@ function withOverlayMeta(reg: Registry, meta: OverlayMeta): Registry { return Object.assign({}, reg, { _overlay: meta }); } +/** + * Loop extension points a capability declares a gate at (the `point` strings off `cap.gates`). + * SINGLE source of truth shared by BOTH the per-candidate `skip()` closure AND the OVL-2 + * buildRegistry-failure fallback (#1461) so a dropped gate-declaring overlay fails CLOSED via the + * SAME extraction the per-candidate path uses — never one path blocking and the other failing open. + * + * #1461 finding 1 (HIGH): this MUST be TOTAL over an UNTRUSTED, possibly-malformed manifest — it + * runs on a candidate BEFORE per-candidate validation has confirmed the shape. A null `cap`, a + * non-object `cap`, a non-array `cap.gates` (e.g. `gates: {}` / `gates: null`), or a malformed gate + * ENTRY (`gates: [null]` / `gates: ["x"]` / a gate with a non-string `point`) must NEVER throw: it + * returns only the extractable `point` strings, filtering null/non-object/malformed entries. A + * `null` gate has no extractable point, so it contributes nothing (no spurious fail-closed block). + */ +function gatePointsOf(cap: unknown): string[] { + if (!cap || typeof cap !== 'object') return []; + const gates = (cap as { gates?: unknown }).gates; + if (!Array.isArray(gates)) return []; + return (gates as unknown[]) + .map((g) => + g && typeof g === 'object' && typeof (g as Record).point === 'string' + ? ((g as Record).point as string) + : null, + ) + .filter((p): p is string => typeof p === 'string'); +} + /** * Load the capability registry, optionally composing the installed overlay. * @@ -420,7 +466,7 @@ export function loadRegistry(options: LoadRegistryOptions = {}): Registry { if (!options.includeInstalled) return base; // eslint-disable-next-line @typescript-eslint/no-require-imports, @typescript-eslint/no-unsafe-assignment - const validator: ValidatorModule = require('./capability-validator.cjs'); + const validator: ValidatorModule = _validatorOverride ?? require('./capability-validator.cjs'); // eslint-disable-next-line @typescript-eslint/no-require-imports, @typescript-eslint/no-unsafe-assignment const semver: SemverModule = require('./semver-compare.cjs'); // eslint-disable-next-line @typescript-eslint/no-require-imports, @typescript-eslint/no-unsafe-assignment @@ -467,7 +513,7 @@ export function loadRegistry(options: LoadRegistryOptions = {}): Registry { const getGenerator = (): GeneratorModule => { if (generatorMod) return generatorMod; // eslint-disable-next-line @typescript-eslint/no-require-imports, @typescript-eslint/no-unsafe-assignment - const mod: GeneratorModule = require('../../../scripts/gen-capability-registry.cjs'); + const mod: GeneratorModule = _generatorOverride ?? require('../../../scripts/gen-capability-registry.cjs'); generatorMod = mod; return mod; }; @@ -526,11 +572,7 @@ export function loadRegistry(options: LoadRegistryOptions = {}): Registry { // Points at which this capability declares a gate — used to fail CLOSED if // the capability is skipped (a skipped deploy gate must block, not pass). - const gatePoints: string[] = Array.isArray(cap.gates) - ? (cap.gates as Array>) - .map((g) => (g && typeof g === 'object' && typeof g.point === 'string' ? g.point : null)) - .filter((p): p is string => typeof p === 'string') - : []; + const gatePoints: string[] = gatePointsOf(cap); const declaresGate = gatePoints.length > 0; const skip = (reason: string): void => { warnings.push({ id, scope: root.scope, reason }); @@ -540,6 +582,21 @@ export function loadRegistry(options: LoadRegistryOptions = {}): Registry { } }; + // #1461 finding 1 (HIGH): make the ENTIRE per-candidate processing body TOTAL. The committed + // validator is NOT total for malformed ARRAY entries — validateGate/validateStep/ + // validateContribution dereference an entry (`.point`, `.into`, …) BEFORE any shape check, so a + // manifest with `gates: [null]` (or `steps: [null]` / `contributions: [null]`) makes + // validateCapability THROW `Cannot read properties of null (reading 'point')`. That throw was + // OUTSIDE any per-candidate guard → it escaped loadRegistry and crashed EVERY consumer + // (loop-resolver, config-loader, surface, capability-state, gsd-tools). ADR-1244 D2 mandates a + // malformed overlay is SKIPPED with a warning, never crashes the loop. Wrapping the whole body + // (manifest already parsed above) means ANY throw from ANY validator/step becomes a structured + // `skip()` + continue to the next candidate — which ALSO fail-closes a declared gate (the `skip` + // closure records incompatibleGateCapIds/blockedGates for the extractable gate points). The + // existing structured skip/continue paths inside are unchanged; this is a fail-safe BACKSTOP for + // a validator/step that THROWS rather than returning errors. `continue` inside this try simply + // advances the `for` loop (there is no finally to interfere). + try { // 1. Reserved namespace — third-party may not impersonate first-party. if (RESERVED_ID_PREFIX.test(id)) { skip('id uses a reserved first-party prefix (gsd-/gsd-core-/anthropic-)'); @@ -666,11 +723,25 @@ export function loadRegistry(options: LoadRegistryOptions = {}): Registry { // owner-uniqueness, config-key exclusivity vs central schema, requires acyclicity // + tier-monotone. Incremental: add the candidate, validate, drop on any error. acceptedMap.set(id, cap); - const crossErrs = [ - ...validator.validateAgainstContract(cap, id), - ...validator.validateConsumesGlobal(acceptedMap), - ...validator.validateCrossCapability(acceptedMap, getCentralKeys()), - ]; + // #1461 OVL-1 (HIGH): these validators are CONTRACTED to RETURN error arrays, but one can THROW + // (e.g. validateConsumesGlobal asserting on a duplicate producer). An unguarded throw here + // escapes loadRegistry and crashes EVERY consumer (loop-resolver, config-loader, surface, + // capability-state, gsd-tools). ADR-1244 D2: a malformed overlay is SKIPPED with a warning, + // never crashes the loop. So a throwing validator is treated EXACTLY like a validation failure: + // drop this one candidate (with a warning) and continue — the rest of the overlay set is + // unaffected. (The returns-errors path below is unchanged.) + let crossErrs: string[]; + try { + crossErrs = [ + ...validator.validateAgainstContract(cap, id), + ...validator.validateConsumesGlobal(acceptedMap), + ...validator.validateCrossCapability(acceptedMap, getCentralKeys()), + ]; + } catch (e) { + acceptedMap.delete(id); + skip('cross-capability validation error: ' + errMessage(e)); + continue; + } if (crossErrs.length) { acceptedMap.delete(id); skip('cross-capability validation failed: ' + crossErrs.slice(0, 3).join('; ')); @@ -692,6 +763,17 @@ export function loadRegistry(options: LoadRegistryOptions = {}): Registry { // dispatchable. (Project-scope ledgers live in the repo tree and are thus only as trustworthy // as the repo — see docs/explanation/capability-trust-model.md.) if (families.length > 0 && committedIds.has(id)) commandRoots[id] = capDir; + } catch (e) { + // #1461 finding 1 (HIGH): ANY throw from ANY validator/step in the per-candidate body lands + // here — drop just THIS candidate with a structured skip-warning and continue with the rest of + // the overlay set (the loop is never crashed). `skip()` ALSO fail-closes the candidate's + // declared gates (incompatibleGateCapIds/blockedGates) so a malformed gate-declaring overlay + // blocks rather than silently passing. Remove any half-committed acceptedMap entry so the + // partially-processed candidate cannot leak into the final buildRegistry compose. + acceptedMap.delete(id); + skip('overlay processing error: ' + errMessage(e)); + continue; + } } } @@ -706,8 +788,38 @@ export function loadRegistry(options: LoadRegistryOptions = {}): Registry { // Compose via the canonical builder so every derived view matches first-party. // acceptedMap already holds first-party ∪ accepted overlays (validated above). - const merged = getGenerator().buildRegistry(acceptedMap); - return withOverlayMeta(merged, meta); + // + // #1461 OVL-2 (HIGH): an overlay can pass every per-candidate step yet trip a STRICTER whole-build + // check inside buildRegistry (config-slice shape, topo cycle across the merged set, configFormat + // parity). An unguarded buildRegistry throw escapes loadRegistry and crashes the loop. ADR-1244 D2 + // mandates NEVER-CRASH: on a compose failure, fall back to the frozen FIRST-PARTY registry plus a + // warning recording why — the loop still gets a usable registry, just without the overlay surfaces. + try { + const merged = getGenerator().buildRegistry(acceptedMap); + return withOverlayMeta(merged, meta); + } catch (e) { + const reason = 'buildRegistry failed composing overlays: ' + errMessage(e) + '; falling back to first-party'; + meta.warnings.push({ id: '*', scope: 'global', reason }); + // #1461 finding 3 (LOW): the fallback DROPS every accepted overlay, so NO dropped overlay may + // retain a command root. A stale `commandRoots[capId]` would let a runtime dispatcher require()/ + // run a third-party command family FROM the install root of a capability the fallback decided NOT + // to load. Clear the map (the first-party base never lists overlay commandRoots — first-party + // command modules ship in bin/lib/, not via _overlay.commandRoots). + meta.commandRoots = {}; + // #1461 OVL-2 fail-CLOSED on compose failure (HIGH): the fallback DROPS every accepted overlay, + // so any accepted overlay that DECLARED a gate would have its gate silently vanish → a blocking + // gate FAILS OPEN, violating ADR-1244 (a skipped capability declaring a gate must FAIL CLOSED). + // Record each dropped gate-declaring overlay's gate as blocked using the SAME extraction the + // per-candidate `skip()` closure uses (gatePointsOf), so loop-resolver injects the synthetic + // blocking gate at each declared point exactly as it would for a per-candidate skip. + for (const cap of overlayCaps) { + const gatePoints = gatePointsOf(cap); + if (gatePoints.length === 0) continue; + meta.incompatibleGateCapIds.push(cap.id); + for (const point of gatePoints) meta.blockedGates.push({ point, capId: cap.id, reason }); + } + return withOverlayMeta(base, meta); + } } -module.exports = { loadRegistry }; +module.exports = { loadRegistry, _setValidatorForTest, _setGeneratorForTest }; diff --git a/src/capability-source.cts b/src/capability-source.cts index 9cfe0a2b4..b385d87e5 100644 --- a/src/capability-source.cts +++ b/src/capability-source.cts @@ -18,7 +18,9 @@ * * ADR-457 build-at-publish: authored as TypeScript .cts → emits .cjs via tsc. * - * Exports: resolveCapabilitySource, parseSpec, _setCapabilitySourceHttpGet + * Exports: resolveCapabilitySource, parseSpec, _setCapabilitySourceHttpGet, + * _setHttpsGetImpl, _readManifestBounded, MAX_RESPONSE_BYTES, + * MANIFEST_MAX_BYTES, MAX_STAGED_BUNDLE_BYTES, MAX_STAGED_BUNDLE_ENTRIES */ import fs from 'node:fs'; @@ -42,6 +44,12 @@ const semverMod = require('./semver-compare.cjs') as { semverSatisfies: (version: unknown, range: unknown) => boolean; }; +// eslint-disable-next-line @typescript-eslint/no-require-imports +const ledgerMod = require('./capability-ledger.cjs') as { + /** Shared fd-based bounded reader: content, null for ENOENT, or THROWS (non-regular/oversized/IO). */ + readSmallRegularFile: (filePath: string, maxBytes: number) => string | null; +}; + // --------------------------------------------------------------------------- // Types // --------------------------------------------------------------------------- @@ -129,19 +137,115 @@ interface HttpResponse { type HttpGetFn = (url: string) => Promise; +/** + * DOS-1 (#1461): GENEROUS but BOUNDED cap on a fetched capability source response. `realHttpsGet` + * previously accumulated `res.on('data')` chunks with NO ceiling, so a hostile or accidental + * oversized tarball (e.g. an HTTP endpoint streaming gigabytes) would buffer unbounded into memory + * and OOM the process. A real capability bundle is a few hundred KiB of declarative JSON + small + * artifacts; 64 MiB is far more than any legitimate bundle yet still a hard ceiling. Enforced two + * ways: (1) a `content-length` header over the cap is rejected BEFORE buffering any body; (2) the + * cumulative streamed byte count is tracked across `data` events and the request is destroyed + + * rejected the instant it exceeds the cap (covers chunked / missing-content-length responses). + */ +const MAX_RESPONSE_BYTES = 64 * 1024 * 1024; + +/** + * #1461 finding 2 (HIGH): GENEROUS but BOUNDED cap on an UNTRUSTED `capability.json` read during + * resolve/staging. Every untrusted manifest (tarball / npm / git / local staging) MUST be read via + * the SHARED bounded reader (`readSmallRegularFile`: open → fstat → require-regular-file → size-cap → + * read-exactly-size), NOT a raw `fs.readFileSync`. A raw read of an oversized extracted-or-local + * `capability.json` reads unbounded into memory (OOM), and a FIFO/device/non-regular manifest BLOCKS + * the resolver forever. A legitimate manifest is a few KiB of declarative JSON; 8 MiB is far more than + * any real capability.json yet a hard ceiling. The reader returns null for a genuinely-missing file + * (ENOENT) and THROWS for non-regular/oversized/IO — both are mapped to a clear "manifest not + * found / refused" rejection (fail-closed: the source never resolves). + */ +const MANIFEST_MAX_BYTES = 8 * 1024 * 1024; + +/** + * #1461 finding 1 (HIGH): ONE uniform aggregate byte-budget over the STAGED bundle directory. The HTTP + * fetch is capped (MAX_RESPONSE_BYTES), but `copyDirRecursive` / `fs.copyFileSync`, `git clone`, + * `npm pack`, and `tar -x` were only TIMEOUT-bounded — so a huge local source tree, a giant git repo, a + * large npm package, or a gzip/tar bomb that expands far beyond the compressed download cap could fill + * disk during staging. This single budget, enforced at the common staging chokepoint (stageValidated, + * AFTER the source is copied into staging and BEFORE validation/promotion), uniformly bounds the RESULT + * of every adapter: it sums the regular-file bytes of the staged dir via a BOUNDED streaming walk and + * fails closed if the total exceeds the cap. 128 MiB is generous for a real capability bundle (a few + * hundred KiB of declarative JSON + small artifacts) yet hard-bounds a bomb. + * + * RESIDUAL (#1461 finding 4): this bounds the staged RESULT — it rejects an oversized install BEFORE + * promotion, but a transient disk-fill DURING extraction/clone (before the post-staging walk runs) is a + * residual a fully-airtight bound would need a streaming byte-quota DURING extraction/clone (e.g. a + * cgroup/disk-quota or a custom streaming extractor) to close. This is a stated, proportionate limit: + * this resolver path is USER-INITIATED `install` only (the cloned-repo / loader overlay path does NOT + * invoke the resolver and is bounded separately by capability-consent's bundleContentHash caps), and + * staging happens under a temp/.staging dir that is rmSync'd on any failure. + */ +const MAX_STAGED_BUNDLE_BYTES = 128 * 1024 * 1024; + +/** + * #1461 finding 1: a cumulative ENTRY-count ceiling for the staged-dir budget walk so the enumeration + * ITSELF is bounded (a hostile bundle with millions of tiny files / a very deep tree cannot force + * unbounded readdir work before the byte cap trips). 100k entries is far more than any real bundle. + */ +const MAX_STAGED_BUNDLE_ENTRIES = 100_000; + +/** + * The low-level `https.get`-shaped transport. Extracted as an overridable module-level reference so + * a test can inject a fake response stream (chunked / oversized / content-length-tagged) to exercise + * the MAX_RESPONSE_BYTES enforcement in realHttpsGet WITHOUT real network I/O. Defaults to the real + * node:https get. (The higher-level `_httpGet` seam below short-circuits realHttpsGet entirely and is + * used by the integrity tests; this seam is specifically for the streaming/size-cap path.) + */ +type HttpsGetImpl = typeof https.get; +let _httpsGetImpl: HttpsGetImpl = https.get; + +/** Test seam: override the low-level https.get transport used by realHttpsGet. Pass null to restore. */ +function _setHttpsGetImpl(fn: HttpsGetImpl | null): void { + _httpsGetImpl = fn ?? https.get; +} + // --------------------------------------------------------------------------- // Injectable HTTP transport (test seam) // --------------------------------------------------------------------------- function realHttpsGet(url: string): Promise { return new Promise((resolve, reject) => { - const req = https.get( + const req = _httpsGetImpl( url, { headers: { 'User-Agent': 'gsd-core-capability-source/1.0' } }, (res) => { + // DOS-1: reject early if the server ADVERTISES a body over the cap — no bytes buffered. + const contentLength = Number(res.headers?.['content-length']); + if (Number.isFinite(contentLength) && contentLength > MAX_RESPONSE_BYTES) { + req.destroy(); + res.destroy?.(); + reject( + new Error( + `response exceeds ${MAX_RESPONSE_BYTES} bytes (content-length ${contentLength}) fetching ${url}` + ) + ); + return; + } const chunks: Buffer[] = []; - res.on('data', (c: Buffer) => chunks.push(c)); + let received = 0; + let aborted = false; + res.on('data', (c: Buffer) => { + if (aborted) return; + received += c.length; + // DOS-1: enforce the ceiling on the ACTUAL streamed bytes (covers chunked / lying or + // absent content-length). Destroy the request/response and reject — never keep buffering. + if (received > MAX_RESPONSE_BYTES) { + aborted = true; + req.destroy(); + res.destroy?.(); + reject(new Error(`response exceeds ${MAX_RESPONSE_BYTES} bytes fetching ${url}`)); + return; + } + chunks.push(c); + }); res.on('end', () => { + if (aborted) return; const body = Buffer.concat(chunks); if (res.statusCode !== 200) { reject(new Error(`HTTP ${res.statusCode ?? 0} fetching ${url}`)); @@ -204,6 +308,42 @@ function verifyIntegrity(buf: Buffer, expected: string): void { } } +/** + * #1461 finding 2 (HIGH): read an UNTRUSTED `capability.json` (extracted or local) via the SHARED + * bounded reader and parse it as a JSON object, failing CLOSED on every untrusted-input condition. + * Replaces the raw `fs.readFileSync(manifestPath,'utf8')` at each resolve/staging site so an oversized + * manifest cannot read unbounded (OOM) and a FIFO/device/non-regular manifest cannot BLOCK forever. + * - ENOENT (reader returns null) → throw `` (genuinely missing). + * - non-regular / oversized / IO (reader THROWS) → throw `: ` (refused). + * - not valid JSON → throw the caller's invalid-JSON message. + * - not a JSON object → throw the caller's not-an-object message. + */ +function readManifestBounded( + manifestPath: string, + notFoundMessage: string, +): Record { + let raw: string | null; + try { + raw = ledgerMod.readSmallRegularFile(manifestPath, MANIFEST_MAX_BYTES); + } catch (err) { + // Non-regular (FIFO/device/dir), oversized, or IO error — fail closed with a clear message. + throw new Error(`${notFoundMessage}: ${(err as Error).message}`); + } + if (raw === null) { + throw new Error(notFoundMessage); // genuinely missing (ENOENT). + } + let cap: unknown; + try { + cap = JSON.parse(raw); + } catch { + throw new Error('capability.json is not valid JSON'); + } + if (typeof cap !== 'object' || cap === null || Array.isArray(cap)) { + throw new Error('capability.json must be a JSON object'); + } + return cap as Record; +} + /** * Reject spec/id values containing path separators or `..`. * Throws if the id is unsafe. @@ -242,28 +382,170 @@ function assertSafeGitUrl(url: string): void { } /** - * Copy a directory tree recursively into destDir. + * Copy a directory tree recursively into destDir — STREAMING and BUDGETED. * * SECURITY: symlinks are REJECTED (fail closed). A fetched bundle could otherwise * smuggle a symlink (e.g. `id_rsa -> ~/.ssh/id_rsa`) that fs.copyFileSync would * FOLLOW, copying an arbitrary host file's bytes into the staged capability dir. * Dirent.isSymbolicLink() reflects the entry itself (lstat semantics), so this * catches both file and directory symlinks before any copy. + * + * #1461 finding 1 (HIGH, ROUND 2): the copy ITSELF is bounded. The former + * `fs.readdirSync(src, { withFileTypes: true })` materialized the ENTIRE directory-entry + * array into memory BEFORE any budget could run — and copyDirRecursive runs at staging time + * BEFORE the post-copy assertStagedBundleWithinBudget walk. So a hostile local/git/npm/tar + * source whose tree has a directory holding millions of tiny files (fetch < 64 MiB, but a + * colossal dirent array) OOMs the process during the COPY, before the post-copy budget can + * fail closed. We now STREAM each directory via fs.opendirSync + dir.readSync() (one entry at + * a time, never the whole array) and thread CUMULATIVE counters across the recursion — total + * entries (cap MAX_STAGED_BUNDLE_ENTRIES) and total regular-file bytes (cap + * MAX_STAGED_BUNDLE_BYTES) — throwing the MOMENT either is exceeded, DURING the copy, before + * reading/copying the rest. The shared mutable `budget` object mirrors bundleContentHash's + * cumulative walk in capability-consent. The throw propagates to stageValidated's catch, which + * rmSync's the staging dir (fail closed, no partial bundle promoted). */ -function copyDirRecursive(src: string, dest: string): void { +function copyDirRecursive( + src: string, + dest: string, + budget: { entries: number; bytes: number } = { entries: 0, bytes: 0 }, +): void { fs.mkdirSync(dest, { recursive: true }); - for (const entry of fs.readdirSync(src, { withFileTypes: true })) { - const srcPath = path.join(src, entry.name); - const destPath = path.join(dest, entry.name); - if (entry.isSymbolicLink()) { - throw new Error(`Refusing to stage symlink in capability bundle: ${entry.name}`); - } else if (entry.isDirectory()) { - copyDirRecursive(srcPath, destPath); - } else if (entry.isFile()) { - fs.copyFileSync(srcPath, destPath); - } - // Non-regular entries (sockets, fifos, devices) are silently skipped. + let dir: fs.Dir; + try { + dir = fs.opendirSync(src); + } catch (err) { + throw new Error(`Cannot read source directory "${src}": ${(err as Error).message}`); } + try { + for (;;) { + let entry: fs.Dirent | null; + try { + entry = dir.readSync(); + } catch (err) { + throw new Error(`Cannot read source directory "${src}": ${(err as Error).message}`); + } + if (entry === null) break; + + // BOUND THE ENUMERATION ITSELF: count this entry and fail closed BEFORE it is processed, + // so a huge directory (or deep tree) is never read in full into memory first. + budget.entries++; + if (budget.entries > MAX_STAGED_BUNDLE_ENTRIES) { + throw new Error( + `Refusing to stage bundle: entry count exceeds the maximum of ${MAX_STAGED_BUNDLE_ENTRIES}` + ); + } + + const srcPath = path.join(src, entry.name); + const destPath = path.join(dest, entry.name); + if (entry.isSymbolicLink()) { + throw new Error(`Refusing to stage symlink in capability bundle: ${entry.name}`); + } else if (entry.isDirectory()) { + copyDirRecursive(srcPath, destPath, budget); + } else if (entry.isFile()) { + // Cumulative byte budget: lstat the entry (NOT stat — a symlink is already rejected above, + // but lstat is the authoritative size of the regular file being copied) and fail closed the + // MOMENT the running total crosses the cap, BEFORE copying the oversized file's bytes. + let st: fs.Stats; + try { + st = fs.lstatSync(srcPath); + } catch (err) { + throw new Error(`Cannot lstat source entry "${srcPath}": ${(err as Error).message}`); + } + budget.bytes += st.size; + if (budget.bytes > MAX_STAGED_BUNDLE_BYTES) { + throw new Error( + `Refusing to stage bundle: total staged size exceeds the maximum of ` + + `${MAX_STAGED_BUNDLE_BYTES} bytes (possible oversized source tree, git repo, npm package, or tar bomb)` + ); + } + fs.copyFileSync(srcPath, destPath); + } + // Non-regular entries (sockets, fifos, devices) are silently skipped. + } + } finally { + try { dir.closeSync(); } catch { /* best-effort: no fd leak per opened Dir */ } + } +} + +/** + * #1461 finding 1 (HIGH): sum the total regular-file bytes under `stagedDir` via a BOUNDED streaming + * walk and fail closed if the total exceeds MAX_STAGED_BUNDLE_BYTES. This is the SINGLE uniform bound on + * the RESULT of staging for EVERY adapter (local copy / git clone / npm pack / tar extraction) — placed + * at the common chokepoint in stageValidated AFTER copyDirRecursive and BEFORE validation/promotion. + * + * Bounded like capability-consent.bundleContentHash's enumeration: each level is STREAMED via + * fs.opendirSync + dir.readSync() with a CUMULATIVE entry counter (`count.n`) that throws the moment it + * exceeds MAX_STAGED_BUNDLE_ENTRIES — BEFORE the rest of a huge/deep level is read — so a hostile bundle + * with millions of tiny files or a very deep tree cannot force unbounded readdir/memory work before the + * byte cap trips. Per-entry: lstat (NOT stat) so a symlink is detected as itself; symlinks and other + * non-regular entries are REJECTED (fail closed — copyDirRecursive already refuses symlinks at copy time, + * but a fresh lstat here is the authoritative check on what actually landed in staging). Regular-file + * st.size is accumulated and the walk throws the moment the running total crosses the cap. + */ +function assertStagedBundleWithinBudget(stagedDir: string): void { + const total = { bytes: 0 }; + const count = { n: 0 }; + const walk = (absDir: string): void => { + let dir: fs.Dir; + try { + dir = fs.opendirSync(absDir); + } catch (err) { + throw new Error(`Cannot read staged directory "${absDir}": ${(err as Error).message}`); + } + const levelEntries: fs.Dirent[] = []; + try { + for (;;) { + let ent: fs.Dirent | null; + try { + ent = dir.readSync(); + } catch (err) { + throw new Error(`Cannot read staged directory "${absDir}": ${(err as Error).message}`); + } + if (ent === null) break; + // BOUND THE ENUMERATION ITSELF: fail closed before this entry is retained, so a huge directory + // (or deep tree) cannot be loaded in full first. + count.n++; + if (count.n > MAX_STAGED_BUNDLE_ENTRIES) { + throw new Error( + `Refusing to stage bundle: entry count exceeds the maximum of ${MAX_STAGED_BUNDLE_ENTRIES}` + ); + } + levelEntries.push(ent); + } + } finally { + try { dir.closeSync(); } catch { /* best-effort */ } + } + for (const ent of levelEntries) { + const abs = path.join(absDir, ent.name); + let st: fs.Stats; + try { + st = fs.lstatSync(abs); + } catch (err) { + throw new Error(`Cannot lstat staged entry "${abs}": ${(err as Error).message}`); + } + if (st.isSymbolicLink()) { + // Defense in depth: copyDirRecursive already refuses symlinks, but the budget walk is the + // authoritative re-check on what actually landed in staging. + throw new Error(`Refusing to stage symlink in capability bundle: ${abs}`); + } + if (st.isDirectory()) { + walk(abs); + continue; + } + if (!st.isFile()) { + // Sockets / FIFOs / devices are not part of a real capability bundle. + throw new Error(`Refusing to stage non-regular file in capability bundle: ${abs}`); + } + total.bytes += st.size; + if (total.bytes > MAX_STAGED_BUNDLE_BYTES) { + throw new Error( + `Refusing to stage bundle: total staged size exceeds the maximum of ` + + `${MAX_STAGED_BUNDLE_BYTES} bytes (possible oversized source tree, git repo, npm package, or tar bomb)` + ); + } + } + }; + walk(stagedDir); } /** @@ -271,6 +553,14 @@ function copyDirRecursive(src: string, dest: string): void { * an absolute path or a `..` segment BEFORE extraction (system tar mostly guards * this, but the hard contract is "traversal rejected", so we verify explicitly). * Symlink members that survive extraction are caught later by copyDirRecursive. + * + * #1461 finding 2 (MED): the former per-member declared-size parse (parseTarMemberSize) was REMOVED. + * It scanned the verbose listing for a date-looking token and treated the previous token as the size, + * but on BSD `tar -tv` the owner/group columns PRECEDE the size, so a member owner/group like "Jan" + * mis-anchored the scan → fail-OPEN (a bomb's real size column skipped). The staged-dir aggregate + * budget (assertStagedBundleWithinBudget, #1461 finding 1) is now the real, non-spoofable bound on the + * extracted RESULT, so the fragile header parse is redundant. This function keeps only the NAME and + * TYPE guards (traversal / symlink / hardlink), which are unambiguous and not size-dependent. */ function assertSafeTarMembers(execTar: TarExecFn, tgzPath: string): void { // (1) Member NAMES — reject path traversal (absolute / ".."). @@ -340,26 +630,37 @@ function stageValidated(opts: { fs.mkdirSync(stagingDir, { recursive: true }); try { - // Copy source into staging. + // Copy source into staging — STREAMING + BUDGETED (#1461 finding 1, ROUND 2). copyDirRecursive now + // enforces BOTH the entry-count and aggregate-byte budget DURING the copy (per-entry, via opendirSync + // + readSync, never readdirSync of the whole array), so a hostile source with millions of tiny files + // or an oversized artifact fails closed IN-PROCESS before the whole directory is materialized — it can + // no longer OOM the process before a post-copy walk runs. The catch below rmSync's the staging dir on + // throw, so an over-budget bundle never lands at the final location. copyDirRecursive(sourceDir, stagingDir); - // Read and parse the capability manifest. + // #1461 finding 1 (HIGH): belt-and-suspenders aggregate byte-budget re-verification on what ACTUALLY + // landed in staging. copyDirRecursive (above) is now the PRIMARY in-process bound — it fails closed + // DURING the copy — so this post-copy walk is no longer the sole guard, but it is kept as a cheap + // authoritative re-lstat of the staged RESULT at the common chokepoint AFTER staging and BEFORE + // validation/promotion: it re-checks the entry/byte caps and re-rejects any symlink / non-regular + // entry on the real staged tree (every staging path here flows through copyDirRecursive — there is no + // in-place-dir staging path — so the copy already bounds it; this is defense in depth). + // + // RESIDUAL (#1461 finding 4): the copy and this walk bound the staged RESULT (rejects an oversized + // install before promotion); a transient disk-fill DURING extraction/clone (system tar/git/npm write + // to a temp dir BEFORE copyDirRecursive streams it into staging) is a residual a fully-airtight bound + // would need a streaming byte-quota DURING extraction/clone to close. Proportionate: this resolver + // path is USER-INITIATED `install` only (the cloned-repo / loader overlay path does NOT invoke the + // resolver and is bounded separately), and the temp/.staging dirs are removed on any failure. + assertStagedBundleWithinBudget(stagingDir); + + // Read and parse the capability manifest via the SHARED bounded reader (#1461 finding 2): an + // oversized/non-regular staged capability.json is refused (fail-closed) rather than read unbounded. const manifestPath = path.join(stagingDir, 'capability.json'); - let rawManifest: string; - try { - rawManifest = fs.readFileSync(manifestPath, 'utf8'); - } catch { - throw new Error(`capability.json not found in staged directory: ${stagingDir}`); - } - let cap: Record; - try { - cap = JSON.parse(rawManifest) as Record; - } catch { - throw new Error('capability.json is not valid JSON'); - } - if (typeof cap !== 'object' || cap === null || Array.isArray(cap)) { - throw new Error('capability.json must be a JSON object'); - } + const cap = readManifestBounded( + manifestPath, + `capability.json not found in staged directory: ${stagingDir}`, + ); // engines.gsd pre-check — reject before staging finalizes (unless the caller owns the gate). const engines = cap['engines']; @@ -516,14 +817,13 @@ function resolveLocal( if (!fs.existsSync(absPath)) { throw new Error(`Local capability path does not exist: ${absPath}`); } - // Read id from capability.json to know the staging dest. + // Read id from capability.json to know the staging dest — via the SHARED bounded reader (#1461 + // finding 2): an oversized/non-regular local capability.json is refused, never read unbounded. const manifestPath = path.join(absPath, 'capability.json'); - let cap: Record; - try { - cap = JSON.parse(fs.readFileSync(manifestPath, 'utf8')) as Record; - } catch { - throw new Error(`Cannot read capability.json from local path: ${manifestPath}`); - } + const cap = readManifestBounded( + manifestPath, + `Cannot read capability.json from local path: ${manifestPath}`, + ); const id = typeof cap['id'] === 'string' ? cap['id'] : ''; if (!id) throw new Error('capability.json missing "id" field'); @@ -558,14 +858,13 @@ function resolveGit( } } - // Read id from capability.json. + // Read id from capability.json via the SHARED bounded reader (#1461 finding 2): a cloned repo's + // oversized/non-regular capability.json is refused, never read unbounded. const manifestPath = path.join(cloneDir, 'capability.json'); - let cap: Record; - try { - cap = JSON.parse(fs.readFileSync(manifestPath, 'utf8')) as Record; - } catch { - throw new Error(`capability.json not found in cloned repo: ${parsed.target}`); - } + const cap = readManifestBounded( + manifestPath, + `capability.json not found in cloned repo: ${parsed.target}`, + ); const id = typeof cap['id'] === 'string' ? cap['id'] : ''; if (!id) throw new Error('capability.json missing "id" field'); @@ -620,14 +919,13 @@ function resolveNpm( const packageDir = path.join(extractDir, 'package'); const sourceDir = fs.existsSync(path.join(packageDir, 'capability.json')) ? packageDir : extractDir; - // Read id from capability.json. + // Read id from capability.json via the SHARED bounded reader (#1461 finding 2): an extracted + // oversized/non-regular capability.json is refused, never read unbounded. const manifestPath = path.join(sourceDir, 'capability.json'); - let cap: Record; - try { - cap = JSON.parse(fs.readFileSync(manifestPath, 'utf8')) as Record; - } catch { - throw new Error(`capability.json not found after npm pack extraction from: ${parsed.target}`); - } + const cap = readManifestBounded( + manifestPath, + `capability.json not found after npm pack extraction from: ${parsed.target}`, + ); const id = typeof cap['id'] === 'string' ? cap['id'] : ''; if (!id) throw new Error('capability.json missing "id" field'); @@ -674,14 +972,13 @@ async function resolveTarball( const packageDir = path.join(extractDir, 'package'); const sourceDir = fs.existsSync(path.join(packageDir, 'capability.json')) ? packageDir : extractDir; - // Read id from capability.json. + // Read id from capability.json via the SHARED bounded reader (#1461 finding 2): an extracted + // oversized/non-regular capability.json is refused, never read unbounded. const manifestPath = path.join(sourceDir, 'capability.json'); - let cap: Record; - try { - cap = JSON.parse(fs.readFileSync(manifestPath, 'utf8')) as Record; - } catch { - throw new Error(`capability.json not found in tarball from: ${parsed.target}`); - } + const cap = readManifestBounded( + manifestPath, + `capability.json not found in tarball from: ${parsed.target}`, + ); const id = typeof cap['id'] === 'string' ? cap['id'] : ''; if (!id) throw new Error('capability.json missing "id" field'); @@ -737,4 +1034,12 @@ export = { resolveCapabilitySource, parseSpec, _setCapabilitySourceHttpGet, + _setHttpsGetImpl, + // #1461 finding 3 test seam: the exact bounded reader stageValidated uses on the COPIED manifest, so + // a test can exercise the staged re-read directly (not just the local pre-read that shadows it). + _readManifestBounded: readManifestBounded, + MAX_RESPONSE_BYTES, + MANIFEST_MAX_BYTES, + MAX_STAGED_BUNDLE_BYTES, + MAX_STAGED_BUNDLE_ENTRIES, }; diff --git a/tests/capability-loader.test.cjs b/tests/capability-loader.test.cjs index a688ed17e..4da5075e1 100644 --- a/tests/capability-loader.test.cjs +++ b/tests/capability-loader.test.cjs @@ -16,7 +16,7 @@ const os = require('node:os'); const path = require('node:path'); const { cleanup } = require('./helpers.cjs'); -const { loadRegistry } = require('../gsd-core/bin/lib/capability-loader.cjs'); +const { loadRegistry, _setValidatorForTest, _setGeneratorForTest } = require('../gsd-core/bin/lib/capability-loader.cjs'); const baseRegistry = require('../gsd-core/bin/lib/capability-registry.cjs'); const { buildRegistry } = require('../scripts/gen-capability-registry.cjs'); @@ -1063,3 +1063,286 @@ describe('loadRegistry — convergence: gate-before-materialize + realpath fail- assert.ok(reg.capabilities['f1r6-ctl-global'], 'a distinct real global cap stays trusted-active (both realpaths OK and differ)'); }); }); + +// --------------------------------------------------------------------------- +// #1461 OVL-1 — a THROWING cross-capability validator drops ONE candidate +// (skip-with-warning), never crashes loadRegistry. ADR-1244 D2 invariant: +// "invalid/incompatible overlays are skipped with a warning at load, never +// crash the loop." The per-candidate cross-validation (validateAgainstContract +// / validateConsumesGlobal / validateCrossCapability) is assumed to RETURN +// error arrays, but a validator can THROW (e.g. a duplicate-producer assertion). +// An unguarded throw escapes loadRegistry and crashes EVERY consumer. +// --------------------------------------------------------------------------- +const realValidator = require('../gsd-core/bin/lib/capability-validator.cjs'); + +describe('loadRegistry — #1461 OVL-1: a throwing cross-capability validator skips one candidate, never crashes', () => { + test('validateConsumesGlobal THROWING for one overlay drops it (warning) and a second valid overlay still loads', (t) => { + // Two valid overlays on disk. A wrapper validator delegates everything to the real validator + // EXCEPT validateConsumesGlobal, which THROWS the moment the poison candidate is in the merged + // map (mimics a validator that asserts rather than returning an error array, e.g. on a + // duplicate-producer). The throw escapes the unguarded per-candidate cross-validation. + const home = makeOverlayHome([ + featureCap('ovl1-poison', { skills: ['ovl1-poison-skill'] }), + featureCap('ovl1-good', { skills: ['ovl1-good-skill'] }), + ]); + t.after(() => { _setValidatorForTest(null); cleanup(home); }); + + _setValidatorForTest({ + ...realValidator, + validateConsumesGlobal(capMap) { + if (capMap.has('ovl1-poison')) { + throw new Error('synthetic validator explosion on duplicate producer'); + } + return realValidator.validateConsumesGlobal(capMap); + }, + }); + + // REVERT-FAILS: with the per-candidate cross-validation UN-wrapped (no try/catch), this throw + // escapes loadRegistry → assert.doesNotThrow fails (loadRegistry throws and crashes the loop). + let reg; + assert.doesNotThrow(() => { + reg = loadRegistry({ includeInstalled: true, gsdHome: home, cwd: home, hostVersion: HOST }); + }, 'a throwing cross-capability validator must NOT crash loadRegistry'); + + // The throwing candidate is dropped with a warning; the second valid overlay still loads. + assert.ok(!reg.capabilities['ovl1-poison'], 'the candidate whose validator threw is skipped, not registered'); + assert.ok(reg._overlay.warnings.some((w) => w.id === 'ovl1-poison' && /cross-capability/i.test(w.reason)), + 'the dropped candidate carries a cross-capability skip warning'); + assert.ok(reg.capabilities['ovl1-good'], 'a SECOND valid overlay still loads after the throwing one is dropped'); + // First-party stays fully intact. + assert.ok(Object.keys(reg.capabilities).length >= Object.keys(baseRegistry.capabilities).length + 1, + 'first-party registry remains intact alongside the surviving overlay'); + }); +}); + +// --------------------------------------------------------------------------- +// #1461 OVL-2 — a THROWING buildRegistry (the final compose) must NOT crash +// loadRegistry. An overlay can pass every per-candidate step yet trip a +// stricter whole-build check inside buildRegistry (config-slice shape, topo +// cycle, configFormat parity). The unguarded final compose would crash the loop. +// Required guarantee: NEVER crash → fall back to the frozen first-party +// registry + a warning. +// --------------------------------------------------------------------------- +const realGenerator = require('../scripts/gen-capability-registry.cjs'); + +describe('loadRegistry — #1461 OVL-2: a throwing buildRegistry falls back to first-party, never crashes', () => { + test('buildRegistry THROWING returns the first-party base + a warning (loop consumers still get a usable registry)', (t) => { + // A single valid overlay reaches the final compose. The generator wrapper delegates + // loadCentralConfigKeys to the real generator but makes buildRegistry THROW — simulating an + // overlay that passes per-candidate validation but breaks the full canonical build. + const home = makeOverlayHome([ + featureCap('ovl2-cap', { skills: ['ovl2-skill'] }), + ]); + t.after(() => { _setGeneratorForTest(null); cleanup(home); }); + + _setGeneratorForTest({ + loadCentralConfigKeys: () => realGenerator.loadCentralConfigKeys(), + buildRegistry() { + throw new Error('synthetic buildRegistry explosion composing overlays'); + }, + }); + + // REVERT-FAILS: with the final `getGenerator().buildRegistry(acceptedMap)` UN-wrapped, this throw + // escapes loadRegistry → assert.doesNotThrow fails (loadRegistry throws and crashes the loop). + let reg; + assert.doesNotThrow(() => { + reg = loadRegistry({ includeInstalled: true, gsdHome: home, cwd: home, hostVersion: HOST }); + }, 'a throwing buildRegistry must NOT crash loadRegistry'); + + // Falls back to the frozen first-party base: every first-party capability is present and the + // overlay is absent (the build that would have added it threw). + assert.ok(!reg.capabilities['ovl2-cap'], 'the overlay is absent — the compose that would add it failed'); + for (const id of Object.keys(baseRegistry.capabilities)) { + assert.ok(reg.capabilities[id], `first-party capability "${id}" survives the fallback`); + } + // A warning records WHY the loop fell back. + assert.ok(reg._overlay.warnings.some((w) => /buildRegistry/i.test(w.reason)), + 'a warning records the buildRegistry failure + first-party fallback'); + }); + + test('a DROPPED gate-declaring overlay still BLOCKS its gate (fail-closed, not fail-open)', (t) => { + // An overlay that DECLARES a blocking gate is ACCEPTED per-candidate and reaches the final + // compose; buildRegistry then THROWS. Dropping the overlay must NOT silently drop its gate: a + // blocking gate that vanishes fails OPEN (ADR-1244: a skipped capability declaring a gate must + // FAIL CLOSED). So the fallback must record the dropped overlay's declared gate as blocked — + // exactly as the per-candidate `skip()` closure does for `declaresGate`. + const home = makeOverlayHome([ + featureCap('ovl2-gate-cap', { + skills: ['ovl2-gate-skill'], + config: { 'workflow.ovl2_gate': { type: 'boolean', default: true, description: 'Gate.' } }, + gates: [{ point: 'execute:wave:post', check: { query: 'x.ovl2_gate' }, blocking: true, onError: 'halt' }], + steps: [{ point: 'execute:wave:post', ref: { skill: 'ovl2-gate-skill' }, produces: ['G.md'], consumes: [], when: 'workflow.ovl2_gate', onError: 'skip' }], + }), + ]); + t.after(() => { _setGeneratorForTest(null); cleanup(home); }); + + _setGeneratorForTest({ + loadCentralConfigKeys: () => realGenerator.loadCentralConfigKeys(), + buildRegistry() { + throw new Error('synthetic buildRegistry explosion dropping a gate-declaring overlay'); + }, + }); + + let reg; + assert.doesNotThrow(() => { + reg = loadRegistry({ includeInstalled: true, gsdHome: home, cwd: home, hostVersion: HOST }); + }, 'a throwing buildRegistry must NOT crash loadRegistry'); + + // Fell back to first-party (the overlay surfaces are gone)... + assert.ok(!reg.capabilities['ovl2-gate-cap'], 'the gate-declaring overlay is absent after compose failure'); + // ...BUT its declared blocking gate is recorded as blocked (fail-closed). + // REVERT-FAILS: without the catch iterating overlayCaps to populate gates, both of these are + // empty (the gate silently fails OPEN) → these assertions fail. + assert.ok(reg._overlay.incompatibleGateCapIds.includes('ovl2-gate-cap'), + 'dropped gate-declaring overlay tracked as a fail-closed blocker'); + assert.ok( + reg._overlay.blockedGates.some((g) => g.point === 'execute:wave:post' && g.capId === 'ovl2-gate-cap'), + 'dropped overlay\'s blocking gate point recorded as blocked (loop injects the synthetic gate)'); + }); + + // #1461 OVL-2 finding 3 (LOW): on the first-party fallback, the returned meta's commandRoots must be + // CLEARED — no dropped overlay may retain a command root in the fallback (a dispatcher reading + // _overlay.commandRoots[capId] would otherwise require()/run a command family from a capability that + // the fallback decided NOT to load). The base first-party registry never lists overlay commandRoots, + // so the fallback meta.commandRoots must be {}. + test('the first-party fallback returns an EMPTY _overlay.commandRoots (no stale command root for a dropped overlay)', (t) => { + // A committed overlay that ships a command family — so commandRoots[id] is populated BEFORE the + // compose step. The committed ledger entry is required for the loader to record the command root. + const home = fs.mkdtempSync(path.join(os.tmpdir(), 'cap-ovl2-cmdroot-')); + t.after(() => { _setGeneratorForTest(null); cleanup(home); }); + const id = 'ovl2-cmd-cap'; + const dir = path.join(home, '.gsd', 'capabilities', id); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync( + path.join(dir, 'capability.json'), + JSON.stringify(featureCap(id, { + skills: ['ovl2-cmd-skill'], + commands: [{ family: 'ovl2-cmd-family', module: 'router.cjs', router: 'route' }], + })), + 'utf8', + ); + // A committed (non-_pending, structurally-valid per isValidLedgerEntry) ledger entry so the loader + // populates commandRoots[id] — requires id/version/source/integrity strings + files[] + sharedEdits[]. + fs.writeFileSync( + path.join(home, '.gsd-capabilities.json'), + JSON.stringify({ version: 1, updatedAt: '2026-01-01T00:00:00.000Z', entries: { [id]: { id, version: '1.0.0', source: 'local', integrity: '', files: [], sharedEdits: [] } } }), + 'utf8', + ); + + _setGeneratorForTest({ + loadCentralConfigKeys: () => realGenerator.loadCentralConfigKeys(), + buildRegistry() { + throw new Error('synthetic buildRegistry explosion forcing the first-party fallback'); + }, + }); + + let reg; + assert.doesNotThrow(() => { + reg = loadRegistry({ includeInstalled: true, gsdHome: home, cwd: home, hostVersion: HOST }); + }, 'a throwing buildRegistry must NOT crash loadRegistry'); + + // The overlay is dropped (compose threw) AND its command root is cleared in the fallback meta. + assert.ok(!reg.capabilities[id], 'the command-shipping overlay is absent after the compose failure'); + // REVERT-FAILS: without `meta.commandRoots = {}` in the OVL-2 catch, commandRoots[id] survives the + // fallback (the loader populated it before the throw) → this assertion fails. + assert.deepStrictEqual(reg._overlay.commandRoots, {}, 'the fallback meta carries NO stale command roots'); + }); +}); + +// --------------------------------------------------------------------------- +// #1461 finding 1 (HIGH) — a per-candidate validator that THROWS on a malformed +// ARRAY entry must NOT crash loadRegistry. The committed validateCapability is +// NOT total: validateGate/validateStep/validateContribution dereference the entry +// (`.point`, `.into`, …) BEFORE any shape check, so `gates: [null]` (or a +// malformed steps/contributions entry) throws `Cannot read properties of null` +// from INSIDE validateCapability — which runs OUTSIDE the per-candidate try/catch. +// ADR-1244 D2: a malformed overlay is SKIPPED with a warning, never crashes the +// loop. The WHOLE per-candidate body must be total. +// --------------------------------------------------------------------------- +describe('loadRegistry — #1461 finding 1: a throwing per-candidate validator skips one overlay, never crashes', () => { + test('an overlay with `gates: [null]` does NOT crash loadRegistry — it is skipped, other valid overlays still load', (t) => { + const home = makeOverlayHome([ + // gates: [null] passes Array.isArray(cap.gates) then validateGate(null) dereferences null.point. + featureCap('null-gate-cap', { skills: ['null-gate-skill'], gates: [null] }), + featureCap('good-after-null-gate', { skills: ['good-after-null-gate-skill'] }), + ]); + t.after(() => cleanup(home)); + + // REVERT-FAILS: with validateCapability OUTSIDE the per-candidate try/catch, validateGate(null) + // throws → loadRegistry throws → assert.doesNotThrow fails (the loop crashes). + let reg; + assert.doesNotThrow(() => { + reg = load(home); + }, 'an overlay with `gates: [null]` must NOT crash loadRegistry'); + + assert.ok(!reg.capabilities['null-gate-cap'], 'the overlay whose validator threw is skipped, not registered'); + assert.ok(reg._overlay.warnings.some((w) => w.id === 'null-gate-cap'), + 'the dropped overlay carries a skip warning'); + assert.ok(reg.capabilities['good-after-null-gate'], 'a SECOND valid overlay still loads after the throwing one is skipped'); + // First-party stays fully intact. + assert.ok(Object.keys(reg.capabilities).length >= Object.keys(baseRegistry.capabilities).length + 1, + 'first-party registry remains intact alongside the surviving overlay'); + }); + + test('an overlay with a malformed `steps`/`contributions` entry (null) does NOT crash loadRegistry — skipped', (t) => { + const home = makeOverlayHome([ + featureCap('null-step-cap', { skills: ['null-step-skill'], steps: [null] }), + featureCap('null-contrib-cap', { skills: ['null-contrib-skill'], contributions: [null] }), + featureCap('good-after-null-step', { skills: ['good-after-null-step-skill'] }), + ]); + t.after(() => cleanup(home)); + + // REVERT-FAILS: validateStep(null)/validateContribution(null) deref null.point → throw escapes the + // unguarded validateCapability → loadRegistry throws → assert.doesNotThrow fails. + let reg; + assert.doesNotThrow(() => { + reg = load(home); + }, 'an overlay with a null steps/contributions entry must NOT crash loadRegistry'); + + assert.ok(!reg.capabilities['null-step-cap'], 'the overlay with a null step is skipped'); + assert.ok(!reg.capabilities['null-contrib-cap'], 'the overlay with a null contribution is skipped'); + assert.ok(reg._overlay.warnings.some((w) => w.id === 'null-step-cap'), 'null-step overlay carries a warning'); + assert.ok(reg._overlay.warnings.some((w) => w.id === 'null-contrib-cap'), 'null-contrib overlay carries a warning'); + assert.ok(reg.capabilities['good-after-null-step'], 'a valid overlay still loads after the malformed ones are skipped'); + }); + + test('a gate entry with a VALID point but otherwise malformed still FAILS CLOSED (gatePointsOf is total)', (t) => { + // The gate object HAS a string `point` (so the point is extractable) but is otherwise malformed + // (no valid `check`, no `blocking`) → validateCapability returns errors (not a throw) → the cap is + // skipped via the structured `skip()` path. Because it declares a gate at an extractable point, that + // point must be recorded as fail-closed (incompatibleGateCapIds + blockedGates). + const home = makeOverlayHome([ + featureCap('valid-point-bad-gate', { + skills: ['valid-point-bad-gate-skill'], + gates: [{ point: 'execute:wave:post' }], // valid point, missing check/blocking → validation errors + }), + ]); + t.after(() => cleanup(home)); + + let reg; + assert.doesNotThrow(() => { reg = load(home); }); + assert.ok(!reg.capabilities['valid-point-bad-gate'], 'a malformed-but-point-bearing gate cap is skipped'); + // The extractable point fail-closes (a skipped gate-declaring cap must block, not pass). + assert.ok(reg._overlay.incompatibleGateCapIds.includes('valid-point-bad-gate'), + 'a skipped cap declaring a gate at an extractable point is tracked as a fail-closed blocker'); + assert.ok(reg._overlay.blockedGates.some((g) => g.point === 'execute:wave:post' && g.capId === 'valid-point-bad-gate'), + 'the extractable gate point is recorded as blocked'); + }); + + test('a `null` gate (no extractable point) is a no-crash SKIP with no spurious blocked gate', (t) => { + // gatePointsOf must be TOTAL over `gates: [null]`: a null entry has no extractable `point`, so it + // contributes NO blocked gate (declaresGate is false) — but it must not crash either. + const home = makeOverlayHome([ + featureCap('null-gate-no-block', { skills: ['null-gate-no-block-skill'], gates: [null] }), + ]); + t.after(() => cleanup(home)); + + let reg; + assert.doesNotThrow(() => { reg = load(home); }); + assert.ok(!reg.capabilities['null-gate-no-block'], 'the null-gate overlay is skipped'); + assert.ok(!reg._overlay.incompatibleGateCapIds.includes('null-gate-no-block'), + 'a null gate has no extractable point → no spurious fail-closed block'); + assert.ok(!reg._overlay.blockedGates.some((g) => g.capId === 'null-gate-no-block'), + 'a null gate records no blockedGates entry'); + }); +}); diff --git a/tests/capability-source.test.cjs b/tests/capability-source.test.cjs index 90723ebdb..9fc494078 100644 --- a/tests/capability-source.test.cjs +++ b/tests/capability-source.test.cjs @@ -29,7 +29,13 @@ const { resolveCapabilitySource, parseSpec, _setCapabilitySourceHttpGet, + _setHttpsGetImpl, + MAX_RESPONSE_BYTES, + MANIFEST_MAX_BYTES, + MAX_STAGED_BUNDLE_BYTES, + MAX_STAGED_BUNDLE_ENTRIES, } = capSource; +const { EventEmitter } = require('node:events'); // --------------------------------------------------------------------------- // Helpers @@ -73,6 +79,16 @@ function sha512b64(buf) { return 'sha512-' + crypto.createHash('sha512').update(buf).digest('base64'); } +/** A minimal fs.Dirent-shaped object for synthetic streaming-opendir tests. */ +function makeDirent(name, { file = false, dir = false, symlink = false } = {}) { + return { + name, + isSymbolicLink: () => symlink, + isDirectory: () => dir, + isFile: () => file, + }; +} + // --------------------------------------------------------------------------- // parseSpec — kind detection // --------------------------------------------------------------------------- @@ -688,3 +704,573 @@ function _fakeTarball(cap) { // We just need a buffer; the injected tar override does the actual "extraction". return Buffer.from(JSON.stringify({ _fakeTarball: true, id: cap.id }), 'utf8'); } + +// --------------------------------------------------------------------------- +// #1461 DOS-1 — realHttpsGet must BOUND the fetched response size. Without a cap, +// res.on('data') accumulates chunks and Buffer.concat'd into memory with no +// ceiling → a hostile/oversized tarball OOMs the process. Verified by injecting a +// fake low-level https.get (the _setHttpsGetImpl seam) that streams a response. +// --------------------------------------------------------------------------- + +/** + * Build a fake https.get implementation that drives realHttpsGet's streaming path. + * @param chunks array of Buffers to emit on the response 'data' events + * @param headers response headers (e.g. { 'content-length': '...' }) + * @param statusCode response status (default 200) + * Returns a function matching the https.get(url, opts, cb) shape. It captures whether + * req.destroy() / res.destroy() were called so a test can assert the cap aborts the stream. + */ +function makeFakeHttpsGet(chunks, headers = {}, statusCode = 200) { + const state = { reqDestroyed: false, resDestroyed: false, emittedBytes: 0, ended: false }; + const fn = (_url, _opts, cb) => { + const req = new EventEmitter(); + req.setTimeout = () => req; + req.destroy = () => { state.reqDestroyed = true; }; + const res = new EventEmitter(); + res.statusCode = statusCode; + res.headers = headers; + res.destroy = () => { state.resDestroyed = true; }; + // Drive the response on the next tick so realHttpsGet has attached its listeners. + setImmediate(() => { + cb(res); + for (const c of chunks) { + if (state.reqDestroyed || state.resDestroyed) break; + state.emittedBytes += c.length; + res.emit('data', c); + } + if (!state.reqDestroyed && !state.resDestroyed) { + state.ended = true; + res.emit('end'); + } + }); + return req; + }; + fn.state = state; + return fn; +} + +describe('#1461 DOS-1 — realHttpsGet bounds the response size (MAX_RESPONSE_BYTES)', () => { + let gsdHome = ''; + beforeEach(() => { gsdHome = createTempDir('gsd-dos-home-'); }); + afterEach(() => { + _setHttpsGetImpl(null); // restore the real https.get + _setCapabilitySourceHttpGet(null); + cleanup(gsdHome); + }); + + test('MAX_RESPONSE_BYTES is a sane bounded cap (64 MiB)', () => { + assert.strictEqual(MAX_RESPONSE_BYTES, 64 * 1024 * 1024, 'cap is generous but bounded'); + }); + + test('a streamed body exceeding MAX_RESPONSE_BYTES is rejected and not buffered unboundedly', async () => { + // Stream chunks whose cumulative length exceeds the cap. Each chunk is 1 MiB; we emit cap+2 + // chunks. With the cap in place the stream is destroyed after crossing MAX_RESPONSE_BYTES, so + // far fewer than all chunks are ever emitted. + const ONE_MIB = 1024 * 1024; + const chunkCount = MAX_RESPONSE_BYTES / ONE_MIB + 2; // 66 chunks of 1 MiB + const chunks = Array.from({ length: chunkCount }, () => Buffer.alloc(ONE_MIB, 0x61)); + const fake = makeFakeHttpsGet(chunks, {} /* no content-length → exercise the streaming guard */); + _setHttpsGetImpl(fake); + + // REVERT-FAILS: without the per-`data` size guard in realHttpsGet, all chunks are accumulated + // and the promise RESOLVES with an oversized body (then proceeds to integrity/extraction) — + // assert.rejects below fails because nothing rejected. + await assert.rejects( + () => resolveCapabilitySource('https://example.com/huge.tgz', { gsdHome, hostVersion: '1.5.0' }), + /exceeds .*bytes/i, + 'an oversized streamed body must reject with the size error' + ); + // The stream was aborted: the request/response were destroyed and NOT every chunk was emitted. + assert.ok(fake.state.reqDestroyed || fake.state.resDestroyed, 'the request/response is destroyed on overflow'); + assert.ok(fake.state.emittedBytes <= (MAX_RESPONSE_BYTES + ONE_MIB), + 'streaming stops shortly after crossing the cap (not all chunks buffered)'); + assert.ok(!fake.state.ended, 'the response never reaches end — it was cut off'); + }); + + test('a content-length header over the cap is rejected BEFORE buffering any body', async () => { + // Advertise an oversized content-length but emit NO data — the early header check must reject. + const fake = makeFakeHttpsGet([], { 'content-length': String(MAX_RESPONSE_BYTES + 1) }); + _setHttpsGetImpl(fake); + + // REVERT-FAILS: without the content-length pre-check in realHttpsGet, the (empty) stream simply + // ends and the promise RESOLVES — assert.rejects fails because nothing rejected. + await assert.rejects( + () => resolveCapabilitySource('https://example.com/lying.tgz', { gsdHome, hostVersion: '1.5.0' }), + /exceeds .*bytes|content-length/i, + 'an over-cap content-length must reject before buffering' + ); + assert.strictEqual(fake.state.emittedBytes, 0, 'no body bytes were buffered before rejection'); + assert.ok(fake.state.reqDestroyed || fake.state.resDestroyed, 'the request/response is destroyed on the header check'); + }); + + test('CONTROL: a normal small tarball still fetches + installs through realHttpsGet', async () => { + // A small valid body that passes the cap. The high-level _httpGet seam is NOT used here — we go + // through the real realHttpsGet via the injected low-level https.get so the size-cap code path is + // exercised on the happy path too. A tar override performs the "extraction". + const cap = featureCap('dos-control-cap'); + const tgzBuf = _fakeTarball(cap); + const fake = makeFakeHttpsGet([tgzBuf], { 'content-length': String(tgzBuf.length) }); + _setHttpsGetImpl(fake); + + const result = await resolveCapabilitySource('https://example.com/dos-control-cap.tgz', { + gsdHome, + hostVersion: '1.5.0', + execOverrides: { + tar: (_prog, args) => { + if (args[0] === '-tzf') { + return { exitCode: 0, stdout: 'capability.json\n', stderr: '', signal: null, error: null }; + } + if (args[0] === '-tvzf') { + return { exitCode: 0, stdout: '-rw-r--r-- 0 user group 10 Jan 1 2020 capability.json\n', stderr: '', signal: null, error: null }; + } + const extractDir = args[args.indexOf('-C') + 1]; + fs.writeFileSync(path.join(extractDir, 'capability.json'), JSON.stringify(cap), 'utf8'); + return { exitCode: 0, stdout: '', stderr: '', signal: null, error: null }; + }, + }, + }); + + assert.strictEqual(result.id, 'dos-control-cap', 'a normal small tarball resolves through the bounded fetch path'); + assert.ok(fs.existsSync(result.stagedDir), 'staged dir exists after a normal fetch+install'); + assert.ok(fake.state.ended, 'a within-cap body streams to completion'); + }); +}); + +// --------------------------------------------------------------------------- +// #1461 finding 2 (HIGH) — the resolver/staging must read every UNTRUSTED +// capability.json via the shared bounded reader (regular-file + size cap), NOT a +// raw fs.readFileSync. A repo-planted/extracted oversized (or FIFO/non-regular) +// manifest would otherwise read unbounded into memory (OOM) or BLOCK the resolver. +// A null/oversized/non-regular read → reject the source with a clear error. +// --------------------------------------------------------------------------- +describe('#1461 finding 2 — untrusted capability.json reads are size-bounded (no OOM/hang)', () => { + let gsdHome = ''; + beforeEach(() => { gsdHome = createTempDir('gsd-cap-manifest-bound-'); }); + afterEach(() => { cleanup(gsdHome); }); + + test('MANIFEST_MAX_BYTES is a sane bounded cap (8 MiB)', () => { + assert.strictEqual(MANIFEST_MAX_BYTES, 8 * 1024 * 1024, 'manifest cap is generous but bounded'); + }); + + test('local: an OVERSIZED capability.json fails closed with a clear error (no OOM)', async () => { + // Build a local bundle whose capability.json EXCEEDS the cap. The bounded reader's fstat-size + // check refuses it WITHOUT reading the whole file into memory. + const dir = createTempDir('gsd-cap-oversized-local-'); + try { + // One byte over the cap is enough for the size check to refuse. + const oversized = Buffer.alloc(MANIFEST_MAX_BYTES + 1, 0x20); // spaces (still "JSON-ish" length-wise) + fs.writeFileSync(path.join(dir, 'capability.json'), oversized); + + // REVERT-FAILS: with a raw fs.readFileSync of capability.json, the whole oversized file is read + // into memory and JSON.parse fails with a SYNTAX error (not the bounded-reader size error). The + // bounded reader rejects on the fstat size BEFORE reading — so the error message names the size. + await assert.rejects( + () => resolveCapabilitySource(dir, { gsdHome, hostVersion: '1.5.0' }), + /exceeds|size|maximum|not a regular file|cannot read/i, + 'an oversized local capability.json must be refused by the bounded reader', + ); + } finally { + cleanup(dir); + } + assert.ok(!fs.existsSync(path.join(gsdHome, '.gsd', 'capabilities')) || + fs.readdirSync(path.join(gsdHome, '.gsd', 'capabilities')).filter((e) => e !== '.staging').length === 0, + 'no capability is staged after an oversized manifest is refused'); + }); + + test('CONTROL: a small valid local capability.json still resolves through the bounded reader', async () => { + const dir = makeLocalCap(featureCap('bounded-control-cap')); + try { + const result = await resolveCapabilitySource(dir, { gsdHome, hostVersion: '1.5.0' }); + assert.strictEqual(result.id, 'bounded-control-cap', 'a small valid manifest still resolves'); + assert.ok(fs.existsSync(path.join(result.stagedDir, 'capability.json')), 'staged manifest present'); + } finally { + cleanup(dir); + } + }); + + test('local PRE-READ: an oversized capability.json is refused by the bounded reader (before any staging)', async () => { + // HONEST SCOPE (#1461 finding 3): for a LOCAL source the adapter's bounded pre-read of + // capability.json (to learn the id) runs FIRST and refuses an oversized manifest BEFORE staging — + // so this proves the LOCAL PRE-READ bound, NOT the stageValidated staged re-read (an earlier + // version of this test falsely claimed to exercise the staged re-read). The staged re-read is + // covered directly below via _readManifestBounded. + const dir = createTempDir('gsd-cap-oversized-stage-'); + try { + const cap = featureCap('oversized-stage-cap'); + // A valid manifest padded past the cap via a large filler field — still parseable JSON shape but + // over the byte cap, so the bounded reader refuses it on size. + const padded = JSON.stringify({ ...cap, _filler: 'x'.repeat(MANIFEST_MAX_BYTES) }); + fs.writeFileSync(path.join(dir, 'capability.json'), padded); + await assert.rejects( + () => resolveCapabilitySource(dir, { gsdHome, hostVersion: '1.5.0' }), + /exceeds|size|maximum|cannot read/i, + 'an oversized local capability.json must be refused by the bounded pre-read', + ); + } finally { + cleanup(dir); + } + }); + + test('staged re-read: the bounded manifest reader refuses an oversized staged capability.json directly', () => { + // #1461 finding 3: exercise stageValidated's bounded re-read WITHOUT the local pre-read shadowing + // it. _readManifestBounded is the exact reader stageValidated uses on the COPIED manifest; an + // oversized staged file is refused on size (fail-closed), not read unbounded. + // + // REVERT-FAILS: with a raw fs.readFileSync in the staged re-read, this would read the whole + // oversized file and either OOM or surface a JSON SyntaxError, not the bounded size refusal. + const dir = createTempDir('gsd-cap-stagedread-direct-'); + try { + const padded = Buffer.alloc(MANIFEST_MAX_BYTES + 1, 0x20); // spaces — over the cap by one byte. + fs.writeFileSync(path.join(dir, 'capability.json'), padded); + assert.throws( + () => capSource._readManifestBounded(path.join(dir, 'capability.json'), 'capability.json not found'), + /exceeds|size|maximum|not a regular file|cannot read|not found/i, + 'the bounded staged re-read refuses an oversized manifest on size', + ); + } finally { + cleanup(dir); + } + }); +}); + +// --------------------------------------------------------------------------- +// #1461 finding 1 (HIGH) — the STAGED bundle directory is byte-bounded UNIFORMLY. +// realHttpsGet is capped, but copyDirRecursive / git clone / npm pack / tar +// extraction RESULTS were only TIMEOUT-bounded, so a huge source tree / repo / +// package / tar bomb could fill disk during staging. stageValidated now enforces +// ONE aggregate byte-budget (MAX_STAGED_BUNDLE_BYTES) over the staged dir via a +// BOUNDED streaming walk AFTER staging and BEFORE validation/promotion — a single +// chokepoint that covers EVERY adapter (tar / npm / git / local). +// --------------------------------------------------------------------------- +describe('#1461 finding 1 — the staged bundle dir is aggregate-byte-bounded (uniform DoS bound)', () => { + let gsdHome = ''; + beforeEach(() => { gsdHome = createTempDir('gsd-cap-stagebudget-'); }); + afterEach(() => { + _setCapabilitySourceHttpGet(null); + cleanup(gsdHome); + }); + + test('MAX_STAGED_BUNDLE_BYTES is a sane bounded cap (128 MiB)', () => { + assert.strictEqual(MAX_STAGED_BUNDLE_BYTES, 128 * 1024 * 1024, 'staged-bundle cap is generous but bounded'); + }); + + test('local: a staged bundle whose TOTAL bytes exceed the budget is refused before promotion', async () => { + // A valid small manifest (so id/validation pass), plus a sibling artifact whose size pushes the + // bundle TOTAL over the budget. The bounded streaming walk in stageValidated sums regular-file + // bytes and fails closed BEFORE validation/promotion. + // + // REVERT-FAILS: without the staged-dir aggregate budget, copyDirRecursive copies the oversized + // artifact into staging and the source resolves+promotes normally — the oversized bundle lands on + // disk. With the budget, the bounded walk throws and the source never resolves. + const dir = createTempDir('gsd-cap-stagebudget-local-'); + try { + fs.writeFileSync(path.join(dir, 'capability.json'), JSON.stringify(featureCap('stagebudget-cap')), 'utf8'); + // One byte over the budget across a single artifact is enough for the cumulative counter to trip. + // ftruncate makes a sparse file so we don't actually write 128 MiB of real bytes (st.size still + // reports the full length, which is what the budget walk sums). + const big = path.join(dir, 'artifact.bin'); + const fd = fs.openSync(big, 'w'); + try { fs.ftruncateSync(fd, MAX_STAGED_BUNDLE_BYTES + 1); } finally { fs.closeSync(fd); } + + await assert.rejects( + () => resolveCapabilitySource(dir, { gsdHome, hostVersion: '1.5.0' }), + /staged bundle|exceeds|budget|maximum|too large/i, + 'an over-budget staged bundle must be refused before promotion', + ); + } finally { + cleanup(dir); + } + const capRoot = path.join(gsdHome, '.gsd', 'capabilities'); + assert.ok(!fs.existsSync(capRoot) || + fs.readdirSync(capRoot).filter((e) => e !== '.staging').length === 0, + 'no capability is promoted after an over-budget bundle is refused'); + // The staging dir for the rejected bundle must be cleaned up (atomicity). + const stagingRoot = path.join(capRoot, '.staging'); + if (fs.existsSync(stagingRoot)) { + assert.strictEqual(fs.readdirSync(stagingRoot).length, 0, '.staging must be empty after an over-budget refusal'); + } + }); + + test('tarball: an extracted bundle over the budget is refused at the common staging chokepoint', async () => { + // Drives the budget via the tarball adapter to prove the chokepoint is adapter-agnostic: the tar + // override "extracts" an oversized artifact next to a valid manifest; stageValidated's budget walk + // (after copyDirRecursive) rejects it. + const cap = featureCap('stagebudget-tar-cap'); + const tgzBuf = _fakeTarball(cap); + _setCapabilitySourceHttpGet(() => Promise.resolve({ statusCode: 200, body: tgzBuf })); + await assert.rejects( + () => resolveCapabilitySource('https://example.com/big.tgz', { + gsdHome, hostVersion: '1.5.0', + execOverrides: { + tar: (_prog, args) => { + if (args[0] === '-tzf') { + return { exitCode: 0, stdout: 'capability.json\nbomb.bin\n', stderr: '', signal: null, error: null }; + } + if (args[0] === '-tvzf') { + return { + exitCode: 0, + stdout: + '-rw-r--r-- 0 user group 10 Jan 1 2020 capability.json\n' + + '-rw-r--r-- 0 user group 10 Jan 1 2020 bomb.bin\n', + stderr: '', signal: null, error: null, + }; + } + // "extraction": write a valid manifest + an oversized (sparse) artifact into extractDir. + const extractDir = args[args.indexOf('-C') + 1]; + fs.writeFileSync(path.join(extractDir, 'capability.json'), JSON.stringify(cap), 'utf8'); + const fd = fs.openSync(path.join(extractDir, 'bomb.bin'), 'w'); + try { fs.ftruncateSync(fd, MAX_STAGED_BUNDLE_BYTES + 1); } finally { fs.closeSync(fd); } + return { exitCode: 0, stdout: '', stderr: '', signal: null, error: null }; + }, + }, + }), + /staged bundle|exceeds|budget|maximum|too large/i, + 'an over-budget extracted tarball must be refused at the staging chokepoint', + ); + const capRoot = path.join(gsdHome, '.gsd', 'capabilities'); + assert.ok(!fs.existsSync(capRoot) || + fs.readdirSync(capRoot).filter((e) => e !== '.staging').length === 0, + 'no capability is promoted after an over-budget tarball is refused'); + }); + + test('CONTROL: a within-budget bundle still resolves + promotes normally', async () => { + const dir = makeLocalCap(featureCap('stagebudget-control-cap')); + try { + const result = await resolveCapabilitySource(dir, { gsdHome, hostVersion: '1.5.0' }); + assert.strictEqual(result.id, 'stagebudget-control-cap', 'a within-budget bundle resolves'); + assert.ok(fs.existsSync(path.join(result.stagedDir, 'capability.json')), 'staged manifest present'); + } finally { + cleanup(dir); + } + }); +}); + +// --------------------------------------------------------------------------- +// #1461 finding 1 (HIGH, ROUND 2) — copyDirRecursive itself is STREAMING + +// BUDGETED, so the COPY can never materialize a whole hostile directory. +// +// The post-copy assertStagedBundleWithinBudget walk is bounded, but it runs +// AFTER copyDirRecursive. The OLD copyDirRecursive used +// `fs.readdirSync(src, { withFileTypes: true })`, which materializes the ENTIRE +// directory-entry array into memory BEFORE the budget walk can run — so a +// hostile source tree with a directory holding millions of tiny entries +// (fetch < 64 MiB, but a colossal dirent array) OOMs the process during the +// COPY, before the post-copy budget can fail closed. copyDirRecursive now +// streams each directory via fs.opendirSync + dir.readSync() and threads a +// cumulative entry + byte counter, throwing the MOMENT either cap is exceeded — +// DURING the copy, before reading/copying the rest. +// --------------------------------------------------------------------------- +describe('#1461 finding 1 (ROUND 2) — copyDirRecursive streams + budgets the copy (no full materialization)', () => { + let gsdHome = ''; + beforeEach(() => { gsdHome = createTempDir('gsd-cap-copybudget-'); }); + afterEach(() => { + _setCapabilitySourceHttpGet(null); + cleanup(gsdHome); + }); + + test('MAX_STAGED_BUNDLE_ENTRIES is exported as a sane bounded cap (100k)', () => { + assert.strictEqual(MAX_STAGED_BUNDLE_ENTRIES, 100_000, 'staged-bundle entry cap is generous but bounded'); + }); + + test('a source dir with more than MAX_STAGED_BUNDLE_ENTRIES entries is refused, and the copy NEVER readdirSyncs the source', async () => { + // ANTI-VACUOUS, two-pronged: + // (1) the bounded entry error fires (fail closed), AND + // (2) the COPY enumerated the source via opendirSync/readSync — it did NOT call the + // whole-array `fs.readdirSync(src, { withFileTypes:true })` materialization on the source. + // + // We don't actually create 100k real files (slow + disk). Instead we monkeypatch fs.opendirSync to + // return a SYNTHETIC Dir whose readSync() yields lazily-generated tiny-file dirents far past the cap, + // while a real on-disk capability.json + a few real files back the source so non-enumeration fs ops + // still work. We also spy fs.readdirSync to PROVE the whole-array materialization is never used on + // the source during the copy. + // + // REVERT-FAILS: reverting copyDirRecursive to `fs.readdirSync(src, { withFileTypes:true })` makes the + // copy call readdirSync on the source (assertion (2) fails) AND would materialize the entire (here + // synthetic, effectively unbounded) dirent array before any cap could trip. + const src = createTempDir('gsd-cap-copybudget-src-'); + try { + fs.writeFileSync(path.join(src, 'capability.json'), JSON.stringify(featureCap('copybudget-cap')), 'utf8'); + + const TOTAL_SYNTH = MAX_STAGED_BUNDLE_ENTRIES + 50_000; // far past the cap + let readSyncCalls = 0; + let readdirOnSrc = 0; + let opendirOnSrc = 0; + + const realOpendir = fs.opendirSync; + const realReaddir = fs.readdirSync; + const realLstat = fs.lstatSync; + const realCopyFile = fs.copyFileSync; + + // Make any per-entry lstat/copy of a synthetic file a no-op (the files don't exist on disk). + fs.lstatSync = function patchedLstat(p, ...rest) { + if (typeof p === 'string' && /[/\\]synth-\d+\.txt$/.test(p)) { + return { + isSymbolicLink: () => false, + isDirectory: () => false, + isFile: () => true, + size: 1, + }; + } + return realLstat.call(this, p, ...rest); + }; + fs.copyFileSync = function patchedCopyFile(s, d, ...rest) { + if (typeof s === 'string' && /[/\\]synth-\d+\.txt$/.test(s)) return undefined; + return realCopyFile.call(this, s, d, ...rest); + }; + + fs.readdirSync = function patchedReaddir(p, ...rest) { + if (p === src) readdirOnSrc++; + return realReaddir.call(this, p, ...rest); + }; + + fs.opendirSync = function patchedOpendir(p, ...rest) { + if (p === src) { + opendirOnSrc++; + let i = 0; + // A streaming Dir that yields the real manifest first, then synthetic tiny files lazily. + return { + readSync() { + readSyncCalls++; + if (i === 0) { i++; return makeDirent('capability.json', { file: true }); } + if (i <= TOTAL_SYNTH) { const n = i++; return makeDirent(`synth-${n}.txt`, { file: true }); } + return null; + }, + closeSync() {}, + [Symbol.iterator]() { return this; }, + }; + } + return realOpendir.call(this, p, ...rest); + }; + + try { + await assert.rejects( + () => resolveCapabilitySource(src, { gsdHome, hostVersion: '1.5.0' }), + /entry count exceeds|maximum of 100000|too many entries/i, + 'a source with more than the entry cap must be refused during the streaming copy', + ); + } finally { + fs.opendirSync = realOpendir; + fs.readdirSync = realReaddir; + fs.lstatSync = realLstat; + fs.copyFileSync = realCopyFile; + } + + // (2a) The copy enumerated the source via the streaming opendir/readSync path. + assert.ok(opendirOnSrc >= 1, 'copyDirRecursive must opendirSync the source (streaming)'); + assert.ok(readSyncCalls >= 1, 'copyDirRecursive must readSync the source (streaming)'); + // (2b) The copy did NOT use the whole-array readdirSync materialization on the source. + assert.strictEqual(readdirOnSrc, 0, 'copyDirRecursive must NOT readdirSync the source (no full materialization)'); + // (2c) It aborted at ~the cap, NOT after enumerating all TOTAL_SYNTH entries. + assert.ok( + readSyncCalls <= MAX_STAGED_BUNDLE_ENTRIES + 5, + `streaming copy must abort at ~the cap (readSync called ${readSyncCalls}, cap ${MAX_STAGED_BUNDLE_ENTRIES})`, + ); + } finally { + cleanup(src); + } + // Atomicity: nothing promoted; .staging cleaned up. + const capRoot = path.join(gsdHome, '.gsd', 'capabilities'); + assert.ok(!fs.existsSync(capRoot) || + fs.readdirSync(capRoot).filter((e) => e !== '.staging').length === 0, + 'no capability is promoted after an over-entry-budget bundle is refused'); + const stagingRoot = path.join(capRoot, '.staging'); + if (fs.existsSync(stagingRoot)) { + assert.strictEqual(fs.readdirSync(stagingRoot).length, 0, '.staging must be empty after an over-entry refusal'); + } + }); + + test('a source whose copied bytes exceed the budget is refused DURING the copy (cumulative byte counter)', async () => { + // The streaming copy threads a cumulative BYTE counter too: an oversized regular file trips the byte + // cap during the copy, before the rest of the tree is read/copied. + // + // REVERT-FAILS: the old copyDirRecursive copied everything unconditionally and relied solely on the + // post-copy walk; if that post-copy walk were ALSO removed (and copy not budgeted), the oversized + // artifact would land in staging. With the in-copy byte budget the copy itself fails closed. + const src = createTempDir('gsd-cap-copybudget-bytes-'); + try { + fs.writeFileSync(path.join(src, 'capability.json'), JSON.stringify(featureCap('copybudget-bytes-cap')), 'utf8'); + const big = path.join(src, 'artifact.bin'); + const fd = fs.openSync(big, 'w'); + try { fs.ftruncateSync(fd, MAX_STAGED_BUNDLE_BYTES + 1); } finally { fs.closeSync(fd); } + + await assert.rejects( + () => resolveCapabilitySource(src, { gsdHome, hostVersion: '1.5.0' }), + /exceeds|budget|maximum|too large|staged bundle/i, + 'an over-byte-budget source must be refused during the streaming copy', + ); + } finally { + cleanup(src); + } + const capRoot = path.join(gsdHome, '.gsd', 'capabilities'); + assert.ok(!fs.existsSync(capRoot) || + fs.readdirSync(capRoot).filter((e) => e !== '.staging').length === 0, + 'no capability is promoted after an over-byte-budget bundle is refused'); + }); + + test('CONTROL: a normal small source still stages through the streaming copy', async () => { + const dir = createTempDir('gsd-cap-copybudget-control-'); + try { + fs.writeFileSync(path.join(dir, 'capability.json'), JSON.stringify(featureCap('copybudget-control-cap')), 'utf8'); + fs.mkdirSync(path.join(dir, 'nested'), { recursive: true }); + fs.writeFileSync(path.join(dir, 'nested', 'extra.txt'), 'hello', 'utf8'); + + const result = await resolveCapabilitySource(dir, { gsdHome, hostVersion: '1.5.0' }); + assert.strictEqual(result.id, 'copybudget-control-cap', 'a within-budget bundle resolves'); + assert.ok(fs.existsSync(path.join(result.stagedDir, 'capability.json')), 'staged manifest present'); + assert.ok(fs.existsSync(path.join(result.stagedDir, 'nested', 'extra.txt')), 'nested file copied through the streaming copy'); + assert.strictEqual(fs.readFileSync(path.join(result.stagedDir, 'nested', 'extra.txt'), 'utf8'), 'hello', 'nested file bytes preserved'); + } finally { + cleanup(dir); + } + }); +}); + +// --------------------------------------------------------------------------- +// #1461 finding 2 (MED) — the spoofable parseTarMemberSize header-size parse was +// REMOVED. The staged-dir aggregate budget (finding 1) is now the real bound on +// the extracted RESULT, so the fragile/spoofable BSD-vs-GNU date-token size scan +// is gone. The tar NAME/TYPE guards (traversal, symlink, hardlink) remain and a +// within-name/type tarball still resolves through extraction. +// --------------------------------------------------------------------------- +describe('#1461 finding 2 — tar header-size parse removed; NAME/TYPE guards remain', () => { + let gsdHome = ''; + beforeEach(() => { gsdHome = createTempDir('gsd-cap-tarsize-removed-'); }); + afterEach(() => { + _setCapabilitySourceHttpGet(null); + cleanup(gsdHome); + }); + + test('the spoofable per-member size cap export (MAX_TAR_MEMBER_BYTES) is REMOVED', () => { + // REVERT-FAILS: if parseTarMemberSize / its export are re-introduced, this asserts the constant + // is gone (the spoofable BSD-owner="Jan" mis-anchor fail-open is no longer relied upon). + assert.strictEqual(capSource.MAX_TAR_MEMBER_BYTES, undefined, 'the spoofable per-member tar size cap is removed'); + }); + + test('a tar with a HUGE declared member size (formerly rejected by the size parse) now extracts — and the staged budget is the real bound', async () => { + // This listing once tripped the per-member size cap. With that removed, the NAME/TYPE guards pass + // and extraction proceeds; a WITHIN-budget extracted result resolves normally. (An over-budget + // result would be caught by finding 1's staged-dir budget, exercised in the finding-1 suite.) + const cap = featureCap('tarsize-removed-cap'); + const tgzBuf = _fakeTarball(cap); + _setCapabilitySourceHttpGet(() => Promise.resolve({ statusCode: 200, body: tgzBuf })); + const result = await resolveCapabilitySource('https://example.com/huge-decl.tgz', { + gsdHome, hostVersion: '1.5.0', + execOverrides: { + tar: (_prog, args) => { + if (args[0] === '-tzf') { + return { exitCode: 0, stdout: 'capability.json\n', stderr: '', signal: null, error: null }; + } + if (args[0] === '-tvzf') { + // A wildly large DECLARED size in the column — formerly rejected, now ignored. + return { exitCode: 0, stdout: '-rw-r--r-- 0 user group 999999999999 Jan 1 2020 capability.json\n', stderr: '', signal: null, error: null }; + } + const extractDir = args[args.indexOf('-C') + 1]; + fs.writeFileSync(path.join(extractDir, 'capability.json'), JSON.stringify(cap), 'utf8'); + return { exitCode: 0, stdout: '', stderr: '', signal: null, error: null }; + }, + }, + }); + assert.strictEqual(result.id, 'tarsize-removed-cap', 'a huge-declared-size tar with safe names/types now extracts'); + assert.ok(fs.existsSync(result.stagedDir), 'staged dir exists after extraction'); + }); +});