Repository navigation
Conversation
… the commands that read their own flags With `--disallow-code-generation-from-strings=strict` in `BUN_OPTIONS`, `bun install --analyze`, `bun add --analyze` and `bun create <component>` ran macros. `arguments::parse` read the flag for four command tags only, and `bun lint` and `bun fmt` had a helper of their own. `command::init` is now a cold wrapper around `create_context_data`. For a command that `parse` does not read the flag for, it reads the flag among the flags of `bun`: those that `BUN_OPTIONS` adds, and those before the command. The last one counts, as in `parse`. The package manager and bunx take it from `BUN_OPTIONS` only, and keep their error for a flag that is typed. `bun create` also takes it after `create`. `bun lint` and `bun fmt` use this reader in place of their helper. `bun <file>` and `bun run` call `create_context_data` directly. `bun build` is not read.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 51 seconds. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Updated 5:05 AM PT - Oct 21st, 2026
❌ @robobun, your commit f1893d2 has some failures in 🧪 To try this PR locally: bunx bun-pr 46275That installs a local version of the PR into your bun-46275 --bun |
|
Status Reproduced on main 9b4284f with a macro that writes a marker file: // macro.ts
import { writeFileSync } from "node:fs";
import { join } from "node:path";
export function f() {
writeFileSync(join(import.meta.dir, "MACRO_RAN"), "");
return "INLINED_RESULT";
}
// entry.ts
import { f } from "./macro.ts" with { type: "macro" };
console.log(f());
PR: #46275 |
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline findings, I also checked whether flags_of_bun can read a command's own typed argument for exec, upgrade and the other USES_GLOBAL_OPTIONS commands — it slices argv only up to the command found by place_of_first_word, so a value typed after the command is never read. I also checked the lint/fmt change from max-wins (_as_compiled) to last-wins (_among): it matches what arguments::parse does for bun run, and the new test rows pin that order.
Extended reasoning...
The change makes command::init a cold wrapper that applies --disallow-code-generation-from-strings from BUN_OPTIONS and from the flags before the subcommand for commands whose own parser does not read runtime flags, with a second scan of bun create's own arguments; it touches CLI dispatch in src/runtime/cli/mod.rs and Arguments.rs and removes the bespoke lint/fmt helper. The surface is the security-relevant macro/eval gate, and the four inline findings (the -- edge in the create rescan, the pre-existing bun build gap, and the new hard exit on a non-strict value in BUN_OPTIONS for ~20 commands) plus the behavioral change to lint/fmt precedence warrant a human look.
| let ctx = create_context_data(cmd, log)?; | ||
| // `bun build` reads what stands before it, and what `BUN_OPTIONS` adds, with its own flags. | ||
| if !arguments::reads_runtime_flags(cmd) && cmd != Tag::BuildCommand { | ||
| arguments::disallow_code_generation_from_strings_among(arguments::flags_of_bun(cmd)); |
There was a problem hiding this comment.
🟡 (optional) Users with a mistyped or future value in BUN_OPTIONS now get exit 1 from bun install, bun upgrade, bun x, bun lint --help and about 20 other commands that ran on the base branch. init at src/runtime/cli/mod.rs:991-994 reads the flag from BUN_OPTIONS for every command that does not read runtime flags, before the command parses its own argv or prints help, and disallow_code_generation_from_strings at Arguments.rs:788-794 exits on any value other than strict or empty. Fix: for commands that run no JavaScript (upgrade, exec, getcompletes, pm, publish, outdated, audit, info, link, the --help paths) ignore or warn on an unknown value instead of exiting, or only validate when the level is about to be used.
Why this was flagged
The trigger is any BUN_OPTIONS containing --disallow-code-generation-from-strings=<x> with <x> not strict or empty, reaching command::init via the dispatch macro at mod.rs:1529. init calls flags_of_bun which always includes the BUN_OPTIONS tokens (Arguments.rs:821, 849), and disallow_code_generation_from_strings_among calls disallow_code_generation_from_strings, which prints the error and Global::exit(1) (Arguments.rs:789-793). On the base branch init was an alias of create_context_data and parse only read the flag for Auto/Run/Test/RunAsNode (Arguments.rs base:1048), so bun install, bun upgrade, bun install --help and the rest ran. The population is every CI or developer shell that exports BUN_OPTIONS globally; a value written for a newer or older bun, or a typo, now blocks bun upgrade itself, the command the user would run to recover. The PR counts this as an accepted downside, but 22 commands that run no JavaScript now fail on a value that affects nothing they do.
Verification: Trigger: BUN_OPTIONS contains --disallow-code-generation-from-strings=<x> with <x> neither empty nor strict. init (mod.rs:985-996) runs disallow_code_generation_from_strings (Arguments.rs:785-796), whose None arm prints the error and Global::exit(1). On the base branch these commands ran: arguments::parse read the flag only for the four runtime tags.
There was a problem hiding this comment.
Kept as it is. The reasons:
- On main the same value in
BUN_OPTIONSalready stopsbun <file>,bun run,bun testandbun repl, with this error. A shell with that value cannot run a program, before and after this change. - The reader validates the value where it reads it, at startup, as
arguments::parsedoes. With validation at first use,bun installpasses with a mistyped value.bun install --analyzethen fails in the same shell. - A list of commands that run no JavaScript fails open when it is out of date.
bun installruns none, andbun install --analyzebundles.inithas one exception,bun build, which waits for a ruling. - The error names the flag and the value.
BUN_OPTIONS= bun upgraderuns the upgrade.
The cost is real. The PR body has it under Downsides: 22 more commands stop, and for 20 of them the error comes before --help.
The second read of `--disallow-code-generation-from-strings` in `bun create` started at the first argument. A `--` before `create` ended that read, so `bun -- create ./App.tsx <flag>=strict` ran macros. The read now starts after `create`. `command::init` has read what stands before it. Tests: - A row for `bun -- create <component> <flag>=strict`. - A row that pins `bun build` with the flag in `BUN_OPTIONS`: it still runs macros. - The row for `bun create <component> -- <flag>=strict` asserts only that the macro ran. On Windows the command then fails where it writes `App.build.ts` (`SourceFileProjectGenerator.rs:275`), which this change does not touch.
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
The latest push addresses the two points from my earlier review that had a code fix: the second bun create scan now starts at after_create (the index after the create positional, computed from remainder_i + 2 over args[1..]), so a -- from BUN_OPTIONS or before create no longer ends create's own flags, and the new bun build row with =strict in BUN_OPTIONS pins the BuildCommand exclusion in init. Beyond the inline nit, I also re-checked flags_of_bun's slice bounds (1.min(len)..end.min(len)) for an argv of length 1 and for BUN_OPTIONS longer than the typed argv — both clamp without panicking.
Extended reasoning...
The change makes command::init a cold wrapper that applies --disallow-code-generation-from-strings from BUN_OPTIONS and from the tokens before the command for every non-runtime command except bun build, adds a second read inside bun create, drops the lint/fmt helper, updates the docs page, and adds a 311-line CLI test file. It touches a security-relevant runtime gate (macro refusal under =strict) and the behaviour of about twenty commands when BUN_OPTIONS carries a mistyped value, which the author lists as a known downside rather than a bug. The one surviving finding is a test-assertion nit, and the design question about bun build and the exit-on-mistyped-value tradeoff are still open for a maintainer, so a human look remains worthwhile.
Still open from earlier reviews (2):
- Unresolved: 2 minor or pre-existing.
No code changes in `src/`. Test: the row for `bun create <component> -- <flag>=strict` asserts again that the command writes its files, except on Windows, where it asserts only that the macro ran.
Problem
--disallow-code-generation-from-strings=strictinBUN_OPTIONS,bun install --analyze,bun add --analyzeandbun create <component>run macros.bun <flag> file.tsrefuses them:error: Macros cannot be used with --disallow-code-generation-from-strings=strict.arguments::parsereads the flag for four command tags only (src/runtime/cli/Arguments.rs:1048).Fix
command::init(src/runtime/cli/mod.rs) is a cold wrapper aroundcreate_context_data. It reads the flag among the flags ofbun: those ofBUN_OPTIONS, and those before the command. The last one counts.BUN_OPTIONSonly, and keep their error for a typed flag.bun lintandbun fmtuse this reader, not their helper.test/cli/disallow-code-generation-from-strings.test.ts(61 rows, 43 fail on main). Also the lint, fmt and check suites. Self-reviewed: 13 concerns raised, 11 addressed.Background
=strictrefuses a macro: its entry point is text made at run time.bun buildis not read and still runs macros: Compile TypeScript to machine code ahead of time:bun build --compile --aot#45631 makes the flag there the level of the compiled executable. Question for a maintainer: does the flag inBUN_OPTIONSor beforebuildset the level of the build process?Downsides
strictinBUN_OPTIONSstops 22 more commands, before--helpfor 20.BUN_OPTIONShas it, these commands now refuse a macro at=strict, andeval()throws in a macro without a value.bun <file>andbun run+0 instructions,bun test+28. Release.text+656 B.Notes
How it was found
A read of the code while #45937 (
--no-macrosas a switch of the process) was written. No user reported it. Reproduce it with a macro that writes a marker file:<flag>is--disallow-code-generation-from-strings,<strict>is<flag>=strict.BUN_OPTIONS=<strict> bun install --analyze ./entry.tserror: Macros cannot be used with <strict>, exit 1BUN_OPTIONS=<strict> bun add --analyze ./entry.tsBUN_OPTIONS=<strict> bun create ./App.tsx,bun <strict> create ./App.tsx,bun create <strict> ./App.tsx,bun create ./App.tsx <strict>,bun -- create ./App.tsx <strict>BUN_OPTIONS=<flag> bun install --analyze ./entry.tseval()works in the macroeval()throws in the macrobun <strict> <flag> lint,BUN_OPTIONS=<strict> bun <flag> lint=strictBUN_OPTIONS=<flag>=bogus bun installerror: Invalid value for <flag>: "bogus". Must be "strict", or no value, exit 1bun <strict> install,bun <flag>=bogus installerror: bun install has no <flag> flag.bun build ./entry.tswith the flag in any placeWhere the flags of
bunendBUN_OPTIONSadds always count. They stand in argv before what was typed.create,lint,fmt,check, and the commands thatarguments::parseparses (exec,upgrade,getcompletes).bun install has no ... flag.,bunx has no ... flag.). The reader leaves that to them, also for a value that is notstrict.which()finds it. On main that is after the leading tokens that start with-, or forcheck,lintandfmtafter the values of flags too (place_of_first_word). The reader takes the further of the two. CLI: fixes for bun run, flags, bunfig.toml, .env, --filter, bun init and completions #46007 makeswhich()use the second walk for every command, and the reader then still reads every flag before the command.--ends the flags.bunas its value.bun buildarguments::parsegivesbun buildone table for all of argv, so a token fromBUN_OPTIONSor beforebuildis read with the flags ofbun build.bun build --compile --aot#45631 (draft) adds the flag to that table as the level of the compiled executable, and exits with--disallow-code-generation-from-strings requires --compilewithout--compile. With the flag inBUN_OPTIONS, that error stops every plainbun build. This is from a read of its diff. It was not built.bun build, in any place.BUN_OPTIONS=<strict> bun build ./entry.tsstill runs macros, and a row holds that.Bun.build()in a program that runs with the flag refuses them.command::initand rows in the test file.bun createinitreadsBUN_OPTIONSand the tokens beforecreate.bun createignores every--flagit does not know, wherever it stands. So a second read covers the tokens aftercreate, up to--, once the kind of template is known. A--beforecreateends only the flags ofbun.bun create vite), the tokens after the name of the template go tocreate-<template>. They are not read.bun <strict> create ./App.tsx <flag>refuses macros).Not covered
bun build, see above.bunx --bunandbun create <template> --buncan start the package in the bunx process. With the flag inBUN_OPTIONSthat process now has the level. This is from a read of the code. It was not run: no Windows machine here.--no-addons,--no-ffi-ccand the rest ofRUNTIME_PARAMS_) are still read for four command tags only. A plugin ofbun lintunder--no-addonscan still callprocess.dlopen()(checked). Make --no-ffi-cc and --no-addons disable all of bun:ffi #42153 has thebun buildcase of those two flags. The docs page now says which commands read them.exec,upgradeandgetcompletesthe flag is parsed and not read. These commands run no JavaScript.bun init,bun completionsand the help do not callinit.--no-macrosa switch of the process #45937 passes the level to a childbunthroughBUN_OPTIONS. A child that isbun installorbun xreads it only with this change. A child that isbun builddoes not read it.Tests
test/js/node/disallow-code-generation-from-strings.test.tsandtest/cli/lint/js-plugins.test.tsdo not pass as whole files underbun bd teston a debug build with the default 5 s limit, with or without this change (13 of 38 rows of the first time out on main: it compiles executables and starts Workers).test/clihas one file for a flag in other cases (console-depth.test.ts,user-agent.test.ts).=strictmust refuse a macro (install --analyze,add --analyze,createin six places, two order cases),strictbefore the name of a template,bun lintandbun fmt,strictinBUN_OPTIONS, one per command,--inbun create, the arguments of a template,install --analyzewith no flag and with no macro,bun buildwith the flag inBUN_OPTIONS,bun testwith the flag on both sides, and the cases ofbun lintandbun fmtthat main has right (8).bun upgradeandbun update --interactivewere checked by hand (BUN_OPTIONS=<flag>=bogusgives the error). They are not rows: on a build without the fix the row would run a real upgrade.bun lintandbun fmtrun one at a time, with one thread. A thread ofbun lintstarts a VM, and in a debug build that takes seconds beside twenty other rows.bun install --analyzethat completes exits 134 on main too (test/cli/install/bun-link.test.tsskips that case). The 5 rows that complete such a command do not pin the exit code whenisASAN. The file passes in the environment of the ASAN lane, emulated here with the debug build: 61 pass. In that emulationbun buildwith a macro exits 134 too.bun create <component>fails where it writesApp.build.ts, after the macros ran (ENOENT,src/runtime/cli/create/SourceFileProjectGenerator.rs:275, seen in CI). This change does not touch that code. On Windows the row for--asserts only that the macro ran.test/cli/lint/help.test.ts361 pass, theBUN_OPTIONSrows oflint.test.ts(3),format.test.ts(2),check.test.ts(1), the flag rows ofjs-plugins.test.ts(2).Numbers
Release builds with
--lto=offof main 9b4284f and of this branch at 8ed8be0, in one build directory, linux x64. The commit after 8ed8be0 changes comments insrc/and one test row. Thestepi, malloc and syscall counts are from the commit before 8ed8be0:init,flags_of_bunanddisallow_code_generation_from_strings_amonghave the same listings in the two builds. Tools:objdump,nm -S,size -A, gdb.strace,perf,valgrindandbloatyare not installed here, and apt has no candidate for them.create_context_data,which,start,arguments::parse,exec_auto_or_run): 5 of 5 functions have the same size and the same instructions, +0 B (204 B and 53 instructions, 2,571 and 621, 1,529 and 349, 25,071 and 5,298, 8,481 and 1,719. objdump listings with addresses masked, thencmp).bun test,bun repl,node,bun buildand a compiled executable with flags: +28 instructions, +3 conditional branches and +1 call per process, in the coldcommand::init(111 B. objdump, and gdbstepiforbun testin an empty directory).bun pm58,bun install61,bun lint84. gdbstepi)..text77,618,384 to 77,619,040 (+656 B)..rodata,.dataand.bss+0. The stripped file has 102,713,144 B in both builds.command::init111 B,arguments::flags_of_bun553 B,arguments::disallow_code_generation_from_strings_among365 B. Removed:lint_command::disallow_code_generation_before534 B.bun create11 and 11,bun exec24 and 24,bun check --help35 and 35,bun x12 and 12,bun install --help83 and 83,bun lint6,366 and 6,363.Vec:bun --silent create13 and 14,bun --cwd=. exec28 and 29.bun --cwd=. lint6,403 and 6,401.catch syscall, two for each call, main then branch):bun create185 and 185,bun exec187 and 187,bun x185 and 185,bun install --help191 and 191,bun build x.ts --outdir oup to its first thread 294 and 294. After the first thread the count ofbun buildis not stable on one binary.bun build --compile --aot#45631, Make--no-macrosa switch of the process #45937 and CLI: fixes for bun run, flags, bunfig.toml, .env, --filter, bun init and completions #46007: 0, 0 and 0 (git merge-fileon the five edited files. The three PRs have conflicts with main already).create_context_data(has two call sites:initandexec_auto_or_run.strictinBUN_OPTIONSnow comes before--helpfor 20 commands: add, audit, dedupe, prune, x, check, lint, fmt, create, install, link, outdated, pm, patch, patch-commit, publish, remove, unlink, update, why. The help stays first for build, exec, upgrade, getcompletes and info.Self-review
13 concerns. Done:
bun buildis out of this change until a maintainer rules (it was in, with a named refusal forbun build --appthat is out with it).which()alone. It is the further of that loop andplace_of_first_word.BUN_OPTIONSonly, so a typed flag with a wrong value keeps the error of main.bun(arguments::flags_of_bun), for the next switch that needs them.bun create <template>no longer reads the arguments of the template.bun install.bun check: fix errors lost since #45800, and 9 to 37% faster #46001. The comments that putiniton the path ofbun runare corrected.--cwd <dir>beforelint,fmtandcheck, a-before the command,bun fmtat both levels, the stop at--, a command that still works at=strict,bun testwith the flag on both sides.Not done:
--compile-exec-argv <flags>, a flag ofbun build.