Repository navigation
Add dependency cooldown and remediate vulnerabilities - #200
Conversation
ritz078
left a comment
There was a problem hiding this comment.
Three cross-cutting items with no single line to anchor to.
npm audit fix no longer works in these examples. A/B on web/ui-customization, npm 12.0.2, no lockfile, identical inputs except the cooldown:
| exit | result | |
|---|---|---|
with min-release-age=15 |
1 | 2 vulnerabilities (1 moderate, 1 high), prints fix available via 'npm audit fix' |
| control | 0 | found 0 vulnerabilities |
The control resolves to exactly the versions this PR hand-pins, so the cooldown blocks the remediation from being reachable by the repo's own tooling, and the retry hint loops with no exit. The surviving vulns are this PR's own targets (brace-expansion high, postcss moderate). Scoping note: npm ci from the committed lockfiles is unaffected across all 25 npm examples — npm never validates a lockfile against the age window, only pnpm 11 does, which is why the breakage is confined to the three pnpm examples.
.tool-versions and CI disagree about whether the cooldown exists. nodejs 22.12.0 → npm 10.9.0, which predates the feature; CI's lts/* → Node 24.19.0 → npm 11.17.0, which enforces it. A contributor following the repo's own pin regenerates lockfiles with no cooldown applied and can't reproduce any of this. Bumping .tool-versions and pinning the workflow to the same version would make the policy enforce identically in both places.
The committed lockfiles were generated with the cooldown not in effect — they introduce entries 2 days old (nanoid@3.3.17, next@16.3.0 + 9 @next/*, fast-uri@3.1.5, @emnapi/runtime@1.11.3). Harmless for npm ci, but it means the artifacts can't be reproduced by anyone honoring the policy this same commit adds.
Also — could you spell out what "preserve the protected cache-related dependency versions" refers to? I couldn't find a policy file or anything in AGENTS.md, and no cache-adjacent package (flatted, flat-cache, lru-cache, caniuse-lite) changed version in any of the 26 lockfiles, so there's nothing for a reviewer to check against.
- Scope min-release-age-exclude / minimumReleaseAgeExclude to the pinned security-remediation versions so fresh resolution, npm audit fix, and pnpm's lockfile supply-chain policy all pass under the 15-day cooldown; document the npm version requirements (11.10.0+ for min-release-age, 11.17.0+ for the exclusions) and the intentional npm/pnpm exclusion asymmetry, with a dated TODO to drop the exclusions after 2026-08-18. - Migrate the three dual npm/pnpm examples to pnpm 11 config: move package.json#pnpm.overrides to pnpm-workspace.yaml, mirror the full map into package.json#overrides for npm, bound the cross-major sharp/postcss ranges, replace the exact next override with a direct ^16.3.0 dependency, and regenerate the lockfiles under the pinned managers. - Pin the enforcing toolchain: Node 24.19.0 in .tool-versions, the typecheck workflow reading it via node-version-file, and packageManager pnpm@11.20.0 in the dual-manager examples. - Add a Dependency policy workflow that keeps the duplicated npm/pnpm override maps identical and verifies the committed pnpm lockfiles against the supply-chain policy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> 🔮 View transcript: https://nutrient-agentlogs.dev/s/dkd0hknzkpeagh2nhsbz1p1m
|
Replying to the review-level items from #200 (review) — all addressed in f0883d5:
|
- check-override-sync.mjs discovers dual-manager examples by globbing for directories committing both lockfiles instead of a hardcoded list, and fails when a discovered example lacks a pnpm-workspace.yaml to mirror overrides into. - Rename the pnpm CI step to say what it verifies (lockfile vs manifest and overrides) and document that a frozen install does not re-check minimumReleaseAge. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> 🔮 View transcript: https://nutrient-agentlogs.dev/s/crp7fpnzsx3aup8to3tobdfs
Today's npm audit flags js-yaml <=4.3.0 (GHSA-5p4m-2wfm-xmqj, quadratic CPU in !!omap resolution) in 10 examples. The fix, 4.3.1, was published 2026-07-31 and is still inside the 15-day cooldown, so it gets the same exclusion treatment as the existing remediations; the exclusions become removable on 2026-08-15. - Add js-yaml to min-release-age-exclude / minimumReleaseAgeExclude in the affected examples. - Widen the dual-manager override rule to js-yaml@>=4.0.0 <4.3.1 => ">=4.3.1 <5" (bounded to 4.x so eslint's ^4.x dependency does not jump to js-yaml 5), subsuming the earlier <=4.1.1 rule. - Regenerate the affected npm and pnpm lockfiles; all 25 examples audit clean again. - Bump eslint-config-next to ^16.2.11 (mature, published 2026-07-21) in signing-demo-complete, removing the last exact pin and closing most of the skew with next 16.3.0. - Extend the root .npmrc TODO with the js-yaml exit date and the override consolidation cleanup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ritz078
left a comment
There was a problem hiding this comment.
Three things that have no single line to hang off:
- The audit claim in the description is stale. Re-running
npm audittoday against this branch reports advisories in 17 of the 25 examples, not a clean sweep as of 2026-08-07. Worth re-running and refreshing the remediation set before merge, since the overrides were generated from the older audit output. - Nothing enforces that an example has an
.npmrc. The policy is only as good as its coverage, but the new workflow'spaths:filter fires on**/.npmrc— i.e. only when one is touched. A new example added without one silently opts out of the cooldown forever. A CI assertion that every directory with a lockfile also hasmin-release-ageset would close that. - The npm side has no lockfile verification job, and can't easily get one.
pnpm-lockfile-policycovers pnpm; there is no npm equivalent, andnpm ciis not a substitute — npm appliesmin-release-ageonly at resolution time, sonpm ci,npm install, andnpm install --package-lock-onlyall install a cooldown-bypassingpackage-lock.jsonwithout complaint. That asymmetry is worth stating somewhere, because today the pnpm job reads as if both managers are covered.
…t remediations The dated TODO in the root .npmrc came due (youngest pinned version left the 15-day window on 2026-08-22), so the exclusions are gone rather than extended: - Remove every min-release-age-exclude / minimumReleaseAgeExclude entry; standardize the .npmrc comment (min-release-age landed in npm 11.10.1). - Collapse the stacked audit-generated override rules to one rule per package per major line; brace-expansion exact pins relaxed to ranged floors; dompurify floor raised to >=3.4.13 (GHSA-55q2-fjhq-7xh7); nanoid floor >=3.3.18 added (GHSA-2v37-7h3g-55p8); linkify-it floor raised to >=5.0.2 (GHSA-v245-v573-v5vm). - Regenerate all lockfiles with the cooldown active and no exclusions; npm audit and pnpm audit are clean across all 27 examples. - Cover the examples merged from master: web/viewer/web-sdk-demo (9 advisories: vite pin bumped to 6.4.3, overrides added) and gdpicture/office-templating (.npmrc added). - New check-cooldown-coverage.mjs fails CI when a directory commits a lockfile without opting into the cooldown. - dependency-policy.yml: least-privilege permissions block; the pnpm-lockfile-policy matrix is now generated from check-override-sync --json instead of a hand-maintained list; corrected the frozen-install comment (pnpm 11.20 does re-check minimumReleaseAge) and documented the npm-side asymmetry. - check-override-sync.mjs: fs.globSync takes exclude, not ignore. - Root package.json gains engines.npm >=11.10.1 with engine-strict so a too-old npm fails loudly instead of skipping the cooldown. - signing-demo-complete README: carry the pnpm path through steps 4-5. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> 🔮 View transcript: https://nutrient-agentlogs.dev/s/f986wn16h6zflcujoiwbcaye
|
Re the three cross-cutting items in the 2026-08-10 review, all addressed in d5d3e74:
|
- Apply the fail-loudly npm guard per example, not just at the repo root: every lockfile-bearing directory now sets engine-strict=true in .npmrc and engines.npm >=11.10.1 in package.json (npm reads project config from the nearest package.json, so the root-only guard covered nothing). check-cooldown-coverage.mjs enforces both. - Guard the nanoid 5.x line (GHSA-2v37-7h3g-55p8) in the three dual-manager examples, matching web-sdk-demo's existing rule. - check-cooldown-coverage.mjs: validate the cooldown value against the 15-day policy floor instead of accepting any integer (pnpm's minimumReleaseAge is minutes: floor 21600); fail on zero glob matches like check-override-sync.mjs does; reject unsupported lockfile types (yarn.lock, bun.lock, bun.lockb, npm-shrinkwrap.json), which the workflow paths filter now also watches; tolerate ini whitespace in min-release-age. - dependency-policy.yml: split the discover step so the node exit code fails the step instead of being swallowed by echo. - Bound every override floor to its major line (ajv <7, lodash <5, minimatch <4/<10, picomatch <3/<5, flatted <4, markdown-it <15, linkify-it <6, @babel/core <8, esbuild/rollup capped) so no floor can force a cross-major jump on regeneration. - Regenerate polygon-clipping-outline and document-generator-vanillajs lockfiles under the active cooldown; their deleted exclusion entries were the only remaining record of hand-pinned remediation versions. - Lockfiles for the four override-map examples regenerated; audits clean (27 npm examples, 3 pnpm), frozen-install policy check passes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> 🔮 View transcript: https://nutrient-agentlogs.dev/s/f986wn16h6zflcujoiwbcaye
ritz078
left a comment
There was a problem hiding this comment.
Reviewed the policy tooling by running it rather than reading it — both scripts against a worktree of this SHA, plus npm 11.17.0/12.0.2 extracted from the registry to exercise the cooldown for real. The mechanism holds up well: engine-strict genuinely enforces the root engines.npm, the cooldown is really applied (min-release-age=40 → no matching version … before 7/24/2026), and I confirmed all 1,971 distinct name@version pairs across all 30 lockfiles predate the 15-day cutoff — so the committed lockfiles are reproducible under the cooldown they enforce, with zero exclusions, and nothing was lost collapsing the stacked override rules. npm audit fix also works with the cooldown fully active now, which closes my earlier objection.
Three things with no single line to anchor to:
The audit sweep needs re-running before merge. npm audit --package-lock-only across all 27 examples today reports 16 of them (plus all 3 pnpm-lock.yaml) carrying two high-severity browserslist advisories — GHSA-c83g-rgw3-j3cx and GHSA-73wf-gq98-2v4g, <=4.28.6. Not your oversight: both were published about 65 minutes after your final commit. The fix is unobstructed — 4.28.8 (2026-08-08) is well past the 15-day window, and npm audit fix --package-lock-only under the active cooldown took ui-customization from 4.28.6 to 4.28.8 with found 0 vulnerabilities. Nothing else is unremediated anywhere. Worth dropping the "clean across all 27 examples" line from the description until it's re-run.
The npm floor is a user-facing breaking change that the docs don't mention. Because Node 20 and 22 both ship npm 10.x, npm install now fails in all 27 examples for anyone not on Node 24.14.1+. That's the intended "fail loudly" behaviour, but 29 READMEs still say npm install and the root README mentions neither Node nor npm — so for a clone-and-run examples repo, the failure arrives with no explanation. One line in the root README plus the engines.node change would cover it.
No contributor-facing documentation of the policy. Searching for min-release-age|minimumReleaseAge|cooldown outside lockfiles matches only the three pnpm-workspace.yaml files. AGENTS.md and the root README are untouched, so someone adding an example learns about the .npmrc + engines.npm + pnpm-workspace.yaml requirement by getting a red check. The upstream monorepo points at documentation/dependency-cooldowns.md; a short equivalent here would help.
Review response: - Fail on every cooldown escape hatch: min-release-age-exclude (incl. [] form), minimumReleaseAgeExclude, minimumReleaseAgeStrict other than true, trustLockfile other than false. - Read .npmrc last-wins the way npm does: collect every occurrence, reject duplicate policy keys as ambiguous, evaluate the last value. - Value-check engines.npm as a single >=x.y.z floor of at least 11.10.0 and require engines.node to equal npm 11's supported range (^20.17.0 || >=22.9.0) in the root and all 27 examples. - Report missing/malformed package.json as policy messages instead of crashing the run; aggregate every violation. - Anchor lockfile discovery to the script location so cwd cannot cause partial discovery; run the policy tests in CI; add push:master trigger; scope corepack to pnpm; fix the override-sync termination comment; min-release-age first shipped in npm 11.10.0. - Document the contributor/user contract (README, AGENTS.md, documentation/dependency-cooldowns.md, example READMEs) and remediate the browserslist advisories (GHSA-c83g-rgw3-j3cx, GHSA-73wf-gq98-2v4g). Local re-review hardening: - .npmrc parser keeps key case, treats bare keys as true, and rejects ini [section] headers - npm ignores Min-Release-Age and section-scoped keys, so lowercasing let a no-op config pass. - pnpm-workspace.yaml parser fails closed on quoted keys, anchors, aliases, tags, merge keys, block scalars and flow documents, all of which pnpm 11.20 honours but the line parser did not see. - Main-module guard compares realpaths so a symlinked checkout path no longer turns the CLI into a silent exit 0. - CLI test compares against in-process discovery instead of a literal 27/3 count; symlink and adversarial-syntax tests added. - Root package.json gets a stable name so the lockfile stops inheriting the checkout directory name; READMEs state the exact Node range. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Addressed the three unanchored items from the review in f58dc02:
|
Summary
min-release-agefor npm,minimumReleaseAgefor pnpm) to every independently installable example, and enforce coverage in CI:check-cooldown-coverage.mjsfails when any directory commits a lockfile without opting into the cooldown, so a new example can't silently opt out.tool-versionsand in CI vianode-version-file, pnpm 11.20.0 viapackageManager, plusengines.npm >=11.10.1+engine-strictat the root so a too-old npm fails loudly instead of skipping the cooldownDependency policyworkflow (least-privilegecontents: read) that keeps the duplicated npm/pnpm override maps in sync and verifies the committed pnpm lockfiles against manifests, overrides, and the cooldown itself — pnpm 11.20's frozen install re-checksminimumReleaseAgeper lockfile entry. The dual-manager matrix is generated fromcheck-override-sync.mjs --json, not maintained by hand. npm has no equivalent check (npm cinever re-validates the cooldown); that asymmetry is documented in the workflow.Cooldown exclusions: removed
The first iteration scoped exclusions to remediation versions published inside the window. All of them matured out by 2026-08-22 (youngest: nanoid 3.3.18, published 2026-08-07), so the dated TODO came due: every
min-release-age-exclude/minimumReleaseAgeExcludeentry is gone, the stacked audit-generated override rules are collapsed to one rule per package per major line, and the exactbrace-expansionpins are relaxed back to ranged floors. Fresh resolution andnpm audit fixwork again with the cooldown fully active.Audit state as of 2026-09-01
Full re-sweep on 2026-09-01 found 17 examples with advisories (mostly nanoid GHSA-2v37-7h3g-55p8 and DOMPurify GHSA-55q2-fjhq-7xh7), including two examples merged from master with no policy coverage yet (
web/viewer/web-sdk-demo— 9 advisories,gdpicture/office-templating— no.npmrc). All remediated:>=3.3.18 <4in dual-manager examples,npm audit fixelsewhere)<3.4.13 → >=3.4.13 <4, replacing the exact 3.4.12 pin)@babel/core,markdown-it,postcssin web-sdk-demonpm auditis clean across all 27 examples andpnpm auditis clean in the three dual-manager examples; the pnpm frozen-install policy check passes on all three.If this PR sits unmerged for a few days, re-run
npm auditacross the examples before merging — new advisories may have appeared, and the resolved versions here go stale.🤖 Generated with Claude Code