From a2dff01beaf18da9b944885552eb2f31dea99dd4 Mon Sep 17 00:00:00 2001 From: Bryan Fawcett Date: Sat, 3 Oct 2026 03:10:05 +0800 Subject: [PATCH 1/3] fix: read the AuthKit issuer only from configuration Remove WORKOS_AUTHORIZATION_SERVER = "https://accounts.mukoko.com" from worker/wrangler.toml [vars], the "https://accounts.mukoko.com" fallback in the agent card and the "https://api.workos.com" fallback in the resource metadata. Both documents now take the issuer only from the WORKOS_AUTHORIZATION_SERVER Worker secret (normalised: bare host or https origin, no trailing slash) and answer 503 "WORKOS_AUTHORIZATION_SERVER is not configured" when it is missing. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GHjaH1Vt2dVLWU68iz8tFq --- worker/README.md | 11 +++++++---- worker/src/a2a.ts | 13 ++++++++++--- worker/src/auth.ts | 31 ++++++++++++++++++++++++++----- worker/src/env.ts | 2 +- worker/src/index.ts | 10 ++++++++-- worker/test/a2a.test.ts | 14 ++++++++++---- worker/test/policy.test.ts | 12 ++++++++++-- worker/wrangler.toml | 17 +++++++++-------- 8 files changed, 81 insertions(+), 29 deletions(-) diff --git a/worker/README.md b/worker/README.md index 3c802fe..9dc4690 100644 --- a/worker/README.md +++ b/worker/README.md @@ -116,13 +116,16 @@ private key is the secret: ```bash wrangler secret put GITHUB_APP_PRIVATE_KEY # the whole .pem, BEGIN/END included -wrangler secret put WORKOS_JWKS_URL # https://accounts.mukoko.com/oauth2/jwks -wrangler secret put WORKOS_ISSUER # https://accounts.mukoko.com +wrangler secret put WORKOS_AUTHORIZATION_SERVER # +wrangler secret put WORKOS_JWKS_URL # /oauth2/jwks +wrangler secret put WORKOS_ISSUER # wrangler secret put WORKOS_AUDIENCE # https://github.shamwari.ai/mcp ``` -The three WorkOS values are not guesses. `issuer` and `jwks_uri` are what -`https://accounts.mukoko.com/.well-known/oauth-authorization-server` serves; +The AuthKit domain is set per environment (1Password `nyuchi/workos`, field +`WORKOS_AUTHKIT_DOMAIN`) and never committed; there is no default in code. +`issuer` and `jwks_uri` are what +`/.well-known/oauth-authorization-server` serves; the audience is this worker's resource URI. **Do not use `https://api.workos.com/sso/jwks/`.** That is the older diff --git a/worker/src/a2a.ts b/worker/src/a2a.ts index 3634383..4a5ce7c 100644 --- a/worker/src/a2a.ts +++ b/worker/src/a2a.ts @@ -22,6 +22,7 @@ // something that looks right and no current client will parse. import type { Env } from "./env"; +import { authorizationServer } from "./auth"; import { GitHubError } from "./github"; import { reviewPullRequest, REVIEW_MODELS, DEFAULT_MODEL } from "./review"; @@ -49,10 +50,16 @@ export interface A2ARequest { * Served unauthenticated, like the OAuth resource metadata beside it: a * client cannot authenticate until it has read which scheme to use. Nothing * here is secret, and the skills it lists are the ones actually implemented. + * + * Null when WORKOS_AUTHORIZATION_SERVER is unset — the security scheme names + * the authorization server, which comes only from configuration. */ -export function agentCard(env: Env, baseUrl: string): Record { - const authServer = - env.WORKOS_AUTHORIZATION_SERVER || "https://accounts.mukoko.com"; +export function agentCard( + env: Env, + baseUrl: string, +): Record | null { + const authServer = authorizationServer(env); + if (!authServer) return null; return { name: "Shamwari for GitHub", description: diff --git a/worker/src/auth.ts b/worker/src/auth.ts index 32594d6..9cc5fec 100644 --- a/worker/src/auth.ts +++ b/worker/src/auth.ts @@ -272,13 +272,34 @@ export function resourceUrl(env: Env): string { return env.MCP_RESOURCE_URL || "https://github.shamwari.ai/mcp"; } -/** OAuth 2.0 Protected Resource Metadata (RFC 9728) — points clients at WorkOS. */ -export function protectedResourceMetadata(env: Env): Record { +export const AUTHORIZATION_SERVER_MISSING = + "WORKOS_AUTHORIZATION_SERVER is not configured"; + +/** + * The AuthKit issuer to advertise, from configuration only — there is no + * compiled-in default and no fallback host. Accepts a bare host or an https + * origin; trims whitespace and any trailing slash. Null when unset. + */ +export function authorizationServer(env: Env): string | null { + const raw = (env.WORKOS_AUTHORIZATION_SERVER || "").trim(); + if (!raw) return null; + const origin = /^https?:\/\//.test(raw) ? raw : `https://${raw}`; + return origin.replace(/\/+$/, ""); +} + +/** + * OAuth 2.0 Protected Resource Metadata (RFC 9728) — points clients at WorkOS. + * Null when WORKOS_AUTHORIZATION_SERVER is unset: the caller answers 503 + * rather than advertising a guessed host. + */ +export function protectedResourceMetadata( + env: Env, +): Record | null { + const issuer = authorizationServer(env); + if (!issuer) return null; return { resource: resourceUrl(env), - authorization_servers: [ - env.WORKOS_AUTHORIZATION_SERVER || "https://api.workos.com", - ], + authorization_servers: [issuer], bearer_methods_supported: ["header"], // NO scopes_supported, deliberately. // diff --git a/worker/src/env.ts b/worker/src/env.ts index 194b007..e0f351e 100644 --- a/worker/src/env.ts +++ b/worker/src/env.ts @@ -34,7 +34,7 @@ export interface Env { WORKOS_JWKS_URL?: string; // WorkOS Connect app's JWKS endpoint WORKOS_ISSUER?: string; // expected `iss` claim WORKOS_AUDIENCE?: string; // accepted `aud` values, comma-separated - WORKOS_AUTHORIZATION_SERVER?: string; // advertised in resource metadata + WORKOS_AUTHORIZATION_SERVER?: string; // AuthKit issuer advertised in resource metadata + agent card — required secret, no default MCP_RESOURCE_URL?: string; // this resource's canonical URL // ---- Review agent (Workers AI) ------------------------------------------ diff --git a/worker/src/index.ts b/worker/src/index.ts index 6d6dc3a..826d941 100644 --- a/worker/src/index.ts +++ b/worker/src/index.ts @@ -14,6 +14,7 @@ import { GitHubError } from "./github"; import { AuthError, callerIdentity, + AUTHORIZATION_SERVER_MISSING, protectedResourceMetadata, resourceUrl, verifyWorkosToken, @@ -142,13 +143,18 @@ export default { url.pathname === "/.well-known/agent-card.json" || url.pathname === "/.well-known/agent.json" ) { - return json(agentCard(env, url.origin)); + const card = agentCard(env, url.origin); + // Fail closed: the authorization server comes only from configuration. + if (!card) return json({ error: AUTHORIZATION_SERVER_MISSING }, 503); + return json(card); } // OAuth 2.0 Protected Resource Metadata — public, so clients can discover // WorkOS as the authorization server. if (url.pathname === "/.well-known/oauth-protected-resource") { - return json(protectedResourceMetadata(env)); + const meta = protectedResourceMetadata(env); + if (!meta) return json({ error: AUTHORIZATION_SERVER_MISSING }, 503); + return json(meta); } const isA2A = url.pathname === "/a2a"; diff --git a/worker/test/a2a.test.ts b/worker/test/a2a.test.ts index 7bad58f..f101410 100644 --- a/worker/test/a2a.test.ts +++ b/worker/test/a2a.test.ts @@ -20,7 +20,8 @@ import { const env = () => ({ - WORKOS_AUTHORIZATION_SERVER: "https://accounts.mukoko.com", + // Test fixture: the issuer is configuration, never a default in code. + WORKOS_AUTHORIZATION_SERVER: "https://identity.example.test", }) as unknown as Env; const send = (parts: unknown[], over: Record = {}) => ({ @@ -33,7 +34,7 @@ const send = (parts: unknown[], over: Record = {}) => ({ // --- the card ------------------------------------------------------------- test("the card declares the 1.0 protocol version", () => { - const c = agentCard(env(), "https://github.shamwari.ai"); + const c = agentCard(env(), "https://github.shamwari.ai")!; assert.equal(c.protocolVersion, A2A_PROTOCOL_VERSION); assert.equal(c.url, "https://github.shamwari.ai/a2a"); }); @@ -61,12 +62,17 @@ test("the security scheme uses the v1.0 member-name discriminator", () => { assert.ok(schemes.workos.openIdConnectSecurityScheme); assert.match( schemes.workos.openIdConnectSecurityScheme.openIdConnectUrl ?? "", - /accounts\.mukoko\.com\/\.well-known\/openid-configuration/, + /identity\.example\.test\/\.well-known\/openid-configuration/, ); }); +test("the card fails closed when WORKOS_AUTHORIZATION_SERVER is unset", () => { + // No compiled-in authorization server: the caller answers 503. + assert.equal(agentCard({} as unknown as Env, "https://x.test"), null); +}); + test("the card says plainly that it cannot approve", () => { - const c = agentCard(env(), "https://x.test"); + const c = agentCard(env(), "https://x.test")!; assert.match(String(c.description), /[Cc]annot approve/); }); diff --git a/worker/test/policy.test.ts b/worker/test/policy.test.ts index 8a1f2ba..43d9a99 100644 --- a/worker/test/policy.test.ts +++ b/worker/test/policy.test.ts @@ -164,13 +164,21 @@ test("the metadata advertises no scopes", () => { // scope=github:read -> 302 ?error=invalid_scope const meta = protectedResourceMetadata({ MCP_RESOURCE_URL: "https://github.shamwari.ai/mcp", - WORKOS_AUTHORIZATION_SERVER: "https://accounts.mukoko.com", + WORKOS_AUTHORIZATION_SERVER: "https://identity.example.test/", } as Env); + assert.ok(meta); assert.equal("scopes_supported" in meta, false); - assert.deepEqual(meta.authorization_servers, ["https://accounts.mukoko.com"]); + assert.deepEqual(meta.authorization_servers, ["https://identity.example.test"]); assert.equal(meta.resource, "https://github.shamwari.ai/mcp"); }); +test("resource metadata fails closed when WORKOS_AUTHORIZATION_SERVER is unset", () => { + assert.equal( + protectedResourceMetadata({ MCP_RESOURCE_URL: "https://github.shamwari.ai/mcp" } as Env), + null, + ); +}); + // --- tool annotations ----------------------------------------------------- // // Hints, not enforcement — the real guarantees are the scoped token and diff --git a/worker/wrangler.toml b/worker/wrangler.toml index 2cae01a..44219dd 100644 --- a/worker/wrangler.toml +++ b/worker/wrangler.toml @@ -49,11 +49,11 @@ GITHUB_ALLOWED_REPOS = "nyuchi/web-services,nyuchi/api-gateway,nyuchi/mukoko-pla # App and the installation re-authorised. `github_whoami` reports the gap. GITHUB_TOKEN_PERMISSIONS = "pull_requests:write,issues:write,contents:read,metadata:read" -# The WorkOS AuthKit issuer — the OAuth 2.1 authorization server MCP clients -# discover from our resource metadata and run their PKCE flow against. Same -# host nyuchi-fly-mcp uses; see that file for why the three sibling hosts -# (auth.mukoko.com, api.nyuchi.com, api.mukoko.com) are NOT this. -WORKOS_AUTHORIZATION_SERVER = "https://accounts.mukoko.com" +# WORKOS_AUTHORIZATION_SERVER — the WorkOS AuthKit issuer MCP clients discover +# from our resource metadata — is deliberately NOT a var here. It is REQUIRED +# and set per environment as a secret (see the list below); never committed, +# no default in code. The resource metadata and agent card answer 503 until it +# is set. MCP_RESOURCE_URL = "https://github.shamwari.ai/mcp" # Authorization policy (public, versioned for auditability). @@ -115,8 +115,9 @@ REVIEW_ENABLED = "true" # GitHub hands you) or PKCS#8 both work — the worker # wraps PKCS#1 itself, so this is byte-identical to # RELEASE_APP_PRIVATE_KEY with no conversion. -# WORKOS_JWKS_URL https://accounts.mukoko.com/oauth2/jwks -# WORKOS_ISSUER https://accounts.mukoko.com +# WORKOS_AUTHORIZATION_SERVER the AuthKit domain (https origin) +# WORKOS_JWKS_URL /oauth2/jwks +# WORKOS_ISSUER # WORKOS_AUDIENCE https://github.shamwari.ai/mcp,client_01KVTX0V2K1VM3PSC0DJ9VZWTV # # WORKOS_AUDIENCE takes a comma-separated list because `aud` depends on how @@ -126,7 +127,7 @@ REVIEW_ENABLED = "true" # accepts either and still rejects tokens minted for anything else. # # Those three are not guesses: they are the `jwks_uri` and `issuer` that -# https://accounts.mukoko.com/.well-known/oauth-authorization-server serves, +# /.well-known/oauth-authorization-server serves, # and the resource URI below. Do NOT use the older SSO form # https://api.workos.com/sso/jwks/ — AuthKit access tokens here are # signed by the authorization server above, so that JWKS has no matching key From c65473efe256c22aba3d453d050d4c984b7003a7 Mon Sep 17 00:00:00 2001 From: Bryan Fawcett Date: Sat, 3 Oct 2026 03:28:41 +0800 Subject: [PATCH 2/3] fix: strip trailing slashes without a regex Clears CodeQL js/polynomial-redos on the issuer normaliser. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GHjaH1Vt2dVLWU68iz8tFq --- worker/src/auth.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/worker/src/auth.ts b/worker/src/auth.ts index 9cc5fec..b7b45f7 100644 --- a/worker/src/auth.ts +++ b/worker/src/auth.ts @@ -283,8 +283,10 @@ export const AUTHORIZATION_SERVER_MISSING = export function authorizationServer(env: Env): string | null { const raw = (env.WORKOS_AUTHORIZATION_SERVER || "").trim(); if (!raw) return null; - const origin = /^https?:\/\//.test(raw) ? raw : `https://${raw}`; - return origin.replace(/\/+$/, ""); + let origin = /^https?:\/\//.test(raw) ? raw : `https://${raw}`; + // Strip trailing slashes without a regex (no backtracking on long input). + while (origin.endsWith("/")) origin = origin.slice(0, -1); + return origin; } /** From f541580702fa2e0ef91bb2f54487ced177390494 Mon Sep 17 00:00:00 2001 From: Bryan Fawcett Date: Sat, 3 Oct 2026 03:49:45 +0800 Subject: [PATCH 3/3] fix(auth): parse WORKOS_AUTHORIZATION_SERVER into an https origin Parse the advertised AuthKit issuer with URL rather than prepending a scheme to a string. A bare host or an https origin (any case) is accepted; path, query and fragment are dropped; http, other schemes, credentials and unparseable values are rejected and treated as unset, so the resource metadata and agent card answer 503. The agent card's OpenID Connect URL is built with new URL. A correctly configured https value resolves to the same origin as before. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GHjaH1Vt2dVLWU68iz8tFq --- worker/src/a2a.ts | 5 +++- worker/src/auth.ts | 45 +++++++++++++++++++++++++++------ worker/test/a2a.test.ts | 33 ++++++++++++++++++++++++ worker/test/policy.test.ts | 52 +++++++++++++++++++++++++++++++++++--- 4 files changed, 123 insertions(+), 12 deletions(-) diff --git a/worker/src/a2a.ts b/worker/src/a2a.ts index 4a5ce7c..c9d532d 100644 --- a/worker/src/a2a.ts +++ b/worker/src/a2a.ts @@ -81,7 +81,10 @@ export function agentCard( securitySchemes: { workos: { openIdConnectSecurityScheme: { - openIdConnectUrl: `${authServer}/.well-known/openid-configuration`, + openIdConnectUrl: new URL( + "/.well-known/openid-configuration", + authServer, + ).href, }, }, }, diff --git a/worker/src/auth.ts b/worker/src/auth.ts index b7b45f7..43f6765 100644 --- a/worker/src/auth.ts +++ b/worker/src/auth.ts @@ -275,18 +275,47 @@ export function resourceUrl(env: Env): string { export const AUTHORIZATION_SERVER_MISSING = "WORKOS_AUTHORIZATION_SERVER is not configured"; +/** + * Parse — never concatenate — a configured AuthKit domain into an https origin. + * + * Accepts a bare host or an https origin, in any case. Any path, query or + * fragment is dropped. A blank value, `http:`, any other scheme, embedded + * credentials and anything `URL` cannot parse all throw an error whose message + * starts with `AUTHORIZATION_SERVER_MISSING`. The result is `URL.origin`. + */ +export function normaliseAuthkitDomain(value: string | undefined): string { + const raw = value?.trim(); + if (!raw) throw new Error(AUTHORIZATION_SERVER_MISSING); + let url: URL; + try { + url = new URL( + /^[a-z][a-z0-9+.-]*:\/\//i.test(raw) ? raw : `https://${raw}`, + ); + } catch { + throw new Error( + `${AUTHORIZATION_SERVER_MISSING} (not a valid host or URL)`, + ); + } + if (url.protocol !== "https:" || url.username || url.password) { + throw new Error( + `${AUTHORIZATION_SERVER_MISSING} (must be an https origin)`, + ); + } + return url.origin; +} + /** * The AuthKit issuer to advertise, from configuration only — there is no - * compiled-in default and no fallback host. Accepts a bare host or an https - * origin; trims whitespace and any trailing slash. Null when unset. + * compiled-in default and no fallback host. Parsed by `normaliseAuthkitDomain`. + * Null when unset or unusable (http, another scheme, credentials, + * unparseable): the metadata and agent-card routes then answer 503. */ export function authorizationServer(env: Env): string | null { - const raw = (env.WORKOS_AUTHORIZATION_SERVER || "").trim(); - if (!raw) return null; - let origin = /^https?:\/\//.test(raw) ? raw : `https://${raw}`; - // Strip trailing slashes without a regex (no backtracking on long input). - while (origin.endsWith("/")) origin = origin.slice(0, -1); - return origin; + try { + return normaliseAuthkitDomain(env.WORKOS_AUTHORIZATION_SERVER); + } catch { + return null; + } } /** diff --git a/worker/test/a2a.test.ts b/worker/test/a2a.test.ts index f101410..c07871b 100644 --- a/worker/test/a2a.test.ts +++ b/worker/test/a2a.test.ts @@ -71,6 +71,39 @@ test("the card fails closed when WORKOS_AUTHORIZATION_SERVER is unset", () => { assert.equal(agentCard({} as unknown as Env, "https://x.test"), null); }); +test("the card fails closed when WORKOS_AUTHORIZATION_SERVER is not an https origin", () => { + for (const bad of [ + "http://identity.example.test", + "https://user:pass@identity.example.test", + ]) { + assert.equal( + agentCard( + { WORKOS_AUTHORIZATION_SERVER: bad } as unknown as Env, + "https://x.test", + ), + null, + bad, + ); + } +}); + +test("the card builds its OpenID Connect URL on the parsed origin", () => { + const c = agentCard( + { + WORKOS_AUTHORIZATION_SERVER: "HTTPS://Identity.Example.Test/some/path", + } as unknown as Env, + "https://x.test", + )!; + const schemes = c.securitySchemes as unknown as Record< + string, + Record + >; + assert.equal( + schemes.workos.openIdConnectSecurityScheme.openIdConnectUrl, + "https://identity.example.test/.well-known/openid-configuration", + ); +}); + test("the card says plainly that it cannot approve", () => { const c = agentCard(env(), "https://x.test")!; assert.match(String(c.description), /[Cc]annot approve/); diff --git a/worker/test/policy.test.ts b/worker/test/policy.test.ts index 43d9a99..4957a76 100644 --- a/worker/test/policy.test.ts +++ b/worker/test/policy.test.ts @@ -4,7 +4,11 @@ import { test } from "node:test"; import assert from "node:assert/strict"; import type { Env } from "../src/env"; -import { protectedResourceMetadata } from "../src/auth"; +import { + authorizationServer, + normaliseAuthkitDomain, + protectedResourceMetadata, +} from "../src/auth"; import { createReview, missingPermissions, resolveRepo } from "../src/github"; const env = (over: Partial = {}): Env => @@ -168,17 +172,59 @@ test("the metadata advertises no scopes", () => { } as Env); assert.ok(meta); assert.equal("scopes_supported" in meta, false); - assert.deepEqual(meta.authorization_servers, ["https://identity.example.test"]); + assert.deepEqual(meta.authorization_servers, [ + "https://identity.example.test", + ]); assert.equal(meta.resource, "https://github.shamwari.ai/mcp"); }); test("resource metadata fails closed when WORKOS_AUTHORIZATION_SERVER is unset", () => { assert.equal( - protectedResourceMetadata({ MCP_RESOURCE_URL: "https://github.shamwari.ai/mcp" } as Env), + protectedResourceMetadata({ + MCP_RESOURCE_URL: "https://github.shamwari.ai/mcp", + } as Env), null, ); }); +test("the advertised authorization server is parsed into an https origin", () => { + const at = (v: string | undefined) => + authorizationServer({ WORKOS_AUTHORIZATION_SERVER: v } as Env); + assert.equal(at(undefined), null); + assert.equal(at(" "), null); + assert.equal( + at("https://identity.example.test"), + "https://identity.example.test", + ); + assert.equal(at("identity.example.test"), "https://identity.example.test"); + assert.equal( + at("HTTPS://Identity.Example.Test"), + "https://identity.example.test", + ); + assert.equal( + at("https://identity.example.test/x/y?z=1#f"), + "https://identity.example.test", + ); + for (const bad of [ + "http://identity.example.test", + "javascript://identity.example.test", + "https://user:pass@identity.example.test", + "user@identity.example.test", + "https://", + ]) { + assert.equal(at(bad), null, bad); + assert.equal( + protectedResourceMetadata({ WORKOS_AUTHORIZATION_SERVER: bad } as Env), + null, + bad, + ); + assert.throws( + () => normaliseAuthkitDomain(bad), + /WORKOS_AUTHORIZATION_SERVER is not configured/, + ); + } +}); + // --- tool annotations ----------------------------------------------------- // // Hints, not enforcement — the real guarantees are the scoped token and