From 0b7d80cee0e08404c16fe58c7138658e26059ed9 Mon Sep 17 00:00:00 2001
From: kami <kami@kvmx.ru>
Date: Mon, 27 Jul 2026 18:59:28 +0400
Subject: [PATCH] fix(authz): reject Surface: system from HTTP requests (B8)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

System means "the plane itself, in-process" (router, coordinator, adapters,
lease-expiry reclaim) and is unconditionally FullControl with no token
gate. But it was reachable straight from the X-Orchestra-Surface HTTP
header, both in authz.HTTP's token check and in main.go's own `surface`
closure (which every handler actually calls to authorize an event — it
re-parses the header independently of what the HTTP middleware resolved).
Since no deployment configures ORCHESTRA_SYSTEM_TOKEN (no legitimate HTTP
caller should ever need one), tokens[System] is always "", so the token
check was skipped entirely: any LAN request with
"X-Orchestra-Surface: system" got unauthenticated full control to emit any
event on any task.

Both the authz.HTTP middleware and main.go's `surface` closure now
downgrade System to Web before doing anything else with it, so the header
can never resolve to System over HTTP regardless of token config.

AUDIT.md B8.
---
 cmd/orchestra/main.go        |  6 +++++-
 internal/authz/authz.go      | 10 ++++++++++
 internal/authz/authz_test.go | 35 ++++++++++++++++++++++++++++++++++-
 3 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/cmd/orchestra/main.go b/cmd/orchestra/main.go
index e1d0831..d9aeddb 100644
--- a/cmd/orchestra/main.go
+++ b/cmd/orchestra/main.go
@@ -142,7 +142,11 @@ func main() {
 	providerHealth := map[string]*provider.Supervisor{}
 	surface := func(r *http.Request) authz.Surface {
 		v := authz.ParseSurface(r.Header.Get("X-Orchestra-Surface"))
-		if v == "" {
+		if v == "" || v == authz.System {
+			// System means "the plane itself, in-process" and is always
+			// FullControl with no token gate (AUDIT.md B8) — it must never
+			// be constructible from an HTTP request, or any LAN caller
+			// declaring this header gets unauthenticated full control.
 			return authz.Web
 		}
 		return v
diff --git a/internal/authz/authz.go b/internal/authz/authz.go
index 340040b..a3729f5 100644
--- a/internal/authz/authz.go
+++ b/internal/authz/authz.go
@@ -75,6 +75,16 @@ func HTTP(tokens map[Surface]string, next http.Handler) http.Handler {
 		if s == "" {
 			s = Web
 		}
+		// System means "the plane itself, in-process" (router, coordinator,
+		// adapters, lease-expiry reclaim) and is always FullControl with no
+		// token gate — it must never be reachable by declaring it over HTTP.
+		// Without this, tokens[System] being unset (as it is by default: no
+		// caller ever needs a System token) makes the check at line ~78 a
+		// no-op, and any LAN request with this header gets unauthenticated
+		// full control over every task.
+		if s == System {
+			s = Web
+		}
 		if expected := tokens[s]; expected != "" && r.Header.Get("Authorization") != "Bearer "+expected {
 			http.Error(w, "unauthorized surface", http.StatusUnauthorized)
 			return
diff --git a/internal/authz/authz_test.go b/internal/authz/authz_test.go
index a29e4f3..9de6fe5 100644
--- a/internal/authz/authz_test.go
+++ b/internal/authz/authz_test.go
@@ -1,6 +1,10 @@
 package authz
 
-import "testing"
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
 
 func TestSurfaceCapabilities(t *testing.T) {
 	if Telegram.CanEmit("TaskCreated") || Ntfy.CanEmit("ApprovalRequested") {
@@ -13,3 +17,32 @@ func TestSurfaceCapabilities(t *testing.T) {
 		t.Fatal("mcp gate is wrong")
 	}
 }
+
+// TestSystemSurfaceDowngradedByHTTPMiddleware guards half of B8: System
+// means "the plane itself, in-process" and is always FullControl with no
+// token gate, since no deployment configures a token for a surface no HTTP
+// caller is meant to use. HTTP() must never let a request pass through
+// treated as System, or a caller declaring X-Orchestra-Surface: system gets
+// an unconditional, unauthenticated bypass of the token check below it.
+// (Downstream handlers must independently avoid re-deriving System from the
+// raw header themselves — see cmd/orchestra/main.go's `surface` closure,
+// which this package cannot test directly.)
+func TestSystemSurfaceDowngradedByHTTPMiddleware(t *testing.T) {
+	tokens := map[Surface]string{System: "should-never-be-checked"}
+	h := HTTP(tokens, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	}))
+	req := httptest.NewRequest(http.MethodPost, "/v1/tasks/t1/complete", nil)
+	req.Header.Set("X-Orchestra-Surface", "system")
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, req)
+	// System's token check is intentionally skipped by HTTP() (it downgrades
+	// to Web before the token comparison), so the request reaching the
+	// handler at all is expected here — the guard that matters is that
+	// nothing downstream can observe "system" as the resolved surface. This
+	// test documents the middleware's half of the fix; main.go's `surface`
+	// closure carries the other half.
+	if rec.Code != http.StatusOK {
+		t.Fatalf("unexpected status %d", rec.Code)
+	}
+}