diff --git a/.github/workflows/polyfill-connectors.yml b/.github/workflows/polyfill-connectors.yml index 8875f8e4d..76dfc304c 100644 --- a/.github/workflows/polyfill-connectors.yml +++ b/.github/workflows/polyfill-connectors.yml @@ -72,7 +72,7 @@ jobs: verify: name: verify + test runs-on: ubuntu-latest - timeout-minutes: 10 + timeout-minutes: 15 steps: - name: Checkout diff --git a/packages/polyfill-connectors/bin/scenario-verify-strict.test.ts b/packages/polyfill-connectors/bin/scenario-verify-strict.test.ts index eadfbdc70..96dd4a6c1 100644 --- a/packages/polyfill-connectors/bin/scenario-verify-strict.test.ts +++ b/packages/polyfill-connectors/bin/scenario-verify-strict.test.ts @@ -857,6 +857,10 @@ function eligibleDigestObservations(): { currentSourceDigestComputed: boolean; observedUnsupportedEvidenceSurface: boolean; driverEvidenceSatisfied: boolean; + isolationEvidenceBoundaryProven: boolean; + preexistingSocketsUnderReadOnlyBinds: readonly string[]; + preexistingSocketScanIncomplete: boolean; + preexistingSocketScanUnreadablePaths: readonly string[]; } { return { capturedDeclarationDigestPresent: true, @@ -865,6 +869,10 @@ function eligibleDigestObservations(): { currentSourceDigestComputed: true, observedUnsupportedEvidenceSurface: false, driverEvidenceSatisfied: true, + isolationEvidenceBoundaryProven: true, + preexistingSocketScanIncomplete: false, + preexistingSocketScanUnreadablePaths: [], + preexistingSocketsUnderReadOnlyBinds: [], }; } @@ -954,6 +962,10 @@ test("evaluateClaimEligibility negative control: source-only historical scenario currentSourceDigestComputed: true, observedUnsupportedEvidenceSurface: false, driverEvidenceSatisfied: true, + isolationEvidenceBoundaryProven: true, + preexistingSocketsUnderReadOnlyBinds: [], + preexistingSocketScanIncomplete: false, + preexistingSocketScanUnreadablePaths: [], isNamespaceIsolationActive: true, }); assert.equal(decision.claim, "diagnostic_replay"); @@ -971,6 +983,10 @@ test("evaluateClaimEligibility negative control: declaration-only scenario (sour currentSourceDigestComputed: true, observedUnsupportedEvidenceSurface: false, driverEvidenceSatisfied: true, + isolationEvidenceBoundaryProven: true, + preexistingSocketsUnderReadOnlyBinds: [], + preexistingSocketScanIncomplete: false, + preexistingSocketScanUnreadablePaths: [], isNamespaceIsolationActive: true, }); assert.equal(decision.claim, "diagnostic_replay"); @@ -988,6 +1004,10 @@ test("evaluateClaimEligibility negative control: missing current manifest (decla currentSourceDigestComputed: true, observedUnsupportedEvidenceSurface: false, driverEvidenceSatisfied: true, + isolationEvidenceBoundaryProven: true, + preexistingSocketsUnderReadOnlyBinds: [], + preexistingSocketScanIncomplete: false, + preexistingSocketScanUnreadablePaths: [], isNamespaceIsolationActive: true, }); assert.equal(decision.claim, "diagnostic_replay"); @@ -1005,6 +1025,10 @@ test("evaluateClaimEligibility negative control: missing current connector sourc currentSourceDigestComputed: false, observedUnsupportedEvidenceSurface: false, driverEvidenceSatisfied: true, + isolationEvidenceBoundaryProven: true, + preexistingSocketsUnderReadOnlyBinds: [], + preexistingSocketScanIncomplete: false, + preexistingSocketScanUnreadablePaths: [], isNamespaceIsolationActive: true, }); assert.equal(decision.claim, "diagnostic_replay"); @@ -1029,6 +1053,10 @@ test("evaluateClaimEligibility negative control: legacy top-level digests only ( currentSourceDigestComputed: true, observedUnsupportedEvidenceSurface: false, driverEvidenceSatisfied: true, + isolationEvidenceBoundaryProven: true, + preexistingSocketsUnderReadOnlyBinds: [], + preexistingSocketScanIncomplete: false, + preexistingSocketScanUnreadablePaths: [], isNamespaceIsolationActive: true, }); assert.equal(decision.claim, "diagnostic_replay"); @@ -1111,6 +1139,10 @@ test("evaluateClaimEligibility: multiple failing conditions are all reported at observedUnsupportedEvidenceSurface: true, driverEvidenceSatisfied: false, isNamespaceIsolationActive: false, + isolationEvidenceBoundaryProven: false, + preexistingSocketsUnderReadOnlyBinds: [], + preexistingSocketScanIncomplete: false, + preexistingSocketScanUnreadablePaths: [], }); assert.equal(decision.claim, "diagnostic_replay"); assert.ok(decision.claim === "diagnostic_replay"); @@ -1152,6 +1184,197 @@ test("evaluateClaimEligibility: with every other condition eligible, the claim t } }); +// ─── Bounded P1 repair (external review of ab415be6c): isolation evidence +// boundary — launcher trust + recursive read-only, on TOP of namespace +// activity alone ──────────────────────────────────────────────────────────── +// +// The review found that `isNamespaceIsolationActive: true` (the OS +// namespaces genuinely exist) was being treated as sufficient for +// `recorded_replay`, even though two separate defects meant the FILESYSTEM +// half of that isolation could be unproven: the `unshare`/`bwrap` launcher +// binaries were resolved through the caller's inherited `$PATH` (a +// PATH-prepended fake launcher could be selected), and the unshare +// mechanism's `--rbind` submounts only had their top mount remounted +// read-only, leaving nested mounts under a `ro` bind writable. These tests +// pin that namespace-active alone can never reach `recorded_replay` — the +// new `isolationEvidenceBoundaryProven` observation must ALSO be true. + +test("evaluateClaimEligibility: namespace isolation active but isolationEvidenceBoundaryProven false withholds recorded_replay (launcher trust / recursive-ro not proven)", () => { + const decision = evaluateClaimEligibility({ + scenario: eligibleScenario(), + isEntrypointOverride: false, + ...eligibleDigestObservations(), + isNamespaceIsolationActive: true, + isolationEvidenceBoundaryProven: false, + }); + assert.equal(decision.claim, "diagnostic_replay"); + assert.ok(decision.claim === "diagnostic_replay"); + assert.deepEqual(decision.limitations, [ + "network isolation: launcher trust or recursive read-only filesystem closure not proven for this run", + ]); +}); + +test("evaluateClaimEligibility: namespace isolation active AND isolationEvidenceBoundaryProven true — recorded_replay is reachable again", () => { + const decision = evaluateClaimEligibility({ + scenario: eligibleScenario(), + isEntrypointOverride: false, + ...eligibleDigestObservations(), + isNamespaceIsolationActive: true, + isolationEvidenceBoundaryProven: true, + preexistingSocketsUnderReadOnlyBinds: [], + }); + assert.deepEqual(decision, { claim: "recorded_replay" }); +}); + +test("evaluateClaimEligibility: namespace isolation NOT active reports only the coarser process-local limitation, never BOTH isolation limitations at once", () => { + // When isolation isn't active at all, the boundary-proof limitation is + // redundant with (and would be confusing alongside) the coarser + // process-local-only limitation — evaluateClaimEligibility's `else if` + // must report exactly one of the two, never both. + const decision = evaluateClaimEligibility({ + scenario: eligibleScenario(), + isEntrypointOverride: false, + ...eligibleDigestObservations(), + isNamespaceIsolationActive: false, + isolationEvidenceBoundaryProven: false, + }); + assert.equal(decision.claim, "diagnostic_replay"); + assert.ok(decision.claim === "diagnostic_replay"); + assert.deepEqual(decision.limitations, ["network isolation: process-local only - descendant escape not excluded"]); +}); + +// ─── Repository-UDS exception, reconciled (P1, external review of ab415be6c) +// ──────────────────────────────────────────────────────────────────────────── +// +// Recursive read-only closes the ability to CREATE a socket under a ro +// bind, but not the ability to DIAL one that already existed at spawn time +// — see claims.ts's `preexistingSocketsUnderReadOnlyBinds` doc comment. +// These tests pin the eligibility gate's own handling of the scan result: +// a non-empty result withholds recorded_replay and names every path found; +// an empty result does not withhold on this condition at all. + +test("evaluateClaimEligibility: a non-empty preexistingSocketsUnderReadOnlyBinds withholds recorded_replay and names the socket path", () => { + const decision = evaluateClaimEligibility({ + scenario: eligibleScenario(), + isEntrypointOverride: false, + ...eligibleDigestObservations(), + isNamespaceIsolationActive: true, + isolationEvidenceBoundaryProven: true, + preexistingSocketsUnderReadOnlyBinds: ["/repo/root/.leftover.sock"], + }); + assert.equal(decision.claim, "diagnostic_replay"); + assert.ok(decision.claim === "diagnostic_replay"); + assert.deepEqual(decision.limitations, [ + "pre-existing socket(s) found under a read-only bind at spawn time, dialable despite recursive read-only: /repo/root/.leftover.sock", + ]); +}); + +test("evaluateClaimEligibility: multiple preexistingSocketsUnderReadOnlyBinds are all named in one limitation, comma-joined", () => { + const decision = evaluateClaimEligibility({ + scenario: eligibleScenario(), + isEntrypointOverride: false, + ...eligibleDigestObservations(), + isNamespaceIsolationActive: true, + isolationEvidenceBoundaryProven: true, + preexistingSocketsUnderReadOnlyBinds: ["/repo/root/a.sock", "/repo/root/nested/b.sock"], + }); + assert.ok(decision.claim === "diagnostic_replay"); + assert.deepEqual(decision.limitations, [ + "pre-existing socket(s) found under a read-only bind at spawn time, dialable despite recursive read-only: /repo/root/a.sock, /repo/root/nested/b.sock", + ]); +}); + +test("evaluateClaimEligibility: an empty preexistingSocketsUnderReadOnlyBinds does not withhold on this condition — recorded_replay reachable", () => { + const decision = evaluateClaimEligibility({ + scenario: eligibleScenario(), + isEntrypointOverride: false, + ...eligibleDigestObservations(), + isNamespaceIsolationActive: true, + isolationEvidenceBoundaryProven: true, + preexistingSocketsUnderReadOnlyBinds: [], + }); + assert.deepEqual(decision, { claim: "recorded_replay" }); +}); + +// ─── Scan-completeness gate (P1-2, external review of ced8300be) ────────── +// +// A scan that could not fully enumerate a subtree is NOT the same fact as +// "scanned, found nothing" — `preexistingSocketScanIncomplete` must +// independently withhold `recorded_replay`, on the same severity as a +// non-empty `preexistingSocketsUnderReadOnlyBinds`, even when the socket +// list itself is empty (an incomplete scan means that empty list cannot be +// trusted as exhaustive). + +test("evaluateClaimEligibility: preexistingSocketScanIncomplete withholds recorded_replay even when preexistingSocketsUnderReadOnlyBinds is empty", () => { + const decision = evaluateClaimEligibility({ + scenario: eligibleScenario(), + isEntrypointOverride: false, + ...eligibleDigestObservations(), + isNamespaceIsolationActive: true, + isolationEvidenceBoundaryProven: true, + preexistingSocketsUnderReadOnlyBinds: [], + preexistingSocketScanIncomplete: true, + preexistingSocketScanUnreadablePaths: ["/repo/.pdpp-blocked-subtree"], + }); + assert.equal(decision.claim, "diagnostic_replay"); + assert.ok(decision.claim === "diagnostic_replay"); + assert.deepEqual(decision.limitations, [ + "pre-existing-socket scan could not fully enumerate one or more read-only bind subtrees (unreadable path(s), possibly hiding a dialable socket): /repo/.pdpp-blocked-subtree", + ]); +}); + +test("evaluateClaimEligibility: a non-empty preexistingSocketsUnderReadOnlyBinds takes priority over preexistingSocketScanIncomplete (the more specific, more actionable limitation wins)", () => { + const decision = evaluateClaimEligibility({ + scenario: eligibleScenario(), + isEntrypointOverride: false, + ...eligibleDigestObservations(), + isNamespaceIsolationActive: true, + isolationEvidenceBoundaryProven: true, + preexistingSocketsUnderReadOnlyBinds: ["/repo/found.sock"], + preexistingSocketScanIncomplete: true, + preexistingSocketScanUnreadablePaths: ["/repo/.pdpp-blocked-subtree"], + }); + assert.equal(decision.claim, "diagnostic_replay"); + assert.ok(decision.claim === "diagnostic_replay"); + assert.deepEqual(decision.limitations, [ + "pre-existing socket(s) found under a read-only bind at spawn time, dialable despite recursive read-only: /repo/found.sock", + ]); +}); + +test("evaluateClaimEligibility: preexistingSocketScanIncomplete is irrelevant when isolation itself is not active (the coarser limitation fires instead)", () => { + const decision = evaluateClaimEligibility({ + scenario: eligibleScenario(), + isEntrypointOverride: false, + ...eligibleDigestObservations(), + isNamespaceIsolationActive: false, + isolationEvidenceBoundaryProven: false, + preexistingSocketsUnderReadOnlyBinds: [], + preexistingSocketScanIncomplete: true, + preexistingSocketScanUnreadablePaths: ["/repo/.pdpp-blocked-subtree"], + }); + assert.equal(decision.claim, "diagnostic_replay"); + assert.ok(decision.claim === "diagnostic_replay"); + assert.deepEqual(decision.limitations, ["network isolation: process-local only - descendant escape not excluded"]); +}); + +test("evaluateClaimEligibility: the socket-scan limitation only fires when isolation is active AND the evidence boundary is proven (not a fourth, independent gate)", () => { + // If isolation isn't active at all, the coarser process-local limitation + // must fire instead — a non-empty socket scan result is meaningless + // (and, in bin/scenario-verify.ts's real wiring, always empty) when + // isolation was never active for this run. + const decision = evaluateClaimEligibility({ + scenario: eligibleScenario(), + isEntrypointOverride: false, + ...eligibleDigestObservations(), + isNamespaceIsolationActive: false, + isolationEvidenceBoundaryProven: false, + preexistingSocketsUnderReadOnlyBinds: ["/repo/root/.leftover.sock"], + }); + assert.equal(decision.claim, "diagnostic_replay"); + assert.ok(decision.claim === "diagnostic_replay"); + assert.deepEqual(decision.limitations, ["network isolation: process-local only - descendant escape not excluded"]); +}); + // ─── Repair wave 6, P1-1: driver-evidence prerequisite ───────────────────── // // `wire-registry.ts`'s `DRIVER_EVIDENCE_POLICIES` map — `recorded-http`'s diff --git a/packages/polyfill-connectors/bin/scenario-verify.ts b/packages/polyfill-connectors/bin/scenario-verify.ts index 8a38b2562..060b32e51 100644 --- a/packages/polyfill-connectors/bin/scenario-verify.ts +++ b/packages/polyfill-connectors/bin/scenario-verify.ts @@ -92,9 +92,11 @@ import { import { evaluateClaimEligibility } from "../src/scenario/claims.ts"; import type { ConnectorScenario, ScenarioUserInteraction } from "../src/scenario/format.ts"; import { + findPreexistingSocketsUnderReadOnlyBinds, type IsolationMechanism, isNamespaceIsolationAvailable, type NamespaceIsolationCapability, + type SocketScanResult, sandboxScratchEnv, spawnWithNetworkIsolation, } from "../src/scenario/isolation.ts"; @@ -1387,6 +1389,40 @@ function resolveIsolationMechanism(capability: NamespaceIsolationCapability): fa return capability.available ? capability.mechanism : false; } +/** + * REPOSITORY-UDS EXCEPTION, host-side PRE-FLIGHT scan (P1, external review + * of ab415be6c; return contract and TOCTOU scope corrected P1-2, external + * review of ced8300be) — this is ONE of THREE scans a fully-isolated replay + * now performs, not the only one: it runs ONCE, from the host process, + * before any run's subprocess spawns, and its result is EARLY signal, not + * the sole authority the strong claim rests on. `isolation.ts`'s + * `inNamespaceSocketScanStatement` runs the equivalent check TWICE more, IN + * NAMESPACE (after every ro-bind remount completes, and again immediately + * before `exec` — see that function's doc comment), narrowing the TOCTOU + * window this scan alone cannot close: a `ro` bind stops the SANDBOX from + * creating a socket under a bound path, but NOT a separate HOST process + * (outside the sandbox, with ordinary write access to the bind's SOURCE + * directory) from creating one between this early scan and the target + * command's actual `exec` — the earlier version of this comment claimed + * "nothing new can appear under a ro bind while replay runs," which + * overstated what recursive read-only proves (see `isolation.ts`'s + * `findPreexistingSocketsUnderReadOnlyBinds()` doc comment for the full + * correction). Only meaningful when isolation is actually active for this + * replay — an empty scan result under process-local-only isolation would be + * misleading (the escape this scan closes is specific to the OS-isolation + * boundary), so this returns a vacuously-clean result without scanning + * otherwise, which is also the CORRECT input for `evaluateClaimEligibility` + * in that case — the coarser `isNamespaceIsolationActive: false` limitation + * already fires and takes priority, per that function's `else if` chain. + * Split out of `main` purely to keep `main`'s own cognitive complexity + * under this package's lint ceiling. + */ +function scanPreexistingSocketsIfIsolated(isolationCapability: NamespaceIsolationCapability): SocketScanResult { + return isolationCapability.available + ? findPreexistingSocketsUnderReadOnlyBinds() + : { sockets: [], complete: true, errors: [] }; +} + async function main(): Promise { const args = parseArgs(process.argv.slice(2)); const connectorPath = resolveConnectorPath(args); @@ -1455,6 +1491,7 @@ async function main(): Promise { ? "network isolation: os-namespace" : `network isolation: process-local only (${isolationCapability.reason})`; process.stdout.write(` ${isolationLine}\n`); + const socketScanResult = scanPreexistingSocketsIfIsolated(isolationCapability); // Every replayed response is served from the recording, not a live // provider, so a connector's own pacing/backoff timers (governor pacing, // an inline PAGE_DELAY sleep, anything else built on setTimeout/ @@ -1716,7 +1753,8 @@ async function main(): Promise { isolationLine, digestObservation, isolationCapability, - observedUnsupportedEvidenceSurface(allRunMessages) + observedUnsupportedEvidenceSurface(allRunMessages), + socketScanResult ); process.exitCode = 0; } @@ -1748,7 +1786,8 @@ function printCoverageReport( isolationLine: string, digestObservation: CaptureSourceDigestObservation, isolationCapability: NamespaceIsolationCapability, - observedUnsupportedEvidenceSurfaceFlag: boolean + observedUnsupportedEvidenceSurfaceFlag: boolean, + socketScanResult: SocketScanResult ): void { const capturedAt = scenario.capture.captured_at; // state_seeded_second_run_with_changed_requests (formerly named @@ -1844,11 +1883,49 @@ function printCoverageReport( currentDeclarationDigestComputed: digestObservation.currentDeclarationDigestComputed, currentSourceDigestComputed: digestObservation.currentSourceDigestComputed, isNamespaceIsolationActive: isolationCapability.available, + // WITHHELD AGAIN PENDING INDEPENDENT REVIEW OF THIS ROUND'S REPAIR + // (external review of ced8300be, R11): the previous round + // (2714089be/774c4a620) flipped this from a hardcoded `false` to + // `isolationCapability.available`, genuinely re-enabling the strong + // claim, on the belief that the trusted-launcher and recursive-read-only + // repair was complete. The external reviewer's next pass found it was + // NOT: unshare (and bwrap) still invoked their inner `sh` through + // inherited PATH before any trusted PATH took effect (now fixed — see + // `resolveTrustedLauncherPath("sh")`), the pre-existing-socket scanner + // failed OPEN on an unreadable subtree and remained TOCTOU-prone (now + // fixed — see `SocketScanResult`/`inNamespaceSocketScanStatement`), and + // the mountinfo-based submount verification used naive word-splitting + // that could omit escaped paths and could not verify file submounts + // (see isolation.ts's mountinfo-parsing fix). Every one of THIS round's + // fixes is proven by its own test — but per the reviewer's explicit + // instruction ("keep recorded_replay withheld"), this literal stays + // hardcoded `false` until an INDEPENDENT review (not the maker who wrote + // these fixes) confirms the repair, matching this module's own + // "the maker is not the judge" discipline. Do not flip this back to + // `isolationCapability.available` in this same PR/round — that decision + // belongs to whoever reviews R11's changes, not to the commit that makes + // them. + isolationEvidenceBoundaryProven: false, + preexistingSocketsUnderReadOnlyBinds: socketScanResult.sockets, + preexistingSocketScanIncomplete: !socketScanResult.complete, + preexistingSocketScanUnreadablePaths: socketScanResult.errors, observedUnsupportedEvidenceSurface: observedUnsupportedEvidenceSurfaceFlag, driverEvidenceSatisfied: driverEvidenceOk, }); if (decision.claim === "recorded_replay") { - process.stdout.write(`\nrecorded_replay: PASS (captured ${capturedAt})\n`); + // A bare "PASS (captured ...)" line let a reader tell THAT the strong + // claim was earned but not WHAT was checked — the preconditions only + // ever surfaced as named limitations on the WITHHELD path. This branch + // is currently UNREACHABLE (isolationEvidenceBoundaryProven is + // hardcoded false above, pending independent review of R11's changes — + // see that field's own doc comment), kept ready for the commit that + // re-enables it once that review confirms the repair: once reachable + // again, it restates the (by-then-proven) preconditions inline instead + // of making the reader go read claims.ts. + process.stdout.write( + `\nrecorded_replay: PASS (captured ${capturedAt}; preconditions: trusted absolute launcher path, ` + + "every ro-bind submount verified read-only, no pre-existing sockets under writable-bound paths)\n" + ); } else { process.stdout.write(`\ndiagnostic_replay: PASS (captured ${capturedAt})\n`); process.stdout.write("recorded_replay: WITHHELD\n"); diff --git a/packages/polyfill-connectors/src/scenario/claims.ts b/packages/polyfill-connectors/src/scenario/claims.ts index e6a68a7a6..21ae0a346 100644 --- a/packages/polyfill-connectors/src/scenario/claims.ts +++ b/packages/polyfill-connectors/src/scenario/claims.ts @@ -6,7 +6,9 @@ * binding split and the ASSISTANCE-withholding condition added repair wave * 4, P1-1/P1-2; driver-evidence prerequisite added repair wave 6, P1-1; * recorded-browser driver support and its mandatory staleness limitation - * added alongside `browser-har-replay.ts`). + * added alongside `browser-har-replay.ts`; isolation-evidence-boundary and + * repository-UDS-socket-scan conditions added by the bounded P1 repair + * following external review of the merged tree ab415be6c). * * `bin/scenario-verify.ts` used to print `recorded_replay: PASS` the moment * every per-run comparison passed — but a passing comparison only proves the @@ -90,6 +92,40 @@ import type { ConnectorScenario } from "./format.ts"; export type ScenarioStalenessLimitation = `recorded-browser: verified against capture of ${string}; asserts nothing about the live provider`; +/** + * The repository-UDS-socket withholding limitation, PARAMETERIZED on the + * comma-joined list of pre-existing socket paths `findPreexistingSocketsUnderReadOnlyBinds()` + * found under a `ro` bind for THIS run — a TypeScript template-literal type, + * matching `ScenarioStalenessLimitation`'s own reasoning: the path(s) are + * genuinely per-run data, not a closed enum value. See + * `buildPreexistingSocketLimitation` below for the single place this string + * is constructed, and this module's doc comment ("REPOSITORY-UDS EXCEPTION, + * RECONCILED") for the reasoning: recursive read-only (isolation.ts's + * `recursiveReadOnlyRemountCommand`) closes the ability to CREATE a socket + * under a `ro` bind, so the only sockets an isolated child can ever dial + * through one are sockets that ALREADY EXISTED at spawn time — a finite, + * checkable fact about this specific run, not an open-ended exception. + */ +export type PreexistingSocketLimitation = + `pre-existing socket(s) found under a read-only bind at spawn time, dialable despite recursive read-only: ${string}`; + +/** + * The socket-scan-incomplete limitation, PARAMETERIZED on the comma-joined + * list of subtree paths `findPreexistingSocketsUnderReadOnlyBinds()` could + * NOT fully enumerate (P1-2, external review of ced8300be) — distinct from + * `PreexistingSocketLimitation` (a socket was actually FOUND) because "I + * could not prove this subtree clean" and "I found a socket" are two + * independently true facts a caller must be able to tell apart, matching + * `SocketScanResult`'s own `sockets`/`errors` split. An unreadable subtree + * (whether `chmod 000` or `chmod 311` — search-without-read, a genuinely + * separate DAC permission, confirmed to hide a live, connectable socket + * from enumeration exactly as effectively as `000` does) means the scan's + * `sockets` list cannot be trusted as exhaustive, so this withholds the + * strong claim identically to actually finding one. + */ +export type SocketScanIncompleteLimitation = + `pre-existing-socket scan could not fully enumerate one or more read-only bind subtrees (unreadable path(s), possibly hiding a dialable socket): ${string}`; + export type ClaimLimitation = | "unbound entrypoint replay" | "no capture-time declaration digest" @@ -100,11 +136,37 @@ export type ClaimLimitation = | "non-recorded-http driver - canonical replay is defined only for recorded-http" | "legacy scenario without protocol trace" | "network isolation: process-local only - descendant escape not excluded" + | "network isolation: launcher trust or recursive read-only filesystem closure not proven for this run" | "connector exercised an evidence surface the oracle cannot observe (ASSISTANCE)" | "no recorded provider interaction - driver evidence for recorded-http not satisfied" | "no recorded HAR entries - driver evidence for recorded-browser not satisfied" + | PreexistingSocketLimitation + | SocketScanIncompleteLimitation | ScenarioStalenessLimitation; +/** + * Builds the exact repository-UDS-socket limitation string for a run whose + * pre-existing-socket scan found at least one match. `socketPaths` must be + * non-empty — callers only invoke this when + * `preexistingSocketsUnderReadOnlyBinds.length > 0` (see + * `evaluateClaimEligibility`). + */ +export function buildPreexistingSocketLimitation(socketPaths: readonly string[]): PreexistingSocketLimitation { + return `pre-existing socket(s) found under a read-only bind at spawn time, dialable despite recursive read-only: ${socketPaths.join(", ")}`; +} + +/** + * Builds the exact socket-scan-incomplete limitation string for a run whose + * scan hit at least one unreadable subtree. `unreadablePaths` must be + * non-empty — callers only invoke this when + * `preexistingSocketScanIncomplete` is true (see `evaluateClaimEligibility`). + */ +export function buildSocketScanIncompleteLimitation( + unreadablePaths: readonly string[] +): SocketScanIncompleteLimitation { + return `pre-existing-socket scan could not fully enumerate one or more read-only bind subtrees (unreadable path(s), possibly hiding a dialable socket): ${unreadablePaths.join(", ")}`; +} + /** * Builds the exact staleness limitation string for a scenario carrying at * least one `recorded-browser` run — see `ScenarioStalenessLimitation`'s doc @@ -166,6 +228,26 @@ export interface ClaimEligibilityInput { * `isNamespaceIsolationAvailable()`) was ACTIVE for this replay, as * opposed to the weaker process-local-only fallback (condition f). */ isNamespaceIsolationActive: boolean; + /** + * External review of the merged tree (ab415be6c) found the evidence + * boundary this claim rests on was itself unproven in two ways: (1) the + * `unshare`/`bwrap` launcher binaries were resolved through the CALLER's + * inherited `$PATH` rather than a trusted absolute path, so a + * PATH-prepended fake launcher could be selected in place of the real one; + * (2) the unshare mechanism's `--rbind` submounts only had their TOP mount + * remounted read-only — Linux does not apply `remount,ro,bind` recursively + * — so a nested mount under a `ro` bind (e.g. `REPO_ROOT`) stayed writable. + * Either defect lets `isNamespaceIsolationActive` read `true` (namespaces + * genuinely exist) while the filesystem/launcher half of the OS-isolation + * claim does not actually hold. `isNamespaceIsolationActive` alone is no + * longer sufficient for `recorded_replay`: this field must ALSO be true, + * set by `bin/scenario-verify.ts` only once both the trusted-launcher + * resolution (isolation.ts's `resolveTrustedLauncherPath`) and the + * recursive-read-only post-pivot verification + * (`postPivotVerificationStatements`'s per-submount check) are wired in and + * this replay's own isolated child was verified against them. + */ + isolationEvidenceBoundaryProven: boolean; /** Repair wave 4 (P1-2, FIX 2d): true when this run's messages included at * least one kind `TRACE_POLICY` (verify.ts) dispositions * `"unsupported_claim_withheld"` — today, ASSISTANCE or ASSISTANCE_STATUS. @@ -173,6 +255,48 @@ export interface ClaimEligibilityInput { * oracle cannot observe, so even an otherwise-fully-eligible run must not * print the unqualified `recorded_replay: PASS` claim. */ observedUnsupportedEvidenceSurface: boolean; + /** + * SCAN COMPLETENESS (P1-2, external review of ced8300be): `false` whenever + * `findPreexistingSocketsUnderReadOnlyBinds()`'s scan could not fully + * enumerate one or more subtrees (an unreadable directory — `readdirSync` + * throws `EACCES` identically whether the directory is `chmod 000` or + * `chmod 311` — search permitted, read denied, a genuinely separate DAC + * bit that still leaves a KNOWN-name socket inside it fully connectable, + * confirmed empirically). Gated on the SAME `else if` rung as + * `preexistingSocketsUnderReadOnlyBinds` (see `evaluateClaimEligibility`) + * — an incomplete scan withholds `recorded_replay` exactly like a + * non-empty socket list does, never merely a softer warning, because an + * incomplete scan means the empty-array case above cannot be trusted as + * "scanned clean." + */ + preexistingSocketScanIncomplete: boolean; + /** Absolute paths of every subtree the scan could not enumerate — named + * explicitly so `buildSocketScanIncompleteLimitation`'s limitation string + * points at the exact path, matching this module's "name the path" + * discipline for every other socket-scan-derived limitation. Non-empty + * only when `preexistingSocketScanIncomplete` is true. */ + preexistingSocketScanUnreadablePaths: readonly string[]; + /** + * REPOSITORY-UDS EXCEPTION, RECONCILED (P1, external review of ab415be6c): + * a `ro` bind (e.g. `REPO_ROOT`) blocks WRITES, not reads/dials — a Unix + * domain socket file that already existed under a `ro` bind at spawn time + * stays dialable from inside the isolated child regardless of recursive + * read-only, confirmed empirically (a `curl --unix-socket` against a real + * `REPO_ROOT`-internal socket succeeds with both the trusted-launcher and + * recursive-ro fixes applied). Recursive read-only DOES close the other + * half: a connector cannot CREATE a new socket under a `ro` bind once + * every submount is genuinely read-only, so the only sockets reachable + * this way are ones that existed BEFORE the spawn — a finite, checkable + * precondition, not an open-ended gap. `bin/scenario-verify.ts` populates + * this from `isolation.ts`'s `findPreexistingSocketsUnderReadOnlyBinds()`, + * run immediately before spawning. An EMPTY array means the scan found + * nothing — combined with `isolationEvidenceBoundaryProven`, this is what + * justifies `recorded_replay`'s OS-isolation claim being airtight for this + * specific run; a NON-EMPTY array withholds the strong claim and names + * every path found (see `buildPreexistingSocketLimitation`), rather than + * silently accepting the old, undocumented, open-ended exception. + */ + preexistingSocketsUnderReadOnlyBinds: readonly string[]; scenario: ConnectorScenario; } @@ -250,6 +374,22 @@ export function evaluateClaimEligibility(input: ClaimEligibilityInput): ClaimDec } if (!input.isNamespaceIsolationActive) { limitations.push("network isolation: process-local only - descendant escape not excluded"); + } else if (!input.isolationEvidenceBoundaryProven) { + limitations.push( + "network isolation: launcher trust or recursive read-only filesystem closure not proven for this run" + ); + } else if (input.preexistingSocketsUnderReadOnlyBinds.length > 0) { + limitations.push(buildPreexistingSocketLimitation(input.preexistingSocketsUnderReadOnlyBinds)); + } else if (input.preexistingSocketScanIncomplete) { + // P1-2 (external review of ced8300be): a scan that could not fully + // enumerate a subtree is NOT the same fact as "scanned, found nothing" + // — it must withhold the strong claim on its own rung, checked AFTER + // the non-empty-sockets case above (a run that both found a socket AND + // hit an unreadable subtree reports the found-socket limitation, the + // more specific and more actionable of the two) but still strictly + // gating `recorded_replay`, same severity as every other condition on + // this chain. + limitations.push(buildSocketScanIncompleteLimitation(input.preexistingSocketScanUnreadablePaths)); } if (input.observedUnsupportedEvidenceSurface) { limitations.push("connector exercised an evidence surface the oracle cannot observe (ASSISTANCE)"); diff --git a/packages/polyfill-connectors/src/scenario/isolation-mechanism.test.ts b/packages/polyfill-connectors/src/scenario/isolation-mechanism.test.ts index 2e64052fa..1c63daf09 100644 --- a/packages/polyfill-connectors/src/scenario/isolation-mechanism.test.ts +++ b/packages/polyfill-connectors/src/scenario/isolation-mechanism.test.ts @@ -10,18 +10,33 @@ // under it has no outbound network. import assert from "node:assert/strict"; import { spawn, spawnSync } from "node:child_process"; -import { cpSync, existsSync, mkdtempSync, readFileSync, readlinkSync, rmSync, writeFileSync } from "node:fs"; +import { + chmodSync, + cpSync, + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + readlinkSync, + rmSync, + symlinkSync, + writeFileSync, +} from "node:fs"; import { createServer } from "node:http"; +import { createConnection } from "node:net"; import { homedir, tmpdir } from "node:os"; import { join, resolve as resolvePath } from "node:path"; import test from "node:test"; import { fileURLToPath } from "node:url"; import { bwrapArgvForFilesystemClosure, + findPreexistingSocketsUnderReadOnlyBinds, isNamespaceIsolationAvailable, postPivotVerificationStatements, requiredFilesystemBinds, + resolveTrustedLauncherPath, spawnWithNetworkIsolation, + submountEnumeratorFunctionDefinition, } from "./isolation.ts"; const bwrapUsable = @@ -33,6 +48,36 @@ const unshareUsable = process.platform === "linux" && spawnSync("unshare", ["-r", "-n", "-m", "true"], { stdio: "ignore", timeout: 5000 }).status === 0; +/** True when this test process can actually shadow a real trusted-path + * binary via a host-level bind mount (root or an equivalent capability) — + * the injection mechanism the test below needs now that the prelude + * resolves its setup commands through a fixed `TRUSTED_SETUP_PATH` + * (P1-1, ninth review) rather than the caller's inherited `$PATH`. Probed + * by attempting the exact bind-then-unbind sequence the real test performs, + * against a scratch file, so the skip condition matches the test's actual + * requirement rather than a proxy for it (e.g. `process.getuid() === 0` + * would be wrong inside a rootless-but-capable container). */ +function canBindMountOverAFile(): boolean { + if (process.platform !== "linux") { + return false; + } + const probeSource = mkdtempSync(join(tmpdir(), "pdpp-bindmount-probe-src-")); + const probeTarget = mkdtempSync(join(tmpdir(), "pdpp-bindmount-probe-dst-")); + const srcFile = join(probeSource, "a"); + const dstFile = join(probeTarget, "b"); + writeFileSync(srcFile, ""); + writeFileSync(dstFile, ""); + const bound = spawnSync("mount", ["--bind", srcFile, dstFile], { stdio: "ignore" }).status === 0; + if (bound) { + spawnSync("umount", [dstFile], { stdio: "ignore" }); + } + rmSync(probeSource, { recursive: true, force: true }); + rmSync(probeTarget, { recursive: true, force: true }); + return bound; +} + +const bindMountCapable = unshareUsable && canBindMountOverAFile(); + test("a host that denies `unshare` but ships a working bwrap still reports isolation AVAILABLE", { skip: !bwrapUsable, }, () => { @@ -84,56 +129,61 @@ test("an isolated child has NO outbound network — the property, not the mechan // use) rather than trusted by reading the source, so a future edit that // reverts to a bare/simplified probe argv is caught mechanically. +// INJECTION MECHANISM (P1, external review of ab415be6c — trusted launcher +// resolution): this test used to shadow `bwrap` via a PATH-prepended shim +// directory. Now that the probe resolves the launcher through +// `resolveTrustedLauncherPath` (a fixed allowlist of trusted directories, +// never the caller's inherited `$PATH`), a PATH-prepended shim is no longer +// selected — proving the fix works, but also meaning a test that still +// relied on PATH-shadowing to intercept the launcher would silently stop +// exercising anything. The injection is done the only way that still +// reaches a trusted-path binary: bind-mounting the logging shim DIRECTLY +// OVER the real trusted-path `bwrap` binary for the duration of the test, +// via `withShimmedTrustedBinary` (defined below — a hoisted function +// declaration, callable here despite the later textual position). test("[bwrap] the fixed probeBwrap() invokes bwrap with the SAME production argv shape bwrapArgvForFilesystemClosure builds — not a bare --dev-bind / / check", { - skip: !bwrapUsable, -}, () => { + skip: !(bwrapUsable && bindMountCapable), +}, async () => { const logDir = mkdtempSync(join(tmpdir(), "pdpp-isolation-probe-argv-log-")); const logPath = join(logDir, "invocations.log"); writeFileSync(logPath, ""); - // A real, delegating shim (not a bare "exit 0") — the probe's own - // production-equivalence is only meaningful if the shim still actually - // RUNS bwrap for real (so a genuinely broken derived-bind shape would - // still be caught), it just additionally logs the argv it was given. - const realBwrapPath = spawnSync("which", ["bwrap"], { encoding: "utf8" }).stdout.trim(); - const shimDir = mkdtempSync(join(tmpdir(), "pdpp-isolation-probe-argv-shim-")); - writeFileSync( - join(shimDir, "bwrap"), - ["#!/bin/sh", `echo "$*" >> ${JSON.stringify(logPath)}`, `exec ${realBwrapPath} "$@"`].join("\n"), - { mode: 0o755 } - ); - const realPath = process.env.PATH; - process.env.PATH = `${shimDir}:${realPath ?? ""}`; try { - const cap = isNamespaceIsolationAvailable(); - // Only meaningful when bwrap is genuinely what got selected (on a host - // where unshare is denied but bwrap works — this suite's own - // AppArmor-restricted dev sandbox is exactly that shape); skip the argv - // assertion (but still ran the probe) otherwise. - const invocations = readFileSync(logPath, "utf8").trim().split("\n").filter(Boolean); - if (cap.available && cap.mechanism === "bwrap") { - assert.equal( - invocations.length, - 1, - `expected exactly one bwrap probe invocation; got ${JSON.stringify(invocations)}` - ); - const probeArgv = invocations[0] ?? ""; - assert.ok( - probeArgv.includes("--tmpfs") && !probeArgv.includes("--dev-bind"), - `probe argv must use the empty --tmpfs / root, never --dev-bind / /; got ${JSON.stringify(probeArgv)}` - ); - assert.ok( - probeArgv.includes("--unshare-pid"), - `probe argv must include --unshare-pid, matching production's derived closure; got ${JSON.stringify(probeArgv)}` - ); - assert.ok( - probeArgv.includes("--ro-bind") || probeArgv.includes("--bind"), - `probe argv must include the derived requiredFilesystemBinds() entries, not just namespace flags; got ${JSON.stringify(probeArgv)}` - ); - } + await withShimmedTrustedBinary( + "bwrap", + (realBwrapPath) => + ["#!/bin/sh", `echo "$*" >> ${JSON.stringify(logPath)}`, `exec ${realBwrapPath} "$@"`].join("\n"), + () => { + const cap = isNamespaceIsolationAvailable(); + // Only meaningful when bwrap is genuinely what got selected (on a + // host where unshare is denied but bwrap works — this suite's own + // AppArmor-restricted dev sandbox is exactly that shape); skip the + // argv assertion (but still ran the probe) otherwise. + const invocations = readFileSync(logPath, "utf8").trim().split("\n").filter(Boolean); + if (cap.available && cap.mechanism === "bwrap") { + assert.equal( + invocations.length, + 1, + `expected exactly one bwrap probe invocation; got ${JSON.stringify(invocations)}` + ); + const probeArgv = invocations[0] ?? ""; + assert.ok( + probeArgv.includes("--tmpfs") && !probeArgv.includes("--dev-bind"), + `probe argv must use the empty --tmpfs / root, never --dev-bind / /; got ${JSON.stringify(probeArgv)}` + ); + assert.ok( + probeArgv.includes("--unshare-pid"), + `probe argv must include --unshare-pid, matching production's derived closure; got ${JSON.stringify(probeArgv)}` + ); + assert.ok( + probeArgv.includes("--ro-bind") || probeArgv.includes("--bind"), + `probe argv must include the derived requiredFilesystemBinds() entries, not just namespace flags; got ${JSON.stringify(probeArgv)}` + ); + } + return Promise.resolve(); + } + ); } finally { - process.env.PATH = realPath; rmSync(logDir, { recursive: true, force: true }); - rmSync(shimDir, { recursive: true, force: true }); } }); @@ -157,142 +207,101 @@ test("[bwrap] the fixed probeBwrap() invokes bwrap with the SAME production argv // logic-level proof of the fix, complementary to (not a replacement for) the // live container reproduction recorded in the review notes. -/** Writes a fake `unshare` that mimics the exact advertise-vs-honor failure - * shape: any invocation whose argv contains `mount -t proc proc /proc` (the - * probe's own mount-and-verify dry run, or the real prelude's) exits 32 with - * a stderr line matching the real kernel refusal, `mount: /proc: permission +// INJECTION MECHANISM (P1, external review of ab415be6c — trusted launcher +// resolution): these two tests used to shadow `unshare`/`bwrap` via a +// PATH-prepended fake-bin directory. Now that both the probe and the real +// execution resolve the launcher through `resolveTrustedLauncherPath` (a +// fixed allowlist, never the caller's `$PATH`), that injection no longer +// reaches anything — proving the fix, but also meaning a test still relying +// on it would silently stop exercising the fallback logic. Both fakes are +// now installed via bind-mounting DIRECTLY OVER the real trusted-path +// `unshare`/`bwrap` binaries (`withShimmedTrustedBinary`, same technique the +// setup-command forced-failure tests below already use), nested so both +// binaries are shimmed for the duration of each test. + +/** Shim body mimicking the exact advertise-vs-honor failure shape: any + * invocation whose argv contains `mount -t proc proc /proc` (the probe's + * own mount-and-verify dry run, or the real prelude's) exits 32 with a + * stderr line matching the real kernel refusal, `mount: /proc: permission * denied.` — every other invocation shape (a bare capability probe like * `-r -n true`) exits 0, so namespace CREATION still looks available; only * the procfs mount specifically is refused, mirroring the CAP_SYS_ADMIN - * -only container shape exactly. */ -function fakeUnshareRefusingProcMount(dir: string): void { - const scriptPath = join(dir, "unshare"); - writeFileSync( - scriptPath, - [ - "#!/bin/sh", - 'case "$*" in', - ' *"mount -t proc proc /proc"*)', - ' echo "mount: /proc: permission denied." 1>&2', - " exit 32", - " ;;", - " *)", - " exit 0", - " ;;", - "esac", - ].join("\n"), - { mode: 0o755 } - ); + * -only container shape exactly. Ignores `realUnsharePath` (unlike the + * setup-command shims elsewhere in this file, this fake never delegates — + * the whole point is to mimic the OLD probe's blind spot, not to actually + * run real `unshare`). */ +function unshareShimRefusingProcMount(_realUnsharePath: string): string { + return [ + "#!/bin/sh", + 'case "$*" in', + ' *"mount -t proc proc /proc"*)', + ' echo "mount: /proc: permission denied." 1>&2', + " exit 32", + " ;;", + " *)", + " exit 0", + " ;;", + "esac", + ].join("\n"); } -/** Writes a fake `bwrap` that always succeeds — used to prove the fallback - * path is taken (not just that `unshare` was correctly rejected). */ -function fakeBwrapAlwaysAvailable(dir: string): void { - const scriptPath = join(dir, "bwrap"); - writeFileSync(scriptPath, "#!/bin/sh\nexit 0\n", { mode: 0o755 }); +/** Shim body that always succeeds — used to prove the fallback path is + * taken (not just that `unshare` was correctly rejected). */ +function bwrapShimAlwaysAvailable(_realBwrapPath: string): string { + return "#!/bin/sh\nexit 0\n"; } -/** Writes a fake `bwrap` that always fails — used so the "no fallback - * available" test is not accidentally rescued by a REAL, working bwrap - * elsewhere on this host's PATH; PATH-shadowing alone isn't enough because - * omitting a bwrap entry from the fake dir doesn't hide a real one located - * later in the (still-inherited) PATH string. */ -function fakeBwrapAlwaysUnavailable(dir: string): void { - const scriptPath = join(dir, "bwrap"); - writeFileSync( - scriptPath, - '#!/bin/sh\necho "bwrap: Creating new namespace failed: Operation not permitted" 1>&2\nexit 1\n', - { - mode: 0o755, - } - ); +/** Shim body that always fails — used so the "no fallback available" test + * is not accidentally rescued by a real, working bwrap. */ +function bwrapShimAlwaysUnavailable(_realBwrapPath: string): string { + return '#!/bin/sh\necho "bwrap: Creating new namespace failed: Operation not permitted" 1>&2\nexit 1\n'; } -test("probe reports UNAVAILABLE (not available-then-crash) when unshare's PID-ns procfs mount is refused, with no working bwrap fallback", () => { - const fakeBinDir = mkdtempSync(join(tmpdir(), "pdpp-isolation-fake-refusal-")); - fakeUnshareRefusingProcMount(fakeBinDir); - // A fake bwrap that ALSO fails — not just an absent one — because the fake - // dir is only PREPENDED to PATH; a real, working bwrap later in the - // inherited PATH would otherwise rescue this "no fallback" scenario and - // make the test assert something false about this host. - fakeBwrapAlwaysUnavailable(fakeBinDir); - const realPath = process.env.PATH; - process.env.PATH = `${fakeBinDir}:${realPath ?? ""}`; - try { - const cap = isNamespaceIsolationAvailable(); - assert.equal( - cap.available, - false, - "a host where namespace creation succeeds but the PID-namespace procfs mount is refused must report UNAVAILABLE, not available-then-crash-later" - ); - if (!cap.available) { - assert.ok( - /permission denied|mount/i.test(cap.reason), - `the diagnostic must name the kernel-level mount refusal, not just 'unavailable'; got ${JSON.stringify(cap.reason)}` +test("probe reports UNAVAILABLE (not available-then-crash) when unshare's PID-ns procfs mount is refused, with no working bwrap fallback", { + skip: !bindMountCapable, +}, async () => { + await withShimmedTrustedBinary("unshare", unshareShimRefusingProcMount, () => + withShimmedTrustedBinary("bwrap", bwrapShimAlwaysUnavailable, () => { + const cap = isNamespaceIsolationAvailable(); + assert.equal( + cap.available, + false, + "a host where namespace creation succeeds but the PID-namespace procfs mount is refused must report UNAVAILABLE, not available-then-crash-later" ); - } - } finally { - process.env.PATH = realPath; - rmSync(fakeBinDir, { recursive: true, force: true }); - } + if (!cap.available) { + assert.ok( + /permission denied|mount/i.test(cap.reason), + `the diagnostic must name the kernel-level mount refusal, not just 'unavailable'; got ${JSON.stringify(cap.reason)}` + ); + } + return Promise.resolve(); + }) + ); }); -test("probe falls back to bwrap when unshare's procfs mount is refused but bwrap genuinely works", () => { - const fakeBinDir = mkdtempSync(join(tmpdir(), "pdpp-isolation-fake-refusal-fallback-")); - fakeUnshareRefusingProcMount(fakeBinDir); - fakeBwrapAlwaysAvailable(fakeBinDir); - const realPath = process.env.PATH; - process.env.PATH = `${fakeBinDir}:${realPath ?? ""}`; - try { - const cap = isNamespaceIsolationAvailable(); - assert.equal( - cap.available, - true, - "a host where unshare's procfs mount is refused but bwrap genuinely works must still report AVAILABLE — the fallback exists precisely for this shape" - ); - if (cap.available) { +test("probe falls back to bwrap when unshare's procfs mount is refused but bwrap genuinely works", { + skip: !bindMountCapable, +}, async () => { + await withShimmedTrustedBinary("unshare", unshareShimRefusingProcMount, () => + withShimmedTrustedBinary("bwrap", bwrapShimAlwaysAvailable, () => { + const cap = isNamespaceIsolationAvailable(); assert.equal( - cap.mechanism, - "bwrap", - "must select bwrap, not unshare — unshare demonstrably cannot honor the isolation it would advertise on this (simulated) host" + cap.available, + true, + "a host where unshare's procfs mount is refused but bwrap genuinely works must still report AVAILABLE — the fallback exists precisely for this shape" ); - } - } finally { - process.env.PATH = realPath; - rmSync(fakeBinDir, { recursive: true, force: true }); - } + if (cap.available) { + assert.equal( + cap.mechanism, + "bwrap", + "must select bwrap, not unshare — unshare demonstrably cannot honor the isolation it would advertise on this (simulated) host" + ); + } + return Promise.resolve(); + }) + ); }); -/** True when this test process can actually shadow a real trusted-path - * binary via a host-level bind mount (root or an equivalent capability) — - * the injection mechanism the test below needs now that the prelude - * resolves its setup commands through a fixed `TRUSTED_SETUP_PATH` - * (P1-1, ninth review) rather than the caller's inherited `$PATH`. Probed - * by attempting the exact bind-then-unbind sequence the real test performs, - * against a scratch file, so the skip condition matches the test's actual - * requirement rather than a proxy for it (e.g. `process.getuid() === 0` - * would be wrong inside a rootless-but-capable container). */ -function canBindMountOverAFile(): boolean { - if (process.platform !== "linux") { - return false; - } - const probeSource = mkdtempSync(join(tmpdir(), "pdpp-bindmount-probe-src-")); - const probeTarget = mkdtempSync(join(tmpdir(), "pdpp-bindmount-probe-dst-")); - const srcFile = join(probeSource, "a"); - const dstFile = join(probeTarget, "b"); - writeFileSync(srcFile, ""); - writeFileSync(dstFile, ""); - const bound = spawnSync("mount", ["--bind", srcFile, dstFile], { stdio: "ignore" }).status === 0; - if (bound) { - spawnSync("umount", [dstFile], { stdio: "ignore" }); - } - rmSync(probeSource, { recursive: true, force: true }); - rmSync(probeTarget, { recursive: true, force: true }); - return bound; -} - -const bindMountCapable = unshareUsable && canBindMountOverAFile(); - test("a forced PID-ns procfs-mount refusal inside the real unshare-mechanism prelude fails the spawn closed, never silently proceeds", { skip: !bindMountCapable, }, async () => { @@ -374,6 +383,330 @@ test("a forced PID-ns procfs-mount refusal inside the real unshare-mechanism pre } }); +// ─── Trusted launcher resolution (P1, external review of ab415be6c) ─────── +// +// The review's exact finding: `probeUnshare()`/`probeBwrap()` and +// `spawnWithNetworkIsolation`'s real execution both spawned the launcher via +// a BARE command name (`spawnSync("unshare", ...)`, `spawn("bwrap", ...)`), +// which `node:child_process` resolves through the CALLING process's own +// inherited `$PATH` — so a PATH-prepended fake `unshare`/`bwrap` earlier in +// `$PATH` than the real, trusted one gets selected instead. These tests +// prove the fix: (1) at the unit level, `resolveTrustedLauncherPath` finds +// the REAL binary regardless of what `$PATH` says, even with a fake +// prepended; (2) at the end-to-end level, a PATH-prepended fake `unshare` +// is never invoked by either the probe or a real isolated spawn. + +test("resolveTrustedLauncherPath: resolves the real trusted-location binary, ignoring a fake earlier in $PATH", { + skip: process.platform !== "linux", +}, () => { + const fakeBinDir = mkdtempSync(join(tmpdir(), "pdpp-isolation-fake-launcher-path-")); + const fakeMarkerPath = join(fakeBinDir, "fake-unshare-ran"); + writeFileSync( + join(fakeBinDir, "unshare"), + ["#!/bin/sh", `touch ${JSON.stringify(fakeMarkerPath)}`, "exit 0"].join("\n"), + { mode: 0o755 } + ); + const realPath = process.env.PATH; + process.env.PATH = `${fakeBinDir}:${realPath ?? ""}`; + try { + const resolved = resolveTrustedLauncherPath("unshare"); + assert.ok( + !resolved.startsWith(fakeBinDir), + `resolveTrustedLauncherPath must never return the PATH-prepended fake; got ${JSON.stringify(resolved)}` + ); + assert.ok( + ["/usr/sbin/unshare", "/usr/bin/unshare", "/sbin/unshare", "/bin/unshare"].includes(resolved), + `expected a real trusted-directory path; got ${JSON.stringify(resolved)}` + ); + // Actually running the resolved binary must not be the fake — the fake + // would touch its own marker file the instant it started. + spawnSync(resolved, ["-r", "-n", "true"], { stdio: "ignore" }); + assert.ok( + !existsSync(fakeMarkerPath), + "the fake unshare's marker file must NOT exist — resolveTrustedLauncherPath's return value must never invoke the fake" + ); + } finally { + process.env.PATH = realPath; + rmSync(fakeBinDir, { recursive: true, force: true }); + } +}); + +test("a PATH-prepended fake `unshare` is never selected by the probe or by a real isolated spawn", { + skip: !unshareUsable, +}, async () => { + const fakeBinDir = mkdtempSync(join(tmpdir(), "pdpp-isolation-fake-launcher-e2e-")); + const fakeMarkerPath = join(fakeBinDir, "fake-unshare-ran"); + // The fake always "succeeds" instantly (exit 0, no real namespace, no real + // isolation) — if it were ever selected, both the probe and a real spawn + // would misreport success while providing NO isolation at all. Also + // touches its own marker so this test can prove, directly, that the fake + // was never invoked (not just that isolation happened to still work). + writeFileSync( + join(fakeBinDir, "unshare"), + ["#!/bin/sh", `touch ${JSON.stringify(fakeMarkerPath)}`, "exit 0"].join("\n"), + { mode: 0o755 } + ); + const realPath = process.env.PATH; + process.env.PATH = `${fakeBinDir}:${realPath ?? ""}`; + try { + const cap = isNamespaceIsolationAvailable(); + assert.ok( + !existsSync(fakeMarkerPath), + "the fake unshare's marker must NOT exist after the capability probe — the probe must resolve the real trusted-path binary, never the PATH-prepended fake" + ); + if (cap.available && cap.mechanism === "unshare") { + // Only meaningful when the probe genuinely selected unshare (true on + // this suite's own host, where unshare works natively) — prove a real + // spawn under `isolate: true` (which re-derives the mechanism itself, + // exercising the SAME resolution path as production) also never + // touches the fake, and that the child is genuinely isolated (no + // outbound network) rather than the fake's instant, unisolated exit 0. + const exitCode = await new Promise((resolveExit) => { + const child = spawnWithNetworkIsolation( + process.execPath, + [ + "-e", + 'require("http").get("http://1.1.1.1",()=>process.exit(9)).on("error",()=>process.exit(0));setTimeout(()=>process.exit(0),4000)', + ], + { isolate: true, stdio: "ignore" } + ); + child.on("close", resolveExit); + }); + assert.ok( + !existsSync(fakeMarkerPath), + "the fake unshare's marker must NOT exist after a real isolated spawn — spawnWithNetworkIsolation must resolve the real trusted-path binary, never the PATH-prepended fake" + ); + assert.equal( + exitCode, + 0, + "the spawn must still be genuinely network-isolated (exit 9 would mean the fake ran instead and no real isolation happened)" + ); + } + } finally { + process.env.PATH = realPath; + rmSync(fakeBinDir, { recursive: true, force: true }); + } +}); + +// ─── Trusted shell resolution (P1-1, external review of ced8300be) ──────── +// +// The review's exact finding: the trusted-launcher fix above closes how +// `unshare`/`bwrap` THEMSELVES are resolved, but never touched the `sh` those +// launchers exec their closure script into — both `unshareProcMountProbeArgv()` +// (the probe) and `spawnWithNetworkIsolation`'s `unshare` branch (the real +// execution) passed the bare string `"sh"` as an argv entry to the +// already-trusted `unshare` binary (`unshare ... -- sh -c