Skip to content

bake: validate bundlerOptions is an object before property access#30579

Closed
robobun wants to merge 1 commit into
mainfrom
farm/d83d67fa/bake-bundler-options-validation
Closed

bake: validate bundlerOptions is an object before property access#30579
robobun wants to merge 1 commit into
mainfrom
farm/d83d67fa/bake-bundler-options-validation

Conversation

@robobun

@robobun robobun commented May 12, 2026

Copy link
Copy Markdown
Collaborator

Fuzzer found a debug assertion in JSValue.get() when passing a non-object value to app.bundlerOptions in Bun.serve().

Bun.serve({ app: { bundlerOptions: 315 } });
panic(main thread): reached unreachable code
jsc.JSValue.JSValue.get
/workspace/bun/src/jsc/JSValue.zig:1534:24
bake.bake.UserOptions.fromJS
/workspace/bun/src/bake/bake.zig:71:43

getOptional(global, "bundlerOptions", JSValue) returns the raw value without type checking, then we called .getOptional() on it which asserts the target is an object.

Same issue existed for bundlerOptions.{server,client,ssr} and for the nested minify option (which also crashed on minify: false since the boolean check only broke out of the block when true). All of these now throw a proper TypeError.

Fingerprint: 2406707844214a48

Bun.serve({ app: { bundlerOptions: 315 } }) would hit a debug assertion
in JSValue.get() because we called property getters on a non-object.

Also fix the same issue for bundlerOptions.{server,client,ssr} and for
the nested minify option (which also crashed on minify: false).
@robobun

robobun commented May 12, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 12:52 PM PT - May 12th, 2026

@robobun, your commit 2652ab4 has 1 failures in Build #53817 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 30579

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

bun-30579 --bun

@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ec3afebd-c541-4228-b23f-d17a295f60b4

📥 Commits

Reviewing files that changed from the base of the PR and between 314ffe3 and 2652ab4.

📒 Files selected for processing (2)
  • src/bake/bake.zig
  • test/js/bun/http/bun-serve-args.test.ts

Walkthrough

Bake adds runtime type validation for bundlerOptions configuration: UserOptions and BuildConfigSubset now require bundlerOptions to be an object, minify parsing explicitly handles boolean values and rejects invalid types, and new tests verify these validation paths throw appropriate errors.

Changes

bundlerOptions Validation and Minify Parsing

Layer / File(s) Summary
bundlerOptions object validation
src/bake/bake.zig
UserOptions.fromJS and BuildConfigSubset.fromJS each add guards requiring bundlerOptions to be an object, throwing invalid-arguments errors if it is not.
minify parsing refactoring and test coverage
src/bake/bake.zig, test/js/bun/http/bun-serve-args.test.ts
minify parsing now explicitly sets minify_syntax, minify_identifiers, and minify_whitespace to boolean values directly (including false), and throws a type error for non-boolean, non-object input. New describe("app") test suite validates that non-object bundlerOptions, invalid nested entries, and non-object minify values throw TypeError.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding validation that bundlerOptions is an object before accessing its properties.
Description check ✅ Passed The description thoroughly explains the issue (fuzzer-discovered debug assertion), provides concrete reproduction steps, explains the root cause, and describes the fix.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM — straightforward input validation to convert fuzzer-found assertion failures into proper TypeErrors, with tests.

Extended reasoning...

Overview

This PR fixes a fuzzer-discovered debug assertion in bake.zig where non-object values passed to app.bundlerOptions (or its server/client/ssr/minify sub-options) would hit reached unreachable code in JSValue.get(). The fix adds isObject() checks before property access in three locations and throws ERR_INVALID_ARG_TYPE instead. It also fixes a related bug where minify: false fell through the boolean check (which only matched true) into the object property-access path.

Security risks

None. This is purely defensive input validation on a user-facing config API — it converts crashes into proper thrown errors. No new attack surface, auth, or data handling.

Level of scrutiny

Low. The change is small (~20 lines of Zig), mechanical, and follows the exact same if (!x.isObject()) return global.throwInvalidArguments(...) pattern already used elsewhere in this file (e.g., the config check at the top of UserOptions.fromJS, the reactFastRefresh/serverComponents checks). The minify boolean refactor is a clear correctness improvement — previously isBoolean() and asBoolean() only short-circuited on true, leaving false to crash on the subsequent .getBooleanLoose() calls.

Other factors

  • Four new tests in bun-serve-args.test.ts exercise each new error path plus the minify: false regression.
  • throwInvalidArguments produces ERR_INVALID_ARG_TYPE which is a TypeError, matching the test assertions.
  • No bugs flagged by the bug-hunting system, no outstanding reviewer comments, no prior reviews on this PR.

@github-actions

Copy link
Copy Markdown
Contributor

This PR may be a duplicate of:

  1. bake: validate bundlerOptions values are objects before property access #30125 - Validates bundlerOptions and nested server/client/ssr properties are objects
  2. Validate app.bundlerOptions types in Bun.serve #30402 - Validates app.bundlerOptions types and minify option in Bun.serve

🤖 Generated with Claude Code

@robobun

robobun commented May 12, 2026

Copy link
Copy Markdown
Collaborator Author

Duplicate of #30125 which already covers all these cases with better error messages.

@robobun robobun closed this May 12, 2026
@robobun robobun deleted the farm/d83d67fa/bake-bundler-options-validation branch May 12, 2026 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant