kami/Maven
1
0
Fork 0
Code Issues 1 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Bind mavweb to loopback and make the step-up fail-open loud and overridable

Browse Source 
The /tools enable action takes name+cmd from form fields and calls
core.EnableTool, so it defines arbitrary argv that internal/tool then executes.
Its step-up gate read `session != nil && !session.IsStepUp()`, and
stepUpSession is nil unless both -webauthn-origin and -webauthn-rpid are set —
so with neither flag the gate was skipped entirely. compose passed neither and
published 9201 on every host interface, while /ptt proxies to the voice server
unauthenticated, so a caller could enable a tool, trigger it, and answer its
own confirm turn. internal/tool's boundary reasoning ("a compromised router
can't grant itself a capability") held; the outer boundary it depends on was an
unwritten deployment assumption.

The fail-open itself stays: gating on a session that can never be asserted
would 403 permanently, and that reasoning is sound. What was missing is the
compensating control.

- compose publishes 127.0.0.1:9201 so reaching the UI requires the wg tunnel by
  construction rather than by convention. Verified no other service reaches
  mavweb by host-published port; mavpoll is host-networked but only dials
  netdata and kuma.
- stepUpOK() replaces the two inline gates in handleTools and handleRevert, so
  one decision point covers both surfaces.
- -require-stepup (default false, behaviour byte-for-byte unchanged) fails those
  actions closed when step-up cannot be asserted.
- A startup warning names both unguarded surfaces when stepUpSession is nil,
  in fail-open and fail-closed variants.

Also repoints one doc comment at DESIGN.md, since it shared a hunk with the
warning block.

The committed kuma key is deliberately left for a separate change: the old
value is in git history forever, so rotation means a genuinely new key, not a
re-commit under a variable.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X5JApcrCRVGmqrxnhynSik
This commit is contained in:
kami
2026-07-30 23:39:07 +04:00
parent fb8b470f78
commit 691c3660d2
3 changed files with 110 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cmd/mavweb/handlers_test.go
+71 -22
View File
@@ -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())
}