fix(v1): attribute every REST write to the API key that made it (#2074)
commitEntry() already reads getActor() and forwards it to commit_journal_entry, which stamps journal_entries.committed_actor_* and the audit_log COMMIT row. But runWithActor had exactly one production call site, the pending-operations commit, so everything reaching the ledger by any other route committed anonymously. Setting the scope once in withApiV1 attributes every v1 write, including the ones that get to the ledger several frames down through reverseEntry, correctEntry and the supplier-invoice paths. No signature changes, no migration. The severity is not the headline aggregate. 184 873 of the 193 466 unattributed entries are source_type='import', bulk SIE migration with no meaningful actor. Excluding those it is 8 593 of 11 009, and it concentrates where it matters most: 99.8% of storno entries and 100% of correction entries had no actor. Those are the two sanctioned rättelse paths under BFL 5 kap. 5 §, where "who did this, and when" is a legal question. Only the authenticated branch is wrapped. The anonymous public path has no attribution worth recording. Verified by removing the wrapper and watching 3 of 4 tests fail; the fourth, which checks AsyncLocalStorage does not leak between requests, correctly still passed. Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
co-authored by
Jakob Wennberg
Claude Opus 5
Jakob Wennberg
Claude Opus 5
parent
36123cef23
commit
413a0978c0
@@ -1391,6 +1391,7 @@ One line per decision: `[YYYY-MM-DD] <decision>: <why>`. Appended by agents and
|
||||
[2026-08-31] Login/register methods come from GoTrue (/auth/v1/settings + admin customProviders) instead of app-side flags; NEXT_PUBLIC_GOOGLE_AUTH_ENABLED removed (PR #1869): the Supabase dashboard becomes the single switch, an allowlist of auth-js provider ids filters non-login entries like anonymous_users, and hosted rendering is unchanged because Google is enabled in prod GoTrue. The Vercel env var stays set for old-build rollback safety; delete it after a few deploys.
|
||||
[2026-08-31] Single prominent amount is PROMOTED into editable totals.total (totalSource='prominent') instead of living in a read-only Belopp row: Emil's call, an uncorrectable load-bearing value violated the prefill-override-editors rule. Provenance keeps matching fallback-grade (discount, date guard, hunt exclusion); a user edit of TOTALT clears the stamp. Multi-amount docs keep the Belopp row: promoting one of several figures would invent a total.
|
||||
[2026-08-31] Image-scan red fixed by bumping the node:22-alpine digest (alpine 3.23 to 3.24.1), not by widening the gate: the Dockerfile's apk-upgrade layer is frozen by the GHCR buildx layer cache, so a fix published after the last cache-busting change (libssl3 3.5.8-r0 for CVE-2026-14456) never reaches the published image until the FROM digest moves; the red scheduled scan is the designed alarm for exactly this bump. cron.Dockerfile gained the same apk upgrade (it had none).
|
||||
[2026-08-31] Every v1 REST handler now runs inside a runWithActor({type:'api_key', label:<key name>}) scope, set once in withApiV1 rather than threaded through each route. commitEntry() already reads getActor() as its fallback and forwards it to commit_journal_entry, which stamps journal_entries.committed_actor_* and the audit_log COMMIT row (20260619120000), so one wrapper attributes EVERY v1 write, including the ones that reach the ledger several frames down via reverseEntry/correctEntry/the supplier-invoice paths. No signature changes, no migration. Before this, runWithActor had exactly ONE production call site (lib/pending-operations/commit.ts:6358), so everything committing outside the staged path was anonymous. Measured on prod over 90 days: 99.8% of storno entries and 100% of correction entries had no actor, which are precisely the two sanctioned rättelse paths under BFL 5 kap. 5 § and the place where "who did this, and when" is a legal question rather than a nicety. NOTE the headline "98.8% of all entries" from the original issue was true but misleading: 184 873 of the 193 466 unattributed entries are source_type='import' (bulk SIE migration, no meaningful actor); excluding import it is 8 593 of 11 009 non-import entries, and the severity lives in storno/correction, not in the aggregate. Wrapped at the authenticated invocation only, not the anonymous/public branch, where no attribution is meaningful. actor-context-node's own docstring names API routes as a sanctioned import site (it is server-only; the isomorphic half exists so engine.ts stays client-bundle safe). Verified the test discriminates by removing the wrapper and watching 3 of 4 fail, while the AsyncLocalStorage leak check correctly still passed.
|
||||
[2026-08-31] Agent approval-authority ceiling stored on api_keys, not company_settings: a company-scoped money threshold catches humans too, and company_settings.agent_auto_commit_max_amount was already tried (20260501120000) and dropped four days later (20260505190027) for exactly that reason.
|
||||
[2026-08-31] Ceiling enforced in TypeScript at the two commit doors, not inside commit_journal_entry: a RAISE there is swallowed by engine.ts into BookkeepingDatabaseError (500, retryable), and on the MCP path that burns the pending op to terminal 'rejected', destroying the staged verifikat. Both doors refuse BEFORE the atomic claim / before commitEntry, so the work survives as 'pending' or as a draft and a human can approve the same verifikat.
|
||||
[2026-08-31] The agent ceiling prices every money-posting operation type from preview_data, including the batch and settlement paths (match_batch_allocate/total_allocated, bulk_book_transactions/tx_sum, link_transaction_journal_entry/transaction_amount, link_supplier_invoice_voucher/payment_amount, mark_invoice_paid/total): all verified present and numeric on 100% of prod rows over 120 days. An earlier cut left these unpriced on the assumption their totals only existed in SQL at dispatch; that was wrong and let a limited key post any amount through the four largest settlement paths. Only reconciliation_match stays unpriced, because pair_count is a count and not kronor.
|
||||
|
||||
@@ -0,0 +1,154 @@
|
||||
/**
|
||||
* Every v1 write runs inside the commit-actor scope.
|
||||
*
|
||||
* `commitEntry()` reads `getActor()` as its fallback and forwards it to the
|
||||
* commit_journal_entry RPC, which stamps `journal_entries.committed_actor_*`
|
||||
* and the audit_log COMMIT row. Before this, `runWithActor` had exactly one
|
||||
* production call site (the pending-operations commit), so anything reaching
|
||||
* the ledger by another route committed anonymously.
|
||||
*
|
||||
* Measured on production over 90 days: 99.8% of `storno` entries and 100% of
|
||||
* `correction` entries had no actor. Those are the two sanctioned rättelse
|
||||
* paths under BFL 5 kap. 5 §, which makes them the entries where "who did
|
||||
* this" is a legal question and the ones with the least attribution.
|
||||
*
|
||||
* The test asserts the scope is visible from INSIDE the handler, which is what
|
||||
* a helper several frames down (reverseEntry, correctEntry) actually sees.
|
||||
*/
|
||||
import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { NextResponse } from 'next/server'
|
||||
|
||||
beforeAll(() => {
|
||||
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<typeof import('@/lib/auth/api-keys')>('@/lib/auth/api-keys')
|
||||
return { ...actual, validateApiKey: vi.fn(), createServiceClientNoCookies: vi.fn() }
|
||||
})
|
||||
|
||||
vi.mock('@supabase/supabase-js', async () => {
|
||||
const actual = await vi.importActual<typeof import('@supabase/supabase-js')>('@supabase/supabase-js')
|
||||
return { ...actual, createClient: vi.fn().mockReturnValue({}) }
|
||||
})
|
||||
|
||||
import { validateApiKey, createServiceClientNoCookies } from '@/lib/auth/api-keys'
|
||||
import { getActor } from '@/lib/bookkeeping/actor-context'
|
||||
import { withApiV1 } from '../with-api-v1'
|
||||
import { ok } from '../response'
|
||||
|
||||
const mockValidate = vi.mocked(validateApiKey)
|
||||
const mockServiceClient = vi.mocked(createServiceClientNoCookies)
|
||||
|
||||
const COMPANY = '11111111-1111-4111-8111-111111111111'
|
||||
|
||||
/**
|
||||
* `companies.list` is the authenticated static route the sibling suite uses.
|
||||
* The wrapper resolves the operation against the registry before it reaches
|
||||
* the handler, so an invented path 404s and the handler never runs.
|
||||
*/
|
||||
const OPERATION = 'companies.list'
|
||||
const URL_FOR = 'https://x.test/api/v1/companies'
|
||||
|
||||
/** Minimal flexible supabase double: every chain resolves to a membership row. */
|
||||
function makeSupabase() {
|
||||
const chain: unknown = new Proxy(
|
||||
{},
|
||||
{
|
||||
get(_t, prop) {
|
||||
if (prop === 'then') {
|
||||
return (resolve: (v: unknown) => void) =>
|
||||
resolve({ data: { company_id: COMPANY, role: 'owner' }, error: null })
|
||||
}
|
||||
return () => chain
|
||||
},
|
||||
},
|
||||
)
|
||||
return { from: () => chain, rpc: () => chain }
|
||||
}
|
||||
|
||||
function makeRequest(url: string, init?: RequestInit) {
|
||||
return new Request(url, {
|
||||
headers: { Authorization: 'Bearer gnubok_sk_live_x', ...(init?.headers ?? {}) },
|
||||
...init,
|
||||
}) as never
|
||||
}
|
||||
|
||||
/** Next.js 16 hands a static route `{ params: undefined }`. */
|
||||
function staticParams() {
|
||||
return { params: undefined } as never
|
||||
}
|
||||
|
||||
describe('v1 handlers run inside the commit-actor scope', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
mockServiceClient.mockReturnValue(makeSupabase() as never)
|
||||
mockValidate.mockResolvedValue({
|
||||
userId: 'user-1',
|
||||
companyId: COMPANY,
|
||||
scopes: ['companies:read'],
|
||||
apiKeyId: 'key-1',
|
||||
apiKeyName: 'DueCue automation stager',
|
||||
mode: 'live',
|
||||
} as Awaited<ReturnType<typeof validateApiKey>>)
|
||||
})
|
||||
|
||||
it('exposes an api_key actor to code running inside the handler', async () => {
|
||||
let seen: ReturnType<typeof getActor>
|
||||
const handler = withApiV1(OPERATION, async (_req, ctx) => {
|
||||
// This is what commitEntry() sees, several frames down.
|
||||
seen = getActor()
|
||||
return ok({ ok: true }, { requestId: ctx.requestId })
|
||||
})
|
||||
|
||||
await handler(makeRequest(URL_FOR), staticParams())
|
||||
|
||||
expect(seen).toBeDefined()
|
||||
expect(seen?.type).toBe('api_key')
|
||||
})
|
||||
|
||||
it('carries the key name through to the actor label', async () => {
|
||||
let seen: ReturnType<typeof getActor>
|
||||
const handler = withApiV1(OPERATION, async (_req, ctx) => {
|
||||
seen = getActor()
|
||||
return ok({ ok: true }, { requestId: ctx.requestId })
|
||||
})
|
||||
|
||||
await handler(makeRequest(URL_FOR), staticParams())
|
||||
|
||||
expect(seen?.label).toBe('DueCue automation stager')
|
||||
})
|
||||
|
||||
it('falls back to a stable label when the key is unnamed', async () => {
|
||||
mockValidate.mockResolvedValue({
|
||||
userId: 'user-1',
|
||||
companyId: COMPANY,
|
||||
scopes: ['companies:read'],
|
||||
apiKeyId: 'key-1',
|
||||
apiKeyName: undefined,
|
||||
mode: 'live',
|
||||
} as Awaited<ReturnType<typeof validateApiKey>>)
|
||||
|
||||
let seen: ReturnType<typeof getActor>
|
||||
const handler = withApiV1(OPERATION, async (_req, ctx) => {
|
||||
seen = getActor()
|
||||
return ok({ ok: true }, { requestId: ctx.requestId })
|
||||
})
|
||||
|
||||
await handler(makeRequest(URL_FOR), staticParams())
|
||||
|
||||
expect(seen?.label).toBe('Unnamed API key')
|
||||
})
|
||||
|
||||
it('leaves no actor bound after the request completes', async () => {
|
||||
// AsyncLocalStorage must not leak across requests: a later unattributed
|
||||
// path must stay unattributed rather than inherit the previous caller.
|
||||
const handler = withApiV1(OPERATION, async (_req, ctx) =>
|
||||
ok({ ok: true }, { requestId: ctx.requestId }),
|
||||
)
|
||||
await handler(makeRequest(URL_FOR), staticParams())
|
||||
|
||||
expect(getActor()).toBeUndefined()
|
||||
})
|
||||
})
|
||||
@@ -48,6 +48,7 @@ import {
|
||||
RATE_LIMIT_RETRY_AFTER_SECONDS,
|
||||
validateApiKey,
|
||||
} from '@/lib/auth/api-keys'
|
||||
import { runWithActor } from '@/lib/bookkeeping/actor-context-node'
|
||||
|
||||
// Per CLAUDE.md: any route that emits events via eventBus must call
|
||||
// ensureInitialized() at module level to wire extension event handlers
|
||||
@@ -499,8 +500,28 @@ export function withApiV1<P extends DynamicParams = { params: Promise<Record<str
|
||||
idempotencyKey,
|
||||
}
|
||||
|
||||
// 9. Invoke handler.
|
||||
const response = await handler(workingRequest, ctx, params)
|
||||
// 9. Invoke handler, inside the commit-actor scope.
|
||||
//
|
||||
// commitEntry() reads getActor() as its fallback and forwards it to the
|
||||
// commit_journal_entry RPC, which stamps journal_entries.committed_actor_*
|
||||
// and the audit_log COMMIT row (migration 20260619120000). Wrapping here
|
||||
// rather than threading a parameter means EVERY v1 write is attributed,
|
||||
// including the ones that reach the ledger through a helper several
|
||||
// frames down (reverseEntry, correctEntry, the supplier-invoice paths).
|
||||
//
|
||||
// Before this, runWithActor had exactly ONE production call site, the
|
||||
// pending-operations commit. Everything committing outside that path was
|
||||
// anonymous: on production, 99.8% of storno entries and 100% of
|
||||
// correction entries carried no actor at all, which are precisely the two
|
||||
// sanctioned rättelse paths under BFL 5 kap. 5 § and the place where
|
||||
// "who did this, and when" is a legal question rather than a nicety.
|
||||
//
|
||||
// `api_key` is the honest label for this surface: a gnubok_sk_ bearer
|
||||
// token. The OAuth/MCP surfaces set their own actor and are unaffected.
|
||||
const response = await runWithActor(
|
||||
{ type: 'api_key', label: auth.apiKeyName ?? 'Unnamed API key' },
|
||||
() => handler(workingRequest, ctx, params),
|
||||
)
|
||||
|
||||
// Signal test mode on every test-key response so integrators can see the
|
||||
// request was simulation-only without inspecting the body.
|
||||
|
||||
Reference in New Issue
Block a user