Repository navigation
fix(tunnel): give the unauthenticated tunnel its own acknowledgement - #718
Conversation
The network acknowledgement that install.sh writes for every passwordless LAN install also satisfied the tunnel guard, so one click on the welcome screen's tunnel link published full terminal control to a public URL with no login and no confirm. The tunnel now needs CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL or --allow-unauthenticated-tunnel (carried by web -d and service install), and with no login the browser asks before every enable, reading a new loginConfigured field from GET /api/tunnel/status. The guard is now decided once per request and required again where the tunnel starts, so a save that found the tunnel running can no longer restart it unguarded after a concurrent stop. Closes Ark0N#706
|
Thank you, JDProfresh, for the careful work here. This PR gives the unauthenticated Cloudflare tunnel its own opt-in ( It matches the answer on #706 point by point, and the race fix in the same guard is welcome: deciding the authorization once and checking it again at Three small things, which I will apply at merge time:
One observation that does not block this PR: when the confirm is cancelled in App Settings ( This is ready to merge with the three touches above. |
- docs/api-reference.md:1222: the tunnel status shape is { running, url, loginConfigured } (getStatus() returns only running and url)
- src/web/routes/system-routes.ts:1233: the unauthenticated-tunnel warning keys on !isLoginConfigured(), so it no longer fires in multi-user mode
- test/install-sh-invariants.test.ts:537: install.sh must not contain --allow-unauthenticated-tunnel either (it writes service argv as well as env)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s so a quick-start cannot tear the file Conflict in src/web/routes/system-routes.ts (PUT /api/settings) resolved by keeping both sides: #718's line that never stores an unauthorized tunnelEnabled:true runs first, then #719's serialized mutateSettingsFile() write replaces the inline read and plain writeFile. test/routes/system-routes-tunnel-guard.test.ts: #718's new tests asserted plain writeFile calls on settings.json. With #719 every write goes to a temp file renamed over it, so the assertions read writes through a settingsWrites() helper (writeFile contents keyed by their rename), and the concurrent-stop harness applies a write and the stop at the rename. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK and the new CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL (#718), with their CLI flags, join the documented environment variables the versioning policy covers, from 1.46.0, the release that gave each its own meaning. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Merged, and it ships in 1.46.0 (https://github.com/Ark0N/Codeman/releases/tag/codeman%401.46.0). Thank you for reporting #706 and then fixing it point by point; deciding the authorization once and requiring it again at start closed a race nobody had asked about. At merge I applied the three touches from the review: the status shape in the API reference, |
The network acknowledgement (
CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK) also satisfied the tunnel guard, andinstall.shwrites it for every LAN install without a password, so on those installs one click on the welcome screen's Cloudflare Tunnel link published full terminal control to a public URL with no login and no confirm.What changed
Point by point against your answer on #706:
CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL=1and--allow-unauthenticated-tunneloncodeman web. The flag is carried from the commander parse throughtoWebLaunchOptions(),startWebServer()and theWebServerconstructor into the route context (InfraPort.allowUnauthenticatedTunnel), where the guard reads it next to the env var (isUnauthenticatedTunnelAcknowledged()innetwork-auth-policy.ts).buildWebArgs()emits it, socodeman web -dandcodeman service install(resolveServicePlan()) carry it.codeman service installtakes it because it sharesaddWebLaunchOptions()withweb.CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK/--allow-unauthenticated-networkmean the bind only. The guard no longer reads them.install.shis untouched and never writes the new variable.acknowledgeUnauthTunnel: true, which keeps its documented meaning.loginConfiguredfield onGET /api/tunnel/status, computed in the route handler from the sameisLoginConfigured()the guard uses (CODEMAN_PASSWORDset or multi-user on), not inTunnelManager.getStatus(). Neither opt-in variable makes it true._confirmTunnelEnable()insettings-ui.js, reads the status and shows the existing SECURITY WARNING confirm; on confirm the enable goes out withacknowledgeUnauthTunnel: true, on cancel nothing is sent and the tunnel stays off. The welcome link (toggleTunnelFromWelcome), the tunnel panel (_tunnelPanelToggle) and the App Settings save all go through it. The save asks only when it turns the tunnel on: a tunnel the status reports as already running is not asked about (the server does not guard that case either). A cancelled save sends no settings PUT at all, exactly like the server's 403 refusal already did: the tunnel toggle goes back off, and the other settings changed in that save stay on this device until the next save. The confirm text is the existing SECURITY WARNING string, moved unchanged into one helper (_askUnauthTunnel()) that the 403 fallback uses too, so there is no new string to translate.CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL=1, and when the network variable is set it adds that this variable only acknowledges the network bind and no longer covers the tunnel. The refusal also logs one[tunnel] Refused ...line.start()comes after the settings file read and write. A save that carriedtunnelEnabled: truewhile the tunnel was running skipped the guard, and if another request stopped the tunnel during those awaits, the save then started a public tunnel with no login, opt-in or acknowledgement. The authorization (login, tunnel variable, flag, oracknowledgeUnauthTunnel: true) is now decided once per request whether or not the tunnel is running, and required again at the start site. An enable without it is still accepted while the tunnel keeps running (as before), but it never writestunnelEnabled: true, and if the tunnel stopped in the meantime it is not restarted: the answer is the same403 OPERATION_FAILED, saying the tunnel stopped while the settings were being saved, was not restarted, and that the other settings were saved, plus a[tunnel] Refused ...log line. I chose not writingtunnelEnabled: trueover rolling it back afterwards, because a rollback tofalsecould overwrite a concurrent authorized enable.CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL=1or--allow-unauthenticated-tunnel.docs/security-architecture.md(the bind acknowledgement at the startup-policy list, the tunnel caveat that named the network variable, and a new quick-reference row),docs/wiki/Settings-Reference.md(the network row narrowed, a tunnel row added),docs/api-reference.md(thePUT /api/settingsrow and aGET /api/tunnel/statusrow with the new field), and the Tunnel row of the Security table inCLAUDE.md. Agit grepfor the old claim also turned updocs/architecture-invariants.md(Hook bypass row),docs/wiki/Remote-Access.md,docs/wiki/Security.md,docs/tailscale-installer-plan.mdand the comments insystem-routes.tsandschemas.ts, which are updated too.Tests
test/routes/system-routes-tunnel-guard.test.ts, rewritten: the exact incident (network variable set, no password, no ack: 403OPERATION_FAILED, the message namesCODEMAN_ALLOW_UNAUTHENTICATED_TUNNELand says the network variable no longer covers the tunnel, no start, nothing persisted), no opt-in at all, the tunnel variable allowed without an ack for every accepted spelling and refused for0/false/empty/other, the flag through the route context, the ack alone, a false ack, a password, a multi-user admin, disable unguarded, and the status field for password, multi-user, neither, an empty password, the network variable only, the tunnel variable only and the flag only.test/tunnel-flag-plumbing.test.ts: the flag through the realWebServerconstructor andstartWebServer()to the route (allowed with it, refused without it).test/cli-tunnel-flag.test.ts: the real commander program carries the flag intostartDaemon(web -d),startWebServer(foreground, eighth argument) andinstallService, true and false.test/daemon-control.test.ts,test/service-installer.test.ts,test/cli-commands.test.ts:buildWebArgs()andresolveServicePlan()argv, flag registration and help.test/install-sh-invariants.test.ts:install.shnever containsCODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL.test/tunnel-enable-confirm-ui.test.ts(JSDOM with the realindex.htmlandsettings-ui.js, in thenpm testgate): for each of the three entry points, the confirm shows before the PUT when the status says no login even though the PUT would succeed, it asks again on every attempt, cancel sends no PUT and leaves the tunnel off, a configured login or a status without the field skips it, and a failed status read sends the PUT without the ack. Plus: the save does not ask for a tunnel that is already running, the welcome link keeps the 403 fallback and retry, and disabling never asks.system-routes-tunnel-guard.test.ts: the tunnel stub reports running until the settings write and stopped after it. With no authorization, start is not called, the answer is 403 with the not-restarted wording, and the storedtunnelEnabled: falsestays false while the other key is saved. With each of password, tunnel variable, flag, ack and multi-user, start is called once andtrueis stored. An unauthorized enable on a tunnel that keeps running answers 200 without touching the stored value, and the authorization is the one decided at the start of the request (an env change during the write neither grants nor revokes it). Three of these failed before the fix.Every new test failed on master for the reason it names (38 red) except the regression pins, which already held. I also took each guard out one at a time (env var, flag, route-context wiring, ack, the network variable put back, multi-user, password, status field, 403 wording, the confirm per entry point, the running check, the unknown-field case, the failed-read case, the save ack and cancel,
buildWebArgs, the CLI mapping, the foreground and factory forwarding) and watched a test go red for each.npm test: 12640 passed, 2 failed, 12 skipped (the two failures are environment ones this machine shows on master too:tui-switch-guardon tmux 3.2a and thedeepseek-transcriptzstd case on Node 26).Verified
Observed on a throwaway server from this branch bound to
127.0.0.1with its ownCODEMAN_INSTANCE, withTunnelManager.startreplaced by a stub through a preload so no cloudflared was ever launched (every "allowed" below means the stub logged the start call, nothing reached the internet):CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK=1and no password (my incident setup):GET /api/tunnel/statusansweredloginConfigured: false, and aPUT /api/settings {"tunnelEnabled":true}from curl answered 403 with the new message, including the sentence that the network variable no longer covers the tunnel. In Chromium (Playwright), clicking the welcome link read the status and showed the SECURITY WARNING confirm; dismissing it sent no PUT and left the link inactive, accepting it sent{"tunnelEnabled":true,"acknowledgeUnauthTunnel":true}.codeman web --allow-unauthenticated-tunnel: curl's enable without an ack was allowed, and the welcome link still asked first and sent nothing on cancel. The real App Settings Save button with the tunnel box ticked and the tunnel panel's start both asked first; cancelling the save sent no PUT and unticked the box.CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL=1alone:loginConfigured: false, curl's enable allowed.CODEMAN_PASSWORDset:loginConfigured: true. Nothing set:loginConfigured: falseand a 403 without the network sentence.codeman web -d --allow-unauthenticated-tunnel: the detached child's command line carried--allow-unauthenticated-tunnel.Not checked by hand: a phone (the phone overview has no tunnel entry), and the save path with a tunnel actually running (covered by the JSDOM test only, since I did not start a real tunnel).
Two edges I left alone, both outside what the guard protects: a save whose body still carries
tunnelEnabled: truefrom page state that predates a stop can storetrueagain with the tunnel off (only the stored value is wrong; the file-level half of that is the settings write race I am sending as its own PR next), and on the late 403 the browser treats the save as fully refused, so it does not say the other settings were saved and skips the post-save steps. Both need two requests to interleave within a few milliseconds.Closes #706