ci: scope on the verified org, not on a query parameter

This service shipped with /v1/runs answering 200 to anyone on the internet, disclosing repo names, workflow names, branches, commit SHAs, actor logins and pass/fail across EVERY org. The cause was a category error, not a missing check: `?org=` narrowed what was rendered and read like tenancy, so it looked like the surface had one. A query parameter is a request for a view. It can never be the authority for one. The authority is now X-Org-Id, minted by admin-guard from the IAM-verified `owner` claim and written onto the request by the ingress middleware's authResponseHeaders (Traefik overwrites any client-sent value, so it cannot be forged on the wired path). Three properties, each with a test that fails without it: - ABSENCE IS FATAL. No X-Org-Id => 403, never "no filter". Defaulting an absent scope to "everything" is precisely the bug; absence means the request did not come through the gate, so refusing is the only honest answer. The refusal body carries no repo names. - THE PARAMETER CAN ONLY NARROW. Permission is applied first, then `?org=` selects within it. A lux viewer asking ?org=hanzo gets an empty list, not hanzo's builds. - THE ORG LIST IS SCOPED TOO. A tenant sees only its own org in the nav. Hiding the runs but listing every org still discloses the set of orgs that build on the platform. The admin org keeps the cross-tenant fleet view, matched to admin-guard's IAM_ADMIN_ORG via CI_ADMIN_ORG — the guard decides who gets in, this decides who sees everything, and the two must name the same org or the fleet view silently collapses (or, set too wide, promotes a tenant into it). renderDashboard now takes the viewer and is handed only rows that already passed v.visible. A template that can see everything is one edit away from showing it. Mutation-verified: restoring `visible` to the old filter-as-gate behaviour fails TestTenantCannotWidenWithQueryParam and the end-to-end handler test. Co-authored-by: Hanzo Dev <dev@hanzo.ai>
2026-07-28 10:33:07 -07:00
parent aeb6adf4d5
commit 8c54cfea8e
5 changed files with 305 additions and 9 deletions
@@ -64,14 +64,18 @@ func main() {
 		writeJSON(w, http.StatusOK, map[string]any{"status": "ok"})
 	})
 	mux.HandleFunc("/v1/runs", func(w http.ResponseWriter, r *http.Request) {
+		v, ok := requireViewer(w, r, cfg.adminOrg)
+		if !ok {
+			return
+		}
 		snap := cache.get()
 		writeJSON(w, http.StatusOK, map[string]any{
-			"runs":      filterByOrg(snap.Runs, r.URL.Query().Get("org")),
+			"runs":      v.visible(snap.Runs, r.URL.Query().Get("org")),
 			"fetchedAt": snap.FetchedAt,
 			"stale":     snap.stale(cfg.staleAfter),
 			"sourceErr": snap.errString(),
 			"repos":     snap.Repos,
-			"orgs":      orgsOf(snap.Runs),
+			"orgs":      v.orgs(snap.Runs),
 		})
 	})
 	mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
@@ -79,7 +83,11 @@ func main() {
 			http.NotFound(w, r)
 			return
 		}
-		renderDashboard(w, cache.get(), r.URL.Query().Get("org"), cfg)
+		v, ok := requireViewer(w, r, cfg.adminOrg)
+		if !ok {
+			return
+		}
+		renderDashboard(w, cache.get(), v, r.URL.Query().Get("org"), cfg)
 	})

 	srv := &http.Server{
@@ -104,8 +112,13 @@ func main() {
 // ───────────────────────────── config ─────────────────────────────

 type config struct {
-	listen     string
-	gitBase    string
+	listen  string
+	gitBase string
+	// adminOrg is the ONE org whose members see across tenants. It must match
+	// admin-guard's IAM_ADMIN_ORG — the guard decides who gets in, this decides
+	// who sees everything, and a mismatch would silently demote the fleet view to
+	// a single-org view (or, if set too wide, promote a tenant to it).
+	adminOrg   string
 	gitToken   string
 	refresh    time.Duration
 	staleAfter time.Duration
@@ -117,6 +130,7 @@ func loadConfig() (config, error) {
 	c := config{
 		listen:    env("CI_LISTEN", ":8080"),
 		gitBase:   strings.TrimRight(env("CI_GIT_BASE", "https://git.hanzo.ai"), "/"),
+		adminOrg:  env("CI_ADMIN_ORG", "admin"),
 		gitToken:  os.Getenv("CI_GIT_TOKEN"),
 		scanRepos: envInt("CI_SCAN_REPOS", 60),
 		runsPer:   envInt("CI_RUNS_PER_REPO", 8),
@@ -14,8 +14,12 @@ import (
 // has to be readable when the thing it reports on is broken, which is exactly
 // when a build pipeline for its own frontend is the wrong dependency.

-func renderDashboard(w http.ResponseWriter, snap snapshot, org string, cfg config) {
-	runs := filterByOrg(snap.Runs, org)
+// renderDashboard writes the page for ONE viewer. Every row it renders has
+// already passed v.visible — the template is never handed the full snapshot and
+// asked to be careful with it, because a template that can see everything is one
+// edit away from showing it.
+func renderDashboard(w http.ResponseWriter, snap snapshot, v viewer, org string, cfg config) {
+	runs := v.visible(snap.Runs, org)
 	if len(runs) > 200 {
 		runs = runs[:200]
 	}
@@ -24,6 +28,8 @@ func renderDashboard(w http.ResponseWriter, snap snapshot, org string, cfg confi
 		Runs      []Run
 		Orgs      []string
 		Org       string
+		Viewer    string
+		Sudo      bool
 		Repos     int
 		FetchedAt time.Time
 		Age       string
@@ -33,8 +39,10 @@ func renderDashboard(w http.ResponseWriter, snap snapshot, org string, cfg confi
 		Counts    map[string]int
 	}{
 		Runs:      runs,
-		Orgs:      orgsOf(snap.Runs),
+		Orgs:      v.orgs(snap.Runs),
 		Org:       org,
+		Viewer:    v.org,
+		Sudo:      v.sudo,
 		Repos:     snap.Repos,
 		FetchedAt: snap.FetchedAt,
 		Age:       humanAge(snap.FetchedAt),
@@ -144,6 +152,7 @@ nav{display:flex;gap:6px;padding:0 24px 16px;flex-wrap:wrap}
 nav a{padding:5px 11px;border:1px solid var(--line);border-radius:999px;
      background:var(--panel);color:var(--dim);text-decoration:none;font-size:12px}
 nav a.on{border-color:var(--accent);color:var(--fg)}
+nav .who{margin-left:auto;align-self:center;color:var(--dim);font-size:12px}
 .warn{margin:0 24px 16px;padding:10px 14px;border:1px solid var(--run);
      border-radius:8px;background:#221b0c;color:#f0d58c;font-size:13px}
 table{width:100%;border-collapse:collapse}
@@ -183,8 +192,9 @@ footer{padding:16px 24px;color:var(--dim);font-size:12px;border-top:1px solid va
 </div>

 <nav>
-  <a href="/" {{if eq .Org ""}}class="on"{{end}}>all orgs</a>
+  {{if .Sudo}}<a href="/" {{if eq .Org ""}}class="on"{{end}}>all orgs</a>{{end}}
  {{range .Orgs}}<a href="/?org={{.}}" {{if eq $.Org .}}class="on"{{end}}>{{.}}</a>{{end}}
+  <span class="who">signed in as {{.Viewer}}{{if .Sudo}} &middot; fleet view{{end}}</span>
 </nav>

 {{if .Stale}}<div class="warn">
@@ -0,0 +1,96 @@
+package main
+
+import (
+	"net/http"
+	"strings"
+)
+
+// scope.go answers exactly one question: whose builds may THIS request see?
+//
+// It exists because the first cut of this service conflated a FILTER with a
+// GATE. `?org=lux` narrowed what was rendered and read like tenancy, but it
+// decided nothing about who was allowed to ask — so /v1/runs answered 200 to
+// anyone on the internet with every org's repo names, branches, commit SHAs and
+// actor logins. A query parameter is a request for a view; it can never be the
+// authority for one.
+//
+// The authority is X-Org-Id, minted by admin-guard from the IAM-verified `owner`
+// claim and written onto the request by the ingress middleware's
+// authResponseHeaders. Traefik OVERWRITES any client-sent X-Org-Id with the
+// guard's value, so on the wired path the header cannot be forged. This file
+// still treats its ABSENCE as fatal rather than as "no filter", because absence
+// is the signal that the request did not come through the guard at all.
+
+// orgHeader is the identity the whole surface is scoped by. One name, one
+// meaning, platform-wide (see the X-* header convention: X-Org-Id is the org
+// slug from the JWT `owner` claim).
+const orgHeader = "X-Org-Id"
+
+// viewer is the resolved, trusted answer. Constructed only from headers the
+// guard controls — never from the query string, never from a cookie.
+type viewer struct {
+	// org is the caller's home org slug, from the verified `owner` claim.
+	org string
+	// sudo reports whether org is the platform admin org, which is the ONE
+	// identity that may see across tenants (the fleet view).
+	sudo bool
+}
+
+// resolveViewer lifts the guard-set header into a viewer. It fails closed: a
+// missing or blank X-Org-Id yields ok=false and the caller MUST refuse the
+// request.
+//
+// Defaulting an absent header to "no filter" is the specific bug this function
+// exists to prevent — that default is what turns "reached ci without the guard"
+// into "rendered every org's builds".
+func resolveViewer(r *http.Request, adminOrg string) (viewer, bool) {
+	org := strings.TrimSpace(r.Header.Get(orgHeader))
+	if org == "" {
+		return viewer{}, false
+	}
+	return viewer{org: org, sudo: strings.EqualFold(org, strings.TrimSpace(adminOrg))}, true
+}
+
+// visible narrows runs to what v is permitted to see, then applies want (the
+// optional `?org=` selection) WITHIN that permission.
+//
+// The ordering is the whole point: permission is applied first and `want` can
+// only ever narrow the result. A lux viewer asking for `?org=hanzo` gets an
+// empty list, not hanzo's builds — the parameter selects among what you may
+// already see, it never reaches for more.
+func (v viewer) visible(runs []Run, want string) []Run {
+	want = strings.TrimSpace(want)
+	if v.sudo {
+		// The fleet view: every org, narrowed by the requested one if given.
+		return filterByOrg(runs, want)
+	}
+	if want != "" && !strings.EqualFold(want, v.org) {
+		return nil
+	}
+	return filterByOrg(runs, v.org)
+}
+
+// orgs lists the org tabs this viewer may choose between. A tenant gets exactly
+// its own org — rendering the full org list to a tenant would leak the set of
+// orgs that build on the platform even though their runs are correctly hidden.
+func (v viewer) orgs(runs []Run) []string {
+	if v.sudo {
+		return orgsOf(runs)
+	}
+	return []string{v.org}
+}
+
+// requireViewer resolves the viewer or writes the refusal. It returns ok=false
+// when the request must not proceed.
+func requireViewer(w http.ResponseWriter, r *http.Request, adminOrg string) (viewer, bool) {
+	v, ok := resolveViewer(r, adminOrg)
+	if ok {
+		return v, true
+	}
+	// 403, not 401: a 401 invites a credential retry, but there is nothing the
+	// CALLER can add to fix this. The header is set by infrastructure, so its
+	// absence is a routing fault (ci reached off-guard) and the honest answer is
+	// that this path is not authorized to serve, whoever is asking.
+	http.Error(w, "forbidden: no "+orgHeader+" (this service is only reachable through the IAM gate)", http.StatusForbidden)
+	return viewer{}, false
+}
@@ -0,0 +1,176 @@
+package main
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+)
+
+// scope_test.go is the regression suite for the leak this service shipped with:
+// /v1/runs answered 200 to anyone, with every org's repo names, branches, commit
+// SHAs and actor logins, because `?org=` was a filter being used as a gate.
+//
+// The properties asserted here are the ones that made it a leak, not merely the
+// ones that make the new code work.
+
+func testRuns() []Run {
+	return []Run{
+		{Org: "hanzo", Repo: "cloud", Workflow: "build", Status: "completed", Conclusion: "success"},
+		{Org: "lux", Repo: "node", Workflow: "build", Status: "completed", Conclusion: "failure"},
+		{Org: "zoo", Repo: "app", Workflow: "test", Status: "in_progress"},
+	}
+}
+
+// TestNoOrgHeaderIsRefused is the core fix. An absent X-Org-Id means the request
+// did not come through the IAM gate; the ONLY safe answer is to refuse. The old
+// code treated the equivalent condition (no `?org=`) as "show everything".
+func TestNoOrgHeaderIsRefused(t *testing.T) {
+	for _, hdr := range []string{"", "   "} {
+		r := httptest.NewRequest(http.MethodGet, "/v1/runs", nil)
+		if hdr != "" {
+			r.Header.Set(orgHeader, hdr)
+		}
+		w := httptest.NewRecorder()
+
+		v, ok := requireViewer(w, r, "admin")
+		if ok {
+			t.Fatalf("X-Org-Id=%q admitted as viewer %+v — absence must fail closed", hdr, v)
+		}
+		if w.Code != http.StatusForbidden {
+			t.Errorf("X-Org-Id=%q: status=%d want 403", hdr, w.Code)
+		}
+	}
+}
+
+// TestTenantCannotWidenWithQueryParam is the attack the original design invited:
+// the caller picks the org. Now the header decides and the parameter may only
+// narrow, so a lux viewer asking for hanzo's builds gets nothing — NOT hanzo's
+// builds, and not a silent fallback to its own either (that would be confusing,
+// but it is the empty answer that matters for security).
+func TestTenantCannotWidenWithQueryParam(t *testing.T) {
+	lux := viewer{org: "lux"}
+
+	got := lux.visible(testRuns(), "hanzo")
+	if len(got) != 0 {
+		t.Fatalf("lux viewer asking ?org=hanzo saw %d runs (%+v) — must see none", len(got), got)
+	}
+
+	own := lux.visible(testRuns(), "")
+	if len(own) != 1 || own[0].Org != "lux" {
+		t.Fatalf("lux viewer saw %+v; want exactly its own org", own)
+	}
+	if same := lux.visible(testRuns(), "lux"); len(same) != 1 {
+		t.Errorf("lux viewer asking ?org=lux saw %d runs; want its own 1", len(same))
+	}
+}
+
+// TestSudoSeesFleetAndCanNarrow asserts the admin org keeps the cross-tenant
+// view that makes this dashboard useful to the platform, and that `?org=` still
+// works as a plain filter for it.
+func TestSudoSeesFleetAndCanNarrow(t *testing.T) {
+	sudo := viewer{org: "admin", sudo: true}
+
+	if all := sudo.visible(testRuns(), ""); len(all) != 3 {
+		t.Fatalf("sudo saw %d runs; want all 3", len(all))
+	}
+	one := sudo.visible(testRuns(), "zoo")
+	if len(one) != 1 || one[0].Org != "zoo" {
+		t.Fatalf("sudo ?org=zoo saw %+v; want zoo only", one)
+	}
+}
+
+// TestResolveViewerSudoDetection pins the sudo bit to the configured admin org,
+// case-insensitively, and proves an ordinary org never gets it.
+func TestResolveViewerSudoDetection(t *testing.T) {
+	cases := []struct {
+		hdr, adminOrg string
+		wantSudo      bool
+	}{
+		{"admin", "admin", true},
+		{"ADMIN", "admin", true},
+		{" admin ", "admin", true},
+		{"lux", "admin", false},
+		{"administrator", "admin", false}, // prefix must not match
+		{"admin", "root", false},          // honours a non-default admin org
+	}
+	for _, tc := range cases {
+		r := httptest.NewRequest(http.MethodGet, "/", nil)
+		r.Header.Set(orgHeader, tc.hdr)
+		v, ok := resolveViewer(r, tc.adminOrg)
+		if !ok {
+			t.Fatalf("X-Org-Id=%q: not resolved", tc.hdr)
+		}
+		if v.sudo != tc.wantSudo {
+			t.Errorf("X-Org-Id=%q adminOrg=%q: sudo=%v want %v", tc.hdr, tc.adminOrg, v.sudo, tc.wantSudo)
+		}
+	}
+}
+
+// TestTenantOrgListIsNotTheFleetList covers the quieter leak: even with runs
+// correctly hidden, rendering every org's NAME in the nav would disclose the set
+// of orgs that build on the platform.
+func TestTenantOrgListIsNotTheFleetList(t *testing.T) {
+	lux := viewer{org: "lux"}
+	orgs := lux.orgs(testRuns())
+	if len(orgs) != 1 || orgs[0] != "lux" {
+		t.Fatalf("tenant org list = %v; want only its own org", orgs)
+	}
+	if sudoOrgs := (viewer{org: "admin", sudo: true}).orgs(testRuns()); len(sudoOrgs) != 3 {
+		t.Errorf("sudo org list = %v; want all 3", sudoOrgs)
+	}
+}
+
+// TestRunsEndpointScopesEndToEnd drives the actual HTTP handler wiring, not just
+// the predicates — the leak was in the handler, so the handler is what must be
+// asserted.
+func TestRunsEndpointScopesEndToEnd(t *testing.T) {
+	cache := &runCache{}
+	cache.put(snapshot{Runs: testRuns(), Repos: 3})
+	cfg := config{adminOrg: "admin"}
+
+	h := func(w http.ResponseWriter, r *http.Request) {
+		v, ok := requireViewer(w, r, cfg.adminOrg)
+		if !ok {
+			return
+		}
+		snap := cache.get()
+		writeJSON(w, http.StatusOK, map[string]any{
+			"runs": v.visible(snap.Runs, r.URL.Query().Get("org")),
+			"orgs": v.orgs(snap.Runs),
+		})
+	}
+
+	t.Run("anonymous → 403", func(t *testing.T) {
+		w := httptest.NewRecorder()
+		h(w, httptest.NewRequest(http.MethodGet, "/v1/runs", nil))
+		if w.Code != http.StatusForbidden {
+			t.Fatalf("status=%d want 403; body=%s", w.Code, w.Body.String())
+		}
+		if strings.Contains(w.Body.String(), "cloud") || strings.Contains(w.Body.String(), "node") {
+			t.Error("refusal body leaked repo names")
+		}
+	})
+
+	t.Run("lux viewer sees only lux, even asking for hanzo", func(t *testing.T) {
+		r := httptest.NewRequest(http.MethodGet, "/v1/runs?org=hanzo", nil)
+		r.Header.Set(orgHeader, "lux")
+		w := httptest.NewRecorder()
+		h(w, r)
+
+		var got struct {
+			Runs []Run    `json:"runs"`
+			Orgs []string `json:"orgs"`
+		}
+		if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil {
+			t.Fatalf("decode: %v", err)
+		}
+		if len(got.Runs) != 0 {
+			t.Errorf("lux asking ?org=hanzo got %+v; want none", got.Runs)
+		}
+		if len(got.Orgs) != 1 || got.Orgs[0] != "lux" {
+			t.Errorf("orgs=%v; want [lux]", got.Orgs)
+		}
+	})
+}