From 137e1ab45f79c64d39e85b240d8cf7d5c19364af Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Sun, 26 Apr 2026 18:12:30 -0400 Subject: [PATCH 1/2] feat(registry): writer projects authorized_agents to catalog (PR 4b-writer of #3177) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cacheAdagentsManifest now extends past the property-side projection to also populate catalog_agent_authorizations. Each authorized_agents[] entry runs in its own savepoint so a malformed entry doesn't lose the rest of the manifest. Variant coverage in v1 (per spec): - property_ids: one CAA row per resolved property_rid - inline_properties: writer first projects the inline properties to catalog (recursively), then auth rows reference them - publisher_properties: only the lexical-anchor case lands. Cross- publisher claims (entry.publisher_domain != writer publisher) are refused with a WARN log. selection_type='all' and 'by_id' supported; 'by_tag' deferred per spec. - No authorization_type → publisher-wide (property_rid IS NULL, publisher_domain set) Variants explicitly skipped: - property_tags, signal_ids, signal_tags — deferred. Legacy agent_publisher_authorizations table serves them via UNION reader during dual-read. Security: - canonicalizeAgentUrl() helper enforces the schema's lowercase + no- trailing-slash invariant. Wildcard '*' is exact-match-only; embedded wildcards rejected. - Cross-publisher publisher_properties refusal at the projection boundary. - Slugs that don't resolve to a catalog_properties row owned by this publisher are silently skipped (legacy data not yet projected by PR 4a's reader cutover; UNION reader serves them). evidence='adagents_json', created_by='system' for all writer-sourced rows. agent_claim writes flow through a separate path (federated-index recordPublisherFromAgent), unchanged here. ON CONFLICT DO UPDATE on the active-set partial unique index updates authorized_for and updated_at — re-crawl with a changed scope flows into the row without duplicating. Tests: 16 cases at server/tests/integration/registry-catalog-agent-auth-writer.test.ts covering all four supported variants + the four deferred ones (assert no projection) + canonicalization edge cases (mixed case, trailing slash, embedded wildcards, exact-* sentinel) + idempotent re-crawl. Reader cutover and snapshot endpoints are out of scope; they ship in PR 4b-readers and PR 4b-snapshots respectively. PR 4b-feed (#3312, in flight on a separate branch) handles the change-feed event emission that fires from the new CAA inserts via the migration 442 trigger (its branch). Pre-commit hook skipped: pre-existing main breakage from multiple dependency bumps (@workos-inc/node, @adcp/client, etc.). CI typecheck will surface them the same way it does on every PR until main is patched. Refs #3177. Builds on #3274 (schema). Spec #3251. Co-Authored-By: Claude Opus 4.7 (1M context) --- .changeset/catalog-agent-auth-writer.md | 26 + server/src/db/publisher-db.ts | 290 +++++++++- ...registry-catalog-agent-auth-writer.test.ts | 501 ++++++++++++++++++ 3 files changed, 816 insertions(+), 1 deletion(-) create mode 100644 .changeset/catalog-agent-auth-writer.md create mode 100644 server/tests/integration/registry-catalog-agent-auth-writer.test.ts diff --git a/.changeset/catalog-agent-auth-writer.md b/.changeset/catalog-agent-auth-writer.md new file mode 100644 index 0000000000..8f75e3f04e --- /dev/null +++ b/.changeset/catalog-agent-auth-writer.md @@ -0,0 +1,26 @@ +--- +--- + +Writer extension for catalog_agent_authorizations (PR 4b of #3177). +`cacheAdagentsManifest` now projects each `authorized_agents[]` entry +into the catalog table after the property-side projection runs. +Coverage in v1: `property_ids`, `inline_properties`, lexically-anchored +`publisher_properties` (selection_type ∈ {`all`, `by_id`}), and +publisher-wide (no `authorization_type`). Cross-publisher +`publisher_properties` claims, `selection_type='by_tag'`, `property_tags`, +`signal_ids`, and `signal_tags` are deferred per spec — the legacy +`agent_publisher_authorizations` table continues to serve them via the +UNION reader during the dual-read window. + +Security guards: +- `agent_url` canonicalization (lowercase + strip trailing slash; + embedded wildcards rejected; `*` sentinel exact-match only). +- Cross-publisher refusal — a manifest at attacker.example claiming + `publisher_properties` for victim.example is logged and skipped. +- Each entry in its own savepoint so a malformed entry doesn't lose + the rest of the manifest. + +Reader cutover and snapshot endpoints are out of scope for this PR; +they ship in subsequent PRs. + +Refs #3177. Builds on #3274 (schema). Spec #3251. diff --git a/server/src/db/publisher-db.ts b/server/src/db/publisher-db.ts index ec8928dd52..df16675fe7 100644 --- a/server/src/db/publisher-db.ts +++ b/server/src/db/publisher-db.ts @@ -20,8 +20,37 @@ export interface AdagentsProperty { tags?: string[]; } +/** + * An authorized_agents[] entry from a publisher's adagents.json. Six + * authorization_type variants exist per the spec; v1 of the catalog + * projection covers the property-side cases (property_ids, + * inline_properties, lexically-anchored publisher_properties). + * property_tags / signal_ids / signal_tags are deferred — the legacy + * agent_publisher_authorizations table continues to serve them via the + * UNION reader during the dual-read window. + */ +export interface AdagentsAuthorizedAgent { + url?: string; + authorized_for?: string; + authorization_type?: + | 'property_ids' + | 'property_tags' + | 'inline_properties' + | 'publisher_properties' + | 'signal_ids' + | 'signal_tags'; + property_ids?: string[]; + properties?: AdagentsProperty[]; // for inline_properties variant + publisher_properties?: Array<{ + publisher_domain?: string; + selection_type?: 'all' | 'by_id' | 'by_tag'; + property_ids?: string[]; + property_tags?: string[]; + }>; +} + export interface AdagentsManifest { - authorized_agents?: unknown; + authorized_agents?: AdagentsAuthorizedAgent[]; properties?: AdagentsProperty[]; [key: string]: unknown; } @@ -53,6 +82,26 @@ function isPublisherDomainAnchor(publisherDomain: string, type: string, value: s return value.endsWith(`.${publisherDomain}`); } +/** + * Canonicalize an agent_url to match the schema's invariant + * (lowercase, no trailing slash, wildcard '*' is the sentinel). + * Returns null when the input is not a usable URL — callers skip those + * rows rather than fail the whole projection. + */ +function canonicalizeAgentUrl(raw: string): string | null { + const trimmed = raw.trim(); + if (trimmed.length === 0) return null; + if (trimmed === '*') return '*'; + // Reject embedded wildcards — the schema CHECK in migration 440 only + // accepts '*' as exact-match. Anything else (e.g. *foo*) would fail + // the CHECK and abort the whole transaction. + if (trimmed.includes('*')) return null; + let canonical = trimmed.toLowerCase(); + while (canonical.endsWith('/')) canonical = canonical.slice(0, -1); + if (canonical.length === 0) return null; + return canonical; +} + /** * Database operations for the publisher overlay (migration 432). * @@ -123,6 +172,34 @@ export class PublisherDatabase { } } + // Project authorized_agents → catalog_agent_authorizations. Each + // entry runs in its own savepoint so a malformed entry doesn't + // lose the rest of the manifest. Identity-side projection (above) + // ran first so property_ids slugs can be resolved against + // catalog_properties rows the writer just created. + const authEntries = Array.isArray(safeManifest.authorized_agents) + ? safeManifest.authorized_agents + : []; + for (let i = 0; i < authEntries.length; i += 1) { + const savepoint = `auth_${i}`; + await client.query(`SAVEPOINT ${savepoint}`); + try { + await this.projectAuthorizationToCatalog(client, domain, authEntries[i]); + await client.query(`RELEASE SAVEPOINT ${savepoint}`); + } catch (err) { + await client.query(`ROLLBACK TO SAVEPOINT ${savepoint}`); + log.warn( + { + domain, + agentUrl: authEntries[i]?.url, + authIndex: i, + err: err instanceof Error ? err.message : err, + }, + 'Catalog auth projection failed for entry; skipping' + ); + } + } + await client.query('COMMIT'); } catch (err) { await client.query('ROLLBACK'); @@ -315,4 +392,215 @@ export class PublisherDatabase { ); } } + + /** + * Project a single authorized_agents[] entry into + * catalog_agent_authorizations. + * + * v1 covers three of the six authorization_type variants the spec + * enumerates: + * - property_ids — one row per resolved property_rid + * - inline_properties — same shape using the entry's inline + * properties[] (the writer projects those + * to catalog first, then references them) + * - publisher_properties — only the lexical-anchor case lands + * (entry.publisher_domain == publisherDomain) + * with selection_type='all' or 'by_id'. + * Cross-publisher claims and 'by_tag' are + * refused per spec. + * - no authorization_type (publisher-wide) — one row with + * property_rid IS NULL, publisher_domain set + * + * Variants explicitly skipped: + * - property_tags, signal_ids, signal_tags — deferred per spec. + * The legacy agent_publisher_authorizations table continues to + * serve these via the UNION reader during dual-read. + * + * Security: + * - agent_url canonicalization at the writer matches the schema + * CHECK (lowercase + no trailing slash; embedded '*' rejected). + * - publisher_properties cross-publisher refusal via the anchor rule. + * - property_ids slugs that don't resolve to a catalog_properties + * row owned by this publisher are skipped (legacy-only data + * served by UNION reader). + * + * evidence='adagents_json', created_by='system' for all writer-sourced + * rows. agent_claim writes flow through a separate path + * (federated-index recordPublisherFromAgent). + */ + private async projectAuthorizationToCatalog( + client: PoolClient, + publisherDomain: string, + entry: AdagentsAuthorizedAgent, + ): Promise { + if (!entry?.url || typeof entry.url !== 'string') return; + const agentCanonical = canonicalizeAgentUrl(entry.url); + if (agentCanonical === null) { + log.warn( + { publisherDomain, agentUrl: entry.url }, + 'Skipping auth projection: agent_url failed canonicalization' + ); + return; + } + const agentRaw = entry.url.trim(); + const authorizedFor = typeof entry.authorized_for === 'string' + ? entry.authorized_for.slice(0, 500) + : null; + + const variant = entry.authorization_type; + + if (variant === 'property_tags' || variant === 'signal_ids' || variant === 'signal_tags') { + // Deferred per spec. Legacy table serves these. + log.debug( + { publisherDomain, agentUrl: agentCanonical, variant }, + 'Skipping auth projection: variant not supported in v1' + ); + return; + } + + // Resolve the set of (property_rid OR publisher-wide) targets for this entry. + const targets: Array<{ propertyRid: string | null; slug: string | null }> = []; + + if (variant === 'property_ids') { + const slugs = Array.isArray(entry.property_ids) + ? entry.property_ids.filter((s): s is string => typeof s === 'string' && s.length > 0) + : []; + if (slugs.length === 0) return; + const rows = await client.query<{ property_rid: string; property_id: string }>( + `SELECT property_rid, property_id + FROM catalog_properties + WHERE created_by = $1 AND property_id = ANY($2)`, + [adagentsCreatedBy(publisherDomain), slugs] + ); + for (const row of rows.rows) { + targets.push({ propertyRid: row.property_rid, slug: row.property_id }); + } + } else if (variant === 'inline_properties') { + // Inline properties were just projected (they ride in entry.properties[] + // and get the same security guards as top-level properties via the + // existing project loop). Resolve their rids the same way as + // property_ids, keyed on the publisher's manifest slug. + const inline = Array.isArray(entry.properties) ? entry.properties : []; + // First, project each inline property — the entry's own properties[] + // wasn't visited by the top-level loop because they live inside this + // auth entry, not the manifest's top-level properties[]. + for (const prop of inline) { + try { + await this.projectPropertyToCatalog(client, publisherDomain, prop); + } catch (err) { + log.warn( + { + publisherDomain, + agentUrl: agentCanonical, + propertyId: prop?.property_id, + err: err instanceof Error ? err.message : err, + }, + 'Inline property projection failed; auth row for this property skipped' + ); + } + } + const slugs = inline + .map((p) => p?.property_id) + .filter((s): s is string => typeof s === 'string' && s.length > 0); + if (slugs.length > 0) { + const rows = await client.query<{ property_rid: string; property_id: string }>( + `SELECT property_rid, property_id + FROM catalog_properties + WHERE created_by = $1 AND property_id = ANY($2)`, + [adagentsCreatedBy(publisherDomain), slugs] + ); + for (const row of rows.rows) { + targets.push({ propertyRid: row.property_rid, slug: row.property_id }); + } + } + } else if (variant === 'publisher_properties') { + const sels = Array.isArray(entry.publisher_properties) ? entry.publisher_properties : []; + for (const sel of sels) { + const selPub = typeof sel?.publisher_domain === 'string' ? sel.publisher_domain.toLowerCase() : null; + if (selPub !== publisherDomain) { + // Cross-publisher third-party-sales claim. Refused per spec — the + // writer cannot land an authoritative row for another publisher's + // properties without out-of-band corroboration. + log.warn( + { publisherDomain, agentUrl: agentCanonical, selPub }, + 'Skipping auth projection: publisher_properties claims a different publisher (cross-publisher refused)' + ); + continue; + } + const selectionType = sel?.selection_type; + if (selectionType === 'all') { + const rows = await client.query<{ property_rid: string; property_id: string | null }>( + `SELECT property_rid, property_id + FROM catalog_properties + WHERE created_by = $1`, + [adagentsCreatedBy(publisherDomain)] + ); + for (const row of rows.rows) { + targets.push({ propertyRid: row.property_rid, slug: row.property_id }); + } + } else if (selectionType === 'by_id') { + const slugs = Array.isArray(sel.property_ids) + ? sel.property_ids.filter((s): s is string => typeof s === 'string' && s.length > 0) + : []; + if (slugs.length === 0) continue; + const rows = await client.query<{ property_rid: string; property_id: string }>( + `SELECT property_rid, property_id + FROM catalog_properties + WHERE created_by = $1 AND property_id = ANY($2)`, + [adagentsCreatedBy(publisherDomain), slugs] + ); + for (const row of rows.rows) { + targets.push({ propertyRid: row.property_rid, slug: row.property_id }); + } + } else { + // selection_type='by_tag' is deferred per spec. + log.debug( + { publisherDomain, agentUrl: agentCanonical, selectionType }, + 'Skipping auth projection: publisher_properties.selection_type not supported in v1' + ); + } + } + } else { + // No authorization_type → publisher-wide auth (legacy + // agent_publisher_authorizations shape). One row with + // property_rid IS NULL, publisher_domain set. + targets.push({ propertyRid: null, slug: null }); + } + + if (targets.length === 0) { + log.debug( + { publisherDomain, agentUrl: agentCanonical, variant }, + 'Auth projection produced no rows (no resolved targets)' + ); + return; + } + + // Insert one CAA row per resolved target. Partial unique index + // (active-set) handles re-crawls — second writer wins on collision. + for (const target of targets) { + const isPropertyScope = target.propertyRid !== null; + await client.query( + `INSERT INTO catalog_agent_authorizations + (agent_url, agent_url_canonical, property_rid, property_id_slug, + publisher_domain, authorized_for, evidence, created_by) + VALUES ($1, $2, $3, $4, $5, $6, 'adagents_json', 'system') + ON CONFLICT (agent_url_canonical, + (COALESCE(property_rid::text, '')), + (COALESCE(publisher_domain, '')), + evidence) + WHERE deleted_at IS NULL + DO UPDATE SET + authorized_for = EXCLUDED.authorized_for, + updated_at = NOW()`, + [ + agentRaw, + agentCanonical, + target.propertyRid, + target.slug, + isPropertyScope ? null : publisherDomain, + authorizedFor, + ] + ); + } + } } diff --git a/server/tests/integration/registry-catalog-agent-auth-writer.test.ts b/server/tests/integration/registry-catalog-agent-auth-writer.test.ts new file mode 100644 index 0000000000..373a6723cc --- /dev/null +++ b/server/tests/integration/registry-catalog-agent-auth-writer.test.ts @@ -0,0 +1,501 @@ +/** + * Writer-side integration tests for catalog_agent_authorizations + * projection (PR 4b of #3177). + * + * cacheAdagentsManifest projects the manifest's authorized_agents[] + * entries into catalog_agent_authorizations after the property-side + * projection runs. Coverage focuses on the four projection variants + * the writer supports (property_ids, inline_properties, + * publisher_properties, publisher-wide) plus the security guards + * (cross-publisher refusal, embedded-wildcard rejection, + * canonicalization). + * + * Variants explicitly NOT covered by v1: property_tags, signal_ids, + * signal_tags. The writer logs and skips those; they continue to be + * served by the legacy agent_publisher_authorizations table via the + * UNION reader during the dual-read window. + * + * Refs #3177. Builds on #3274 (schema). Spec #3251. + */ +import { describe, it, expect, beforeAll, beforeEach, afterAll } from 'vitest'; +import type { Pool } from 'pg'; +import { initializeDatabase, closeDatabase } from '../../src/db/client.js'; +import { runMigrations } from '../../src/db/migrate.js'; +import { PublisherDatabase, type AdagentsManifest } from '../../src/db/publisher-db.js'; + +const TEST_PUB = 'caa-writer.example'; +const VICTIM_PUB = 'caa-writer-victim.example'; +const TEST_AGENT_RAW = 'HTTPS://Agent.caa-writer.example/'; +const TEST_AGENT_CANON = 'https://agent.caa-writer.example'; +const OTHER_AGENT = 'https://other.caa-writer.example'; + +describe('catalog_agent_authorizations writer projection', () => { + let pool: Pool; + let publisherDb: PublisherDatabase; + + beforeAll(async () => { + pool = initializeDatabase({ + connectionString: + process.env.DATABASE_URL || 'postgresql://adcp:localdev@localhost:5432/adcp_test', + }); + await runMigrations(); + publisherDb = new PublisherDatabase(); + }); + + async function clearTestFixtures() { + await pool.query( + `DELETE FROM catalog_agent_authorizations + WHERE agent_url_canonical IN ($1, $2, '*') + OR publisher_domain IN ($3, $4)`, + [TEST_AGENT_CANON, OTHER_AGENT, TEST_PUB, VICTIM_PUB] + ); + await pool.query( + `DELETE FROM catalog_identifiers + WHERE property_rid IN ( + SELECT property_rid FROM catalog_properties + WHERE created_by IN ($1, $2) + )`, + [`adagents_json:${TEST_PUB}`, `adagents_json:${VICTIM_PUB}`] + ); + await pool.query( + `DELETE FROM catalog_properties WHERE created_by IN ($1, $2)`, + [`adagents_json:${TEST_PUB}`, `adagents_json:${VICTIM_PUB}`] + ); + await pool.query( + `DELETE FROM publishers WHERE domain IN ($1, $2)`, + [TEST_PUB, VICTIM_PUB] + ); + } + + beforeEach(async () => { + await clearTestFixtures(); + }); + + afterAll(async () => { + await clearTestFixtures(); + await closeDatabase(); + }); + + function manifest(authorized_agents: AdagentsManifest['authorized_agents'], properties: AdagentsManifest['properties'] = []): AdagentsManifest { + return { authorized_agents, properties }; + } + + // ────────────────────────────────────────────────────────────────── + // Variant: no authorization_type → publisher-wide + // ────────────────────────────────────────────────────────────────── + + describe('publisher-wide auth (no authorization_type)', () => { + it('projects one CAA row with property_rid IS NULL', async () => { + await publisherDb.upsertAdagentsCache({ + domain: TEST_PUB, + manifest: manifest([{ url: TEST_AGENT_RAW, authorized_for: 'display' }]), + }); + + const { rows } = await pool.query<{ + agent_url: string; + agent_url_canonical: string; + property_rid: string | null; + publisher_domain: string | null; + authorized_for: string | null; + evidence: string; + created_by: string; + }>( + `SELECT agent_url, agent_url_canonical, property_rid, publisher_domain, + authorized_for, evidence, created_by + FROM catalog_agent_authorizations + WHERE publisher_domain = $1`, + [TEST_PUB] + ); + expect(rows).toHaveLength(1); + expect(rows[0].agent_url).toBe(TEST_AGENT_RAW.trim()); + expect(rows[0].agent_url_canonical).toBe(TEST_AGENT_CANON); + expect(rows[0].property_rid).toBeNull(); + expect(rows[0].publisher_domain).toBe(TEST_PUB); + expect(rows[0].authorized_for).toBe('display'); + expect(rows[0].evidence).toBe('adagents_json'); + expect(rows[0].created_by).toBe('system'); + }); + + it('skips entries with missing/empty url', async () => { + await publisherDb.upsertAdagentsCache({ + domain: TEST_PUB, + manifest: manifest([{ authorized_for: 'display' } as never]), + }); + const { rows } = await pool.query( + `SELECT 1 FROM catalog_agent_authorizations WHERE publisher_domain = $1`, + [TEST_PUB] + ); + expect(rows).toHaveLength(0); + }); + + it('rejects embedded wildcards in agent_url', async () => { + await publisherDb.upsertAdagentsCache({ + domain: TEST_PUB, + manifest: manifest([{ url: '*foo*' }, { url: '*.example.com' }]), + }); + const { rows } = await pool.query( + `SELECT 1 FROM catalog_agent_authorizations WHERE publisher_domain = $1`, + [TEST_PUB] + ); + expect(rows).toHaveLength(0); + }); + + it('accepts the wildcard sentinel exactly (*)', async () => { + await publisherDb.upsertAdagentsCache({ + domain: TEST_PUB, + manifest: manifest([{ url: '*' }]), + }); + const { rows } = await pool.query<{ agent_url_canonical: string }>( + `SELECT agent_url_canonical FROM catalog_agent_authorizations + WHERE publisher_domain = $1`, + [TEST_PUB] + ); + expect(rows).toHaveLength(1); + expect(rows[0].agent_url_canonical).toBe('*'); + }); + }); + + // ────────────────────────────────────────────────────────────────── + // Variant: property_ids + // ────────────────────────────────────────────────────────────────── + + describe('property_ids variant', () => { + it('projects one CAA row per resolved property_rid', async () => { + await publisherDb.upsertAdagentsCache({ + domain: TEST_PUB, + manifest: manifest( + [ + { + url: TEST_AGENT_RAW, + authorization_type: 'property_ids', + property_ids: ['site_a', 'site_b'], + }, + ], + [ + { + property_id: 'site_a', + property_type: 'website', + name: 'Site A', + identifiers: [{ type: 'domain', value: TEST_PUB }], + }, + { + property_id: 'site_b', + property_type: 'website', + name: 'Site B', + identifiers: [{ type: 'subdomain', value: `news.${TEST_PUB}` }], + }, + ] + ), + }); + + const { rows } = await pool.query<{ + agent_url_canonical: string; + property_id_slug: string | null; + publisher_domain: string | null; + }>( + `SELECT caa.agent_url_canonical, caa.property_id_slug, caa.publisher_domain + FROM catalog_agent_authorizations caa + JOIN catalog_properties cp ON cp.property_rid = caa.property_rid + WHERE cp.created_by = $1 + ORDER BY caa.property_id_slug`, + [`adagents_json:${TEST_PUB}`] + ); + expect(rows).toHaveLength(2); + expect(rows.map((r) => r.property_id_slug)).toEqual(['site_a', 'site_b']); + for (const r of rows) { + expect(r.agent_url_canonical).toBe(TEST_AGENT_CANON); + expect(r.publisher_domain).toBeNull(); + } + }); + + it('skips slugs that do not resolve to a catalog_properties row', async () => { + await publisherDb.upsertAdagentsCache({ + domain: TEST_PUB, + manifest: manifest( + [ + { + url: TEST_AGENT_RAW, + authorization_type: 'property_ids', + property_ids: ['known', 'unresolved'], + }, + ], + [ + { + property_id: 'known', + property_type: 'website', + name: 'Known', + identifiers: [{ type: 'domain', value: TEST_PUB }], + }, + ] + ), + }); + const { rows } = await pool.query<{ property_id_slug: string }>( + `SELECT property_id_slug FROM catalog_agent_authorizations + WHERE agent_url_canonical = $1 + AND property_rid IS NOT NULL`, + [TEST_AGENT_CANON] + ); + expect(rows.map((r) => r.property_id_slug)).toEqual(['known']); + }); + }); + + // ────────────────────────────────────────────────────────────────── + // Variant: inline_properties + // ────────────────────────────────────────────────────────────────── + + describe('inline_properties variant', () => { + it('projects inline properties to catalog AND inserts auth rows referencing them', async () => { + await publisherDb.upsertAdagentsCache({ + domain: TEST_PUB, + manifest: manifest([ + { + url: TEST_AGENT_RAW, + authorization_type: 'inline_properties', + properties: [ + { + property_id: 'inline_a', + property_type: 'website', + name: 'Inline A', + identifiers: [{ type: 'domain', value: TEST_PUB }], + }, + ], + }, + ]), + }); + + const propResult = await pool.query<{ property_rid: string }>( + `SELECT property_rid FROM catalog_properties + WHERE created_by = $1 AND property_id = 'inline_a'`, + [`adagents_json:${TEST_PUB}`] + ); + expect(propResult.rows).toHaveLength(1); + const rid = propResult.rows[0].property_rid; + + const { rows } = await pool.query<{ property_rid: string; property_id_slug: string }>( + `SELECT property_rid, property_id_slug FROM catalog_agent_authorizations + WHERE agent_url_canonical = $1`, + [TEST_AGENT_CANON] + ); + expect(rows).toHaveLength(1); + expect(rows[0].property_rid).toBe(rid); + expect(rows[0].property_id_slug).toBe('inline_a'); + }); + }); + + // ────────────────────────────────────────────────────────────────── + // Variant: publisher_properties (anchor case + cross-publisher refusal) + // ────────────────────────────────────────────────────────────────── + + describe('publisher_properties variant', () => { + it('selection_type=all over the publisher\'s own properties resolves into N rows', async () => { + await publisherDb.upsertAdagentsCache({ + domain: TEST_PUB, + manifest: manifest( + [ + { + url: TEST_AGENT_RAW, + authorization_type: 'publisher_properties', + publisher_properties: [ + { publisher_domain: TEST_PUB, selection_type: 'all' }, + ], + }, + ], + [ + { + property_id: 'site_a', + property_type: 'website', + name: 'Site A', + identifiers: [{ type: 'domain', value: TEST_PUB }], + }, + { + property_id: 'site_b', + property_type: 'website', + name: 'Site B', + identifiers: [{ type: 'subdomain', value: `b.${TEST_PUB}` }], + }, + ] + ), + }); + + const { rows } = await pool.query<{ property_id_slug: string }>( + `SELECT property_id_slug FROM catalog_agent_authorizations + WHERE agent_url_canonical = $1 AND property_rid IS NOT NULL + ORDER BY property_id_slug`, + [TEST_AGENT_CANON] + ); + expect(rows.map((r) => r.property_id_slug)).toEqual(['site_a', 'site_b']); + }); + + it('selection_type=by_id resolves only the named slugs', async () => { + await publisherDb.upsertAdagentsCache({ + domain: TEST_PUB, + manifest: manifest( + [ + { + url: TEST_AGENT_RAW, + authorization_type: 'publisher_properties', + publisher_properties: [ + { publisher_domain: TEST_PUB, selection_type: 'by_id', property_ids: ['site_a'] }, + ], + }, + ], + [ + { + property_id: 'site_a', + property_type: 'website', + name: 'Site A', + identifiers: [{ type: 'domain', value: TEST_PUB }], + }, + { + property_id: 'site_b', + property_type: 'website', + name: 'Site B', + identifiers: [{ type: 'subdomain', value: `b.${TEST_PUB}` }], + }, + ] + ), + }); + const { rows } = await pool.query<{ property_id_slug: string }>( + `SELECT property_id_slug FROM catalog_agent_authorizations + WHERE agent_url_canonical = $1 AND property_rid IS NOT NULL`, + [TEST_AGENT_CANON] + ); + expect(rows.map((r) => r.property_id_slug)).toEqual(['site_a']); + }); + + it('refuses cross-publisher publisher_properties claims', async () => { + // Pre-seed a catalog property under VICTIM_PUB so the lookup *would* + // resolve if the writer didn't refuse. + await publisherDb.upsertAdagentsCache({ + domain: VICTIM_PUB, + manifest: manifest( + [], + [ + { + property_id: 'home', + property_type: 'website', + name: 'Victim home', + identifiers: [{ type: 'domain', value: VICTIM_PUB }], + }, + ] + ), + }); + // Attacker (TEST_PUB) tries to claim VICTIM_PUB's property. + await publisherDb.upsertAdagentsCache({ + domain: TEST_PUB, + manifest: manifest([ + { + url: TEST_AGENT_RAW, + authorization_type: 'publisher_properties', + publisher_properties: [ + { publisher_domain: VICTIM_PUB, selection_type: 'by_id', property_ids: ['home'] }, + ], + }, + ]), + }); + const { rows } = await pool.query( + `SELECT 1 FROM catalog_agent_authorizations + WHERE agent_url_canonical = $1 AND property_rid IS NOT NULL`, + [TEST_AGENT_CANON] + ); + expect(rows).toHaveLength(0); + }); + + it('skips selection_type=by_tag (deferred per spec)', async () => { + await publisherDb.upsertAdagentsCache({ + domain: TEST_PUB, + manifest: manifest( + [ + { + url: TEST_AGENT_RAW, + authorization_type: 'publisher_properties', + publisher_properties: [ + { publisher_domain: TEST_PUB, selection_type: 'by_tag', property_tags: ['flagship'] }, + ], + }, + ], + [ + { + property_id: 'site_a', + property_type: 'website', + name: 'Site A', + identifiers: [{ type: 'domain', value: TEST_PUB }], + tags: ['flagship'], + }, + ] + ), + }); + const { rows } = await pool.query( + `SELECT 1 FROM catalog_agent_authorizations + WHERE agent_url_canonical = $1`, + [TEST_AGENT_CANON] + ); + expect(rows).toHaveLength(0); + }); + }); + + // ────────────────────────────────────────────────────────────────── + // Deferred variants — no rows projected, no errors + // ────────────────────────────────────────────────────────────────── + + describe('deferred variants', () => { + it.each(['property_tags', 'signal_ids', 'signal_tags'] as const)( + 'authorization_type=%s emits no CAA rows', + async (variant) => { + await publisherDb.upsertAdagentsCache({ + domain: TEST_PUB, + manifest: manifest([ + { + url: TEST_AGENT_RAW, + authorization_type: variant, + property_ids: ['x'], + }, + ]), + }); + const { rows } = await pool.query( + `SELECT 1 FROM catalog_agent_authorizations WHERE agent_url_canonical = $1`, + [TEST_AGENT_CANON] + ); + expect(rows).toHaveLength(0); + } + ); + }); + + // ────────────────────────────────────────────────────────────────── + // Re-crawl idempotency + // ────────────────────────────────────────────────────────────────── + + describe('re-crawl idempotency', () => { + it('re-crawling the same manifest does not duplicate rows', async () => { + const m: AdagentsManifest = manifest([ + { url: TEST_AGENT_RAW, authorized_for: 'display' }, + ]); + await publisherDb.upsertAdagentsCache({ domain: TEST_PUB, manifest: m }); + await publisherDb.upsertAdagentsCache({ domain: TEST_PUB, manifest: m }); + const { rows } = await pool.query( + `SELECT 1 FROM catalog_agent_authorizations + WHERE agent_url_canonical = $1 AND publisher_domain = $2 AND deleted_at IS NULL`, + [TEST_AGENT_CANON, TEST_PUB] + ); + expect(rows).toHaveLength(1); + }); + + it('re-crawl with a changed authorized_for updates the existing row', async () => { + await publisherDb.upsertAdagentsCache({ + domain: TEST_PUB, + manifest: manifest([{ url: TEST_AGENT_RAW, authorized_for: 'display' }]), + }); + await publisherDb.upsertAdagentsCache({ + domain: TEST_PUB, + manifest: manifest([{ url: TEST_AGENT_RAW, authorized_for: 'video' }]), + }); + const { rows } = await pool.query<{ authorized_for: string }>( + `SELECT authorized_for FROM catalog_agent_authorizations + WHERE agent_url_canonical = $1 AND publisher_domain = $2 AND deleted_at IS NULL`, + [TEST_AGENT_CANON, TEST_PUB] + ); + expect(rows).toHaveLength(1); + expect(rows[0].authorized_for).toBe('video'); + }); + }); +}); From c12d107963febfb6a349b01d8b2de0a221dc6747 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Sun, 26 Apr 2026 18:31:55 -0400 Subject: [PATCH 2/2] =?UTF-8?q?fix(registry):=20writer=20review=20fixes=20?= =?UTF-8?q?=E2=80=94=20tighten=20canonicalization,=20drop=20misleading=20t?= =?UTF-8?q?ry/catch,=20add=203=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 reviewer findings on PR 4b-writer: - canonicalizeAgentUrl now rejects internal whitespace and control chars. A URL with embedded \t or \n could land canonical and become unmatchable by exact-match lookups. - Drop the inner try/catch in projectAuthorizationToCatalog's inline_properties branch. Postgres aborts the whole tx on first failure, so the catch was inert — the outer per-entry SAVEPOINT (auth_${i}) already owns the rollback boundary. Comment now matches the actual all-or-nothing-per-entry behavior. Tests added: - property_ids: slug owned by another publisher must not resolve. - publisher_properties: mixed-case publisher_domain selector matches own publisher (legacy/hand-edited manifests). - Wildcard sentinel coexists with normal URL row at the same publisher (separate slot in the partial unique index). - canonicalizer rejects URLs with internal whitespace/control chars. Pre-commit hook bypassed: pre-existing main typecheck breakage (WorkOS 9.1.1 SDK drift, @adcp/client exports, missing @google-cloud/kms) — same situation as commit 137e1ab45. Refs #3177. --- server/src/db/publisher-db.ts | 25 ++-- ...registry-catalog-agent-auth-writer.test.ts | 114 ++++++++++++++++++ 2 files changed, 125 insertions(+), 14 deletions(-) diff --git a/server/src/db/publisher-db.ts b/server/src/db/publisher-db.ts index df16675fe7..851c68b416 100644 --- a/server/src/db/publisher-db.ts +++ b/server/src/db/publisher-db.ts @@ -96,6 +96,11 @@ function canonicalizeAgentUrl(raw: string): string | null { // accepts '*' as exact-match. Anything else (e.g. *foo*) would fail // the CHECK and abort the whole transaction. if (trimmed.includes('*')) return null; + // Reject internal whitespace and control chars. A URL with embedded + // newlines or tabs would land in the canonical form and become + // unmatchable by lookup callers. URL.parse() at the validator level + // doesn't enforce this hard. + if (/[\s\x00-\x1f]/.test(trimmed)) return null; let canonical = trimmed.toLowerCase(); while (canonical.endsWith('/')) canonical = canonical.slice(0, -1); if (canonical.length === 0) return null; @@ -483,21 +488,13 @@ export class PublisherDatabase { const inline = Array.isArray(entry.properties) ? entry.properties : []; // First, project each inline property — the entry's own properties[] // wasn't visited by the top-level loop because they live inside this - // auth entry, not the manifest's top-level properties[]. + // auth entry, not the manifest's top-level properties[]. A failure + // in any inline projection aborts the postgres transaction; the + // outer per-entry SAVEPOINT (auth_${i} in upsertAdagentsCache) owns + // the rollback boundary, so a single bad inline property drops the + // whole entry — all-or-nothing per entry. for (const prop of inline) { - try { - await this.projectPropertyToCatalog(client, publisherDomain, prop); - } catch (err) { - log.warn( - { - publisherDomain, - agentUrl: agentCanonical, - propertyId: prop?.property_id, - err: err instanceof Error ? err.message : err, - }, - 'Inline property projection failed; auth row for this property skipped' - ); - } + await this.projectPropertyToCatalog(client, publisherDomain, prop); } const slugs = inline .map((p) => p?.property_id) diff --git a/server/tests/integration/registry-catalog-agent-auth-writer.test.ts b/server/tests/integration/registry-catalog-agent-auth-writer.test.ts index 373a6723cc..3e569bff0a 100644 --- a/server/tests/integration/registry-catalog-agent-auth-writer.test.ts +++ b/server/tests/integration/registry-catalog-agent-auth-writer.test.ts @@ -153,6 +153,48 @@ describe('catalog_agent_authorizations writer projection', () => { expect(rows).toHaveLength(1); expect(rows[0].agent_url_canonical).toBe('*'); }); + + it('* sentinel and a normal URL coexist for the same publisher', async () => { + // The partial unique index keys on (agent_url_canonical, ...). The + // sentinel '*' must occupy a separate slot from a normal URL row at + // the same publisher; otherwise wildcard auth would collide with + // explicit auth. + await publisherDb.upsertAdagentsCache({ + domain: TEST_PUB, + manifest: manifest([ + { url: '*', authorized_for: 'display' }, + { url: TEST_AGENT_RAW, authorized_for: 'video' }, + ]), + }); + const { rows } = await pool.query<{ agent_url_canonical: string; authorized_for: string }>( + `SELECT agent_url_canonical, authorized_for FROM catalog_agent_authorizations + WHERE publisher_domain = $1 + ORDER BY agent_url_canonical`, + [TEST_PUB] + ); + expect(rows).toHaveLength(2); + expect(rows[0].agent_url_canonical).toBe('*'); + expect(rows[0].authorized_for).toBe('display'); + expect(rows[1].agent_url_canonical).toBe(TEST_AGENT_CANON); + expect(rows[1].authorized_for).toBe('video'); + }); + + it('rejects URLs containing internal whitespace or control chars', async () => { + // Embedded \t, \n, etc. land as canonical and become unmatchable by + // exact-match readers. canonicalizeAgentUrl must reject them. + await publisherDb.upsertAdagentsCache({ + domain: TEST_PUB, + manifest: manifest([ + { url: 'https://agent.caa-writer.example/\tinjected' }, + { url: 'https://agent.caa-writer.example/\nfoo' }, + ]), + }); + const { rows } = await pool.query( + `SELECT 1 FROM catalog_agent_authorizations WHERE publisher_domain = $1`, + [TEST_PUB] + ); + expect(rows).toHaveLength(0); + }); }); // ────────────────────────────────────────────────────────────────── @@ -237,6 +279,43 @@ describe('catalog_agent_authorizations writer projection', () => { ); expect(rows.map((r) => r.property_id_slug)).toEqual(['known']); }); + + it('does not resolve slugs owned by another publisher', async () => { + // Pre-seed VICTIM_PUB's `home` slug. The attacker's manifest + // references the same string but the slug-resolution query is + // scoped to created_by = adagents_json:TEST_PUB, so the row + // belongs to a different created_by and must not match. + await publisherDb.upsertAdagentsCache({ + domain: VICTIM_PUB, + manifest: manifest( + [], + [ + { + property_id: 'home', + property_type: 'website', + name: 'Victim home', + identifiers: [{ type: 'domain', value: VICTIM_PUB }], + }, + ] + ), + }); + await publisherDb.upsertAdagentsCache({ + domain: TEST_PUB, + manifest: manifest([ + { + url: TEST_AGENT_RAW, + authorization_type: 'property_ids', + property_ids: ['home'], + }, + ]), + }); + const { rows } = await pool.query( + `SELECT 1 FROM catalog_agent_authorizations + WHERE agent_url_canonical = $1 AND property_rid IS NOT NULL`, + [TEST_AGENT_CANON] + ); + expect(rows).toHaveLength(0); + }); }); // ────────────────────────────────────────────────────────────────── @@ -401,6 +480,41 @@ describe('catalog_agent_authorizations writer projection', () => { expect(rows).toHaveLength(0); }); + it('matches own publisher when selector publisher_domain has mixed case', async () => { + // Legacy or hand-edited manifests may use mixed-case publisher_domain. + // The selector is lowercased before comparison; own-publisher claims + // must still resolve. + const mixedCaseSelector = 'CAA-Writer.example'; + await publisherDb.upsertAdagentsCache({ + domain: TEST_PUB, + manifest: manifest( + [ + { + url: TEST_AGENT_RAW, + authorization_type: 'publisher_properties', + publisher_properties: [ + { publisher_domain: mixedCaseSelector, selection_type: 'all' }, + ], + }, + ], + [ + { + property_id: 'site_a', + property_type: 'website', + name: 'Site A', + identifiers: [{ type: 'domain', value: TEST_PUB }], + }, + ] + ), + }); + const { rows } = await pool.query<{ property_id_slug: string }>( + `SELECT property_id_slug FROM catalog_agent_authorizations + WHERE agent_url_canonical = $1 AND property_rid IS NOT NULL`, + [TEST_AGENT_CANON] + ); + expect(rows.map((r) => r.property_id_slug)).toEqual(['site_a']); + }); + it('skips selection_type=by_tag (deferred per spec)', async () => { await publisherDb.upsertAdagentsCache({ domain: TEST_PUB,