Skip to content

fix(ci): lockfile-age-audit — fail closed on non-registry sources + prettify - #886

Merged
peter-leonov-ch merged 3 commits into
mainfrom
fix/lockfile-age-audit-failclosed-prettier
Jun 24, 2026
Merged

fix(ci): lockfile-age-audit — fail closed on non-registry sources + prettify#886
peter-leonov-ch merged 3 commits into
mainfrom
fix/lockfile-age-audit-failclosed-prettier

Conversation

@peter-leonov-ch

Copy link
Copy Markdown
Collaborator

Summary

Addresses CI failure and review feedback on the lockfile age gate (scripts/ci/lockfile-age-audit.mjs, introduced in #882). These changes are made in a fresh PR to main because #883 targets the release branch and cannot be modified.

Two fixes, both in scripts/ci/lockfile-age-audit.mjs:

  1. Fail closed on non-registry sources (Copilot review comment). Previously extractNpmResolutions silently continued on any entry whose resolved host was not an allowed registry, so a PR could bypass the age gate entirely by pinning a new dependency from an alternative registry, a plain-http URL, or a git+https source. Now isRegistryEntry is replaced by classifyResolved:

    • registry — allowed HTTPS registry tarball → audited against the age gate (unchanged behavior).
    • foreign — a URL but not an allowed HTTPS registry host → fails closed (new).
    • local — no resolved URL (workspace source dirs, file: links) → skipped, nothing to age-check.

    The lockfile-age-skip PR label (handled in .github/workflows/lockfile-age-audit.yml) remains the intentional escape hatch.

  2. Prettier formatting (Copilot review comment + the failing code-quality check). The file was written in single-quote / no-semicolon style; the repo's Prettier defaults (.prettierrc = {}) use double quotes + semicolons. Reformatted with prettier --write.

Verification

  • prettier --check passes on the file (full-repo check is clean apart from an untracked local scratch dir).
  • node --check passes.
  • Manually exercised the fail-closed path against synthetic lockfiles: new deps resolved from an alternative host, git+https, and plain http all fail closed; unchanged legit registry entries are not flagged.

Test plan

  • npm run prettier:check clean for tracked files
  • Fail-closed logic verified against synthetic base/head lockfiles

🤖 Generated with Claude Code

…rettify

Addresses review feedback on the lockfile age gate (introduced in #882):

- Fail closed on newly-added deps resolved from a non-registry source
  (alternative registry, plain http, git+https, etc.) instead of silently
  skipping them. Previously a PR could bypass the age gate entirely by
  pinning a package from a non-allowed host. The 'lockfile-age-skip' label
  (handled in the workflow) remains the intentional escape hatch.
  `isRegistryEntry` becomes `classifyResolved` -> registry | foreign | local;
  'local' (workspace/file: entries with no resolved URL) is still skipped,
  'foreign' now fails closed.
- Format with the repo's Prettier defaults (the file was unformatted, which
  failed the code-quality check).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 24, 2026 10:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Updates the CI lockfile age gate script to close a bypass where newly-added dependencies could avoid age checks by using non-registry resolved sources, and reformats the script to match repo Prettier defaults.

Changes:

  • Replace registry-only filtering with classifyResolved to distinguish registry vs foreign vs local entries and fail closed on newly-added non-registry sources.
  • Apply Prettier formatting (double quotes + semicolons) across the script.

Comment thread scripts/ci/lockfile-age-audit.mjs
Comment thread scripts/ci/lockfile-age-audit.mjs
peter-leonov-ch and others added 2 commits June 24, 2026 12:35
…branch

Captures the workflow for addressing CI failures and review comments on a PR
that targets the protected `release` branch: route the fix through a separate
PR to `main`, then reply to and resolve the original review threads. Encodes the
gotchas hit in practice — the release PR's head is `main` (so `gh pr checkout`
is a no-op), the `success` check is just an aggregate gate, Prettier house style
is the `.prettierrc {}` defaults, and `prettier:check` may flag untracked scratch
dirs that CI never sees.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…as local, redact creds

Copilot review feedback on this PR:

- classifyResolved: a present-but-unparseable `resolved` string was classified
  'local' (skipped), allowing a bypass via a hand-edited lockfile — now fails
  closed ('foreign'). file: dependencies parse as URLs and were falling through
  to 'foreign'; they are now explicitly 'local' (not a registry download, nothing
  to age-check), matching the documented intent.
- Redact the `resolved` value in the fail-closed log line via redactResolved()
  (protocol + host only) so credentials embedded in a URL
  (https://user:token@host/...) can't leak into CI logs.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@peter-leonov-ch
peter-leonov-ch enabled auto-merge (squash) June 24, 2026 10:39
@peter-leonov-ch
peter-leonov-ch merged commit 3351cb6 into main Jun 24, 2026
10 checks passed
@peter-leonov-ch
peter-leonov-ch deleted the fix/lockfile-age-audit-failclosed-prettier branch June 24, 2026 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants