fix(consent): key the star guard on the authenticating credential, not headers

The agent-consent refusal on POST /api/github/star asked whether the request
carried an Origin plus the GUI-origin and CSRF headers, on the stated premise
that requireManagementAuth had already matched them against a minted session.
It had not. The gate accepts a raw admin token and returns BEFORE it consults
the session table, so those headers were never validated for a token-authorized
call, and the admin token is readable by anything running as the user, which is
precisely the caller this guard exists to refuse. Three headers with arbitrary
values were enough to star the repository with the user's identity.

managementPrincipal() now resolves which credential passed the gate, from the
same session table and the same CSRF comparison the gate uses, and the server
passes it into the management dispatcher. The route asks for a gui-session
principal: a session this process minted for a browser, which is only accepted
for a mutation after origin and per-session CSRF both match. An unresolved
principal (direct dispatch in tests, any future internal caller) is untrusted.

Behavior for real users is unchanged: dashboard clicks still star, hand-typed
runs still star, and the non-loopback operator dashboard on a raw admin token
keeps the documented fail-closed edge. Both regressions were driven red against
the old header check.

Author: lidge-jun

src/server/index.ts

@@ -157,7 +157,7 @@ import { handleImages } from "./images";
 import { handleLive, logLiveSidebandFrame, parseLiveSidebandTarget, resolveLiveSidebandUpgrade } from "./live";
 import { handleSearch } from "./search";
 import { fetchAllModels, handleManagementAPI, VERSION } from "./management-api";
-import { initializeManagementAuthState, issueGuiSession, requireManagementAuth } from "./management-auth";
+import { initializeManagementAuthState, issueGuiSession, managementPrincipal, requireManagementAuth } from "./management-auth";
 
 const MAX_WS_FRAME_BYTES = 50 * 1024 * 1024;
 const WEBSOCKET_IDLE_TIMEOUT_SECONDS = 0;
@@ -448,7 +448,11 @@ export function startServer(port?: number) {
       if (url.pathname.startsWith("/api/")) {
         const apiAuthError = requireManagementAuth(req, managementAuth, config);
         if (apiAuthError) return withManagementCors(apiAuthError, req, config);
-        const mgmtResponse = await handleManagementAPI(req, url, config);
+        // Which credential passed the gate, resolved from the same session table the
+        // gate used. Consent-bearing routes need this: request headers are forgeable
+        // by anything holding the admin token, the credential is not.
+        const principal = managementPrincipal(req, managementAuth, config) ?? undefined;
+        const mgmtResponse = await handleManagementAPI(req, url, config, {}, principal);
         if (mgmtResponse) return withManagementCors(mgmtResponse, req, config);
         return withManagementCors(formatErrorResponse(404, "not_found", `Unknown endpoint: ${req.method} ${url.pathname}`), req, config);
       }

src/server/management-api.ts

@@ -68,6 +68,7 @@ import { handleSystemRoutes } from "./management/system-routes";
 import { handleSidebarRoutes } from "./management/sidebar-routes";
 import { handleIntegrationRoutes } from "./management/integration-routes";
 import type { ManagementContext } from "./management/context";
+import type { ManagementPrincipal } from "./management-auth";
 export type { ManagementApiDeps } from "./management/context";
 import { fetchAllModels } from "./management/shared";
 import { CatalogGatherBusyError } from "../codex/catalog/provider-fetch";
@@ -82,7 +83,13 @@ export const VERSION = (() => {
   }
 })();
 
-export async function handleManagementAPI(req: Request, url: URL, config: OcxConfig, deps: ManagementApiDeps = {}): Promise<Response | null> {
+export async function handleManagementAPI(
+  req: Request,
+  url: URL,
+  config: OcxConfig,
+  deps: ManagementApiDeps = {},
+  principal?: ManagementPrincipal,
+): Promise<Response | null> {
   if (!isAllowedManagementOrigin(req, config)) {
     return jsonResponse({ error: "cross-origin request blocked" }, 403, req, config);
   }
@@ -125,7 +132,7 @@ export async function handleManagementAPI(req: Request, url: URL, config: OcxCon
       }
     } catch { /* best-effort */ }
   }
-  const ctx: ManagementContext = { req, url, config, deps, refreshCodexCatalogBestEffort, syncClaudeAgentDefsBestEffort };
+  const ctx: ManagementContext = { req, url, config, deps, principal, refreshCodexCatalogBestEffort, syncClaudeAgentDefsBestEffort };
   let routed: Response | null;
   try {
     routed = (await handleConfigRoutes(ctx))

src/server/management-auth.ts

@@ -237,6 +237,38 @@ export function issueGuiSession(
   return { token, ...session };
 }
 
+/**
+ * Which credential actually authorized a management request.
+ *
+ * `admin-token` is the raw token from disk/env: anything running as the user can
+ * read it, including a coding agent. `gui-session` is a session token this process
+ * minted for a browser, and it only authorizes a mutation after the origin and the
+ * per-session CSRF token match. Consent-bearing routes must key off this value
+ * rather than off request headers, which the token holder can forge freely.
+ */
+export type ManagementPrincipal = "admin-token" | "gui-session";
+
+/**
+ * The principal for a request that already passed `requireManagementAuth`. Kept as a
+ * separate resolution (rather than a changed return type) so every existing caller
+ * keeps its `Response | null` contract; the value is derived from the same session
+ * table and the same CSRF comparison the gate uses, so the two cannot disagree.
+ */
+export function managementPrincipal(
+  req: Request,
+  state: ManagementAuthState,
+  config?: OcxConfig,
+): ManagementPrincipal | null {
+  if (!state.available) return null;
+  const actual = req.headers.get("x-opencodex-api-key")?.trim()
+    || req.headers.get("authorization")?.replace(/^Bearer\s+/i, "").trim();
+  if (!actual) return null;
+  if (equalSecret(actual, state.token)) return "admin-token";
+  if (!config) return null;
+  removeExpiredSessions(state);
+  return state.sessions.has(actual) ? "gui-session" : null;
+}
+
 export function requireManagementAuth(
   req: Request,
   state: ManagementAuthState,

src/server/management/context.ts

@@ -1,5 +1,6 @@
 import type { OcxConfig } from "../../types";
 import type { StartupInstallAction } from "../startup-action-control";
+import type { ManagementPrincipal } from "../management-auth";
 
 export interface ManagementApiDeps {
   toggleCodexMultiAgentV2?: (enabled: boolean) => void;
@@ -26,6 +27,15 @@ export interface ManagementContext {
   url: URL;
   config: OcxConfig;
   deps: ManagementApiDeps;
+  /**
+   * Which credential authorized this request, resolved by the auth gate before
+   * dispatch. Routes that spend the USER's identity (not just the proxy's) must
+   * branch on this instead of on request headers: the admin token is readable by
+   * anything running as the user, so a token holder can forge any header a route
+   * might otherwise treat as browser evidence. Undefined only in direct-dispatch
+   * tests, which are treated as the untrusted `admin-token` case.
+   */
+  principal?: ManagementPrincipal;
   refreshCodexCatalogBestEffort: () => Promise<void>;
   syncClaudeAgentDefsBestEffort: () => Promise<void>;
 }

src/server/management/sidebar-routes.ts

@@ -16,25 +16,28 @@
  *
  * The dashboard button must keep working even when the proxy itself was started by
  * an agent, which is the common case — the person is at the browser, not at the
- * spawning shell. A GUI click is therefore distinguished by its browser session
- * evidence (a same-origin `Origin` plus the minted CSRF header, both already
- * verified by the management auth gate) rather than by the proxy's own env.
+ * spawning shell. A GUI click is therefore distinguished by the CREDENTIAL that
+ * authorized the request — a GUI session this process minted for a browser, which
+ * the auth gate only accepts after matching origin and the per-session CSRF token —
+ * rather than by the proxy's own env or by request headers.
  */
 import { jsonResponse } from "../auth-cors";
 import { agentDrivenMarkers, isAgentDriven } from "../../cli/agent-driven";
 import type { ManagementContext } from "./context";
 
 /**
- * True when this request carries the browser-session evidence a dashboard click
- * always has: an `Origin` (only a browser sends one) plus the per-session CSRF
- * token, which `requireManagementAuth` has already matched against the minted
- * session before dispatch. A shell/HTTP caller holding only the admin token has
- * neither, which is exactly the case the agent guard is aimed at.
+ * True only when a minted GUI session authorized this request.
+ *
+ * The previous version of this check looked for an `Origin` plus the CSRF headers
+ * and reasoned that the auth gate had already validated them. It had not: the gate
+ * accepts a raw admin token BEFORE it ever consults the session table, so a caller
+ * holding that token (any process running as the user, a coding agent included)
+ * could add three nonempty headers of its choosing and satisfy this check without
+ * a browser ever being involved. The credential itself is the only part of the
+ * request an agent cannot fabricate, so that is what this now reads.
  */
-function hasBrowserSessionEvidence(req: Request): boolean {
-  return !!req.headers.get("Origin")?.trim()
-    && !!req.headers.get("x-opencodex-csrf-token")?.trim()
-    && !!req.headers.get("x-opencodex-gui-origin")?.trim();
+function hasBrowserSessionEvidence(ctx: ManagementContext): boolean {
+  return ctx.principal === "gui-session";
 }
 
 // Known edge, deliberately fail-closed: a non-loopback operator dashboard signs in
@@ -58,7 +61,7 @@ export async function handleSidebarRoutes(ctx: ManagementContext): Promise<Respo
     // account owner. An agent-driven caller without browser-session evidence
     // cannot have obtained it, and must relay the question instead of answering
     // it with an HTTP call.
-    if (isAgentDriven() && !hasBrowserSessionEvidence(req)) {
+    if (isAgentDriven() && !hasBrowserSessionEvidence(ctx)) {
       return jsonResponse({
         ok: false,
         state: "not-starred",

tests/sidebar-routes.test.ts

@@ -19,13 +19,14 @@ async function call(
   method: string,
   pathname: string,
   headers: Record<string, string> = {},
+  principal?: "admin-token" | "gui-session",
 ): Promise<{ status: number; body: unknown; raw: string; routed: boolean }> {
   // `isAllowedManagementOrigin` derives the expected origin from the Host header and
   // rejects the request outright when it is missing, so Host is required here. Omitting
   // Origin models the GUI's own same-origin fetch.
   const url = new URL(`http://127.0.0.1:10100${pathname}`);
   const req = new Request(url, { method, headers: { host: "127.0.0.1:10100", ...headers } });
-  const res = await handleManagementAPI(req, url, config);
+  const res = await handleManagementAPI(req, url, config, {}, principal);
   if (!res) return { status: 404, body: null, raw: "", routed: false };
   const raw = await res.text();
   return { status: res.status, body: raw ? JSON.parse(raw) : null, raw, routed: true };
@@ -183,19 +184,62 @@ describe("route surface", () => {
       async runGh(args) { calls.push(args); return { status: 0 }; },
     }, async () => {
       invalidateStarStatusCache();
-      // Browser-session evidence: same-origin Origin plus the minted CSRF/GUI-origin
-      // headers the management auth gate has already verified before dispatch.
+      // Browser-session evidence is the CREDENTIAL, not the headers: the auth gate
+      // resolved a minted GUI session, which it only issues to a browser and only
+      // accepts for a mutation after matching origin and the per-session CSRF token.
       const { status, body } = await call("POST", "/api/github/star", {
         origin: "http://127.0.0.1:10100",
         "x-opencodex-gui-origin": "http://127.0.0.1:10100",
         "x-opencodex-csrf-token": "csrf-token",
-      });
+      }, "gui-session");
       expect(status).toBe(200);
       expect((body as Record<string, unknown>).ok).toBe(true);
     }));
     expect(calls.some(args => args.includes("PUT"))).toBe(true);
   });
 
+  test("forged dashboard headers on an admin-token call cannot star", async () => {
+    // The consent guard used to read the request's Origin/CSRF/GUI-origin headers and
+    // trust them as proof of a browser click. The auth gate accepts a raw admin token
+    // BEFORE it consults the session table, so an agent that can read that token — any
+    // process running as the user — could send exactly these headers with arbitrary
+    // values and star the repository with the user's identity.
+    const calls: string[][] = [];
+    await withEnv({ ...NO_AGENT_ENV, CODEX_THREAD_ID: "019fbc94" }, () => withStarDeps({
+      nowMs: () => 0,
+      async runGh(args) { calls.push(args); return { status: 0 }; },
+    }, async () => {
+      invalidateStarStatusCache();
+      const { status, body } = await call("POST", "/api/github/star", {
+        origin: "http://127.0.0.1:10100",
+        "x-opencodex-gui-origin": "http://127.0.0.1:10100",
+        "x-opencodex-csrf-token": "forged-by-the-token-holder",
+      }, "admin-token");
+      expect(status).toBe(403);
+      expect((body as Record<string, unknown>).code).toBe("agent_consent_required");
+    }));
+    expect(calls).toEqual([]);
+  });
+
+  test("a direct dispatch with no resolved principal is treated as untrusted", async () => {
+    // Defense in depth for callers that bypass the HTTP gate (route-level tests, future
+    // internal dispatchers): an unknown principal must never satisfy the consent check.
+    const calls: string[][] = [];
+    await withEnv({ ...NO_AGENT_ENV, CODEX_THREAD_ID: "019fbc94" }, () => withStarDeps({
+      nowMs: () => 0,
+      async runGh(args) { calls.push(args); return { status: 0 }; },
+    }, async () => {
+      invalidateStarStatusCache();
+      const { status } = await call("POST", "/api/github/star", {
+        origin: "http://127.0.0.1:10100",
+        "x-opencodex-gui-origin": "http://127.0.0.1:10100",
+        "x-opencodex-csrf-token": "csrf-token",
+      });
+      expect(status).toBe(403);
+    }));
+    expect(calls).toEqual([]);
+  });
+
   test("a hand-typed run is not blocked by the agent guard", async () => {
     const calls: string[][] = [];
     await withEnv(NO_AGENT_ENV, () => withStarDeps({

tests/startup-prompt.test.ts

@@ -120,7 +120,12 @@ describe("startup star prompt", () => {
     // A dashboard click must still work when an agent started the proxy, so the
     // refusal is conditioned on the absence of browser-session evidence.
     expect(routes).toContain("hasBrowserSessionEvidence");
-    expect(routes).toMatch(/isAgentDriven\(\)\s*&&\s*!hasBrowserSessionEvidence\(req\)/);
+    expect(routes).toMatch(/isAgentDriven\(\)\s*&&\s*!hasBrowserSessionEvidence\(ctx\)/);
+    // And that evidence must be the authenticating CREDENTIAL, never a request
+    // header: the admin token is readable by anything running as the user, so a
+    // header-shaped check is forgeable by the exact caller this guard refuses.
+    expect(routes).toMatch(/principal === "gui-session"/);
+    expect(routes).not.toMatch(/hasBrowserSessionEvidence[\s\S]*?headers\.get\("x-opencodex-csrf-token"\)/);
   });
 
   test("the consent rule is written down where agents and users read it", async () => {