Validate app.bundlerOptions types in Bun.serve#30405
Conversation
Bun.serve({ app: { bundlerOptions: ... } }) would hit a debug
assertion when bundlerOptions, bundlerOptions.{server,client,ssr},
or their minify option were not objects. Throw a TypeError instead.
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR enhances bundler options validation in the Bake configuration system. The ChangesBundler Options Validation Enhancement
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
|
Duplicate of #30125. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — straightforward input validation fix converting debug-assert crashes into TypeErrors, with tests.
Extended reasoning...
Overview
This PR adds type validation for app.bundlerOptions and its server/client/ssr sub-objects (and their minify field) in Bun.serve. Previously, passing a non-object primitive for any of these would hit a debugAssert(target.isObject()) panic inside JSValue.get. The fix adds isObject() checks that throw a descriptive TypeError instead, threads a comptime name parameter into BuildConfigSubset.fromJS so the error message identifies the offending property path, and fixes minify: false which previously fell through the boolean fast-path and hit the same assertion. A new test file covers all error paths plus the minify: true/false happy paths.
Security risks
None. This is purely defensive input validation in a configuration parsing path — it adds checks rather than removing them, and there is no auth, crypto, file I/O, or command execution involved.
Level of scrutiny
Low. The change is small (~20 lines of logic), mechanical, and follows the same isObject() → throwInvalidArguments pattern already used elsewhere in this file (e.g., the config, reactFastRefresh, serverComponents checks). It converts a fuzzer-discovered crash into a proper error with no behavioral change to valid inputs.
Other factors
- The bug-hunting system found no issues.
- All three callers of
BuildConfigSubset.fromJSwere updated for the new comptime parameter; there are no other call sites. - The
minify: falsechange is correct: previouslyisBoolean() and asBoolean()only short-circuited ontrue, lettingfalsefall through togetBooleanLooseon a non-object. Now any boolean is handled and assigned directly. - No CODEOWNERS apply to
src/bake/or the test file (only*.d.tsis owned). - New tests assert the exact error messages and verify booleans no longer crash.
1 participant
Bun.serve({ app: { bundlerOptions: ... } })hit a debug assertion inJSValue.getwhenbundlerOptions,bundlerOptions.{server,client,ssr}, or theirminifyoption were non-object primitives (e.g. a number). The code called.getOptional()on these values without checking they were objects first.Now throws a
TypeErrorwith the offending property path instead. Also fixesminify: false, which previously fell through the boolean check and hit the same assertion.Found by Fuzzilli. Fingerprint:
b2de5bfcd9b59c23