From 4c87a2d90d36804410ee82dfab4de8ecf7619c76 Mon Sep 17 00:00:00 2001 From: Bryan Fawcett Date: Sat, 3 Oct 2026 03:12:11 +0800 Subject: [PATCH 1/2] fix: read the AuthKit issuer only from configuration Remove WORKOS_ISSUER = "https://accounts.mukoko.com" from the [vars] of both site/wrangler.toml (nyuchi-docs) and nyuchi-docs-mcp-worker/wrangler.toml (nyuchi-docs-mcp). The issuer is a required Worker secret set per environment. The site gate answers 503 "WORKOS_ISSUER is not configured" for internal pages and the OAuth callback when it is missing; the MCP worker already fails closed (no token verifies, internal content stays hidden). Both normalise the value (bare host or https origin, no trailing slash). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GHjaH1Vt2dVLWU68iz8tFq --- nyuchi-docs-mcp-worker/src/auth.ts | 21 ++++++++++-- nyuchi-docs-mcp-worker/src/worker.ts | 2 +- nyuchi-docs-mcp-worker/wrangler.toml | 16 ++++----- site/src/worker/gate.ts | 49 +++++++++++++++++++++------- site/wrangler.toml | 16 ++++----- 5 files changed, 70 insertions(+), 34 deletions(-) diff --git a/nyuchi-docs-mcp-worker/src/auth.ts b/nyuchi-docs-mcp-worker/src/auth.ts index 8e1b276..87eed45 100644 --- a/nyuchi-docs-mcp-worker/src/auth.ts +++ b/nyuchi-docs-mcp-worker/src/auth.ts @@ -17,11 +17,28 @@ import { createRemoteJWKSet, jwtVerify } from 'jose'; let jwks: ReturnType | undefined; let jwksIssuer: string | undefined; +/** + * The AuthKit issuer origin, from configuration only (WORKOS_ISSUER) — no + * compiled-in default. Accepts a bare host or an https origin; trims + * whitespace and any trailing slash. Null when unset. + */ +export function normaliseIssuer(raw: string | undefined): string | null { + const value = (raw ?? '').trim(); + if (!value) return null; + const origin = /^https?:\/\//.test(value) ? value : `https://${value}`; + return origin.replace(/\/+$/, ''); +} + export async function verifyBearerAuth( req: Request, - issuer: string | undefined + configuredIssuer: string | undefined ): Promise<{ authorized: boolean; subject?: string }> { - if (!issuer) return { authorized: false }; + const issuer = normaliseIssuer(configuredIssuer); + if (!issuer) { + // Fail closed: without a configured issuer no token can be verified. + if (req.headers.has('authorization')) console.error('[auth] WORKOS_ISSUER is not configured'); + return { authorized: false }; + } const header = req.headers.get('authorization') ?? ''; const match = header.match(/^Bearer\s+(.+)$/i); if (!match) return { authorized: false }; diff --git a/nyuchi-docs-mcp-worker/src/worker.ts b/nyuchi-docs-mcp-worker/src/worker.ts index aa5af22..77021ef 100644 --- a/nyuchi-docs-mcp-worker/src/worker.ts +++ b/nyuchi-docs-mcp-worker/src/worker.ts @@ -69,7 +69,7 @@ export interface Env { FEEDBACK?: FeedbackStore; /** Optional secret — when set, raise_issue files real GitHub issues. */ GITHUB_TOKEN?: string; - /** WorkOS issuer used to verify a caller's own bearer token (see src/auth.ts). Unset = every caller is treated as unauthenticated (public-only). */ + /** WorkOS AuthKit issuer used to verify a caller's own bearer token (see src/auth.ts). REQUIRED, set per environment as a secret — never committed, no default. Unset = every caller is treated as unauthenticated (public-only; fails closed). */ WORKOS_ISSUER?: string; /** Shared with nyuchi-docs's site worker — sent on internal-page fetches once a caller is verified, so the read skips the browser OIDC flow. */ INTERNAL_FETCH_KEY?: string; diff --git a/nyuchi-docs-mcp-worker/wrangler.toml b/nyuchi-docs-mcp-worker/wrangler.toml index d73de0f..09e8e39 100644 --- a/nyuchi-docs-mcp-worker/wrangler.toml +++ b/nyuchi-docs-mcp-worker/wrangler.toml @@ -15,16 +15,12 @@ routes = [ [vars] TOP_K = "5" ALLOWED_ORIGINS = "https://docs.nyuchi.com,https://docs.bundu.org" -# The WorkOS **AuthKit issuer** — the OAuth 2.1 authorization server whose -# /.well-known/openid-configuration + /oauth2/jwks verify a caller's own -# bearer token (src/auth.ts); this worker never originates a login flow. -# It is NOT auth.mukoko.com (the WorkOS *auth API* host, which serves no -# authorization-server metadata), and neither api.nyuchi.com nor -# api.mukoko.com is WorkOS at all. Not a secret — an OIDC issuer URL is -# public by design — but keep it in sync with the same value the site -# worker uses. The pre-Aug-2026 value identity.nyuchi.com has no DNS -# record any more; do not restore it. -WORKOS_ISSUER = "https://accounts.mukoko.com" +# WORKOS_ISSUER — the WorkOS **AuthKit issuer** whose /oauth2/jwks verifies a +# caller's own bearer token (src/auth.ts) — is deliberately NOT a var here. It +# is REQUIRED and set per environment as a secret (`wrangler secret put +# WORKOS_ISSUER`; owner script, 1Password nyuchi/workos), never committed, no +# default in code. Until it is set no bearer token verifies, so internal +# content stays hidden (fail closed); public docs still answer. # Same AI Search instance the Ask-AI tab uses — read tools ride the # existing nyuchi-docs corpus; this worker adds no ingestion of its own. diff --git a/site/src/worker/gate.ts b/site/src/worker/gate.ts index 8bb26ea..63f7ed8 100644 --- a/site/src/worker/gate.ts +++ b/site/src/worker/gate.ts @@ -24,7 +24,12 @@ import { securityTxtResponse } from './security-txt.js'; export interface Env { ASSETS: Fetcher; WORKOS_CLIENT_ID: string; - WORKOS_ISSUER: string; + /** + * The WorkOS AuthKit issuer. REQUIRED, set per environment as a Worker + * secret — never committed, no default in code. Internal pages answer 503 + * "WORKOS_ISSUER is not configured" until it is set. + */ + WORKOS_ISSUER?: string; /** * Optional: only present for a confidential Connect app. The app this * gate actually uses — WorkOS's shared "Nyuchi Internal Tools" app, @@ -79,19 +84,35 @@ async function pkcePair(): Promise<{ verifier: string; challenge: string }> { return { verifier, challenge }; } +const ISSUER_MISSING = 'WORKOS_ISSUER is not configured'; + +/** + * The AuthKit issuer origin, from configuration only — no compiled-in default. + * Accepts a bare host or an https origin; trims whitespace and any trailing + * slash (the `iss` check is an exact string match). Null when unset. + */ +function issuerOf(env: Env): string | null { + const raw = (env.WORKOS_ISSUER ?? '').trim(); + if (!raw) return null; + const origin = /^https?:\/\//.test(raw) ? raw : `https://${raw}`; + return origin.replace(/\/+$/, ''); +} + // jose's remote JWKS caches keys internally; module-level so it survives // across requests to the same isolate instead of re-fetching every time. let jwks: ReturnType | undefined; let jwksIssuer: string | undefined; async function verifySession(env: Env, token: string): Promise { + const issuer = issuerOf(env); + if (!issuer) return null; try { - if (!jwks || jwksIssuer !== env.WORKOS_ISSUER) { - jwks = createRemoteJWKSet(new URL(`${env.WORKOS_ISSUER}/oauth2/jwks`)); - jwksIssuer = env.WORKOS_ISSUER; + if (!jwks || jwksIssuer !== issuer) { + jwks = createRemoteJWKSet(new URL(`${issuer}/oauth2/jwks`)); + jwksIssuer = issuer; } const { payload } = await jwtVerify(token, jwks, { - issuer: env.WORKOS_ISSUER, + issuer, audience: env.WORKOS_CLIENT_ID, }); return payload; @@ -100,8 +121,8 @@ async function verifySession(env: Env, token: string): Promise { +async function handleCallback(issuer: string, env: Env, req: Request, url: URL): Promise { const cookies = parseCookies(req); const raw = cookies[OAUTH_STATE_COOKIE]; if (!raw) return new Response('Login expired — go back and try again.', { status: 400 }); @@ -147,7 +168,7 @@ async function handleCallback(env: Env, req: Request, url: URL): Promise; diff --git a/site/wrangler.toml b/site/wrangler.toml index 2ba16d5..bebbfda 100644 --- a/site/wrangler.toml +++ b/site/wrangler.toml @@ -35,16 +35,12 @@ enabled = true # https://docs.nyuchi.com/oauth/callback is registered on that app. [vars] WORKOS_CLIENT_ID = "client_01KVTX0V2K1VM3PSC0DJ9VZWTV" -# accounts.mukoko.com is the WorkOS **AuthKit issuer** — the OAuth 2.1 -# authorization server serving /.well-known/openid-configuration, -# /oauth2/authorize, /oauth2/token and /oauth2/jwks (what gate.ts calls). -# It is NOT auth.mukoko.com (the WorkOS *auth API* / JWKS-fetch host, which -# serves no authorization-server metadata), and neither api.nyuchi.com (the -# Nyuchi API gateway) nor api.mukoko.com (a separate gateway, still being -# built) is WorkOS at all. The pre-Aug-2026 value identity.nyuchi.com has no -# DNS record any more — do not restore it; a dead issuer makes every -# `visibility: internal` page unsignable-into. -WORKOS_ISSUER = "https://accounts.mukoko.com" +# WORKOS_ISSUER — the WorkOS **AuthKit issuer** (serves +# /.well-known/openid-configuration, /oauth2/authorize, /oauth2/token and +# /oauth2/jwks, which gate.ts calls) — is deliberately NOT a var here. It is +# REQUIRED and set per environment as a secret (`wrangler secret put +# WORKOS_ISSUER`; owner script, 1Password nyuchi/workos), never committed, no +# default in code. `visibility: internal` pages answer 503 until it is set. # Secret (wrangler secret put INTERNAL_FETCH_KEY): shared with # nyuchi-docs-mcp-worker so it can read internal pages on behalf of a From 4888e38a38387fc6a01bb08e7accd65ad399a740 Mon Sep 17 00:00:00 2001 From: Bryan Fawcett Date: Sat, 3 Oct 2026 03:51:01 +0800 Subject: [PATCH 2/2] fix(auth): parse WORKOS_ISSUER into an https origin Parse the configured AuthKit issuer with URL rather than prepending a scheme to a string, in both the MCP worker and the site gate. 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 treated as unset, so the MCP worker treats callers as unauthenticated and the gate answers 503. The JWKS, authorize and token URLs are built with new URL, and iss is bound by exact match against the origin. 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 --- nyuchi-docs-mcp-worker/src/auth.ts | 25 ++++-- nyuchi-docs-mcp-worker/tests/issuer.test.ts | 88 +++++++++++++++++++++ site/src/worker/gate.ts | 37 ++++++--- 3 files changed, 133 insertions(+), 17 deletions(-) create mode 100644 nyuchi-docs-mcp-worker/tests/issuer.test.ts diff --git a/nyuchi-docs-mcp-worker/src/auth.ts b/nyuchi-docs-mcp-worker/src/auth.ts index 87eed45..52470a9 100644 --- a/nyuchi-docs-mcp-worker/src/auth.ts +++ b/nyuchi-docs-mcp-worker/src/auth.ts @@ -18,15 +18,26 @@ let jwks: ReturnType | undefined; let jwksIssuer: string | undefined; /** - * The AuthKit issuer origin, from configuration only (WORKOS_ISSUER) — no - * compiled-in default. Accepts a bare host or an https origin; trims - * whitespace and any trailing slash. Null when unset. + * Parse — never concatenate — the configured AuthKit issuer (WORKOS_ISSUER) + * into an https origin. No compiled-in default. + * + * Accepts a bare host or an https origin, in any case. Any path, query or + * fragment is dropped. A blank value is null (unset); `http:`, any other + * scheme, embedded credentials and anything `URL` cannot parse are also null — + * an unusable issuer fails closed exactly like a missing one. The result is + * `URL.origin`, which the `iss` claim must equal exactly. */ export function normaliseIssuer(raw: string | undefined): string | null { const value = (raw ?? '').trim(); if (!value) return null; - const origin = /^https?:\/\//.test(value) ? value : `https://${value}`; - return origin.replace(/\/+$/, ''); + let url: URL; + try { + url = new URL(/^[a-z][a-z0-9+.-]*:\/\//i.test(value) ? value : `https://${value}`); + } catch { + return null; + } + if (url.protocol !== 'https:' || url.username || url.password) return null; + return url.origin; } export async function verifyBearerAuth( @@ -36,7 +47,7 @@ export async function verifyBearerAuth( const issuer = normaliseIssuer(configuredIssuer); if (!issuer) { // Fail closed: without a configured issuer no token can be verified. - if (req.headers.has('authorization')) console.error('[auth] WORKOS_ISSUER is not configured'); + if (req.headers.has('authorization')) console.error('[auth] WORKOS_ISSUER is not configured (or is not an https origin)'); return { authorized: false }; } const header = req.headers.get('authorization') ?? ''; @@ -45,7 +56,7 @@ export async function verifyBearerAuth( try { if (!jwks || jwksIssuer !== issuer) { - jwks = createRemoteJWKSet(new URL(`${issuer}/oauth2/jwks`)); + jwks = createRemoteJWKSet(new URL('/oauth2/jwks', issuer)); jwksIssuer = issuer; } const { payload } = await jwtVerify(match[1], jwks, { issuer }); diff --git a/nyuchi-docs-mcp-worker/tests/issuer.test.ts b/nyuchi-docs-mcp-worker/tests/issuer.test.ts new file mode 100644 index 0000000..fb0fca7 --- /dev/null +++ b/nyuchi-docs-mcp-worker/tests/issuer.test.ts @@ -0,0 +1,88 @@ +// WORKOS_ISSUER is parsed — never concatenated — into an https origin, and a +// token's `iss` must equal that origin exactly. The site gate keeps an +// identical copy of the normaliser; both are exercised here. +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { SignJWT, exportJWK, generateKeyPair } from 'jose'; +import { normaliseIssuer, verifyBearerAuth } from '../src/auth.js'; +import { normaliseIssuer as gateNormaliseIssuer } from '../../site/src/worker/gate.js'; + +for (const [name, normalise] of [ + ['mcp worker', normaliseIssuer], + ['site gate', gateNormaliseIssuer], +] as const) { + describe(`normaliseIssuer (${name})`, () => { + it('is null when unset or blank', () => { + expect(normalise(undefined)).toBeNull(); + expect(normalise(' ')).toBeNull(); + }); + + it('accepts a bare host or an https origin in any case, and returns the origin', () => { + expect(normalise('https://identity.example.test')).toBe('https://identity.example.test'); + expect(normalise('identity.example.test')).toBe('https://identity.example.test'); + expect(normalise('https://identity.example.test/')).toBe('https://identity.example.test'); + expect(normalise('HTTPS://Identity.Example.Test')).toBe('https://identity.example.test'); + }); + + it('drops any path, query or fragment', () => { + expect(normalise('https://identity.example.test/x/y?z=1#f')).toBe('https://identity.example.test'); + }); + + it('treats anything that is not an https origin as unconfigured', () => { + for (const bad of [ + 'http://identity.example.test', + 'javascript://identity.example.test', + 'https://user:pass@identity.example.test', + 'user@identity.example.test', + 'https://', + ]) { + expect(normalise(bad), bad).toBeNull(); + } + }); + }); +} + +describe('verifyBearerAuth binds iss to the normalised origin', () => { + afterEach(() => vi.unstubAllGlobals()); + + it('accepts iss equal to the origin and rejects prefix, slash and http variants', async () => { + const { publicKey, privateKey } = await generateKeyPair('RS256'); + const jwk = { ...(await exportJWK(publicKey)), kid: 'k1', alg: 'RS256', use: 'sig' }; + const fetched: string[] = []; + vi.stubGlobal('fetch', async (input: RequestInfo | URL) => { + fetched.push(typeof input === 'string' ? input : input instanceof URL ? input.href : input.url); + return new Response(JSON.stringify({ keys: [jwk] }), { headers: { 'content-type': 'application/json' } }); + }); + const sign = (iss: string) => + new SignJWT({}) + .setProtectedHeader({ alg: 'RS256', kid: 'k1' }) + .setIssuer(iss) + .setSubject('user_123') + .setIssuedAt() + .setExpirationTime('5m') + .sign(privateKey); + const req = async (iss: string) => + new Request('https://docs.test/mcp', { headers: { authorization: `Bearer ${await sign(iss)}` } }); + const configured = 'HTTPS://Iss-Bind.Example.Test/some/path'; + + expect(await verifyBearerAuth(await req('https://iss-bind.example.test'), configured)).toEqual({ + authorized: true, + subject: 'user_123', + }); + expect(fetched).toEqual(['https://iss-bind.example.test/oauth2/jwks']); + for (const iss of [ + 'https://iss-bind.example.test.evil.test', + 'https://iss-bind.example.test/', + 'http://iss-bind.example.test', + ]) { + expect(await verifyBearerAuth(await req(iss), configured), iss).toEqual({ authorized: false }); + } + }); + + it('fails closed on an issuer that is not an https origin', async () => { + const res = await verifyBearerAuth( + new Request('https://docs.test/mcp', { headers: { authorization: 'Bearer a.b.c' } }), + 'http://iss-bind.example.test' + ); + expect(res).toEqual({ authorized: false }); + }); +}); diff --git a/site/src/worker/gate.ts b/site/src/worker/gate.ts index 63f7ed8..d255b40 100644 --- a/site/src/worker/gate.ts +++ b/site/src/worker/gate.ts @@ -86,16 +86,33 @@ async function pkcePair(): Promise<{ verifier: string; challenge: string }> { const ISSUER_MISSING = 'WORKOS_ISSUER is not configured'; +// Kept identical to normaliseIssuer in nyuchi-docs-mcp-worker/src/auth.ts +// (that package's tests exercise both copies). /** - * The AuthKit issuer origin, from configuration only — no compiled-in default. - * Accepts a bare host or an https origin; trims whitespace and any trailing - * slash (the `iss` check is an exact string match). Null when unset. + * Parse — never concatenate — the configured AuthKit issuer (WORKOS_ISSUER) + * into an https origin. No compiled-in default. + * + * Accepts a bare host or an https origin, in any case. Any path, query or + * fragment is dropped. A blank value is null (unset); `http:`, any other + * scheme, embedded credentials and anything `URL` cannot parse are also null — + * an unusable issuer fails closed exactly like a missing one. The result is + * `URL.origin`, which the `iss` claim must equal exactly. */ +export function normaliseIssuer(raw: string | undefined): string | null { + const value = (raw ?? '').trim(); + if (!value) return null; + let url: URL; + try { + url = new URL(/^[a-z][a-z0-9+.-]*:\/\//i.test(value) ? value : `https://${value}`); + } catch { + return null; + } + if (url.protocol !== 'https:' || url.username || url.password) return null; + return url.origin; +} + function issuerOf(env: Env): string | null { - const raw = (env.WORKOS_ISSUER ?? '').trim(); - if (!raw) return null; - const origin = /^https?:\/\//.test(raw) ? raw : `https://${raw}`; - return origin.replace(/\/+$/, ''); + return normaliseIssuer(env.WORKOS_ISSUER); } // jose's remote JWKS caches keys internally; module-level so it survives @@ -108,7 +125,7 @@ async function verifySession(env: Env, token: string): Promise