feat(auth): SoD acknowledge on stage+approve keys + agent:write scope for memory tools (P0-3) (#681)
* feat(auth): SoD acknowledge on stage+approve keys + agent:write scope for memory tools Segregation of duties on API keys is now warn + explicit acknowledgement (not block): minting a key with any staging write scope AND pending_operations:approve returns 409 API_KEY_SOD_CONFLICT unless the caller re-POSTs with acknowledge_sod: true. The acknowledgement is recorded (sod_acknowledged_at / sod_acknowledged_by) for an auditable risk acceptance (ISO 27001:2022 A.5.3 / BFNAR 2013:2). The create UI surfaces an inline warning and an explicit confirm dialog before submitting the ack — the default "all scopes ticked" create routes through that path. Also introduces the agent:write scope and maps the previously-UNMAPPED memory tools gnubok_remember_fact / gnubok_forget_fact to it. Because unmapped tools were callable by any key, the migration grandfathers agent:write onto every existing non-revoked key with an explicit scope list so nothing regresses; new keys must opt in. agent:write is deliberately excluded from the default grants and is NOT a staging scope (no SoD conflict with approve). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(db): enforce both-or-neither on the SoD acknowledgement pair Review finding (Greptile P2): sod_acknowledged_at/sod_acknowledged_by were independently nullable, so a partial write could silently pass and undermine the auditable risk acceptance (ISO 27001 A.5.3 / SOC 2 CC6.1). Adds a paired-NULL CHECK constraint + pg-real coverage for both partial-write directions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(auth)+feat(auth): compliance-review round — self-attestation documented, ack logged, SoD boundary assumption captured - Migration header now states explicitly that the SoD acknowledgement is a SELF-attestation by deliberate design (enskild firma has no second person; the claude.ai approval flow needs stage+approve on one credential) — the control objective is informed consent + audit record, not dual control. - The acknowledge_sod=true path now emits a structured log.warn (api_key.sod_acknowledged with key id/prefix, conflicting scope, scopes, acknowledger, company) so the acceptance lands in the logging pipeline in addition to the sod_acknowledged_* columns (ASVS V16.1.1). - STAGING_SCOPES carries the documented system control (BFNAR 2013:2 systemdokumentation) for why agent:write is not a staging scope: memory tools write advisory agent context and cannot stage räkenskapsinformation. Dismissed as by-design/verified: hard-block and second-approver remediations (user decision: warn + acknowledge); scope-update gap (the [id] route only supports DELETE — scopes are immutable post-creation); session-auth concern (withRouteContext is cookie+MFA only; API-key auth exists only on /api/v1 and MCP). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: re-trigger CI (Supabase Preview 502 infra hiccup) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
co-authored by
Claude Opus 4.8
Claude Opus 4.8
parent
305f469fc3
commit
0bc81d4c88
@@ -0,0 +1,193 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest'
|
||||
import { createMockRequest, parseJsonResponse } from '@/tests/helpers'
|
||||
|
||||
// ── Mocks ──────────────────────────────────────────────────────────
|
||||
// withRouteContext resolves auth via requireAuth (createClient under the hood),
|
||||
// the active company via getActiveCompanyId, and the write gate via
|
||||
// requireWritePermission. Mock all three so we can drive each branch.
|
||||
|
||||
const mockSupabase = {
|
||||
auth: { getUser: vi.fn() },
|
||||
from: vi.fn(),
|
||||
}
|
||||
|
||||
vi.mock('@/lib/supabase/server', () => ({
|
||||
createClient: () => Promise.resolve(mockSupabase),
|
||||
}))
|
||||
|
||||
const getActiveCompanyIdMock = vi.fn()
|
||||
vi.mock('@/lib/company/context', () => ({
|
||||
getActiveCompanyId: (...args: unknown[]) => getActiveCompanyIdMock(...args),
|
||||
}))
|
||||
|
||||
const requireWritePermissionMock = vi.fn()
|
||||
vi.mock('@/lib/auth/require-write', () => ({
|
||||
requireWritePermission: (...args: unknown[]) => requireWritePermissionMock(...args),
|
||||
}))
|
||||
|
||||
import { POST } from '../route'
|
||||
|
||||
const mockUser = { id: 'user-1', email: 'test@test.se' }
|
||||
|
||||
// Records the payload passed to .insert(), and lets us program the count
|
||||
// returned by the quota pre-check and the row returned by the insert.
|
||||
function setupFrom(opts: {
|
||||
count?: number | null
|
||||
insertResult?: { data?: unknown; error?: unknown }
|
||||
}) {
|
||||
const insertSpy = vi.fn()
|
||||
|
||||
mockSupabase.from.mockImplementation(() => {
|
||||
// The quota pre-check: .select(..., { head: true }).eq().is() → resolves
|
||||
// to { count }. The insert: .insert().select().single() → resolves to the
|
||||
// row. We expose both via a single chainable proxy whose terminal value
|
||||
// depends on whether insert() was called.
|
||||
let isInsert = false
|
||||
const result = () =>
|
||||
isInsert
|
||||
? Promise.resolve({
|
||||
data: opts.insertResult?.data ?? null,
|
||||
error: opts.insertResult?.error ?? null,
|
||||
})
|
||||
: Promise.resolve({ count: opts.count ?? 0, data: null, error: null })
|
||||
|
||||
const chain: Record<string, unknown> = {}
|
||||
const handler: ProxyHandler<object> = {
|
||||
get(_t, prop) {
|
||||
if (prop === 'then') {
|
||||
return (resolve: (v: unknown) => void) => resolve(result() as unknown)
|
||||
}
|
||||
if (prop === 'insert') {
|
||||
return (payload: unknown) => {
|
||||
isInsert = true
|
||||
insertSpy(payload)
|
||||
return new Proxy(chain, handler)
|
||||
}
|
||||
}
|
||||
if (prop === 'single' || prop === 'maybeSingle') {
|
||||
return () => result()
|
||||
}
|
||||
return () => new Proxy(chain, handler)
|
||||
},
|
||||
}
|
||||
return new Proxy(chain, handler)
|
||||
})
|
||||
|
||||
return { insertSpy }
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
mockSupabase.auth.getUser.mockResolvedValue({ data: { user: mockUser } })
|
||||
getActiveCompanyIdMock.mockResolvedValue('company-1')
|
||||
requireWritePermissionMock.mockResolvedValue({ ok: true })
|
||||
})
|
||||
|
||||
describe('POST /api/settings/api-keys', () => {
|
||||
it('returns 401 when not authenticated', async () => {
|
||||
mockSupabase.auth.getUser.mockResolvedValue({ data: { user: null } })
|
||||
const res = await POST(
|
||||
createMockRequest('/api/settings/api-keys', {
|
||||
method: 'POST',
|
||||
body: { name: 'k', scopes: ['reports:read'] },
|
||||
}),
|
||||
)
|
||||
expect(res.status).toBe(401)
|
||||
})
|
||||
|
||||
it('returns 400 for an invalid scope', async () => {
|
||||
setupFrom({ count: 0 })
|
||||
const res = await POST(
|
||||
createMockRequest('/api/settings/api-keys', {
|
||||
method: 'POST',
|
||||
body: { name: 'k', scopes: ['totally:bogus'] },
|
||||
}),
|
||||
)
|
||||
const { status, body } = await parseJsonResponse<{ error: { code: string } }>(res)
|
||||
expect(status).toBe(400)
|
||||
expect(body.error.code).toBe('API_KEY_SCOPE_INVALID')
|
||||
})
|
||||
|
||||
it('returns 409 API_KEY_SOD_CONFLICT for stage+approve without acknowledgement', async () => {
|
||||
setupFrom({ count: 0 })
|
||||
const res = await POST(
|
||||
createMockRequest('/api/settings/api-keys', {
|
||||
method: 'POST',
|
||||
body: {
|
||||
name: 'k',
|
||||
scopes: ['invoices:write', 'pending_operations:approve'],
|
||||
},
|
||||
}),
|
||||
)
|
||||
const { status, body } = await parseJsonResponse<{
|
||||
error: { code: string; details: { conflicting_scope: string; approve_scope: string } }
|
||||
}>(res)
|
||||
expect(status).toBe(409)
|
||||
expect(body.error.code).toBe('API_KEY_SOD_CONFLICT')
|
||||
expect(body.error.details.conflicting_scope).toBe('invoices:write')
|
||||
expect(body.error.details.approve_scope).toBe('pending_operations:approve')
|
||||
})
|
||||
|
||||
it('records sod_acknowledged_at/by in the insert when acknowledge_sod is true', async () => {
|
||||
const { insertSpy } = setupFrom({
|
||||
count: 0,
|
||||
insertResult: {
|
||||
data: {
|
||||
id: 'ak-1',
|
||||
key_prefix: 'gnubok_sk_abcd',
|
||||
name: 'k',
|
||||
scopes: ['invoices:write', 'pending_operations:approve'],
|
||||
created_at: '2026-06-05T10:00:00Z',
|
||||
},
|
||||
},
|
||||
})
|
||||
const res = await POST(
|
||||
createMockRequest('/api/settings/api-keys', {
|
||||
method: 'POST',
|
||||
body: {
|
||||
name: 'k',
|
||||
scopes: ['invoices:write', 'pending_operations:approve'],
|
||||
acknowledge_sod: true,
|
||||
},
|
||||
}),
|
||||
)
|
||||
const { status, body } = await parseJsonResponse<{ data: { key: string } }>(res)
|
||||
expect(status).toBe(200)
|
||||
expect(body.data.key).toMatch(/^gnubok_sk_/)
|
||||
|
||||
expect(insertSpy).toHaveBeenCalledTimes(1)
|
||||
const payload = insertSpy.mock.calls[0][0] as Record<string, unknown>
|
||||
expect(payload.sod_acknowledged_by).toBe('user-1')
|
||||
expect(typeof payload.sod_acknowledged_at).toBe('string')
|
||||
// ISO timestamp
|
||||
expect(payload.sod_acknowledged_at).toMatch(/^\d{4}-\d{2}-\d{2}T/)
|
||||
})
|
||||
|
||||
it('creates a clean key without approve scope and does not set SoD fields', async () => {
|
||||
const { insertSpy } = setupFrom({
|
||||
count: 0,
|
||||
insertResult: {
|
||||
data: {
|
||||
id: 'ak-2',
|
||||
key_prefix: 'gnubok_sk_efgh',
|
||||
name: 'reader',
|
||||
scopes: ['reports:read'],
|
||||
created_at: '2026-06-05T10:00:00Z',
|
||||
},
|
||||
},
|
||||
})
|
||||
const res = await POST(
|
||||
createMockRequest('/api/settings/api-keys', {
|
||||
method: 'POST',
|
||||
body: { name: 'reader', scopes: ['reports:read'] },
|
||||
}),
|
||||
)
|
||||
const { status } = await parseJsonResponse(res)
|
||||
expect(status).toBe(200)
|
||||
|
||||
const payload = insertSpy.mock.calls[0][0] as Record<string, unknown>
|
||||
expect(payload).not.toHaveProperty('sod_acknowledged_at')
|
||||
expect(payload).not.toHaveProperty('sod_acknowledged_by')
|
||||
expect(payload.scopes).toEqual(['reports:read'])
|
||||
})
|
||||
})
|
||||
@@ -1,5 +1,10 @@
|
||||
import { NextResponse } from 'next/server'
|
||||
import { generateApiKey, DEFAULT_SCOPES, validateScopes } from '@/lib/auth/api-keys'
|
||||
import {
|
||||
generateApiKey,
|
||||
DEFAULT_SCOPES,
|
||||
validateScopes,
|
||||
findStageApproveConflict,
|
||||
} from '@/lib/auth/api-keys'
|
||||
import { withRouteContext } from '@/lib/api/with-route-context'
|
||||
import { errorResponse, errorResponseFromCode } from '@/lib/errors/get-structured-error'
|
||||
import type { ApiKeyScope } from '@/lib/auth/api-keys'
|
||||
@@ -38,11 +43,13 @@ export const POST = withRouteContext(
|
||||
|
||||
let name = 'Unnamed key'
|
||||
let scopes: ApiKeyScope[] = DEFAULT_SCOPES
|
||||
let acknowledgeSod = false
|
||||
try {
|
||||
const body = await request.json()
|
||||
if (body.name && typeof body.name === 'string') {
|
||||
name = body.name.slice(0, 100)
|
||||
}
|
||||
acknowledgeSod = body.acknowledge_sod === true
|
||||
const parsed = validateScopes(body.scopes)
|
||||
if (parsed) {
|
||||
scopes = parsed
|
||||
@@ -56,6 +63,22 @@ export const POST = withRouteContext(
|
||||
// Empty body — use defaults.
|
||||
}
|
||||
|
||||
// Segregation of duties: warn + require explicit acknowledgement (not block)
|
||||
// when a single key both stages bookkeeping AND can approve it. Surfacing a
|
||||
// 409 lets the UI raise an explicit confirm dialog and the agent inform the
|
||||
// user before re-POSTing with acknowledge_sod: true.
|
||||
const conflictingScope = findStageApproveConflict(scopes)
|
||||
if (conflictingScope && !acknowledgeSod) {
|
||||
return errorResponseFromCode('API_KEY_SOD_CONFLICT', log, {
|
||||
requestId,
|
||||
details: {
|
||||
conflicting_scope: conflictingScope,
|
||||
approve_scope: 'pending_operations:approve',
|
||||
},
|
||||
})
|
||||
}
|
||||
const sodAcknowledgedAt = conflictingScope ? new Date().toISOString() : null
|
||||
|
||||
const { count } = await supabase
|
||||
.from('api_keys')
|
||||
.select('id', { count: 'exact', head: true })
|
||||
@@ -80,6 +103,9 @@ export const POST = withRouteContext(
|
||||
key_prefix: prefix,
|
||||
name,
|
||||
scopes,
|
||||
...(sodAcknowledgedAt
|
||||
? { sod_acknowledged_at: sodAcknowledgedAt, sod_acknowledged_by: user.id }
|
||||
: {}),
|
||||
})
|
||||
.select('id, key_prefix, name, scopes, created_at')
|
||||
.single()
|
||||
@@ -92,6 +118,21 @@ export const POST = withRouteContext(
|
||||
})
|
||||
}
|
||||
|
||||
if (sodAcknowledgedAt) {
|
||||
// High-risk security event: the creator self-attested the stage+approve
|
||||
// combination. The durable record is the sod_acknowledged_* pair on the
|
||||
// key row; this structured entry additionally lands the acceptance in
|
||||
// the logging pipeline (ASVS V16.1.1 / SOC 2 CC6.1).
|
||||
log.warn('api_key.sod_acknowledged', {
|
||||
keyId: data.id,
|
||||
keyPrefix: data.key_prefix,
|
||||
conflictingScope,
|
||||
scopes,
|
||||
acknowledgedBy: user.id,
|
||||
companyId,
|
||||
})
|
||||
}
|
||||
|
||||
return NextResponse.json({
|
||||
data: {
|
||||
...data,
|
||||
|
||||
Reference in New Issue
Block a user