From d0f8948f3efd1636efdfa6324970603afa10f98a Mon Sep 17 00:00:00 2001 From: baladithyab Date: Wed, 27 May 2026 22:24:08 -0700 Subject: [PATCH] feat(embed): pin OIDC job_workflow_ref + scope discovery to user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two hardening changes that close the last open trust boundaries on the project-embed pipeline: 1. **OIDC job_workflow_ref pin** (src/lib/github-oidc.ts): authorizeForEmbedUpload now requires job_workflow_ref to start with 'baladithyab/web-embed-workflows/.github/workflows/static-passthrough.yml@'. Previously, any caller workflow under repository_owner === 'baladithyab' with id-token: write could mint a valid OIDC token and upload to any slug. Now ONLY our reviewed reusable workflow can — even if some unrelated workflow in one of our repos later sets id-token: write. Six new tests cover: missing claim, wrong workflow file, wrong repo, ref-as-SHA, ref-as-tag, prefix-substring forgery. 2. **Discovery scope to ALLOWED_OWNER** (src/scripts/sync-project-manifests.ts): GitHub search query is now 'topic:codeseys-embed user:baladithyab'. Strangers adding the topic no longer leak into /projects. Defense-in-depth: validate() now also rejects manifests whose source owner doesn't match. 184 / 184 tests passing (was 178; +6 OIDC pin tests). Astro check clean. --- src/lib/github-oidc.test.ts | 69 ++++++++++++++++++++++++++- src/lib/github-oidc.ts | 56 +++++++++++++++++++--- src/scripts/sync-project-manifests.ts | 29 ++++++++++- 3 files changed, 146 insertions(+), 8 deletions(-) diff --git a/src/lib/github-oidc.test.ts b/src/lib/github-oidc.test.ts index e1fed4d..4975a7c 100644 --- a/src/lib/github-oidc.test.ts +++ b/src/lib/github-oidc.test.ts @@ -246,7 +246,7 @@ describe('verifyGithubOidc', () => { }) describe('authorizeForEmbedUpload', () => { - it('accepts our owner', () => { + it('accepts our owner with the right job_workflow_ref', () => { const r = authorizeForEmbedUpload(makeClaims()) expect(r.ok).toBe(true) }) @@ -266,4 +266,71 @@ describe('authorizeForEmbedUpload', () => { const r = authorizeForEmbedUpload(claims) expect(r.ok).toBe(false) }) + + it('rejects when job_workflow_ref is missing', () => { + const claims = makeClaims() + delete claims.job_workflow_ref + const r = authorizeForEmbedUpload(claims) + expect(r.ok).toBe(false) + if (!r.ok) { + expect(r.status).toBe(403) + expect(r.message).toMatch(/job_workflow_ref/) + } + }) + + it('rejects when job_workflow_ref points to a different reusable workflow', () => { + const r = authorizeForEmbedUpload( + makeClaims({ + job_workflow_ref: 'baladithyab/some-other-repo/.github/workflows/evil.yml@refs/heads/main', + }) + ) + expect(r.ok).toBe(false) + if (!r.ok) { + expect(r.status).toBe(403) + expect(r.message).toMatch(/not from the allowed reusable workflow/) + } + }) + + it('rejects when job_workflow_ref names a different workflow file in our repo', () => { + const r = authorizeForEmbedUpload( + makeClaims({ + job_workflow_ref: + 'baladithyab/web-embed-workflows/.github/workflows/some-other.yml@refs/heads/main', + }) + ) + expect(r.ok).toBe(false) + if (!r.ok) expect(r.message).toMatch(/not from the allowed reusable workflow/) + }) + + it('accepts when job_workflow_ref pins to a SHA instead of a branch', () => { + const r = authorizeForEmbedUpload( + makeClaims({ + job_workflow_ref: + 'baladithyab/web-embed-workflows/.github/workflows/static-passthrough.yml@deadbeefcafebabe1234567890abcdef', + }) + ) + expect(r.ok).toBe(true) + }) + + it('accepts when job_workflow_ref pins to a tag', () => { + const r = authorizeForEmbedUpload( + makeClaims({ + job_workflow_ref: + 'baladithyab/web-embed-workflows/.github/workflows/static-passthrough.yml@refs/tags/v1.0.0', + }) + ) + expect(r.ok).toBe(true) + }) + + it('rejects a forged job_workflow_ref that starts with our prefix as a substring', () => { + // Defense against e.g. 'evil-baladithyab/web-embed-workflows/...'. + // startsWith() catches this — but document it. + const r = authorizeForEmbedUpload( + makeClaims({ + job_workflow_ref: + 'evil-prefix-baladithyab/web-embed-workflows/.github/workflows/static-passthrough.yml@refs/heads/main', + }) + ) + expect(r.ok).toBe(false) + }) }) diff --git a/src/lib/github-oidc.ts b/src/lib/github-oidc.ts index b37fc15..00f1d19 100644 --- a/src/lib/github-oidc.ts +++ b/src/lib/github-oidc.ts @@ -284,16 +284,42 @@ export type OidcAuthzResult = | { ok: true; claims: GithubOidcClaims } | { ok: false; status: 403; message: string } +/** + * Workflow pin: only OIDC tokens minted by this exact reusable workflow + * are allowed to upload. Format matches the `job_workflow_ref` JWT claim: + * + * //.github/workflows/@ + * + * GitHub stamps `job_workflow_ref` with whatever ref the reusable + * workflow was called from — branch (`refs/heads/main`), tag, or SHA. + * We pin the prefix (everything before the `@`) so any ref of the + * audited reusable workflow passes, but no other workflow does. + * + * Why this matters: without this pin, any caller workflow under + * `repository_owner === ALLOWED_OWNER` with `id-token: write` could mint + * a valid OIDC token and upload to any slug. With the pin, only our + * reviewed reusable workflow can — even if some unrelated workflow in + * one of our repos later sets `id-token: write` for some other purpose. + */ +export const ALLOWED_JOB_WORKFLOW_REF_PREFIX = + 'baladithyab/web-embed-workflows/.github/workflows/static-passthrough.yml@' + /** * Given verified claims, decide whether the workflow is authorized to - * upload embeds. Today: must be from `repository_owner === ALLOWED_OWNER`. + * upload embeds. Layered checks: + * 1. `repository_owner` must be ALLOWED_OWNER (we're the only ones + * whose workflows can upload). + * 2. `job_workflow_ref` must start with the pinned prefix (only OUR + * reusable workflow can upload, not arbitrary workflows in our + * repos). * * Future hardening hooks (commented out for now): - * - require `event_name === 'push'` or `'workflow_dispatch'` (block PRs from forks) - * - require `ref === 'refs/heads/master'` or `'refs/heads/main'` - * - require `job_workflow_ref` to start with `baladithyab/web-embed-workflows/` - * (so only OUR reusable workflow gets to upload, not arbitrary workflows - * in our repos) + * - require `event_name === 'push'` or `'workflow_dispatch'` (block PRs + * from forks; today this is also covered by GitHub's own OIDC + * suppression on fork PRs) + * - pin `job_workflow_ref` to a specific tag rather than the branch + * (e.g. `@refs/tags/v1.2.3`) once the reusable workflow has stable + * versioning */ export function authorizeForEmbedUpload(claims: GithubOidcClaims): OidcAuthzResult { if (claims.repository_owner !== ALLOWED_OWNER) { @@ -303,5 +329,23 @@ export function authorizeForEmbedUpload(claims: GithubOidcClaims): OidcAuthzResu message: `repository_owner '${claims.repository_owner}' is not authorized`, } } + // job_workflow_ref pin: must be from our audited reusable workflow. + // GitHub spec says this claim is set whenever a reusable workflow is + // called. If it's somehow missing, fail closed. + const jwfr = claims.job_workflow_ref + if (!jwfr) { + return { + ok: false, + status: 403, + message: 'OIDC token has no job_workflow_ref claim', + } + } + if (!jwfr.startsWith(ALLOWED_JOB_WORKFLOW_REF_PREFIX)) { + return { + ok: false, + status: 403, + message: `job_workflow_ref '${jwfr}' is not from the allowed reusable workflow`, + } + } return { ok: true, claims } } diff --git a/src/scripts/sync-project-manifests.ts b/src/scripts/sync-project-manifests.ts index 555647b..fbf5ac0 100644 --- a/src/scripts/sync-project-manifests.ts +++ b/src/scripts/sync-project-manifests.ts @@ -26,6 +26,17 @@ import { join, resolve } from 'node:path' import { ProjectManifest, normalizeManifest } from '../lib/types/project-manifest.ts' const TOPIC = 'codeseys-embed' +/** + * GitHub user/org whose repos are eligible to appear in the personal + * site. Without this scope, anyone in the world adding the + * `codeseys-embed` topic would surface on /projects. + * + * Defense in depth: even if a stranger sets the topic, manifest + * validation + the OIDC upload path together mean the worst case is a + * `pending/` placeholder card with a manifest from someone else; no + * artifact upload would actually succeed. + */ +const ALLOWED_OWNER = 'baladithyab' const OUT_DIR = resolve(process.cwd(), 'src/content/projects') const MANIFEST_FILE = 'web.codeseys.json' @@ -70,7 +81,7 @@ async function listEmbedRepos(token: string | undefined): Promise /* eslint-disable no-constant-condition */ while (true) { const url = new URL('https://api.github.com/search/repositories') - url.searchParams.set('q', `topic:${TOPIC}`) + url.searchParams.set('q', `topic:${TOPIC} user:${ALLOWED_OWNER}`) url.searchParams.set('per_page', '100') url.searchParams.set('page', String(page)) @@ -128,6 +139,22 @@ interface ValidEntry { } function validate(entry: FetchedManifest): ValidEntry | null { + // Defense in depth: even if the search query somehow returns a repo + // outside ALLOWED_OWNER, refuse to write its manifest. `entry.source` + // is the canonical `/` from the search result. + const slashIdx = entry.source.indexOf('/') + if (slashIdx === -1) { + console.warn(` · ${entry.source}: malformed source (no slash); rejected`) + return null + } + const owner = entry.source.slice(0, slashIdx) + if (owner !== ALLOWED_OWNER) { + console.warn( + ` · ${entry.source}: owner '${owner}' is not the allowed owner '${ALLOWED_OWNER}'; rejected` + ) + return null + } + let parsed: unknown try { parsed = JSON.parse(entry.raw)