Repository navigation
Add workerd run: one Worker from a file, --eval or stdin - #8084
bartlomieju wants to merge 3 commits into
Conversation
`workerd run <file>` serves one JavaScript or TypeScript file as a Worker with no config file. It is served as a wrangler config whose only field is `main`: the file is bundled with esbuild, found the same way (WORKERD_ESBUILD, else the nearest node_modules/.bin/esbuild, else PATH), so TypeScript, relative imports and npm packages work, and --watch reloads on any input to the bundle. The Worker gets the newest compatibility date this build supports and listens on localhost:8787, as a wrangler config does. --port, --compat-date and --compat-flag change them, and the rest of `serve`'s options apply. wrangler.rs's load() is split into parsing and build(), which load_file() reuses with a synthesized config and the socket address as a parameter. The esbuild-not-found message no longer assumes a wrangler config.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8084 +/- ##
==========================================
- Coverage 68.46% 68.33% -0.14%
==========================================
Files 722 724 +2
Lines 186735 187122 +387
Branches 29364 29365 +1
==========================================
+ Hits 127843 127863 +20
- Misses 41726 42092 +366
- Partials 17166 17167 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
We would need It might also be nice to allow the |
Source can be given instead of a file, with --eval (-e) or on stdin as `-`, so a here-document works. It is bundled with esbuild through its stdin as TypeScript, its imports resolved from the working directory and its bundle written under the system's temporary directory. Stdin under --watch is a usage error: a reload cannot read it again. Under `run`, -e is --eval; --external-addr keeps its long form. --test runs the Worker's tests instead of serving it and exits with their result. It adds no socket and conflicts with --port. --config <template> takes a Cap'n Proto config with exactly one Worker that has no modules, which the code becomes. --compat-date replaces its date; --compat-flag and --autogate add to its lists. A template without sockets gets one on localhost:<port>; one with sockets keeps them, and --port is then an error. --autogate <name> enables an autogate, with or without the workerd-autogate- prefix; an unknown name is a usage error listing this build's, from a new autogate_names() in the bootstrap's bridge. `run` prints `Listening on <url>` to stdout for each HTTP(S) socket, through the server's Control when RunOptions::announce is set, so --port 0 is usable from a script. When no port is given and 8787 cannot be bound, the server tries the next ports, 10 in all (RunOptions::port_attempts); the search happens as it binds, so no other process can take a port between a check and the bind. The run logic moves out of wrangler.rs into run.rs. wrangler.rs exposes its bundling and module-writing helpers, and bundle() takes source as well as a file; `serve wrangler.jsonc` is unchanged. For source, esbuild gets PWD set to its working directory, so the source's path in its messages does not climb to `/` when the inherited PWD is a symlinked spelling of that directory. Tests: unit tests for the arguments, filling a template, bundling source and the port search; //src/workerd/server/tests/run:run-test runs the binary with the repository's native esbuild.
bartlomieju
changed the title
![clanky-the-clanker[bot]](https://avatars.githubusercontent.com/in/5306670?s=60&v=4){kind=small}
There was a problem hiding this comment.
💬 Approved with comments — The design and tests are solid. There is one WARNING: --eval/stdin bundles go to a predictable directory in the shared temp dir, and another local user could take that directory over. Two smaller non-blocking notes.
Findings: 1 warning · 1 info · 1 suggestion (1 is an inline comment on the diff)
Not opened by the correctness review: 1 changed file it was meant to read
src/workerd/server/factory/bootstrap.h
The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.
🔧 Fix with your agent
Paste the following into your coding agent to investigate and fix these issues.
Fix the following code review issues found in pull request #8084.
Before making changes, review each issue and present a brief summary table:
- For each finding, state whether you agree, disagree, or need clarification
- If you disagree (e.g., the fix would require disproportionate changes
for minimal ROI, or the finding is factually incorrect), explain why
- If you need to clarify something before deciding, ask those questions
- Then share your plan for which issues to tackle and in what order
## WARNING (1)
### [Security] Another local user can take over the bundle directory for --eval/stdin source: its name is predictable (`workerd-bundle-<pid>-<n>` in the shared temp dir) and `create_dir_all` reuses an existing directory, so a pre-created writable directory lets them replace the bundled code.
- **File:** src/workerd/server/cli/wrangler.rs:333
## INFO (1)
### [Documentation] The README's `workerd run` section could note that on Windows, WORKERD_ESBUILD must point at esbuild.exe unless an esbuild executable is on PATH.
- **File:** README.md:118
## SUGGESTION (1)
### [Code Quality] If esbuild exits before reading the source, the stdin write fails with BrokenPipe and returns early, hiding esbuild's stderr. Ignore BrokenPipe and still call wait_with_output().
- **File:** src/workerd/server/cli/wrangler.rs:408
🔒 Security: ⚠️ 1 warning
The bundle directory for --eval/stdin source has a predictable name in the shared system temp dir.
Recommendation: 📝 Minor Comments
- [WARNING] Another local user can take over the bundle directory for --eval/stdin source: its name is predictable (
workerd-bundle-<pid>-<n>in the shared temp dir) andcreate_dir_allreuses an existing directory, so a pre-created writable directory lets them replace the bundled code. (src/workerd/server/cli/wrangler.rs:333)
View full review
src/workerd/server/cli/wrangler.rs:326-336: ForInput::Source, the bundle is written totemp_dir()/workerd-bundle-<pid>-<counter>.- The name can be guessed, and the counter starts at 0.
- The directory is prepared with
remove_dir_allfollowed bycreate_dir_all, andcreate_dir_allaccepts a directory that already exists.
- On a host where several users share
/tmp, another local user can create that directory ahead of time and make it world-writable. No race is needed. The sticky bit stops the victim'sremove_dir_allfrom deleting it.- esbuild then writes
index.jsinto a directory the attacker controls. - The attacker can replace that file before workerd reads it back, so their code runs as the victim's Worker.
- esbuild then writes
- Fix: create a directory with a random name, atomically and owner-only. For example, use
fs::create_dirwith mode 0700 and a random suffix, retrying onAlreadyExists, or atempfile::TempDir-style helper. Do not callremove_dir_allbeforehand. - The rest of the change keeps the trust model of
serve wrangler.jsonc:runonly bundles files the invoking user names.- The default socket binds
localhost. - The port fallback only moves forward from the configured port.
📋 Code Quality: 💬 1 suggestion
One minor error-reporting issue in the esbuild stdin path.
Recommendation: 💬 Optional Nits
- [SUGGESTION] If esbuild exits before reading the source, the stdin write fails with BrokenPipe and returns early, hiding esbuild's stderr. Ignore BrokenPipe and still call wait_with_output(). (
src/workerd/server/cli/wrangler.rs:408)
View full review
src/workerd/server/cli/wrangler.rs:405-411: TheInput::Sourcepath writes the source to esbuild's stdin withstdin.write_all(...)?, inside theand_thenclosure.- If esbuild exits before it reads stdin, the write fails with
BrokenPipeandwait_with_output()is never called. esbuild might exit early because it rejects an option, or becauseWORKERD_ESBUILDpoints at the wrong binary. - The user then sees
<esbuild>: Broken pipeinstead of esbuild's own stderr.
- If esbuild exits before it reads stdin, the write fails with
- Fix: ignore a
BrokenPipeerror from the write, then still callwait_with_output(), so the real exit status and stderr are reported.
📝 Documentation: 💬 1 info
The README could mention the Windows esbuild limitation.
Recommendation: 💬 Optional Nits
- [INFO] The README's
workerd runsection could note that on Windows, WORKERD_ESBUILD must point at esbuild.exe unless an esbuild executable is on PATH. (README.md:118)
View full review
README.md: The newworkerd runsection does not mention the Windows limitation described in the pull request.runcannot launchnode_modules/.bin/esbuildoresbuild.cmd.- So
WORKERD_ESBUILDmust point atesbuild.exe, unless an esbuild executable is onPATH.
- A one-line note would save Windows users some confusion.
✅ All other checks passed
📖 AGENTS.md: ✅ LGTM
The src/workerd/server/AGENTS.md updates match the change.
⚡ Performance: ✅ LGTM
The port fallback is capped, bundling runs once per config load, and the rest of the change is one-off CLI work.
Reviewed commit: cd7b5a6a
| ); | ||
| let out = match input { | ||
| Input::File(_) => root.join(".wrangler/tmp").join(dir), | ||
| Input::Source { .. } => std::env::temp_dir().join(format!("workerd-{dir}")), |
There was a problem hiding this comment.
Another local user can take over the bundle directory for --eval/stdin source: its name is predictable (workerd-bundle-<pid>-<n> in the shared temp dir) and create_dir_all reuses an existing directory, so a pre-created writable directory lets them replace the bundled code.
For source given with --eval/stdin, the output directory is temp_dir()/workerd-bundle-<pid>-<counter>, and that name can be guessed. On a multi-user host, another user can create it ahead of time with mode 0777. Because of the sticky bit, remove_dir_all cannot delete it, and create_dir_all then accepts it. esbuild writes index.js there, and the other user can replace it before workerd reads it, so their code runs as this Worker. Create the directory with a random name, atomically (fs::create_dir, mode 0700, retry on AlreadyExists), instead of remove-then-create_dir_all.
|
Given that |
|
@jasnell sure, we can do that. I'll wait for some more feedback from the team before iterating further |
ry
left a comment
ry
left a comment
There was a problem hiding this comment.
I don't think we should add this yet.
workerd testalready exists.- Most applications need bindings, and bindings have to come from a config file. Without one,
workerd run index.jshas no well-defined behavior. - We should be wary of adding subcommands, especially ones without well-defined behavior.
- Probably the config file should be the entrypoint.
- I do want to support
workerd config.jsonas an alias forworkerd serve config.json - The most obvious direction for
workerd runis as an alias forserve. But let's hold off onrun, since we may want it for something else later.
|
I think this is great.
Maybe it would make sense to support
Shouldn't need to bundle anything for Python. Would love to see .py files just work (TM) with this command. |
workerd runserves or tests one Worker without a full config file. The code is a JavaScript or TypeScript file, or source given on the command line:Code
serve wrangler.jsoncbundlesmain. esbuild comes fromWORKERD_ESBUILD, else the nearestnode_modules/.bin/esbuild, elseesbuildonPATH. TypeScript, relative imports and npm packages work, and--watchreloads when any input to the bundle changes.--eval <source>(-e), or from stdin when<file>is-, so a here-document works. It goes to esbuild's stdin as TypeScript. Its imports resolve from the working directory, and the bundle is written to the system temp directory, so nothing is left in the working directory. Reading stdin under--watchis a usage error, because a reload cannot read it again.run,-emeans--eval;--external-addrkeeps its long form there, and-ekeeps its meaning in every other command.Serving and testing
localhost:8787, as a wrangler config does. If 8787 is unavailable, it listens on the next free port, trying up to 10; a port given with--port,--socket-addror a template is used as it is, and fails if taken.runprintsListening on <url>to stdout for each HTTP(S) socket once it listens. With--port 0or a moved default port, that line gives the port in use.--testruns the Worker's tests instead of serving it, asworkerd testdoes, and exits with their result. It adds no socket and conflicts with--port.Configuration
--compat-date,--compat-flag(repeatable) and--autogate(repeatable) set the Worker's options.--autogatetakes the gate name with or without theworkerd-autogate-prefix; an unknown name is a usage error listing the valid ones.--config <template>provides bindings, Durable Objects and other services. The template is a Cap'n Proto config, compiled as forserve, with exactly one Worker that has no modules; the code becomes that Worker's modules.--compat-datereplaces the template's date.--compat-flagand--autogateadd to the template's lists. A template without sockets gets one onlocalhost:<port>; one with sockets keeps them,--socket-addroverrides them, and--portis an error.The rest of
serve's options apply.Errors
--watchwaits for a fix.Implementation
cli/run.rsbuilds the config: it bundles the code and fills it into the template, or into a lone Worker namedmain.cli/wrangler.rsexposes its bundling and module-writing helpers, andbundle()takes source on stdin as well as a file. For source, esbuild runs withPWDset to its working directory, so the source's path in esbuild's messages stays relative.Configcan be read back as a message, which is how the template is filled.Controlprints theListening onlines whenRunOptions::announceis set. Onlyrunsets it.RunOptions::port_attemptsletsrun's default socket move to the next free port when its own can't be bound. The server tries the ports itself as it binds, so no other process can take a port between a check and the bind.autogate_names()infactory/bootstrap.{h,c++}andcli/bridge.rslets--autogatebe checked.serve wrangler.jsoncbehaves as before, except that the esbuild-not-found message no longer assumes a wrangler config.Out of scope
--print-config.node_modules/.bin/esbuildis a shell script and npm's Windows launcher isesbuild.cmd, neither of whichruncan launch, soWORKERD_ESBUILDmust point atesbuild.exeunless one is onPATH. The lookup is shared withserve wrangler.jsoncand can be fixed separately.