fix: adversarial audit follow-ups (security, proxy, lifecycle) - #122
Open
daquinoaldo wants to merge 27 commits into
Open
daquinoaldo wants to merge 27 commits into
daquinoaldo wants to merge 27 commits into
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
The job ran pnpm publish and pnpm build (both trigger prepack -> tsc) without ever installing dependencies, so releases always failed. Add the install step, restore publish provenance, and add a concurrency group so two dispatches cannot race. 🤖 Generated with [OpenCode](https://opencode.ai) (Smart-router)
The upgrade handler filtered headers through the standard hop-by-hop filter, stripping Connection and Upgrade. Upstreams therefore received a downgraded plain request and never saw the upgrade, so WebSocket proxying silently never worked. Keep those two headers on the upgrade path only (RFC 9110 §7.8.1, RFC 6455 §4.2.1). 🤖 Generated with [OpenCode](https://opencode.ai) (Smart-router)
…rade The upgrade handler only listened for upgrade and error events. When the upstream answered a non-101 response (e.g. 400/403), nobody consumed it and the client socket hung forever. Relay the upstream status and close both sockets. 🤖 Generated with [OpenCode](https://opencode.ai) (Smart-router)
The path check was purely lexical, so a symlink inside the served folder pointing outside it could expose arbitrary readable files. Resolve the real path of both root and target before serving and reject anything that escapes the root (403). 🤖 Generated with [OpenCode](https://opencode.ai) (Smart-router)
The downloader followed only 302s, never checked the response status, and happily piped 4xx/5xx bodies (or redirect pages from 301/307/308) into the executable file. A truncated or empty cached binary then permanently broke startup with a cryptic exec error because the cache short-circuit never re-downloaded. Check the status code (follow 3xx with a redirect cap, reject anything that is not 200), clean up the partial file on failure, and validate the cached executable (non-empty and executable) before trusting it, re-downloading otherwise. 🤖 Generated with [OpenCode](https://opencode.ai) (Smart-router)
Headers named by the Connection header are hop-by-hop per RFC 9110 §7.6.1 but were forwarded in both directions, leaking connection-scoped headers (and, from the upstream, response-scoped ones) end to end. Parse the Connection list and filter those names too, in both the request and the response path. The WebSocket upgrade path keeps Connection and Upgrade untouched. 🤖 Generated with [OpenCode](https://opencode.ai) (Smart-router)
listen() and redirect() always created a fresh server without closing a previously bound one and without any error listener: a second call on the same port emitted an uncaught EADDRINUSE and left the caller's promise pending forever, while a call on another port silently leaked the old server. Close the previous server when re-listening and reject with the listen error instead. 🤖 Generated with [OpenCode](https://opencode.ai) (Smart-router)
The directory redirect prefixed the full request path with './', so at depth > 1 the browser resolved it against the wrong base (e.g. /deep/a/b redirected to /deep/a/deep/a/b). Emit a root-relative location instead. 🤖 Generated with [OpenCode](https://opencode.ai) (Smart-router)
darwin always downloaded the amd64 binary (an arm64 release asset exists) and linux mapped both arm and arm64 to the armv7 binary, so first run failed on Apple Silicon without Rosetta and on arm64 Linux. 🤖 Generated with [OpenCode](https://opencode.ai) (Smart-router)
getExe() ran process.exit(0) on unsupported platforms. getCerts() calls it on every invocation, so any application importing the library on an unsupported platform was silently terminated with a success exit code. Throw a descriptive error and let the caller decide how to handle it. 🤖 Generated with [OpenCode](https://opencode.ai) (Smart-router)
serve --port abc dumped a raw ZodError with a stack trace. Also, the start calls (proxy/serve/redirect) were fire-and-forget, so a rejection (e.g. a certificate download failure) died through Node's default unhandled-rejection path instead of the friendly EACCES/EADDRINUSE handler. Summarize validation issues as one line each and route both uncaught exceptions and unhandled rejections through the same handler. 🤖 Generated with [OpenCode](https://opencode.ai) (Smart-router)
The redirect replaced the first ":<httpPort>" occurrence anywhere in the Host header, which could mangle hosts containing that substring and mishandled IPv6 literals. Parse the port with lastIndexOf and the bracket rule for IPv6 hosts instead. 🤖 Generated with [OpenCode](https://opencode.ai) (Smart-router)
Incremental build info files were written into dist/ and shipped in the npm package (they appeared in pnpm pack). Write them to .cache/tsc/ instead. Also expose ./package.json through the exports map so tooling that reads a package version keeps working. 🤖 Generated with [OpenCode](https://opencode.ai) (Smart-router)
The build used POSIX-only shell constructs (rm -rf, echo redirection) which fail under cmd.exe on Windows. Replace it with a small Node script performing the same steps portably. 🤖 Generated with [OpenCode](https://opencode.ai) (Smart-router)
Multi-range requests were answered with a 206 serving only the first range, silently dropping the rest. RFC 9110 §14.2 lets a server ignore Range, so answer unparseable-but-bytes-shaped and multi-range requests with 200 while keeping 416 for genuinely unsatisfiable single ranges. 🤖 Generated with [OpenCode](https://opencode.ai) (Smart-router)
Document the -h/--help flag the CLI supports, clarify when to install the module as a dev dependency versus a runtime dependency, and warn that proxy mode serves every response with Access-Control-Allow-Origin: *, so any website the developer visits can read the proxied backend. 🤖 Generated with [OpenCode](https://opencode.ai) (Smart-router)
CLI coverage relied on the bin heuristic; an explicit entry keeps it detected if the bin field ever changes. 🤖 Generated with [OpenCode](https://opencode.ai) (Smart-router)
knip already detects src/cli.ts through the bin field and warns that an explicit entry is redundant. Drop it and keep the configuration hint free. 🤖 Generated with [OpenCode](https://opencode.ai) (Smart-router)
After a WebSocket upgrade the client and upstream sockets were piped together with no error or close handling: a disconnect left both sides half-open forever, and a socket error on either side could crash the process with an unhandled error event. Destroy the other side on error or close. 🤖 Generated with [OpenCode](https://opencode.ai) (Smart-router)
Cover Connection-listed hop-by-hop filtering, the non-101 upgrade relay, WebSocket upgrade header forwarding with bidirectional data, root-relative directory redirects (including nested), the IPv6-safe http->https redirect, and the symlink confinement. 🤖 Generated with [OpenCode](https://opencode.ai) (Smart-router)
- proxy: validate every header name against the RFC 9110 token grammar before writing it to the outgoing headers object (remote property injection), and always write lowercased names. - static: validate the directory-redirect Location against a strict printable-ASCII same-site path shape before writing it (server-side URL redirect). - static: open the file first and fstat the descriptor so the metadata and the content served come from the same open file (file-system race / TOCTOU between statSync and readFileSync). - certs: retry execFile briefly on ETXTBSY, fixing the Ubuntu CI flake where the freshly downloaded mkcert binary is still held by the downloader fd. - test: create the outside-symlink target in a random mkdtemp directory instead of a predictable file in the shared temp dir (insecure temporary file). 🤖 Generated with [OpenCode](https://opencode.ai) (Smart-router)
🤖 Generated with [OpenCode](https://opencode.ai) (Smart-router)
… warning https-localhost is a dev server: it is always a dev dependency. Also remove the CORS warning that overstated the exposure of a localhost development tool. 🤖 Generated with [OpenCode](https://opencode.ai) (Smart-router)
Replace the helper build script with the two-step script used by the Bending Spoons typescript monorepo libraries (pico, orion): one tsc project per module format, then write the dist package.json type marker. Reuses master's fix/lint scripts that the rebase had clobbered. 🤖 Generated with [OpenCode](https://opencode.ai) (Smart-router)
daquinoaldo
force-pushed
the
audit/fixes
branch
from
September 16, 2026 11:31
44c42c4 to
5a0f31e
Compare
Ubuntu's execFile call can surface ETXTBSY while the freshly downloaded binary is still held by the downloader as a synchronous spawn error, which bypassed the callback-only retry and failed both Ubuntu test jobs. Handle both synchronous throws and callback errors with the same bounded retry. 🤖 Generated with [OpenCode](https://opencode.ai) (Smart-router)
The ETXTBSY retries were not enough on the Ubuntu runners: the writer's file descriptor kept the executable busy well past the retry window. Download to a sibling temp file and atomically rename it to the final name, giving the executable a fresh inode no writer holds. 🤖 Generated with [OpenCode](https://opencode.ai) (Smart-router)
The Ubuntu runners kept the freshly created destination inode busy even after the download stream closed. Delay creating the temp output file until a 200 response is confirmed, then atomically rename it into place; no downloader-created executable inode is ever spawned. 🤖 Generated with [OpenCode](https://opencode.ai) (Smart-router)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Findings from a deep adversarial review, one commit per finding. Everything was verified empirically before fixing (raw-socket clients, spy upstreams, symlink harnesses). Rebased on master after #123/#124/#125.
High
3c3c23d): the job ranpnpm publish/pnpm build(both triggerprepack→tsc) without ever installing dependencies, so releases always failed. Adds the install step, restores--provenance, adds a concurrency group.8dcb658): the upgrade handler strippedConnection/Upgradethrough the standard hop-by-hop filter, so upstreams never saw an upgrade request. Preserved on the upgrade path only.920144d): the handler listened only forupgrade/error; a 400/403 from upstream parked both sockets. Now relays the status and closes both.ad6cf6c): the path check was purely lexical; a symlink inside the served root could expose arbitrary readable files (verified against/etc/passwd-style targets). Now resolves real paths of root and target and 403s anything outside.d4a8389): never checked the HTTP status (4xx/5xx bodies and 301/307/308 redirect pages were piped into the "binary" and exec'd); a truncated/empty cached binary permanently broke startup. Now: status checks, redirect cap, cleanup on failure, cache validation with re-download.Medium
1aa5526): headers named byConnection:were forwarded in both directions (RFC 9110 §7.6.1). Parsed and filtered both ways.listen()/redirect()lifecycle (38dc257): a secondlisten()emitted an uncaught EADDRINUSE and left the caller's promise pending forever; on another port it leaked the old server. Now closes the previous server and rejects with the error.7780516):./deep/a/b/resolved against the wrong base. Root-relative now.d0b0f91): darwin always downloaded amd64; linux mapped arm64 to armv7. Now per-arch (assets verified to exist upstream).process.exit(0)in library code (028dd53): an importing app on an unsupported platform was silently terminated with exit code 0. Throws now.Low
Fixed the dead update check (
9b857d0), friendlier CLI errors + unhandled-rejection routing, port-stripping in redirect Location incl. IPv6 (b37f7d8), tsbuildinfo out of the npm tarball +./package.jsonexport (1d4a615), multi-range requests served as full representations (60d2be1), docs:--helpflag (40b2493), dropped the redundant knip entry (d4c4efa).Tunnel teardown when one peer disconnects (
d511d1a).CodeQL / GH security findings (
e2355b5)fstats the descriptor, so metadata and content come from the same open file (TOCTOU between stat and read eliminated).mkdtempdirectory instead of a predictable temp path.Post-review adjustments
test/fixtures/sub/deep/index.htmlfixture (aa06e1b).d337602).tscproject per module format plus adist/*/package.jsontype marker, as plain pnpm scripts, no helper file (5a0f31e).CI fixes
ETXTBSYflake (2c4abc3,117a973,5ded146):execFileon a freshly downloaded binary failed withspawn ETXTBSYwhile the writer's descriptor was still open — the callback-only retry missed the synchronous spawn error, and even the retry window was too short on the runners. The downloader now writes to a sibling temp file (created only after a 200 response) and atomically renames it into place, so the executable inode is never held by a writer; a bounded retry remains as a safety net for both sync and callback errors.Tests
New coverage for all fixed behaviors (Connection filtering, non-101 relay, WS upgrade forwarding, nested redirects, IPv6 redirect, symlink confinement). All checks green: CodeQL pass, Test pass on macOS and Ubuntu (Node 24 and 26).
🤖 Generated with OpenCode (Smart-router)