Skip to content

Reject out-of-policy npm transitive updates - #16496

Open
myevolve wants to merge 9 commits into
dependabot:mainfrom
myevolve:fix/npm-transitive-version-policy
Open

myevolve wants to merge 9 commits into
dependabot:mainfrom
myevolve:fix/npm-transitive-version-policy

Conversation

@myevolve

@myevolve myevolve commented Oct 7, 2026 •

Copy link
Copy Markdown

Problem

Native npm update accepts package names, not per-package version constraints. Core passed only the name, so a resolver cap/ignore rule could be bypassed by the actual lockfile, and the writer could write a different version from the one requested. Clamping parsed metadata alone does not constrain the file.

Change

  • Validate every changed modern npm lockfile occurrence before reporting a resolver candidate, including nested packages and installed/real alias names. Reject versions above the allowable target or matching an ignored range; preserve unchanged occurrences.
  • Validate the current writer result after native update/audit fallback; raise UpdateNotPossible if npm exceeds the requested version. Keep the previous-state diagnostic probe unchanged so registry failures retain their original classification.
  • Reuse the existing typed JSON lockfile records; no npm overrides, manifest changes, new dependencies, feature flags, or package-manager policy changes.
  • Replace parser-mocked aggregate-version assertions that hid a higher actual lock entry with consumer-visible policy and error-classification regressions.

Initial feature verification

  • 140 examples, 0 failures across native helpers, subdependency resolution, npm lockfile writing, and JSON lockfile parsing.
  • RuboCop: 7 files inspected, no offenses.
  • Complete repository Sorbet check against checked-in RBIs: No errors. The production image srb wrapper injects a mismatched/duplicate gem RBI set; the same installed Sorbet static executable was run directly against repository config instead.
  • Actual Core + npm 11.19.0, isolated loopback registry, unchanged parent ^1.15.9, external networking disabled:
Allowed target Before After
1.16.0; ignore >1.16.0 Resolver and writer select 1.16.1 Resolver rejects; writer raises UpdateNotPossible
1.16.1; ignore >1.16.1 Allowed Resolver and writer agree on 1.16.1

Source manifests remain unchanged. The grouped registry-error regression also failed before moving the guard to the publication path and passes afterward. Independent correctness and failure-path reviews completed; the diagnostic finding was reproduced and fixed.

Boundary

This intentionally rejects an out-of-policy native result; it does not promise to make npm search for an older allowed transitive candidate. Temporary overrides would replace parent requirements rather than safely intersect them. That limitation is documented.

Related: dependabot/smoke-tests#592. Independently based on current main, not stacked on #16490 or #16492; it does not modify those PR branches or existing smoke expectations. Clocked regeneration using dependabot/cli#668 is being exercised separately with original ignore conditions preserved.

Current integration verification

Head: de4a7cb8bf466b2667fdb4b8bc5948a1061d86c2, incorporating main 0286aa0868b1f0de1b908ce3358bbf994e5272c1 (Julia stdlib notices and shared INFO/fallback alert rendering). The npm implementation, input policy and fixture expectations are unchanged by this merge.

  • Final-head full-repository Sorbet: no errors, using the installed static executable against checked-in RBIs.
  • Actual shared-notice rendering: INFO and an unrecognized mode produce [!NOTE]; WARN remains [!WARNING], ERROR remains [!IMPORTANT].
  • Actual CLI #668 replays using the combined refreshed Honor cooldown when resolving npm transitive updates #16492 + Reject out-of-policy npm transitive updates #16496 runtime and draft smoke-tests #593: 3/3 complete outputs byte-identical, with 202/202, 200/200 and 249/249 cached HTTP calls, unchanged recorded clocks and no metadata or job errors. This replay was performed at policy 260f77e6e25e0b6cbcf00882ada974398de69a82; the subsequent spec-only isolation commit leaves all mounted npm/common/updater runtime libraries unchanged.
  • Test-isolation follow-up at de4a7cb8bf466b2667fdb4b8bc5948a1061d86c2: package-manager specs left npm selectors active across examples, producing 24 failures in the previous full npm run. Shared cleanup now resets both active and per-directory selectors after each example; redundant local cleanup hooks were removed. The existing two-example reproducer fails before / passes after, the complete previously failing worker passes 564 examples, 0 failures at seed 16904, all five changed Ruby files pass RuboCop, and full-repository Sorbet reports no errors. No production runtime code, cooldown/ignore guard or fixture contract changed.

21/21 required checks passed; 65 successful / 3 failed / 4 skipped check records, none pending. Full npm/yarn CI at de4a7cb8bf466b2667fdb4b8bc5948a1061d86c2: 2231 examples, 0 failures, 4 existing pending, seed 28164. The previous 24 required-spec failures are resolved.

Smoke actually ran 13 E2E jobs: 10 passed both the test and Diff steps, 3 failed. npm and rules omit the out-of-policy follow-redirects 1.16.1 / form-data 4.0.6 PRs still expected by main fixtures; cache coverage is 124/197 and 122/196. semver omits those dependencies from the expected group and has corresponding lockfile/metadata differences, with 148/223 cached calls. All three used released CLI v1.94.0 and main fixtures/official caches, not draft #593. These failures remain visible; no baseline-identical or released-toolchain-pass claim is made.

Maintainer review remains required. REST confirms dependabot/maintainers is already requested on all three Core prerequisite PRs and CLI #668; CLI also requests dependabot/azure-dev-ops. Empty gh pr view reviewer lists hid these existing team requests. The later individual requests for JamieMagee and jakecoffman returned HTTP 404 and added no individual reviewers; no duplicate request was made. The independent discovery fix #16498 remains separate; fixtures stay draft pending releases, official cache refreshes and released-toolchain Smoke.

Independent AI review follow-up

Current head: 17d417e99102a7567762baebd43bccb4f60ba19e; runtime implementation remains 683673c. Independent review fixed legacy-v1 bypass, unsafe removed-occurrence rebinding, and alias-identity substitution. AI review and before/after evidence. Focused checks passed: 85 examples, Sorbet, changed-file RuboCop, and three byte-identical full npm fixture replays.

Full CI then exposed two old npm6 assertions accepting root 5.7.4 despite a nested occurrence changing to 6.4.2 under a 6.0.2 cap. The test-only follow-ups now require rejection and include a compliant 6.4.2 positive control; no production guard or fixture expectation was weakened. Final-head CI passed: 2283 npm examples, 0 failures, 4 pending; all 21 required checks pass. Earlier CI counts above are historical. This AI technical review is not maintainer approval.

@myevolve

myevolve commented Oct 7, 2026 •

Copy link
Copy Markdown
Author

Current verification: de4a7cb8bf466b2667fdb4b8bc5948a1061d86c2

Latest runtime heads are Core #16492 97c3cac44b62df8ecf93b08f9041849e85ffa20f and Core #16496 de4a7cb8bf466b2667fdb4b8bc5948a1061d86c2, both incorporating main 0286aa0868b1f0de1b908ce3358bbf994e5272c1. The merge changes shared notice rendering and Julia stdlib notices, without weakening npm policy guards or fixture contracts.

Test-isolation follow-up at de4a7cb8bf466b2667fdb4b8bc5948a1061d86c2: package-manager specs left npm selectors active across examples, producing 24 failures in the previous full npm run. Shared cleanup now resets both active and per-directory selectors after each example; redundant local cleanup hooks were removed. The existing two-example reproducer fails before / passes after, the complete previously failing worker passes 564 examples, 0 failures at seed 16904, all five changed Ruby files pass RuboCop, and full-repository Sorbet reports no errors. No production runtime code, cooldown/ignore guard or fixture contract changed.

21/21 required checks passed; 65 successful / 3 failed / 4 skipped check records, none pending. Full npm/yarn CI at de4a7cb8bf466b2667fdb4b8bc5948a1061d86c2: 2231 examples, 0 failures, 4 existing pending, seed 28164. The previous 24 required-spec failures are resolved.

Smoke actually ran 13 E2E jobs: 10 passed both the test and Diff steps, 3 failed. npm and rules omit the out-of-policy follow-redirects 1.16.1 / form-data 4.0.6 PRs still expected by main fixtures; cache coverage is 124/197 and 122/196. semver omits those dependencies from the expected group and has corresponding lockfile/metadata differences, with 148/223 cached calls. All three used released CLI v1.94.0 and main fixtures/official caches, not draft #593. These failures remain visible; no baseline-identical or released-toolchain-pass claim is made.

Full-repository Sorbet passed on all three refreshed Core branches and again after the policy spec-isolation correction. Direct shared-notice rendering confirms INFO/fallback → NOTE, WARN → WARNING, ERROR → IMPORTANT. Three complete CLI replays at cooldown 97c3cac44b62df8ecf93b08f9041849e85ffa20f + policy 260f77e6e25e0b6cbcf00882ada974398de69a82 are byte-identical, with 202/202, 200/200 and 249/249 cached calls, unchanged clocks and no metadata/job errors. Policy’s subsequent de4a7cb8bf466b2667fdb4b8bc5948a1061d86c2 commit changes only spec isolation and its README; all mounted runtime library trees are unchanged. CLI source is unchanged; no new Go-suite run is claimed.

The policy remains fail-closed: out-of-policy native lockfile results are rejected, and registry errors retain their classification. Draft #593 preserves all original policy inputs and complete generated metadata.

Core #16498 at f4f87c4f4645b14edfbe7aee80c63ab38243126e: 21/21 required checks passed; 150 checks succeeded, 2 failed, 4 skipped. Smoke run discovered and executed all 97 E2E jobs: 95 passed both the actual ecosystem test and Diff steps; 2 failed. All 13 npm E2E jobs passed. The failures are Gradle (missing expected PR events after Maven metadata HTTP 404s; 95/125 calls cached) and pip-compile (pycparser 3.0 → 3.1; 96/130 cached). This is main runtime/main fixtures, not the unreleased npm policy or draft recordings; the overall Smoke run is not green. No fixture assertion or unrelated ecosystem code was changed, and no failed job was rerun.

Maintainer actions: (1) review/merge the independent discovery workflow fix and obtain genuine runtime-branch E2E evidence; (2) review/merge CLI #668 and Core #16492/#16496, then release the CLI clock support and an updater containing both runtime fixes; (3) run Cache One for npm, npm-group-rules, and npm-group-semver at fixture head aeed1cf2ffe310b228b69cc493de09182455372f, with read-only recording credentials and checked metadata-fetch logs; (4) run released-toolchain Smoke and obtain fixture review. CLI v1.94.0 still lacks clock replay. No upstream approval, upstream merge, release, official cache refresh, raised cap, changed source commit, or workflow/protection bypass was performed.

Review-routing correction: REST GET /repos/{owner}/{repo}/pulls/{number}/requested_reviewers confirms that dependabot/maintainers is already requested on Core #16492, #16496, #16498 and CLI #668. CLI #668 also requests dependabot/azure-dev-ops. These requests predate this continuation: Core requests are dated October 7/8 and both CLI requests October 7. gh pr view displayed empty reviewer lists, while GraphQL reports the existing requests with requestedReviewer: null; the empty display did not mean reviews were unassigned. Core REST evidence; CLI REST evidence.

The later individual requests for JamieMagee and jakecoffman returned HTTP 404 and added no individual reviewers. Existing team requests need review responses, not duplicate assignment. All four prerequisite PRs still report REVIEW_REQUIRED; the account remains read-only upstream. No new review request, approval, upstream merge, release, official cache refresh or protection bypass was performed.

@myevolve myevolve left a comment •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent AI-assisted technical review

Reviewed the original PR and re-reviewed the repairs now published at 683673c7fb7831c7a5715abb15f2b4055bff495d. The independent Core reviewer found three reproducible policy holes, all now fixed:

  • v1 lockfiles bypassed validation because only the modern packages table was traversed.
  • Removing a nested occurrence could rebind its consumer to an unchanged, over-cap ancestor installation.
  • An alias could change package identity at the same installation path while presenting an otherwise compliant version.

Production-Ruby before/after probes demonstrated the unsafe cases changing from accepted to rejected. The final re-review found no remaining actionable correctness defect; unchanged effective higher versions and normalized legacy-to-modern alias conversions remain supported.

Parent verification on the final implementation: 85 helper/parser examples passed, repository-configured Sorbet passed, and RuboCop passed all four changed Ruby files. The rebuilt CLI at 49b4c75 also replayed npm, group-rules, and group-semver with the combined repaired Core sources; all full generated YAMLs were byte-identical to the unchanged #593 fixtures. Broader native integration specs and fresh upstream CI are separate from this focused evidence.

This is an explicitly AI-authored technical COMMENT, not maintainer approval. Earlier CI counts in the thread belong to their recorded older heads, not this new commit.

Legacy contract follow-up

The initial full CI run at 683673c exposed two stale acceptance assertions, not a reason to relax the validator. Actual native npm resolution on the untouched legacy fixture changes root Acorn 5.5.3→5.7.4 and nested Acorn 6.1.1→6.4.2, while those examples requested a 6.0.2 cap. Returning no resolvable update is correct.

Final test/documentation-only head: 17d417e99102a7567762baebd43bccb4f60ba19e. The two cases now require rejection; a positive control allows 6.4.2 and expects the enabled all-occurrences resolver to return 6.4.2. The independent reviewer re-checked this final contract and found no remaining actionable issue. Production validation and all fixture expectations remain unchanged from the replayed implementation.

Canonical CI for the final head passed: 2283 examples, 0 failures, 4 pending in the npm suite, and all 21 required checks passed. The earlier failing run remains historical; it was not relabeled green.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant