test(ENG-21524): Fuzzilli Stop-Gap #24826
Conversation
|
Updated 10:35 PM PT - Nov 20th, 2025
❌ @markovejnovic, your commit 8401958 has 3 failures in
🧪 To try this PR locally: bunx bun-pr 24826That installs a local version of the PR into your bun-24826 --bun |
WalkthroughAdds optional Fuzzilli support: new CMake option and flags, build wiring to propagate a Zig enable flag, a CLI command and Zig command implementation, C++ REPRL bindings with ASan coverage and reset APIs, and a JS REPRL runner that executes fuzzer-provided scripts and resets coverage. Changes
Suggested reviewers
Pre-merge checks✅ Passed checks (4 passed)
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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmake/targets/BuildBun.cmake (1)
977-1012: Broadly disabling deprecation warnings; consider narrowing scopeAdding
-Wno-deprecated-declarationshere means all deprecation diagnostics are completely suppressed in non‑Windows builds, not just treated as non‑errors, while-Werrorremains enabled.If the main goal is to unbreak fuzzilli/ASAN builds that rely on deprecated JSC or sanitizer APIs, it would be safer to either:
- Use
-Wno-error=deprecated-declarationsso you still see the warnings, or- Gate the deprecation suppression behind a fuzzilli‑specific CMake option (or
ENABLE_ASAN), instead of all configurations.This keeps the mainline build closer to existing warning hygiene while still unblocking fuzzing.
📜 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.
📒 Files selected for processing (7)
cmake/CompilerFlags.cmake(1 hunks)cmake/targets/BuildBun.cmake(2 hunks)src/bun.js/bindings/FuzzilliREPRL.cpp(1 hunks)src/bun.js/bindings/ZigGlobalObject.cpp(2 hunks)src/cli.zig(6 hunks)src/cli/fuzzilli_command.zig(1 hunks)src/js/internal/fuzzilli-reprl-minimal.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:37.256Z
Learning: In the Bun codebase, FFI (Foreign Function Interface) code is compiled separately using TinyCC (tcc), which barely supports C99. Headers like src/bun.js/api/FFI.h and src/bun.js/api/ffi-stdbool.h are only used for FFI compilation with tcc, not with the main compiler. Therefore, C standard changes to the main Bun target do not affect FFI code compilation.
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:38.580Z
Learning: In the Bun codebase, FFI is compiled with tcc (TinyCC), which barely supports C99. The headers `src/bun.js/api/FFI.h` and `src/bun.js/api/ffi-stdbool.h` are only used for FFI compilation with tcc, not for the main Bun target. Therefore, C23 compatibility concerns (such as bool/true/false keyword conflicts) do not apply to these FFI headers.
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24423
File: test/js/node/path/browserify.test.js:472-475
Timestamp: 2025-11-07T17:28:58.045Z
Learning: markovejnovic prefers functional programming patterns (e.g., using reduce) over imperative loops and does not want style-based readability suggestions unless there's a concrete technical reason beyond personal preference.
<!--
📚 Learning: 2025-10-15T20:19:37.256Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:37.256Z
Learning: In the Bun codebase, FFI (Foreign Function Interface) code is compiled separately using TinyCC (tcc), which barely supports C99. Headers like src/bun.js/api/FFI.h and src/bun.js/api/ffi-stdbool.h are only used for FFI compilation with tcc, not with the main compiler. Therefore, C standard changes to the main Bun target do not affect FFI code compilation.
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cppsrc/bun.js/bindings/FuzzilliREPRL.cppsrc/cli/fuzzilli_command.zig
📚 Learning: 2025-10-15T20:19:38.580Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:38.580Z
Learning: In the Bun codebase, FFI is compiled with tcc (TinyCC), which barely supports C99. The headers `src/bun.js/api/FFI.h` and `src/bun.js/api/ffi-stdbool.h` are only used for FFI compilation with tcc, not for the main Bun target. Therefore, C23 compatibility concerns (such as bool/true/false keyword conflicts) do not apply to these FFI headers.
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cppsrc/bun.js/bindings/FuzzilliREPRL.cppsrc/cli/fuzzilli_command.zig
📚 Learning: 2025-10-18T20:50:47.750Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:366-373
Timestamp: 2025-10-18T20:50:47.750Z
Learning: In Bun's Zig codebase (src/bun.js/bindings/JSValue.zig), the JSValue enum uses `.null` (not `.js_null`) for JavaScript's null value. Only `js_undefined` has the `js_` prefix to avoid collision with Zig's built-in `undefined` keyword. The correct enum fields are: `js_undefined`, `null`, `true`, `false`, and `zero`.
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cppsrc/bun.js/bindings/FuzzilliREPRL.cpp
📚 Learning: 2025-09-20T05:35:57.318Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 22534
File: src/bun.js/bindings/headers.h:729-731
Timestamp: 2025-09-20T05:35:57.318Z
Learning: symbols.txt in the Bun codebase is specifically for V8 API mangled symbols (without leading underscore), not for general Bun host functions declared with BUN_DECLARE_HOST_FUNCTION. Host functions are handled through different build mechanisms.
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cppsrc/bun.js/bindings/FuzzilliREPRL.cpp
📚 Learning: 2025-09-05T19:49:26.188Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: src/valkey/js_valkey_functions.zig:852-867
Timestamp: 2025-09-05T19:49:26.188Z
Learning: In Bun’s Zig code, `.js_undefined` is a valid and preferred JSValue literal for “undefined” (e.g., resolving JSPromise). Do not refactor usages to `jsc.JSValue.jsUndefined()`, especially in src/valkey/js_valkey_functions.zig unsubscribe().
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-11-03T20:43:06.996Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24273
File: src/bun.js/test/snapshot.zig:19-19
Timestamp: 2025-11-03T20:43:06.996Z
Learning: In Bun's Zig codebase, when storing JSValue objects in collections like ArrayList, use `jsc.Strong.Optional` (not raw JSValue). When adding values, wrap them with `jsc.Strong.Optional.create(value, globalThis)`. In cleanup code, iterate the collection calling `.deinit()` on each Strong.Optional item before calling `.deinit()` on the ArrayList itself. This pattern automatically handles GC protection. See examples in src/bun.js/test/ScopeFunctions.zig and src/bun.js/node/node_cluster_binding.zig.
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cppsrc/bun.js/bindings/FuzzilliREPRL.cpp
📚 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/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-11-12T04:11:52.293Z
Learnt from: cirospaciari
Repo: oven-sh/bun PR: 24622
File: src/deps/uws/us_socket_t.zig:112-113
Timestamp: 2025-11-12T04:11:52.293Z
Learning: In Bun's Zig codebase, when passing u32 values to C FFI functions that expect c_uint parameters, no explicit intCast is needed because c_uint is equivalent to u32 on Bun's target platforms and Zig allows implicit coercion between equivalent types. This pattern is used consistently throughout src/deps/uws/us_socket_t.zig in functions like setTimeout, setLongTimeout, and setKeepalive.
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-10-11T15:19:30.301Z
Learnt from: mastermakrela
Repo: oven-sh/bun PR: 19167
File: src/bun.js/api.zig:31-31
Timestamp: 2025-10-11T15:19:30.301Z
Learning: The learnings about `pub const js = JSC.Codegen.JS<ClassName>` and re-exporting toJS/fromJS/fromJSDirect apply to class-based Zig bindings with constructors and prototype methods (e.g., those with `new` constructors). They do NOT apply to simple namespace-style API objects like TOMLObject, YAMLObject, and CSVObject which expose static functions via a `create()` method that returns a plain JS object.
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-10-16T02:17:35.237Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/analytics.zig:15-21
Timestamp: 2025-10-16T02:17:35.237Z
Learning: In src/analytics.zig and similar files using bun.EnvVar boolean environment variables: the new EnvVar API for boolean flags (e.g., bun.EnvVar.do_not_track.get(), bun.EnvVar.ci.get()) is designed to parse and return boolean values from environment variables, not just check for their presence. This is an intentional design change from the previous presence-based checks using bun.getenvZ().
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-11-14T16:07:01.064Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
Applied to files:
src/bun.js/bindings/FuzzilliREPRL.cpp
📚 Learning: 2025-10-19T03:01:29.084Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:286-289
Timestamp: 2025-10-19T03:01:29.084Z
Learning: In src/bun.js/telemetry.zig, the guard preventing double configuration (lines 213-216) is intentional. The telemetry API uses a single-shot configuration model where configure() is called once during application startup. Users must call Bun.telemetry.configure(null) to reset before reconfiguring. This design ensures: (1) predictable state—callbacks don't change mid-request, avoiding race conditions; (2) zero overhead when disabled—no checking for callback changes on every request; (3) clear ownership—one adapter (e.g., bun-otel) owns the telemetry config. Merge-style reconfiguration would require atomic updates during active requests, adding overhead to the hot path.
<!-- [/add_learning]
Applied to files:
src/bun.js/bindings/FuzzilliREPRL.cpp
📚 Learning: 2025-09-30T22:53:19.887Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 23117
File: src/bun.js/test/snapshot.zig:265-276
Timestamp: 2025-09-30T22:53:19.887Z
Learning: In Bun's snapshot testing (src/bun.js/test/snapshot.zig), multiple inline snapshots at the same line and column (same call position) must have identical values. However, multiple inline snapshots on the same line at different columns are allowed to have different values. The check is position-specific (line+col), not line-wide.
Applied to files:
src/bun.js/bindings/FuzzilliREPRL.cpp
🧬 Code graph analysis (2)
src/bun.js/bindings/ZigGlobalObject.cpp (2)
src/bun.js/bindings/FuzzilliREPRL.cpp (2)
Bun__REPRL__registerFuzzilliFunction(163-183)Bun__REPRL__registerFuzzilliFunction(163-163)src/bun.js/bindings/ZigGlobalObject.h (1)
globalObject(127-127)
src/js/internal/fuzzilli-reprl-minimal.ts (1)
src/node-fallbacks/buffer.js (1)
Buffer(136-145)
🪛 Biome (2.1.2)
src/js/internal/fuzzilli-reprl-minimal.ts
[error] 58-58: eval() exposes to security risks and performance issues.
See the MDN web docs for more details.
Refactor the code so that it doesn't need to call eval().
(lint/security/noGlobalEval)
🪛 GitHub Check: Lint JavaScript
src/js/internal/fuzzilli-reprl-minimal.ts
[failure] 7-7: eslint(no-unused-vars)
Variable 'REPRL_DWFD' is declared but never used. Unused variables should start with a '_'.
🔇 Additional comments (8)
src/bun.js/bindings/ZigGlobalObject.cpp (1)
261-263: Fuzzilli registration hook looks correct; just ensure the env var contract is stableThe
Bun__REPRL__registerFuzzilliFunctionhook and its invocationif (std::getenv("BUN_FUZZILLI_MODE")) { Bun__REPRL__registerFuzzilliFunction(static_cast<Zig::GlobalObject*>(globalObject)); }are well placed:
- The call happens after the global object is created, installed as default, and GC‑protected, so the registration code can safely add the
fuzzillifunction and install signal handlers.- Gating on
BUN_FUZZILLI_MODElines up with the CLI command, which sets this env var before booting the REPRL script.Two minor points to keep in mind:
- The check treats any non‑empty value as “on”; if you later want more nuanced configuration, consider normalizing on
BUN_FUZZILLI_MODE=1/0.- This will also register
fuzzillion worker/macro globals created while the env var is set, which is probably fine but worth being aware of.Otherwise, the integration point looks good.
Also applies to: 507-510
cmake/CompilerFlags.cmake (1)
47-63: FuzzilliREPRL.cpp lacks platform guards; coverage gating is incompleteThe review correctly identifies a platform scope issue but misdiagnoses the root cause.
Actual problem: FuzzilliREPRL.cpp includes POSIX-only headers (
<unistd.h>,<fcntl.h>,<sys/mman.h>) and calls POSIXsignal()functions (lines 169-172) with no#if !OS(WINDOWS)guards. Compare with the pattern used elsewhere in the codebase (e.g.,c-bindings.cpp:94). IfENABLE_ASANis ever enabled on Windows orBUN_FUZZILLI_MODEis set, compilation will fail.What the review got wrong:
- The coverage flags themselves are not the blocker; clang-cl accepts
-fsanitize-coveragesyntax.- Claims the CLI "explicitly errors on non‑POSIX"—the actual failure would be compilation-time, not runtime.
Mitigation already in place:
ENABLE_ASANdefaults toOFFon Windows (only ON for DEBUG + APPLE aarch64 or LINUX), so accidental Windows ASAN builds are unlikely.Recommended fix:
Gating coverage flags to UNIX is reasonable for clarity, but the more critical fix is adding platform guards aroundBun__REPRL__registerFuzzilliFunction()and/or wrapping the entire fuzzilli REPRL initialization (headers + signal setup) in#if !OS(WINDOWS)guards.Likely an incorrect or invalid review comment.
src/cli.zig (3)
94-94: Fuzzilli command wiring into the CLI looks consistentThe new
FuzzilliCommandimport, root matcher case ("fuzzilli"→.FuzzilliCommand), and dispatch arm that initializes a context and callsFuzzilliCommand.exec(ctx)all follow the existing command patterns (e.g.RunCommand,TestCommand). Nothing stands out as a correctness or ergonomics issue here, and the command remains effectively hidden from everyday users because it’s not listed in the main help text or default completions.Also applies to: 630-631, 940-944
1332-1333: Help fallback for unknown commands keeps fuzzilli effectively hiddenRouting the generic
Tag.printHelpfallback toHelpCommand.printWithReason(.explicit, false)means commands without dedicated help (includingFuzzilliCommand) just show the standard top-level help, without surfacingfuzzilliitself. That matches the “internal tooling” nature of this command and avoids confusing users with an undocumented entry.
980-981: Verify FuzzilliCommand mapping was synchronized to bun.report backendThe local implementation is correct:
FuzzilliCommandis properly added to the tag enum (line 980) with the 'F' mapping inchar()(line 1018). However, the code includes an explicit comment at line 984 linking to the bun.report backend remap.ts file, confirming that external synchronization is required. Verify that the corresponding change was submitted to the bun.report repository to ensure crash reports display the correct tag.src/bun.js/bindings/FuzzilliREPRL.cpp (3)
40-147:fuzzilli()host function behavior matches typical Fuzzilli expectationsThe
functionFuzzilliimplementation covers:
FUZZILLI_CRASH: integer-typed crash selector with a variety of crash modes (abort, trap, heap/stack OOB, UAF, double free, null deref, and a “DEBUG/ASAN enabled” sanity check).FUZZILLI_PRINT: printing the second argument to stdout.Exception scopes are handled correctly around
toWTFStringcalls, and a missing/unknown command just results inundefined, which is what Fuzzilli expects from other engines. The deliberate leaks and memory errors in specific crash modes are intentional for ASAN and look appropriate.I don’t see any correctness issues here for the fuzzing use case.
Also applies to: 147-157, 159-160
283-301: ASAN-disabled stubs correctly preserve linkage without overheadThe stub implementations for
__sanitizer_cov_trace_pc_guard_init,__sanitizer_cov_trace_pc_guard, andBun__REPRL__resetCoveragewhen ASAN isn’t enabled are minimal and safe; they satisfy the linker and introduce no behavior in non-fuzzing/non-ASAN builds.
196-205: Verify that ASAN coverage flags are scoped to single module and document bitmap handling assumptionsThe coverage implementation correctly enforces single-module instrumentation—
__sanitizer_cov_trace_pc_guard_initat line 242-245 exits with_exit(-1)if called with a different region (different start/stop pointers). The bitmap concern is also valid:__sanitizer_cov_reset_edgeguards()(lines 225-232) resets only the guard values, not theedges[]bitmap itself. Coverage accumulates across iterations unless the REPRL driver clears the shared memory between runs.Before broadening
-fsanitize-coverage=trace-pc-guardbeyond the current module in future work:
- Ensure coverage compilation flags are explicitly scoped to only the instrumented module (e.g., via build system constraints or target-specific flags).
- Document for the Fuzzilli harness that the shared bitmap is not reset by
Bun__REPRL__resetCoverage()and must be zeroed externally between iterations.
| const REPRL_CRFD = 100; // Control read FD | ||
| const REPRL_CWFD = 101; // Control write FD | ||
| const REPRL_DRFD = 102; // Data read FD | ||
| const REPRL_DWFD = 103; // Data write FD |
There was a problem hiding this comment.
🧩 Analysis chain
Unused DWFD FD constant trips lint; underscore-prefix or remove until used
REPRL_DWFD is declared but never used, which matches the ESLint no-unused-vars failure.
If this FD is intentionally reserved for future writes, you can keep the declaration but satisfy lint by marking it as intentionally unused:
-const REPRL_DWFD = 103; // Data write FD
+const _REPRL_DWFD = 103; // Data write FD (reserved for future use)or simply remove it until the write side is implemented.
This keeps the harness compiling cleanly without disabling global lint rules.
I need to verify whether REPRL_DWFD is actually unused in the codebase. Let me gather that information now.
🏁 Script executed:
#!/bin/bash
# Search for all references to REPRL_DWFD across the entire codebase
echo "=== Searching for REPRL_DWFD usage ==="
rg -n 'REPRL_DWFD' --type ts --type js
# Also check the specific file to understand context
echo -e "\n=== Checking src/js/internal/fuzzilli-reprl-minimal.ts ==="
fd -e ts -e js 'fuzzilli-reprl-minimal' --exec cat {}Length of output: 2151
Underscore-prefix or remove unused REPRL_DWFD constant
Verification confirms REPRL_DWFD is declared on line 7 but never referenced elsewhere in the file or codebase. The other three REPRL constants (CRFD, CWFD, DRFD) are all actively used for reading commands, writing status, and reading script data respectively.
Since REPRL_DWFD is logically reserved for future write operations but not yet implemented, rename it with an underscore prefix to satisfy linters:
-const REPRL_DWFD = 103; // Data write FD
+const _REPRL_DWFD = 103; // Data write FDor remove it entirely until the write side is ready.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const REPRL_CRFD = 100; // Control read FD | |
| const REPRL_CWFD = 101; // Control write FD | |
| const REPRL_DRFD = 102; // Data read FD | |
| const REPRL_DWFD = 103; // Data write FD | |
| const REPRL_CRFD = 100; // Control read FD | |
| const REPRL_CWFD = 101; // Control write FD | |
| const REPRL_DRFD = 102; // Data read FD | |
| const _REPRL_DWFD = 103; // Data write FD |
🧰 Tools
🪛 GitHub Check: Lint JavaScript
[failure] 7-7: eslint(no-unused-vars)
Variable 'REPRL_DWFD' is declared but never used. Unused variables should start with a '_'.
🤖 Prompt for AI Agents
In src/js/internal/fuzzilli-reprl-minimal.ts around lines 4 to 7, the constant
REPRL_DWFD is declared but never used; to satisfy linters either delete the
declaration entirely or rename it with a leading underscore (e.g. _REPRL_DWFD)
to mark it intentionally unused. Update the single declaration accordingly and
ensure no other code references it; run the linter and tests to confirm the
warning is resolved.
| while (true) { | ||
| console.error("[REPRL] Waiting for command..."); | ||
| // Read command | ||
| const cmd = Buffer.alloc(4); | ||
| const cmd_n = fs.readSync(REPRL_CRFD, cmd, 0, 4, null); | ||
| console.error(`[REPRL] Read command: ${cmd.toString()}, bytes: ${cmd_n}`); | ||
|
|
||
| if (cmd_n === 0) { | ||
| break; // EOF | ||
| } | ||
|
|
||
| if (cmd_n !== 4 || cmd.toString() !== "exec") { | ||
| throw new Error(`Invalid REPRL command: expected 'exec', got ${cmd.toString()}`); | ||
| } | ||
|
|
||
| // Read script size (8 bytes, little-endian) | ||
| const size_bytes = Buffer.alloc(8); | ||
| fs.readSync(REPRL_CRFD, size_bytes, 0, 8, null); | ||
| const script_size = Number(size_bytes.readBigUInt64LE(0)); | ||
|
|
||
| // Read script data from REPRL_DRFD | ||
| const script_data = Buffer.alloc(script_size); | ||
| let total_read = 0; | ||
| while (total_read < script_size) { | ||
| const n = fs.readSync(REPRL_DRFD, script_data, total_read, script_size - total_read, null); | ||
| if (n === 0) break; | ||
| total_read += n; | ||
| } | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider tightening read/error handling and logging in the main REPRL loop
A few small adjustments would make the harness more robust and faster under fuzzing load:
fs.readSyncfor the command and size fields assumes it will always return the full 4/8 bytes in one call. On pipes that’s usually true, but it’s safer to treat short reads as protocol errors and exit instead of continuing with partially filled buffers.- The script size (
script_size) comes directly from the fuzzer; clamping it to a reasonable maximum (e.g., a few MB) would avoid accidental huge allocations if the REPRL header gets corrupted. - The per-iteration
console.errorlogging will significantly slow down fuzzing in practice.
All of these are non-blocking for a “stop-gap” harness but worth tightening when you move beyond the minimal baseline: treat short reads as fatal protocol errors, clamp script_size, and gate verbose logging behind an env flag or a debug mode.
🤖 Prompt for AI Agents
In src/js/internal/fuzzilli-reprl-minimal.ts around lines 24 to 52, the REPRL
loop currently assumes readSync returns full 4/8 bytes and always logs every
iteration; update it to treat any short read of the command or size fields as a
protocol error and exit (do not continue with partially filled buffers),
validate and clamp the parsed script_size to a safe maximum (e.g., a few MB)
before allocating the script buffer and fail if size exceeds that limit, ensure
the loop reads the script_data until total_read == script_size but treats an
unexpected short read as a fatal error, and remove or gate the per-iteration
console.error calls behind a debug/env flag to avoid blowing up fuzzing
performance.
| let exit_code = 0; | ||
| try { | ||
| (0, eval)(script); | ||
| } catch (e) { | ||
| console.log(`uncaught:${e}`); | ||
| exit_code = 1; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
eval is expected here but should be explicitly annotated to satisfy security lint
The Biome warning about eval() is valid in general, but for a fuzzing harness this is precisely what we want: execute arbitrary generated programs under instrumentation.
Instead of refactoring away from eval, keep the behavior but document the intent and silence the rule locally:
- let exit_code = 0;
- try {
- (0, eval)(script);
- } catch (e) {
- console.log(`uncaught:${e}`);
- exit_code = 1;
- }
+ let exit_code = 0;
+ try {
+ // biome-ignore lint/security/noGlobalEval: Fuzzilli REPRL harness must eval fuzzer-provided JS
+ (0, eval)(script);
+ } catch (e) {
+ console.log(`uncaught:${e}`);
+ exit_code = 1;
+ }This makes it clear to future readers and static-analysis tools that the eval is deliberate and confined to fuzzing-only code paths.
I'll verify this review comment by examining the file and checking the Biome configuration.
Now I need to verify the file and code state in the codebase:
🏁 Script executed:
#!/bin/bash
# Find the file and check its content around lines 56-62
find . -name "fuzzilli-reprl-minimal.ts" -type f 2>/dev/null | head -5Length of output: 43
🏁 Script executed:
#!/bin/bash
# Check the file content around lines 56-62
if [ -f "src/js/internal/fuzzilli-reprl-minimal.ts" ]; then
echo "=== Lines 50-70 ==="
sed -n '50,70p' "src/js/internal/fuzzilli-reprl-minimal.ts"
echo ""
echo "=== Full file line count ==="
wc -l "src/js/internal/fuzzilli-reprl-minimal.ts"
else
echo "File not found"
fiLength of output: 495
🏁 Script executed:
#!/bin/bash
# Search for Biome configuration
find . -maxdepth 2 -type f \( -name "biome.json" -o -name "biome.jsonc" \) 2>/dev/null
echo ""
echo "=== Checking for security rules in biome config ==="
for config in biome.json biome.jsonc; do
if [ -f "$config" ]; then
echo "Found: $config"
head -50 "$config"
fi
doneLength of output: 1482
🏁 Script executed:
#!/bin/bash
# Search for existing biome-ignore patterns in the file
if [ -f "src/js/internal/fuzzilli-reprl-minimal.ts" ]; then
echo "=== Searching for biome-ignore patterns ==="
grep -n "biome-ignore" "src/js/internal/fuzzilli-reprl-minimal.ts" || echo "No biome-ignore found"
fiLength of output: 65
🏁 Script executed:
#!/bin/bash
# Search for eval usage patterns across codebase
echo "=== Searching for eval usage ==="
grep -rn "eval" --include="*.ts" --include="*.js" . 2>/dev/null | grep -v node_modules | head -20Length of output: 2186
Let me check the full Biome configuration for security rules and verify the correct syntax:
Now let me search for the correct Biome rule name and syntax:
🏁 Script executed:
#!/bin/bash
# Get the full biome.json file to check security rules
echo "=== Full biome.json ==="
cat biome.jsonLength of output: 1806
🏁 Script executed:
#!/bin/bash
# Check if there are any lint errors currently in the file
echo "=== Checking file context for security/eval ==="
head -20 "src/js/internal/fuzzilli-reprl-minimal.ts"Length of output: 623
🌐 Web query:
Biome security noGlobalEval rule lint suppression syntax
💡 Result:
Use Biome suppression comments with the rule's diagnostic category: lint/security/noGlobalEval.
Examples:
-
Inline (next line only)
// biome-ignore lint/security/noGlobalEval: reason
eval("..."); -
Range
// biome-ignore-start lint/security/noGlobalEval: reason
...code that uses eval...
// biome-ignore-end lint/security/noGlobalEval: reason -
File-wide (top of file)
// biome-ignore-all lint/security/noGlobalEval: reason
(You can also configure the rule in biome.json instead.) [1][2]
Add biome-ignore comment to document deliberate eval usage in fuzzing harness
The review comment is correct. Biome supports the lint/security/noGlobalEval rule suppression with the syntax // biome-ignore lint/security/noGlobalEval: reason, and the proposed fix accurately documents why eval is necessary here. The file is confirmed to be the Fuzzilli REPRL fuzzing harness, where executing arbitrary generated programs via eval is the intended behavior, making this suppression justified and appropriate.
let exit_code = 0;
try {
- (0, eval)(script);
+ // biome-ignore lint/security/noGlobalEval: Fuzzilli REPRL harness must eval fuzzer-provided JS
+ (0, eval)(script);
} catch (e) {
console.log(`uncaught:${e}`);
exit_code = 1;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let exit_code = 0; | |
| try { | |
| (0, eval)(script); | |
| } catch (e) { | |
| console.log(`uncaught:${e}`); | |
| exit_code = 1; | |
| } | |
| let exit_code = 0; | |
| try { | |
| // biome-ignore lint/security/noGlobalEval: Fuzzilli REPRL harness must eval fuzzer-provided JS | |
| (0, eval)(script); | |
| } catch (e) { | |
| console.log(`uncaught:${e}`); | |
| exit_code = 1; | |
| } |
🧰 Tools
🪛 Biome (2.1.2)
[error] 58-58: eval() exposes to security risks and performance issues.
See the MDN web docs for more details.
Refactor the code so that it doesn't need to call eval().
(lint/security/noGlobalEval)
🤖 Prompt for AI Agents
In src/js/internal/fuzzilli-reprl-minimal.ts around lines 56 to 62, the use of
eval is deliberate for executing generated fuzzing programs but currently
triggers the Biome lint rule noGlobalEval; add a Biome ignore comment
immediately above the eval call using the syntax // biome-ignore
lint/security/noGlobalEval: reason, e.g. explaining that eval is intentionally
used to run generated Fuzzilli programs in the REPRL harness, so the linter will
skip this rule for that line.
markovejnovic
force-pushed
the
u/markovejnovic/ENG-21237
branch
15 times, most recently
from
627cda3 to
f57108e
Compare
markovejnovic
force-pushed
the
u/markovejnovic/ENG-21237
branch
from
69b03cd to
1b87501
Compare
markovejnovic
force-pushed
the
u/markovejnovic/ENG-21237
branch
from
b13a52d to
87e89ec
Compare
markovejnovic
force-pushed
the
u/markovejnovic/ENG-21237
branch
from
868fc25 to
c5e3879
Compare
There was a problem hiding this comment.
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 (4)
src/output.zig (1)
812-881: Scoped logger visibility now permanently disabled for all tagged logs
evaluateIsVisibleunconditionally setsreally_disabletotrue, soisVisible()will always returnfalsewheneverEnvironment.enable_logsis enabled. That effectively disables allScoped(...)logs and makes theVisibilityenum +BUN_DEBUG_*/BUN_DEBUG_ALLenvironment knobs non-functional.If the goal is only to quiet logs during fuzzilli runs, this should likely be conditioned on a fuzzilli-specific signal (e.g.,
Environment.enable_fuzzillior a dedicated env var), and preserve the previous env‑driven behavior for normal debug/log builds.package.json (1)
29-46: Consider isolating fuzzilli builds into a separate build directory
build:debug:fuzzillireuses-B build/debug, so running it will reconfigure the standard debug build directory to include fuzzilli instrumentation. That’s fine for CI, but locally it can be hard to tell whetherbuild/debug/bun-debugis a fuzzilli build or a plain debug build, depending on the last script invoked.Non-blocking, but consider pointing this at a dedicated directory (e.g.
build/debug-fuzzilli) to keep artifacts clearly separated.src/bun.js/bindings/ZigGlobalObject.cpp (1)
503-515: Fix inconsistentgetenvusage in ZigGlobalObject.cppLine 436 uses unqualified
getenv("BUN_DISABLE_STOP_IF_NECESSARY_TIMER")while line 511 usesstd::getenv("BUN_FUZZILLI_MODE"). This inconsistency risks portability issues if transitive includes change.To resolve, either:
- Switch line 511 to plain
getenv("BUN_FUZZILLI_MODE")for consistency with line 436, or- Add
#include <cstdlib>and usestd::getenvconsistently throughoutcmake/CompilerFlags.cmake (1)
47-70: Add platform guards to ENABLE_FUZZILLI compiler flags for consistency and portabilityThe flags for fuzzilli coverage and macro are correctly wired, but the block lacks platform scoping unlike other conditional compiler flags in this file. Since
-fsanitize-coverage=trace-pc-guardis clang/gcc-specific and unsupported on Windows/MSVC, wrap theENABLE_FUZZILLIblock withif(UNIX)to match the pattern used elsewhere in CompilerFlags.cmake:if(UNIX AND ENABLE_FUZZILLI) register_compiler_flags(...) ... endif()This prevents confusing build failures if someone accidentally sets
-DENABLE_FUZZILLI=ONon an unsupported platform and maintains consistency with how other compiler flags are gated in the file.
♻️ Duplicate comments (1)
src/bun.js/bindings/FuzzilliREPRL.cpp (1)
8-15: POSIX headers and coverage plumbing still need_WIN32guards to avoid Windows build failuresThe file still unconditionally includes
<unistd.h>,<fcntl.h>,<sys/mman.h>,<sys/stat.h>and usesshm_open,mmap,MAP_SHARED,MAP_FAILED,S_IREAD | S_IWRITE,fsync, and POSIX FDs. On Windows toolchains these headers/APIs don’t exist, so any FUZZILLI_ENABLED Windows build will fail here even if ASAN/coverage aren’t actually used.You should:
- Wrap POSIX‑only headers:
-#include <unistd.h> -#include <fcntl.h> -#include <sys/mman.h> -#include <sys/stat.h> +#ifndef _WIN32 +#include <unistd.h> +#include <fcntl.h> +#include <sys/mman.h> +#include <sys/stat.h> +#endif
- Restrict the ASAN/coverage block to non‑Windows:
-#if __has_feature(address_sanitizer) || defined(__SANITIZE_ADDRESS__) +#if !defined(_WIN32) && (__has_feature(address_sanitizer) || defined(__SANITIZE_ADDRESS__))
- Similarly guard any code that calls
fsync, relies onSTDOUT_FILENO/STDERR_FILENO, or usessignal()with POSIX signals so it’s not compiled on_WIN32.This keeps Fuzzilli/coverage Linux‑only while letting Windows builds succeed, even when FUZZILLI_ENABLED is toggled on in other configurations.
Also applies to: 195-303
📜 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.
📒 Files selected for processing (13)
build.zig(5 hunks)cmake/CompilerFlags.cmake(1 hunks)cmake/Options.cmake(1 hunks)cmake/targets/BuildBun.cmake(1 hunks)package.json(1 hunks)src/bun.js/bindings/BunObject.cpp(2 hunks)src/bun.js/bindings/FuzzilliREPRL.cpp(1 hunks)src/bun.js/bindings/ZigGlobalObject.cpp(2 hunks)src/cli.zig(6 hunks)src/cli/fuzzilli_command.zig(1 hunks)src/env.zig(1 hunks)src/js/internal/fuzzilli-reprl.ts(1 hunks)src/output.zig(1 hunks)
🧰 Additional context used
🧠 Learnings (29)
📓 Common learnings
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24423
File: test/js/node/path/browserify.test.js:472-475
Timestamp: 2025-11-07T17:28:58.045Z
Learning: markovejnovic prefers functional programming patterns (e.g., using reduce) over imperative loops and does not want style-based readability suggestions unless there's a concrete technical reason beyond personal preference.
<!--
📚 Learning: 2025-10-15T20:19:37.256Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:37.256Z
Learning: In the Bun codebase, FFI (Foreign Function Interface) code is compiled separately using TinyCC (tcc), which barely supports C99. Headers like src/bun.js/api/FFI.h and src/bun.js/api/ffi-stdbool.h are only used for FFI compilation with tcc, not with the main compiler. Therefore, C standard changes to the main Bun target do not affect FFI code compilation.
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cppsrc/cli/fuzzilli_command.zigcmake/targets/BuildBun.cmakebuild.zigsrc/bun.js/bindings/BunObject.cppsrc/bun.js/bindings/FuzzilliREPRL.cpp
📚 Learning: 2025-10-15T20:19:38.580Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:38.580Z
Learning: In the Bun codebase, FFI is compiled with tcc (TinyCC), which barely supports C99. The headers `src/bun.js/api/FFI.h` and `src/bun.js/api/ffi-stdbool.h` are only used for FFI compilation with tcc, not for the main Bun target. Therefore, C23 compatibility concerns (such as bool/true/false keyword conflicts) do not apply to these FFI headers.
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cppsrc/cli/fuzzilli_command.zigcmake/targets/BuildBun.cmakebuild.zigsrc/bun.js/bindings/BunObject.cppsrc/bun.js/bindings/FuzzilliREPRL.cpp
📚 Learning: 2025-10-18T20:50:47.750Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:366-373
Timestamp: 2025-10-18T20:50:47.750Z
Learning: In Bun's Zig codebase (src/bun.js/bindings/JSValue.zig), the JSValue enum uses `.null` (not `.js_null`) for JavaScript's null value. Only `js_undefined` has the `js_` prefix to avoid collision with Zig's built-in `undefined` keyword. The correct enum fields are: `js_undefined`, `null`, `true`, `false`, and `zero`.
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-11-03T20:43:06.996Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24273
File: src/bun.js/test/snapshot.zig:19-19
Timestamp: 2025-11-03T20:43:06.996Z
Learning: In Bun's Zig codebase, when storing JSValue objects in collections like ArrayList, use `jsc.Strong.Optional` (not raw JSValue). When adding values, wrap them with `jsc.Strong.Optional.create(value, globalThis)`. In cleanup code, iterate the collection calling `.deinit()` on each Strong.Optional item before calling `.deinit()` on the ArrayList itself. This pattern automatically handles GC protection. See examples in src/bun.js/test/ScopeFunctions.zig and src/bun.js/node/node_cluster_binding.zig.
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-09-20T05:35:57.318Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 22534
File: src/bun.js/bindings/headers.h:729-731
Timestamp: 2025-09-20T05:35:57.318Z
Learning: symbols.txt in the Bun codebase is specifically for V8 API mangled symbols (without leading underscore), not for general Bun host functions declared with BUN_DECLARE_HOST_FUNCTION. Host functions are handled through different build mechanisms.
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cpppackage.jsonsrc/bun.js/bindings/BunObject.cppsrc/bun.js/bindings/FuzzilliREPRL.cpp
📚 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/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-09-05T19:49:26.188Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: src/valkey/js_valkey_functions.zig:852-867
Timestamp: 2025-09-05T19:49:26.188Z
Learning: In Bun’s Zig code, `.js_undefined` is a valid and preferred JSValue literal for “undefined” (e.g., resolving JSPromise). Do not refactor usages to `jsc.JSValue.jsUndefined()`, especially in src/valkey/js_valkey_functions.zig unsubscribe().
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cppsrc/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-11-12T04:11:52.293Z
Learnt from: cirospaciari
Repo: oven-sh/bun PR: 24622
File: src/deps/uws/us_socket_t.zig:112-113
Timestamp: 2025-11-12T04:11:52.293Z
Learning: In Bun's Zig codebase, when passing u32 values to C FFI functions that expect c_uint parameters, no explicit intCast is needed because c_uint is equivalent to u32 on Bun's target platforms and Zig allows implicit coercion between equivalent types. This pattern is used consistently throughout src/deps/uws/us_socket_t.zig in functions like setTimeout, setLongTimeout, and setKeepalive.
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cppsrc/cli/fuzzilli_command.zig
📚 Learning: 2025-10-16T02:17:35.237Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/analytics.zig:15-21
Timestamp: 2025-10-16T02:17:35.237Z
Learning: In src/analytics.zig and similar files using bun.EnvVar boolean environment variables: the new EnvVar API for boolean flags (e.g., bun.EnvVar.do_not_track.get(), bun.EnvVar.ci.get()) is designed to parse and return boolean values from environment variables, not just check for their presence. This is an intentional design change from the previous presence-based checks using bun.getenvZ().
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cppsrc/env.zigsrc/output.zigsrc/cli/fuzzilli_command.zigcmake/targets/BuildBun.cmakebuild.zig
📚 Learning: 2025-10-11T15:19:30.301Z
Learnt from: mastermakrela
Repo: oven-sh/bun PR: 19167
File: src/bun.js/api.zig:31-31
Timestamp: 2025-10-11T15:19:30.301Z
Learning: The learnings about `pub const js = JSC.Codegen.JS<ClassName>` and re-exporting toJS/fromJS/fromJSDirect apply to class-based Zig bindings with constructors and prototype methods (e.g., those with `new` constructors). They do NOT apply to simple namespace-style API objects like TOMLObject, YAMLObject, and CSVObject which expose static functions via a `create()` method that returns a plain JS object.
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-10-15T22:03:50.832Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/envvars.zig:135-144
Timestamp: 2025-10-15T22:03:50.832Z
Learning: In src/envvars.zig, the boolean feature flag cache uses a single atomic enum and should remain monotonic. Only the string cache (which uses two atomics: ptr and len) requires acquire/release ordering to prevent torn reads.
Applied to files:
src/env.zigsrc/cli/fuzzilli_command.zig
📚 Learning: 2025-10-18T02:06:31.606Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 23796
File: src/bun.js/test/Execution.zig:624-625
Timestamp: 2025-10-18T02:06:31.606Z
Learning: In Zig files using bun.Output.scoped(), the visibility level `.visible` means logs are enabled by default in debug builds unless `BUN_DEBUG_QUIET_LOGS=1` is set; if that environment variable is set, the logs can be re-enabled with `BUN_DEBUG_<scope>=1`. Use `.visible` for logs that should appear by default in debug builds, and `.hidden` for logs that require explicit opt-in via `BUN_DEBUG_<scope>=1`.
Applied to files:
src/output.zig
📚 Learning: 2025-11-14T16:07:01.064Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
Applied to files:
package.jsoncmake/targets/BuildBun.cmakebuild.zig
📚 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:
package.jsonsrc/js/internal/fuzzilli-reprl.ts
📚 Learning: 2025-10-08T13:56:00.875Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 23373
File: src/bun.js/api/BunObject.zig:2514-2521
Timestamp: 2025-10-08T13:56:00.875Z
Learning: For Bun codebase: prefer using `bun.path` utilities (e.g., `bun.path.joinAbsStringBuf`, `bun.path.join`) over `std.fs.path` functions for path operations.
Applied to files:
package.json
📚 Learning: 2025-10-16T17:32:03.074Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/install/PackageManager/PackageManagerOptions.zig:187-193
Timestamp: 2025-10-16T17:32:03.074Z
Learning: In Bun's codebase (particularly in files like src/install/PackageManager/PackageManagerOptions.zig), mixing bun.EnvVar.*.get() and bun.EnvVar.*.platformGet() for environment variable lookups is intentional and safe. The code is protected by compile-time platform checks (Environment.isWindows, etc.), and compilation will fail if the wrong function is used on the wrong platform. This pattern should not be flagged as a consistency issue.
Applied to files:
src/cli/fuzzilli_command.zigsrc/bun.js/bindings/FuzzilliREPRL.cpp
📚 Learning: 2025-10-16T21:24:52.779Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/crash_handler.zig:1415-1423
Timestamp: 2025-10-16T21:24:52.779Z
Learning: When a boolean EnvVar in src/envvars.zig is defined with a default value (e.g., `.default = false`), the `get()` method returns `bool` instead of `?bool`. This means you cannot distinguish between "environment variable not set" and "environment variable explicitly set to the default value". For opt-out scenarios where detection of explicit setting is needed (like `BUN_ENABLE_CRASH_REPORTING` on platforms where crash reporting defaults to enabled), either: (1) don't provide a default value so `get()` returns `?bool`, or (2) use the returned boolean directly instead of only checking if it's true.
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-10-24T10:43:09.398Z
Learnt from: fmguerreiro
Repo: oven-sh/bun PR: 23774
File: src/install/PackageManager/updatePackageJSONAndInstall.zig:548-548
Timestamp: 2025-10-24T10:43:09.398Z
Learning: In Bun's Zig codebase, the `as(usize, intCast(...))` cast pattern triggers a Zig compiler bug that causes compilation to hang indefinitely when used in complex control flow contexts (loops + short-circuit operators + optional unwrapping). Avoid this pattern and use simpler alternatives like just `intCast(...)` if type casting is necessary.
Applied to files:
src/cli/fuzzilli_command.zigbuild.zigsrc/bun.js/bindings/FuzzilliREPRL.cpp
📚 Learning: 2025-11-10T00:57:09.173Z
Learnt from: franciscop
Repo: oven-sh/bun PR: 24514
File: src/bun.js/api/crypto/PasswordObject.zig:86-101
Timestamp: 2025-11-10T00:57:09.173Z
Learning: In Bun's Zig codebase (PasswordObject.zig), when validating the parallelism parameter for Argon2, the upper limit is set to 65535 (2^16 - 1) rather than using `std.math.maxInt(u24)` because the latter triggers Zig's truncation limit checks. The value 65535 is a practical upper bound that avoids compiler issues while being sufficient for thread parallelism use cases.
Applied to files:
src/cli/fuzzilli_command.zig
📚 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:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-10-01T21:49:27.862Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23169
File: src/bun.js/bindings/BunIDLConvert.h:29-42
Timestamp: 2025-10-01T21:49:27.862Z
Learning: In Bun's IDL bindings (src/bun.js/bindings/BunIDLConvert.h), IDLStrictNull intentionally treats both undefined and null as null (using isUndefinedOrNull()), matching WebKit's IDLNull & IDLNullable behavior. This is the correct implementation and should not be changed to only accept null.
Applied to files:
src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-10-01T21:59:54.571Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23169
File: src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h:47-74
Timestamp: 2025-10-01T21:59:54.571Z
Learning: In the new bindings generator (bindgenv2) for `src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h`, the context-aware enumeration conversion overloads intentionally use stricter validation (requiring `value.isString()` without ToString coercion), diverging from Web IDL semantics. This is a design decision documented in comments.
Applied to files:
src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-10-20T01:38:02.660Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/BunFetchInstrumentation.ts:126-131
Timestamp: 2025-10-20T01:38:02.660Z
Learning: In BunFetchInstrumentation.ts, the force-restore to ORIGINAL_FETCH in the disable() method is intentionally kept (despite appearing unsafe) because it's required for proper test cleanup when instrumentation is repeatedly enabled/disabled. Without it, 15 distributed tracing and context propagation tests fail. Shimmer's unwrap() doesn't reliably restore the original fetch in Bun's globalThis context. The isBunOtelPatched safety check ensures the restore only happens when the current fetch is still ours, preventing clobbering of other tools' wrappers.
Applied to files:
src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-10-17T01:21:35.060Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23755
File: src/bun.js/bindings/BunIDLConvert.h:79-95
Timestamp: 2025-10-17T01:21:35.060Z
Learning: In JavaScriptCore (used by Bun), the `toBoolean()` method implements ECMAScript's ToBoolean abstract operation, which is side-effect-free and cannot throw exceptions. It does not execute arbitrary JavaScript or invoke user-defined methods like Symbol.toPrimitive. Exception handling is not needed after toBoolean() calls.
Applied to files:
src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-10-19T03:01:29.084Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:286-289
Timestamp: 2025-10-19T03:01:29.084Z
Learning: In src/bun.js/telemetry.zig, the guard preventing double configuration (lines 213-216) is intentional. The telemetry API uses a single-shot configuration model where configure() is called once during application startup. Users must call Bun.telemetry.configure(null) to reset before reconfiguring. This design ensures: (1) predictable state—callbacks don't change mid-request, avoiding race conditions; (2) zero overhead when disabled—no checking for callback changes on every request; (3) clear ownership—one adapter (e.g., bun-otel) owns the telemetry config. Merge-style reconfiguration would require atomic updates during active requests, adding overhead to the hot path.
<!-- [/add_learning]
Applied to files:
src/bun.js/bindings/FuzzilliREPRL.cpp
📚 Learning: 2025-09-30T22:53:19.887Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 23117
File: src/bun.js/test/snapshot.zig:265-276
Timestamp: 2025-09-30T22:53:19.887Z
Learning: In Bun's snapshot testing (src/bun.js/test/snapshot.zig), multiple inline snapshots at the same line and column (same call position) must have identical values. However, multiple inline snapshots on the same line at different columns are allowed to have different values. The check is position-specific (line+col), not line-wide.
Applied to files:
src/bun.js/bindings/FuzzilliREPRL.cpp
📚 Learning: 2025-09-02T17:41:07.869Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22227
File: src/memory.zig:60-76
Timestamp: 2025-09-02T17:41:07.869Z
Learning: In bun's memory utilities, when handling const pointers in deinit operations, prefer compile-time errors over silent skipping to avoid hard-to-find memory leaks. Users expect explicit failures rather than silent omissions in memory management.
Applied to files:
src/bun.js/bindings/FuzzilliREPRL.cpp
📚 Learning: 2025-10-19T02:52:37.412Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/tsconfig.json:1-15
Timestamp: 2025-10-19T02:52:37.412Z
Learning: In the Bun repository, packages under packages/ (e.g., bun-otel) can follow a TypeScript-first pattern where package.json exports point directly to .ts files (not compiled .js files). Bun natively runs TypeScript, so consumers import .ts sources directly and receive full type information without needing compiled .d.ts declaration files. For such packages, adding "declaration": true or "outDir" in tsconfig.json is unnecessary and would break the export structure.
<!-- [remove_learning]
ceedde95-980e-4898-a2c6-40ff73913664
Applied to files:
src/js/internal/fuzzilli-reprl.ts
🧬 Code graph analysis (2)
src/bun.js/bindings/ZigGlobalObject.cpp (1)
src/bun.js/bindings/FuzzilliREPRL.cpp (2)
Bun__REPRL__registerFuzzilliFunction(167-187)Bun__REPRL__registerFuzzilliFunction(167-167)
src/bun.js/bindings/BunObject.cpp (2)
src/bun.js/bindings/ZigGlobalObject.h (1)
globalObject(127-127)src/bun.js/bindings/FuzzilliREPRL.cpp (4)
Bun__REPRL__resetCoverage(280-283)Bun__REPRL__resetCoverage(280-280)Bun__REPRL__resetCoverage(299-301)Bun__REPRL__resetCoverage(299-299)
🪛 Biome (2.1.2)
src/js/internal/fuzzilli-reprl.ts
[error] 308-308: eval() exposes to security risks and performance issues.
See the MDN web docs for more details.
Refactor the code so that it doesn't need to call eval().
(lint/security/noGlobalEval)
🪛 GitHub Check: Lint JavaScript
src/js/internal/fuzzilli-reprl.ts
[failure] 257-257: eslint(no-unused-vars)
Catch parameter 'e' is caught but never used.
[failure] 256-256: eslint(no-unused-vars)
Variable 'stat' is declared but never used. Unused variables should start with a '_'.
[failure] 238-238: eslint(no-unused-vars)
Parameter 'pattern' is declared but never used. Unused parameters should start with a '_'.
[failure] 223-223: eslint(no-unused-vars)
Variable 'params' is declared but never used. Unused variables should start with a '_'.
[failure] 223-223: eslint(no-unused-vars)
Parameter 'sql' is declared but never used. Unused parameters should start with a '_'.
[failure] 217-217: eslint(no-unused-vars)
Parameter 'end' is declared but never used. Unused parameters should start with a '_'.
[failure] 217-217: eslint(no-unused-vars)
Parameter 'start' is declared but never used. Unused parameters should start with a '_'.
[failure] 8-8: eslint(no-unused-vars)
Variable 'REPRL_DWFD' is declared but never used. Unused variables should start with a '_'.
🔇 Additional comments (8)
src/env.zig (1)
36-56: Fuzzilli build flag exposure looks consistentWiring
enable_fuzzillithroughbuild_optionsnext to the other feature flags is consistent with existing patterns and provides a clear single source of truth for Zig-side gating.cmake/Options.cmake (1)
123-138: ENABLE_FUZZILLI option wiring looks correctDefining
ENABLE_FUZZILLIas a boolean option withDEFAULT OFFmatches existing patterns for feature toggles and keeps fuzzilli strictly opt‑in at configuration time.cmake/targets/BuildBun.cmake (1)
680-710: Zig define propagation for fuzzilli is consistentPassing
-Denable_fuzzilli=$<IF:$<BOOL:${ENABLE_FUZZILLI}>,true,false>matches how other feature flags are forwarded intobuild.zigand keeps the flag well-integrated with the existing Zig configuration surface.src/bun.js/bindings/ZigGlobalObject.cpp (1)
256-265: Fuzzilli REPRL registration is correctly gatedThe
FUZZILLI_ENABLEDguard aroundBun__REPRL__registerFuzzilliFunctionkeeps the REPRL bits completely out of non‑fuzz builds, and the extraBUN_FUZZILLI_MODEruntime check ensures thefuzzilli()function and signal handlers are only installed when the harness asks for them.This is a good, low‑risk way to integrate REPRL into the global object lifecycle.
src/cli/fuzzilli_command.zig (1)
3-55: Fuzzilli command wiring and REPRL bootstrap look correctThe compile‑time
enable_fuzzilligate, POSIX check, REPRL FD verification, embedded harness write to/tmp/bun-fuzzilli-reprl.js, andRun.bootcall all line up with the REPRL protocol and the TS harness. I don’t see additional issues beyond those already raised in earlier review comments.src/bun.js/bindings/BunObject.cpp (1)
445-470: Bun.resetCoverage host hook looks consistent with Fuzzilli REPRL integrationThe FUZZILLI_ENABLED‑gated
jsResetCoverageplusconstructResetCoverageand the new LUT entry give Bun.resetCoverage the right behavior (callable only in FUZZILLI builds, undefined stub otherwise) and match the C++ implementation in FuzzilliREPRL.cpp. No additional issues from this side.Also applies to: 796-797
build.zig (2)
35-35: LGTM: Consistent option integrationThe
enable_fuzzillioption is correctly integrated following the established pattern for other boolean build options. The field is properly declared, exposed in the build options module, initialized with a clear description and safe default (false), and propagated through the multi-check system.Also applies to: 85-85, 260-260, 496-496
612-626: Critical: Attempting to set non-existent fieldsanitize_coverage_trace_pc_guardLine 616 sets
obj.sanitize_coverage_trace_pc_guard = true;on the Compile object, but this field does not exist in Zig'sBuild.Step.Compiletype. This will fail during compilation with an undefined field error. The code checks forsanitize_addresssupport on line 614 but applies a different (non-existent) field directly toobjon line 616.Additionally, the error message at line 624 could be misleading when
enable_asan=truebut fast build is enabled—the message says "fuzzilli requires asan" but ASAN is actually enabled in options, just skipped due to fast build mode.Remove the line 616 assignment or verify what the intended instrumentation flag should be. If this feature isn't supported in the current Zig version, add a safety check or remove the dead code entirely.
⛔ Skipped due to learnings
Learnt from: fmguerreiro Repo: oven-sh/bun PR: 23774 File: src/install/PackageManager/updatePackageJSONAndInstall.zig:548-548 Timestamp: 2025-10-24T10:43:09.398Z Learning: In Bun's Zig codebase, the `as(usize, intCast(...))` cast pattern triggers a Zig compiler bug that causes compilation to hang indefinitely when used in complex control flow contexts (loops + short-circuit operators + optional unwrapping). Avoid this pattern and use simpler alternatives like just `intCast(...)` if type casting is necessary.Learnt from: taylordotfish Repo: oven-sh/bun PR: 22227 File: src/allocators/allocation_scope.zig:284-314 Timestamp: 2025-09-02T18:25:27.976Z Learning: In bun's custom Zig implementation, the `#` prefix for private fields is valid syntax and should not be flagged as invalid. The syntax `#fieldname` creates private fields that cannot be accessed from outside the defining struct, and usage like `self.#fieldname` is correct within the same struct. This applies to fields like `#parent`, `#state`, `#allocator`, `#trace`, etc. throughout the codebase.Learnt from: markovejnovic Repo: oven-sh/bun PR: 23710 File: src/install/PackageManager/PackageManagerOptions.zig:187-193 Timestamp: 2025-10-16T17:32:03.074Z Learning: In Bun's codebase (particularly in files like src/install/PackageManager/PackageManagerOptions.zig), mixing bun.EnvVar.*.get() and bun.EnvVar.*.platformGet() for environment variable lookups is intentional and safe. The code is protected by compile-time platform checks (Environment.isWindows, etc.), and compilation will fail if the wrong function is used on the wrong platform. This pattern should not be flagged as a consistency issue.Learnt from: alii Repo: oven-sh/bun PR: 22193 File: src/install/PackageManager/security_scanner.zig:549-577 Timestamp: 2025-09-04T03:53:11.793Z Learning: When reviewing code for potential compilation issues, avoid making definitive statements about compilation bugs without being able to verify through actual compilation. Frame such observations as potential concerns or suggestions to investigate rather than certain errors.Learnt from: theshadow27 Repo: oven-sh/bun PR: 23798 File: src/bun.js/telemetry.zig:286-289 Timestamp: 2025-10-19T03:01:29.084Z Learning: In src/bun.js/telemetry.zig, the guard preventing double configuration (lines 213-216) is intentional. The telemetry API uses a single-shot configuration model where configure() is called once during application startup. Users must call Bun.telemetry.configure(null) to reset before reconfiguring. This design ensures: (1) predictable state—callbacks don't change mid-request, avoiding race conditions; (2) zero overhead when disabled—no checking for callback changes on every request; (3) clear ownership—one adapter (e.g., bun-otel) owns the telemetry config. Merge-style reconfiguration would require atomic updates during active requests, adding overhead to the hot path. <!-- [/add_learning]
| pub const FuzzilliCommand = @import("./cli/fuzzilli_command.zig").FuzzilliCommand; | ||
|
|
There was a problem hiding this comment.
Fix HelpCommand.printWithReason call after signature change
HelpCommand.printWithReason now takes two parameters (reason, show_all_flags), but the .HelpCommand branch in Tag.printHelp still calls it with a single argument:
Command.Tag.HelpCommand => {
HelpCommand.printWithReason(.explicit);
},This will fail to compile.
Update the call to pass the second argument explicitly (probably false, matching the default behavior elsewhere):
- Command.Tag.HelpCommand => {
- HelpCommand.printWithReason(.explicit);
- },
+ Command.Tag.HelpCommand => {
+ HelpCommand.printWithReason(.explicit, false);
+ },The rest of the FuzzilliCommand routing and gating against bun.Environment.enable_fuzzilli in which() and start() looks consistent.
Also applies to: 561-638, 731-737, 1211-1213, 1339-1340
🤖 Prompt for AI Agents
In src/cli.zig at lines 94-95, 561-638, 731-737, 1211-1213, and 1339-1340, the
HelpCommand.printWithReason call sites still pass a single argument but the
function signature changed to (reason, show_all_flags); update each
HelpCommand.printWithReason(...) invocation to pass the second parameter
explicitly (use false to preserve previous/default behavior) so calls become
printWithReason(<reason>, false).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/js/internal/fuzzilli-reprl.ts (1)
5-8: Fix lint blockers (unused vars/params) and document intentionalevalin REPRL harnessThe REPRL loop and mocks look fine functionally, but current Biome/eslint failures will keep CI red. You can resolve them without changing behavior by:
- Marking intentionally unused constants/params with
_prefix.- Removing unused locals (
stat) and unused catch binding.- Adding a short comment + suppression around the indirect
eval, since executing fuzzed scripts is exactly what this harness must do.Concrete patch:
@@ -const REPRL_CRFD = 100; // Control read FD -const REPRL_CWFD = 101; // Control write FD -const REPRL_DRFD = 102; // Data read FD -const REPRL_DWFD = 103; // Data write FD +const REPRL_CRFD = 100; // Control read FD +const REPRL_CWFD = 101; // Control write FD +const REPRL_DRFD = 102; // Data read FD +const _REPRL_DWFD = 103; // Data write FD (used only on the C++ side) @@ globalThis.MOCK_FILE = { @@ - stream: () => new ReadableStream(), - slice: (start, end) => globalThis.MOCK_FILE, + stream: () => new ReadableStream(), + slice: (_start, _end) => globalThis.MOCK_FILE, writer: () => ({ write: () => 0, end: () => 0, flush: () => Promise.resolve() }), }; @@ globalThis.MOCK_DB = { - query: (sql, ...params) => ({ + query: (_sql, ..._params) => ({ @@ - prepare: sql => globalThis.MOCK_DB.query(sql), + prepare: sql => globalThis.MOCK_DB.query(sql), @@ globalThis.MOCK_GLOB = { - scan: pattern => ({ + scan: (_pattern) => ({ @@ -try { - // Try to stat fd 100 to see if it exists - const stat = fs.fstatSync(REPRL_CRFD); -} catch (e) { +try { + // Try to stat fd 100 to see if it exists + fs.fstatSync(REPRL_CRFD); +} catch (_err) { @@ - // Execute script + // Execute script let exit_code = 0; try { - // Use indirect eval to execute in global scope - (0, eval)(script); + // Use indirect eval to execute in global scope + // biome-ignore lint/security/noGlobalEval: REPRL must execute arbitrary fuzzed scripts + // eslint-disable-next-line no-eval + (0, eval)(script); } catch (e) { @@ - resetCoverage(); + resetCoverage();This keeps the stop‑gap harness behavior identical while unblocking the JS lint job.
Also applies to: 176-247, 252-261, 288-323
src/bun.js/bindings/FuzzilliREPRL.cpp (1)
8-15: Guard POSIX-only headers, coverage, and fsync usage to avoid Windows build breakageThis translation unit still assumes a POSIX environment:
- Unconditionally includes
<unistd.h>,<fcntl.h>,<sys/mman.h>,<sys/stat.h>.- Coverage implementation uses
shm_open,mmap,MAP_SHARED,MAP_FAILED,S_IREAD | S_IWRITE.fuzzilliSignalHandlerusesfsyncandSTDOUT_FILENO/STDERR_FILENO.Bun__REPRL__registerFuzzilliFunctionsunconditionally registers signal handlers.If
FUZZILLI_ENABLEDever ends up defined on Windows, this will fail to compile even when ASAN/coverage isn’t in use.You can fix this by:
- Guarding the POSIX headers.
- Requiring
!_WIN32for the ASAN/coverage block.- Guarding
fsyncand POSIX-specific signal behavior with#ifndef _WIN32.Suggested patch:
-#include <csignal> -#include <cstdlib> -#include <cstring> -#include <unistd.h> -#include <fcntl.h> -#include <sys/mman.h> -#include <sys/stat.h> -#include <cerrno> +#include <csignal> +#include <cstdlib> +#include <cstring> +#include <cerrno> + +#ifndef _WIN32 +#include <unistd.h> +#include <fcntl.h> +#include <sys/mman.h> +#include <sys/stat.h> +#endif @@ -static void fuzzilliSignalHandler(int sig) +static void fuzzilliSignalHandler(int sig) { // Flush all output fflush(stdout); fflush(stderr); - fsync(STDOUT_FILENO); - fsync(STDERR_FILENO); +#ifndef _WIN32 + fsync(STDOUT_FILENO); + fsync(STDERR_FILENO); +#endif @@ -#if __has_feature(address_sanitizer) || defined(__SANITIZE_ADDRESS__) +#if !defined(_WIN32) && (__has_feature(address_sanitizer) || defined(__SANITIZE_ADDRESS__)) @@ - // Install signal handlers to ensure output is flushed before crashes - // This is important for ASAN output to be captured - signal(SIGABRT, fuzzilliSignalHandler); - signal(SIGSEGV, fuzzilliSignalHandler); - signal(SIGILL, fuzzilliSignalHandler); - signal(SIGFPE, fuzzilliSignalHandler); + // Install signal handlers to ensure output is flushed before crashes + // This is important for ASAN output to be captured +#ifndef _WIN32 + signal(SIGABRT, fuzzilliSignalHandler); + signal(SIGSEGV, fuzzilliSignalHandler); + signal(SIGILL, fuzzilliSignalHandler); + signal(SIGFPE, fuzzilliSignalHandler); +#endifThis keeps the full functionality on Unix-like targets while preventing the Windows build from pulling in missing POSIX APIs.
Also applies to: 19-21, 26-37, 172-238, 268-274
📜 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.
📒 Files selected for processing (4)
package.json(1 hunks)src/bun.js/bindings/FuzzilliREPRL.cpp(1 hunks)src/bun.js/bindings/ZigGlobalObject.cpp(2 hunks)src/js/internal/fuzzilli-reprl.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (19)
📓 Common learnings
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:37.256Z
Learning: In the Bun codebase, FFI (Foreign Function Interface) code is compiled separately using TinyCC (tcc), which barely supports C99. Headers like src/bun.js/api/FFI.h and src/bun.js/api/ffi-stdbool.h are only used for FFI compilation with tcc, not with the main compiler. Therefore, C standard changes to the main Bun target do not affect FFI code compilation.
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:38.580Z
Learning: In the Bun codebase, FFI is compiled with tcc (TinyCC), which barely supports C99. The headers `src/bun.js/api/FFI.h` and `src/bun.js/api/ffi-stdbool.h` are only used for FFI compilation with tcc, not for the main Bun target. Therefore, C23 compatibility concerns (such as bool/true/false keyword conflicts) do not apply to these FFI headers.
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24423
File: test/js/node/path/browserify.test.js:472-475
Timestamp: 2025-11-07T17:28:58.045Z
Learning: markovejnovic prefers functional programming patterns (e.g., using reduce) over imperative loops and does not want style-based readability suggestions unless there's a concrete technical reason beyond personal preference.
<!--
📚 Learning: 2025-11-14T16:07:01.064Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
Applied to files:
package.jsonsrc/js/internal/fuzzilli-reprl.ts
📚 Learning: 2025-11-20T19:51:32.266Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24880
File: packages/bun-vscode/package.json:382-385
Timestamp: 2025-11-20T19:51:32.266Z
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:
package.json
📚 Learning: 2025-09-20T05:35:57.318Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 22534
File: src/bun.js/bindings/headers.h:729-731
Timestamp: 2025-09-20T05:35:57.318Z
Learning: symbols.txt in the Bun codebase is specifically for V8 API mangled symbols (without leading underscore), not for general Bun host functions declared with BUN_DECLARE_HOST_FUNCTION. Host functions are handled through different build mechanisms.
Applied to files:
package.jsonsrc/bun.js/bindings/FuzzilliREPRL.cppsrc/bun.js/bindings/ZigGlobalObject.cpp
📚 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:
package.jsonsrc/js/internal/fuzzilli-reprl.ts
📚 Learning: 2025-10-08T13:56:00.875Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 23373
File: src/bun.js/api/BunObject.zig:2514-2521
Timestamp: 2025-10-08T13:56:00.875Z
Learning: For Bun codebase: prefer using `bun.path` utilities (e.g., `bun.path.joinAbsStringBuf`, `bun.path.join`) over `std.fs.path` functions for path operations.
Applied to files:
package.json
📚 Learning: 2025-10-19T02:52:37.412Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/tsconfig.json:1-15
Timestamp: 2025-10-19T02:52:37.412Z
Learning: In the Bun repository, packages under packages/ (e.g., bun-otel) can follow a TypeScript-first pattern where package.json exports point directly to .ts files (not compiled .js files). Bun natively runs TypeScript, so consumers import .ts sources directly and receive full type information without needing compiled .d.ts declaration files. For such packages, adding "declaration": true or "outDir" in tsconfig.json is unnecessary and would break the export structure.
<!-- [remove_learning]
ceedde95-980e-4898-a2c6-40ff73913664
Applied to files:
src/js/internal/fuzzilli-reprl.ts
📚 Learning: 2025-10-15T20:19:37.256Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:37.256Z
Learning: In the Bun codebase, FFI (Foreign Function Interface) code is compiled separately using TinyCC (tcc), which barely supports C99. Headers like src/bun.js/api/FFI.h and src/bun.js/api/ffi-stdbool.h are only used for FFI compilation with tcc, not with the main compiler. Therefore, C standard changes to the main Bun target do not affect FFI code compilation.
Applied to files:
src/bun.js/bindings/FuzzilliREPRL.cppsrc/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-10-15T20:19:38.580Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:38.580Z
Learning: In the Bun codebase, FFI is compiled with tcc (TinyCC), which barely supports C99. The headers `src/bun.js/api/FFI.h` and `src/bun.js/api/ffi-stdbool.h` are only used for FFI compilation with tcc, not for the main Bun target. Therefore, C23 compatibility concerns (such as bool/true/false keyword conflicts) do not apply to these FFI headers.
Applied to files:
src/bun.js/bindings/FuzzilliREPRL.cppsrc/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-10-16T17:32:03.074Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/install/PackageManager/PackageManagerOptions.zig:187-193
Timestamp: 2025-10-16T17:32:03.074Z
Learning: In Bun's codebase (particularly in files like src/install/PackageManager/PackageManagerOptions.zig), mixing bun.EnvVar.*.get() and bun.EnvVar.*.platformGet() for environment variable lookups is intentional and safe. The code is protected by compile-time platform checks (Environment.isWindows, etc.), and compilation will fail if the wrong function is used on the wrong platform. This pattern should not be flagged as a consistency issue.
Applied to files:
src/bun.js/bindings/FuzzilliREPRL.cpp
📚 Learning: 2025-10-24T10:43:09.398Z
Learnt from: fmguerreiro
Repo: oven-sh/bun PR: 23774
File: src/install/PackageManager/updatePackageJSONAndInstall.zig:548-548
Timestamp: 2025-10-24T10:43:09.398Z
Learning: In Bun's Zig codebase, the `as(usize, intCast(...))` cast pattern triggers a Zig compiler bug that causes compilation to hang indefinitely when used in complex control flow contexts (loops + short-circuit operators + optional unwrapping). Avoid this pattern and use simpler alternatives like just `intCast(...)` if type casting is necessary.
Applied to files:
src/bun.js/bindings/FuzzilliREPRL.cpp
📚 Learning: 2025-11-03T20:43:06.996Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24273
File: src/bun.js/test/snapshot.zig:19-19
Timestamp: 2025-11-03T20:43:06.996Z
Learning: In Bun's Zig codebase, when storing JSValue objects in collections like ArrayList, use `jsc.Strong.Optional` (not raw JSValue). When adding values, wrap them with `jsc.Strong.Optional.create(value, globalThis)`. In cleanup code, iterate the collection calling `.deinit()` on each Strong.Optional item before calling `.deinit()` on the ArrayList itself. This pattern automatically handles GC protection. See examples in src/bun.js/test/ScopeFunctions.zig and src/bun.js/node/node_cluster_binding.zig.
Applied to files:
src/bun.js/bindings/FuzzilliREPRL.cppsrc/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-10-18T20:59:45.579Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:458-475
Timestamp: 2025-10-18T20:59:45.579Z
Learning: In src/bun.js/telemetry.zig, the RequestId (u64) to JavaScript number (f64) conversion in jsRequestId() is intentionally allowed to lose precision beyond 2^53-1. This is acceptable because: (1) at 1M requests/sec it takes ~285 years to overflow, (2) the counter resets per-process, and (3) these are observability IDs, not critical distributed IDs. Precision loss is an acceptable trade-off for this use case.
Applied to files:
src/bun.js/bindings/FuzzilliREPRL.cpp
📚 Learning: 2025-10-18T20:50:47.750Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:366-373
Timestamp: 2025-10-18T20:50:47.750Z
Learning: In Bun's Zig codebase (src/bun.js/bindings/JSValue.zig), the JSValue enum uses `.null` (not `.js_null`) for JavaScript's null value. Only `js_undefined` has the `js_` prefix to avoid collision with Zig's built-in `undefined` keyword. The correct enum fields are: `js_undefined`, `null`, `true`, `false`, and `zero`.
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cpp
📚 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/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-10-11T15:19:30.301Z
Learnt from: mastermakrela
Repo: oven-sh/bun PR: 19167
File: src/bun.js/api.zig:31-31
Timestamp: 2025-10-11T15:19:30.301Z
Learning: The learnings about `pub const js = JSC.Codegen.JS<ClassName>` and re-exporting toJS/fromJS/fromJSDirect apply to class-based Zig bindings with constructors and prototype methods (e.g., those with `new` constructors). They do NOT apply to simple namespace-style API objects like TOMLObject, YAMLObject, and CSVObject which expose static functions via a `create()` method that returns a plain JS object.
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-11-12T04:11:52.293Z
Learnt from: cirospaciari
Repo: oven-sh/bun PR: 24622
File: src/deps/uws/us_socket_t.zig:112-113
Timestamp: 2025-11-12T04:11:52.293Z
Learning: In Bun's Zig codebase, when passing u32 values to C FFI functions that expect c_uint parameters, no explicit intCast is needed because c_uint is equivalent to u32 on Bun's target platforms and Zig allows implicit coercion between equivalent types. This pattern is used consistently throughout src/deps/uws/us_socket_t.zig in functions like setTimeout, setLongTimeout, and setKeepalive.
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-09-05T19:49:26.188Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: src/valkey/js_valkey_functions.zig:852-867
Timestamp: 2025-09-05T19:49:26.188Z
Learning: In Bun’s Zig code, `.js_undefined` is a valid and preferred JSValue literal for “undefined” (e.g., resolving JSPromise). Do not refactor usages to `jsc.JSValue.jsUndefined()`, especially in src/valkey/js_valkey_functions.zig unsubscribe().
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-10-16T02:17:35.237Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/analytics.zig:15-21
Timestamp: 2025-10-16T02:17:35.237Z
Learning: In src/analytics.zig and similar files using bun.EnvVar boolean environment variables: the new EnvVar API for boolean flags (e.g., bun.EnvVar.do_not_track.get(), bun.EnvVar.ci.get()) is designed to parse and return boolean values from environment variables, not just check for their presence. This is an intentional design change from the previous presence-based checks using bun.getenvZ().
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cpp
🧬 Code graph analysis (1)
src/bun.js/bindings/ZigGlobalObject.cpp (1)
src/bun.js/bindings/FuzzilliREPRL.cpp (2)
Bun__REPRL__registerFuzzilliFunctions(264-294)Bun__REPRL__registerFuzzilliFunctions(264-264)
🪛 Biome (2.1.2)
src/js/internal/fuzzilli-reprl.ts
[error] 308-308: eval() exposes to security risks and performance issues.
See the MDN web docs for more details.
Refactor the code so that it doesn't need to call eval().
(lint/security/noGlobalEval)
🪛 GitHub Check: Lint JavaScript
src/js/internal/fuzzilli-reprl.ts
[failure] 257-257: eslint(no-unused-vars)
Catch parameter 'e' is caught but never used.
[failure] 256-256: eslint(no-unused-vars)
Variable 'stat' is declared but never used. Unused variables should start with a '_'.
[failure] 238-238: eslint(no-unused-vars)
Parameter 'pattern' is declared but never used. Unused parameters should start with a '_'.
[failure] 223-223: eslint(no-unused-vars)
Variable 'params' is declared but never used. Unused variables should start with a '_'.
[failure] 223-223: eslint(no-unused-vars)
Parameter 'sql' is declared but never used. Unused parameters should start with a '_'.
[failure] 217-217: eslint(no-unused-vars)
Parameter 'end' is declared but never used. Unused parameters should start with a '_'.
[failure] 217-217: eslint(no-unused-vars)
Parameter 'start' is declared but never used. Unused parameters should start with a '_'.
[failure] 8-8: eslint(no-unused-vars)
Variable 'REPRL_DWFD' is declared but never used. Unused variables should start with a '_'.
🔇 Additional comments (2)
src/bun.js/bindings/ZigGlobalObject.cpp (1)
261-264: Fuzzilli registration wiring looks correct and well‑gatedThe
extern "C"declaration and the call fromZig__GlobalObject__createare correctly guarded byFUZZILLI_ENABLEDand additionally byBUN_FUZZILLI_MODE, so normal debug/release globals remain untouched and the registration happens only in fuzzilli builds.No changes needed here; remaining concerns are in the implementation of
Bun__REPRL__registerFuzzilliFunctionsitself inFuzzilliREPRL.cpp(stubs/portability), which are covered in that file’s comments.Also applies to: 509-514
package.json (1)
36-36: Fuzzilli debug build script is consistent and isolatedThe
build:debug:fuzzilliscript correctly mirrorsbuild:debugwhile targetingbuild/debug-fuzzand passing-DENABLE_FUZZILLI=ON, so it shouldn’t affect normal debug builds.Looks good.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/cli/fuzzilli_command.zig (2)
1-1: Replace rawextern setenvwith a portable wrapper or gate compilation to POSIXThis file still declares and uses a raw
extern "c" fn setenv(...), which assumes asetenvsymbol exists on all targets whereenable_fuzzillimight be enabled. On Windows toolchains this is not guaranteed and can produce link failures once fuzzilli is turned on there.Prefer one of:
- Use a Zig/std/libuv helper that abstracts
setenv(e.g.std.posix.setenv/setenvZoruv_os_setenvviasrc/deps/libuv.zig), and drop the manual extern; or- Ensure this command (and the call to
setenv) is only compiled for POSIX targets inbuild.zig/CMake, not just guarded at runtime withEnvironment.isPosix.This keeps the “stop‑gap” fuzzilli mode from accidentally breaking non‑POSIX builds when fuzzing support is enabled.
If you want to double‑check existing helpers and gating, you can run:
#!/bin/bash # Inspect existing environment mutators and fuzzilli build gating rg -n "uv_os_setenv" src --type=zig || echo "no uv_os_setenv found" rg -n "setenvZ" src --type=zig || echo "no std.posix.setenvZ found" rg -n "enable_fuzzilli" src --type=zig -C3
23-49: Temp file handling is okay for now but fragile for parallel runs and non‑standard POSIX envsUsing
/tmp/bun-fuzzilli-reprl.jsviastd.fs.cwd().openDir("/tmp", .{})works for Linux/macOS, but it hardcodes:
- The temp directory (
/tmp) instead of using a platform‑aware temp helper, and- A single fixed filename, so multiple fuzzilli instances on the same host could stomp each other’s harness file.
Given this is a stop‑gap, it’s acceptable, but consider (when convenient):
- Using a temp‑dir helper (e.g.,
std.fs.tmpDir/equivalent Bun helper), and- Including PID or random suffix in the filename and optionally unlinking it after
Run.boot.src/js/fuzz/fuzzilli-reprl.ts (1)
5-8: Fix REPRL harness lint failures and explicitly justifyevalusageBiome/eslint errors reported in CI are still present:
REPRL_DWFDis declared but unused.- Several parameters/locals are unused (
start,end,pattern,sql,params,stat, catch parame).evalis used without any annotation, triggeringnoGlobalEval.These keep the PR red but are easy to fix without changing behavior. Suggested patch:
@@ -const REPRL_CRFD = 100; // Control read FD -const REPRL_CWFD = 101; // Control write FD -const REPRL_DRFD = 102; // Data read FD -const REPRL_DWFD = 103; // Data write FD +const REPRL_CRFD = 100; // Control read FD +const REPRL_CWFD = 101; // Control write FD +const REPRL_DRFD = 102; // Data read FD +const _REPRL_DWFD = 103; // Data write FD (used only on the C++ side) @@ -globalThis.MOCK_FILE = { +globalThis.MOCK_FILE = { @@ - slice: (start, end) => globalThis.MOCK_FILE, + slice: (_start, _end) => globalThis.MOCK_FILE, @@ -globalThis.MOCK_DB = { - query: (sql, ...params) => ({ +globalThis.MOCK_DB = { + query: (_sql, ..._params) => ({ @@ -globalThis.MOCK_GLOB = { - scan: pattern => ({ +globalThis.MOCK_GLOB = { + scan: (_pattern) => ({ @@ -try { - // Try to stat fd 100 to see if it exists - const stat = fs.fstatSync(REPRL_CRFD); -} catch (e) { +try { + // Try to stat fd 100 to see if it exists + fs.fstatSync(REPRL_CRFD); +} catch (_err) { @@ - try { - // Use indirect eval to execute in global scope - (0, eval)(script); - } catch (e) { + try { + // Use indirect eval to execute in global scope + // biome-ignore lint/security/noGlobalEval: REPRL must execute arbitrary fuzzed scripts + // eslint-disable-next-line no-eval + (0, eval)(script); + } catch (e) {This keeps behavior intact while satisfying Biome/eslint and documenting why
evalis necessary in this context.After applying, rerun the repo’s JS lint task (e.g.
bun run lintor the existing “Lint JavaScript” CI job) to confirm the harness is clean.Also applies to: 176-247, 252-261, 304-323
📜 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.
📒 Files selected for processing (2)
src/cli/fuzzilli_command.zig(1 hunks)src/js/fuzz/fuzzilli-reprl.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:37.256Z
Learning: In the Bun codebase, FFI (Foreign Function Interface) code is compiled separately using TinyCC (tcc), which barely supports C99. Headers like src/bun.js/api/FFI.h and src/bun.js/api/ffi-stdbool.h are only used for FFI compilation with tcc, not with the main compiler. Therefore, C standard changes to the main Bun target do not affect FFI code compilation.
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:38.580Z
Learning: In the Bun codebase, FFI is compiled with tcc (TinyCC), which barely supports C99. The headers `src/bun.js/api/FFI.h` and `src/bun.js/api/ffi-stdbool.h` are only used for FFI compilation with tcc, not for the main Bun target. Therefore, C23 compatibility concerns (such as bool/true/false keyword conflicts) do not apply to these FFI headers.
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 22606
File: src/glob/GlobWalker.zig:449-452
Timestamp: 2025-09-12T18:16:50.754Z
Learning: For Bun codebase: prefer using `std.fs.path.sep` over manual platform separator detection, and use `bun.strings.lastIndexOfChar` instead of `std.mem.lastIndexOfScalar` for string operations.
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.
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24423
File: test/js/node/path/browserify.test.js:472-475
Timestamp: 2025-11-07T17:28:58.045Z
Learning: markovejnovic prefers functional programming patterns (e.g., using reduce) over imperative loops and does not want style-based readability suggestions unless there's a concrete technical reason beyond personal preference.
<!--
📚 Learning: 2025-10-19T04:55:33.099Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/http/node-telemetry.test.ts:27-203
Timestamp: 2025-10-19T04:55:33.099Z
Learning: In test/js/bun/http/node-telemetry.test.ts and the Bun.telemetry._node_binding API, after the architecture refactor, the _node_binding interface only contains two methods: handleIncomingRequest(req, res) and handleWriteHead(res, statusCode). The handleRequestFinish hook and other lifecycle hooks were removed during simplification. Both current methods are fully tested.
Applied to files:
src/js/fuzz/fuzzilli-reprl.ts
📚 Learning: 2025-11-14T16:07:01.064Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
Applied to files:
src/js/fuzz/fuzzilli-reprl.tssrc/cli/fuzzilli_command.zig
📚 Learning: 2025-10-15T20:19:37.256Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:37.256Z
Learning: In the Bun codebase, FFI (Foreign Function Interface) code is compiled separately using TinyCC (tcc), which barely supports C99. Headers like src/bun.js/api/FFI.h and src/bun.js/api/ffi-stdbool.h are only used for FFI compilation with tcc, not with the main compiler. Therefore, C standard changes to the main Bun target do not affect FFI code compilation.
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-10-16T02:17:35.237Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/analytics.zig:15-21
Timestamp: 2025-10-16T02:17:35.237Z
Learning: In src/analytics.zig and similar files using bun.EnvVar boolean environment variables: the new EnvVar API for boolean flags (e.g., bun.EnvVar.do_not_track.get(), bun.EnvVar.ci.get()) is designed to parse and return boolean values from environment variables, not just check for their presence. This is an intentional design change from the previous presence-based checks using bun.getenvZ().
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-09-12T22:30:48.490Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 22613
File: src/cli/package_manager_command.zig:0-0
Timestamp: 2025-09-12T22:30:48.490Z
Learning: Commands like `bun info` and `bun whoami` that are promoted from pm-only subcommands to top-level commands use manual `bun.argv` parsing to detect direct invocation vs. pm invocation. This is different from regular commands that should use `ctx.positionals`.
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-10-16T17:32:03.074Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/install/PackageManager/PackageManagerOptions.zig:187-193
Timestamp: 2025-10-16T17:32:03.074Z
Learning: In Bun's codebase (particularly in files like src/install/PackageManager/PackageManagerOptions.zig), mixing bun.EnvVar.*.get() and bun.EnvVar.*.platformGet() for environment variable lookups is intentional and safe. The code is protected by compile-time platform checks (Environment.isWindows, etc.), and compilation will fail if the wrong function is used on the wrong platform. This pattern should not be flagged as a consistency issue.
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-10-16T21:24:52.779Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/crash_handler.zig:1415-1423
Timestamp: 2025-10-16T21:24:52.779Z
Learning: When a boolean EnvVar in src/envvars.zig is defined with a default value (e.g., `.default = false`), the `get()` method returns `bool` instead of `?bool`. This means you cannot distinguish between "environment variable not set" and "environment variable explicitly set to the default value". For opt-out scenarios where detection of explicit setting is needed (like `BUN_ENABLE_CRASH_REPORTING` on platforms where crash reporting defaults to enabled), either: (1) don't provide a default value so `get()` returns `?bool`, or (2) use the returned boolean directly instead of only checking if it's true.
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-10-15T22:03:50.832Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/envvars.zig:135-144
Timestamp: 2025-10-15T22:03:50.832Z
Learning: In src/envvars.zig, the boolean feature flag cache uses a single atomic enum and should remain monotonic. Only the string cache (which uses two atomics: ptr and len) requires acquire/release ordering to prevent torn reads.
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-10-24T10:43:09.398Z
Learnt from: fmguerreiro
Repo: oven-sh/bun PR: 23774
File: src/install/PackageManager/updatePackageJSONAndInstall.zig:548-548
Timestamp: 2025-10-24T10:43:09.398Z
Learning: In Bun's Zig codebase, the `as(usize, intCast(...))` cast pattern triggers a Zig compiler bug that causes compilation to hang indefinitely when used in complex control flow contexts (loops + short-circuit operators + optional unwrapping). Avoid this pattern and use simpler alternatives like just `intCast(...)` if type casting is necessary.
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-11-10T00:57:09.173Z
Learnt from: franciscop
Repo: oven-sh/bun PR: 24514
File: src/bun.js/api/crypto/PasswordObject.zig:86-101
Timestamp: 2025-11-10T00:57:09.173Z
Learning: In Bun's Zig codebase (PasswordObject.zig), when validating the parallelism parameter for Argon2, the upper limit is set to 65535 (2^16 - 1) rather than using `std.math.maxInt(u24)` because the latter triggers Zig's truncation limit checks. The value 65535 is a practical upper bound that avoids compiler issues while being sufficient for thread parallelism use cases.
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-11-12T04:11:52.293Z
Learnt from: cirospaciari
Repo: oven-sh/bun PR: 24622
File: src/deps/uws/us_socket_t.zig:112-113
Timestamp: 2025-11-12T04:11:52.293Z
Learning: In Bun's Zig codebase, when passing u32 values to C FFI functions that expect c_uint parameters, no explicit intCast is needed because c_uint is equivalent to u32 on Bun's target platforms and Zig allows implicit coercion between equivalent types. This pattern is used consistently throughout src/deps/uws/us_socket_t.zig in functions like setTimeout, setLongTimeout, and setKeepalive.
Applied to files:
src/cli/fuzzilli_command.zig
📚 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:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-10-15T20:19:38.580Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:38.580Z
Learning: In the Bun codebase, FFI is compiled with tcc (TinyCC), which barely supports C99. The headers `src/bun.js/api/FFI.h` and `src/bun.js/api/ffi-stdbool.h` are only used for FFI compilation with tcc, not for the main Bun target. Therefore, C23 compatibility concerns (such as bool/true/false keyword conflicts) do not apply to these FFI headers.
Applied to files:
src/cli/fuzzilli_command.zig
🪛 Biome (2.1.2)
src/js/fuzz/fuzzilli-reprl.ts
[error] 308-308: eval() exposes to security risks and performance issues.
See the MDN web docs for more details.
Refactor the code so that it doesn't need to call eval().
(lint/security/noGlobalEval)
🪛 GitHub Check: Lint JavaScript
src/js/fuzz/fuzzilli-reprl.ts
[failure] 257-257: eslint(no-unused-vars)
Catch parameter 'e' is caught but never used.
[failure] 256-256: eslint(no-unused-vars)
Variable 'stat' is declared but never used. Unused variables should start with a '_'.
[failure] 238-238: eslint(no-unused-vars)
Parameter 'pattern' is declared but never used. Unused parameters should start with a '_'.
[failure] 223-223: eslint(no-unused-vars)
Variable 'params' is declared but never used. Unused variables should start with a '_'.
[failure] 223-223: eslint(no-unused-vars)
Parameter 'sql' is declared but never used. Unused parameters should start with a '_'.
[failure] 217-217: eslint(no-unused-vars)
Parameter 'end' is declared but never used. Unused parameters should start with a '_'.
[failure] 217-217: eslint(no-unused-vars)
Parameter 'start' is declared but never used. Unused parameters should start with a '_'.
[failure] 8-8: eslint(no-unused-vars)
Variable 'REPRL_DWFD' is declared but never used. Unused variables should start with a '_'.
| // Read script size (8 bytes, little-endian) | ||
| const size_bytes = Buffer.alloc(8); | ||
| fs.readSync(REPRL_CRFD, size_bytes, 0, 8, null); | ||
| const script_size = Number(size_bytes.readBigUInt64LE(0)); | ||
|
|
||
| // Read script data from REPRL_DRFD | ||
| const script_data = Buffer.alloc(script_size); | ||
| let total_read = 0; | ||
| while (total_read < script_size) { | ||
| const n = fs.readSync(REPRL_DRFD, script_data, total_read, script_size - total_read, null); | ||
| if (n === 0) break; | ||
| total_read += n; | ||
| } | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Handle partial reads and truncation in REPRL script-size/data reads
The REPRL loop currently assumes:
fs.readSync(REPRL_CRFD, size_bytes, 0, 8, null)always returns 8 bytes, and- The loop reading from
REPRL_DRFDeither fills the entirescript_databuffer or hits EOF, but you never check whethertotal_read === script_size.
If the controller or OS ever delivers a short read (valid for pipes/sockets) or closes the FD early, you may end up:
- Interpreting an invalid
script_size, or - Executing a truncated script, which can cause noisy failures and make fuzzing less deterministic.
Suggest tightening this logic, e.g.:
- const size_bytes = Buffer.alloc(8);
- fs.readSync(REPRL_CRFD, size_bytes, 0, 8, null);
+ const size_bytes = Buffer.alloc(8);
+ const sizeRead = fs.readSync(REPRL_CRFD, size_bytes, 0, 8, null);
+ if (sizeRead !== 8) {
+ throw new Error(`REPRL: expected 8 bytes for script size, got ${sizeRead}`);
+ }
@@
- const script_data = Buffer.alloc(script_size);
- let total_read = 0;
- while (total_read < script_size) {
- const n = fs.readSync(REPRL_DRFD, script_data, total_read, script_size - total_read, null);
- if (n === 0) break;
- total_read += n;
- }
-
- const script = script_data.toString("utf8");
+ const script_data = Buffer.alloc(script_size);
+ let total_read = 0;
+ while (total_read < script_size) {
+ const n = fs.readSync(
+ REPRL_DRFD,
+ script_data,
+ total_read,
+ script_size - total_read,
+ null,
+ );
+ if (n === 0) {
+ throw new Error(
+ `REPRL: unexpected EOF while reading script (read ${total_read} of ${script_size} bytes)`,
+ );
+ }
+ total_read += n;
+ }
+
+ const script = script_data.toString("utf8");This makes the harness more robust without changing the external protocol.
You can sanity‑check this behavior by fuzzing with an instrumented harness or by temporarily injecting a fake short‑read path (e.g., manually forcing sizeRead or n to be less than requested) to ensure the error path triggers instead of executing truncated scripts.
I'll verify the code and gather the necessary context to assess this review comment.
---
🏁 Script executed:
```shell
#!/bin/bash
# Verify the code exists at the specified location and get surrounding context
cat src/js/fuzz/fuzzilli-reprl.ts | sed -n '280,310p'
Length of output: 895
🏁 Script executed:
#!/bin/bash
# Find REPRL_CRFD and REPRL_DRFD definitions and usage
rg -n "REPRL_CRFD|REPRL_DRFD" src/js/fuzz/fuzzilli-reprl.ts -B2 -A2Length of output: 1342
🏁 Script executed:
#!/bin/bash
# Search for any error handling or validation patterns in the file
rg -n "readSync" src/js/fuzz/fuzzilli-reprl.ts -B1 -A3Length of output: 798
🏁 Script executed:
#!/bin/bash
# Look for REPRL protocol documentation or comments
rg -n "REPRL" src/js/fuzz/fuzzilli-reprl.ts | head -20Length of output: 1184
🏁 Script executed:
#!/bin/bash
# Check if there are any tests or other files that might reference this behavior
fd "reprl" --type fLength of output: 67
🏁 Script executed:
#!/bin/bash
# Look for any fuzzing-related documentation
fd -e md -e txt | xargs grep -l "REPRL" 2>/dev/null || echo "No REPRL docs found"Length of output: 19
Validate REPRL script-size and ensure complete data reads
The code at line 290 lacks validation of the bytes returned from reading the script size, and the loop at lines 296–300 breaks on EOF without verifying that the entire script was read. This creates two problems:
- Line 290 does not check the return value from
fs.readSync, unlike the HELO response validation at line 268–270, risking interpretation of invalid/uninitialized data asscript_size. - Lines 296–300 break silently on EOF (
if (n === 0) break) without confirmingtotal_read === script_size, allowing truncated scripts to execute.
Implement the validation to match the existing pattern:
// Read script size (8 bytes, little-endian)
const size_bytes = Buffer.alloc(8);
- fs.readSync(REPRL_CRFD, size_bytes, 0, 8, null);
+ const sizeRead = fs.readSync(REPRL_CRFD, size_bytes, 0, 8, null);
+ if (sizeRead !== 8) {
+ throw new Error(`REPRL: expected 8 bytes for script size, got ${sizeRead}`);
+ }
const script_size = Number(size_bytes.readBigUInt64LE(0));
// Read script data from REPRL_DRFD
const script_data = Buffer.alloc(script_size);
let total_read = 0;
while (total_read < script_size) {
const n = fs.readSync(REPRL_DRFD, script_data, total_read, script_size - total_read, null);
- if (n === 0) break;
+ if (n === 0) {
+ throw new Error(
+ `REPRL: unexpected EOF while reading script (read ${total_read} of ${script_size} bytes)`,
+ );
+ }
total_read += n;
}
const script = script_data.toString("utf8");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Read script size (8 bytes, little-endian) | |
| const size_bytes = Buffer.alloc(8); | |
| fs.readSync(REPRL_CRFD, size_bytes, 0, 8, null); | |
| const script_size = Number(size_bytes.readBigUInt64LE(0)); | |
| // Read script data from REPRL_DRFD | |
| const script_data = Buffer.alloc(script_size); | |
| let total_read = 0; | |
| while (total_read < script_size) { | |
| const n = fs.readSync(REPRL_DRFD, script_data, total_read, script_size - total_read, null); | |
| if (n === 0) break; | |
| total_read += n; | |
| } | |
| // Read script size (8 bytes, little-endian) | |
| const size_bytes = Buffer.alloc(8); | |
| const sizeRead = fs.readSync(REPRL_CRFD, size_bytes, 0, 8, null); | |
| if (sizeRead !== 8) { | |
| throw new Error(`REPRL: expected 8 bytes for script size, got ${sizeRead}`); | |
| } | |
| const script_size = Number(size_bytes.readBigUInt64LE(0)); | |
| // Read script data from REPRL_DRFD | |
| const script_data = Buffer.alloc(script_size); | |
| let total_read = 0; | |
| while (total_read < script_size) { | |
| const n = fs.readSync(REPRL_DRFD, script_data, total_read, script_size - total_read, null); | |
| if (n === 0) { | |
| throw new Error( | |
| `REPRL: unexpected EOF while reading script (read ${total_read} of ${script_size} bytes)`, | |
| ); | |
| } | |
| total_read += n; | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
src/cli/fuzzilli_command.zig (1)
1-1: Rawsetenvextern is still a portability risk; prefer a Zig/libuv wrapper or POSIX-only build gatingThe top‑level
extern "c" fn setenv(...)still requires asetenvsymbol to exist in the target libc, which is not guaranteed on all non‑POSIX toolchains (e.g. some Windows builds), and this file also pulls instd.posix. Even though the command is runtime‑guarded and feature‑gated viabun.Environment.enable_fuzzilli, the extern itself and any compiled call sites can still break non‑POSIX builds.Given this is intended as a Linux/CI‑only stop‑gap, I’d strongly recommend either:
- Route the environment mutation through an existing portable helper (e.g. a
std_posix.setenvZoruv_os_setenvwrapper) and delete this raw extern, or- Explicitly gate this file / command to POSIX targets at build time so it is never compiled or linked on Windows.
This addresses the earlier portability concern without changing the intended behavior of the fuzzilli command.
Please double‑check your non‑POSIX build jobs (especially Windows) after making this change to confirm there are no new link/compile failures in the fuzzilli build configuration.
src/js/eval/fuzzilli-reprl.ts (4)
288-300: Validate script size read and enforce full script data read before evalThe REPRL loop still:
- Ignores the return value of
fs.readSyncwhen reading the 8‑byte script size.- Allows the data loop to break on
n === 0without checking thattotal_read === script_size.Short reads or early EOF on the control/data FDs would then produce an invalid
script_sizeor execute a truncated script, which is brittle under fuzzing.Recommend tightening this to mirror your HELO handshake check:
// Read script size (8 bytes, little-endian) const size_bytes = Buffer.alloc(8); - fs.readSync(REPRL_CRFD, size_bytes, 0, 8, null); + const sizeRead = fs.readSync(REPRL_CRFD, size_bytes, 0, 8, null); + if (sizeRead !== 8) { + throw new Error(`REPRL: expected 8 bytes for script size, got ${sizeRead}`); + } const script_size = Number(size_bytes.readBigUInt64LE(0)); // Read script data from REPRL_DRFD const script_data = Buffer.alloc(script_size); let total_read = 0; while (total_read < script_size) { const n = fs.readSync(REPRL_DRFD, script_data, total_read, script_size - total_read, null); - if (n === 0) break; + if (n === 0) { + throw new Error( + `REPRL: unexpected EOF while reading script (read ${total_read} of ${script_size} bytes)`, + ); + } total_read += n; }This keeps the protocol simple but avoids silently executing garbage or truncated inputs.
After applying this, it’s worth running a short fuzz run to ensure no unexpected protocol mismatches (e.g. by watching for the new error messages instead of unexplained crashes).
305-313: Document and suppress intentionalevalusage to satisfy security lintThe indirect
evalis intrinsic to executing fuzzed scripts, but Biome/eslint still flag it:// Use indirect eval to execute in global scope (0, eval)(script);Rather than trying to avoid
eval, explicitly document why it’s required and suppress the lint at this call site:let exit_code = 0; try { - // Use indirect eval to execute in global scope - (0, eval)(script); + // Use indirect eval to execute in global scope. + // biome-ignore lint/security/noGlobalEval: REPRL must execute arbitrary fuzzed scripts. + // eslint-disable-next-line no-eval + (0, eval)(script); } catch (e) {This makes the intent clear and should unblock the security rule without changing behavior.
Please re‑run your JS lint job (Biome + eslint) after this change to confirm the warnings on this line are gone and no new ones appear.
5-8: MarkREPRL_DWFDas intentionally unused to satisfy lint rules
REPRL_DWFDis declared but not used on the JS side, which matches the intent (data write is driven from the C++/REPRL controller). Lint is flagging this as an unused variable.A minimal fix is to rename it with the conventional unused prefix:
-const REPRL_DWFD = 103; // Data write FD +const _REPRL_DWFD = 103; // Data write FD (used only on the native side)This keeps the documentation while silencing
no-unused-vars.
207-246: Prefix unused parameters in mocks to clearno-unused-varswithout changing behaviorSeveral mock helpers declare parameters that aren’t used (
start,end,sql,params,pattern). ESLint is complaining about these, but they’re useful for keeping the mock signatures realistic.You can fix this by prefixing the parameters with
_:- slice: (start, end) => globalThis.MOCK_FILE, + slice: (_start, _end) => globalThis.MOCK_FILE, @@ -globalThis.MOCK_DB = { - query: (sql, ...params) => ({ +globalThis.MOCK_DB = { + query: (_sql, ..._params) => ({ @@ - prepare: sql => globalThis.MOCK_DB.query(sql), + prepare: (sql) => globalThis.MOCK_DB.query(sql), @@ -globalThis.MOCK_GLOB = { - scan: pattern => ({ +globalThis.MOCK_GLOB = { + scan: (_pattern) => ({(Adjust any other unused params similarly.) This keeps the API shape while satisfying lint.
📜 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.
📒 Files selected for processing (2)
src/cli/fuzzilli_command.zig(1 hunks)src/js/eval/fuzzilli-reprl.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:37.256Z
Learning: In the Bun codebase, FFI (Foreign Function Interface) code is compiled separately using TinyCC (tcc), which barely supports C99. Headers like src/bun.js/api/FFI.h and src/bun.js/api/ffi-stdbool.h are only used for FFI compilation with tcc, not with the main compiler. Therefore, C standard changes to the main Bun target do not affect FFI code compilation.
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:38.580Z
Learning: In the Bun codebase, FFI is compiled with tcc (TinyCC), which barely supports C99. The headers `src/bun.js/api/FFI.h` and `src/bun.js/api/ffi-stdbool.h` are only used for FFI compilation with tcc, not for the main Bun target. Therefore, C23 compatibility concerns (such as bool/true/false keyword conflicts) do not apply to these FFI headers.
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24423
File: test/js/node/path/browserify.test.js:472-475
Timestamp: 2025-11-07T17:28:58.045Z
Learning: markovejnovic prefers functional programming patterns (e.g., using reduce) over imperative loops and does not want style-based readability suggestions unless there's a concrete technical reason beyond personal preference.
<!--
📚 Learning: 2025-10-15T20:19:37.256Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:37.256Z
Learning: In the Bun codebase, FFI (Foreign Function Interface) code is compiled separately using TinyCC (tcc), which barely supports C99. Headers like src/bun.js/api/FFI.h and src/bun.js/api/ffi-stdbool.h are only used for FFI compilation with tcc, not with the main compiler. Therefore, C standard changes to the main Bun target do not affect FFI code compilation.
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-11-14T16:07:01.064Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-09-12T22:30:48.490Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 22613
File: src/cli/package_manager_command.zig:0-0
Timestamp: 2025-09-12T22:30:48.490Z
Learning: Commands like `bun info` and `bun whoami` that are promoted from pm-only subcommands to top-level commands use manual `bun.argv` parsing to detect direct invocation vs. pm invocation. This is different from regular commands that should use `ctx.positionals`.
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-10-16T17:32:03.074Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/install/PackageManager/PackageManagerOptions.zig:187-193
Timestamp: 2025-10-16T17:32:03.074Z
Learning: In Bun's codebase (particularly in files like src/install/PackageManager/PackageManagerOptions.zig), mixing bun.EnvVar.*.get() and bun.EnvVar.*.platformGet() for environment variable lookups is intentional and safe. The code is protected by compile-time platform checks (Environment.isWindows, etc.), and compilation will fail if the wrong function is used on the wrong platform. This pattern should not be flagged as a consistency issue.
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-10-16T02:17:35.237Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/analytics.zig:15-21
Timestamp: 2025-10-16T02:17:35.237Z
Learning: In src/analytics.zig and similar files using bun.EnvVar boolean environment variables: the new EnvVar API for boolean flags (e.g., bun.EnvVar.do_not_track.get(), bun.EnvVar.ci.get()) is designed to parse and return boolean values from environment variables, not just check for their presence. This is an intentional design change from the previous presence-based checks using bun.getenvZ().
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-10-16T21:24:52.779Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/crash_handler.zig:1415-1423
Timestamp: 2025-10-16T21:24:52.779Z
Learning: When a boolean EnvVar in src/envvars.zig is defined with a default value (e.g., `.default = false`), the `get()` method returns `bool` instead of `?bool`. This means you cannot distinguish between "environment variable not set" and "environment variable explicitly set to the default value". For opt-out scenarios where detection of explicit setting is needed (like `BUN_ENABLE_CRASH_REPORTING` on platforms where crash reporting defaults to enabled), either: (1) don't provide a default value so `get()` returns `?bool`, or (2) use the returned boolean directly instead of only checking if it's true.
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-10-15T22:03:50.832Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/envvars.zig:135-144
Timestamp: 2025-10-15T22:03:50.832Z
Learning: In src/envvars.zig, the boolean feature flag cache uses a single atomic enum and should remain monotonic. Only the string cache (which uses two atomics: ptr and len) requires acquire/release ordering to prevent torn reads.
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-10-24T10:43:09.398Z
Learnt from: fmguerreiro
Repo: oven-sh/bun PR: 23774
File: src/install/PackageManager/updatePackageJSONAndInstall.zig:548-548
Timestamp: 2025-10-24T10:43:09.398Z
Learning: In Bun's Zig codebase, the `as(usize, intCast(...))` cast pattern triggers a Zig compiler bug that causes compilation to hang indefinitely when used in complex control flow contexts (loops + short-circuit operators + optional unwrapping). Avoid this pattern and use simpler alternatives like just `intCast(...)` if type casting is necessary.
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-11-10T00:57:09.173Z
Learnt from: franciscop
Repo: oven-sh/bun PR: 24514
File: src/bun.js/api/crypto/PasswordObject.zig:86-101
Timestamp: 2025-11-10T00:57:09.173Z
Learning: In Bun's Zig codebase (PasswordObject.zig), when validating the parallelism parameter for Argon2, the upper limit is set to 65535 (2^16 - 1) rather than using `std.math.maxInt(u24)` because the latter triggers Zig's truncation limit checks. The value 65535 is a practical upper bound that avoids compiler issues while being sufficient for thread parallelism use cases.
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-11-12T04:11:52.293Z
Learnt from: cirospaciari
Repo: oven-sh/bun PR: 24622
File: src/deps/uws/us_socket_t.zig:112-113
Timestamp: 2025-11-12T04:11:52.293Z
Learning: In Bun's Zig codebase, when passing u32 values to C FFI functions that expect c_uint parameters, no explicit intCast is needed because c_uint is equivalent to u32 on Bun's target platforms and Zig allows implicit coercion between equivalent types. This pattern is used consistently throughout src/deps/uws/us_socket_t.zig in functions like setTimeout, setLongTimeout, and setKeepalive.
Applied to files:
src/cli/fuzzilli_command.zig
📚 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:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-10-15T20:19:38.580Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:38.580Z
Learning: In the Bun codebase, FFI is compiled with tcc (TinyCC), which barely supports C99. The headers `src/bun.js/api/FFI.h` and `src/bun.js/api/ffi-stdbool.h` are only used for FFI compilation with tcc, not for the main Bun target. Therefore, C23 compatibility concerns (such as bool/true/false keyword conflicts) do not apply to these FFI headers.
Applied to files:
src/cli/fuzzilli_command.zig
📚 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:
src/js/eval/fuzzilli-reprl.ts
🪛 Biome (2.1.2)
src/js/eval/fuzzilli-reprl.ts
[error] 308-308: eval() exposes to security risks and performance issues.
See the MDN web docs for more details.
Refactor the code so that it doesn't need to call eval().
(lint/security/noGlobalEval)
🪛 GitHub Check: Lint JavaScript
src/js/eval/fuzzilli-reprl.ts
[failure] 257-257: eslint(no-unused-vars)
Catch parameter 'e' is caught but never used.
[failure] 256-256: eslint(no-unused-vars)
Variable 'stat' is declared but never used. Unused variables should start with a '_'.
[failure] 238-238: eslint(no-unused-vars)
Parameter 'pattern' is declared but never used. Unused parameters should start with a '_'.
[failure] 223-223: eslint(no-unused-vars)
Variable 'params' is declared but never used. Unused variables should start with a '_'.
[failure] 223-223: eslint(no-unused-vars)
Parameter 'sql' is declared but never used. Unused parameters should start with a '_'.
[failure] 217-217: eslint(no-unused-vars)
Parameter 'end' is declared but never used. Unused parameters should start with a '_'.
[failure] 217-217: eslint(no-unused-vars)
Parameter 'start' is declared but never used. Unused parameters should start with a '_'.
[failure] 8-8: eslint(no-unused-vars)
Variable 'REPRL_DWFD' is declared but never used. Unused variables should start with a '_'.
There was a problem hiding this comment.
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.
📒 Files selected for processing (1)
src/bun.js/bindings/ZigGlobalObject.cpp(2 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24423
File: test/js/node/path/browserify.test.js:472-475
Timestamp: 2025-11-07T17:28:58.045Z
Learning: markovejnovic prefers functional programming patterns (e.g., using reduce) over imperative loops and does not want style-based readability suggestions unless there's a concrete technical reason beyond personal preference.
<!--
📚 Learning: 2025-10-15T20:19:37.256Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:37.256Z
Learning: In the Bun codebase, FFI (Foreign Function Interface) code is compiled separately using TinyCC (tcc), which barely supports C99. Headers like src/bun.js/api/FFI.h and src/bun.js/api/ffi-stdbool.h are only used for FFI compilation with tcc, not with the main compiler. Therefore, C standard changes to the main Bun target do not affect FFI code compilation.
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-10-15T20:19:38.580Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:38.580Z
Learning: In the Bun codebase, FFI is compiled with tcc (TinyCC), which barely supports C99. The headers `src/bun.js/api/FFI.h` and `src/bun.js/api/ffi-stdbool.h` are only used for FFI compilation with tcc, not for the main Bun target. Therefore, C23 compatibility concerns (such as bool/true/false keyword conflicts) do not apply to these FFI headers.
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-10-18T20:50:47.750Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:366-373
Timestamp: 2025-10-18T20:50:47.750Z
Learning: In Bun's Zig codebase (src/bun.js/bindings/JSValue.zig), the JSValue enum uses `.null` (not `.js_null`) for JavaScript's null value. Only `js_undefined` has the `js_` prefix to avoid collision with Zig's built-in `undefined` keyword. The correct enum fields are: `js_undefined`, `null`, `true`, `false`, and `zero`.
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-11-03T20:43:06.996Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24273
File: src/bun.js/test/snapshot.zig:19-19
Timestamp: 2025-11-03T20:43:06.996Z
Learning: In Bun's Zig codebase, when storing JSValue objects in collections like ArrayList, use `jsc.Strong.Optional` (not raw JSValue). When adding values, wrap them with `jsc.Strong.Optional.create(value, globalThis)`. In cleanup code, iterate the collection calling `.deinit()` on each Strong.Optional item before calling `.deinit()` on the ArrayList itself. This pattern automatically handles GC protection. See examples in src/bun.js/test/ScopeFunctions.zig and src/bun.js/node/node_cluster_binding.zig.
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cpp
📚 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/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-09-20T05:35:57.318Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 22534
File: src/bun.js/bindings/headers.h:729-731
Timestamp: 2025-09-20T05:35:57.318Z
Learning: symbols.txt in the Bun codebase is specifically for V8 API mangled symbols (without leading underscore), not for general Bun host functions declared with BUN_DECLARE_HOST_FUNCTION. Host functions are handled through different build mechanisms.
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-10-11T15:19:30.301Z
Learnt from: mastermakrela
Repo: oven-sh/bun PR: 19167
File: src/bun.js/api.zig:31-31
Timestamp: 2025-10-11T15:19:30.301Z
Learning: The learnings about `pub const js = JSC.Codegen.JS<ClassName>` and re-exporting toJS/fromJS/fromJSDirect apply to class-based Zig bindings with constructors and prototype methods (e.g., those with `new` constructors). They do NOT apply to simple namespace-style API objects like TOMLObject, YAMLObject, and CSVObject which expose static functions via a `create()` method that returns a plain JS object.
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-09-05T19:49:26.188Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: src/valkey/js_valkey_functions.zig:852-867
Timestamp: 2025-09-05T19:49:26.188Z
Learning: In Bun’s Zig code, `.js_undefined` is a valid and preferred JSValue literal for “undefined” (e.g., resolving JSPromise). Do not refactor usages to `jsc.JSValue.jsUndefined()`, especially in src/valkey/js_valkey_functions.zig unsubscribe().
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-11-12T04:11:52.293Z
Learnt from: cirospaciari
Repo: oven-sh/bun PR: 24622
File: src/deps/uws/us_socket_t.zig:112-113
Timestamp: 2025-11-12T04:11:52.293Z
Learning: In Bun's Zig codebase, when passing u32 values to C FFI functions that expect c_uint parameters, no explicit intCast is needed because c_uint is equivalent to u32 on Bun's target platforms and Zig allows implicit coercion between equivalent types. This pattern is used consistently throughout src/deps/uws/us_socket_t.zig in functions like setTimeout, setLongTimeout, and setKeepalive.
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-09-02T18:25:27.976Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22227
File: src/allocators/allocation_scope.zig:284-314
Timestamp: 2025-09-02T18:25:27.976Z
Learning: In bun's custom Zig implementation, the `#` prefix for private fields is valid syntax and should not be flagged as invalid. The syntax `#fieldname` creates private fields that cannot be accessed from outside the defining struct, and usage like `self.#fieldname` is correct within the same struct. This applies to fields like `#parent`, `#state`, `#allocator`, `#trace`, etc. throughout the codebase.
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cpp
🧬 Code graph analysis (1)
src/bun.js/bindings/ZigGlobalObject.cpp (1)
src/bun.js/bindings/FuzzilliREPRL.cpp (2)
Bun__REPRL__registerFuzzilliFunctions(264-294)Bun__REPRL__registerFuzzilliFunctions(264-264)
🔇 Additional comments (1)
src/bun.js/bindings/ZigGlobalObject.cpp (1)
509-511: Placement of Fuzzilli global registration in GlobalObject creation is appropriateRegistering the Fuzzilli helpers right after
JSC::gcProtect(globalObject)and underFUZZILLI_ENABLEDensures:
- No effect on standard debug/release builds when the macro is unset.
- The helpers run with a fully constructed
Zig::GlobalObject*before other runtime wiring, which is the right time to attach globals and signal handlers for the fuzz harness.No changes requested here.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
src/bun.js/bindings/FuzzilliREPRL.cpp (3)
1-17: CRITICAL: POSIX headers will break Windows builds—apply platform guards.Past reviews flagged this as a critical issue and marked it "addressed," but the current code still includes POSIX-only headers (lines 12, 14-16) without
#ifndef _WIN32guards. WhenFUZZILLI_ENABLEDis defined on Windows, the build will fail because these headers don't exist.The PR reports actual build failures on multiple platforms, confirming this is an active blocker.
Apply platform guards to fix Windows builds:
#ifdef FUZZILLI_ENABLED #include "JavaScriptCore/CallFrame.h" #include "JavaScriptCore/Identifier.h" #include "JavaScriptCore/JSGlobalObject.h" #include "ZigGlobalObject.h" #include "root.h" #include "wtf/text/WTFString.h" #include <cerrno> #include <csignal> #include <cstdlib> #include <cstring> + +#ifndef _WIN32 #include <fcntl.h> -#include <sanitizer/asan_interface.h> #include <sys/mman.h> #include <sys/stat.h> #include <unistd.h> +#endifNote: The
<sanitizer/asan_interface.h>include (line 13) appears unused in this file and can be removed unless it's required for other reasons.
23-34: Guard signal handler for POSIX-only systems.The signal handler uses
fsync()andSTDOUT_FILENO/STDERR_FILENO, which are POSIX-only. When compiling for Windows, these will cause build failures.Wrap the signal handler definition and its usage with platform guards:
+#ifndef _WIN32 // Signal handler to ensure output is flushed before crash static void fuzzilliSignalHandler(int sig) { // Flush all output fflush(stdout); fflush(stderr); fsync(STDOUT_FILENO); fsync(STDERR_FILENO); // Re-raise the signal with default handler signal(sig, SIG_DFL); raise(sig); } +#endifAlso guard the registration at lines 258-261:
void Bun__REPRL__registerFuzzilliFunctions(Zig::GlobalObject* globalObject) { JSC::VM& vm = globalObject->vm(); +#ifndef _WIN32 // Install signal handlers to ensure output is flushed before crashes // This is important for ASAN output to be captured signal(SIGABRT, fuzzilliSignalHandler); signal(SIGSEGV, fuzzilliSignalHandler); signal(SIGILL, fuzzilliSignalHandler); signal(SIGFPE, fuzzilliSignalHandler); +#endif
186-226: Guard coverage initialization for POSIX-only systems.The coverage initialization uses POSIX-specific shared memory APIs (
shm_open,mmap,MAP_SHARED,MAP_FAILED,S_IREAD | S_IWRITE) that don't exist on Windows. This will cause linker failures on Windows builds.Wrap the POSIX-specific shared memory code:
// Map the shared memory region const char* shm_key = getenv("SHM_ID"); if (!shm_key) { fprintf(stderr, "[COV] no shared memory bitmap available, using malloc\n"); __shmem = (struct shmem_data*)malloc(SHM_SIZE); if (!__shmem) { fprintf(stderr, "[COV] Failed to allocate coverage bitmap\n"); _exit(-1); } memset(__shmem, 0, SHM_SIZE); } else { +#ifndef _WIN32 int fd = shm_open(shm_key, O_RDWR, S_IREAD | S_IWRITE); if (fd <= -1) { fprintf(stderr, "[COV] Failed to open shared memory region: %s\n", strerror(errno)); _exit(-1); } __shmem = (struct shmem_data*)mmap(0, SHM_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); if (__shmem == MAP_FAILED) { fprintf(stderr, "[COV] Failed to mmap shared memory region\n"); _exit(-1); } +#else + fprintf(stderr, "[COV] Shared memory not supported on Windows, using malloc fallback\n"); + __shmem = (struct shmem_data*)malloc(SHM_SIZE); + if (!__shmem) { + fprintf(stderr, "[COV] Failed to allocate coverage bitmap\n"); + _exit(-1); + } + memset(__shmem, 0, SHM_SIZE); +#endif }
📜 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.
📒 Files selected for processing (2)
src/bun.js/bindings/FuzzilliREPRL.cpp(1 hunks)src/cli/fuzzilli_command.zig(1 hunks)
🧰 Additional context used
🧠 Learnings (21)
📓 Common learnings
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24423
File: test/js/node/path/browserify.test.js:472-475
Timestamp: 2025-11-07T17:28:58.045Z
Learning: markovejnovic prefers functional programming patterns (e.g., using reduce) over imperative loops and does not want style-based readability suggestions unless there's a concrete technical reason beyond personal preference.
<!--
📚 Learning: 2025-10-15T20:19:37.256Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:37.256Z
Learning: In the Bun codebase, FFI (Foreign Function Interface) code is compiled separately using TinyCC (tcc), which barely supports C99. Headers like src/bun.js/api/FFI.h and src/bun.js/api/ffi-stdbool.h are only used for FFI compilation with tcc, not with the main compiler. Therefore, C standard changes to the main Bun target do not affect FFI code compilation.
Applied to files:
src/cli/fuzzilli_command.zigsrc/bun.js/bindings/FuzzilliREPRL.cpp
📚 Learning: 2025-11-14T16:07:01.064Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-10-16T02:17:35.237Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/analytics.zig:15-21
Timestamp: 2025-10-16T02:17:35.237Z
Learning: In src/analytics.zig and similar files using bun.EnvVar boolean environment variables: the new EnvVar API for boolean flags (e.g., bun.EnvVar.do_not_track.get(), bun.EnvVar.ci.get()) is designed to parse and return boolean values from environment variables, not just check for their presence. This is an intentional design change from the previous presence-based checks using bun.getenvZ().
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-10-16T17:32:03.074Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/install/PackageManager/PackageManagerOptions.zig:187-193
Timestamp: 2025-10-16T17:32:03.074Z
Learning: In Bun's codebase (particularly in files like src/install/PackageManager/PackageManagerOptions.zig), mixing bun.EnvVar.*.get() and bun.EnvVar.*.platformGet() for environment variable lookups is intentional and safe. The code is protected by compile-time platform checks (Environment.isWindows, etc.), and compilation will fail if the wrong function is used on the wrong platform. This pattern should not be flagged as a consistency issue.
Applied to files:
src/cli/fuzzilli_command.zigsrc/bun.js/bindings/FuzzilliREPRL.cpp
📚 Learning: 2025-10-16T21:24:52.779Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/crash_handler.zig:1415-1423
Timestamp: 2025-10-16T21:24:52.779Z
Learning: When a boolean EnvVar in src/envvars.zig is defined with a default value (e.g., `.default = false`), the `get()` method returns `bool` instead of `?bool`. This means you cannot distinguish between "environment variable not set" and "environment variable explicitly set to the default value". For opt-out scenarios where detection of explicit setting is needed (like `BUN_ENABLE_CRASH_REPORTING` on platforms where crash reporting defaults to enabled), either: (1) don't provide a default value so `get()` returns `?bool`, or (2) use the returned boolean directly instead of only checking if it's true.
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-10-15T22:03:50.832Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/envvars.zig:135-144
Timestamp: 2025-10-15T22:03:50.832Z
Learning: In src/envvars.zig, the boolean feature flag cache uses a single atomic enum and should remain monotonic. Only the string cache (which uses two atomics: ptr and len) requires acquire/release ordering to prevent torn reads.
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-10-24T10:43:09.398Z
Learnt from: fmguerreiro
Repo: oven-sh/bun PR: 23774
File: src/install/PackageManager/updatePackageJSONAndInstall.zig:548-548
Timestamp: 2025-10-24T10:43:09.398Z
Learning: In Bun's Zig codebase, the `as(usize, intCast(...))` cast pattern triggers a Zig compiler bug that causes compilation to hang indefinitely when used in complex control flow contexts (loops + short-circuit operators + optional unwrapping). Avoid this pattern and use simpler alternatives like just `intCast(...)` if type casting is necessary.
Applied to files:
src/cli/fuzzilli_command.zigsrc/bun.js/bindings/FuzzilliREPRL.cpp
📚 Learning: 2025-11-10T00:57:09.173Z
Learnt from: franciscop
Repo: oven-sh/bun PR: 24514
File: src/bun.js/api/crypto/PasswordObject.zig:86-101
Timestamp: 2025-11-10T00:57:09.173Z
Learning: In Bun's Zig codebase (PasswordObject.zig), when validating the parallelism parameter for Argon2, the upper limit is set to 65535 (2^16 - 1) rather than using `std.math.maxInt(u24)` because the latter triggers Zig's truncation limit checks. The value 65535 is a practical upper bound that avoids compiler issues while being sufficient for thread parallelism use cases.
Applied to files:
src/cli/fuzzilli_command.zig
📚 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:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-10-15T20:19:38.580Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:38.580Z
Learning: In the Bun codebase, FFI is compiled with tcc (TinyCC), which barely supports C99. The headers `src/bun.js/api/FFI.h` and `src/bun.js/api/ffi-stdbool.h` are only used for FFI compilation with tcc, not for the main Bun target. Therefore, C23 compatibility concerns (such as bool/true/false keyword conflicts) do not apply to these FFI headers.
Applied to files:
src/cli/fuzzilli_command.zigsrc/bun.js/bindings/FuzzilliREPRL.cpp
📚 Learning: 2025-09-12T22:30:48.490Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 22613
File: src/cli/package_manager_command.zig:0-0
Timestamp: 2025-09-12T22:30:48.490Z
Learning: Commands like `bun info` and `bun whoami` that are promoted from pm-only subcommands to top-level commands use manual `bun.argv` parsing to detect direct invocation vs. pm invocation. This is different from regular commands that should use `ctx.positionals`.
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-09-20T05:35:57.318Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 22534
File: src/bun.js/bindings/headers.h:729-731
Timestamp: 2025-09-20T05:35:57.318Z
Learning: symbols.txt in the Bun codebase is specifically for V8 API mangled symbols (without leading underscore), not for general Bun host functions declared with BUN_DECLARE_HOST_FUNCTION. Host functions are handled through different build mechanisms.
Applied to files:
src/bun.js/bindings/FuzzilliREPRL.cpp
📚 Learning: 2025-11-03T20:43:06.996Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24273
File: src/bun.js/test/snapshot.zig:19-19
Timestamp: 2025-11-03T20:43:06.996Z
Learning: In Bun's Zig codebase, when storing JSValue objects in collections like ArrayList, use `jsc.Strong.Optional` (not raw JSValue). When adding values, wrap them with `jsc.Strong.Optional.create(value, globalThis)`. In cleanup code, iterate the collection calling `.deinit()` on each Strong.Optional item before calling `.deinit()` on the ArrayList itself. This pattern automatically handles GC protection. See examples in src/bun.js/test/ScopeFunctions.zig and src/bun.js/node/node_cluster_binding.zig.
Applied to files:
src/bun.js/bindings/FuzzilliREPRL.cpp
📚 Learning: 2025-11-03T20:40:59.655Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24273
File: src/bun.js/bindings/JSValue.zig:545-586
Timestamp: 2025-11-03T20:40:59.655Z
Learning: In Bun's Zig codebase, JSErrors (returned as `bun.JSError!T`) must always be properly handled. Using `catch continue` or `catch { break; }` to silently suppress JSErrors is a bug. Errors should either be explicitly handled or propagated with `try`. This applies to snapshot serializer error handling where Jest's behavior is to throw when serializers throw.
Applied to files:
src/bun.js/bindings/FuzzilliREPRL.cpp
📚 Learning: 2025-09-05T19:49:26.188Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: src/valkey/js_valkey_functions.zig:852-867
Timestamp: 2025-09-05T19:49:26.188Z
Learning: In Bun’s Zig code, `.js_undefined` is a valid and preferred JSValue literal for “undefined” (e.g., resolving JSPromise). Do not refactor usages to `jsc.JSValue.jsUndefined()`, especially in src/valkey/js_valkey_functions.zig unsubscribe().
Applied to files:
src/bun.js/bindings/FuzzilliREPRL.cpp
📚 Learning: 2025-09-02T19:17:26.376Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 0
File: :0-0
Timestamp: 2025-09-02T19:17:26.376Z
Learning: In Bun's Zig codebase, when handling error unions where the same cleanup operation (like `rawFree`) needs to be performed regardless of success or failure, prefer using boolean folding with `else |err| switch (err)` over duplicating the cleanup call in multiple switch branches. This approach avoids code duplication while maintaining compile-time error checking.
Applied to files:
src/bun.js/bindings/FuzzilliREPRL.cpp
📚 Learning: 2025-10-18T20:50:47.750Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:366-373
Timestamp: 2025-10-18T20:50:47.750Z
Learning: In Bun's Zig codebase (src/bun.js/bindings/JSValue.zig), the JSValue enum uses `.null` (not `.js_null`) for JavaScript's null value. Only `js_undefined` has the `js_` prefix to avoid collision with Zig's built-in `undefined` keyword. The correct enum fields are: `js_undefined`, `null`, `true`, `false`, and `zero`.
Applied to files:
src/bun.js/bindings/FuzzilliREPRL.cpp
📚 Learning: 2025-09-02T18:25:27.976Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22227
File: src/allocators/allocation_scope.zig:284-314
Timestamp: 2025-09-02T18:25:27.976Z
Learning: In bun's custom Zig implementation, the `#` prefix for private fields is valid syntax and should not be flagged as invalid. The syntax `#fieldname` creates private fields that cannot be accessed from outside the defining struct, and usage like `self.#fieldname` is correct within the same struct. This applies to fields like `#parent`, `#state`, `#allocator`, `#trace`, etc. throughout the codebase.
Applied to files:
src/bun.js/bindings/FuzzilliREPRL.cpp
📚 Learning: 2025-09-30T22:53:19.887Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 23117
File: src/bun.js/test/snapshot.zig:265-276
Timestamp: 2025-09-30T22:53:19.887Z
Learning: In Bun's snapshot testing (src/bun.js/test/snapshot.zig), multiple inline snapshots at the same line and column (same call position) must have identical values. However, multiple inline snapshots on the same line at different columns are allowed to have different values. The check is position-specific (line+col), not line-wide.
Applied to files:
src/bun.js/bindings/FuzzilliREPRL.cpp
📚 Learning: 2025-09-05T19:49:26.188Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: src/valkey/js_valkey_functions.zig:852-867
Timestamp: 2025-09-05T19:49:26.188Z
Learning: In Bun's Zig codebase, `.js_undefined` is a valid way to represent JavaScript's undefined value when working with JSPromise.resolve() and similar JavaScript interop functions. This is the correct pattern to use rather than `jsc.JSValue.jsUndefined()`.
Applied to files:
src/bun.js/bindings/FuzzilliREPRL.cpp
There was a problem hiding this comment.
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.
📒 Files selected for processing (1)
src/cli/fuzzilli_command.zig(1 hunks)
🧰 Additional context used
🧠 Learnings (15)
📓 Common learnings
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24423
File: test/js/node/path/browserify.test.js:472-475
Timestamp: 2025-11-07T17:28:58.045Z
Learning: markovejnovic prefers functional programming patterns (e.g., using reduce) over imperative loops and does not want style-based readability suggestions unless there's a concrete technical reason beyond personal preference.
<!--
📚 Learning: 2025-10-15T20:19:37.256Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:37.256Z
Learning: In the Bun codebase, FFI (Foreign Function Interface) code is compiled separately using TinyCC (tcc), which barely supports C99. Headers like src/bun.js/api/FFI.h and src/bun.js/api/ffi-stdbool.h are only used for FFI compilation with tcc, not with the main compiler. Therefore, C standard changes to the main Bun target do not affect FFI code compilation.
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-11-14T16:07:01.064Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-10-16T02:17:35.237Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/analytics.zig:15-21
Timestamp: 2025-10-16T02:17:35.237Z
Learning: In src/analytics.zig and similar files using bun.EnvVar boolean environment variables: the new EnvVar API for boolean flags (e.g., bun.EnvVar.do_not_track.get(), bun.EnvVar.ci.get()) is designed to parse and return boolean values from environment variables, not just check for their presence. This is an intentional design change from the previous presence-based checks using bun.getenvZ().
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-09-12T22:30:48.490Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 22613
File: src/cli/package_manager_command.zig:0-0
Timestamp: 2025-09-12T22:30:48.490Z
Learning: Commands like `bun info` and `bun whoami` that are promoted from pm-only subcommands to top-level commands use manual `bun.argv` parsing to detect direct invocation vs. pm invocation. This is different from regular commands that should use `ctx.positionals`.
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-10-16T17:32:03.074Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/install/PackageManager/PackageManagerOptions.zig:187-193
Timestamp: 2025-10-16T17:32:03.074Z
Learning: In Bun's codebase (particularly in files like src/install/PackageManager/PackageManagerOptions.zig), mixing bun.EnvVar.*.get() and bun.EnvVar.*.platformGet() for environment variable lookups is intentional and safe. The code is protected by compile-time platform checks (Environment.isWindows, etc.), and compilation will fail if the wrong function is used on the wrong platform. This pattern should not be flagged as a consistency issue.
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-10-16T21:24:52.779Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/crash_handler.zig:1415-1423
Timestamp: 2025-10-16T21:24:52.779Z
Learning: When a boolean EnvVar in src/envvars.zig is defined with a default value (e.g., `.default = false`), the `get()` method returns `bool` instead of `?bool`. This means you cannot distinguish between "environment variable not set" and "environment variable explicitly set to the default value". For opt-out scenarios where detection of explicit setting is needed (like `BUN_ENABLE_CRASH_REPORTING` on platforms where crash reporting defaults to enabled), either: (1) don't provide a default value so `get()` returns `?bool`, or (2) use the returned boolean directly instead of only checking if it's true.
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-10-15T22:03:50.832Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/envvars.zig:135-144
Timestamp: 2025-10-15T22:03:50.832Z
Learning: In src/envvars.zig, the boolean feature flag cache uses a single atomic enum and should remain monotonic. Only the string cache (which uses two atomics: ptr and len) requires acquire/release ordering to prevent torn reads.
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-10-24T10:43:09.398Z
Learnt from: fmguerreiro
Repo: oven-sh/bun PR: 23774
File: src/install/PackageManager/updatePackageJSONAndInstall.zig:548-548
Timestamp: 2025-10-24T10:43:09.398Z
Learning: In Bun's Zig codebase, the `as(usize, intCast(...))` cast pattern triggers a Zig compiler bug that causes compilation to hang indefinitely when used in complex control flow contexts (loops + short-circuit operators + optional unwrapping). Avoid this pattern and use simpler alternatives like just `intCast(...)` if type casting is necessary.
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-11-10T00:57:09.173Z
Learnt from: franciscop
Repo: oven-sh/bun PR: 24514
File: src/bun.js/api/crypto/PasswordObject.zig:86-101
Timestamp: 2025-11-10T00:57:09.173Z
Learning: In Bun's Zig codebase (PasswordObject.zig), when validating the parallelism parameter for Argon2, the upper limit is set to 65535 (2^16 - 1) rather than using `std.math.maxInt(u24)` because the latter triggers Zig's truncation limit checks. The value 65535 is a practical upper bound that avoids compiler issues while being sufficient for thread parallelism use cases.
Applied to files:
src/cli/fuzzilli_command.zig
📚 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:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-09-06T03:37:41.154Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22229
File: src/bundler/LinkerGraph.zig:0-0
Timestamp: 2025-09-06T03:37:41.154Z
Learning: In Bun's codebase, when checking import record source indices in src/bundler/LinkerGraph.zig, prefer using `if (import_index >= self.import_records.len)` bounds checking over `isValid()` checks, as the bounds check is more robust and `isValid()` is a strict subset of this condition.
Applied to files:
src/cli/fuzzilli_command.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/cli/fuzzilli_command.zig
📚 Learning: 2025-09-02T18:25:27.976Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22227
File: src/allocators/allocation_scope.zig:284-314
Timestamp: 2025-09-02T18:25:27.976Z
Learning: In bun's custom Zig implementation, the `#` prefix for private fields is valid syntax and should not be flagged as invalid. The syntax `#fieldname` creates private fields that cannot be accessed from outside the defining struct, and usage like `self.#fieldname` is correct within the same struct. This applies to fields like `#parent`, `#state`, `#allocator`, `#trace`, etc. throughout the codebase.
Applied to files:
src/cli/fuzzilli_command.zig
📚 Learning: 2025-10-15T20:19:38.580Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:38.580Z
Learning: In the Bun codebase, FFI is compiled with tcc (TinyCC), which barely supports C99. The headers `src/bun.js/api/FFI.h` and `src/bun.js/api/ffi-stdbool.h` are only used for FFI compilation with tcc, not for the main Bun target. Therefore, C23 compatibility concerns (such as bool/true/false keyword conflicts) do not apply to these FFI headers.
Applied to files:
src/cli/fuzzilli_command.zig
🔇 Additional comments (2)
src/cli/fuzzilli_command.zig (2)
1-8: LGTM: Conditional compilation and POSIX guard are sound.The conditional compilation pattern appropriately gates the entire command behind the
enable_fuzzilliflag, and the POSIX check with helpful error messaging provides a clear early exit for unsupported platforms.
12-52: REPRL verification and runtime boot logic are correct.The REPRL file descriptor verification provides clear guidance on usage, the embedded script approach is efficient for this small file, and the error handling throughout is comprehensive with helpful messages. The defer-based cleanup is properly structured.
Note: Past reviews have flagged the hardcoded
/tmppath and fixed filename as potential issues for parallel runs and portability, but these are noted as acceptable for the current "stop-gap" objective.
| // Set an environment variable so we can detect fuzzilli mode in JavaScript | ||
|
|
There was a problem hiding this comment.
Remove stale comment about setting environment variable.
This comment references code that no longer exists (the setenv call was removed when addressing the Windows link issue). Either remove the comment entirely or update it to reflect the current implementation.
Apply this diff:
- // Set an environment variable so we can detect fuzzilli mode in JavaScript
-
// Verify REPRL file descriptors are available🤖 Prompt for AI Agents
In src/cli/fuzzilli_command.zig around lines 10 to 11, remove or update the
stale comment that mentions setting an environment variable for "fuzzilli mode"
since the corresponding setenv call was removed; specifically delete the comment
line or replace it with an accurate note describing the current behavior (e.g.,
explaining the Windows link workaround or that no env var is set), ensuring the
source reflects the actual implementation.
What does this PR do?
Adds @mschwarzl's Fuzzilli Support PR with the changes necessary to be able to:
debugandreleasemode.How did you verify your code works?