From f558980358b23aef84608185bdd3d8225ce391ce Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Tue, 11 Aug 2026 14:07:21 -0400 Subject: [PATCH] feat(release): check the oauth client metadata against every shipped id The extension bundles oauth/client-metadata.json and validates its own runtime redirect URI against it, so metadata that does not cover the id an install runs under throws "Invalid redirect_uri" before the authorization request is pushed. That is invisible to every server-side check: the hosted metadata and the OAuth server are both fine. Declare the ids the extension ships under in oauth/extension-ids.json and check them three ways: the unpacked id is derived from the manifest key rather than trusted, every declared id has a redirect URI (and every redirect URI a declared id), and the built JS carries both. The bundle check is the only one that inspects the artifact being uploaded rather than the sources it should have come from. --- oauth/extension-ids.json | 4 + scripts/check-oauth-metadata.mjs | 158 ++++++++++++++++++++++++++ scripts/check-oauth-metadata.test.mjs | 123 ++++++++++++++++++++ 3 files changed, 285 insertions(+) create mode 100644 oauth/extension-ids.json create mode 100644 scripts/check-oauth-metadata.mjs create mode 100644 scripts/check-oauth-metadata.test.mjs diff --git a/oauth/extension-ids.json b/oauth/extension-ids.json new file mode 100644 index 0000000..fa47211 --- /dev/null +++ b/oauth/extension-ids.json @@ -0,0 +1,4 @@ +{ + "store": "mecbfognmmefgjekidnddjjlddnfnlki", + "unpacked": "degljbilkggdpbobomfbgnellecgbkjj" +} diff --git a/scripts/check-oauth-metadata.mjs b/scripts/check-oauth-metadata.mjs new file mode 100644 index 0000000..531438e --- /dev/null +++ b/scripts/check-oauth-metadata.mjs @@ -0,0 +1,158 @@ +// Guards the OAuth client metadata against the failure that shipped in +// v1.2.1: the extension bundles oauth/client-metadata.json (see +// src/lib/oauth.ts), and at runtime asks for the redirect URI of whatever +// extension id it is actually running under. The store build runs under the +// store-assigned id, so a metadata file that lists only the unpacked dev id +// makes @atproto/oauth-client throw "Invalid redirect_uri" before the +// authorization request is ever pushed — sign-in is dead for every store +// user, while every server-side check still passes. +// +// The build cannot discover its own store id, so the ids the extension ships +// under are declared in oauth/extension-ids.json. The unpacked id is not +// taken on faith: it is derived from the manifest `key` and compared, so +// rotating the key without updating the metadata fails here too. +// +// Usage: +// node scripts/check-oauth-metadata.mjs # offline checks only +// node scripts/check-oauth-metadata.mjs --dist dist # also check the build +// node scripts/check-oauth-metadata.mjs --hosted # also fetch client_id +// +// Plain node, no dependencies. The pure helpers are exported for the tests. + +import { createHash } from 'node:crypto' +import { existsSync, readFileSync, readdirSync } from 'node:fs' +import { join, resolve } from 'node:path' + +/** + * The extension id Chrome derives from a manifest `key`: sha256 of the DER + * public key (which is what `key` base64-decodes to), first 16 bytes, hex, + * with each hex digit mapped 0-f -> a-p. + */ +export function extensionIdFromKey(base64Key) { + const digest = createHash('sha256').update(Buffer.from(base64Key, 'base64')).digest('hex') + return [...digest.slice(0, 32)] + .map((c) => String.fromCharCode(97 + Number.parseInt(c, 16))) + .join('') +} + +/** Mirrors oauthRedirectUri() in src/lib/authflow.ts. Must stay in step. */ +export function redirectUriFor(extensionId) { + return `https://${extensionId}.chromiumapp.org/oauth2` +} + +/** + * Every declared id has a redirect URI, and every redirect URI belongs to a + * declared id. The second half matters as much as the first: an unrecognized + * URI means the id list and the metadata have drifted apart, and the next + * person to read either one learns the wrong thing. + */ +export function checkRedirectUris(metadata, ids) { + const problems = [] + const declared = Object.entries(ids) + const want = new Map(declared.map(([role, id]) => [redirectUriFor(id), role])) + const have = new Set(metadata.redirect_uris ?? []) + + for (const [uri, role] of want) { + if (!have.has(uri)) { + problems.push(`client-metadata.json has no redirect_uri for the ${role} id: ${uri}`) + } + } + for (const uri of have) { + if (!want.has(uri)) { + problems.push(`client-metadata.json has redirect_uri ${uri}, which is not a declared id`) + } + } + return problems +} + +/** The declared unpacked id must be the one the manifest `key` produces. */ +export function checkManifestKey(manifest, ids) { + if (!manifest.key) return [] // store builds have the key stripped + const derived = extensionIdFromKey(manifest.key) + if (derived !== ids.unpacked) { + return [ + `manifest key derives extension id ${derived}, but extension-ids.json declares unpacked ${ids.unpacked}`, + ] + } + return [] +} + +/** + * The hosted copy at client_id is what a PDS reads; the bundled copy is what + * the extension validates against. They must agree or sign-in fails on one + * side of the flow only, which is exactly the shape of bug this file exists + * to catch. + */ +export function checkHostedMatchesLocal(local, hosted) { + const canon = (o) => JSON.stringify(o, Object.keys(o).sort()) + if (canon(local) === canon(hosted)) return [] + return [ + `the hosted metadata at ${local.client_id} differs from oauth/client-metadata.json` + + ` (redirect_uris hosted=[${(hosted.redirect_uris ?? []).join(', ')}]` + + ` local=[${(local.redirect_uris ?? []).join(', ')}])`, + ] +} + +/** + * Every declared redirect URI must appear literally in the built JS. Vite + * inlines the imported JSON, so a stale bundle is visible as a missing + * string — the one check that looks at the artifact actually being uploaded + * rather than at the sources it was supposed to come from. + */ +export function checkBundle(distDir, ids) { + const problems = [] + const js = readdirSync(distDir, { recursive: true }).filter((f) => String(f).endsWith('.js')) + const sources = js.map((name) => readFileSync(join(distDir, String(name)), 'utf8')) + for (const [role, id] of Object.entries(ids)) { + const uri = redirectUriFor(id) + if (!sources.some((src) => src.includes(uri))) { + problems.push(`no built JS in ${distDir} contains the ${role} redirect URI ${uri}`) + } + } + return problems +} + +// --- CLI --------------------------------------------------------------------- + +async function main(argv) { + const root = resolve(import.meta.dirname, '..') + const readJson = (rel) => JSON.parse(readFileSync(join(root, rel), 'utf8')) + + const distFlag = argv.indexOf('--dist') + const distDir = distFlag === -1 ? undefined : resolve(argv[distFlag + 1] ?? 'dist') + const hosted = argv.includes('--hosted') + + const metadata = readJson('oauth/client-metadata.json') + const ids = readJson('oauth/extension-ids.json') + const manifest = readJson('public/manifest.json') + + const problems = [ + ...checkManifestKey(manifest, ids), + ...checkRedirectUris(metadata, ids), + ] + + if (distDir) { + if (!existsSync(distDir)) problems.push(`${distDir} does not exist — run \`npm run build\` first`) + else problems.push(...checkBundle(distDir, ids)) + } + + if (hosted) { + try { + const res = await fetch(metadata.client_id, { signal: AbortSignal.timeout(15_000) }) + if (!res.ok) problems.push(`fetching ${metadata.client_id} returned ${res.status}`) + else problems.push(...checkHostedMatchesLocal(metadata, await res.json())) + } catch (err) { + problems.push(`could not fetch ${metadata.client_id}: ${err.message}`) + } + } + + if (problems.length > 0) { + console.error(`check-oauth-metadata: FAILED (${problems.length} problem(s)):`) + for (const p of problems) console.error(` - ${p}`) + process.exit(1) + } + const scope = ['ids', distDir && 'bundle', hosted && 'hosted'].filter(Boolean).join(' + ') + console.log(`check-oauth-metadata: OK — ${scope}`) +} + +if (process.argv[1] === import.meta.filename) await main(process.argv.slice(2)) diff --git a/scripts/check-oauth-metadata.test.mjs b/scripts/check-oauth-metadata.test.mjs new file mode 100644 index 0000000..97a4301 --- /dev/null +++ b/scripts/check-oauth-metadata.test.mjs @@ -0,0 +1,123 @@ +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' +import { + checkBundle, + checkHostedMatchesLocal, + checkManifestKey, + checkRedirectUris, + extensionIdFromKey, + redirectUriFor, +} from './check-oauth-metadata.mjs' + +// The real pair from public/manifest.json, so a key or derivation change +// that would silently move the unpacked id fails here. +const MANIFEST_KEY = + 'MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA9X42ElF0Js6o5Q4khUvEZJCEnEJuXEswyH4AXc9pA98vP5K/wIyj3grJIREAVHvJYCtAfvWPlLq//WQdnoRt5j7RljVDrwRxuxqDXZ7k3M6ipX8czg7wKfS0Gq2rTgVZD1po96eHTFNyxbaR08maLNIwNNWYqbwCECRR41uXdpw+XS66KvGZ8RybnPArkk0jW9lQ5ro/WlDpyjmTxWWYIM2nkZ/m2e66ZLCuT7HcYpHJmsEOH/xBUkgRZMzmEcQJk5m3rSuEOaB4DzSGZTaRy2OLzxMbAnpgKPTOSqa2RLYvb4MsrUgs2akezzGD0pJD46FkeB6eaLxqCprrIn3fIQIDAQAB' +const UNPACKED = 'degljbilkggdpbobomfbgnellecgbkjj' +const STORE = 'mecbfognmmefgjekidnddjjlddnfnlki' +const IDS = { store: STORE, unpacked: UNPACKED } + +let dirs = [] +afterEach(() => { + for (const dir of dirs) rmSync(dir, { recursive: true, force: true }) + dirs = [] +}) + +function distWith(contents) { + const dir = mkdtempSync(join(tmpdir(), 'check-oauth-')) + dirs.push(dir) + writeFileSync(join(dir, 'bundle.js'), contents) + return dir +} + +describe('extensionIdFromKey', () => { + it('derives the unpacked id Chrome assigns to the manifest key', () => { + expect(extensionIdFromKey(MANIFEST_KEY)).toBe(UNPACKED) + }) + + it('only ever produces the a-p alphabet Chrome uses', () => { + expect(extensionIdFromKey(MANIFEST_KEY)).toMatch(/^[a-p]{32}$/) + }) +}) + +describe('checkRedirectUris', () => { + it('passes when every declared id has a redirect uri', () => { + const metadata = { redirect_uris: [redirectUriFor(STORE), redirectUriFor(UNPACKED)] } + expect(checkRedirectUris(metadata, IDS)).toEqual([]) + }) + + // The v1.2.1 regression: metadata carrying only the unpacked dev id, which + // makes sign-in throw "Invalid redirect_uri" in every store install. + it('fails the exact shape that shipped broken in v1.2.1', () => { + const metadata = { redirect_uris: [redirectUriFor(UNPACKED)] } + const problems = checkRedirectUris(metadata, IDS) + expect(problems).toHaveLength(1) + expect(problems[0]).toContain('store') + expect(problems[0]).toContain(STORE) + }) + + it('fails a redirect uri that belongs to no declared id', () => { + const metadata = { + redirect_uris: [ + redirectUriFor(STORE), + redirectUriFor(UNPACKED), + redirectUriFor('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'), + ], + } + expect(checkRedirectUris(metadata, IDS)).toEqual([ + expect.stringContaining('not a declared id'), + ]) + }) + + it('reports missing metadata rather than throwing', () => { + expect(checkRedirectUris({}, IDS)).toHaveLength(2) + }) +}) + +describe('checkManifestKey', () => { + it('accepts the key that derives the declared unpacked id', () => { + expect(checkManifestKey({ key: MANIFEST_KEY }, IDS)).toEqual([]) + }) + + it('fails when the key and the declared unpacked id disagree', () => { + expect(checkManifestKey({ key: MANIFEST_KEY }, { ...IDS, unpacked: 'b'.repeat(32) })).toEqual([ + expect.stringContaining(UNPACKED), + ]) + }) + + it('skips the check for a store build, whose key is stripped', () => { + expect(checkManifestKey({}, IDS)).toEqual([]) + }) +}) + +describe('checkHostedMatchesLocal', () => { + const local = { client_id: 'https://example.test/m.json', redirect_uris: ['a'] } + + it('passes on identical documents regardless of key order', () => { + expect( + checkHostedMatchesLocal(local, { redirect_uris: ['a'], client_id: local.client_id }), + ).toEqual([]) + }) + + it('fails when the hosted copy is missing a redirect uri', () => { + const problems = checkHostedMatchesLocal(local, { ...local, redirect_uris: [] }) + expect(problems).toEqual([expect.stringContaining('differs from')]) + }) +}) + +describe('checkBundle', () => { + it('passes when the built JS carries every declared redirect uri', () => { + const dir = distWith(`x=${JSON.stringify([redirectUriFor(STORE), redirectUriFor(UNPACKED)])}`) + expect(checkBundle(dir, IDS)).toEqual([]) + }) + + // A rebuild from stale sources is invisible everywhere else: the metadata + // file, the id list and the hosted copy can all agree while the artifact + // about to be uploaded predates them. + it('fails a stale bundle that predates the store id', () => { + const dir = distWith(`x=${JSON.stringify([redirectUriFor(UNPACKED)])}`) + expect(checkBundle(dir, IDS)).toEqual([expect.stringContaining(redirectUriFor(STORE))]) + }) +}) -- 2.51.2