From 8098495abf705eeb6417534f8ad6fb313924cba7 Mon Sep 17 00:00:00 2001 From: Tony James Date: Thu, 27 Aug 2026 12:51:53 +0100 Subject: [PATCH] Pin dependency policy exceptions to versions --- README.md | 27 ++++++++++++++++++--------- SECURITY.md | 4 ++-- lib/check.js | 10 +++++++--- lib/constants.js | 5 +++-- lib/policy.js | 42 +++++++++++++++++++++++++++++++++++++----- lib/project.js | 8 +++++--- lib/verify.js | 4 +++- test/check.test.js | 20 ++++++++++++++++++++ test/project.test.js | 12 ++++++++++-- test/verify.test.js | 20 ++++++++++++++++++++ 10 files changed, 125 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index c079329..331c538 100644 --- a/README.md +++ b/README.md @@ -95,8 +95,15 @@ runner without placing them in the policy or report. ```json { - "allowInstallScripts": ["fsevents"], - "allowDirectLifecycleScripts": [], + "allowInstallScripts": { + "fsevents": { + "versions": ["2.3.3"], + "reason": "Reviewed optional native filesystem watcher install script.", + "expiresAt": "2027-01-01T00:00:00Z" + } + }, + "allowDirectLifecycleScripts": {}, + "archiveSizeExceptions": {}, "sourceEvidenceExceptions": {}, "trustedRepositories": { "example-package": "https://github.com/example/example-package" @@ -111,10 +118,11 @@ runner without placing them in the policy or report. } ``` -- `allowInstallScripts` permits `hasInstallScript` on any matching lock entry. The - default contains only `fsevents`. -- `allowDirectLifecycleScripts` separately permits direct packages whose registry - manifest declares `preinstall`, `install`, or `postinstall`. +- `allowInstallScripts` permits `hasInstallScript` only for listed exact versions. +- `allowDirectLifecycleScripts` separately permits exact direct package versions + whose registry manifest declares `preinstall`, `install`, or `postinstall`. +- `archiveSizeExceptions` permits exact package versions to use reviewed archive + limits above the project defaults. - `sourceEvidenceExceptions` permits only an exact package version to proceed when source provenance or tag evidence is unavailable. Each entry requires a detailed `reason` and an `expiresAt` ISO date, and produces a warning. @@ -129,7 +137,8 @@ Unknown policy fields and malformed values fail closed. ## Exceptions -Treat allowlists as reviewed security exceptions: +Treat exception maps as reviewed security exceptions. Lifecycle and archive entries +require exact versions, a detailed reason, and an expiry: 1. Confirm the exact package and locked version. 2. Inspect the lifecycle script and every executable or downloaded artifact it uses. @@ -138,10 +147,10 @@ Treat allowlists as reviewed security exceptions: 5. Record the rationale and reviewer beside the policy change in the pull request. 6. Remove the exception when the package or dependency is removed. -Do not add a package to both lifecycle allowlists automatically. A transitive +Do not add a package to both lifecycle exception maps automatically. A transitive `hasInstallScript` exception and a direct manifest-hook exception represent separate trust decisions. Scripts remain disabled during the guard's own `npm ci`; an -allowlist permits review to pass but does not execute the script. +exception permits review to pass but does not execute the script. ## Evidence report diff --git a/SECURITY.md b/SECURITY.md index e54280a..5ec0b4e 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -32,8 +32,8 @@ Node.js 20 and newer are required. - Do not write credentials, authorization headers, environment contents, metadata response bodies, or attestation bundles to reports. - Do not weaken repository, commit, tag, signature, integrity, or lifecycle checks to - accommodate a package. Require an explicit, reviewed policy exception where one is - supported. + accommodate a package. Require an explicit, version-pinned, expiring policy + exception where one is supported. ## Disclosure diff --git a/lib/check.js b/lib/check.js index 623a3ef..56dfe70 100644 --- a/lib/check.js +++ b/lib/check.js @@ -30,7 +30,7 @@ export async function checkProject(options, injected = {}) { try { const project = await readProject(projectPath); - const lockInspection = inspectLock(project.lock, options.policy); + const lockInspection = inspectLock(project.lock, options.policy, dependencies.now()); report.lockfile = { lockfileVersion: project.lock.lockfileVersion, registryEntries: lockInspection.registryEntries }; report.failures.push(...lockInspection.failures.map((message) => `lockfile: ${message}`)); @@ -47,8 +47,12 @@ export async function checkProject(options, injected = {}) { const packed = JSON.parse(pack.result.stdout)[0]; packageResult.tarballSizeBytes = packed.size; packageResult.unpackedSizeBytes = packed.unpackedSize; - if (!Number.isFinite(packed.size) || packed.size > options.policy.maxTarballSizeBytes) packageResult.failures.push(`tarball size ${packed.size ?? "unknown"} exceeds ${options.policy.maxTarballSizeBytes}`); - if (!Number.isFinite(packed.unpackedSize) || packed.unpackedSize > options.policy.maxUnpackedSizeBytes) packageResult.failures.push(`unpacked size ${packed.unpackedSize ?? "unknown"} exceeds ${options.policy.maxUnpackedSizeBytes}`); + const archiveException = options.policy.archiveSizeExceptions[item.name]; + const archiveExceptionValid = archiveException?.version === item.version && Date.parse(archiveException.expiresAt) > dependencies.now(); + const tarballLimit = archiveExceptionValid ? archiveException.maxTarballSizeBytes ?? options.policy.maxTarballSizeBytes : options.policy.maxTarballSizeBytes; + const unpackedLimit = archiveExceptionValid ? archiveException.maxUnpackedSizeBytes ?? options.policy.maxUnpackedSizeBytes : options.policy.maxUnpackedSizeBytes; + if (!Number.isFinite(packed.size) || packed.size > tarballLimit) packageResult.failures.push(`tarball size ${packed.size ?? "unknown"} exceeds ${tarballLimit}`); + if (!Number.isFinite(packed.unpackedSize) || packed.unpackedSize > unpackedLimit) packageResult.failures.push(`unpacked size ${packed.unpackedSize ?? "unknown"} exceeds ${unpackedLimit}`); } catch { packageResult.failures.push("npm pack returned invalid JSON evidence"); } diff --git a/lib/constants.js b/lib/constants.js index 7615c91..8a25d4f 100644 --- a/lib/constants.js +++ b/lib/constants.js @@ -1,6 +1,7 @@ export const DEFAULT_POLICY = Object.freeze({ - allowInstallScripts: ["fsevents"], - allowDirectLifecycleScripts: [], + allowInstallScripts: {}, + allowDirectLifecycleScripts: {}, + archiveSizeExceptions: {}, sourceEvidenceExceptions: {}, trustedRepositories: {}, trustedMaintainers: {}, diff --git a/lib/policy.js b/lib/policy.js index e1dc670..def5fca 100644 --- a/lib/policy.js +++ b/lib/policy.js @@ -19,8 +19,9 @@ export async function loadPolicy(file) { if (!KEYS.has(key)) throw new Error(`unknown policy key: ${key}`); } const policy = { ...structuredClone(DEFAULT_POLICY), ...parsed }; - validateStringArray(policy, "allowInstallScripts"); - validateStringArray(policy, "allowDirectLifecycleScripts"); + validateVersionExceptions(policy.allowInstallScripts, "allowInstallScripts"); + validateVersionExceptions(policy.allowDirectLifecycleScripts, "allowDirectLifecycleScripts"); + validateArchiveExceptions(policy.archiveSizeExceptions); validateSourceExceptions(policy.sourceEvidenceExceptions); validateMap(policy, "trustedRepositories", false); validateMap(policy, "trustedMaintainers", true); @@ -60,9 +61,40 @@ function validateSourceExceptions(value) { } } -function validateStringArray(policy, key) { - if (!Array.isArray(policy[key]) || policy[key].some((value) => typeof value !== "string" || !value)) { - throw new Error(`${key} must be an array of non-empty strings`); +function validateVersionExceptions(value, key) { + if (!value || Array.isArray(value) || typeof value !== "object") throw new Error(`${key} must be an object`); + for (const [name, exception] of Object.entries(value)) { + validateExceptionBase(name, exception, key, ["versions"]); + if (!Array.isArray(exception.versions) || exception.versions.length === 0 || exception.versions.some((version) => typeof version !== "string" || !version)) { + throw new Error(`${key}.${name}.versions must be an array of non-empty strings`); + } + } +} + +function validateArchiveExceptions(value) { + if (!value || Array.isArray(value) || typeof value !== "object") throw new Error("archiveSizeExceptions must be an object"); + for (const [name, exception] of Object.entries(value)) { + validateExceptionBase(name, exception, "archiveSizeExceptions", ["version", "maxTarballSizeBytes", "maxUnpackedSizeBytes"]); + if (typeof exception.version !== "string" || !exception.version) throw new Error(`archiveSizeExceptions.${name}.version must be a non-empty string`); + const limits = [exception.maxTarballSizeBytes, exception.maxUnpackedSizeBytes].filter((limit) => limit !== undefined); + if (limits.length === 0 || limits.some((limit) => !Number.isFinite(limit) || limit < 0)) { + throw new Error(`archiveSizeExceptions.${name} must contain at least one non-negative size limit`); + } + } +} + +function validateExceptionBase(name, exception, key, allowedFields) { + if (!name || !exception || Array.isArray(exception) || typeof exception !== "object") { + throw new Error(`invalid ${key} entry for ${name || ""}`); + } + if (Object.keys(exception).some((field) => ![...allowedFields, "reason", "expiresAt"].includes(field))) { + throw new Error(`unknown ${key} field for ${name}`); + } + if (typeof exception.reason !== "string" || exception.reason.trim().length < 20) { + throw new Error(`${key}.${name}.reason must contain at least 20 characters`); + } + if (typeof exception.expiresAt !== "string" || !Number.isFinite(Date.parse(exception.expiresAt))) { + throw new Error(`${key}.${name}.expiresAt must be an ISO date`); } } diff --git a/lib/project.js b/lib/project.js index 8bf65aa..669ce9f 100644 --- a/lib/project.js +++ b/lib/project.js @@ -20,14 +20,16 @@ export async function readProject(projectPath) { return { manifest, lock, direct }; } -export function inspectLock(lock, policy) { +export function inspectLock(lock, policy, now = Date.now()) { const failures = []; let registryEntries = 0; for (const [lockPath, entry] of Object.entries(lock.packages)) { if (!lockPath || entry.link) continue; const name = packageNameFromLockPath(lockPath, entry) ?? lockPath; - if (entry.hasInstallScript && !policy.allowInstallScripts.includes(name)) { - failures.push(`${name}: lock entry declares hasInstallScript; add a reviewed allowInstallScripts exception if intentional`); + const scriptException = policy.allowInstallScripts[name]; + const scriptAllowed = scriptException?.versions.includes(entry.version) && Date.parse(scriptException.expiresAt) > now; + if (entry.hasInstallScript && !scriptAllowed) { + failures.push(`${name}@${entry.version ?? "unknown"}: lock entry declares hasInstallScript; add a reviewed, version-pinned allowInstallScripts exception if intentional`); } if (!lockPath.includes("node_modules/")) continue; registryEntries += 1; diff --git a/lib/verify.js b/lib/verify.js index 585eb06..b5685ba 100644 --- a/lib/verify.js +++ b/lib/verify.js @@ -36,7 +36,9 @@ export async function verifyDirectPackage(item, document, policy, dependencies) } const lifecycle = ["preinstall", "install", "postinstall"].filter((name) => versionData.scripts?.[name]); - if (lifecycle.length && !policy.allowDirectLifecycleScripts.includes(item.name)) { + const lifecycleException = policy.allowDirectLifecycleScripts[item.name]; + const lifecycleAllowed = lifecycleException?.versions.includes(item.version) && Date.parse(lifecycleException.expiresAt) > dependencies.now(); + if (lifecycle.length && !lifecycleAllowed) { failures.push(`direct package declares lifecycle hook(s): ${lifecycle.join(", ")}`); } diff --git a/test/check.test.js b/test/check.test.js index e103788..42ccfe3 100644 --- a/test/check.test.js +++ b/test/check.test.js @@ -53,6 +53,26 @@ test("check records npm pack size violations without installing", async () => { assert.equal(report.commands.length, 1); }); +test("check applies an archive override only to its reviewed package version", async () => { + const directory = await makeProject(); + const document = packageDocument(); + const gitHead = document.versions["1.2.3"].gitHead; + const policy = { ...structuredClone(DEFAULT_POLICY), maxTarballSizeBytes: 10 }; + policy.archiveSizeExceptions.alpha = { + version: "1.2.3", + maxTarballSizeBytes: 100, + reason: "Reviewed archive is larger than the default project limit.", + expiresAt: "2025-02-01T00:00:00Z", + }; + const report = await checkProject({ path: directory, report: join(directory, "report.json"), policy, skipInstall: true }, { + fetch: async () => jsonResponse(document), + remoteRefs: async () => [{ sha: gitHead, ref: "refs/tags/v1.2.3" }], + run: passingRun(), + now: () => Date.parse("2025-01-01T00:00:00Z"), + }); + assert.equal(report.status, "pass"); +}); + test("check parses full npm pack JSON while truncating report evidence", async () => { const directory = await makeProject(); const reportPath = join(directory, "report.json"); diff --git a/test/project.test.js b/test/project.test.js index ab2d285..1e91698 100644 --- a/test/project.test.js +++ b/test/project.test.js @@ -19,7 +19,7 @@ test("lock inspection rejects unexpected install scripts and missing integrity", assert.match(result.failures[1], /integrity/); }); -test("default lock allowlist permits fsevents install script", () => { +test("version-pinned lock exception permits only the reviewed fsevents version", () => { const lock = { packages: { "": {}, @@ -31,5 +31,13 @@ test("default lock allowlist permits fsevents install script", () => { }, }, }; - assert.deepEqual(inspectLock(lock, structuredClone(DEFAULT_POLICY)).failures, []); + const policy = structuredClone(DEFAULT_POLICY); + policy.allowInstallScripts.fsevents = { + versions: ["2.3.3"], + reason: "Reviewed optional native filesystem watcher install script.", + expiresAt: "2027-01-01T00:00:00Z", + }; + assert.deepEqual(inspectLock(lock, policy, Date.parse("2026-01-01T00:00:00Z")).failures, []); + lock.packages["node_modules/fsevents"].version = "2.3.4"; + assert.match(inspectLock(lock, policy, Date.parse("2026-01-01T00:00:00Z")).failures[0], /2\.3\.4/); }); diff --git a/test/verify.test.js b/test/verify.test.js index 545519c..ebe596b 100644 --- a/test/verify.test.js +++ b/test/verify.test.js @@ -115,6 +115,26 @@ test("rejects direct lifecycle scripts and untrusted maintainers", async () => { assert.ok(result.failures.some((message) => message.includes("registry maintainer set changed"))); }); +test("permits a direct lifecycle script only for the reviewed version", async () => { + const document = packageDocument({ version: { scripts: { postinstall: "node setup.js" } } }); + const policy = structuredClone(DEFAULT_POLICY); + policy.allowDirectLifecycleScripts.alpha = { + versions: ["1.2.3"], + reason: "Reviewed package setup script required by this exact release.", + expiresAt: "2025-02-01T00:00:00Z", + }; + const dependencies = { + remoteRefs: async () => [{ sha: document.versions["1.2.3"].gitHead, ref: "refs/tags/v1.2.3" }], + now, + timeoutMs: 100, + }; + const accepted = await verifyDirectPackage(item, document, policy, dependencies); + assert.ok(!accepted.failures.some((message) => message.includes("lifecycle hook"))); + policy.allowDirectLifecycleScripts.alpha.versions = ["1.2.2"]; + const rejected = await verifyDirectPackage(item, document, policy, dependencies); + assert.ok(rejected.failures.some((message) => message.includes("lifecycle hook"))); +}); + test("rejects a missing previously trusted maintainer", async () => { const document = packageDocument({ version: { maintainers: [{ name: "alice" }] } }); const policy = structuredClone(DEFAULT_POLICY);