fix(authz): reject Surface: system from HTTP requests (B8)

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.
This commit is contained in:
kami
2026-07-27 18:59:28 +04:00
parent 19bffaf77d
commit 0b7d80cee0
3 changed files with 49 additions and 2 deletions
+5 -1
View File
@@ -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
+10
View File
@@ -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
+34 -1
View File
@@ -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)
}
}