Require auth on /api/v1/ and derive capability guards server-side

Findings 1 and 2 of REVIEW-2026-07-30.md, which must land together: every workspace capability registered with enabled=false, so the only working provider could never execute. Fixing that alone would have turned a dead execution path into a reachable one on an unauthenticated port. Auth: a shared bearer token (HEXIS_API_TOKEN) is now required on the whole /api/v1/ surface, compared with crypto/subtle.ConstantTimeCompare. /health and /ready stay open for probes. It fails closed twice over — hexisd refuses to start with an empty token, and the middleware returns 503 rather than ever serving unauthenticated. Guards: `enabled` and `requires_confirmation` are no longer readable from the request body at all. Previously the handler derived the correct §4.3 default and then let the caller override it, which is worse than no guard because it reads as enforced. Both are now derived from the risk tier by shared helpers in domain, used by the HTTP and provider registration paths alike; unrecognised tiers fail closed to requiring confirmation. BuildCapabilities sets Enabled, RequiresConfirmation and TimeoutSeconds explicitly, and hexisd reconciles drifted rows on startup instead of skipping any capability whose ID already exists — without that, allowlist edits never reach an existing database. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uea55zaiWuEByEDC4UBSdd
2026-07-30 23:39:13 +04:00
parent 945e4ba1ac
commit c7325a20d4
7 changed files with 368 additions and 90 deletions
@@ -4,7 +4,9 @@ import (
 	"encoding/json"
 	"net/http"
 	"net/http/httptest"
+	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"

@@ -23,9 +25,11 @@ func newTestHandler(t *testing.T) (*Handler, *storage.Store) {
 	}
 	t.Cleanup(func() { store.Close() })
 	engine := execution.New(store, provider.NewRegistry())
-	return NewHandler(store, engine), store
+	return NewHandler(store, engine, testToken), store
 }

+const testToken = "test-token"
+
 // TestHandleChanges_SinceFiltersEvents verifies the `since` query param
 // actually filters to events after that sequence number, rather than being
 // silently reset to 0 (the bug: `since = 0` unconditionally, ignoring the
@@ -78,3 +82,211 @@ func itoa(n int64) string {
 	b, _ := json.Marshal(n)
 	return string(b)
 }
+
+// TestAPIV1_RequiresBearerToken pins finding 2: /api/v1/ must not be reachable
+// without the shared token, while /health stays open for probes.
+func TestAPIV1_RequiresBearerToken(t *testing.T) {
+	h, _ := newTestHandler(t)
+	mux := http.NewServeMux()
+	h.Register(mux)
+
+	body := `{"capability_id":"cap_x","target_entity_id":"ent_x"}`
+
+	cases := []struct {
+		name   string
+		header string
+		want   int
+	}{
+		{"no header", "", http.StatusUnauthorized},
+		{"wrong token", "Bearer nope", http.StatusUnauthorized},
+		{"not bearer", "Basic " + testToken, http.StatusUnauthorized},
+		{"correct token", "Bearer " + testToken, http.StatusNotFound}, // past auth: capability missing
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			req := httptest.NewRequest(http.MethodPost, "/api/v1/execute", strings.NewReader(body))
+			if tc.header != "" {
+				req.Header.Set("Authorization", tc.header)
+			}
+			w := httptest.NewRecorder()
+			mux.ServeHTTP(w, req)
+			if w.Code != tc.want {
+				t.Fatalf("expected %d, got %d: %s", tc.want, w.Code, w.Body.String())
+			}
+		})
+	}
+
+	// /health must remain unauthenticated.
+	w := httptest.NewRecorder()
+	mux.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/health", nil))
+	if w.Code != http.StatusOK {
+		t.Fatalf("expected /health to stay open, got %d", w.Code)
+	}
+}
+
+// TestAPIV1_UnsetTokenFailsClosed: an empty configured token must reject the
+// API outright, never serve it openly.
+func TestAPIV1_UnsetTokenFailsClosed(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "hexis.db")
+	store, err := storage.Open(path)
+	if err != nil {
+		t.Fatalf("open store: %v", err)
+	}
+	t.Cleanup(func() { store.Close() })
+	h := NewHandler(store, execution.New(store, provider.NewRegistry()), "")
+	mux := http.NewServeMux()
+	h.Register(mux)
+
+	req := httptest.NewRequest(http.MethodGet, "/api/v1/capabilities", nil)
+	req.Header.Set("Authorization", "Bearer anything")
+	w := httptest.NewRecorder()
+	mux.ServeHTTP(w, req)
+	if w.Code != http.StatusServiceUnavailable {
+		t.Fatalf("expected 503 when no token is configured, got %d: %s", w.Code, w.Body.String())
+	}
+}
+
+// TestCreateCapability_GuardsAreServerDerived pins finding 2: a caller cannot
+// enable a destructive capability or opt out of confirmation via the body.
+func TestCreateCapability_GuardsAreServerDerived(t *testing.T) {
+	h, _ := newTestHandler(t)
+
+	cases := []struct {
+		name                     string
+		body                     string
+		wantEnabled              bool
+		wantRequiresConfirmation bool
+	}{
+		{
+			name:        "destructive cannot be enabled from the body",
+			body:        `{"name":"prune","provider":"workspace_mcp","operation":"docker.prune","risk":"destructive","enabled":true}`,
+			wantEnabled: false, wantRequiresConfirmation: true,
+		},
+		{
+			name:        "medium risk gets confirmation even when the body says otherwise",
+			body:        `{"name":"stop","provider":"workspace_mcp","operation":"docker.stop_container","risk":"medium","requires_confirmation":false}`,
+			wantEnabled: true, wantRequiresConfirmation: true,
+		},
+		{
+			name:        "read risk stays confirmation-free",
+			body:        `{"name":"list","provider":"workspace_mcp","operation":"docker.list_containers","risk":"read"}`,
+			wantEnabled: true, wantRequiresConfirmation: false,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			req := httptest.NewRequest(http.MethodPost, "/api/v1/capabilities", strings.NewReader(tc.body))
+			w := httptest.NewRecorder()
+			h.createCapability(w, req)
+			if w.Code != http.StatusCreated {
+				t.Fatalf("expected 201, got %d: %s", w.Code, w.Body.String())
+			}
+			var cap domain.Capability
+			if err := json.Unmarshal(w.Body.Bytes(), &cap); err != nil {
+				t.Fatalf("decode: %v", err)
+			}
+			if cap.Enabled != tc.wantEnabled {
+				t.Errorf("enabled = %v, want %v", cap.Enabled, tc.wantEnabled)
+			}
+			if cap.RequiresConfirmation != tc.wantRequiresConfirmation {
+				t.Errorf("requires_confirmation = %v, want %v", cap.RequiresConfirmation, tc.wantRequiresConfirmation)
+			}
+			if cap.TimeoutSeconds != domain.DefaultCapabilityTimeoutSeconds {
+				t.Errorf("timeout_seconds = %d, want %d", cap.TimeoutSeconds, domain.DefaultCapabilityTimeoutSeconds)
+			}
+		})
+	}
+}
+
+// TestExecute_WorkspaceCapabilityEndToEnd pins finding 1: a capability
+// registered by BuildCapabilities must actually execute, not 403 as disabled.
+func TestExecute_WorkspaceCapabilityEndToEnd(t *testing.T) {
+	ws := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/api/tool/docker.list_containers" {
+			http.NotFound(w, r)
+			return
+		}
+		w.Header().Set("Content-Type", "application/json")
+		w.Write([]byte(`{"containers":["hexis"]}`))
+	}))
+	t.Cleanup(ws.Close)
+
+	allowlistPath := filepath.Join(t.TempDir(), "allowlist.yaml")
+	allowlistYAML := "tools:\n" +
+		"  docker.list_containers:\n" +
+		"    capability: workspace.docker.list\n" +
+		"    risk: read\n" +
+		"    read_only: true\n"
+	if err := os.WriteFile(allowlistPath, []byte(allowlistYAML), 0o600); err != nil {
+		t.Fatalf("write allowlist: %v", err)
+	}
+	allowlist, err := provider.LoadToolAllowlist(allowlistPath)
+	if err != nil {
+		t.Fatalf("load allowlist: %v", err)
+	}
+
+	store, err := storage.Open(filepath.Join(t.TempDir(), "hexis.db"))
+	if err != nil {
+		t.Fatalf("open store: %v", err)
+	}
+	t.Cleanup(func() { store.Close() })
+
+	wsProvider := provider.NewWorkspaceMCPProvider(ws.URL, allowlist)
+	reg := provider.NewRegistry()
+	reg.Register(wsProvider)
+
+	caps := wsProvider.BuildCapabilities()
+	if len(caps) != 1 {
+		t.Fatalf("expected 1 built capability, got %d", len(caps))
+	}
+	built := caps[0]
+	if !built.Enabled {
+		t.Fatalf("BuildCapabilities produced a disabled capability: %+v", built)
+	}
+	if built.TimeoutSeconds <= 0 {
+		t.Fatalf("BuildCapabilities produced timeout_seconds=%d", built.TimeoutSeconds)
+	}
+	if err := store.CreateCapability(&built); err != nil {
+		t.Fatalf("create capability: %v", err)
+	}
+
+	h := NewHandler(store, execution.New(store, reg), testToken)
+	mux := http.NewServeMux()
+	h.Register(mux)
+
+	body := `{"capability_id":"` + built.ID + `","target_entity_id":"ent_host_homesrv"}`
+	req := httptest.NewRequest(http.MethodPost, "/api/v1/execute", strings.NewReader(body))
+	req.Header.Set("Authorization", "Bearer "+testToken)
+	w := httptest.NewRecorder()
+	mux.ServeHTTP(w, req)
+
+	if w.Code != http.StatusOK {
+		t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
+	}
+	var exec domain.Execution
+	if err := json.Unmarshal(w.Body.Bytes(), &exec); err != nil {
+		t.Fatalf("decode execution: %v", err)
+	}
+	if exec.Status != domain.ExecutionSucceeded {
+		t.Fatalf("expected succeeded, got %q (error: %s)", exec.Status, exec.Error)
+	}
+}
+
+// TestBuildCapabilities_RiskDerivation pins that mutating workspace tools are
+// registered enabled but confirmation-gated.
+func TestBuildCapabilities_RiskDerivation(t *testing.T) {
+	allowlist := provider.ToolAllowlist{Tools: map[string]provider.ToolMapping{
+		"docker.stop_container": {Capability: "workspace.docker.stop", Risk: "medium", TargetType: "container"},
+	}}
+	caps := provider.NewWorkspaceMCPProvider("http://unused", allowlist).BuildCapabilities()
+	if len(caps) != 1 {
+		t.Fatalf("expected 1 capability, got %d", len(caps))
+	}
+	if !caps[0].Enabled {
+		t.Errorf("medium-risk capability should be enabled")
+	}
+	if !caps[0].RequiresConfirmation {
+		t.Errorf("medium-risk capability must require confirmation")
+	}
+}