Skip to content

Use SHA-512 integrity in package locks - #14701

Open
Sean McManus (sean-mcmanus) wants to merge 4 commits into
mainfrom
seanmcm/devbox2-wsl/agent76/sha512-lock-integrity
Open

Use SHA-512 integrity in package locks#14701
Sean McManus (sean-mcmanus) wants to merge 4 commits into
mainfrom
seanmcm/devbox2-wsl/agent76/sha512-lock-integrity

Conversation

@sean-mcmanus

@sean-mcmanus Sean McManus (sean-mcmanus) commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Upgrade dependency integrity from SHA-1-only values to SHA-512 across the Yarn and npm lockfiles. Add repository-wide validation and a byte-verifying npm lock updater, and make npm consumers use frozen installs so builds consume the reviewed dependency graph.

The updater fetches exact tarballs from the configured approved registry, verifies each existing SHA-1 value against the downloaded bytes, computes SHA-512 independently, and protects lockfile replacement against partial or concurrent writes.

Validation

  • Verified 617 unique npm tarballs and 932 Yarn entries against fetched package bytes
  • Exercised clean online and offline restores for all lockfiles
  • Confirmed corrupted npm and Yarn cache entries fail integrity validation
  • Added 31 focused lockfile/updater tests
  • Passed extension compile, ESLint, repository lock validation, updater idempotence, and git diff --check

This PR was investigated and created by GitHub Copilot in VS Code. Any message starting with ✨Copilot: was sent by GitHub Copilot.

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

Upgrades dependency integrity and enforces reproducible dependency installation.

Changes:

  • Migrates lockfile integrity values to SHA-512.
  • Adds lockfile validation, updater tooling, and tests.
  • Replaces mutable npm installs with npm ci.

Reviewed changes

Copilot reviewed 29 out of 34 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
Themes/package-lock.json Migrates integrity hashes to SHA-512.
Extension/readme.developer.md Documents verified Yarn bootstrap.
Extension/package.json Adds lockfile scripts and bootstrap changes.
Extension/.yarn-bootstrap/package-lock.json Upgrades Yarn integrity to SHA-512.
Extension/.scripts/verifyYarnLock.test.mjs Tests lockfile integrity validation.
Extension/.scripts/verifyYarnLock.mjs Validates Yarn and npm lockfiles.
Extension/.scripts/updatePackageLockIntegrity.test.mjs Tests updater and atomic writes.
Extension/.scripts/updatePackageLockIntegrity.mjs Adds byte-verifying lockfile updater.
Extension/.scripts/subresourceIntegrity.mjs Parses and computes integrity values.
Extension/.scripts/packageLockFiles.mjs Discovers repository package locks.
Build/package/jobs_package_vsix.yml Uses frozen npm installation.
.github/workflows/question-closer.yml Uses npm ci.
.github/workflows/question-closer-debugger.yml Uses npm ci.
.github/workflows/more-info-needed-closer.yml Uses npm ci.
.github/workflows/more-info-needed-closer-debugger.yml Uses npm ci.
.github/workflows/locker.yml Uses npm ci.
.github/workflows/job-compile-and-test.yml Runs repository lock validation.
.github/workflows/investigate-costing-closer-debugger.yml Uses npm ci.
.github/workflows/investigate-closer-debugger.yml Uses npm ci.
.github/workflows/feature-request-reopener.yml Uses npm ci.
.github/workflows/feature-request-debugger.yml Uses npm ci.
.github/workflows/feature-request-closer-triage.yml Uses npm ci.
.github/workflows/feature-request-closer-no-milestone.yml Uses npm ci.
.github/workflows/external-closer-debugger.yml Uses npm ci.
.github/workflows/enhancement-reopener.yml Uses npm ci.
.github/workflows/enhancement-closer-triage.yml Uses npm ci.
.github/workflows/enhancement-closer-no-milestone.yml Uses npm ci.
.github/workflows/duplicate-closer.yml Uses npm ci.
.github/workflows/by-design-closer.yml Uses npm ci.
.github/workflows/by-design-closer-debugger.yml Uses npm ci.
.github/workflows/bug-debugger.yml Uses npm ci.
Files not reviewed (1)
  • Extension/.yarn-bootstrap/package-lock.json: Generated file

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread Extension/.scripts/subresourceIntegrity.mjs Outdated
Comment thread Extension/.scripts/verifyYarnLock.mjs Outdated
Comment thread Extension/.scripts/verifyYarnLock.mjs Outdated

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

Copilot reviewed 29 out of 34 changed files in this pull request and generated 2 comments.

Files not reviewed (1)
  • Extension/.yarn-bootstrap/package-lock.json: Generated file

Comment thread Extension/.scripts/packageLockFiles.mjs Outdated
Comment thread Extension/.scripts/packageLockFiles.mjs Outdated

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

Copilot reviewed 29 out of 34 changed files in this pull request and generated 1 comment.

Files not reviewed (1)
  • Extension/.yarn-bootstrap/package-lock.json: Generated file

Comment thread Extension/.scripts/updatePackageLockIntegrity.mjs Outdated

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

Copilot reviewed 29 out of 34 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • Extension/.yarn-bootstrap/package-lock.json: Generated file

@sean-mcmanus
Sean McManus (sean-mcmanus) marked this pull request as ready for review August 22, 2026 00:22
@sean-mcmanus
Sean McManus (sean-mcmanus) requested a review from a team as a code owner August 22, 2026 00:22

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

✨Copilot (agent102): Reviewed at 493b432e merged onto main 92768bc0. The merge base is the current target head, so the merged result is byte-identical to the PR head.

Independent validation performed locally:

  • Every changed line in Extension/yarn.lock, Extension/.yarn-bootstrap/package-lock.json, ExtensionPack/package-lock.json, Themes/package-lock.json, and .github/actions/package-lock.json is an integrity value; no version, resolution, or dependency-graph drift.
  • 35 sampled SHA-512 values (npm locks and yarn entries) match the dist.integrity published by the upstream registry exactly.
  • yarn 1.22.22's bundled ssri selects the strongest listed algorithm, so the retained sha1-… sha512-… pairs are verified against SHA-512, not SHA-1.
  • npm ci --omit=dev in .github/actions (85 packages), npm ci in Themes (283 packages), and the new npm ci --prefix .yarn-bootstrap + offline global install all succeed and yield yarn 1.22.22; ExtensionPack's non-root package set is identical to Themes. These paths are not covered by PR CI, so they were exercised directly.
  • verify-lockfiles passes and all 31 tests pass.

Only two nits below; nothing blocking from my side.

Comment thread Extension/package.json
"test-yarn-lock": "node --test .scripts/verifyYarnLock.test.mjs",
"test-lockfiles": "node --test .scripts/verifyYarnLock.test.mjs .scripts/updatePackageLockIntegrity.test.mjs",
"verify-yarn-lock": "node .scripts/verifyYarnLock.mjs",
"verify-lockfiles": "node .scripts/verifyYarnLock.mjs",

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

✨Copilot (agent102): [Nit] verify-lockfiles is byte-identical to verify-yarn-lock, and test-lockfiles is a superset of test-yarn-lock. After this change nothing in the repository references the two older names any more (CI now runs test-lockfiles/verify-lockfiles), so they are dead aliases that will make a future reader wonder which one is authoritative. Consider dropping test-yarn-lock and verify-yarn-lock.

function validatePackageLocks(packageLockPaths) {
const unsupportedEntries = [];
for (const packageLockPath of packageLockPaths) {
const packageLock = JSON.parse(fs.readFileSync(packageLockPath, 'utf8'));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

✨Copilot (agent102): [Nit] Neither failure mode in this loop names the offending file, even though findPackageLockPaths now discovers an unbounded set of lockfiles across the repository. Verified against this revision:

  • JSON.parse on a malformed lockfile → Expected property name or '}' in JSON at position 2 (line 1 column 3)
  • findUnsupportedPackageLockIntegrityEntries on a lockfile with no packagespackage-lock.json does not contain a packages object.

Both leave the developer to guess which of the lockfiles is broken. loadPackageLocks in the updater already prefixes its equivalent error with packageLockPath; wrapping the read/parse here (or passing the path into findUnsupportedPackageLockIntegrityEntries) would make verify-lockfiles equally diagnosable.

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

Labels

None yet

Projects

Status: Pull Request

Development

Successfully merging this pull request may close these issues.

2 participants