diff --git a/packages/core/src/claustrum.ts b/packages/core/src/claustrum.ts index c17a7dbb..fe9a3c28 100644 --- a/packages/core/src/claustrum.ts +++ b/packages/core/src/claustrum.ts @@ -273,10 +273,48 @@ export function custodyCredentialId(label: string): string { return `${CUSTODY_CREDENTIAL_PREFIX}${label}` } +// A resolved MANIFEST binding carries the credential id verbatim (the parser +// stores it as-written and the resolver returns it; round 2 dropped the +// parse-time derivation check). Anything else — legacy source, unresolved, +// or a manifest entry whose id happens to be undefined — derives from the +// label, the only id the caller can construct without a binding in hand. +// Callers building completion records (local-exit, reachability probe) +// pass this single rule instead of inlining it. +export function custodyCredentialIdFromResolution( + resolution: CustodyHandleResolution, + label: string, +): string { + if (resolution.status === 'resolved' && resolution.source === 'manifest') { + return resolution.credentialId ?? custodyCredentialId(label) + } + return custodyCredentialId(label) +} + function isValidCustodyCredentialId(value: unknown): value is string { return typeof value === 'string' && value.length > 0 } +// The manifest is a co-tenant file: a sibling plugin (`provider: 'openai'`, +// `serve: 'openai-auth'`) writes its own block in the same file. Provider +// scoping is what stops us from binding its handles — the parser has to +// enforce it because the sibling plugin never talks to us. +// +// Scope on the SECOND colon-separated segment (the provider segment). The +// first segment is a "kind" prefix the vault owner extends (`oauth:`, +// `chatgpt:`, `antigravity:`, `apikey:`, …) and the rule deliberately does +// NOT enumerate or constrain it: enumerating the kinds would reject live +// credentials the moment a new one ships. The third-and-later segments are +// the label, which is never consulted here — the label is the lookup key +// elsewhere, not an authorization check. +function isScopedCustodyCredentialId( + value: unknown, + provider: string, +): value is string { + if (typeof value !== 'string' || value.length === 0) return false + const segments = value.split(':') + return segments[1] === provider +} + function legacyOrUnresolved( account: OAuthAccount, reason: Extract['reason'], @@ -321,10 +359,10 @@ export function resolveCustodyHandle(input: { return { status: 'unresolved', reason: 'corrupt-binding' } } + // The manifest carries the credential id verbatim; the runtime fence in + // custody-mode.ts is the one that compares it against vault ground truth. const entry = manifest.accounts.find( - (candidate) => - candidate.label === account.label && - candidate.credentialId === custodyCredentialId(account.label), + (candidate) => candidate.label === account.label, ) if (!entry) return legacyOrUnresolved(account, 'missing-entry') if (manifest.superseded.has(entry.handle)) { @@ -372,6 +410,27 @@ export function readCustodyHandles( if (!Object.hasOwn(source, 'accounts') || !Array.isArray(source.accounts)) { throw new Error('invalid manifest accounts') } + // The label is the sole lookup key; two entries with the same label cannot + // tell the resolver which one to bind, so every such entry is marked corrupt + // rather than silently picking a winner. + const labelCounts = new Map() + for (const candidate of source.accounts) { + if ( + !isRecord(candidate) || + !Object.hasOwn(candidate, 'label') || + typeof candidate.label !== 'string' || + !isValidCustodyLabel(candidate.label) + ) + continue + labelCounts.set( + candidate.label, + (labelCounts.get(candidate.label) ?? 0) + 1, + ) + } + const duplicateLabels = new Set( + [...labelCounts].filter(([, count]) => count > 1).map(([label]) => label), + ) + const superseded = new Set() const corruptLabels = new Set() const accounts: CustodyHandleAccount[] = [] @@ -388,13 +447,15 @@ export function readCustodyHandles( !Object.hasOwn(entry, 'credential_id') || typeof entry.handle !== 'string' || !isValidCustodyHandle(entry.handle) || - !isValidCustodyCredentialId(entry.credential_id) || - (provider === 'anthropic' && - entry.credential_id !== custodyCredentialId(entry.label)) + !isScopedCustodyCredentialId(entry.credential_id, provider) ) { corruptLabels.add(entry.label) continue } + if (duplicateLabels.has(entry.label)) { + corruptLabels.add(entry.label) + continue + } if (Object.hasOwn(entry, 'superseded')) { if (!Array.isArray(entry.superseded)) { corruptLabels.add(entry.label) @@ -958,8 +1019,7 @@ export async function writeCustodyHandleManifestEntry( if ( !isValidCustodyLabel(input.entry.label) || !isValidCustodyHandle(input.entry.handle) || - !isValidCustodyCredentialId(input.entry.credentialId) || - input.entry.credentialId !== custodyCredentialId(input.entry.label) + !isValidCustodyCredentialId(input.entry.credentialId) ) { return refusal('invalid entry') } @@ -987,8 +1047,7 @@ export async function removeCustodyHandleManifestEntry( if ( !isValidCustodyLabel(input.entry.label) || !isValidCustodyHandle(input.entry.handle) || - !isValidCustodyCredentialId(input.entry.credentialId) || - input.entry.credentialId !== custodyCredentialId(input.entry.label) + !isValidCustodyCredentialId(input.entry.credentialId) ) { return { status: 'refused' } } @@ -1514,6 +1573,7 @@ export type ClaustrumCredential = { recordVersion: number projectId?: string accountId?: string + credentialId?: string } export type ClaustrumServedCredential = Pick< @@ -1552,6 +1612,7 @@ type CredentialGetResult = { recordVersion: number projectId?: string accountId?: string + credentialId?: string } function credentialErrorAction( @@ -1688,6 +1749,9 @@ function decodeCredentialGetResponse(response: unknown): CredentialGetResult { ...(typeof result?.account_id === 'string' && { accountId: result.account_id, }), + ...(typeof result?.credential_id === 'string' && { + credentialId: result.credential_id, + }), } } @@ -1897,6 +1961,9 @@ export class ClaustrumCredentialCache { recordVersion: result.recordVersion, ...(result.projectId !== undefined && { projectId: result.projectId }), ...(result.accountId !== undefined && { accountId: result.accountId }), + ...(result.credentialId !== undefined && { + credentialId: result.credentialId, + }), } if ( credential.expiresAtMs !== null && diff --git a/packages/core/src/tests/claustrum.test.ts b/packages/core/src/tests/claustrum.test.ts index 6fd85914..d648c0f8 100644 --- a/packages/core/src/tests/claustrum.test.ts +++ b/packages/core/src/tests/claustrum.test.ts @@ -1,7 +1,14 @@ import { describe, expect, test } from 'bun:test' import { basename, win32 } from 'node:path' -import { __deriveCustodyManifestStaleLockPrefix } from '../claustrum.ts' +import { + __deriveCustodyManifestStaleLockPrefix, + type CustodyHandleManifest, + type CustodyHandleResolution, + custodyCredentialIdFromResolution, + readCustodyHandles, + resolveCustodyHandle, +} from '../claustrum.ts' describe('custody manifest stale-lock prefix', () => { test('derives prefixes from POSIX and Windows path basenames', () => { @@ -19,3 +26,284 @@ describe('custody manifest stale-lock prefix', () => { ).toBe('handles.json.lock.stale-') }) }) + +describe('custodyCredentialIdFromResolution', () => { + test("returns a resolved manifest binding's credential id verbatim, not the derived form", () => { + // The provider-default main case: the manifest carries `oauth:anthropic`, + // not `oauth:anthropic:main`. This is the assertion round 2's e2e test + // could not make — it had to feed the id in by hand because the helper + // was inline. + const resolution: CustodyHandleResolution = { + status: 'resolved', + source: 'manifest', + handle: 'ckh_a'.padEnd(47, '_'), + credentialId: 'oauth:anthropic', + } + expect(custodyCredentialIdFromResolution(resolution, 'main')).toBe( + 'oauth:anthropic', + ) + }) + + test('derives from the label when the resolved source is legacy', () => { + const resolution: CustodyHandleResolution = { + status: 'resolved', + source: 'legacy', + handle: 'ckh_b'.padEnd(47, '_'), + } + expect(custodyCredentialIdFromResolution(resolution, 'work-alt')).toBe( + 'oauth:anthropic:work-alt', + ) + }) + + test('derives from the label when the resolution is unresolved', () => { + const resolution: CustodyHandleResolution = { + status: 'unresolved', + reason: 'missing-entry', + } + expect(custodyCredentialIdFromResolution(resolution, 'main')).toBe( + 'oauth:anthropic:main', + ) + }) + + test("derives from the label when the manifest binding's credential id is undefined", () => { + const resolution = { + status: 'resolved' as const, + source: 'manifest' as const, + handle: 'ckh_c'.padEnd(47, '_'), + credentialId: undefined, + } + expect(custodyCredentialIdFromResolution(resolution, 'main')).toBe( + 'oauth:anthropic:main', + ) + }) +}) + +// The manifest is a co-tenant file: a sibling plugin (`provider: 'openai'`, +// `serve: 'openai-auth'`) writes its own block in the same file, and the +// `serve`/`provider` split is what stops the two plugins from binding each +// other's handles. Provider scoping has to live in the parser because the +// sibling plugin never talks to us — it only writes the file. +describe('readCustodyHandles provider scope', () => { + const anthropicHandle = `ckh_${'A'.repeat(43)}` + const openaiHandle = `ckh_${'O'.repeat(43)}` + const googleHandle = `ckh_${'G'.repeat(43)}` + + function makeManifest(provider: string, serve: string) { + return { + version: 1, + providers: [ + { + provider, + serve, + accounts: [], + }, + ], + } as const + } + + function manifestFromParsed( + parsed: ReturnType, + ): CustodyHandleManifest { + return { + version: 1, + provider: parsed.provider as 'anthropic', + serve: parsed.serve as 'anthropic-auth', + accounts: parsed.accounts, + superseded: parsed.superseded, + corruptLabels: parsed.corruptLabels, + } + } + + function oauthAccount(label: string) { + return { id: `uuid-${label}`, type: 'oauth' as const, refresh: 'r', label } + } + + // (a) A real foreign id inside the anthropic block — the case the parse-time + // derivation check used to catch. The kind prefix (`chatgpt:`) is one + // the vault serves today; the rule is that `chatgpt:openai` belongs in + // an `openai` block, not in ours. + test('rejects an entry whose credential id names a different provider', () => { + const doc = makeManifest('anthropic', 'anthropic-auth') + doc.providers[0]!.accounts.push({ + label: 'main', + handle: anthropicHandle, + credential_id: 'chatgpt:openai', + }) + + const parsed = readCustodyHandles(doc, 'anthropic', 'anthropic-auth') + expect(parsed.corruptLabels).toEqual(new Set(['main'])) + expect(parsed.accounts).toEqual([]) + + const result = resolveCustodyHandle({ + account: oauthAccount('main'), + manifest: manifestFromParsed(parsed), + }) + expect(result).toEqual({ status: 'unresolved', reason: 'corrupt-binding' }) + }) + + // (b) The main case this branch exists for: the unlabelled provider-default + // credential id. Must still resolve — the runtime fence in + // custody-mode.ts is the only thing that ever proves it against the + // vault, but the parser must accept it verbatim. + test("resolves the unlabelled provider-default credential id (main's id is `oauth:anthropic`)", () => { + const doc = makeManifest('anthropic', 'anthropic-auth') + doc.providers[0]!.accounts.push({ + label: 'main', + handle: anthropicHandle, + credential_id: 'oauth:anthropic', + }) + + const parsed = readCustodyHandles(doc, 'anthropic', 'anthropic-auth') + expect(parsed.corruptLabels).toEqual(new Set()) + expect(parsed.accounts).toHaveLength(1) + expect(parsed.accounts[0]?.credentialId).toBe('oauth:anthropic') + + const result = resolveCustodyHandle({ + account: oauthAccount('main'), + manifest: manifestFromParsed(parsed), + }) + expect(result).toEqual({ + status: 'resolved', + source: 'manifest', + handle: anthropicHandle, + credentialId: 'oauth:anthropic', + }) + }) + + // (c) The labelled form: credential id `oauth::