harden: bind to 127.0.0.1 by default; honor CAMOFOX_HOST#4730
Closed
robertbentleyhq wants to merge 6 commits into
Closed
harden: bind to 127.0.0.1 by default; honor CAMOFOX_HOST#4730robertbentleyhq wants to merge 6 commits into
robertbentleyhq wants to merge 6 commits into
Conversation
server.js previously called `app.listen(PORT)` with no host argument, which makes Node bind to the unspecified address (0.0.0.0 on most kernels). The launchd wrapper on macOS exported CAMOFOX_HOST=localhost but nothing read it, so the service was reachable on every interface the host had configured -- a silent regression of the host-bind config. Changes: - lib/config.js: add `host` to the loaded config, defaulting to `127.0.0.1` (loopback-only). Trim whitespace and treat empty-string as unset. Forward CAMOFOX_HOST through `serverEnv` so subprocesses see the same value. - server.js: pass HOST to `app.listen(PORT, HOST, ...)`, log it on server:started / EADDRINUSE for operator visibility. - README.md: document CAMOFOX_HOST in the env-var table with the 127.0.0.1 default and the Docker/Fly escape hatch. - tests/unit/config.test.js: lock the loopback default, explicit-override, whitespace trim, empty-string fallback, and serverEnv-forwarding contracts. Why default to loopback: the global CAMOFOX_ACCESS_KEY gate added earlier already enforces auth, but defense-in-depth says the network surface should not be reachable beyond the local machine unless the operator opts in. The Fly.io / Docker deployment flow already sets CAMOFOX_HOST=0.0.0.0 explicitly when it wants public ingress, so the new default doesn't break those. This change pairs with rotating CAMOFOX_ACCESS_KEY on the local launchd wrapper to give browser-control routes both a network-layer and an application-layer gate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Agent-browser sessions need to inspect localhost while the service still uses DataImpulse for external browsing. Add a loopback proxy bypass to normalized Playwright proxy options so local app testing stays local.\n\nAlso quarantine persisted storage-state files when Playwright rejects them during context creation, then retry without the stale state. This prevents one bad profile from blocking tab creation for the default agent identity.
robertbentleyhq
force-pushed
the
harden/access-key-and-loopback-bind
branch
from
July 8, 2026 12:38
7a48fd3 to
689cbe5
Compare
… Firefox 135 tab creation Why: every brain MCP browser_* tab-create had 500'd since ~Jul 8. Root cause established from ground truth, not guesses: commit 080fcd8 ("update browser runtime packages") bumped playwright-core 1.59.1 -> 1.61.1, and playwright-core 1.61 sends a new viewport.isMobile field in the Browser.setDefaultViewport protocol call. The installed Camoufox Firefox binary (135.0.1/beta.24, ~/Library/Caches/camoufox/Camoufox.app) runs an older Juggler build that rejects the unknown field, so the browser LAUNCHES fine but every tab-create fails. camoufox-js declares playwright-core:"*" (no upper bound), which is how the breaking minor slipped in silently with the dep sweep. What was considered: upgrading the Camoufox binary instead (forward fix) was rejected for tonight — larger surface, unattended overnight run; the exact downgrade pin is surgical, needs no native rebuild (better-sqlite3 untouched), and was verified live end-to-end after a launchctl kickstart -k of com.bentleyhq.camofox: browser_health browserConnected:true, browser_open_tab returns a tabId (previously 500), browser_screenshot renders the live dashboard, browser_close_tab ok, no setDefaultViewport errors in the logs. UNPIN CONDITION: when the Camoufox Firefox binary is upgraded past 135.0.1, playwright-core can move forward again — re-test tab creation (the setDefaultViewport call) before and after unpinning. Until then, any "helpful" playwright-core upgrade re-breaks all brain browser_* tools. Context: this working tree IS production (launchd runs it directly, no deploy step, same model as brain-mcp-server) — committed so a stray git checkout cannot silently revert the running fix. Applied during night-shift jo-inc#16 (overnight 2026-07-18 -> 07-19) on the branch that was already checked out. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ght-core pin Why: upstream (jo-inc/camofox-browser) moved 1.11.2 -> 1.12.0 (38 commits) and independently fixed the root cause of the Jul-8 breakage this branch had pinned around: afd7638 "drop fixed viewport for camoufox compatibility" stops sending the Browser.setDefaultViewport call that Camoufox Firefox 135's Juggler rejects. With the call gone, the exact playwright-core=1.59.1 pin (b183706) is no longer load-bearing, so package.json returns to upstream's ^1.58.0 range. NOTE: upstream's own package-lock still resolves playwright-core to 1.59.1, so the running version is unchanged today - the pin's protection now lives in the upstream lockfile + the removed viewport call, not in our manifest. Conflicts resolved (3): server.js import block - union of our quarantinePersistedStorageState import and upstream's new browser-errors / process-ownership imports; tests/unit/config.test.js - kept our host-binding describe block (upstream has no equivalent); package-lock.json - took upstream's, regenerated via npm install against the merged manifest. Fork-only patches retained through the merge (still absent upstream as of 1.12.0): 0821b1e loopback bind by default + CAMOFOX_HOST; 12040cc agent-session hardening (localhost proxy bypass + storage-state quarantine); 689cbe5 idle health-probe skip. Upstream DID adopt access-key auth (jo-inc#586) - that came in with the 1.11.2 baseline already, nothing to reconcile here. Verification at merge time: 53/53 unit+plugin suites green (744 tests); the 13 red suites are tests/e2e/* which require a live SERVER_URL env and fail identically pre-merge. Live service round-trip verified after launchd restart (recorded in the session log). Pre-merge state pushed to fork as b183706 - revert path is git reset --hard b183706 && npm install && kickstart.
Contributor
|
Superseded by #8096, which added the configurable bind host on current master with focused configuration coverage and was released in v1.13.0. This branch now conflicts and also carries unrelated persistence/proxy changes, so we are closing it rather than merging the duplicate implementation. Thank you for raising the default-bind hardening motivation. |
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
server.jspreviously calledapp.listen(PORT)with no host argument, which makes Node bind to the unspecified address (0.0.0.0on most kernels). The launchd wrapper this server ships with on macOS exportsCAMOFOX_HOST=localhost, but nothing in the code reads it -- so the service is reachable on every interface the host has configured. This PR closes that gap and adds documentation + tests for the new env var.The companion global access-key gate (
CAMOFOX_ACCESS_KEY+accessKeyMiddleware) is already in place and unchanged here. Binding to loopback by default gives operators a defense-in-depth layer underneath that auth so the network surface is also reduced.Changes
lib/config.js— addhostto the loaded config, defaulting to127.0.0.1. Trim whitespace, treat empty string as unset. ForwardCAMOFOX_HOSTthroughserverEnvso subprocesses see the same value.server.js— passHOSTtoapp.listen(PORT, HOST, ...), log it onserver:startedandEADDRINUSEfor operator visibility.README.md— documentCAMOFOX_HOSTin the env-var table with the127.0.0.1default and the Docker/Fly escape hatch.tests/unit/config.test.js— lock the loopback default, explicit-override, whitespace-trim, empty-string fallback, andserverEnv-forwarding contracts (5 new tests).Compatibility
Docker / Fly.io / Railway deployments that already want public ingress set
CAMOFOX_HOST=0.0.0.0(or justHOSTif their orchestrator does that) -- they continue to work unchanged. Onlynpm starton a developer laptop changes behavior: it stops listening on0.0.0.0and listens on loopback instead.Test plan
NODE_OPTIONS='--experimental-vm-modules' npx jest tests/unit/config.test.js-- 7 passNODE_OPTIONS='--experimental-vm-modules' npx jest tests/unit/auth.test.js tests/unit/accessKey.test.js-- 70 pass (no regressions on the existing access-key middleware)lsof -nP -iTCP:9377 -sTCP:LISTENshowsTCP 127.0.0.1:9377 (LISTEN)(wasTCP *:9377before)curl http://127.0.0.1:9377/healthreturns 200;curl -H "Authorization: Bearer ..." http://127.0.0.1:9377/tabs?userId=testreturns 200; unauthenticated/tabsreturns 401