Skip to content

fix(tunnel): give the unauthenticated tunnel its own acknowledgement - #718

Merged
Ark0N merged 1 commit into
Ark0N:masterfrom
JDProfresh:fix/tunnel-own-acknowledgement
Oct 27, 2026
Merged

Ark0N merged 1 commit into
Ark0N:masterfrom
JDProfresh:fix/tunnel-own-acknowledgement

Conversation

@JDProfresh

Copy link
Copy Markdown
Contributor

The network acknowledgement (CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK) also satisfied the tunnel guard, and install.sh writes 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:

  • New CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL=1 and --allow-unauthenticated-tunnel on codeman web. The flag is carried from the commander parse through toWebLaunchOptions(), startWebServer() and the WebServer constructor into the route context (InfraPort.allowUnauthenticatedTunnel), where the guard reads it next to the env var (isUnauthenticatedTunnelAcknowledged() in network-auth-policy.ts). buildWebArgs() emits it, so codeman web -d and codeman service install (resolveServicePlan()) carry it. codeman service install takes it because it shares addWebLaunchOptions() with web.
  • CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK / --allow-unauthenticated-network mean the bind only. The guard no longer reads them. install.sh is untouched and never writes the new variable.
  • The variable alone lets API callers through, and so does the flag. Without either, the server still refuses unless the request carries acknowledgeUnauthTunnel: true, which keeps its documented meaning.
  • Additive loginConfigured field on GET /api/tunnel/status, computed in the route handler from the same isLoginConfigured() the guard uses (CODEMAN_PASSWORD set or multi-user on), not in TunnelManager.getStatus(). Neither opt-in variable makes it true.
  • The browser asks first, every time, when there is no login, before the PUT. One helper, _confirmTunnelEnable() in settings-ui.js, reads the status and shows the existing SECURITY WARNING confirm; on confirm the enable goes out with acknowledgeUnauthTunnel: 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.
  • The 403 handler stays as the fallback when the status read fails (rejected, non-OK, or a status without the field).
  • 403 wording. The refusal names 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.
  • No greyed-out toggle, and the welcome link stays.
  • A pre-existing race in the same guard, fixed here. The guard ran only when the tunnel was not running, and the actual start() comes after the settings file read and write. A save that carried tunnelEnabled: true while 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, or acknowledgeUnauthTunnel: 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 writes tunnelEnabled: true, and if the tunnel stopped in the meantime it is not restarted: the answer is the same 403 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 writing tunnelEnabled: true over rolling it back afterwards, because a rollback to false could overwrite a concurrent authorized enable.
  • Changeset (patch): ships as a security fix, and says that anyone who relied on the network variable to start the tunnel needs CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL=1 or --allow-unauthenticated-tunnel.
  • Docs: 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 (the PUT /api/settings row and a GET /api/tunnel/status row with the new field), and the Tunnel row of the Security table in CLAUDE.md. A git grep for the old claim also turned up docs/architecture-invariants.md (Hook bypass row), docs/wiki/Remote-Access.md, docs/wiki/Security.md, docs/tailscale-installer-plan.md and the comments in system-routes.ts and schemas.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: 403 OPERATION_FAILED, the message names CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL and 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 for 0/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 real WebServer constructor and startWebServer() to the route (allowed with it, refused without it).
  • test/cli-tunnel-flag.test.ts: the real commander program carries the flag into startDaemon (web -d), startWebServer (foreground, eighth argument) and installService, true and false.
  • test/daemon-control.test.ts, test/service-installer.test.ts, test/cli-commands.test.ts: buildWebArgs() and resolveServicePlan() argv, flag registration and help.
  • test/install-sh-invariants.test.ts: install.sh never contains CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL.
  • test/tunnel-enable-confirm-ui.test.ts (JSDOM with the real index.html and settings-ui.js, in the npm test gate): 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.
  • The race, in 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 stored tunnelEnabled: false stays false while the other key is saved. With each of password, tunnel variable, flag, ack and multi-user, start is called once and true is 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-guard on tmux 3.2a and the deepseek-transcript zstd case on Node 26).

Verified

Observed on a throwaway server from this branch bound to 127.0.0.1 with its own CODEMAN_INSTANCE, with TunnelManager.start replaced 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):

  • With CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK=1 and no password (my incident setup): GET /api/tunnel/status answered loginConfigured: false, and a PUT /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=1 alone: loginConfigured: false, curl's enable allowed. CODEMAN_PASSWORD set: loginConfigured: true. Nothing set: loginConfigured: false and 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: true from page state that predates a stop can store true again 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

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
@Ark0N

Ark0N commented Oct 26, 2026

Copy link
Copy Markdown
Owner

Thank you, JDProfresh, for the careful work here. This PR gives the unauthenticated Cloudflare tunnel its own opt-in (CODEMAN_ALLOW_UNAUTHENTICATED_TUNNEL / --allow-unauthenticated-tunnel), stops the installer's network acknowledgement from publishing the tunnel, and has the browser ask before every enable when there is no login.

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 start() closes a real hole. The full test gate is green here, and swapping master's system-routes.ts and settings-ui.js back in turns 40 of the new tests red, the incident case included.

Three small things, which I will apply at merge time:

  • docs/api-reference.md:1222: the status shape reads { running, url, ..., loginConfigured }, but getStatus() returns only running and url, so it becomes { running, url, loginConfigured }.
  • src/web/routes/system-routes.ts:1233: the "UNAUTHENTICATED public Cloudflare tunnel" warning still keys on !process.env.CODEMAN_PASSWORD, so it fires in multi-user mode where the tunnel has a login in front of it. That predates this PR, but isLoginConfigured() is right there, so it becomes if (!isLoginConfigured()).
  • test/install-sh-invariants.test.ts:535: the invariant also gets expect(SOURCE).not.toContain('--allow-unauthenticated-tunnel'), since the installer writes service argv as well as env.

One observation that does not block this PR: when the confirm is cancelled in App Settings (settings-ui.js:3338), the save returns before the PUT, so the other synced changes in that save do not reach the server. That is how the old 403 path behaved too, and you documented it. Since the confirm now runs before the PUT, a later change could let the save go ahead with tunnelEnabled: false instead.

This is ready to merge with the three touches above.

@Ark0N
Ark0N merged commit fc0ecc6 into Ark0N:master Oct 27, 2026
2 checks passed
Ark0N pushed a commit that referenced this pull request Oct 27, 2026
- 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>
Ark0N pushed a commit that referenced this pull request Oct 27, 2026
Ark0N pushed a commit that referenced this pull request Oct 27, 2026
…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>
Ark0N pushed a commit that referenced this pull request Oct 27, 2026
Replaces the six contributor changesets (#694, #707, #708, #709, #718,
#713) with one consolidated minor changeset covering all 14 PRs, with the
Thanks section first and the release-review fixes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Ark0N pushed a commit that referenced this pull request Oct 27, 2026
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>
@Ark0N

Ark0N commented Oct 27, 2026

Copy link
Copy Markdown
Owner

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, isLoginConfigured() for the unauthenticated-tunnel warning (it no longer fires in multi-user mode), and the installer invariant for --allow-unauthenticated-tunnel. #719 landed on top of it, so your tunnel-guard tests now read the settings writes through the temp file and rename the new writer uses; they still refuse on every unauthorized path you pinned. Both acknowledgement variables and their flags are now listed as stable in docs/versioning-policy.md.

@JDProfresh
JDProfresh deleted the fix/tunnel-own-acknowledgement branch October 28, 2026 04:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

One click on the Cloudflare Tunnel link publishes a passwordless install with no confirm

2 participants