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..c9d532d 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: @@ -74,7 +81,10 @@ export function agentCard(env: Env, baseUrl: string): Record { 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 32594d6..43f6765 100644 --- a/worker/src/auth.ts +++ b/worker/src/auth.ts @@ -272,13 +272,65 @@ 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"; + +/** + * 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. 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 { + try { + return normaliseAuthkitDomain(env.WORKOS_AUTHORIZATION_SERVER); + } catch { + return null; + } +} + +/** + * 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..c07871b 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,50 @@ 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 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"); + 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..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 => @@ -164,13 +168,63 @@ 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, + ); +}); + +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 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