From d3869e669489686149965e557602807109d73742 Mon Sep 17 00:00:00 2001 From: Jakob Wennberg Date: Wed, 26 Aug 2026 13:34:27 +0200 Subject: [PATCH] fix(api): register the v1 stamp endpoint scope and derive the webhook event catalogue from one source (#1930) POST /api/v1/companies/{companyId}/inbox-items/{id}/stamp registered itself with scope documents:write but had no V1_ENDPOINT_SCOPES entry, and the wrapper resolves the required scope from that map before it validates the bearer token, so the route answered NOT_FOUND to every caller. Add the entry, drop the three phantom entries that had no route (GET openapi.yaml, GET companies/:companyId, GET companies/:companyId/events), and add a parity test that pins the scope map to the endpoint registry in both directions, checks every pattern against an existing route file, and checks every v1 route file is imported by load-routes.ts. The webhook event catalogue was hand-copied in three places and had drifted: the fan-out handler delivered 28 events while the v1 create enum, the OpenAPI spec, the generated agent skill and the docs page listed 24, so the four reconciliation.* events could not be subscribed to. lib/webhooks/public-events.ts is now the single source; the handler set, the Zod enum and the docs section derive from it, with tests that pin each surface to the catalogue. The PATCH webhook docs no longer tell agents to delete and recreate a webhook to rotate its secret: POST .../rotate-secret exists. Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Fable 5 --- DECISIONS.md | 2 + .../[id]/stamp/__tests__/route.test.ts | 158 ++++++++++++++++++ .../[companyId]/webhooks/[id]/route.ts | 7 +- .../companies/[companyId]/webhooks/route.ts | 33 +--- .../__tests__/scope-registry-parity.test.ts | 118 +++++++++++++ lib/auth/__tests__/scopes.test.ts | 16 +- lib/auth/scopes.ts | 18 +- lib/docs/content/webhooks.ts | 59 +++---- .../__tests__/handler-subscriptions.test.ts | 29 ++++ lib/webhooks/__tests__/public-events.test.ts | 47 ++++++ lib/webhooks/handler.ts | 42 +---- lib/webhooks/public-events.ts | 129 ++++++++++++++ skills/accounted-api/references/webhooks.md | 6 +- 13 files changed, 547 insertions(+), 117 deletions(-) create mode 100644 app/api/v1/companies/[companyId]/inbox-items/[id]/stamp/__tests__/route.test.ts create mode 100644 lib/api/v1/__tests__/scope-registry-parity.test.ts create mode 100644 lib/webhooks/__tests__/handler-subscriptions.test.ts create mode 100644 lib/webhooks/__tests__/public-events.test.ts create mode 100644 lib/webhooks/public-events.ts diff --git a/DECISIONS.md b/DECISIONS.md index f3aa3763..d74fb745 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1249,4 +1249,6 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-08-26] RFC 9728 protected-resource metadata is served at THREE locations (root, path-based /.well-known/oauth-protected-resource/, and /.well-known/oauth-protected-resource): Claude.ai's connector setup derives the metadata URL from the server URL and fetches it before any 401, so the root document our WWW-Authenticate header points at was not enough ('Authorization with Accounted failed' with only 404s in the logs). One builder, three routes; the path-based route answers 404 for any path other than the MCP endpoint so no phantom resource is advertised. [2026-08-26] defer_invoice_booking (#967) now gates booking on every door, not just the dashboard: MCP send_invoice / mark_invoice_sent / create_supplier_invoice_from_inbox, v1 invoices send / mark-sent and supplier-invoices create, and the inbox convert route all checked accounting_method === 'accrual' and posted a verifikat at issue for deferred companies. All six now call booksInvoicesOnIssue() (lib/bookkeeping/booking-mode.ts), the same helper the dashboard routes use, so the setting has one meaning. No data repair attempted: vouchers already posted for deferred companies through these doors are legitimate entries and stay. [2026-08-20] The swedish-e-invoicing skill now names Upphandlingsmyndigheten as Sweden Peppol Authority across all eight files, not just the one that was flagged: the handover completed 1 July 2026 (regeringsbeslut Fi2025/01826) and the skill was written in future tense, so a partial fix would have left the atom internally contradictory and still pointed agents at peppol@digg.se. Four digg.se URLs were repointed to their verified 301 targets on upphandlingsmyndigheten.se; the fifth, DIGG Peppol testbadd, is a hard 404 with no redirect and no successor page at the new authority, so it was replaced with the SFTI Validex verification service (https://sfti.validex.net/) rather than left dead or guessed at. Historical attributions (Q4 2025 traffic statistics, the 0007:2021006883 Peppol-ID example) deliberately still say DIGG because they were accurate when published. +[2026-08-26] Webhook event catalogue lives in lib/webhooks/public-events.ts (grouped, with docs prose) and the fan-out handler set, the v1 create enum (so the OpenAPI spec and skills/accounted-api), and the docs page all derive from it: the enum and the docs had drifted to 24 of the 28 events the handler delivered, so the four reconciliation.* events were rejected at subscribe time. No API_V1_VERSION bump: the changelog already lists them as additive. +[2026-08-26] Removed the phantom V1_ENDPOINT_SCOPES entries GET /api/v1/openapi.yaml, GET /api/v1/companies/:companyId and GET /api/v1/companies/:companyId/events instead of building the routes: no route file, registry entry, docs, skill or test referenced them, and the new scope-registry-parity test needs the map to describe only what exists. A company-detail GET can be added later with its entry in the same PR. [2026-08-26] gnubok_connect_bank / gnubok_connect_skatteverket moved from catalogVisibility 'search' to the default catalog: Claude.ai can only invoke tools present in tools/list, so search-only tools are discover-only there and the onboarding skill's steps 3-4 dead-ended on client-side tool-not-found (verified via event_log: the server never received the calls). Search-only visibility remains fine for tools an agent reads about before asking the user, but anything a skill instructs the agent to CALL must be in the default catalog. diff --git a/app/api/v1/companies/[companyId]/inbox-items/[id]/stamp/__tests__/route.test.ts b/app/api/v1/companies/[companyId]/inbox-items/[id]/stamp/__tests__/route.test.ts new file mode 100644 index 00000000..bc9aa213 --- /dev/null +++ b/app/api/v1/companies/[companyId]/inbox-items/[id]/stamp/__tests__/route.test.ts @@ -0,0 +1,158 @@ +/** + * Tests for POST /api/v1/companies/{companyId}/inbox-items/{id}/stamp through + * the real withApiV1 wrapper (auth, scope, membership, idempotency) with the + * Supabase client mocked per table. + * + * The 401 case is the regression guard: the route had no V1_ENDPOINT_SCOPES + * entry, so the wrapper answered NOT_FOUND to every caller (valid key or not) + * before this file existed. + */ +import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' + +beforeAll(() => { + if (process.env.NODE_ENV !== 'test') throw new Error('NODE_ENV=test required') + process.env.NEXT_PUBLIC_SUPABASE_URL ||= 'http://localhost:54321' + process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY ||= 'test-anon-key' +}) + +vi.mock('@/lib/auth/api-keys', async () => { + const actual = await vi.importActual('@/lib/auth/api-keys') + return { ...actual, validateApiKey: vi.fn(), createServiceClientNoCookies: vi.fn() } +}) +vi.mock('@supabase/supabase-js', async () => { + const actual = await vi.importActual('@supabase/supabase-js') + return { ...actual, createClient: vi.fn().mockReturnValue({}) } +}) + +import { validateApiKey, createServiceClientNoCookies } from '@/lib/auth/api-keys' +import { POST } from '../route' + +const mockValidate = validateApiKey as ReturnType +const mockServiceClient = createServiceClientNoCookies as ReturnType + +type MockResult = { data?: unknown; error?: unknown } +function makeFlexibleSupabase(byTable: Record) { + const queues = new Map() + for (const [t, val] of Object.entries(byTable)) queues.set(t, Array.isArray(val) ? [...val] : [val]) + const buildChain = (table: string): unknown => { + const handler: ProxyHandler = { + get(_target, prop) { + if (prop === 'then') { + return (resolve: (v: unknown) => void) => { + const q = queues.get(table) + const next = q && q.length > 1 ? q.shift()! : (q?.[0] ?? { data: null, error: null }) + resolve(next) + } + } + return (..._args: unknown[]) => buildChain(table) + }, + } + return new Proxy({}, handler) + } + return { from: vi.fn((table: string) => buildChain(table)) } +} + +const COMPANY_ID = 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa' +const ITEM_ID = '22222222-2222-4222-8222-222222222222' +const JE_ID = '44444444-4444-4444-8444-444444444444' +const OTHER_JE_ID = '55555555-5555-4555-8555-555555555555' +const url = (itemId = ITEM_ID) => `http://localhost/api/v1/companies/${COMPANY_ID}/inbox-items/${itemId}/stamp` + +function req(init: { body?: unknown; idem?: boolean; itemId?: string; auth?: boolean } = {}): Request { + const headers: Record = { 'Content-Type': 'application/json' } + if (init.auth !== false) headers.Authorization = 'Bearer test-fixture-not-a-real-key' + if (init.idem !== false) headers['Idempotency-Key'] = `idem-${Math.random().toString(36).slice(2)}-aaaa-4abc-8def-1234567890ab` + return new Request(url(init.itemId), { + method: 'POST', + headers, + body: init.body !== undefined ? JSON.stringify(init.body) : undefined, + }) +} + +function authOk(scopes: string[]) { + mockValidate.mockResolvedValue({ valid: true, userId: 'user-1', keyId: 'key-1', keyName: 'Test key', scopes, mode: 'live' }) +} + +const params = (id = ITEM_ID) => ({ params: Promise.resolve({ companyId: COMPANY_ID, id }) }) + +function withTables(tables: Record) { + mockServiceClient.mockReturnValue( + makeFlexibleSupabase({ + company_members: { data: { role: 'owner' } }, + idempotency_keys: { data: null }, + ...tables, + }), + ) +} + +describe('POST /api/v1/companies/{companyId}/inbox-items/{id}/stamp', () => { + beforeEach(() => { + vi.clearAllMocks() + withTables({ + invoice_inbox_items: { data: { id: ITEM_ID, created_journal_entry_id: null } }, + journal_entries: { data: { id: JE_ID } }, + }) + }) + + it('401 without a bearer token and 401 with an invalid key (not 404: the endpoint is registered)', async () => { + mockValidate.mockResolvedValue({ valid: false, error: 'invalid' }) + const noAuth = await POST(req({ body: { journal_entry_id: JE_ID }, auth: false }), params()) + expect(noAuth.status).toBe(401) + const badKey = await POST(req({ body: { journal_entry_id: JE_ID } }), params()) + expect(badKey.status).toBe(401) + expect((await badKey.json()).error.code).not.toBe('NOT_FOUND') + }) + + it('403 without documents:write', async () => { + authOk(['documents:read']) + const res = await POST(req({ body: { journal_entry_id: JE_ID } }), params()) + expect(res.status).toBe(403) + }) + + it('400 without an Idempotency-Key, on a malformed body, and on a non-UUID item id', async () => { + authOk(['documents:write']) + expect((await POST(req({ body: { journal_entry_id: JE_ID }, idem: false }), params())).status).toBe(400) + expect((await POST(req({ body: {} }), params())).status).toBe(400) + expect((await POST(req({ body: { journal_entry_id: JE_ID, extra: 1 } }), params())).status).toBe(400) + expect((await POST(req({ body: { journal_entry_id: JE_ID }, itemId: 'not-a-uuid' }), params('not-a-uuid'))).status).toBe(400) + }) + + it('404 when the inbox item or the journal entry is not in the company', async () => { + authOk(['documents:write']) + withTables({ invoice_inbox_items: { data: null }, journal_entries: { data: { id: JE_ID } } }) + const noItem = await POST(req({ body: { journal_entry_id: JE_ID } }), params()) + expect(noItem.status).toBe(404) + expect((await noItem.json()).error.details.resource).toBe('inbox_item') + + withTables({ invoice_inbox_items: { data: { id: ITEM_ID, created_journal_entry_id: null } }, journal_entries: { data: null } }) + const noJe = await POST(req({ body: { journal_entry_id: JE_ID } }), params()) + expect(noJe.status).toBe(404) + expect((await noJe.json()).error.details.resource).toBe('journal_entry') + }) + + it('stamps an unstamped item and returns the new link', async () => { + authOk(['documents:write']) + const res = await POST(req({ body: { journal_entry_id: JE_ID } }), params()) + expect(res.status).toBe(200) + const body = await res.json() + expect(body.data).toEqual({ id: ITEM_ID, created_journal_entry_id: JE_ID }) + }) + + it('is idempotent when the item is already stamped with the same entry', async () => { + authOk(['documents:write']) + withTables({ invoice_inbox_items: { data: { id: ITEM_ID, created_journal_entry_id: JE_ID } }, journal_entries: { data: { id: JE_ID } } }) + const res = await POST(req({ body: { journal_entry_id: JE_ID } }), params()) + expect(res.status).toBe(200) + expect((await res.json()).data.created_journal_entry_id).toBe(JE_ID) + }) + + it('409 when the item is already stamped with a different entry', async () => { + authOk(['documents:write']) + withTables({ invoice_inbox_items: { data: { id: ITEM_ID, created_journal_entry_id: OTHER_JE_ID } }, journal_entries: { data: { id: JE_ID } } }) + const res = await POST(req({ body: { journal_entry_id: JE_ID } }), params()) + expect(res.status).toBe(409) + const body = await res.json() + expect(body.error.code).toBe('CONFLICT') + expect(body.error.details.current_journal_entry_id).toBe(OTHER_JE_ID) + }) +}) diff --git a/app/api/v1/companies/[companyId]/webhooks/[id]/route.ts b/app/api/v1/companies/[companyId]/webhooks/[id]/route.ts index 5064536f..da3be5bf 100644 --- a/app/api/v1/companies/[companyId]/webhooks/[id]/route.ts +++ b/app/api/v1/companies/[companyId]/webhooks/[id]/route.ts @@ -4,8 +4,8 @@ * GET : return the full webhook row (no secret). * PATCH : update name, description, webhook_url, active. Cannot change * event_type (immutable: would require re-pinning api_version). - * Cannot rotate the secret here (separate flow, deferred to - * Phase 6 follow-up). + * Cannot rotate the secret here: that is POST .../rotate-secret + * (see ./rotate-secret/route.ts). * DELETE: hard delete the webhook. The webhook_deliveries.webhook_id FK * is ON DELETE SET NULL (declared in migration 20260515170000), * so the delivery audit trail SURVIVES webhook deletion @@ -124,7 +124,8 @@ registerEndpoint({ description: 'Update the URL, name, description, or active flag. event_type is immutable: delete and recreate to change it. Setting active=false manually pauses delivery without deleting; setting active=true clears any disabled_at/disabled_reason set by the auto-disable on HTTP 410.', useWhen: 'You need to point an existing webhook at a new URL or temporarily pause delivery.', - doNotUseFor: 'Rotating the signing secret (delete and recreate). Changing event_type.', + doNotUseFor: + 'Rotating the signing secret: use POST /webhooks/{id}/rotate-secret, which issues a fresh secret in place and keeps the webhook id and delivery history. Changing event_type: delete and recreate.', pitfalls: [ 'Re-enabling a webhook (active: true) does NOT replay deliveries that went to dead status while it was disabled: those need POST /webhook-deliveries/{id}/retry.', ], diff --git a/app/api/v1/companies/[companyId]/webhooks/route.ts b/app/api/v1/companies/[companyId]/webhooks/route.ts index 0aafe5dd..1965b639 100644 --- a/app/api/v1/companies/[companyId]/webhooks/route.ts +++ b/app/api/v1/companies/[companyId]/webhooks/route.ts @@ -21,33 +21,12 @@ import { generateWebhookSecret } from '@/lib/webhooks/signing' import { validateWebhookUrl } from '@/lib/webhooks/url-guard' import { API_V1_VERSION } from '@/lib/api/v1/version' import { hasScope } from '@/lib/auth/api-keys' +import { PUBLIC_WEBHOOK_EVENTS } from '@/lib/webhooks/public-events' -const WEBHOOK_EVENT_TYPES = z.enum([ - 'invoice.created', - 'invoice.sent', - 'invoice.paid', - 'credit_note.created', - 'customer.created', - 'supplier.created', - 'supplier_invoice.registered', - 'supplier_invoice.approved', - 'supplier_invoice.paid', - 'supplier_invoice.credited', - 'supplier_invoice.uncredited', - 'transaction.categorized', - 'transaction.reconciled', - 'journal_entry.committed', - 'journal_entry.reversed', - 'journal_entry.corrected', - 'period.locked', - 'period.unlocked', - 'period.year_closed', - 'salary_run.created', - 'salary_run.approved', - 'salary_run.booked', - 'agi.generated', - 'document.uploaded', -]) +// Derived from the single catalogue the fan-out handler and the docs page +// also read, so the events an agent can subscribe to are exactly the events +// that get delivered. +const WEBHOOK_EVENT_TYPES = z.enum(PUBLIC_WEBHOOK_EVENTS) const CreateWebhookSchema = z.object({ event_type: WEBHOOK_EVENT_TYPES, @@ -175,7 +154,7 @@ registerEndpoint({ doNotUseFor: 'Subscribing to internal MCP telemetry events (mcp.tool_called etc. are not delivered as webhooks). Replacing an existing webhook URL: use PATCH instead.', pitfalls: [ - 'The secret is returned exactly once. If lost, delete and recreate the webhook.', + 'The secret is returned exactly once. If lost, rotate it with POST /webhooks/{id}/rotate-secret: a fresh secret is issued in place, the webhook id and delivery history are kept.', 'Delivery is at-least-once with exponential backoff (1m / 5m / 30m / 2h / 12h / 24h / 48h). Receivers MUST be idempotent.', 'HTTP 410 from your receiver auto-disables the webhook (sets active=false + disabled_reason).', ], diff --git a/lib/api/v1/__tests__/scope-registry-parity.test.ts b/lib/api/v1/__tests__/scope-registry-parity.test.ts new file mode 100644 index 00000000..c5ccdc99 --- /dev/null +++ b/lib/api/v1/__tests__/scope-registry-parity.test.ts @@ -0,0 +1,118 @@ +/** + * Scope-map / registry parity guard. + * + * `withApiV1` resolves the required scope for a request from + * `V1_ENDPOINT_SCOPES` (lib/auth/scopes.ts), NOT from the endpoint registry, + * and answers NOT_FOUND when the map has no entry, before it even validates + * the bearer token. So a route that registers itself but forgets the map + * entry is live in the OpenAPI spec and the generated agent skill yet 404s + * for every caller. That is exactly what happened to + * POST .../inbox-items/{id}/stamp. + * + * This test pins the two sources to each other in both directions, checks + * that every pattern points at a route file that exists, and that every v1 + * route file is imported by load-routes.ts (otherwise it is invisible to + * this test and to the spec). + */ + +import { existsSync, readdirSync, readFileSync, statSync } from 'node:fs' +import { join } from 'node:path' +import { fileURLToPath } from 'node:url' +import { describe, expect, it } from 'vitest' +import { V1_ENDPOINT_SCOPES, V1_PUBLIC_ENDPOINTS } from '@/lib/auth/scopes' +import { listEndpoints } from '../registry' +// Side-effect import: populates the ENDPOINTS registry from every route file. +import '../load-routes' + +const REPO_ROOT = fileURLToPath(new URL('../../../../', import.meta.url)) +const V1_APP_DIR = join(REPO_ROOT, 'app', 'api', 'v1') + +/** + * Public endpoints that legitimately have no registry entry. The OpenAPI + * document is generated FROM the registry, so it cannot register itself. + */ +const PUBLIC_WITHOUT_REGISTRY_ENTRY = new Set(['GET /api/v1/openapi.json']) + +/** + * Route files under app/api/v1 that are not part of the registry. Same + * exception as above, in file form. + */ +const ROUTE_FILES_WITHOUT_REGISTRY_ENTRY = new Set(['app/api/v1/openapi.json/route.ts']) + +function endpointKey(e: { method: string; path: string }): string { + return `${e.method} ${e.path}` +} + +/** `POST /api/v1/companies/:companyId/x/:id` to `app/api/v1/companies/[companyId]/x/[id]/route.ts`. */ +function routeFileFor(pattern: string): string { + const path = pattern.split(' ', 2)[1] + return join('app', path.replace(/:([^/]+)/g, '[$1]'), 'route.ts') +} + +function walkRouteFiles(dir: string): string[] { + const out: string[] = [] + for (const name of readdirSync(dir)) { + if (name === '__tests__') continue + const full = join(dir, name) + if (statSync(full).isDirectory()) out.push(...walkRouteFiles(full)) + else if (name === 'route.ts') out.push(full) + } + return out +} + +const registered = listEndpoints() +const registeredByKey = new Map(registered.map((e) => [endpointKey(e), e])) + +describe('V1_ENDPOINT_SCOPES <-> endpoint registry parity', () => { + it('has a scope-map entry, with the same scope, for every registered non-public endpoint', () => { + const problems = registered + .filter((e) => e.scope !== null) + .flatMap((e) => { + const key = endpointKey(e) + const mapped = V1_ENDPOINT_SCOPES[key] + if (mapped === undefined) return [`${key}: registered with scope ${e.scope} but missing from V1_ENDPOINT_SCOPES (the wrapper answers 404)`] + if (mapped !== e.scope) return [`${key}: registry says ${e.scope}, V1_ENDPOINT_SCOPES says ${mapped}`] + return [] + }) + expect(problems).toEqual([]) + }) + + it('lists every registered public (scope: null) endpoint in V1_PUBLIC_ENDPOINTS', () => { + const problems = registered + .filter((e) => e.scope === null) + .map(endpointKey) + .filter((key) => !V1_PUBLIC_ENDPOINTS.includes(key)) + expect(problems).toEqual([]) + }) + + it('has a registered endpoint behind every V1_ENDPOINT_SCOPES entry', () => { + const phantoms = Object.keys(V1_ENDPOINT_SCOPES).filter((key) => !registeredByKey.has(key)) + expect(phantoms).toEqual([]) + }) + + it('has a registered public endpoint (or a documented exception) behind every V1_PUBLIC_ENDPOINTS entry', () => { + const problems = V1_PUBLIC_ENDPOINTS.flatMap((key) => { + if (PUBLIC_WITHOUT_REGISTRY_ENTRY.has(key)) return [] + const def = registeredByKey.get(key) + if (!def) return [`${key}: in V1_PUBLIC_ENDPOINTS but not registered`] + if (def.scope !== null) return [`${key}: in V1_PUBLIC_ENDPOINTS but registered with scope ${def.scope}`] + return [] + }) + expect(problems).toEqual([]) + }) + + it('points every scope-map and public pattern at a route file that exists', () => { + const patterns = [...Object.keys(V1_ENDPOINT_SCOPES), ...V1_PUBLIC_ENDPOINTS] + const missing = patterns.filter((key) => !existsSync(join(REPO_ROOT, routeFileFor(key)))) + expect(missing).toEqual([]) + }) + + it('imports every app/api/v1 route file from load-routes.ts', () => { + const loader = readFileSync(join(REPO_ROOT, 'lib', 'api', 'v1', 'load-routes.ts'), 'utf8') + const notLoaded = walkRouteFiles(V1_APP_DIR) + .map((full) => full.slice(REPO_ROOT.length).replace(/^\/+/, '')) + .filter((rel) => !ROUTE_FILES_WITHOUT_REGISTRY_ENTRY.has(rel)) + .filter((rel) => !loader.includes(`'@/${rel.replace(/\.ts$/, '')}'`)) + expect(notLoaded).toEqual([]) + }) +}) diff --git a/lib/auth/__tests__/scopes.test.ts b/lib/auth/__tests__/scopes.test.ts index a2c56fa7..7dfbe002 100644 --- a/lib/auth/__tests__/scopes.test.ts +++ b/lib/auth/__tests__/scopes.test.ts @@ -16,8 +16,20 @@ describe('resolveRequiredScope', () => { it('resolves :param patterns to a single scope', () => { expect( - resolveRequiredScope('GET', '/api/v1/companies/8fd5b1f4-1111-2222-3333-444455556666'), - ).toBe('companies:read') + resolveRequiredScope( + 'GET', + '/api/v1/companies/8fd5b1f4-1111-2222-3333-444455556666/customers/0b6c7d8e-1111-2222-3333-444455556666', + ), + ).toBe('customers:read') + }) + + it('resolves the inbox-items stamp verb (previously unregistered: 404 with a valid key)', () => { + expect( + resolveRequiredScope( + 'POST', + '/api/v1/companies/8fd5b1f4-1111-2222-3333-444455556666/inbox-items/0b6c7d8e-1111-2222-3333-444455556666/stamp', + ), + ).toBe('documents:write') }) it('returns null for unknown paths', () => { diff --git a/lib/auth/scopes.ts b/lib/auth/scopes.ts index 7e4c8a70..ba3a2642 100644 --- a/lib/auth/scopes.ts +++ b/lib/auth/scopes.ts @@ -10,7 +10,15 @@ * Endpoints not listed here are public (no auth): only the discovery routes * (`/llms.txt`, `/.well-known/skills`, `/api/v1/health`, `/api/v1/openapi.json`) * fall into that bucket. Everything else under `/api/v1/` MUST be in this map - * or the wrapper will refuse the request with INSUFFICIENT_SCOPE. + * or the wrapper answers NOT_FOUND before it even looks at the bearer token + * (`resolveRequiredScope` returns null for an unknown path). + * + * This map and the endpoint registry (`lib/api/v1/registry.ts`, populated by + * `load-routes.ts`) are kept in lock-step by + * `lib/api/v1/__tests__/scope-registry-parity.test.ts`: every registered + * endpoint needs an entry with the same scope, and every entry needs a + * registered endpoint. The inbox-items stamp route shipped without an entry + * and answered 404 to valid keys until that test existed. */ import type { ApiKeyScope } from './api-keys' @@ -22,7 +30,6 @@ import type { ApiKeyScope } from './api-keys' export const V1_PUBLIC_ENDPOINTS: ReadonlyArray = [ 'GET /api/v1/health', 'GET /api/v1/openapi.json', - 'GET /api/v1/openapi.yaml', ] /** @@ -40,7 +47,6 @@ export const V1_ENDPOINT_SCOPES: Record = { 'GET /api/v1/companies': 'companies:read', // Issue #1814: programmatic company creation (partner provisioning, agents). 'POST /api/v1/companies': 'companies:write', - 'GET /api/v1/companies/:companyId': 'companies:read', // Issue #1348: company-settings write (same field set as the MCP tool // gnubok_update_company_settings; direct write, no staging). 'PATCH /api/v1/companies/:companyId/settings': 'companies:write', @@ -48,9 +54,6 @@ export const V1_ENDPOINT_SCOPES: Record = { // Operations (async long-running tasks) 'GET /api/v1/operations/:id': 'operations:read', - // Events (webhook fallback / event log polling) - 'GET /api/v1/companies/:companyId/events': 'events:read', - // Customers (Phase 2 PR-A: reads; Phase 2 PR-B-1: writes) 'GET /api/v1/companies/:companyId/customers': 'customers:read', 'GET /api/v1/companies/:companyId/customers/:id': 'customers:read', @@ -117,6 +120,9 @@ export const V1_ENDPOINT_SCOPES: Record = { 'POST /api/v1/companies/:companyId/documents': 'documents:write', 'GET /api/v1/companies/:companyId/documents/:id/download': 'documents:read', 'POST /api/v1/companies/:companyId/documents/:id/link': 'documents:write', + // Inbox item stamp: closes an invoice_inbox_items row against the JE it + // was booked to. Rides documents:write like the link verb it complements. + 'POST /api/v1/companies/:companyId/inbox-items/:id/stamp': 'documents:write', // Phase 3: transactions + reconciliation vertical. // Reads diff --git a/lib/docs/content/webhooks.ts b/lib/docs/content/webhooks.ts index 1cc5ffa1..9ce23bfb 100644 --- a/lib/docs/content/webhooks.ts +++ b/lib/docs/content/webhooks.ts @@ -1,3 +1,22 @@ +import { + PUBLIC_WEBHOOK_EVENT_GROUPS, + type PublicWebhookEventGroup, +} from '@/lib/webhooks/public-events' + +/** + * The "Event types" section, rendered from the same catalogue the fan-out + * handler subscribes to and the v1 create schema validates against. + */ +function renderEventTypes(): string { + return PUBLIC_WEBHOOK_EVENT_GROUPS.map((group: PublicWebhookEventGroup) => { + const heading = group.note ? `**${group.title}** *(${group.note})*` : `**${group.title}**` + const lines = group.events.map((event) => + event.description ? `- \`${event.type}\`: ${event.description}` : `- \`${event.type}\``, + ) + return [heading, ...lines].join('\n') + }).join('\n\n') +} + export const WEBHOOKS_MD = `# Webhooks > Receive HMAC-signed POST notifications when state changes in Accounted: invoices paid, journal entries committed, periods locked, salary runs booked, AGI files generated. At-least-once delivery with exponential backoff over ~87 hours (about 3.6 days). @@ -16,45 +35,7 @@ If you've used [Stripe webhooks](https://docs.stripe.com/webhooks), the model is The following event types are deliverable as webhooks. Subscribing to a type that requires elevated scope (\`salary_run.*\` and \`agi.*\` need \`payroll:read\`) returns \`INSUFFICIENT_SCOPE\` at registration time. -**Invoicing** -- \`invoice.created\`: draft invoice created -- \`invoice.sent\`: invoice marked sent (email delivered or external) -- \`invoice.paid\`: invoice fully paid -- \`credit_note.created\`: credit note issued - -**AP / suppliers** -- \`supplier.created\` -- \`supplier_invoice.registered\` -- \`supplier_invoice.approved\` -- \`supplier_invoice.paid\` -- \`supplier_invoice.credited\` -- \`supplier_invoice.uncredited\`: credit reversal - -**Customers** -- \`customer.created\` - -**Bookkeeping** -- \`journal_entry.committed\`: voucher posted (immutable from this point) -- \`journal_entry.reversed\`: storno entry posted -- \`journal_entry.corrected\`: rättelse via \`correctEntry\` (BFL 5 kap 5 §) - -**Transactions** -- \`transaction.categorized\`: bank transaction assigned an account + tax code -- \`transaction.reconciled\`: transaction matched to a posted entry - -**Periods** -- \`period.locked\`: fiscal period closed for writes -- \`period.unlocked\`: fiscal period reopened -- \`period.year_closed\`: full year-end procedure complete - -**Payroll** *(requires \`payroll:read\` scope alongside \`webhooks:manage\`)* -- \`salary_run.created\` -- \`salary_run.approved\` -- \`salary_run.booked\`: journal entries posted -- \`agi.generated\`: AGI XML produced - -**Documents** -- \`document.uploaded\` +${renderEventTypes()} ## Payload shape diff --git a/lib/webhooks/__tests__/handler-subscriptions.test.ts b/lib/webhooks/__tests__/handler-subscriptions.test.ts new file mode 100644 index 00000000..d0d88c9f --- /dev/null +++ b/lib/webhooks/__tests__/handler-subscriptions.test.ts @@ -0,0 +1,29 @@ +/** + * registerWebhookHandler() must subscribe to exactly the catalogue in + * lib/webhooks/public-events.ts: an event an agent can subscribe to via the + * API but that the handler never fans out would be silently undeliverable. + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('@/lib/auth/api-keys', () => ({ + createServiceClientNoCookies: vi.fn(() => ({ __client: true })), +})) +vi.mock('next/server', () => ({ after: vi.fn() })) + +import { eventBus } from '@/lib/events/bus' +import { registerWebhookHandler } from '../handler' +import { PUBLIC_WEBHOOK_EVENTS } from '../public-events' + +describe('registerWebhookHandler', () => { + beforeEach(() => { + eventBus.clear() + vi.clearAllMocks() + }) + + it('subscribes to every catalogued event type and nothing else', () => { + const on = vi.spyOn(eventBus, 'on') + registerWebhookHandler() + const subscribed = on.mock.calls.map(([eventType]) => eventType).sort() + expect(subscribed).toEqual([...PUBLIC_WEBHOOK_EVENTS].sort()) + }) +}) diff --git a/lib/webhooks/__tests__/public-events.test.ts b/lib/webhooks/__tests__/public-events.test.ts new file mode 100644 index 00000000..46b2e8e4 --- /dev/null +++ b/lib/webhooks/__tests__/public-events.test.ts @@ -0,0 +1,47 @@ +/** + * The webhook event catalogue (lib/webhooks/public-events.ts) is the single + * source for three surfaces that used to be hand-copied and drifted (24 vs + * 28 events): the v1 create schema (and so the OpenAPI spec and the generated + * agent skill), the docs page, and the fan-out handler. These tests pin each + * surface back to the catalogue so a re-hardcoded list fails CI. + */ +import { describe, expect, it } from 'vitest' +import { PUBLIC_WEBHOOK_EVENTS, PUBLIC_WEBHOOK_EVENT_GROUPS } from '../public-events' +import { WEBHOOKS_MD } from '@/lib/docs/content/webhooks' +import { generateOpenApiSpec } from '@/lib/api/v1/registry' +// Side-effect import: populates the endpoint registry from every route file. +import '@/lib/api/v1/load-routes' + +describe('public webhook event catalogue', () => { + it('has no duplicate event types and no empty groups', () => { + expect(new Set(PUBLIC_WEBHOOK_EVENTS).size).toBe(PUBLIC_WEBHOOK_EVENTS.length) + for (const group of PUBLIC_WEBHOOK_EVENT_GROUPS) expect(group.events.length).toBeGreaterThan(0) + }) + + it('includes the reconciliation events the handler delivers', () => { + expect(PUBLIC_WEBHOOK_EVENTS).toEqual( + expect.arrayContaining([ + 'reconciliation.matched', + 'reconciliation.unmatched', + 'reconciliation.signed_off', + 'reconciliation.reopened', + ]), + ) + }) + + it('is exactly the event_type enum the v1 create endpoint advertises in the OpenAPI spec', () => { + const spec = generateOpenApiSpec('https://unit.test') + const op = (spec.paths['/api/v1/companies/{companyId}/webhooks'] as Record).post as { + requestBody: { content: Record } }> } + } + const enumValues = op.requestBody.content['application/json'].schema.properties.event_type.enum + expect(enumValues).toEqual([...PUBLIC_WEBHOOK_EVENTS]) + }) + + it('is exactly the list the docs page renders', () => { + const section = WEBHOOKS_MD.split('## Event types')[1]?.split('## Payload shape')[0] ?? '' + const documented = [...section.matchAll(/^- `([a-z_]+\.[a-z_]+)`/gm)].map((m) => m[1]) + expect(documented).toEqual([...PUBLIC_WEBHOOK_EVENTS]) + for (const group of PUBLIC_WEBHOOK_EVENT_GROUPS) expect(section).toContain(`**${group.title}**`) + }) +}) diff --git a/lib/webhooks/handler.ts b/lib/webhooks/handler.ts index f602ec88..45063eea 100644 --- a/lib/webhooks/handler.ts +++ b/lib/webhooks/handler.ts @@ -28,48 +28,16 @@ import { createServiceClientNoCookies } from '@/lib/auth/api-keys' import { createLogger } from '@/lib/logger' import { API_V1_VERSION } from '@/lib/api/v1/version' import { kickWebhookDispatch } from './dispatch-kick' +import { PUBLIC_WEBHOOK_EVENTS as PUBLIC_WEBHOOK_EVENT_CATALOGUE } from './public-events' const log = createLogger('webhooks/handler') /** - * Set of event types that the v1 webhook surface delivers. Restricted to the - * resource-state-change events that are useful to external integrations; - * MCP telemetry events and internal-only flows (event_log writes, etc.) are - * deliberately excluded. - * - * Adding a new event type to this set is a public-API change: bump - * API_V1_VERSION + add to the changelog when you do. + * Set of event types that the v1 webhook surface delivers. Derived from the + * public catalogue in ./public-events.ts, which is also what the v1 create + * schema and the docs page enumerate: add new event types THERE, not here. */ -const PUBLIC_WEBHOOK_EVENTS = new Set([ - 'invoice.created', - 'invoice.sent', - 'invoice.paid', - 'credit_note.created', - 'customer.created', - 'supplier.created', - 'supplier_invoice.registered', - 'supplier_invoice.approved', - 'supplier_invoice.paid', - 'supplier_invoice.credited', - 'supplier_invoice.uncredited', - 'transaction.categorized', - 'transaction.reconciled', - 'reconciliation.matched', - 'reconciliation.unmatched', - 'reconciliation.signed_off', - 'reconciliation.reopened', - 'journal_entry.committed', - 'journal_entry.reversed', - 'journal_entry.corrected', - 'period.locked', - 'period.unlocked', - 'period.year_closed', - 'salary_run.created', - 'salary_run.approved', - 'salary_run.booked', - 'agi.generated', - 'document.uploaded', -]) +const PUBLIC_WEBHOOK_EVENTS = new Set(PUBLIC_WEBHOOK_EVENT_CATALOGUE) let registered = false diff --git a/lib/webhooks/public-events.ts b/lib/webhooks/public-events.ts new file mode 100644 index 00000000..d1847723 --- /dev/null +++ b/lib/webhooks/public-events.ts @@ -0,0 +1,129 @@ +/** + * The public webhook event catalogue: the single source of truth for which + * CoreEventType values the v1 webhook surface delivers. + * + * Everything that enumerates webhook events derives from this module, so the + * lists cannot drift apart again (they did: the handler delivered 28 events + * while the create schema and the docs page listed 24, so nobody could + * subscribe to the four reconciliation.* events): + * + * 1. lib/webhooks/handler.ts subscribes the fan-out handler to every event. + * 2. app/api/v1/companies/[companyId]/webhooks/route.ts builds the create + * schema's `event_type` enum from it, which is what the OpenAPI spec and + * the generated agent skill (skills/accounted-api) advertise. + * 3. lib/docs/content/webhooks.ts renders its "Event types" section from + * the groups below. + * + * Restricted to the resource-state-change events that are useful to external + * integrations; MCP telemetry events and internal-only flows (event_log + * writes, drafts, deletes) are deliberately excluded. + * + * Adding an event type here is a public-API change: add a changelog entry in + * lib/docs/content/changelog.ts. Additive changes do not bump API_V1_VERSION. + * + * This module must stay a leaf (type-only imports): it is imported by docs + * pages, route modules and the event-bus handler alike. + */ + +import type { CoreEventType } from '@/lib/events/types' + +export interface PublicWebhookEventEntry { + type: CoreEventType + /** Short docs prose, rendered after the event name on the docs page. */ + description?: string +} + +export interface PublicWebhookEventGroup { + /** Docs section heading, e.g. "Invoicing". */ + title: string + /** Optional note rendered next to the heading (e.g. an extra scope requirement). */ + note?: string + events: ReadonlyArray +} + +export const PUBLIC_WEBHOOK_EVENT_GROUPS = [ + { + title: 'Invoicing', + events: [ + { type: 'invoice.created', description: 'draft invoice created' }, + { type: 'invoice.sent', description: 'invoice marked sent (email delivered or external)' }, + { type: 'invoice.paid', description: 'invoice fully paid' }, + { type: 'credit_note.created', description: 'credit note issued' }, + ], + }, + { + title: 'AP / suppliers', + events: [ + { type: 'supplier.created' }, + { type: 'supplier_invoice.registered' }, + { type: 'supplier_invoice.approved' }, + { type: 'supplier_invoice.paid' }, + { type: 'supplier_invoice.credited' }, + { type: 'supplier_invoice.uncredited', description: 'credit reversal' }, + ], + }, + { + title: 'Customers', + events: [{ type: 'customer.created' }], + }, + { + title: 'Bookkeeping', + events: [ + { type: 'journal_entry.committed', description: 'voucher posted (immutable from this point)' }, + { type: 'journal_entry.reversed', description: 'storno entry posted' }, + { type: 'journal_entry.corrected', description: 'rättelse via `correctEntry` (BFL 5 kap 5 §)' }, + ], + }, + { + title: 'Transactions', + events: [ + { type: 'transaction.categorized', description: 'bank transaction assigned an account + tax code' }, + { type: 'transaction.reconciled', description: 'transaction matched to a posted entry' }, + ], + }, + { + title: 'Reconciliation', + events: [ + { + type: 'reconciliation.matched', + description: 'an outside item (bank row or skattekonto row) linked to a journal entry', + }, + { type: 'reconciliation.unmatched', description: 'a reconciliation link removed' }, + { type: 'reconciliation.signed_off', description: 'an account signed off as reconciled through a date' }, + { type: 'reconciliation.reopened', description: 'a sign-off reopened' }, + ], + }, + { + title: 'Periods', + events: [ + { type: 'period.locked', description: 'fiscal period closed for writes' }, + { type: 'period.unlocked', description: 'fiscal period reopened' }, + { type: 'period.year_closed', description: 'full year-end procedure complete' }, + ], + }, + { + title: 'Payroll', + note: 'requires `payroll:read` scope alongside `webhooks:manage`', + events: [ + { type: 'salary_run.created' }, + { type: 'salary_run.approved' }, + { type: 'salary_run.booked', description: 'journal entries posted' }, + { type: 'agi.generated', description: 'AGI XML produced' }, + ], + }, + { + title: 'Documents', + events: [{ type: 'document.uploaded' }], + }, +] as const satisfies ReadonlyArray + +/** Union of every deliverable event type, narrowed from the catalogue. */ +export type PublicWebhookEventType = (typeof PUBLIC_WEBHOOK_EVENT_GROUPS)[number]['events'][number]['type'] + +/** + * Flat, ordered list of every deliverable event type. Pass straight to + * `z.enum(...)` or `new Set(...)`. + */ +export const PUBLIC_WEBHOOK_EVENTS: ReadonlyArray = PUBLIC_WEBHOOK_EVENT_GROUPS.flatMap( + (group) => group.events.map((event) => event.type), +) diff --git a/skills/accounted-api/references/webhooks.md b/skills/accounted-api/references/webhooks.md index 1716b9c7..f41b1c3f 100644 --- a/skills/accounted-api/references/webhooks.md +++ b/skills/accounted-api/references/webhooks.md @@ -53,7 +53,7 @@ Creates a webhook subscription for one event type. The response includes a fresh **Do not use for:** Subscribing to internal MCP telemetry events (mcp.tool_called etc. are not delivered as webhooks). Replacing an existing webhook URL: use PATCH instead. **Pitfalls:** -- The secret is returned exactly once. If lost, delete and recreate the webhook. +- The secret is returned exactly once. If lost, rotate it with POST /webhooks/{id}/rotate-secret: a fresh secret is issued in place, the webhook id and delivery history are kept. - Delivery is at-least-once with exponential backoff (1m / 5m / 30m / 2h / 12h / 24h / 48h). Receivers MUST be idempotent. - HTTP 410 from your receiver auto-disables the webhook (sets active=false + disabled_reason). @@ -64,7 +64,7 @@ Creates a webhook subscription for one event type. The response includes a fresh Request body: ```ts { - event_type: "invoice.created" | "invoice.sent" | "invoice.paid" | "credit_note.created" | "customer.created" | "supplier.created" | "supplier_invoice.registered" | "supplier_invoice.approved" | "supplier_invoice.paid" | "supplier_invoice.credited" | "supplier_invoice.uncredited" | "transaction.categorized" | "transaction.reconciled" | "journal_entry.committed" | "journal_entry.reversed" | "journal_entry.corrected" | "period.locked" | "period.unlocked" | "period.year_closed" | "salary_run.created" | "salary_run.approved" | "salary_run.booked" | "agi.generated" | "document.uploaded", + event_type: "invoice.created" | "invoice.sent" | "invoice.paid" | "credit_note.created" | "supplier.created" | "supplier_invoice.registered" | "supplier_invoice.approved" | "supplier_invoice.paid" | "supplier_invoice.credited" | "supplier_invoice.uncredited" | "customer.created" | "journal_entry.committed" | "journal_entry.reversed" | "journal_entry.corrected" | "transaction.categorized" | "transaction.reconciled" | "reconciliation.matched" | "reconciliation.unmatched" | "reconciliation.signed_off" | "reconciliation.reopened" | "period.locked" | "period.unlocked" | "period.year_closed" | "salary_run.created" | "salary_run.approved" | "salary_run.booked" | "agi.generated" | "document.uploaded", webhook_url: string, name: string, description?: string @@ -150,7 +150,7 @@ Response `200`: Update the URL, name, description, or active flag. event_type is immutable: delete and recreate to change it. Setting active=false manually pauses delivery without deleting; setting active=true clears any disabled_at/disabled_reason set by the auto-disable on HTTP 410. **Use when:** You need to point an existing webhook at a new URL or temporarily pause delivery. -**Do not use for:** Rotating the signing secret (delete and recreate). Changing event_type. +**Do not use for:** Rotating the signing secret: use POST /webhooks/{id}/rotate-secret, which issues a fresh secret in place and keeps the webhook id and delivery history. Changing event_type: delete and recreate. **Pitfalls:** - Re-enabling a webhook (active: true) does NOT replay deliveries that went to dead status while it was disabled: those need POST /webhook-deliveries/{id}/retry.