fix(install): store and verify SHA-512 integrity hash for GitHub tarball dependencies - #27019
Conversation
…all dependencies
GitHub git dependencies downloaded as tarballs via the GitHub API were
not having their content integrity verified against the lockfile. A
compromised api.github.com or codeload.github.com server could serve
a different tarball than expected without detection.
This change:
- Computes SHA-512 of GitHub tarball bytes during initial extraction
- Stores the hash in bun.lock alongside the resolved commit hash
- Verifies the hash on subsequent installs when re-downloading
- Maintains backward compatibility with lockfiles without integrity
The lockfile format for GitHub dependencies changes from:
["pkg@github:user/repo#ref", {}, "resolved-commit"]
to:
["pkg@github:user/repo#ref", {}, "resolved-commit", "sha512-..."]
Old lockfiles without the integrity field continue to work normally.
Co-Authored-By: Claude <noreply@anthropic.com>
When re-installing from an old lockfile that doesn't have integrity hashes for GitHub dependencies, compute the hash from the downloaded tarball and force-save the lockfile with the new hash. Also adds comprehensive tests for: - Integrity hash stored on fresh install - Integrity verified on re-install with matching hash - Install rejected when integrity doesn't match - Old lockfile automatically upgraded with integrity hash - Cache hits still work without integrity Co-Authored-By: Claude <noreply@anthropic.com>
|
Updated 6:48 PM PT - Feb 16th, 2026
❌ @Jarred-Sumner, your commit f7e8055 has 2 failures in
🧪 To try this PR locally: bunx bun-pr 27019That installs a local version of the PR into your bun-27019 --bun |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds SHA-512 integrity support for GitHub/Git tarball installs: extraction now attaches or reuses an integrity hash, PackageManager/PackageInstaller persist missing integrity into the lockfile and force-save it, bun.lock parsing/serialization accepts and emits an optional integrity field, and tests exercise capture, verification, mismatch, upgrade, and cache reuse. (50 words) Changes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@test/regression/issue/github-tarball-integrity.test.ts`:
- Around line 70-79: The test currently spawns proc1 via Bun.spawn and awaits
proc1.exited but doesn't assert its exit code before reading bun.lock; update
the test to check proc1.exitCode (or proc1.exitStatus) is 0 immediately after
awaiting proc1.exited and fail the test (include proc1.stdout/proc1.stderr
output in the assertion message) if it is nonzero so we don't proceed after a
failed install; locate the Bun.spawn call and the await proc1.exited usage and
add the exit-code assertion before any code that reads or relies on bun.lock.
- Around line 1-7: The test file name github-tarball-integrity.test.ts in the
test/regression/issue folder violates the naming convention; rename this file to
use a real numeric GitHub issue ID (e.g., 12345.test.ts) or move it out of
test/regression/issue into an appropriate folder; update any imports or
references to the file if present (search for
"github-tarball-integrity.test.ts") so the test runner and CI pick up the
correctly named regression test.
- Around line 8-19: The clearGitHubCache function should respect a custom cache
dir and avoid scanning a missing directory: compute cacheDir from
process.env.BUN_INSTALL_CACHE_DIR || join(homedir(), ".bun", "install",
"cache"), check that cacheDir exists (e.g., with fs.existsSync or fs.stat) and
return early if not present, and only then create Bun.Glob instances and call
glob.scan / indexGlob.scan; keep the existing join(cacheDir, entry) removal
logic and preserve pattern handling in clearGitHubCache.
| import { file } from "bun"; | ||
| import { describe, expect, test } from "bun:test"; | ||
| import { rm } from "fs/promises"; | ||
| import { bunEnv, bunExe, tempDir } from "harness"; | ||
| import { homedir } from "os"; | ||
| import { join } from "path"; | ||
|
|
There was a problem hiding this comment.
Rename this regression test file to a real issue number (or move it out of test/regression/issue).
The filename is not a numeric issue ID, which violates the regression-test naming convention for this folder.
As per coding guidelines, "test/regression/issue/*.test.ts: Place regression tests for GitHub issues in test/regression/issue/${issueNumber}.test.ts and ensure the issue number is REAL, not a placeholder."
🤖 Prompt for AI Agents
In `@test/regression/issue/github-tarball-integrity.test.ts` around lines 1 - 7,
The test file name github-tarball-integrity.test.ts in the test/regression/issue
folder violates the naming convention; rename this file to use a real numeric
GitHub issue ID (e.g., 12345.test.ts) or move it out of test/regression/issue
into an appropriate folder; update any imports or references to the file if
present (search for "github-tarball-integrity.test.ts") so the test runner and
CI pick up the correctly named regression test.
Code ReviewNewest first ✅ 51919 — Looks good! Reviewed 6 files across ✅ 65552 — Looks good! Reviewed 6 files across Powered by Claude Code Review |
- Move test from test/regression/issue/ to test/cli/install/ since there is no numbered GitHub issue for this advisory - Use BUN_INSTALL_CACHE_DIR env var with fallback, and check directory exists before scanning in clearGitHubCache - Add exit code assertion after first install in the re-install test Co-Authored-By: Claude <noreply@anthropic.com>
|
Addressed all review comments in a5f00a0:
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@test/cli/install/bun-install-github-integrity.test.ts`:
- Around line 83-88: The test currently grabs the first sha512 in the lockfile
(lockfileContent -> integrityMatch -> integrityHash) which can be from a
registry; instead, locate the GitHub entry first by matching the resolved GitHub
commit/URL (the resolved line that contains github.com and the commit hash) and
then extract the integrity value adjacent to that GitHub block; update the
regex/lookup to first find the GitHub resolved entry and then search its nearby
lines for the "sha512-..." string, and adjust the integrityMatch/integrityHash
extraction to use that matched integrity for the assertion.
- Around line 10-19: clearGitHubCache currently reads
process.env.BUN_INSTALL_CACHE_DIR but the test spawn uses bunEnv without setting
BUN_INSTALL_CACHE_DIR, causing mismatched cache locations; update the test to
create a per-test cache dir (e.g., const cacheDir = join(packageDir,
".bun-cache")), pass that path into clearGitHubCache(cacheDir) and set it on the
spawned process env by spreading bunEnv and adding BUN_INSTALL_CACHE_DIR:
cacheDir so both cleanup and installs use the same per-test cache; locate uses
of clearGitHubCache, bunEnv, and the spawn call in this test file to apply the
change.
- Replace clearGitHubCache with per-test BUN_INSTALL_CACHE_DIR inside
each temp dir, matching the pattern used by other install tests
- Fix integrity regex to anchor on the GitHub resolved entry
("jonschlinkert-is-number-98e8ff1") to avoid matching npm hashes
- Make cache-hit test deterministic: warm cache with a first install,
strip integrity from lockfile, then verify second install succeeds
from cache
Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@test/cli/install/bun-install-github-integrity.test.ts`:
- Around line 70-71: Reorder the assertions for the first install so that test
output is asserted before checking the process exit; specifically, after
awaiting proc1.stdout.text(), proc1.stderr.text(), and proc1.exited into
stdout1, stderr1, exitCode1 (from proc1), add assertions on stdout1 and/or
stderr1 (e.g., expect(stdout1).toContain(...) or expect(stderr1).toBe(''))
before calling expect(exitCode1).toBe(0) to ensure stderr1 is printed when the
test fails.
- Around line 225-226: Reorder the assertions in the test that reads proc1
outputs: after awaiting stdout1, stderr1, and exitCode1, assert the process
output values (e.g., expect(stdout1).toBe(...) and/or expect(stderr1).toBe(...))
before asserting expect(exitCode1).toBe(0); update the test that uses proc1,
stdout1, stderr1, and exitCode1 so stdout/stderr expectations run prior to the
exitCode assertion to provide better failure diagnostics.
Co-Authored-By: Claude <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| }); | ||
|
|
||
| const [stdout1, stderr1, exitCode1] = await Promise.all([proc1.stdout.text(), proc1.stderr.text(), proc1.exited]); | ||
| expect(stderr1).not.toContain("error:"); |
Code ReviewNewest first ✅ ddec7 — Looks good! Reviewed 12 files implementing SHA-512 integrity verification for GitHub tarball dependencies in the package manager: computes and stores content hashes in lockfiles on first install and verifies them on subsequent installs to detect tarball tampering. Note: One minor nit (exitCode checked before stderr validation at lines 71 and 226 in the test file) was identified but already flagged by CodeRabbit in existing review threads. Powered by Claude Code Review |
| } else { | ||
| // First install (no integrity in the lockfile yet): compute it. | ||
| result.integrity = .{ .tag = .sha512 }; | ||
| std.crypto.hash.sha2.Sha512.hash(bytes, result.integrity.value[0..std.crypto.hash.sha2.Sha512.digest_length], .{}); |
There was a problem hiding this comment.
Use bun.sha.*, not std.crypto.
Jarred-Sumner
left a comment
Jarred-Sumner
left a comment
There was a problem hiding this comment.
Use bun.sha not std.crypto
It must verify integrity before extracting. Not after.
Co-Authored-By: Claude <noreply@anthropic.com>
Code ReviewNewest first ✅ 904c9 — Looks good! Reviewed 33 files across Powered by Claude Code Review |
Code ReviewNewest first ✅ 904c9 — Looks good! Reviewed 37 files across Powered by Claude Code Review |
Code ReviewNewest first ✅ 06a9e — Looks good! Reviewed 5 files across Powered by Claude Code Review |
Code ReviewNewest first ✅ f7e80 — Looks good! Reviewed 6 files across Powered by Claude Code Review |
…all dependencies (oven-sh#27019) ## Summary - Compute SHA-512 hash of GitHub tarball bytes during extraction and store in `bun.lock` - Verify the hash on subsequent installs when re-downloading, rejecting tampered tarballs - Automatically upgrade old lockfiles without integrity by computing and persisting the hash - Maintain backward compatibility with old lockfile format (no integrity field) Fixes GHSA-pfwx-36v6-832x ## Lockfile format change ``` Before: ["pkg@github:user/repo#ref", {}, "resolved-commit"] After: ["pkg@github:user/repo#ref", {}, "resolved-commit", "sha512-..."] ``` The integrity field is optional for backward compatibility. Old lockfiles are automatically upgraded when the tarball is re-downloaded. ## Test plan - [x] Fresh install stores SHA-512 integrity hash in lockfile - [x] Re-install with matching hash succeeds - [x] Re-install with mismatched hash rejects the tarball - [x] Old lockfile without integrity is auto-upgraded with hash on re-download - [x] Cache hits still work without re-downloading - [x] Existing GitHub dependency tests pass (10/10) - [x] Existing git resolution snapshot test passes - [x] Yarn migration snapshot tests pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Bot <claude-bot@bun.sh> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
…all dependencies (oven-sh#27019) ## Summary - Compute SHA-512 hash of GitHub tarball bytes during extraction and store in `bun.lock` - Verify the hash on subsequent installs when re-downloading, rejecting tampered tarballs - Automatically upgrade old lockfiles without integrity by computing and persisting the hash - Maintain backward compatibility with old lockfile format (no integrity field) Fixes GHSA-pfwx-36v6-832x ## Lockfile format change ``` Before: ["pkg@github:user/repo#ref", {}, "resolved-commit"] After: ["pkg@github:user/repo#ref", {}, "resolved-commit", "sha512-..."] ``` The integrity field is optional for backward compatibility. Old lockfiles are automatically upgraded when the tarball is re-downloaded. ## Test plan - [x] Fresh install stores SHA-512 integrity hash in lockfile - [x] Re-install with matching hash succeeds - [x] Re-install with mismatched hash rejects the tarball - [x] Old lockfile without integrity is auto-upgraded with hash on re-download - [x] Cache hits still work without re-downloading - [x] Existing GitHub dependency tests pass (10/10) - [x] Existing git resolution snapshot test passes - [x] Yarn migration snapshot tests pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Bot <claude-bot@bun.sh> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
|
The GitHub integrity test added here installed a real repository from api.github.com and failed whenever GitHub did (build 98240); #39115 serves the tarball locally instead. |
2 participants
Summary
bun.lockFixes GHSA-pfwx-36v6-832x
Lockfile format change
The integrity field is optional for backward compatibility. Old lockfiles are automatically upgraded when the tarball is re-downloaded.
Test plan
🤖 Generated with Claude Code