diff --git a/cmd/mavweb/handlers_test.go b/cmd/mavweb/handlers_test.go new file mode 100644 index 0000000..1f66bd6 --- /dev/null +++ b/cmd/mavweb/handlers_test.go @@ -0,0 +1,282 @@ +package main + +import ( + "context" + "net/http" + "net/http/httptest" + "net/url" + "reflect" + "strings" + "testing" + "time" + + "github.com/kami/maven/internal/ipc" + "github.com/kami/maven/internal/webauthn" +) + +// fakeCore records the mutating calls handleTools makes and returns canned +// tool lists / errors. Embedding ipc.CoreAPI (nil) satisfies the large +// interface — only the methods the handlers touch are overridden; any other +// call would nil-panic, which is fine since the handlers never make them. +type fakeCore struct { + ipc.CoreAPI + + proposed, enabled []ipc.Tool + listErr error + + enableErr error + disableErr error + + // recorded args from the last Enable/Disable call + gotEnableName string + gotEnableCmd []string + gotEnableDest bool + gotDisable string +} + +func (f *fakeCore) EnableTool(_ context.Context, name string, cmd []string, destructive bool, _ time.Time) error { + f.gotEnableName, f.gotEnableCmd, f.gotEnableDest = name, cmd, destructive + return f.enableErr +} + +func (f *fakeCore) DisableTool(_ context.Context, name string) error { + f.gotDisable = name + return f.disableErr +} + +func (f *fakeCore) ListTools(_ context.Context, status string) ([]ipc.Tool, error) { + if f.listErr != nil { + return nil, f.listErr + } + switch status { + case "proposed": + return f.proposed, nil + default: + return f.enabled, nil + } +} + +// --- GET --- + +func TestHandleTools_GET_RendersAndEscapes(t *testing.T) { + core := &fakeCore{ + proposed: []ipc.Tool{{Name: "x", Utterance: "restart the thing"}}, + enabled: []ipc.Tool{{Name: "svc", Cmd: []string{"systemctl", "restart"}, Destructive: true}}, + } + rr := httptest.NewRecorder() + handleTools(rr, httptest.NewRequest(http.MethodGet, "/tools", nil), core) + + if rr.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", rr.Code) + } + body := rr.Body.String() + // html/template must escape the untrusted (STT-sourced) tool name. + if strings.Contains(body, "x") { + t.Errorf("tool name rendered unescaped in output") + } + if !strings.Contains(body, "<b>x") { + t.Errorf("expected escaped tool name <b>x in output") + } + if !strings.Contains(body, "svc") || !strings.Contains(body, "systemctl restart") { + t.Errorf("enabled tool not rendered: %s", body) + } +} + +func TestHandleTools_NilCore_503(t *testing.T) { + rr := httptest.NewRecorder() + handleTools(rr, httptest.NewRequest(http.MethodGet, "/tools", nil), nil) + if rr.Code != http.StatusServiceUnavailable { + t.Fatalf("status = %d, want 503", rr.Code) + } +} + +// --- POST enable --- + +func postForm(action string, vals url.Values) *http.Request { + vals.Set("action", action) + r := httptest.NewRequest(http.MethodPost, "/tools", strings.NewReader(vals.Encode())) + r.Header.Set("Content-Type", "application/x-www-form-urlencoded") + return r +} + +func TestHandleTools_POST_Enable_HappyPath(t *testing.T) { + core := &fakeCore{} + rr := httptest.NewRecorder() + handleTools(rr, postForm("enable", url.Values{ + "name": {"svc"}, + "cmd": {"systemctl restart nginx"}, + "destructive": {"on"}, + }), core) + + if rr.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", rr.Code, rr.Body.String()) + } + if core.gotEnableName != "svc" { + t.Errorf("name = %q, want svc", core.gotEnableName) + } + if want := []string{"systemctl", "restart", "nginx"}; !reflect.DeepEqual(core.gotEnableCmd, want) { + t.Errorf("cmd = %v, want %v", core.gotEnableCmd, want) + } + if !core.gotEnableDest { + t.Errorf("destructive = false, want true") + } +} + +func TestHandleTools_POST_Enable_MissingName_400(t *testing.T) { + core := &fakeCore{} + rr := httptest.NewRecorder() + handleTools(rr, postForm("enable", url.Values{"cmd": {"systemctl restart"}}), core) + if rr.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400", rr.Code) + } +} + +func TestHandleTools_POST_Enable_MissingCmd_400(t *testing.T) { + core := &fakeCore{} + rr := httptest.NewRecorder() + handleTools(rr, postForm("enable", url.Values{"name": {"svc"}}), core) + if rr.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400", rr.Code) + } +} + +func TestHandleTools_POST_Enable_CoreError_502(t *testing.T) { + core := &fakeCore{enableErr: ipc.ErrForbidden} + rr := httptest.NewRecorder() + handleTools(rr, postForm("enable", url.Values{ + "name": {"svc"}, "cmd": {"systemctl restart"}, + }), core) + if rr.Code != http.StatusBadGateway { + t.Fatalf("status = %d, want 502", rr.Code) + } +} + +func TestHandleTools_POST_UnknownAction_400(t *testing.T) { + core := &fakeCore{} + rr := httptest.NewRecorder() + handleTools(rr, postForm("frobnicate", url.Values{"name": {"svc"}}), core) + if rr.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400", rr.Code) + } +} + +// --- POST disable --- + +func TestHandleTools_POST_Disable_HappyPath(t *testing.T) { + core := &fakeCore{} + rr := httptest.NewRecorder() + handleTools(rr, postForm("disable", url.Values{"name": {"svc"}}), core) + if rr.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", rr.Code, rr.Body.String()) + } + if core.gotDisable != "svc" { + t.Errorf("disabled name = %q, want svc", core.gotDisable) + } +} + +func TestHandleTools_POST_Disable_MissingName_400(t *testing.T) { + core := &fakeCore{} + rr := httptest.NewRecorder() + handleTools(rr, postForm("disable", url.Values{}), core) + if rr.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400", rr.Code) + } +} + +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) + if rr.Code != http.StatusBadGateway { + t.Fatalf("status = %d, want 502", rr.Code) + } +} + +// TestEnableTool_NoInProcessAuthGate documents CURRENT behavior: handleTools +// performs the boundary-moving EnableTool with no in-process authentication — +// there is no passkey/session check in the Go handler. A POST enable with no +// prior WebAuthn AssertFinish succeeds (reaches core.EnableTool). Authorization +// is delegated entirely to the nginx+wg layer in front of mavweb. Whether that +// is the intended sole gate is a maintainer decision; this test only pins the +// observed behavior so a future auth gate change is a deliberate, visible edit. +func TestEnableTool_NoInProcessAuthGate(t *testing.T) { + core := &fakeCore{} + rr := httptest.NewRecorder() + // No passkey session, no cookie, no header — just the raw POST. + handleTools(rr, postForm("enable", url.Values{ + "name": {"svc"}, "cmd": {"systemctl restart"}, + }), core) + if rr.Code != http.StatusOK || core.gotEnableName != "svc" { + t.Fatalf("expected unauthenticated enable to reach core (status=%d, name=%q); "+ + "if this now fails, an in-process auth gate was added", rr.Code, core.gotEnableName) + } +} + +// --- webauthn handler wiring (contract level, not crypto) --- + +func newTestPasskey() *PasskeyHandle { + return newPasskeyHandle(webauthn.Config{ + Origin: "https://maven.example", + RPID: "maven.example", + RPName: "maven", + }, nil) // nil core ⇒ assertFn nil; AssertFinish skips the IPC step-up +} + +func TestWebAuthn_Finish_MethodGuards(t *testing.T) { + pk := newTestPasskey() + for _, tc := range []struct { + name string + h http.HandlerFunc + }{ + {"register", pk.RegisterFinish}, + {"assert", pk.AssertFinish}, + } { + rr := httptest.NewRecorder() + tc.h(rr, httptest.NewRequest(http.MethodGet, "/x", nil)) + if rr.Code != http.StatusMethodNotAllowed { + t.Errorf("%s finish GET = %d, want 405", tc.name, rr.Code) + } + } +} + +func TestWebAuthn_Finish_MalformedJSON_400(t *testing.T) { + pk := newTestPasskey() + for _, tc := range []struct { + name string + h http.HandlerFunc + }{ + {"register", pk.RegisterFinish}, + {"assert", pk.AssertFinish}, + } { + rr := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/x", strings.NewReader("{not json")) + tc.h(rr, req) + if rr.Code != http.StatusBadRequest { + t.Errorf("%s finish malformed = %d, want 400", tc.name, rr.Code) + } + } +} + +func TestWebAuthn_Begin_ReturnsChallenge(t *testing.T) { + pk := newTestPasskey() + for _, tc := range []struct { + name string + h http.HandlerFunc + }{ + {"register", pk.RegisterBegin}, + {"assert", pk.AssertBegin}, + } { + rr := httptest.NewRecorder() + tc.h(rr, httptest.NewRequest(http.MethodGet, "/x", nil)) + if rr.Code != http.StatusOK { + t.Errorf("%s begin = %d, want 200", tc.name, rr.Code) + continue + } + if ct := rr.Header().Get("Content-Type"); ct != "application/json" { + t.Errorf("%s begin content-type = %q, want application/json", tc.name, ct) + } + if !strings.Contains(rr.Body.String(), `"challenge"`) { + t.Errorf("%s begin body missing challenge: %s", tc.name, rr.Body.String()) + } + } +} diff --git a/internal/auth/policy.go b/internal/auth/policy.go index 6a48537..bafe402 100644 --- a/internal/auth/policy.go +++ b/internal/auth/policy.go @@ -45,10 +45,13 @@ const ( // which the vet check in dispatch catches via Method existence, not auth). func Requirement(m ipc.Method) Authority { switch m { - case ipc.MethodEnableTool: - // Registration-enable is privilege escalation: it moves the boundary - // (adds a runnable capability). Human-only, step-up asserted — never a - // module or the voice/chat path. maven can propose but never enable. + case ipc.MethodEnableTool, ipc.MethodDisableTool: + // Both mutate the tool allowlist — the boundary. Enable adds a runnable + // capability (privilege escalation); disable removes one (fail-safe + // direction, but still an allowlist mutation and a lever an attacker + // could pull to silence a security-relevant tool). Human-only, step-up + // asserted — never a module or the voice/chat path. maven can propose + // (MethodProposeTool, no step-up: she has no passkey) but never en/disable. return AuthStepUp case ipc.MethodWriteFact: return AuthWrite diff --git a/internal/ipc/api.go b/internal/ipc/api.go index d986de0..5ccf484 100644 --- a/internal/ipc/api.go +++ b/internal/ipc/api.go @@ -221,7 +221,9 @@ type CoreAPI interface { // returns whether a new proposal was written. EnableTool fills cmd + // destructive and flips status to 'enabled'. DisableTool reverts an // enabled tool back to proposed (it stays in the store, won't run). - // All three gate at AuthStepUp. LookupTool/ListTools read them. + // Enable/DisableTool gate at AuthStepUp (allowlist mutation, human-only); + // ProposeTool is maven-callable (no step-up — she has no passkey). + // LookupTool/ListTools read them. ProposeTool(ctx context.Context, name, utterance string, ts time.Time) (bool, error) EnableTool(ctx context.Context, name string, cmd []string, destructive bool, ts time.Time) error DisableTool(ctx context.Context, name string) error