Skip to content

fix: store and verify integrity hashes for tarball dependencies - #25264

Closed
robobun wants to merge 7 commits into
mainfrom
claude/fix-tarball-integrity-vulnerability
Closed

robobun wants to merge 7 commits into
mainfrom
claude/fix-tarball-integrity-vulnerability

Conversation

@robobun

@robobun robobun commented Dec 1, 2025

Copy link
Copy Markdown
Collaborator

Summary

This PR adds integrity hash storage and verification for HTTP tarball and local tarball dependencies, bringing Bun's lockfile behavior in line with npm and Yarn.

Previously, bun.lock did not store integrity hashes for tarball URL dependencies (e.g., "pkg": "https://example.com/pkg.tgz"). Now, Bun computes and stores SHA-512 integrity hashes for these dependencies, just like it already does for npm registry packages.

Changes

  • Compute SHA-512 integrity hash when extracting tarballs
  • Store integrity in bun.lock for remote_tarball and local_tarball packages
  • Verify integrity on subsequent installs

Example

Before:

{
  "packages": {
    "pkg": ["pkg@https://example.com/pkg.tgz", {}]
  }
}

After:

{
  "packages": {
    "pkg": ["pkg@https://example.com/pkg.tgz", {}, "sha512-abc123...=="]
  }
}

Comparison with other package managers

Package Manager Stores integrity for tarball URLs
npm ✅ Yes
Yarn ✅ Yes
pnpm ❌ No
Bun (before) ❌ No
Bun (after) ✅ Yes

Test plan

  • Added regression tests for tarball integrity
  • Verified local tarball dependencies get integrity hashes
  • Verified integrity check fails when tarball content changes but lockfile hash doesn't match

🤖 Generated with Claude Code

Previously, Bun did not store or verify integrity hashes for HTTP tarball
dependencies (remote_tarball) or local tarball dependencies (local_tarball).
Each `bun install` would download/read the tarball without verification,
allowing silent package substitution if the tarball content changed.

This security fix:
1. Computes SHA-512 integrity hash when extracting tarballs that don't
   already have an integrity hash (remote and local tarballs)
2. Stores the computed integrity hash in the package metadata
3. Serializes the integrity hash to bun.lock for remote_tarball and
   local_tarball packages (similar to how npm packages already work)
4. Parses the integrity hash from bun.lock when reading the lockfile
5. Passes the integrity to the tarball extraction task for verification
   on subsequent installs

Now when a remote or local tarball's content changes but the lockfile
integrity doesn't match, bun will fail with "Integrity check failed".

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@robobun

robobun commented Dec 1, 2025 •

Copy link
Copy Markdown
Collaborator Author
Updated 2:56 PM PT - Dec 11th, 2025

❌ @dylan-conway, your commit 9dca31f has 10 failures in Build #33218 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 25264

That installs a local version of the PR into your bun-25264 executable, so you can run:

bun-25264 --bun

@github-actions github-actions Bot added the claude label Dec 1, 2025
Pass Integrity by pointer instead of by value to avoid unnecessary copies.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Dec 1, 2025 •

Copy link
Copy Markdown
Contributor

Walkthrough

Compute and propagate tarball integrity (SHA‑512) from extraction into package metadata and bun.lock, thread Integrity through enqueue/task payloads for local tarballs, add Integrity.compute, and add tests validating lockfile storage and verification failures on tampering.

Changes

Cohort / File(s) Change Summary
Integrity core
src/install/integrity.zig, src/install/install.zig
Add Integrity.compute(bytes) (SHA‑512) and re-export Integrity; add ExtractData.computed_integrity: Integrity to extraction data.
Extraction & metadata persistence
src/install/extract_tarball.zig, src/install/PackageManager/processDependencyList.zig
Compute or reuse integrity during extraction, include computed_integrity in extract results, and assign package.meta.integrity = data.computed_integrity before appending to the lockfile.
Enqueue / task plumbing
src/install/PackageManager/PackageManagerEnqueue.zig, src/install/PackageInstaller.zig, src/install/isolated_install.zig
Add Integrity parameter to enqueueTarballForReading and thread it into enqueueLocalTarball; add .integrity to task payloads and pass lockfile meta integrity into enqueue calls; update signatures/imports.
Lockfile serialization & parsing
src/install/lockfile/bun.lock.zig
Conditionally emit integrity for .local_tarball and .remote_tarball entries when supported; make parsing of tarball integrity optional when absent.
Tests
test/cli/install/tarball-integrity.test.ts
Add tests: assert sha512 integrity stored in bun.lock for local tarballs and assert install fails with "Integrity check failed" when tarball content changes.

Suggested reviewers

  • cirospaciari
  • Jarred-Sumner

Pre-merge checks

✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main change: adding integrity hash storage and verification for tarball dependencies.
Description check ✅ Passed The description includes both required sections (What does this PR do? and How did you verify your code works?) with comprehensive content covering changes, examples, and test verification.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between aa8d97f and 9dca31f.

📒 Files selected for processing (2)
  • src/install/PackageInstaller.zig (1 hunks)
  • src/install/isolated_install.zig (2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.{cpp,zig}

📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)

src/**/*.{cpp,zig}: Use bun bd or bun run build:debug to build debug versions for C++ and Zig source files; creates debug build at ./build/debug/bun-debug
Run tests using bun bd test <test-file> with the debug build; never use bun test directly as it will not include your changes
Execute files using bun bd <file> <...args>; never use bun <file> directly as it will not include your changes
Enable debug logs for specific scopes using BUN_DEBUG_$(SCOPE)=1 environment variable
Code generation happens automatically as part of the build process; no manual code generation commands are required

Files:

  • src/install/isolated_install.zig
  • src/install/PackageInstaller.zig
src/**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)

Use bun.Output.scoped(.${SCOPE}, .hidden) for creating debug logs in Zig code

Implement core functionality in Zig, typically in its own directory in src/

src/**/*.zig: Private fields in Zig are fully supported using the # prefix: struct { #foo: u32 };
Use decl literals in Zig for declaration initialization: const decl: Decl = .{ .binding = 0, .value = 0 };
Prefer @import at the bottom of the file (auto formatter will move them automatically)

Be careful with memory management in Zig code - use defer for cleanup with allocators

Files:

  • src/install/isolated_install.zig
  • src/install/PackageInstaller.zig
**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)

**/*.zig: Expose generated bindings in Zig structs using pub const js = JSC.Codegen.JS<ClassName> with trait conversion methods: toJS, fromJS, and fromJSDirect
Use consistent parameter name globalObject instead of ctx in Zig constructor and method implementations
Use bun.JSError!JSValue return type for Zig methods and constructors to enable proper error handling and exception propagation
Implement resource cleanup using deinit() method that releases resources, followed by finalize() called by the GC that invokes deinit() and frees the pointer
Use JSC.markBinding(@src()) in finalize methods for debugging purposes before calling deinit()
For methods returning cached properties in Zig, declare external C++ functions using extern fn and callconv(JSC.conv) calling convention
Implement getter functions with naming pattern get<PropertyName> in Zig that accept this and globalObject parameters and return JSC.JSValue
Access JavaScript CallFrame arguments using callFrame.argument(i), check argument count with callFrame.argumentCount(), and get this with callFrame.thisValue()
For reference-counted objects, use .deref() in finalize instead of destroy() to release references to other JS objects

Files:

  • src/install/isolated_install.zig
  • src/install/PackageInstaller.zig
🧠 Learnings (1)
📓 Common learnings
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24880
File: packages/bun-vscode/package.json:382-385
Timestamp: 2025-11-20T19:51:32.288Z
Learning: In the Bun repository, dependencies may be explicitly added to package.json files (even when not directly imported in code) to force version upgrades on transitive dependencies, particularly as part of Aikido security scanner remediation to ensure vulnerable transitive dependencies resolve to patched versions.
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 25111
File: packages/bun-types/wasm.d.ts:103-104
Timestamp: 2025-11-27T15:18:21.607Z
Learning: In the Bun repository, autofixci automatically formats code in the main branch, which can cause formatting diffs to appear in pull requests even when the PR author didn't make those changes. These formatting changes from autofixci are expected and not the PR author's responsibility.
🔇 Additional comments (2)
src/install/isolated_install.zig (1)

808-808: LGTM!

The integrity propagation for local tarballs is correctly implemented. The pkg_metas slice extraction at line 808 follows the established pattern for accessing package data, and the integrity reference is correctly passed to enqueueTarballForReading at line 1089 using the appropriate pkg_id variable from the enclosing scope.

Also applies to: 1083-1090

src/install/PackageInstaller.zig (1)

1000-1007: LGTM!

The integrity parameter is correctly passed for local tarball verification. Using this.metas[package_id].integrity is appropriate since the integrity hash is associated with the resolved package in the lockfile.

The asymmetry with remote_tarball (lines 1009-1022) which uses patch_name_and_version_hash instead of integrity is intentional—remote tarballs compute integrity during download, while local tarballs need to verify against the stored lockfile value.

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • SHA-512: Entity not found: Issue - Could not find referenced Issue.

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
src/install/PackageManager/PackageManagerEnqueue.zig (1)

1073-1159: Use of .{} as “no integrity yet” sentinel is acceptable but a bit opaque

Passing .{} // No integrity for first-time install, will be computed during extraction into enqueueLocalTarball for .tarball → .local on first install is reasonable, assuming Integrity treats that value as “no integrity / compute-only” and the extractor will compute and persist a hash rather than attempt to verify against it.

For readability and future-proofing of the Integrity type, consider replacing the raw .{} with a named value or constructor (e.g. a documented Integrity.empty, .none, or similar) so the “no integrity yet” semantics are obvious and remain correct if Integrity’s internal representation changes.

src/install/PackageInstaller.zig (1)

1000-1007: Local tarball enqueue correctly uses lockfile meta integrity (minor style nit)

Routing this.lockfile.packages.items(.meta)[package_id].integrity into enqueueTarballForReading is the right behavior and matches the isolated-install path.

If you want to tighten things up, you could read from the cached this.metas[package_id].integrity instead of re-slicing lockfile.packages, but that’s purely a micro-optimization / consistency tweak.

src/install/lockfile/bun.lock.zig (1)

516-566: Tarball integrity serialization is correct, but parser now rejects older bun.lock files without tarball integrity

The new behavior has two pieces:

  • Writer side: For .local_tarball and .remote_tarball you now emit:
    • ["name@tarball", INFO, "integrity"], symmetric with the existing npm layout (which has ["name@version", registry, INFO, "integrity"]).
    • This is internally consistent and will round-trip correctly with the updated parser.
  • Parser side: parseIntoBinaryLockfile now treats .local_tarball and .remote_tarball the same as .npm for integrity:
    • In the “// integrity” switch you added .local_tarball, .remote_tarball and hard-require a third/fourth array element; if it’s missing, you log “Missing integrity” and return error.InvalidPackageInfo.

Given the PR description explicitly says previous bun.lock did not store integrity for tarball URL dependencies, existing text lockfiles with tarball entries will not have this integrity element. Under the new parser they will now fail to load instead of being transparently upgraded.

That’s a backward-compatibility break: users with an older bun.lock that includes tarball deps can’t run bun install once to migrate; they’ll just get InvalidPackageInfo.

Consider relaxing the parser for tarballs while still enforcing integrity for npm, for example:

-        // integrity
-        switch (res.tag) {
-            .npm, .local_tarball, .remote_tarball => {
-                if (i >= pkg_info.len) {
-                    try log.addError(source, value.loc, "Missing integrity");
-                    return error.InvalidPackageInfo;
-                }
-                const integrity_expr = pkg_info.at(i);
-                i += 1;
-                const integrity_str = integrity_expr.asString(allocator) orelse {
-                    try log.addError(source, integrity_expr.loc, "Expected a string");
-                    return error.InvalidPackageInfo;
-                };
-
-                pkg.meta.integrity = Integrity.parse(integrity_str);
-            },
+        // integrity
+        switch (res.tag) {
+            .npm => {
+                if (i >= pkg_info.len) {
+                    try log.addError(source, value.loc, "Missing integrity");
+                    return error.InvalidPackageInfo;
+                }
+                const integrity_expr = pkg_info.at(i);
+                i += 1;
+                const integrity_str = integrity_expr.asString(allocator) orelse {
+                    try log.addError(source, integrity_expr.loc, "Expected a string");
+                    return error.InvalidPackageInfo;
+                };
+                pkg.meta.integrity = Integrity.parse(integrity_str);
+            },
+            .local_tarball, .remote_tarball => {
+                // Older bun.lock files did not store tarball integrity.
+                // If integrity is present, parse it; if not, leave meta.integrity at its default.
+                if (i < pkg_info.len) {
+                    const integrity_expr = pkg_info.at(i);
+                    if (integrity_expr.isString()) {
+                        const integrity_str = integrity_expr.asString(allocator).?;
+                        pkg.meta.integrity = Integrity.parse(integrity_str);
+                        i += 1;
+                    }
+                }
+            },

This preserves strictness for npm (integrity has always been present there), while allowing legacy tarball entries without integrity to be read, upgraded, and then re-written with a computed SHA‑512 value.

The Stringifier changes (emitting integrity for tarballs using pkg_meta.integrity and importing Integrity at the bottom) are otherwise correct.

Also applies to: 585-622, 1855-1869, 2224-2227

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 9ca8de6 and e2f5857.

📒 Files selected for processing (9)
  • src/install/PackageInstaller.zig (1 hunks)
  • src/install/PackageManager/PackageManagerEnqueue.zig (6 hunks)
  • src/install/PackageManager/processDependencyList.zig (1 hunks)
  • src/install/extract_tarball.zig (2 hunks)
  • src/install/install.zig (2 hunks)
  • src/install/integrity.zig (1 hunks)
  • src/install/isolated_install.zig (1 hunks)
  • src/install/lockfile/bun.lock.zig (3 hunks)
  • test/regression/issue/tarball-integrity.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
src/**/*.{cpp,zig}

📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)

src/**/*.{cpp,zig}: Use bun bd or bun run build:debug to build debug versions for C++ and Zig source files; creates debug build at ./build/debug/bun-debug
Run tests using bun bd test <test-file> with the debug build; never use bun test directly as it will not include your changes
Execute files using bun bd <file> <...args>; never use bun <file> directly as it will not include your changes
Enable debug logs for specific scopes using BUN_DEBUG_$(SCOPE)=1 environment variable
Code generation happens automatically as part of the build process; no manual code generation commands are required

Files:

  • src/install/PackageManager/processDependencyList.zig
  • src/install/integrity.zig
  • src/install/isolated_install.zig
  • src/install/lockfile/bun.lock.zig
  • src/install/install.zig
  • src/install/extract_tarball.zig
  • src/install/PackageInstaller.zig
  • src/install/PackageManager/PackageManagerEnqueue.zig
src/**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)

Use bun.Output.scoped(.${SCOPE}, .hidden) for creating debug logs in Zig code

Implement core functionality in Zig, typically in its own directory in src/

src/**/*.zig: Zig source code should be organized in src/*.zig for core runtime, JavaScript bindings, and package manager
In Zig code, be careful with allocators and use defer for cleanup to manage memory properly
Use BUN_DEBUG_QUIET_LOGS=1 to disable debug logging, or BUN_DEBUG_<scopeName>=1 to enable specific Output.scoped calls in debug builds
Run bun run zig:check-all to compile the Zig code on all platforms when making platform-specific changes

src/**/*.zig: Private fields in Zig are fully supported using the # prefix: struct { #foo: u32 };
Use decl literals in Zig for declaration initialization: const decl: Decl = .{ .binding = 0, .value = 0 };
Prefer @import at the bottom of the file (auto formatter will move them automatically)

Files:

  • src/install/PackageManager/processDependencyList.zig
  • src/install/integrity.zig
  • src/install/isolated_install.zig
  • src/install/lockfile/bun.lock.zig
  • src/install/install.zig
  • src/install/extract_tarball.zig
  • src/install/PackageInstaller.zig
  • src/install/PackageManager/PackageManagerEnqueue.zig
**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)

**/*.zig: Expose generated bindings in Zig structs using pub const js = JSC.Codegen.JS<ClassName> with trait conversion methods: toJS, fromJS, and fromJSDirect
Use consistent parameter name globalObject instead of ctx in Zig constructor and method implementations
Use bun.JSError!JSValue return type for Zig methods and constructors to enable proper error handling and exception propagation
Implement resource cleanup using deinit() method that releases resources, followed by finalize() called by the GC that invokes deinit() and frees the pointer
Use JSC.markBinding(@src()) in finalize methods for debugging purposes before calling deinit()
For methods returning cached properties in Zig, declare external C++ functions using extern fn and callconv(JSC.conv) calling convention
Implement getter functions with naming pattern get<PropertyName> in Zig that accept this and globalObject parameters and return JSC.JSValue
Access JavaScript CallFrame arguments using callFrame.argument(i), check argument count with callFrame.argumentCount(), and get this with callFrame.thisValue()
For reference-counted objects, use .deref() in finalize instead of destroy() to release references to other JS objects

Files:

  • src/install/PackageManager/processDependencyList.zig
  • src/install/integrity.zig
  • src/install/isolated_install.zig
  • src/install/lockfile/bun.lock.zig
  • src/install/install.zig
  • src/install/extract_tarball.zig
  • src/install/PackageInstaller.zig
  • src/install/PackageManager/PackageManagerEnqueue.zig
src/**/*.{ts,zig,cpp}

📄 CodeRabbit inference engine (CLAUDE.md)

Always use absolute paths in file operations

Files:

  • src/install/PackageManager/processDependencyList.zig
  • src/install/integrity.zig
  • src/install/isolated_install.zig
  • src/install/lockfile/bun.lock.zig
  • src/install/install.zig
  • src/install/extract_tarball.zig
  • src/install/PackageInstaller.zig
  • src/install/PackageManager/PackageManagerEnqueue.zig
src/**/*.{ts,zig}

📄 CodeRabbit inference engine (CLAUDE.md)

Avoid shell commands in code - don't use find or grep; use Bun's Glob and built-in tools instead

Files:

  • src/install/PackageManager/processDependencyList.zig
  • src/install/integrity.zig
  • src/install/isolated_install.zig
  • src/install/lockfile/bun.lock.zig
  • src/install/install.zig
  • src/install/extract_tarball.zig
  • src/install/PackageInstaller.zig
  • src/install/PackageManager/PackageManagerEnqueue.zig
test/**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

test/**/*.{js,ts,jsx,tsx}: Write tests as JavaScript and TypeScript files using Jest-style APIs (test, describe, expect) and import from bun:test
Use test.each and data-driven tests to reduce boilerplate when testing multiple similar cases

Files:

  • test/regression/issue/tarball-integrity.test.ts
test/regression/issue/**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

When a test is for a specific numbered GitHub Issue, place it in test/regression/issue/${issueNumber}.test.ts with a REAL issue number, not a placeholder

Files:

  • test/regression/issue/tarball-integrity.test.ts
**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.test.{ts,tsx}: For single-file tests, prefer using -e flag over tempDir
For multi-file tests, prefer using tempDir from harness and Bun.spawn over other temporary directory creation methods
Always use port: 0 for network tests and do not hardcode ports or use custom random port number functions
Use normalizeBunSnapshot to normalize snapshot output of tests
Never write tests that check for no 'panic' or 'uncaught exception' or similar in the test output - that is NOT a valid test
Use tempDir from harness to create temporary directories, do not use tmpdirSync or fs.mkdtempSync
When spawning processes in tests, check stdout/stderr expectations BEFORE checking exit code to get more useful error messages on test failure
Do not write flaky tests - do not use setTimeout in tests, instead await the condition to be met as you are testing the CONDITION not the TIME PASSING
Verify your test fails with USE_SYSTEM_BUN=1 bun test <file> and passes with bun bd test <file> - your test is NOT VALID if it passes with USE_SYSTEM_BUN=1

Files:

  • test/regression/issue/tarball-integrity.test.ts
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Use bun:test with files that end in *.test.{ts,js,jsx,tsx,mjs,cjs}
Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time
Never use hardcoded port numbers in tests. Always use port: 0 to get a random port
Prefer concurrent tests over sequential tests using test.concurrent or describe.concurrent when multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
When spawning Bun processes in tests, use bunExe and bunEnv from harness to ensure the same build of Bun is used and debug logging is silenced
Use -e flag for single-file tests when spawning Bun processes
Use tempDir() from harness to create temporary directories with files for multi-file tests instead of creating files manually
Prefer async/await over callbacks in tests
When callbacks must be used and it's just a single callback, use Promise.withResolvers to create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
Use Buffer.alloc(count, fill).toString() instead of 'A'.repeat(count) to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Use describe blocks for grouping related tests
Always use await using or using to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Always check exit codes and test error scenarios in error tests
Use describe.each() for parameterized tests
Use toMatchSnapshot() for snapshot testing
Use beforeAll(), afterEach(), beforeEach() for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup in afterEach()

Files:

  • test/regression/issue/tarball-integrity.test.ts
test/regression/issue/**/*.test.ts

📄 CodeRabbit inference engine (test/CLAUDE.md)

Regression tests for specific issues go in /test/regression/issue/${issueNumber}.test.ts. Do not put tests without issue numbers in the regression directory

Files:

  • test/regression/issue/tarball-integrity.test.ts
🧠 Learnings (31)
📓 Common learnings
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24880
File: packages/bun-vscode/package.json:382-385
Timestamp: 2025-11-20T19:51:32.288Z
Learning: In the Bun repository, dependencies may be explicitly added to package.json files (even when not directly imported in code) to force version upgrades on transitive dependencies, particularly as part of Aikido security scanner remediation to ensure vulnerable transitive dependencies resolve to patched versions.
📚 Learning: 2025-11-24T18:35:08.612Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.612Z
Learning: Applies to test/bake/dev/bundle.test.ts : Organize bundle tests in bundle.test.ts for tests concerning bundling bugs that only occur in DevServer

Applied to files:

  • test/regression/issue/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:36:33.069Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:33.069Z
Learning: Applies to test/js/bun/**/*.test.{ts,tsx} : For Bun-specific API tests, use the `test/js/bun/` directory (for http, crypto, ffi, shell, etc.)

Applied to files:

  • test/regression/issue/tarball-integrity.test.ts
  • src/install/PackageManager/PackageManagerEnqueue.zig
📚 Learning: 2025-11-24T18:36:33.069Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:33.069Z
Learning: Applies to **/*.test.{ts,tsx} : Verify your test fails with `USE_SYSTEM_BUN=1 bun test <file>` and passes with `bun bd test <file>` - your test is NOT VALID if it passes with `USE_SYSTEM_BUN=1`

Applied to files:

  • test/regression/issue/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output

Applied to files:

  • test/regression/issue/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:35:08.612Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.612Z
Learning: Applies to test/bake/dev/sourcemap.test.ts : Organize source-map tests in sourcemap.test.ts for tests verifying source-maps are correct

Applied to files:

  • test/regression/issue/tarball-integrity.test.ts
📚 Learning: 2025-10-19T02:44:46.354Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.

Applied to files:

  • test/regression/issue/tarball-integrity.test.ts
  • src/install/PackageManager/PackageManagerEnqueue.zig
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/regression/issue/**/*.test.ts : Regression tests for specific issues go in `/test/regression/issue/${issueNumber}.test.ts`. Do not put tests without issue numbers in the regression directory

Applied to files:

  • test/regression/issue/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:35:08.612Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.612Z
Learning: Applies to test/bake/dev/ecosystem.test.ts : Organize ecosystem tests in ecosystem.test.ts for tests involving ensuring certain libraries are correct; prefer testing concrete bugs over testing entire packages

Applied to files:

  • test/regression/issue/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:36:33.069Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:33.069Z
Learning: Applies to **/*.test.{ts,tsx} : Use `normalizeBunSnapshot` to normalize snapshot output of tests

Applied to files:

  • test/regression/issue/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:35:08.612Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.612Z
Learning: Applies to test/bake/dev/esm.test.ts : Organize ESM tests in esm.test.ts for tests about various ESM features in development mode

Applied to files:

  • test/regression/issue/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : When spawning Bun processes in tests, use `bunExe` and `bunEnv` from `harness` to ensure the same build of Bun is used and debug logging is silenced

Applied to files:

  • test/regression/issue/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:35:39.205Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-11-24T18:35:39.205Z
Learning: Add tests for new Bun runtime functionality

Applied to files:

  • test/regression/issue/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:35:50.422Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: Applies to test/cli/**/*.{js,ts,jsx,tsx} : When testing Bun as a CLI, use the `spawn` API from `bun` with the `bunExe()` and `bunEnv` from `harness` to execute Bun commands and validate exit codes, stdout, and stderr

Applied to files:

  • test/regression/issue/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:35:50.422Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: Applies to test/**/*.{js,ts,jsx,tsx} : Write tests as JavaScript and TypeScript files using Jest-style APIs (`test`, `describe`, `expect`) and import from `bun:test`

Applied to files:

  • test/regression/issue/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Unit tests for specific features are organized by module (e.g., `/test/js/bun/`, `/test/js/node/`)

Applied to files:

  • test/regression/issue/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Ensure V8 API tests compare identical C++ code output between Node.js and Bun through the test suite validation process

Applied to files:

  • test/regression/issue/tarball-integrity.test.ts
📚 Learning: 2025-11-20T19:51:32.288Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24880
File: packages/bun-vscode/package.json:382-385
Timestamp: 2025-11-20T19:51:32.288Z
Learning: In the Bun repository, dependencies may be explicitly added to package.json files (even when not directly imported in code) to force version upgrades on transitive dependencies, particularly as part of Aikido security scanner remediation to ensure vulnerable transitive dependencies resolve to patched versions.

Applied to files:

  • src/install/lockfile/bun.lock.zig
  • src/install/PackageManager/PackageManagerEnqueue.zig
📚 Learning: 2025-11-24T18:36:08.558Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-11-24T18:36:08.558Z
Learning: Applies to src/bun.js/bindings/generated_classes_list.zig : Include new class bindings in `src/bun.js/bindings/generated_classes_list.zig` to register them with the code generator

Applied to files:

  • src/install/lockfile/bun.lock.zig
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/src/napi/napi.zig : For each new V8 C++ method, add both GCC/Clang and MSVC mangled symbol names to the V8API struct in src/napi/napi.zig using extern fn declarations

Applied to files:

  • src/install/lockfile/bun.lock.zig
📚 Learning: 2025-11-24T18:35:39.205Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-11-24T18:35:39.205Z
Learning: Applies to src/**/*.zig : Implement core functionality in Zig, typically in its own directory in `src/`

Applied to files:

  • src/install/install.zig
📚 Learning: 2025-08-30T09:09:18.384Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 22231
File: src/bundler/bundle_v2.zig:48-48
Timestamp: 2025-08-30T09:09:18.384Z
Learning: In Zig, when a module exports a top-level struct, import("./Module.zig") directly returns that struct type and can be used as a type alias without needing to access a field within the module. This is a common pattern in the Bun codebase.

Applied to files:

  • src/install/install.zig
📚 Learning: 2025-11-24T18:36:08.558Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-11-24T18:36:08.558Z
Learning: Applies to **/*.zig : Expose generated bindings in Zig structs using `pub const js = JSC.Codegen.JS<ClassName>` with trait conversion methods: `toJS`, `fromJS`, and `fromJSDirect`

Applied to files:

  • src/install/install.zig
📚 Learning: 2025-11-24T18:36:38.104Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:38.104Z
Learning: Applies to src/**/*.zig : Prefer `import` at the bottom of the file (auto formatter will move them automatically)

Applied to files:

  • src/install/install.zig
📚 Learning: 2025-11-24T18:36:38.104Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:38.104Z
Learning: Applies to src/**/*.zig : Private fields in Zig are fully supported using the `#` prefix: `struct { #foo: u32 };`

Applied to files:

  • src/install/install.zig
📚 Learning: 2025-11-24T18:35:39.205Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-11-24T18:35:39.205Z
Learning: Applies to src/bun.js/api/BunObject.zig : Implement getter functions in `src/bun.js/api/BunObject.zig` that return your feature, and export them in the `exportAll()` function

Applied to files:

  • src/install/install.zig
  • src/install/PackageManager/PackageManagerEnqueue.zig
📚 Learning: 2025-11-24T18:35:39.205Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-11-24T18:35:39.205Z
Learning: Applies to src/**/index.zig : Create an `index.zig` entry point in your module directory that exports all module functionality

Applied to files:

  • src/install/install.zig
📚 Learning: 2025-11-24T18:36:33.069Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:33.069Z
Learning: Applies to src/**/*.zig : Zig source code should be organized in `src/*.zig` for core runtime, JavaScript bindings, and package manager

Applied to files:

  • src/install/install.zig
📚 Learning: 2025-09-02T17:14:46.924Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22227
File: src/safety/alloc.zig:93-95
Timestamp: 2025-09-02T17:14:46.924Z
Learning: In bun's Zig codebase, they use a custom extension of Zig that supports private field syntax with the `#` prefix (e.g., `#allocator`, `#trace`). This is not standard Zig syntax but is valid in their custom implementation. Fields prefixed with `#` are private fields that cannot be accessed from outside the defining struct.

Applied to files:

  • src/install/install.zig
📚 Learning: 2025-11-24T18:35:39.205Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-11-24T18:35:39.205Z
Learning: Applies to **/js_*.zig : Implement `JSYourFeature` struct in a file like `js_your_feature.zig` to create JavaScript bindings for Zig functionality

Applied to files:

  • src/install/install.zig
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Write JS builtins for Bun's Node.js compatibility and APIs, and run `bun bd` after changes

Applied to files:

  • src/install/PackageManager/PackageManagerEnqueue.zig
🧬 Code graph analysis (1)
test/regression/issue/tarball-integrity.test.ts (1)
test/harness.ts (2)
  • tempDir (277-284)
  • bunExe (102-105)
🔇 Additional comments (8)
src/install/PackageManager/PackageManagerEnqueue.zig (3)

126-157: Integrity propagation from enqueueTarballForReading to enqueueLocalTarball looks correct; consider behavior with conflicting integrities for same path

Threading integrity: Integrity into enqueueTarballForReading and passing it through to enqueueLocalTarball is consistent with the new .tarball handling and ensures the local tarball task has access to the integrity when scheduled.

One behavioral nuance: because the task queue is keyed only by Task.Id.forTarball(path) and you only call enqueueLocalTarball when !task_queue.found_existing, the first integrity seen for a given tarball path wins; later calls for the same path will reuse the existing task and silently ignore any different integrity value. If it’s possible (even due to user‑edited lockfiles) to end up with multiple Resolutions or dependencies pointing at the same tarball path but with differing integrity values, you may want to assert or at least log in debug builds when that invariant is violated, rather than silently preferring the first one.

If your design guarantees “one path → one integrity” this is fine as‑is, but it’s worth making that invariant explicit in comments or docs.


1293-1333: Local tarball task payload now includes integrity; plumbing looks consistent

Adding the integrity: Integrity parameter to enqueueLocalTarball and wiring it through to .local_tarball.tarball.integrity in the task request cleanly connects the enqueue sites (both first-install .{} and enqueueTarballForReading with a real value) to the worker that will actually open/verify the tarball.

Assuming Integrity is cheap to copy and its backing data outlives the task (which is consistent with how other fields like resolution are handled), this looks correct.


1888-1896: Integrity import alias is correctly added alongside other install types

const Integrity = bun.install.Integrity; is placed with the other bun.install.* aliases and keeps the public surface of this module consistent.

src/install/integrity.zig (1)

183-190: Integrity.compute implementation is consistent with existing hashing paths

SHA-512 digest computation matches the verifyByTag pattern and correctly initializes Integrity with tag = .sha512 and a zeroed buffer. No issues here.

src/install/isolated_install.zig (1)

1083-1089: Isolated install now correctly threads integrity into local tarball enqueue

Passing lockfile.packages.items(.meta)[pkg_id].integrity into enqueueTarballForReading aligns the isolated install flow with the main installer’s integrity handling. This looks correct.

src/install/PackageManager/processDependencyList.zig (1)

178-193: Persisting computed tarball integrity into package meta is correct

Assigning package.meta.integrity = data.computed_integrity for .local_tarball and .remote_tarball ensures the lockfile stores a stable integrity hash derived during extraction, which is exactly what you need for future verification.

src/install/extract_tarball.zig (1)

13-36: run()’s integrity computation and propagation match the intended semantics

  • If a supported integrity is provided (npm/lockfile), you verify against it and carry it through as computed_integrity.
  • If not, you compute a new SHA‑512 digest over the compressed tarball bytes and store it in computed_integrity, while skipping verification on this first install.
  • The extraction result always carries computed_integrity, enabling later lockfile persistence and verification.

This is aligned with npm-style integrity handling and looks solid.

src/install/install.zig (1)

205-215: Adding computed_integrity to ExtractData and re-exporting Integrity is appropriate

  • ExtractData.computed_integrity: Integrity = .{} cleanly carries the extraction-time hash without breaking existing call sites.
  • Re-exporting Integrity from this module matches existing patterns (e.g., Resolution, Store) and simplifies use as bun.install.Integrity.

No issues here.

Also applies to: 278-279

Comment on lines +1 to +7
import { describe, expect, setDefaultTimeout, test } from "bun:test";
import { mkdirSync, readFileSync, writeFileSync } from "fs";
import { bunEnv, bunExe, tempDir } from "harness";
import { join } from "path";

setDefaultTimeout(60_000); // 60 second timeout

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.

⚠️ Potential issue | 🟡 Minor

Test placement and timeout usage don’t fully match the stated guidelines

Two minor points:

  1. Regression test location/name
    The guidelines say regression tests for specific issues should live in test/regression/issue/${issueNumber}.test.ts with a real GitHub issue number, and that tests without issue numbers shouldn’t go in the regression directory. Here the file is test/regression/issue/tarball-integrity.test.ts without an issue number.
    If this is tied to a specific issue, consider renaming it accordingly (e.g. test/regression/issue/25264.test.ts). If it’s a general feature test, it may fit better under a module-focused directory (e.g. test/js/bun/...).

  2. setDefaultTimeout
    The test guidelines explicitly say not to set custom timeouts; Bun’s test runner manages timeouts itself. Since you’re already using timeout options on Bun.spawnSync calls, the global setDefaultTimeout(60_000) can likely be dropped.

Also applies to: 19-19

🤖 Prompt for AI Agents
In test/regression/issue/tarball-integrity.test.ts around lines 1 to 7, the test
file name and global timeout usage do not follow repo guidelines: rename the
file to include the actual GitHub issue number (e.g.
test/regression/issue/25264.test.ts) if this is a regression tied to an issue,
or move it out of test/regression/issue into the appropriate module-focused
directory (e.g. test/js/bun/...) if it’s a general feature test; and remove the
setDefaultTimeout(60_000) line so the test runner’s default timeout is used
(rely on explicit timeout options already passed to Bun.spawnSync calls
instead).

Comment on lines +21 to +166
using dir = tempDir("tarball-integrity-test", {});

// Create a simple test tarball with npm-style 'package' directory
const tarballDir = join(String(dir), "package");
mkdirSync(tarballDir, { recursive: true });
writeFileSync(join(tarballDir, "package.json"), JSON.stringify({ name: "local-pkg", version: "1.0.0" }));

// Create the tarball in the project directory
const projectDir = join(String(dir), "project");
mkdirSync(projectDir, { recursive: true });

const tarResult = Bun.spawnSync({
cmd: ["tar", "-czf", join(projectDir, "local-pkg.tgz"), "-C", String(dir), "package"],
cwd: String(dir),
});
expect(tarResult.exitCode).toBe(0);

// Create a project that depends on the local tarball
writeFileSync(
join(projectDir, "package.json"),
JSON.stringify({
name: "test-project",
dependencies: {
"local-pkg": "local-pkg.tgz",
},
}),
);

// Run bun install
const installResult = Bun.spawnSync({
cmd: [bunExe(), "install"],
cwd: projectDir,
env: {
...bunEnv,
BUN_INSTALL_CACHE_DIR: join(projectDir, ".bun-cache"),
},
timeout: 30000,
});

if (installResult.exitCode !== 0) {
console.log("stdout:", installResult.stdout.toString());
console.log("stderr:", installResult.stderr.toString());
}
expect(installResult.exitCode).toBe(0);

// Read the lockfile and verify it contains the integrity hash
const lockfilePath = join(projectDir, "bun.lock");
const lockfileContent = readFileSync(lockfilePath, "utf8");

// The lockfile should contain an integrity hash for the local tarball
expect(lockfileContent).toContain("sha512-");
expect(lockfileContent).toContain("local-pkg.tgz");
});

test("integrity verification fails when tarball content changes", async () => {
using dir = tempDir("tarball-integrity-test", {});

// Create version 1 tarball
const v1Dir = join(String(dir), "v1", "package");
mkdirSync(v1Dir, { recursive: true });
writeFileSync(join(v1Dir, "package.json"), JSON.stringify({ name: "pkg", version: "1.0.0" }));
Bun.spawnSync({
cmd: ["tar", "-czf", join(String(dir), "v1.tgz"), "-C", join(String(dir), "v1"), "package"],
cwd: String(dir),
});

// Create version 2 tarball (different content)
const v2Dir = join(String(dir), "v2", "package");
mkdirSync(v2Dir, { recursive: true });
writeFileSync(
join(v2Dir, "package.json"),
JSON.stringify({ name: "pkg", version: "2.0.0", description: "changed" }),
);
Bun.spawnSync({
cmd: ["tar", "-czf", join(String(dir), "v2.tgz"), "-C", join(String(dir), "v2"), "package"],
cwd: String(dir),
});

// Create project
const projectDir = join(String(dir), "project");
mkdirSync(projectDir, { recursive: true });
writeFileSync(
join(projectDir, "package.json"),
JSON.stringify({
name: "test-project",
dependencies: {
pkg: `file:../v1.tgz`,
},
}),
);

// First install - should succeed and create lockfile with integrity
const firstInstall = Bun.spawnSync({
cmd: [bunExe(), "install"],
cwd: projectDir,
env: {
...bunEnv,
BUN_INSTALL_CACHE_DIR: join(projectDir, ".bun-cache"),
},
timeout: 30000,
});

expect(firstInstall.exitCode).toBe(0);

// Verify lockfile has integrity
const lockfileContent = readFileSync(join(projectDir, "bun.lock"), "utf8");
expect(lockfileContent).toContain("sha512-");

// Now modify the lockfile to point to v2.tgz but keep the v1 integrity
// This simulates an attacker swapping the tarball content
const modifiedLockfile = lockfileContent.replace(/v1\.tgz/g, "v2.tgz");
writeFileSync(join(projectDir, "bun.lock"), modifiedLockfile);

// Update package.json to match
writeFileSync(
join(projectDir, "package.json"),
JSON.stringify({
name: "test-project",
dependencies: {
pkg: `file:../v2.tgz`,
},
}),
);

// Clean cache and node_modules to force re-extraction
Bun.spawnSync({
cmd: ["rm", "-rf", "node_modules", ".bun-cache"],
cwd: projectDir,
});

// Second install with different tarball content should fail integrity check
const secondInstall = Bun.spawnSync({
cmd: [bunExe(), "install"],
cwd: projectDir,
env: {
...bunEnv,
BUN_INSTALL_CACHE_DIR: join(projectDir, ".bun-cache"),
},
timeout: 30000,
});

// The install should fail because the integrity check failed
expect(secondInstall.exitCode).not.toBe(0);
const stderr = secondInstall.stderr.toString();
expect(stderr).toContain("Integrity check failed");
});

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.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Check for existing tar and rm usage in tests
rg -nP 'Bun\.spawnSync\(\s*{\s*cmd:\s*\["tar"' test || echo "No tar usage found"
rg -nP 'Bun\.spawnSync\(\s*{\s*cmd:\s*\["rm' test || echo "No rm usage found"

Repository: oven-sh/bun

Length of output: 91


🏁 Script executed:

# Check for Windows-specific guards in tests
rg -n 'process\.platform|skip.*[Ww]indows|win32' test --type ts --type js | head -20

Repository: oven-sh/bun

Length of output: 2256


🏁 Script executed:

# Check the specific test file mentioned in the review
cat -n test/regression/issue/tarball-integrity.test.ts | head -180

Repository: oven-sh/bun

Length of output: 6902


🏁 Script executed:

# Check if the test file has any Windows skips
grep -n "skipIf\|skip.*[Ww]indows" test/regression/issue/tarball-integrity.test.ts || echo "No Windows skips found"

Repository: oven-sh/bun

Length of output: 77


🏁 Script executed:

# Check if there's a describe or test.skip pattern at the top
head -30 test/regression/issue/tarball-integrity.test.ts

Repository: oven-sh/bun

Length of output: 1385


Fix missing exit-code assertions and add error diagnostics for shell operations

The test scenarios are sound, but the second test has gaps in error handling that make failures harder to diagnose:

  • Lines 82, 94 (tar commands for v1/v2 tarballs): Missing exitCode checks. Unlike the first test's tar call (line 36), these don't verify success. If tar fails or is unavailable, subsequent steps will fail with confusing diagnostics.
  • Line 146 (rm -rf cleanup): No exitCode assertion. If cleanup fails silently, the second bun install result is unreliable.
  • No error logging: Unlike the bun install failures (lines 60–62), tar/rm failures won't log stderr, making it hard to distinguish between platform issues and actual integrity failures.

Add exitCode checks and error logging for both tar calls and the rm call in the second test, similar to the pattern used for bun install.

Additionally, consider whether this test should skip on Windows (where tar and rm may not be available or behave differently). Check your CI configuration to determine if Windows runs are expected.

🤖 Prompt for AI Agents
In test/regression/issue/tarball-integrity.test.ts around lines 21-166, the
second test is missing exitCode assertions and stderr logging for the tar
commands (v1/v2 at ~lines 82 and 94) and the cleanup rm -rf (around line 146),
which can hide failures; add checks that each Bun.spawnSync call returns
exitCode === 0 and, if not, log stdout/stderr like the existing bun install
block, for the two tar spawns and the rm spawn; mirror the pattern used earlier
to print installResult.stdout/stderr when non-zero; also consider skipping the
test on Windows when tar/rm are unavailable.

… quality

- Move test from test/regression/issue/ to test/cli/install/
- Use tempDirWithFiles and pack helper from harness
- Use fs/promises rm instead of spawning rm command
- Remove setDefaultTimeout
- Use replaceAll for lockfile modification
- Add verification that lockfile was properly modified

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 2

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 4366e68 and 2f10b71.

📒 Files selected for processing (1)
  • test/cli/install/tarball-integrity.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
test/**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

test/**/*.{js,ts,jsx,tsx}: Write tests as JavaScript and TypeScript files using Jest-style APIs (test, describe, expect) and import from bun:test
Use test.each and data-driven tests to reduce boilerplate when testing multiple similar cases

Files:

  • test/cli/install/tarball-integrity.test.ts
test/cli/**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

When testing Bun as a CLI, use the spawn API from bun with the bunExe() and bunEnv from harness to execute Bun commands and validate exit codes, stdout, and stderr

Files:

  • test/cli/install/tarball-integrity.test.ts
test/cli/**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

For CLI command tests, use the test/cli/ directory (install, run, test, etc.)

Files:

  • test/cli/install/tarball-integrity.test.ts
**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.test.{ts,tsx}: For single-file tests, prefer using -e flag over tempDir
For multi-file tests, prefer using tempDir from harness and Bun.spawn over other temporary directory creation methods
Always use port: 0 for network tests and do not hardcode ports or use custom random port number functions
Use normalizeBunSnapshot to normalize snapshot output of tests
Never write tests that check for no 'panic' or 'uncaught exception' or similar in the test output - that is NOT a valid test
Use tempDir from harness to create temporary directories, do not use tmpdirSync or fs.mkdtempSync
When spawning processes in tests, check stdout/stderr expectations BEFORE checking exit code to get more useful error messages on test failure
Do not write flaky tests - do not use setTimeout in tests, instead await the condition to be met as you are testing the CONDITION not the TIME PASSING
Verify your test fails with USE_SYSTEM_BUN=1 bun test <file> and passes with bun bd test <file> - your test is NOT VALID if it passes with USE_SYSTEM_BUN=1

Files:

  • test/cli/install/tarball-integrity.test.ts
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Use bun:test with files that end in *.test.{ts,js,jsx,tsx,mjs,cjs}
Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time
Never use hardcoded port numbers in tests. Always use port: 0 to get a random port
Prefer concurrent tests over sequential tests using test.concurrent or describe.concurrent when multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
When spawning Bun processes in tests, use bunExe and bunEnv from harness to ensure the same build of Bun is used and debug logging is silenced
Use -e flag for single-file tests when spawning Bun processes
Use tempDir() from harness to create temporary directories with files for multi-file tests instead of creating files manually
Prefer async/await over callbacks in tests
When callbacks must be used and it's just a single callback, use Promise.withResolvers to create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
Use Buffer.alloc(count, fill).toString() instead of 'A'.repeat(count) to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Use describe blocks for grouping related tests
Always use await using or using to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Always check exit codes and test error scenarios in error tests
Use describe.each() for parameterized tests
Use toMatchSnapshot() for snapshot testing
Use beforeAll(), afterEach(), beforeEach() for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup in afterEach()

Files:

  • test/cli/install/tarball-integrity.test.ts
🧠 Learnings (13)
📓 Common learnings
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24880
File: packages/bun-vscode/package.json:382-385
Timestamp: 2025-11-20T19:51:32.288Z
Learning: In the Bun repository, dependencies may be explicitly added to package.json files (even when not directly imported in code) to force version upgrades on transitive dependencies, particularly as part of Aikido security scanner remediation to ensure vulnerable transitive dependencies resolve to patched versions.
📚 Learning: 2025-11-24T18:36:33.069Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:33.069Z
Learning: Applies to test/js/bun/**/*.test.{ts,tsx} : For Bun-specific API tests, use the `test/js/bun/` directory (for http, crypto, ffi, shell, etc.)

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-10-19T02:44:46.354Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:36:33.069Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:33.069Z
Learning: Applies to **/*.test.{ts,tsx} : Verify your test fails with `USE_SYSTEM_BUN=1 bun test <file>` and passes with `bun bd test <file>` - your test is NOT VALID if it passes with `USE_SYSTEM_BUN=1`

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : When spawning Bun processes in tests, use `bunExe` and `bunEnv` from `harness` to ensure the same build of Bun is used and debug logging is silenced

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:35:39.205Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-11-24T18:35:39.205Z
Learning: Add tests for new Bun runtime functionality

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:35:50.422Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: Applies to test/cli/**/*.{js,ts,jsx,tsx} : When testing Bun as a CLI, use the `spawn` API from `bun` with the `bunExe()` and `bunEnv` from `harness` to execute Bun commands and validate exit codes, stdout, and stderr

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:35:08.612Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.612Z
Learning: Applies to test/bake/dev/bundle.test.ts : Organize bundle tests in bundle.test.ts for tests concerning bundling bugs that only occur in DevServer

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` with files that end in `*.test.{ts,js,jsx,tsx,mjs,cjs}`

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Ensure V8 API tests compare identical C++ code output between Node.js and Bun through the test suite validation process

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Unit tests for specific features are organized by module (e.g., `/test/js/bun/`, `/test/js/node/`)

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:35:50.422Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: Applies to test/**/*.{js,ts,jsx,tsx} : Write tests as JavaScript and TypeScript files using Jest-style APIs (`test`, `describe`, `expect`) and import from `bun:test`

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
🧬 Code graph analysis (1)
test/cli/install/tarball-integrity.test.ts (1)
test/harness.ts (3)
  • tempDirWithFiles (259-266)
  • pack (1304-1326)
  • bunExe (102-105)

Per review feedback, tests should not assert absence of "panic" - they
should rely on exit code and content assertions instead.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 2f10b71 and a42c326.

📒 Files selected for processing (1)
  • test/cli/install/tarball-integrity.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
test/**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

test/**/*.{js,ts,jsx,tsx}: Write tests as JavaScript and TypeScript files using Jest-style APIs (test, describe, expect) and import from bun:test
Use test.each and data-driven tests to reduce boilerplate when testing multiple similar cases

Files:

  • test/cli/install/tarball-integrity.test.ts
test/cli/**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

When testing Bun as a CLI, use the spawn API from bun with the bunExe() and bunEnv from harness to execute Bun commands and validate exit codes, stdout, and stderr

Files:

  • test/cli/install/tarball-integrity.test.ts
test/cli/**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

For CLI command tests, use the test/cli/ directory (install, run, test, etc.)

Files:

  • test/cli/install/tarball-integrity.test.ts
**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.test.{ts,tsx}: For single-file tests, prefer using -e flag over tempDir
For multi-file tests, prefer using tempDir from harness and Bun.spawn over other temporary directory creation methods
Always use port: 0 for network tests and do not hardcode ports or use custom random port number functions
Use normalizeBunSnapshot to normalize snapshot output of tests
Never write tests that check for no 'panic' or 'uncaught exception' or similar in the test output - that is NOT a valid test
Use tempDir from harness to create temporary directories, do not use tmpdirSync or fs.mkdtempSync
When spawning processes in tests, check stdout/stderr expectations BEFORE checking exit code to get more useful error messages on test failure
Do not write flaky tests - do not use setTimeout in tests, instead await the condition to be met as you are testing the CONDITION not the TIME PASSING
Verify your test fails with USE_SYSTEM_BUN=1 bun test <file> and passes with bun bd test <file> - your test is NOT VALID if it passes with USE_SYSTEM_BUN=1

Files:

  • test/cli/install/tarball-integrity.test.ts
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Use bun:test with files that end in *.test.{ts,js,jsx,tsx,mjs,cjs}
Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time
Never use hardcoded port numbers in tests. Always use port: 0 to get a random port
Prefer concurrent tests over sequential tests using test.concurrent or describe.concurrent when multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
When spawning Bun processes in tests, use bunExe and bunEnv from harness to ensure the same build of Bun is used and debug logging is silenced
Use -e flag for single-file tests when spawning Bun processes
Use tempDir() from harness to create temporary directories with files for multi-file tests instead of creating files manually
Prefer async/await over callbacks in tests
When callbacks must be used and it's just a single callback, use Promise.withResolvers to create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
Use Buffer.alloc(count, fill).toString() instead of 'A'.repeat(count) to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Use describe blocks for grouping related tests
Always use await using or using to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Always check exit codes and test error scenarios in error tests
Use describe.each() for parameterized tests
Use toMatchSnapshot() for snapshot testing
Use beforeAll(), afterEach(), beforeEach() for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup in afterEach()

Files:

  • test/cli/install/tarball-integrity.test.ts
🧠 Learnings (21)
📓 Common learnings
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24880
File: packages/bun-vscode/package.json:382-385
Timestamp: 2025-11-20T19:51:32.288Z
Learning: In the Bun repository, dependencies may be explicitly added to package.json files (even when not directly imported in code) to force version upgrades on transitive dependencies, particularly as part of Aikido security scanner remediation to ensure vulnerable transitive dependencies resolve to patched versions.
📚 Learning: 2025-11-24T18:36:33.069Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:33.069Z
Learning: Applies to test/js/bun/**/*.test.{ts,tsx} : For Bun-specific API tests, use the `test/js/bun/` directory (for http, crypto, ffi, shell, etc.)

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-10-19T02:44:46.354Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:36:33.069Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:33.069Z
Learning: Applies to **/*.test.{ts,tsx} : Verify your test fails with `USE_SYSTEM_BUN=1 bun test <file>` and passes with `bun bd test <file>` - your test is NOT VALID if it passes with `USE_SYSTEM_BUN=1`

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : When spawning Bun processes in tests, use `bunExe` and `bunEnv` from `harness` to ensure the same build of Bun is used and debug logging is silenced

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:35:39.205Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-11-24T18:35:39.205Z
Learning: Add tests for new Bun runtime functionality

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:35:08.612Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.612Z
Learning: Applies to test/bake/dev/bundle.test.ts : Organize bundle tests in bundle.test.ts for tests concerning bundling bugs that only occur in DevServer

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:35:50.422Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: Applies to test/cli/**/*.{js,ts,jsx,tsx} : When testing Bun as a CLI, use the `spawn` API from `bun` with the `bunExe()` and `bunEnv` from `harness` to execute Bun commands and validate exit codes, stdout, and stderr

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` with files that end in `*.test.{ts,js,jsx,tsx,mjs,cjs}`

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Ensure V8 API tests compare identical C++ code output between Node.js and Bun through the test suite validation process

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:36:33.069Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:33.069Z
Learning: Applies to **/*.test.{ts,tsx} : Never write tests that check for no 'panic' or 'uncaught exception' or similar in the test output - that is NOT a valid test

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-10-26T01:32:04.844Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24082
File: test/cli/test/coverage.test.ts:60-112
Timestamp: 2025-10-26T01:32:04.844Z
Learning: In the Bun repository test files (test/cli/test/*.test.ts), when spawning Bun CLI commands with Bun.spawnSync for testing, prefer using stdio: ["inherit", "inherit", "inherit"] to inherit stdio streams rather than piping them.

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:36:33.069Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:33.069Z
Learning: Applies to **/*.test.{ts,tsx} : When spawning processes in tests, check stdout/stderr expectations BEFORE checking exit code to get more useful error messages on test failure

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-09-20T00:58:38.042Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 22568
File: test/js/valkey/valkey.test.ts:561-564
Timestamp: 2025-09-20T00:58:38.042Z
Learning: For test/js/valkey/valkey.test.ts, do not comment on synchronous throw assertions for async Redis methods (like ctx.redis.set(), ctx.redis.unsubscribe(), etc.) - Bun's Redis client implementation differs from Node.js and can throw synchronously even for async methods. The maintainer has explicitly requested to stop looking at this error pattern.

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Always check exit codes and test error scenarios in error tests

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-09-20T00:57:56.685Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 22568
File: test/js/valkey/valkey.test.ts:268-276
Timestamp: 2025-09-20T00:57:56.685Z
Learning: For test/js/valkey/valkey.test.ts, do not comment on synchronous throw assertions for async Redis methods like ctx.redis.set() - the maintainer has explicitly requested to stop looking at this error pattern.

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:35:08.612Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.612Z
Learning: Applies to test/bake/dev/ecosystem.test.ts : Organize ecosystem tests in ecosystem.test.ts for tests involving ensuring certain libraries are correct; prefer testing concrete bugs over testing entire packages

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Unit tests for specific features are organized by module (e.g., `/test/js/bun/`, `/test/js/node/`)

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
📚 Learning: 2025-11-24T18:35:50.422Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: Applies to test/**/*.{js,ts,jsx,tsx} : Write tests as JavaScript and TypeScript files using Jest-style APIs (`test`, `describe`, `expect`) and import from `bun:test`

Applied to files:

  • test/cli/install/tarball-integrity.test.ts
🧬 Code graph analysis (1)
test/cli/install/tarball-integrity.test.ts (1)
test/harness.ts (3)
  • tempDirWithFiles (259-266)
  • pack (1304-1326)
  • bunExe (102-105)
🔇 Additional comments (2)
test/cli/install/tarball-integrity.test.ts (2)

1-15: LGTM: Imports and documentation are appropriate.

The imports correctly use bun:test and harness utilities, and the comment block clearly explains the test's purpose.


58-146: LGTM: Integrity verification test is well-designed.

The test correctly simulates an integrity attack scenario by:

  1. Installing a tarball (v1) and capturing its integrity hash
  2. Modifying the lockfile to point to different content (v2) while preserving v1's integrity
  3. Cleaning cache/node_modules to force re-extraction
  4. Verifying the install fails with "Integrity check failed"

The assertion ordering is correct—checking stdout/stderr for the error message before checking the exit code, which provides better error messages on test failure.

As per coding guidelines, checking stdout/stderr expectations before exit code is the recommended approach.

Claude Bot and others added 3 commits December 1, 2025 05:02
- Mark tests as concurrent to run in parallel
- Remove String() coercions since tempDirWithFiles returns string

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Make integrity hash optional for local_tarball and remote_tarball in
  lockfile parsing (may be missing in migrated lockfiles)
- Only write integrity to lockfile when it's valid (supported tag)
- Fix Windows path separator issue in test by normalizing paths to
  forward slashes when comparing against lockfile content
- Update snapshot for plaintext lockfiles test

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

@dylan-conway dylan-conway left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This doesn't seem to fix the issue. Example:

Add local/remote tarball dependency, run install, edit integrity in lockfile to make it incorrect, delete node_modules, run install again -> installs successfully.

I would expect this to fail to install because the integrity in the lockfile does not match the integrity coming from the tarball.

@github-actions

Copy link
Copy Markdown
Contributor

Closing this PR because it has been inactive for more than 90 days.

@github-actions github-actions Bot closed this Mar 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants