feat(init): surface reverse-proxy exposure mode in netclaw init#1120
Merged
Aaronontheweb merged 7 commits intoMay 20, 2026
Merged
Conversation
netclaw-dev#1114 Reverse proxy has been a fully-supported ExposureMode in the config schema, daemon runtime, ForwardedHeaders middleware, validator, and doctor check — but the netclaw init wizard never offered it as an option. Operators deploying behind nginx / Caddy / Traefik / IIS / ALB had to hand-edit netclaw.json after init or pick a mode they didn't want. This adds Reverse Proxy as the second option in the Network Exposure step and three new sub-steps that collect Daemon.Host (non-loopback) and Daemon.TrustedProxies (≥1 entry required to advance, matching the daemon's startup validator), then show the operator the resulting serving URL http://{Host}:5199 plus their TLS / firewall / X-Forwarded-* responsibilities before continuing to the webhook toggle. - ExposureModeStepView: Reverse Proxy option + BuildReverseProxyHost, BuildReverseProxyTrustedProxies (with empty-list gate + help line), BuildReverseProxyNotice - ExposureModeStepViewModel: Host + TrustedProxies properties; 5-sub-step plan for reverse proxy; TryAdvance blocks at the trusted-proxies sub-step until ≥1 entry; OnEnter(Back) clamps high-water so mode-switches cannot restore an out-of-range sub-step index - WizardConfigBuilder: DaemonConfigSection extended with Host? + TrustedProxies; serializer emits them only when non-default - Tests: +9 reverse-proxy unit tests covering config emission, sub-step walk, gate behavior, mode-downgrade clamp, bootstrap device, and leakage prevention into non-reverse-proxy Daemon sections - Smoke: new init-wizard-reverse-proxy tape + assertion registered in LIGHT_TAPES; jq-asserts Daemon.ExposureMode / Host / TrustedProxies[0] and runs netclaw doctor - netclaw-operations skill: documents wizard support and the deferral guidance (pick Local + re-run init --resume); version bumped to 2.5.0 - SPEC-007: reverse proxy added to wizard exposure-mode list with the trusted-proxies-required-to-advance rule No changes to DaemonExposureValidator, ExposureModeDoctorCheck, the JSON schema, or the daemon's ForwardedHeaders wiring — the runtime already handles reverse-proxy correctly; this is a UI gap fix only.
…ion noise Simplify follow-up to the reverse-proxy wizard work: - Promote the literal 5199 in DaemonConfig.Port's property initializer to a public const DaemonConfig.DefaultPort. The reverse-proxy notice screen in ExposureModeStepView now references DaemonConfig.DefaultPort instead of carrying a private DefaultDaemonPort copy. BindFromConfiguration uses the same constant for its fallback. - Drop a WHAT-narration comment in ExposureModeStepView.BuildContent — the preceding guards already say the branch is the non-reverse-proxy notice case. - Drop an unnecessary .ToArray() in the WizardConfigBuilder serializer; an IReadOnlyList<string> round-trips through System.Text.Json fine.
06d51bc to
bd2af17
Compare
…mantics
Security review found a latent footgun: the reverse-proxy trusted-proxies
gate in ExposureModeStepViewModel.TryAdvance() returned false when blocking,
but per IWizardStepViewModel's contract, false means "step complete — advance
the wizard." The View layer happened to suppress the AdvanceStep() call when
proxies were empty, so the broken return value never reached the orchestrator
in practice — but a future caller (toolbar Next button, hotkey, programmatic
resume) could trip the wire and silently skip the notice + webhook sub-steps,
producing a config with ExposureMode=reverse-proxy and an empty TrustedProxies
list. Daemon startup would still fail-closed via DaemonExposureValidator, so
this never escalated to silent privilege escalation, but it was contract-shaped
wrong.
- Flip the gate to return true ("handled — staying put for validation") and
document the intent at the call site.
- Clarify IWizardStepViewModel.TryAdvance's XML doc so future steps know
validation-block-stay-put is a true return, not a false one.
- Update the corresponding test to assert the new return + that the sub-step
pointer does not move on a blocked advance.
- Fix a docs reference to a non-existent `netclaw init --resume` flag in the
netclaw-operations skill — init has no --resume; the wizard auto-resumes by
skipping completed steps.
Upstream PR netclaw-dev#1075 added github-copilot as a provider, shifting Ollama from position 2 to position 3 in the alphabetical provider list. The baseline init-wizard.tape was updated to use 'Down 2'; this tape was authored against the pre-Copilot ordering and still used 'Down', selecting github-copilot instead of Ollama. Both Native Smoke (Linux) and Native Smoke (macOS) failed on the next 'Wait+Screen@10s /endpoint:/' because the wizard was waiting on the Copilot OAuth prompt, not the Ollama endpoint prompt. Mirror the current baseline tape's provider-step navigation.
In reverse-proxy mode the daemon starts fine on 0.0.0.0:5199, but the CLI cannot loopback-auto-auth back to it (that path is intentionally disabled for reverse-proxy — forwarded headers must not inherit operator privileges via 127.0.0.1). The wizard's chat-page handshake gets 401 and the wizard drops back to the shell instead of opening the TUI. This is the correct security posture, not a bug. But the tape was waiting for the chat-page-ready marker that only appears when the TUI opens, so it timed out after 120s even though the wizard had already finished and written a valid netclaw.json (verified via the smoke-logs artifact: Daemon.ExposureMode=reverse-proxy, Host=0.0.0.0, TrustedProxies=[10.0.0.0/24]). Make the post-Health-Check wait tolerant of either terminal state: the chat-page-ready bar OR the shell prompt with 'netclaw init' visible as the last-typed command (which is what the failure log showed). The post-tape assertion (init-wizard-reverse-proxy.sh) is the real signal.
Second-pass review of PR netclaw-dev#1120 surfaced several real bugs and UX gaps: 1. The trusted-proxies gate only checked Count > 0; it accepted garbage like 'abc' or '10.0.0.0/99'. The wizard's "always-startable config" promise was false — the daemon would reject the config at startup. Now each entry is validated through DaemonExposureValidator.TryParseTrustedProxy (the same canonical helper the daemon uses), with the per-entry error rendered inline. 2. The bind-address input accepted loopback (127.0.0.1, ::1, localhost) despite the help text saying it isn't allowed. The operator only found out at daemon startup. Now inline-rejected via DaemonExposureValidator.IsLoopbackHost. 3. Re-submitting the trusted-proxies input with whitespace/empty text silently overwrote the operator's previously captured entries with [], losing their work. Empty submit now preserves existing entries and surfaces a distinct "empty input ignored" message. 4. Empty-submit feedback was invisible — the screen looked identical to a first arrival because the yellow help line was always shown. Now a red inline error indicator appears only after a failed submit attempt. 5. The notice screen printed "Point your reverse proxy at http://0.0.0.0:5199" verbatim. 0.0.0.0 is a bind sentinel, not a routable address; a naive operator pointing nginx at that URL from a different host gets refused connection. When Host is 0.0.0.0/::, the notice now explains "binds all interfaces" and tells the operator to use this host's loopback (same-machine proxy) or LAN/internal IP (remote proxy). Round-2 cleanups bundled in: - DaemonControlPlaneEndpointResolver.DefaultEndpoint now derives its port from DaemonConfig.DefaultPort instead of a duplicate 5199 literal (the constant promotion in bd2af17 left this loose end). Changed from const to static readonly to allow the interpolation; the field has no callers. - Dropped defensive null-out boilerplate at the top of each Build* method in ExposureModeStepView. Sibling step views (SlackStepView, IdentityStepView) don't do this — the per-Build field reassignment plus ClearFocusState() already cover the rebuild lifecycle. - IPv6 bracketing in FormatServingUrl: replaced host.Contains(':') heuristic with IPAddress.TryParse + AddressFamily.InterNetworkV6 check so a typo like 'hostname:port' isn't bracket-wrapped. - Removed two trivial tests (IsReverseProxy_OnlyTrueForReverseProxyMode, IsHighRisk_ReverseProxy_IsFalse) per the constitution's Testing Guidelines. - Corrected a misleading comment in the smoke tape ("No" = second option) that was copied from the baseline init-wizard.tape — the navigation selects "Yes" (second option), as the comment now states. - Skill text dropped a false "auto-resumes incomplete onboarding by skipping already-completed steps" promise. SPEC-007 describes that behavior but it is not implemented; re-running init currently re-walks every step.
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
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.
Closes #1114.
Summary
Reverse proxy has been a fully-supported
ExposureModein the config schema, daemon runtime,ForwardedHeadersmiddleware, validator, and doctor check — butnetclaw initonly offered four of the five modes (Local, Tailscale Serve / Funnel, Cloudflare Tunnel). Operators deploying behind nginx / Caddy / Traefik / IIS / ALB had to hand-editnetclaw.jsonafter init or pick a mode they didn't want.This PR is a UI-only gap fix:
0.0.0.0; loopback rejected at daemon startup (existing validator).DaemonExposureValidator.Validate's startup contract so the wizard cannot emit a non-startable config.http://{Host}:5199and listing operator responsibilities (TLS termination, firewalling, X-Forwarded-* setup).No deferrable "configure later" path — if an operator doesn't yet know their proxy IP, they pick Local and re-run
netclaw init --resume. This was an explicit design decision (the daemon refuses to start without trusted proxies; a wizard that "succeeded" on an empty list would silently break the user).What did NOT change
DaemonExposureValidator,ExposureModeDoctorCheck, the JSON schema, the daemon'sForwardedHeaderswiring — already correct.Daemon.Portdefault. (This PR promotes5199→DaemonConfig.DefaultPortas a follow-up so the literal lives in one place.)Files
src/Netclaw.Cli/Tui/Wizard/Steps/ExposureModeStepView.cs— option + 3 new render methodssrc/Netclaw.Cli/Tui/Wizard/Steps/ExposureModeStepViewModel.cs—Host+TrustedProxiesfields, 5-sub-step plan, advance gate, mode-downgrade high-water clampsrc/Netclaw.Cli/Tui/Wizard/WizardConfigBuilder.cs—DaemonConfigSection.Host?+TrustedProxies; serializer emits them only when non-defaultsrc/Netclaw.Configuration/DaemonConfig.cs— promote port literal topublic const int DefaultPort = 5199src/Netclaw.Cli.Tests/Tui/Wizard/ExposureModeStepViewModelTests.cs— +9 reverse-proxy tests (config emission, sub-step walk, gate, mode-downgrade clamp, bootstrap device, leakage prevention)tests/smoke/tapes/init-wizard-reverse-proxy.tape+tests/smoke/assertions/init-wizard-reverse-proxy.sh— new smoke tape (registered inLIGHT_TAPES); jq-assertsDaemon.ExposureMode/Host/TrustedProxies[0]and runsnetclaw doctorfeeds/skills/.system/files/netclaw-operations/SKILL.md— documents the wizard support;metadata.version2.4.0 → 2.5.0docs/spec/SPEC-007-guided-onboarding.md— reverse proxy added to the wizard exposure-mode list with the trusted-proxies-required ruleTest plan
dotnet test src/Netclaw.Cli.Tests— 713/713 passdotnet slopwatch analyze— 0 issues./scripts/Add-FileHeaders.ps1 -Verify— all headers presentdotnet build src/Netclaw.Daemon— clean (sanity check for theDefaultPortconstant promotion)./scripts/smoke/run-smoke.sh init-wizard-reverse-proxyin CI (the tape mirrors the proveninit-wizard.tapepattern; both fail locally on machines with~/.claude/skillsbecause of an unrelated External Skills step issue that pre-dates this PR)netclaw init, pick Reverse Proxy, leave bind default, enter10.0.0.0/24, confirm notice, disable webhooks → verify~/.netclaw/netclaw.jsonhas the expectedDaemonsection andnetclaw doctoris clean.