diff --git a/cmd/mavweb/handlers_test.go b/cmd/mavweb/handlers_test.go index a739b09..79cfbc2 100644 --- a/cmd/mavweb/handlers_test.go +++ b/cmd/mavweb/handlers_test.go @@ -164,7 +164,7 @@ func TestHandleTools_GET_RendersAndEscapes(t *testing.T) { enabled: []ipc.Tool{{Name: "svc", Scope: "", Cmd: []string{"systemctl", "restart"}, Destructive: true}}, } rr := httptest.NewRecorder() - handleTools(rr, httptest.NewRequest(http.MethodGet, "/tools", nil), core, nil) + handleTools(rr, httptest.NewRequest(http.MethodGet, "/tools", nil), core, nil, false) if rr.Code != http.StatusOK { t.Fatalf("status = %d, want 200", rr.Code) @@ -184,7 +184,7 @@ func TestHandleTools_GET_RendersAndEscapes(t *testing.T) { func TestHandleTools_NilCore_503(t *testing.T) { rr := httptest.NewRecorder() - handleTools(rr, httptest.NewRequest(http.MethodGet, "/tools", nil), nil, nil) + handleTools(rr, httptest.NewRequest(http.MethodGet, "/tools", nil), nil, nil, false) if rr.Code != http.StatusServiceUnavailable { t.Fatalf("status = %d, want 503", rr.Code) } @@ -214,7 +214,7 @@ func TestHandleTools_POST_Enable_HappyPath(t *testing.T) { "name": {"svc"}, "cmd": {"systemctl restart nginx"}, "destructive": {"on"}, - }), core, stepUpSession()) + }), core, stepUpSession(), false) if rr.Code != http.StatusOK { t.Fatalf("status = %d, want 200; body=%s", rr.Code, rr.Body.String()) @@ -233,7 +233,7 @@ func TestHandleTools_POST_Enable_HappyPath(t *testing.T) { func TestHandleTools_POST_Enable_MissingName_400(t *testing.T) { core := &fakeCore{} rr := httptest.NewRecorder() - handleTools(rr, postForm("enable", url.Values{"cmd": {"systemctl restart"}}), core, stepUpSession()) + handleTools(rr, postForm("enable", url.Values{"cmd": {"systemctl restart"}}), core, stepUpSession(), false) if rr.Code != http.StatusBadRequest { t.Fatalf("status = %d, want 400", rr.Code) } @@ -242,7 +242,7 @@ func TestHandleTools_POST_Enable_MissingName_400(t *testing.T) { func TestHandleTools_POST_Enable_MissingCmd_400(t *testing.T) { core := &fakeCore{} rr := httptest.NewRecorder() - handleTools(rr, postForm("enable", url.Values{"name": {"svc"}}), core, stepUpSession()) + handleTools(rr, postForm("enable", url.Values{"name": {"svc"}}), core, stepUpSession(), false) if rr.Code != http.StatusBadRequest { t.Fatalf("status = %d, want 400", rr.Code) } @@ -253,7 +253,7 @@ func TestHandleTools_POST_Enable_CoreError_502(t *testing.T) { rr := httptest.NewRecorder() handleTools(rr, postForm("enable", url.Values{ "name": {"svc"}, "cmd": {"systemctl restart"}, - }), core, stepUpSession()) + }), core, stepUpSession(), false) if rr.Code != http.StatusBadGateway { t.Fatalf("status = %d, want 502", rr.Code) } @@ -262,7 +262,7 @@ func TestHandleTools_POST_Enable_CoreError_502(t *testing.T) { func TestHandleTools_POST_UnknownAction_400(t *testing.T) { core := &fakeCore{} rr := httptest.NewRecorder() - handleTools(rr, postForm("frobnicate", url.Values{"name": {"svc"}}), core, stepUpSession()) + handleTools(rr, postForm("frobnicate", url.Values{"name": {"svc"}}), core, stepUpSession(), false) if rr.Code != http.StatusBadRequest { t.Fatalf("status = %d, want 400", rr.Code) } @@ -273,7 +273,7 @@ func TestHandleTools_POST_UnknownAction_400(t *testing.T) { func TestHandleTools_POST_Disable_HappyPath(t *testing.T) { core := &fakeCore{} rr := httptest.NewRecorder() - handleTools(rr, postForm("disable", url.Values{"name": {"svc"}}), core, stepUpSession()) + handleTools(rr, postForm("disable", url.Values{"name": {"svc"}}), core, stepUpSession(), false) if rr.Code != http.StatusOK { t.Fatalf("status = %d, want 200; body=%s", rr.Code, rr.Body.String()) } @@ -285,7 +285,7 @@ func TestHandleTools_POST_Disable_HappyPath(t *testing.T) { func TestHandleTools_POST_Disable_MissingName_400(t *testing.T) { core := &fakeCore{} rr := httptest.NewRecorder() - handleTools(rr, postForm("disable", url.Values{}), core, stepUpSession()) + handleTools(rr, postForm("disable", url.Values{}), core, stepUpSession(), false) if rr.Code != http.StatusBadRequest { t.Fatalf("status = %d, want 400", rr.Code) } @@ -294,7 +294,7 @@ func TestHandleTools_POST_Disable_MissingName_400(t *testing.T) { func TestHandleTools_POST_Disable_CoreError_502(t *testing.T) { core := &fakeCore{disableErr: ipc.ErrToolNotFound} rr := httptest.NewRecorder() - handleTools(rr, postForm("disable", url.Values{"name": {"svc"}}), core, stepUpSession()) + handleTools(rr, postForm("disable", url.Values{"name": {"svc"}}), core, stepUpSession(), false) if rr.Code != http.StatusBadGateway { t.Fatalf("status = %d, want 502", rr.Code) } @@ -309,7 +309,7 @@ func TestEnableTool_NoInProcessAuthGate(t *testing.T) { // Session exists (WebAuthn wired) but was never asserted — gate rejects. handleTools(rr, postForm("enable", url.Values{ "name": {"svc"}, "cmd": {"systemctl restart"}, - }), core, webauthn.NewPasskeySession(5*time.Minute)) + }), core, webauthn.NewPasskeySession(5*time.Minute), false) if rr.Code != http.StatusForbidden { t.Fatalf("status = %d, want %d (expected auth gate to reject)", rr.Code, http.StatusForbidden) } @@ -326,7 +326,7 @@ func TestEnableTool_NoWebAuthnConfigured(t *testing.T) { rr := httptest.NewRecorder() handleTools(rr, postForm("enable", url.Values{ "name": {"svc"}, "cmd": {"systemctl restart"}, - }), core, nil) + }), core, nil, false) if rr.Code != http.StatusOK { t.Fatalf("status = %d, want 200; body=%s", rr.Code, rr.Body.String()) } @@ -335,6 +335,55 @@ func TestEnableTool_NoWebAuthnConfigured(t *testing.T) { } } +// TestEnableTool_RequireStepUp_FailsClosed verifies that with -require-stepup +// set (requireStepUp=true) and WebAuthn unconfigured (nil session — step-up can +// never be asserted), POST /tools is DENIED rather than falling open. +func TestEnableTool_RequireStepUp_FailsClosed(t *testing.T) { + core := &fakeCore{} + rr := httptest.NewRecorder() + handleTools(rr, postForm("enable", url.Values{ + "name": {"svc"}, "cmd": {"systemctl restart"}, + }), core, nil, true) + if rr.Code != http.StatusForbidden { + t.Fatalf("status = %d, want 403; body=%s", rr.Code, rr.Body.String()) + } + if core.gotEnableName != "" { + t.Errorf("core.EnableTool called with name=%q, but -require-stepup should deny", core.gotEnableName) + } +} + +// TestHandleRevert_RequireStepUp_FailsClosed is the /api/revert half of the +// same gate: nil session + -require-stepup ⇒ 403, no store mutation. +func TestHandleRevert_RequireStepUp_FailsClosed(t *testing.T) { + core := &fakeCore{revertNewID: 7} + rr := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/api/revert", strings.NewReader("key=k")) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + handleRevert(rr, req, core, nil, true) + if rr.Code != http.StatusForbidden { + t.Fatalf("status = %d, want 403; body=%s", rr.Code, rr.Body.String()) + } + if core.revertKey != "" { + t.Errorf("core.RevertFact called with key=%q, but -require-stepup should deny", core.revertKey) + } +} + +// TestStepUpOK covers the shared decision point directly. +func TestStepUpOK(t *testing.T) { + if !stepUpOK(nil, false) { + t.Errorf("nil session, require=false: want allow (historical fail-open)") + } + if stepUpOK(nil, true) { + t.Errorf("nil session, require=true: want deny") + } + if stepUpOK(webauthn.NewPasskeySession(5*time.Minute), false) { + t.Errorf("unasserted session: want deny") + } + if !stepUpOK(stepUpSession(), true) { + t.Errorf("asserted session, require=true: want allow") + } +} + // TestEnableTool_WithAuthGate_RequiresStepUp verifies that a POST /tools with // an asserted passkey session proceeds past the auth gate to core.EnableTool. func TestEnableTool_WithAuthGate_RequiresStepUp(t *testing.T) { @@ -343,7 +392,7 @@ func TestEnableTool_WithAuthGate_RequiresStepUp(t *testing.T) { sess := stepUpSession() handleTools(rr, postForm("enable", url.Values{ "name": {"svc"}, "cmd": {"systemctl restart"}, - }), core, sess) + }), core, sess, false) if rr.Code != http.StatusOK { t.Fatalf("status = %d, want 200; body=%s", rr.Code, rr.Body.String()) } @@ -761,7 +810,7 @@ func TestHandleRevert(t *testing.T) { t.Run("GET returns 405", func(t *testing.T) { rr := httptest.NewRecorder() - handleRevert(rr, httptest.NewRequest(http.MethodGet, "/api/revert", nil), &fakeCore{}, nil) + handleRevert(rr, httptest.NewRequest(http.MethodGet, "/api/revert", nil), &fakeCore{}, nil, false) if rr.Code != http.StatusMethodNotAllowed { t.Errorf("status = %d, want 405", rr.Code) } @@ -769,7 +818,7 @@ func TestHandleRevert(t *testing.T) { t.Run("nil core returns 503", func(t *testing.T) { rr := httptest.NewRecorder() - handleRevert(rr, httptest.NewRequest(http.MethodPost, "/api/revert", nil), nil, nil) + handleRevert(rr, httptest.NewRequest(http.MethodPost, "/api/revert", nil), nil, nil, false) if rr.Code != http.StatusServiceUnavailable { t.Errorf("status = %d, want 503", rr.Code) } @@ -780,7 +829,7 @@ func TestHandleRevert(t *testing.T) { rr := httptest.NewRecorder() req := httptest.NewRequest(http.MethodPost, "/api/revert", strings.NewReader("key=")) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - handleRevert(rr, req, core, nil) + handleRevert(rr, req, core, nil, false) if rr.Code != http.StatusBadRequest { t.Errorf("status = %d, want 400", rr.Code) } @@ -791,7 +840,7 @@ func TestHandleRevert(t *testing.T) { rr := httptest.NewRecorder() req := httptest.NewRequest(http.MethodPost, "/api/revert", strings.NewReader("key=missing")) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - handleRevert(rr, req, core, nil) + handleRevert(rr, req, core, nil, false) if rr.Code != http.StatusNotFound { t.Errorf("status = %d, want 404", rr.Code) } @@ -802,7 +851,7 @@ func TestHandleRevert(t *testing.T) { rr := httptest.NewRecorder() req := httptest.NewRequest(http.MethodPost, "/api/revert", strings.NewReader("key=somekey")) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - handleRevert(rr, req, core, nil) + handleRevert(rr, req, core, nil, false) if rr.Code != http.StatusBadGateway { t.Errorf("status = %d, want 502", rr.Code) } @@ -813,7 +862,7 @@ func TestHandleRevert(t *testing.T) { rr := httptest.NewRecorder() req := httptest.NewRequest(http.MethodPost, "/api/revert", strings.NewReader("key=test-key")) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - handleRevert(rr, req, core, nil) + handleRevert(rr, req, core, nil, false) if rr.Code != http.StatusOK { t.Fatalf("status = %d, want 200", rr.Code) } @@ -835,7 +884,7 @@ func TestHandleRevert(t *testing.T) { req := httptest.NewRequest(http.MethodPost, "/api/revert", strings.NewReader("key=k")) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") // non-nil, never asserted ⇒ IsStepUp() false ⇒ gate closes. - handleRevert(rr, req, core, webauthn.NewPasskeySession(5*time.Minute)) + handleRevert(rr, req, core, webauthn.NewPasskeySession(5*time.Minute), false) if rr.Code != http.StatusForbidden { t.Errorf("status = %d, want 403", rr.Code) } @@ -849,7 +898,7 @@ func TestHandleRevert(t *testing.T) { rr := httptest.NewRecorder() req := httptest.NewRequest(http.MethodPost, "/api/revert", strings.NewReader("key=k")) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - handleRevert(rr, req, core, stepUpSession()) + handleRevert(rr, req, core, stepUpSession(), false) if rr.Code != http.StatusOK { t.Fatalf("status = %d, want 200", rr.Code) } @@ -864,7 +913,7 @@ func TestHandleTools_ListToolsError_502(t *testing.T) { rr := httptest.NewRecorder() handleTools(rr, postForm("enable", url.Values{ "name": {"svc"}, "cmd": {"systemctl restart"}, - }), core, stepUpSession()) + }), core, stepUpSession(), false) if rr.Code != http.StatusBadGateway { t.Fatalf("status = %d, want 502; body=%s", rr.Code, rr.Body.String()) } diff --git a/cmd/mavweb/main.go b/cmd/mavweb/main.go index a833d3b..8229e8d 100644 --- a/cmd/mavweb/main.go +++ b/cmd/mavweb/main.go @@ -314,6 +314,7 @@ func main() { coreSock := flag.String("core", "", "mavend IPC socket path for presence-signal ingest (empty = disabled)") pkOrigin := flag.String("webauthn-origin", "", "WebAuthn origin URL (e.g. https://maven.kvmx.ru)") pkRPID := flag.String("webauthn-rpid", "", "WebAuthn RP ID (e.g. maven.kvmx.ru)") + requireStepUp := flag.Bool("require-stepup", false, "fail closed on step-up-gated actions (/tools POST, /api/revert) when WebAuthn step-up cannot be asserted; default false preserves the historical fail-open behaviour") pkFile := flag.String("passkey-file", "./passkeys.json", "path to WebAuthn credential store (JSON)") nexusURL := flag.String("nexus", "", "Nexus base URL for the /ecosystem panel (empty = not configured)") praxisURL := flag.String("praxis", "", "Praxis base URL for the /ecosystem panel (empty = not configured)") @@ -416,13 +417,21 @@ func main() { mux.HandleFunc("/auth/webauthn/assert/begin", pk.AssertBegin) mux.HandleFunc("/auth/webauthn/assert/finish", pk.AssertFinish) } + if stepUpSession == nil { + if *requireStepUp { + log.Printf("SECURITY: step-up verification is DISABLED (-webauthn-origin/-webauthn-rpid unset) and -require-stepup is set: POST /tools (tool enable/disable/dismiss — defines and executes arbitrary argv) and POST /api/revert will be DENIED (403). Set -webauthn-origin and -webauthn-rpid to enable passkey step-up.") + } else { + log.Printf("SECURITY WARNING: step-up verification is DISABLED because -webauthn-origin/-webauthn-rpid are unset. UNGUARDED SURFACES: POST /tools (defines arbitrary argv via name+cmd, which internal/tool then EXECUTES) and POST /api/revert (voids the latest fact for a key). These are protected only by whatever transport-level auth sits in front of mavweb (wg+nginx+auth) — do NOT expose -addr on a public interface. Set -webauthn-origin and -webauthn-rpid to require passkey step-up, or pass -require-stepup to fail closed instead.") + } + } // /tools — the authed enable surface. maven proposes acts she can't run; // this page is where a human reviews and enables them (proposed→enabled). - // Enabling is the boundary-moving act (maven.md), so it lives ONLY here, + // Enabling is the boundary-moving act (DESIGN.md § Tool registration — + // drafting is suggest, enabling is act), so it lives ONLY here, // behind wg+nginx+auth — never the voice/chat path. mux.HandleFunc("/tools", func(w http.ResponseWriter, r *http.Request) { - handleTools(w, r, core, stepUpSession) + handleTools(w, r, core, stepUpSession, *requireStepUp) }) // /api/revert voids the latest fact for a key — a store mutation, so it @@ -435,7 +444,7 @@ func main() { handleChatAPI(w, r, core) }) mux.HandleFunc("/api/revert", func(w http.ResponseWriter, r *http.Request) { - handleRevert(w, r, core, stepUpSession) + handleRevert(w, r, core, stepUpSession, *requireStepUp) }) srv := &http.Server{Addr: *addr, Handler: mux} @@ -827,7 +836,24 @@ func handleVoice(w http.ResponseWriter, r *http.Request) { } } -func handleRevert(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI, session *webauthn.PasskeySession) { +// stepUpOK is the single decision point for the AuthStepUp gate shared by +// POST /tools and POST /api/revert. +// +// A nil session means WebAuthn is not configured (-webauthn-origin / +// -webauthn-rpid unset), so step-up can never be asserted — not merely unmet. +// The default is therefore fail-OPEN: gating on an unassertable session would +// 403 those surfaces permanently. In that mode the actions rest on the +// transport-level auth in front of mavweb (wg+nginx+auth), and main logs a +// startup warning naming them. With -require-stepup the same situation fails +// CLOSED instead: no assertable step-up ⇒ deny. +func stepUpOK(session *webauthn.PasskeySession, requireStepUp bool) bool { + if session == nil { + return !requireStepUp + } + return session.IsStepUp() +} + +func handleRevert(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI, session *webauthn.PasskeySession, requireStepUp bool) { if r.Method != http.MethodPost { http.Error(w, "POST only", http.StatusMethodNotAllowed) return @@ -836,12 +862,11 @@ func handleRevert(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI, sess http.Error(w, "revert disabled (no -core)", http.StatusServiceUnavailable) return } - // nil session ⇒ WebAuthn not configured; step-up gate not applicable - // (asserting would be impossible, not just unmet) — matches handleTools. - if session != nil && !session.IsStepUp() { + if !stepUpOK(session, requireStepUp) { http.Error(w, "step-up required: assert a passkey first", http.StatusForbidden) return } + key := strings.TrimSpace(r.FormValue("key")) if key == "" { http.Error(w, "key required", http.StatusBadRequest) @@ -868,7 +893,7 @@ func handleRevert(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI, sess // whitespace-split — argv with embedded spaces isn't supported (ponytail: no // shell-word parsing; the box owner controls this input, quote a wrapper script // if an arg needs spaces). -func handleTools(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI, session *webauthn.PasskeySession) { +func handleTools(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI, session *webauthn.PasskeySession, requireStepUp bool) { if core == nil { http.Error(w, "tools disabled (no -core)", http.StatusServiceUnavailable) return diff --git a/docker-compose.yml b/docker-compose.yml index b6a4b71..0652d17 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -82,7 +82,12 @@ services: command: ["mavweb", "-addr", ":9201", "-voice", "mavend:9100", "-core", "/run/maven/mavend.sock", "-nexus", "http://nexus:9740", "-praxis", "http://praxis:8989", "-hexis", "http://hexis:9741"] depends_on: [mavend] - ports: ["9201:9201"] + # loopback-only on purpose: /tools defines+executes arbitrary argv and + # mavweb has no in-process auth of its own without -webauthn-origin/-rpid. + # Reaching the UI therefore requires the wg tunnel (or the local nginx) by + # construction, not by convention. The other daemons talk to mavweb over + # the compose network, not this published port. + ports: ["127.0.0.1:9201:9201"] # ecosystem: reach the siblings by name for the read-only /ecosystem panel; # default: keep resolving mavend:9100 for voice + the IPC socket peers. networks: [default, ecosystem]