Add Bun runtime support - #378
Conversation
Creates per-thread UDS mirrors for secure HTTP/TCP servers, writing YAML metadata files containing certificate information. Includes cleanup helpers for socket lifecycle management and comprehensive unit tests for metadata serialization and file operations.
When symphony connects to a per-thread UDS mirror and sends a PROXY v1 header, strip it before the HTTP parser sees it and set socket.remoteAddress / socket.remotePort to the real client values. Uses prependListener so our one-time data handler fires before Node's HTTP parser. socket.unshift() returns any non-header bytes back to the read buffer. Connections without a PROXY header pass through unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace node-unix-socket native addon with native reusePort (Node v22+)
- Add BunRequest adapter wrapping Web Fetch API Request
- Branch http.ts to use Bun.serve() with fetch handler when running on Bun
- Bridge Operations API (Fastify) to Bun via inject() in bunDelegateToNodeServer
- Add listenOnPortsBun() using Bun.serve({ reusePort: true }) per worker
- Support TLS/secure ports on Bun via Bun.serve({ tls: { cert, key } })
- Create UDS mirror sockets for secure ports via Bun.serve({ unix })
- Guard Node-specific APIs: v8, inspector, worker.performance, TLS monkey-patches
- Fix performance.eventLoopUtilization() NotImplementedError on Bun
- Skip Node version check when running on Bun
- Parameterize integration tests via HARPER_RUNTIME env var (node|bun)
- Add CI jobs to run integration tests and API tests on Bun
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Required by repo policy that all actions must be pinned to a full-length commit SHA. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The 4.x-upgrade test returns early from before() when HARPER_LEGACY_VERSION_PATH is not set, leaving ctx.harper undefined. killHarper/teardownHarper now no-op gracefully in that case instead of crashing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
listenOnPortsBun() was using +port directly on keys like "127.0.0.14:9926", which produces NaN and hits the isNaN guard, skipping every port. Parse host:port strings the same way listenOnPorts() does for Node — split on the last colon and pass hostname + numeric port to Bun.serve(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
On Node, Harper's httpChain auth middleware sets request._nodeRequest.user for loopback connections in dev mode (AUTHORIZE_LOCAL), which Fastify picks up via req.raw.user and bypasses its own auth check (fastifyAuth.js:35). On Bun, bunDelegateToNodeServer calls fastify.inject() with a fresh synthetic request — no req.raw.user, so Fastify re-runs auth and returns 401. Fix: strip any incoming x-harper-internal-pre-auth-user header (preventing forgery), then set it in the inject() call when Harper's auth middleware has already authenticated the request. fastifyAuth.js trusts this header as an equivalent to req.raw.user. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The 'send' module (used for static files) calls setHeader/writeHead on the pipe destination. In the Bun handler, the destination must be a Writable with a ServerResponse shim so those calls capture headers into the web Response. The critical bug: 'on-finished' (a send dependency) calls isFinished() which checks msg.finished. In Bun, Writable.finished is undefined (not a boolean), so isFinished() returns undefined. Since undefined !== false, on-finished immediately schedules cleanup() via setImmediate, destroying the ReadStream before any data flows — causing the response to hang forever. Fix: add finished: false to the shim object so isFinished() sees a boolean false and correctly waits for the 'finish' event before calling cleanup(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
👀 |
…CE_EXIT` self-termination mechanism
… headers in Node.js v24+
…rove `process.exit` handling for Bun by force-closing server connections.
… headers in Node.js v24+
…to single HTTP worker - Remove socket file descriptor passing and proxying logic from http.ts and threadServer.js - Remove startSocketServer and session affinity implementation from socketRouter.ts - Remove proxySocket, proxyRequest, and deliverSocket functions - Simplify lite.js to only start HTTP threads without socket servers - Add Windows-specific CI workflow for build and integration tests - Limit Windows to single HTTP worker (no SO_REUSEPORT support) - Fix package.json test scripts to use double quotes for Windows compatibility - Disable segfault handler on Bun runtime
| // On Bun, Harper's auth middleware passes pre-authenticated users via this internal header. | ||
| // It is stripped from real network requests in bunDelegateToNodeServer, so it is safe to trust here. | ||
| const preAuthUser = req.headers?.[INTERNAL_USER_HEADER]; | ||
| if (preAuthUser) return next(null, JSON.parse(preAuthUser)); |
There was a problem hiding this comment.
Auth bypass on Node.js (same concern as prior review, still unaddressed)
fastifyAuth.js trusts x-harper-internal-pre-auth-user without a Bun guard. On Bun this is safe — bunDelegateToNodeServer strips the header before fastify.inject(). On Node.js, Fastify receives the raw IncomingMessage via the unhandled-event cascade, with all client headers intact. Any client can forge this header and bypass authentication.
Suggested fix: gate the header check on the Bun runtime:
| if (preAuthUser) return next(null, JSON.parse(preAuthUser)); | |
| const preAuthUser = typeof globalThis.Bun !== 'undefined' && req.headers?.[INTERNAL_USER_HEADER]; |
|
|
||
| // Handle entry events for the default entry handler based on the `files` and `urlPath` options | ||
| scope.handleEntry((entry) => { | ||
| logger.error('static received entry', entry); |
There was a problem hiding this comment.
ReferenceError on every static file scan (same concern as prior review, still unaddressed)
logger is not defined in this scope — the scoped logger is scope.logger (used correctly on line 32). This will throw on the first entry event fired by the static plugin, breaking static file serving entirely. Looks like a debug line accidentally left in.
| logger.error('static received entry', entry); | |
| scope.logger.debug('static received entry', entry); |
Or just remove the line.
|
|
||
| - name: Build | ||
| run: npm run build | ||
| continue-on-error: true # we currently have type errors so just ignore that |
There was a problem hiding this comment.
Build failures silently ignored on Windows
continue-on-error: true suppresses all build failures (not just type warnings). Per repo policy, Harper core's build should pass cleanly — the tsc || true pattern is explicitly called out as not applicable here. A CI job that passes even when compilation fails provides false confidence.
The comment acknowledges existing type errors; those should be fixed before enabling the Windows build job, not masked. If this job is meant to be informational-only in the interim, consider making the whole job continue-on-error: true at the job level and documenting the known failures in a tracking issue.
Windows job was missing `npm install --ignore-scripts` to recreate node_modules/.bin wrappers after artifact download, causing harper-integration-test-run to be unrecognized. Both Windows and Bun jobs were calling test:integration which no longer includes a glob pattern; switch to test:integration:all. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
# Conflicts: # server/serverHelpers/Request.ts
| // On Bun, Harper's auth middleware passes pre-authenticated users via this internal header. | ||
| // It is stripped from real network requests in bunDelegateToNodeServer, so it is safe to trust here. | ||
| const preAuthUser = req.headers?.[INTERNAL_USER_HEADER]; | ||
| if (preAuthUser) return next(null, JSON.parse(preAuthUser)); |
There was a problem hiding this comment.
Blocker (re-raised from prior review — still unresolved): The comment says the header is stripped in bunDelegateToNodeServer, but that only covers the Bun code path. On Node.js, Fastify receives requests via the unhandled event at http.ts:279 with the raw nodeRequest — no header stripping occurs before this point. Any client can send x-harper-internal-pre-auth-user on Node.js to bypass auth.
| if (preAuthUser) return next(null, JSON.parse(preAuthUser)); | |
| if (typeof globalThis.Bun !== 'undefined' && preAuthUser) return next(null, JSON.parse(preAuthUser)); |
|
|
||
| // Handle entry events for the default entry handler based on the `files` and `urlPath` options | ||
| scope.handleEntry((entry) => { | ||
| logger.error('static received entry', entry); |
There was a problem hiding this comment.
Blocker (re-raised from prior review — still unresolved): logger is not defined in this module. This throws ReferenceError: logger is not defined on the first entry event, breaking static file serving entirely. The available logger is scope.logger, but this line looks like a debug leftover and should simply be removed.
| logger.error('static received entry', entry); |
… debug log - security/fastifyAuth.js: only trust x-harper-internal-pre-auth-user when running under Bun. On Node.js, Fastify receives raw sockets directly without the header-stripping that bunDelegateToNodeServer applies, so an unguarded trust would let any client forge the header and bypass auth. - server/static.ts: remove leftover debug `logger.error` call that referenced an undefined `logger` symbol and would throw ReferenceError on the first entry event. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This reverts commit 4eb8361.
| }); | ||
| if (typeof globalThis.Bun === 'undefined') { | ||
| const origCreateSecureContext = tls.createSecureContext; | ||
| tls.createSecureContext = function (options) { |
There was a problem hiding this comment.
Good news, we should be able to remove this hack once we drop Node 20! 🎉
| */ | ||
| class BunRequestBody { | ||
| #webRequest: globalThis.Request; | ||
| #readable: any; // lazily created Readable stream |
There was a problem hiding this comment.
We can/should probably declare the type.
| #readable: any; // lazily created Readable stream | |
| #readable: Readable; // lazily created Readable stream |
| '--expose-internals', // expose Node.js internal utils so jsLoader can use `decorateErrorStack()` | ||
| ]; | ||
| if (envMgr.get(hdbTerms.CONFIG_PARAMS.THREADS_HEAPSNAPSHOTNEARLIMIT)) | ||
| const isBun = typeof globalThis.Bun !== 'undefined'; |
There was a problem hiding this comment.
Already defined above
| const isBun = typeof globalThis.Bun !== 'undefined'; |
| shutdownWorkers(name); // set the state of all the workers to shut down. this should finish the important stuff synchronously | ||
| return Promise.all(workers.map((worker) => worker.terminate())); | ||
| if (isBun) { | ||
| // worker.terminate() triggers a NAPI segfault in Bun; ask workers to self-exit instead |
There was a problem hiding this comment.
Jarred mentioned worker.terminate() was not amazing.
| } | ||
| } | ||
|
|
||
| function startHTTPWorker(index, threadCount = 1, shutdownWhenIdle?) { |
There was a problem hiding this comment.
shutdownWhenIdle is an interesting feature, do we no longer need this?
| externalLogger.tag = null; // don't tag by default | ||
| if (isMainThread) { | ||
| if (isMainThread && typeof globalThis.Bun === 'undefined') { | ||
| // Bun will crash with the segfault handler, ironically |
…ADER absent In Node.js mode, when the httpChain returns status -1 for the operations API port (auth.ts is not in that port's chain), nodeRequest.baseRequest is set on the IncomingMessage. fastifyAuth.js then calls authentication() on it, which applies AUTHORIZE_LOCAL for loopback connections. In Bun mode, requests are delegated via fastify.inject() which has no equivalent baseRequest mechanism. When INTERNAL_USER_HEADER is absent (auth.ts didn't run for the ops API port), fastify's authorize fell through to passport with no credentials, returning 401 for every unauthenticated ops API call even with AUTHORIZE_LOCAL=true. Fix: mirror the Node.js baseRequest path for Bun — build a minimal request shim from the Fastify inject context (ip comes from req.socket.remoteAddress which is '127.0.0.1' in inject) and call authentication() so AUTHORIZE_LOCAL can authenticate loopback requests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| // baseRequest — build a shim request and call authentication() so AUTHORIZE_LOCAL can apply. | ||
| const shimRequest = { | ||
| headers: { asObject: Object.assign({}, req.headers) }, | ||
| ip: req.socket?.remoteAddress ?? '', |
There was a problem hiding this comment.
AUTHORIZE_LOCAL widens to all clients on Bun, not just loopback.
fastify.inject() hardcodes req.socket.remoteAddress to '127.0.0.1' regardless of the real client IP (as the commit message acknowledges). This means when AUTHORIZE_LOCAL=true (DEV_MODE or AUTHENTICATION_AUTHORIZELOCAL), any client that reaches the Bun ops API port is authenticated as superuser — not just loopback clients. On Node.js, baseRequest carries the real socket IP so only actual 127.0.0.x/::1 clients are elevated.
Fastify's inject accepts a remoteAddress option to propagate the real IP. The fix is in bunDelegateToNodeServer (http.ts), not here:
| ip: req.socket?.remoteAddress ?? '', | |
| ip: req.socket?.remoteAddress ?? '', |
Pass the real client IP through inject so this check works correctly:
// in bunDelegateToNodeServer (http.ts), change:
const injectResult = await fastify.inject({
method: webRequest.method,
url: url.pathname + url.search,
headers,
payload: body,
remoteAddress: bunRequest?.ip, // ← add this
});Then req.socket.remoteAddress here reflects the real IP and AUTHORIZE_LOCAL stays loopback-only.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| url: url.pathname + url.search, | ||
| headers, | ||
| payload: body, | ||
| }); |
There was a problem hiding this comment.
Prior blocker still open: AUTHORIZE_LOCAL widens to all clients on Bun
fastify.inject() without remoteAddress causes Fastify's mock socket to report remoteAddress: '127.0.0.1'. When INTERNAL_USER_HEADER is absent (ops API requests where Harper auth didn't run), fastifyAuth.js falls into the shimRequest path and calls authentication() with ip: req.socket?.remoteAddress — which is always '127.0.0.1'. If AUTHORIZE_LOCAL is configured, every external client that reaches this port is elevated to superuser.
| }); | |
| const injectResult = await fastify.inject({ | |
| method: webRequest.method, | |
| url: url.pathname + url.search, | |
| headers, | |
| payload: body, | |
| remoteAddress: bunRequest?.ip, | |
| }); |
|
|
||
| - name: Build | ||
| run: npm run build | ||
| continue-on-error: true # we currently have type errors so just ignore that |
There was a problem hiding this comment.
Prior blocker still open: Windows CI silently ignores build failures
continue-on-error: true on the Build step means a broken dist/ artifact is uploaded and the downstream run-integration-tests-windows job runs against it. Per AGENTS.md, Harper core's build must pass cleanly. Either fix the type errors and remove this flag, or — if Windows is explicitly best-effort for this PR — move continue-on-error: true to the job level so the downstream test job is skipped instead of running on a broken artifact.
The redirector component test randomly failed because it relied on array ordering of rules which were randomly sorted due to ULID generation in the same millisecond during CSV load. Updated to explicitly look up a unique rule. Also removed try/catch from teardownHarper hooks across component tests since the upstream issue in @harperfast/integration-testing is now resolved in v0.3.1.
| const injectResult = await fastify.inject({ | ||
| method: webRequest.method, | ||
| url: url.pathname + url.search, | ||
| headers, | ||
| payload: body, |
There was a problem hiding this comment.
The fastify.inject() call still doesn't pass remoteAddress, so req.socket.remoteAddress inside Fastify is always '127.0.0.1'. The new shimRequest path in fastifyAuth.js uses that value for ip, meaning any external client that reaches the ops API port gets AUTHORIZE_LOCAL elevation on Bun (commit 47ae9304e acknowledges this in its message: "ip comes from req.socket.remoteAddress which is '127.0.0.1' in inject").
| const injectResult = await fastify.inject({ | |
| method: webRequest.method, | |
| url: url.pathname + url.search, | |
| headers, | |
| payload: body, | |
| const injectResult = await fastify.inject({ | |
| method: webRequest.method, | |
| url: url.pathname + url.search, | |
| headers, | |
| payload: body, | |
| remoteAddress: bunRequest?.ip, |
| url: url.pathname + url.search, | ||
| headers, | ||
| payload: body, | ||
| }); |
There was a problem hiding this comment.
Prior blocker still open: AUTHORIZE_LOCAL widens to all clients on Bun ops API
fastify.inject() without remoteAddress causes Fastify's mock socket to report remoteAddress: '127.0.0.1'. When INTERNAL_USER_HEADER is absent (the ops API path where auth.ts didn't run), fastifyAuth.js falls to the shimRequest branch and calls authentication() with ip: req.socket?.remoteAddress — which is always '127.0.0.1'. Any external client that reaches the Bun ops API port is then elevated to superuser whenever AUTHORIZE_LOCAL is configured.
Commit 47ae9304e explicitly acknowledges this in its message ("ip comes from req.socket.remoteAddress which is '127.0.0.1' in inject") but doesn't fix it.
| }); | |
| const injectResult = await fastify.inject({ | |
| method: webRequest.method, | |
| url: url.pathname + url.search, | |
| headers, | |
| payload: body, | |
| remoteAddress: bunRequest?.ip, | |
| }); |
|
|
||
| - name: Build | ||
| run: npm run build | ||
| continue-on-error: true # we currently have type errors so just ignore that |
There was a problem hiding this comment.
Prior blocker still open: Windows CI silently ignores build failures
continue-on-error: true on the Build step means a broken dist/ artifact is uploaded, and run-integration-tests-windows runs against it. Per AGENTS.md, Harper core's build must pass cleanly (tsc || true is explicitly not used here). Either fix the type errors and remove this flag, or move continue-on-error: true to the job level so the downstream test job is skipped on a failed build.
91b61bb to
51ff59b
Compare
| // No pre-auth header: auth.ts didn't run for this port (ops API). Mirror what Node.js does via | ||
| // baseRequest — build a shim request and call authentication() so AUTHORIZE_LOCAL can apply. | ||
| const shimRequest = { | ||
| headers: { asObject: Object.assign({}, req.headers) }, |
There was a problem hiding this comment.
Blocker (re-raise — unresolved from prior review): req.socket?.remoteAddress in a fastify.inject() context comes from light-my-request's mock socket, which defaults to '127.0.0.1' regardless of the real client's IP. auth.ts is not registered for the operations API port, so bunRequest.user is always null for ops API requests — meaning INTERNAL_USER_HEADER is never sent, and every ops API request falls into this shim path. The result: when AUTHORIZE_LOCAL is configured, all clients (including external) match ip.includes('127.0.0.') in auth.ts:245 and receive superuser access.
The fix is to pass the real client IP from bunRequest.ip through the inject call:
| headers: { asObject: Object.assign({}, req.headers) }, | |
| ip: req.socket?.remoteAddress ?? '', |
Change bunDelegateToNodeServer to include remoteAddress: bunRequest?.ip in the fastify.inject() options, so that req.socket.remoteAddress inside the handler reflects the actual client IP instead of light-my-request's 127.0.0.1 default.
|
|
||
| - name: Build | ||
| run: npm run build | ||
| continue-on-error: true # we currently have type errors so just ignore that |
There was a problem hiding this comment.
Blocker (unresolved from prior review): continue-on-error: true on the Build step means a broken Windows build silently passes, and the downstream run-integration-tests-windows job runs against a potentially incomplete dist/. Either fix the type errors so this flag can be removed, or move continue-on-error: true to the job level so the test job is skipped when the build fails.
This also includes work for Windows fixes and runs the core integration tests on Windows.
Neither Windows nor Bun is running the full integration:api-tests. There are failures, and there will need to be follow-up work to provide more comprehensive support of these platforms/runtimes. But this should ensure basis support.