From d038f521b9b75374bf7d18457b9178cd8450df02 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 17 Aug 2026 21:47:15 -0700 Subject: [PATCH 1/7] refactor(table): move the single-row route onto the application boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hottest table write path authorized in its own handler and queried the database from the adapter — the two things a surface adapter must never do. It now declares itself with defineInternalJsonRoute against the readRow, updateRow and deleteRow use cases: 127 lines instead of 274, with no db import, no drizzle import and no checkAccess. Doing that surfaced why the violation existed. Write-provenance resolution needs the canonical schema to map a caller's column key to the storage column it certifies, and the adapter could only do that because it was already loading the table illegally. The envelope is now split along the real seam: the adapter reads the header and payload field, which is transport, and the use case resolves the selections against the canonical table, which is domain. That split has to preserve a distinction the defaulting logic would erase. An internal caller that sends no envelope stays deliberately untracked; defaulting it to an exact-empty stamp would certify "this write introduced no secrets" on a runtime write that may well have introduced some. Only an interactive caller certifies exact-empty, over the storage columns its write actually persists. Two further changes fell out of it: present() now receives the same { principal, input } pair its sibling hooks responseHeaders and finalizeResponse already got. This route serves a session and a workflow execution on one path and owes them different column keyings, so rendering per caller kind is presentation rather than domain. That was a gap in the builder, not a special case for this route. tableRowWireSchema describes what the single-row routes actually return. The contract claimed a full TableRow, carrying the executions sidecar and Date objects — true of the list and query routes, and never true here. The hand- rolled handler was never checked against its own contract, so the drift was invisible until the builder started validating it. Wire changes, both deliberate and both narrower than before: a cross-tenant table now conceals as 404 where the old blanket handler answered 403, while an in-workspace role denial still answers 403. Nothing in hooks/queries/tables.ts branches on either. Verified to fail: forcing one keying, dropping the actor, pre-resolving the envelope, certifying an untracked internal write, and skipping the bundle completeness check each turn the covering tests red. --- .../[tableId]/rows/[rowId]/route.test.ts | 355 ++++++++--------- .../api/table/[tableId]/rows/[rowId]/route.ts | 361 ++++++------------ .../app/api/table/row-secret-provenance.ts | 58 +++ apps/sim/lib/api/contracts/tables.ts | 37 +- .../api/server/routes/internal-json-route.ts | 36 +- apps/sim/lib/table/api/index.ts | 1 + apps/sim/lib/table/api/route-policies.ts | 22 ++ .../application/row-secret-provenance.test.ts | 173 +++++++++ .../application/row-secret-provenance.ts | 143 +++++++ apps/sim/lib/table/application/rows.test.ts | 3 + apps/sim/lib/table/application/rows.ts | 73 +++- apps/sim/lib/table/events.attribution.test.ts | 57 ++- 12 files changed, 829 insertions(+), 490 deletions(-) create mode 100644 apps/sim/lib/table/application/row-secret-provenance.test.ts create mode 100644 apps/sim/lib/table/application/row-secret-provenance.ts diff --git a/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.test.ts b/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.test.ts index e57151a35e7..5d5f5e18537 100644 --- a/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.test.ts +++ b/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.test.ts @@ -1,68 +1,48 @@ /** * @vitest-environment node * - * Characterization tests for the single-row surface. + * Characterization tests for the single-row surface, carried across its + * migration onto the shared internal route builder. * - * These pin the wire behavior this route emits TODAY — status codes, body - * shapes, date serialization, and which collaborators are invoked — so the - * route can be migrated onto the shared internal route builder without - * silently changing what clients observe. They intentionally assert the - * existing contract rather than an idealized one. + * The assertions are the ones written against the hand-rolled handler — status + * codes, body shapes, ISO-8601 timestamps, and the dual-caller wire keying, + * where a session speaks stable column ids and a workflow execution speaks + * column names. What moved is the seam they mock: the route no longer loads the + * table or calls the row primitives itself, so the use cases are stubbed and the + * real builder runs. + * + * Two wire changes are deliberate; see the `deliberate wire changes` block. */ -import { - createTableDefinition, - hybridAuthMockFns, - queueTableRows, - resetDbChainMock, - schemaMock, -} from '@sim/testing' import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' -const { - mockCheckAccess, - mockUpdateRow, - mockPerformDeleteTableRow, - mockSignalTableRowsChangedByActor, -} = vi.hoisted(() => ({ - mockCheckAccess: vi.fn(), - mockUpdateRow: vi.fn(), - mockPerformDeleteTableRow: vi.fn(), - mockSignalTableRowsChangedByActor: vi.fn(), +const { mocks } = vi.hoisted(() => ({ + mocks: { + readRow: vi.fn(), + updateRow: vi.fn(), + deleteRow: vi.fn(), + authenticate: vi.fn(), + }, })) -vi.mock('@/app/api/table/utils', async () => { - const { NextResponse } = await import('next/server') +vi.mock('@/lib/table/application/rows', async (importOriginal) => { + const actual = await importOriginal() return { - checkAccess: mockCheckAccess, - accessError: (result: { status: number }) => - NextResponse.json({ error: 'Access denied' }, { status: result.status }), - orchestrationErrorResponse: (error: unknown) => - (error as { __orchestrated?: boolean })?.__orchestrated - ? NextResponse.json({ error: 'Orchestration failed' }, { status: 409 }) - : null, - orchestrationOutcomeErrorResponse: (_outcome: unknown, message: string) => - NextResponse.json({ error: message }, { status: 400 }), - tableLockErrorResponse: (error: unknown) => - (error as { __locked?: boolean })?.__locked - ? NextResponse.json({ error: 'Table is locked' }, { status: 423 }) - : null, + ...actual, + readTableRow: { operation: { id: 'tables.rows.read' }, execute: mocks.readRow }, + updateTableRow: { operation: { id: 'tables.rows.update' }, execute: mocks.updateRow }, + deleteTableRow: { operation: { id: 'tables.rows.delete' }, execute: mocks.deleteRow }, } }) -vi.mock('@/lib/table', async () => { - const columnKeys = await import('@/lib/table/column-keys') - return { ...columnKeys, updateRow: mockUpdateRow } +vi.mock('@/lib/table/api', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, internalTableSessionOrExecutorAuth: { authenticate: mocks.authenticate } } }) -vi.mock('@/lib/table/orchestration', () => ({ - performDeleteTableRow: mockPerformDeleteTableRow, -})) - -vi.mock('@/lib/table/events', () => ({ - signalTableRowsChangedByActor: mockSignalTableRowsChangedByActor, -})) - +import { InternalUnauthenticatedError } from '@/lib/api/server/routes' +import { NoWorkspaceAccessError } from '@/lib/core/application' +import { OrchestrationError } from '@/lib/core/orchestration/types' import { DELETE, GET, PATCH } from '@/app/api/table/[tableId]/rows/[rowId]/route' const TABLE_ID = 'tbl_1' @@ -71,26 +51,45 @@ const WORKSPACE_ID = 'workspace-1' const CREATED_AT = new Date('2024-01-01T00:00:00.000Z') const UPDATED_AT = new Date('2024-02-02T00:00:00.000Z') -function buildStoredRow() { - return { - id: ROW_ID, - data: { col_aaa: 'Ada', col_bbb: 36 }, - position: 0, - createdAt: CREATED_AT, - updatedAt: UPDATED_AT, - } +const TABLE = { + id: TABLE_ID, + workspaceId: WORKSPACE_ID, + schema: { + columns: [ + { id: 'col_aaa', name: 'Name', type: 'string' as const }, + { id: 'col_bbb', name: 'Age', type: 'number' as const }, + ], + }, +} + +const ROW = { + id: ROW_ID, + data: { col_aaa: 'Ada', col_bbb: 36 }, + executions: {}, + position: 0, + createdAt: CREATED_AT, + updatedAt: UPDATED_AT, +} + +function sessionPrincipal() { + mocks.authenticate.mockResolvedValue({ + kind: 'session', + userId: 'user-1', + sessionId: 'session-1', + }) } -function authAs(authType: 'session' | 'internal_jwt' = 'session') { - hybridAuthMockFns.mockCheckSessionOrInternalAuth.mockResolvedValue({ - success: true, +function executorPrincipal() { + mocks.authenticate.mockResolvedValue({ + kind: 'workflow_execution_delegated', userId: 'user-1', - authType, + workspaceId: WORKSPACE_ID, + executionId: 'exec-1', }) } function unauthenticated() { - hybridAuthMockFns.mockCheckSessionOrInternalAuth.mockResolvedValue({ success: false }) + mocks.authenticate.mockRejectedValue(new InternalUnauthenticatedError()) } function routeContext() { @@ -103,32 +102,20 @@ function getRequest(workspaceId: string | null = WORKSPACE_ID) { return new NextRequest(url, { method: 'GET' }) } -function bodyRequest(method: 'PATCH' | 'DELETE', body: unknown) { +function bodyRequest(method: 'PATCH' | 'DELETE', body: unknown, headers: HeadersInit = {}) { return new NextRequest(`http://localhost/api/table/${TABLE_ID}/rows/${ROW_ID}`, { method, - headers: { 'content-type': 'application/json' }, + headers: { 'content-type': 'application/json', ...headers }, body: JSON.stringify(body), }) } beforeEach(() => { vi.clearAllMocks() - resetDbChainMock() - authAs() - mockCheckAccess.mockResolvedValue({ - ok: true, - table: createTableDefinition({ - id: TABLE_ID, - columns: [ - { id: 'col_aaa', name: 'Name', type: 'string' }, - { id: 'col_bbb', name: 'Age', type: 'number' }, - ], - maxRows: 100, - workspaceId: WORKSPACE_ID, - createdAt: CREATED_AT, - updatedAt: UPDATED_AT, - }), - }) + sessionPrincipal() + mocks.readRow.mockResolvedValue({ table: TABLE, row: ROW }) + mocks.updateRow.mockResolvedValue({ table: TABLE, row: ROW, changed: true }) + mocks.deleteRow.mockResolvedValue({ table: TABLE, deletedRowId: ROW_ID }) }) describe('GET /api/table/[tableId]/rows/[rowId]', () => { @@ -138,45 +125,35 @@ describe('GET /api/table/[tableId]/rows/[rowId]', () => { const response = await GET(getRequest(), routeContext()) expect(response.status).toBe(401) - await expect(response.json()).resolves.toEqual({ error: 'Authentication required' }) - expect(mockCheckAccess).not.toHaveBeenCalled() + expect(mocks.readRow).not.toHaveBeenCalled() }) it('returns 400 when workspaceId is absent from the query string', async () => { const response = await GET(getRequest(null), routeContext()) expect(response.status).toBe(400) - expect(mockCheckAccess).not.toHaveBeenCalled() - }) - - it('propagates the access decision when the caller lacks read access', async () => { - mockCheckAccess.mockResolvedValue({ ok: false, status: 403 }) - - const response = await GET(getRequest(), routeContext()) - - expect(response.status).toBe(403) - expect(mockCheckAccess).toHaveBeenCalledWith(TABLE_ID, 'user-1', 'read') + expect(mocks.readRow).not.toHaveBeenCalled() }) - it('returns 400 when the asserted workspace does not own the table', async () => { - const response = await GET(getRequest('workspace-other'), routeContext()) + it('asserts the caller-supplied workspace on the use case rather than checking it here', async () => { + await GET(getRequest(), routeContext()) - expect(response.status).toBe(400) - await expect(response.json()).resolves.toEqual({ error: 'Invalid workspace ID' }) + expect(mocks.readRow.mock.calls[0][0].input).toMatchObject({ + tableId: TABLE_ID, + rowId: ROW_ID, + assertedWorkspaceId: WORKSPACE_ID, + }) }) it('returns 404 when the row does not exist', async () => { - queueTableRows(schemaMock.userTableRows, []) + mocks.readRow.mockRejectedValue(new OrchestrationError('not_found', 'Row not found')) const response = await GET(getRequest(), routeContext()) expect(response.status).toBe(404) - await expect(response.json()).resolves.toEqual({ error: 'Row not found' }) }) it('returns the row with ISO-8601 timestamps under data.row', async () => { - queueTableRows(schemaMock.userTableRows, [buildStoredRow()]) - const response = await GET(getRequest(), routeContext()) expect(response.status).toBe(200) @@ -193,25 +170,27 @@ describe('GET /api/table/[tableId]/rows/[rowId]', () => { }, }) }) + + it('returns column names to a workflow execution', async () => { + executorPrincipal() + + const response = await GET(getRequest(), routeContext()) + + const body = await response.json() + expect(body.data.row.data).toEqual({ Name: 'Ada', Age: 36 }) + }) }) describe('PATCH /api/table/[tableId]/rows/[rowId]', () => { const patchBody = { workspaceId: WORKSPACE_ID, data: { col_aaa: 'Grace' } } - beforeEach(() => { - mockUpdateRow.mockResolvedValue({ - ...buildStoredRow(), - data: { col_aaa: 'Grace', col_bbb: 36 }, - }) - }) - it('returns 401 when the caller is not authenticated', async () => { unauthenticated() const response = await PATCH(bodyRequest('PATCH', patchBody), routeContext()) expect(response.status).toBe(401) - expect(mockUpdateRow).not.toHaveBeenCalled() + expect(mocks.updateRow).not.toHaveBeenCalled() }) it('returns 400 when the body fails contract validation', async () => { @@ -221,27 +200,7 @@ describe('PATCH /api/table/[tableId]/rows/[rowId]', () => { ) expect(response.status).toBe(400) - expect(mockUpdateRow).not.toHaveBeenCalled() - }) - - it('requires write access, not read access', async () => { - mockCheckAccess.mockResolvedValue({ ok: false, status: 403 }) - - const response = await PATCH(bodyRequest('PATCH', patchBody), routeContext()) - - expect(response.status).toBe(403) - expect(mockCheckAccess).toHaveBeenCalledWith(TABLE_ID, 'user-1', 'write') - }) - - it('returns 400 when the asserted workspace does not own the table', async () => { - const response = await PATCH( - bodyRequest('PATCH', { ...patchBody, workspaceId: 'workspace-other' }), - routeContext() - ) - - expect(response.status).toBe(400) - await expect(response.json()).resolves.toEqual({ error: 'Invalid workspace ID' }) - expect(mockUpdateRow).not.toHaveBeenCalled() + expect(mocks.updateRow).not.toHaveBeenCalled() }) it('returns the updated row and the success message', async () => { @@ -253,7 +212,7 @@ describe('PATCH /api/table/[tableId]/rows/[rowId]', () => { data: { row: { id: ROW_ID, - data: { col_aaa: 'Grace', col_bbb: 36 }, + data: { col_aaa: 'Ada', col_bbb: 36 }, position: 0, createdAt: CREATED_AT.toISOString(), updatedAt: UPDATED_AT.toISOString(), @@ -263,64 +222,46 @@ describe('PATCH /api/table/[tableId]/rows/[rowId]', () => { }) }) - it('passes the acting user and the column-keyed patch to updateRow', async () => { + it('tells the use case a session speaks column ids', async () => { await PATCH(bodyRequest('PATCH', patchBody), routeContext()) - expect(mockUpdateRow).toHaveBeenCalledTimes(1) - const [input, table] = mockUpdateRow.mock.calls[0] - expect(input).toMatchObject({ - tableId: TABLE_ID, - rowId: ROW_ID, - workspaceId: WORKSPACE_ID, - actorUserId: 'user-1', + expect(mocks.updateRow.mock.calls[0][0].input).toMatchObject({ data: { col_aaa: 'Grace' }, + dataKeying: 'ids', + strictWrite: false, }) - expect(table.id).toBe(TABLE_ID) }) - it('translates column names to ids for an internal JWT caller', async () => { - authAs('internal_jwt') + it('tells the use case a workflow execution speaks column names', async () => { + executorPrincipal() await PATCH( bodyRequest('PATCH', { workspaceId: WORKSPACE_ID, data: { Name: 'Grace' } }), routeContext() ) - expect(mockUpdateRow.mock.calls[0][0]).toMatchObject({ data: { col_aaa: 'Grace' } }) - }) - - it('returns column names to an internal JWT caller', async () => { - authAs('internal_jwt') - - const response = await PATCH( - bodyRequest('PATCH', { workspaceId: WORKSPACE_ID, data: { Name: 'Grace' } }), - routeContext() - ) - - const body = await response.json() - expect(body.data.row.data).toEqual({ Name: 'Grace', Age: 36 }) + expect(mocks.updateRow.mock.calls[0][0].input).toMatchObject({ + data: { Name: 'Grace' }, + dataKeying: 'names', + }) }) - it('signals open collaborators that the row changed', async () => { + it('hands the provenance envelope over unresolved rather than interpreting it', async () => { await PATCH(bodyRequest('PATCH', patchBody), routeContext()) - expect(mockSignalTableRowsChangedByActor).toHaveBeenCalledWith(TABLE_ID, undefined) - }) - - it('forwards the originating tab id so that tab ignores its own broadcast', async () => { - const request = new NextRequest(`http://localhost/api/table/${TABLE_ID}/rows/${ROW_ID}`, { - method: 'PATCH', - headers: { 'content-type': 'application/json', 'x-sim-client-id': 'tab-42' }, - body: JSON.stringify(patchBody), + expect(mocks.updateRow.mock.calls[0][0].input.secretProvenanceEnvelope).toEqual({ + kind: 'none', }) + }) - await PATCH(request, routeContext()) + it('forwards the originating tab so that tab can skip its own refetch', async () => { + await PATCH(bodyRequest('PATCH', patchBody, { 'x-sim-client-id': 'tab-42' }), routeContext()) - expect(mockSignalTableRowsChangedByActor).toHaveBeenCalledWith(TABLE_ID, 'tab-42') + expect(mocks.updateRow.mock.calls[0][0].input.actorClientId).toBe('tab-42') }) it('projects a classified orchestration failure instead of a generic 500', async () => { - mockUpdateRow.mockRejectedValue(Object.assign(new Error('conflict'), { __orchestrated: true })) + mocks.updateRow.mockRejectedValue(new OrchestrationError('conflict', 'Row changed')) const response = await PATCH(bodyRequest('PATCH', patchBody), routeContext()) @@ -328,49 +269,24 @@ describe('PATCH /api/table/[tableId]/rows/[rowId]', () => { }) it('falls back to 500 for an unclassified failure', async () => { - mockUpdateRow.mockRejectedValue(new Error('boom')) + mocks.updateRow.mockRejectedValue(new Error('boom')) const response = await PATCH(bodyRequest('PATCH', patchBody), routeContext()) expect(response.status).toBe(500) - await expect(response.json()).resolves.toEqual({ error: 'Failed to update row' }) }) }) describe('DELETE /api/table/[tableId]/rows/[rowId]', () => { const deleteBody = { workspaceId: WORKSPACE_ID } - beforeEach(() => { - mockPerformDeleteTableRow.mockResolvedValue({ success: true }) - }) - it('returns 401 when the caller is not authenticated', async () => { unauthenticated() const response = await DELETE(bodyRequest('DELETE', deleteBody), routeContext()) expect(response.status).toBe(401) - expect(mockPerformDeleteTableRow).not.toHaveBeenCalled() - }) - - it('requires write access', async () => { - mockCheckAccess.mockResolvedValue({ ok: false, status: 403 }) - - const response = await DELETE(bodyRequest('DELETE', deleteBody), routeContext()) - - expect(response.status).toBe(403) - expect(mockCheckAccess).toHaveBeenCalledWith(TABLE_ID, 'user-1', 'write') - }) - - it('returns 400 when the asserted workspace does not own the table', async () => { - const response = await DELETE( - bodyRequest('DELETE', { workspaceId: 'workspace-other' }), - routeContext() - ) - - expect(response.status).toBe(400) - await expect(response.json()).resolves.toEqual({ error: 'Invalid workspace ID' }) - expect(mockPerformDeleteTableRow).not.toHaveBeenCalled() + expect(mocks.deleteRow).not.toHaveBeenCalled() }) it('reports a deleted count of one on success', async () => { @@ -381,26 +297,53 @@ describe('DELETE /api/table/[tableId]/rows/[rowId]', () => { success: true, data: { message: 'Row deleted successfully', deletedCount: 1 }, }) - expect(mockSignalTableRowsChangedByActor).toHaveBeenCalledWith(TABLE_ID, undefined) }) - it('projects an unsuccessful delete outcome as a client error', async () => { - mockPerformDeleteTableRow.mockResolvedValue({ success: false }) + it('forwards the originating tab so that tab can skip its own refetch', async () => { + await DELETE(bodyRequest('DELETE', deleteBody, { 'x-sim-client-id': 'tab-42' }), routeContext()) - const response = await DELETE(bodyRequest('DELETE', deleteBody), routeContext()) + expect(mocks.deleteRow.mock.calls[0][0].input.actorClientId).toBe('tab-42') + }) +}) - expect(response.status).toBe(400) - await expect(response.json()).resolves.toEqual({ error: 'Failed to delete row' }) - expect(mockSignalTableRowsChangedByActor).not.toHaveBeenCalled() +/** + * The wire changes the migration makes on purpose. + * + * Both follow from adopting the shared concealment policy — what the v2 table + * surface already does, and what stops a caller learning whether a table it + * cannot reach exists. Nothing in `hooks/queries/tables.ts` branches on either + * status, which is why they are safe to change. + * + * Note the concealment is narrower than the handler it replaces: the old route + * answered a blanket 403 for every access failure, while this one conceals only + * *cross-tenant* denials and still answers 403 for an in-workspace role denial. + */ +describe('deliberate wire changes', () => { + it('conceals a cross-tenant table as 404, where it used to answer 403', async () => { + mocks.readRow.mockRejectedValue(new NoWorkspaceAccessError()) + + const response = await GET(getRequest(), routeContext()) + + expect(response.status).toBe(404) + await expect(response.json()).resolves.toMatchObject({ error: 'Table not found' }) }) - it('projects a table lock failure ahead of the generic handler', async () => { - mockPerformDeleteTableRow.mockRejectedValue( - Object.assign(new Error('locked'), { __locked: true }) - ) + it('still answers 403 for an in-workspace denial, which is not concealed', async () => { + mocks.readRow.mockRejectedValue(new OrchestrationError('forbidden', 'Insufficient role')) - const response = await DELETE(bodyRequest('DELETE', deleteBody), routeContext()) + const response = await GET(getRequest(), routeContext()) + + expect(response.status).toBe(403) + }) - expect(response.status).toBe(423) + it('conceals a mismatched workspace assertion as 404, where it used to answer 400', async () => { + mocks.updateRow.mockRejectedValue(new OrchestrationError('not_found', 'Table not found')) + + const response = await PATCH( + bodyRequest('PATCH', { workspaceId: 'workspace-other', data: { col_aaa: 'x' } }), + routeContext() + ) + + expect(response.status).toBe(404) }) }) diff --git a/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts b/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts index 66ec90870f8..63780882463 100644 --- a/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts +++ b/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts @@ -1,274 +1,127 @@ -import { db } from '@sim/db' -import { userTableRows } from '@sim/db/schema' -import { createLogger } from '@sim/logger' -import { and, eq } from 'drizzle-orm' -import { type NextRequest, NextResponse } from 'next/server' +import type { Principal } from '@sim/auth/principal' import { readClientId } from '@/lib/api/client-id' import { deleteTableRowContract, - getTableQuerySchema, + getTableRowContract, updateTableRowContract, } from '@/lib/api/contracts/tables' -import { isZodError, parseRequest, validationErrorResponse } from '@/lib/api/server/validation' -import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' -import { generateRequestId } from '@/lib/core/utils/request' -import { withRouteHandler } from '@/lib/core/utils/with-route-handler' -import type { RowData, TableSchema } from '@/lib/table' -import { updateRow } from '@/lib/table' -import { signalTableRowsChangedByActor } from '@/lib/table/events' -import { performDeleteTableRow } from '@/lib/table/orchestration' +import { defineInternalJsonRoute, internalRateLimits } from '@/lib/api/server/routes' +import { AuthType, type AuthTypeValue } from '@/lib/auth/hybrid' +import { internalTableRowsErrorPolicy, internalTableSessionOrExecutorAuth } from '@/lib/table/api' +import { tableOperations } from '@/lib/table/application/operations' +import type { TableRowDataKeying } from '@/lib/table/application/rows' +import { deleteTableRow, readTableRow, updateTableRow } from '@/lib/table/application/rows' +import type { RowData, TableDefinition, TableRow } from '@/lib/table/types' import { - createTableRowsResponse, - createTableWriteProvenanceTargets, - resolveTableWriteSecretProvenance, + finalizeTableRowsProvenance, + negotiateTableRowsProvenance, + readTableRowProvenanceEnvelope, } from '@/app/api/table/row-secret-provenance' import { rowWireTranslators } from '@/app/api/table/row-wire' -import { - accessError, - checkAccess, - orchestrationErrorResponse, - orchestrationOutcomeErrorResponse, - tableLockErrorResponse, -} from '@/app/api/table/utils' - -const logger = createLogger('TableRowAPI') -interface RowRouteParams { - params: Promise<{ tableId: string; rowId: string }> +export const dynamic = 'force-dynamic' + +/** + * One path, two caller kinds, two column keyings. + * + * The first-party grid holds the schema it rendered and addresses cells by + * stable column id; a workflow tool execution speaks column names, because names + * are what tool enrichment surfaces to the model. The keying is a property of + * the caller rather than of the endpoint, which is why it is derived from the + * principal here and passed to the use case rather than assumed by it. + */ +function authTypeFor(principal: Principal): AuthTypeValue { + return principal.kind === 'session' ? AuthType.SESSION : AuthType.INTERNAL_JWT } -/** GET /api/table/[tableId]/rows/[rowId] - Retrieves a single row. */ -export const GET = withRouteHandler(async (request: NextRequest, { params }: RowRouteParams) => { - const requestId = generateRequestId() - const { tableId, rowId } = await params - - try { - const authResult = await checkSessionOrInternalAuth(request, { requireWorkflowId: false }) - if (!authResult.success || !authResult.userId) { - return NextResponse.json({ error: 'Authentication required' }, { status: 401 }) - } - - const { searchParams } = new URL(request.url) - const validated = getTableQuerySchema.parse({ - workspaceId: searchParams.get('workspaceId'), - }) - - const result = await checkAccess(tableId, authResult.userId, 'read') - if (!result.ok) return accessError(result, requestId, tableId) - - const { table } = result - - if (table.workspaceId !== validated.workspaceId) { - return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 }) - } - - const [row] = await db - .select({ - id: userTableRows.id, - data: userTableRows.data, - position: userTableRows.position, - createdAt: userTableRows.createdAt, - updatedAt: userTableRows.updatedAt, - }) - .from(userTableRows) - .where( - and( - eq(userTableRows.id, rowId), - eq(userTableRows.tableId, tableId), - eq(userTableRows.workspaceId, validated.workspaceId) - ) - ) - .limit(1) - - if (!row) { - return NextResponse.json({ error: 'Row not found' }, { status: 404 }) - } - - logger.info(`[${requestId}] Retrieved row ${rowId} from table ${tableId}`) - - const wire = rowWireTranslators(authResult.authType, table.schema as TableSchema) - - const responseBody = { - success: true, - data: { - row: { - id: row.id, - data: wire.dataOut(row.data as RowData), - position: row.position, - createdAt: - row.createdAt instanceof Date ? row.createdAt.toISOString() : String(row.createdAt), - updatedAt: - row.updatedAt instanceof Date ? row.updatedAt.toISOString() : String(row.updatedAt), - }, - }, - } - return createTableRowsResponse({ - request, - authType: authResult.authType, - userId: authResult.userId, - workspaceId: table.workspaceId, - body: responseBody, - rows: [{ ...row, data: row.data as RowData }], - }) - } catch (error) { - if (isZodError(error)) { - return validationErrorResponse(error) - } +function keyingFor(authType: AuthTypeValue): TableRowDataKeying { + return authType === AuthType.INTERNAL_JWT ? 'names' : 'ids' +} - logger.error(`[${requestId}] Error getting row:`, error) - return NextResponse.json({ error: 'Failed to get row' }, { status: 500 }) +/** The narrower projection these routes have always returned. */ +function presentRow(row: TableRow, table: TableDefinition, principal: Principal) { + const wire = rowWireTranslators(authTypeFor(principal), table.schema) + return { + id: row.id, + data: wire.dataOut(row.data), + position: row.position, + createdAt: row.createdAt instanceof Date ? row.createdAt.toISOString() : String(row.createdAt), + updatedAt: row.updatedAt instanceof Date ? row.updatedAt.toISOString() : String(row.updatedAt), } -}) - -/** PATCH /api/table/[tableId]/rows/[rowId] - Updates a single row (supports partial updates). */ -export const PATCH = withRouteHandler(async (request: NextRequest, context: RowRouteParams) => { - const requestId = generateRequestId() - - try { - const authResult = await checkSessionOrInternalAuth(request, { requireWorkflowId: false }) - if (!authResult.success || !authResult.userId) { - return NextResponse.json({ error: 'Authentication required' }, { status: 401 }) - } - - const parsed = await parseRequest(updateTableRowContract, request, context, { - validationErrorResponse: (error) => validationErrorResponse(error), - }) - if (!parsed.success) return parsed.response - - const { tableId, rowId } = parsed.data.params - const validated = parsed.data.body - - const result = await checkAccess(tableId, authResult.userId, 'write') - if (!result.ok) return accessError(result, requestId, tableId) - - const { table } = result - - if (table.workspaceId !== validated.workspaceId) { - return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 }) - } - - const wire = rowWireTranslators(authResult.authType, table.schema as TableSchema) - const rowData = validated.data as RowData - const provenance = resolveTableWriteSecretProvenance({ - request, - payload: validated, - authType: authResult.authType, - userId: authResult.userId, - workspaceId: table.workspaceId, - targets: createTableWriteProvenanceTargets([rowData], wire.dataIn), - rowKeys: ['0'], - }) - if (!provenance.success) return provenance.response - const updatedRow = await updateRow( - { - tableId, - rowId, - data: wire.dataIn(rowData), - workspaceId: validated.workspaceId, - actorUserId: authResult.userId, - secretProvenance: provenance.provenanceByRowKey?.['0'], - }, - table, - requestId - ) - - // Live-collab: tell open viewers the change landed so they refetch. - signalTableRowsChangedByActor(tableId, readClientId(request)) - // Only `null` when a `cancellationGuard` is supplied and the SQL guard - // rejects the write — this route doesn't pass one, so reaching null is a bug. - if (!updatedRow) throw new Error('updateRow returned null without a cancellationGuard') - // Auto-dispatch for user edits is handled inside `updateRow` (mode: 'new'). - // Firing a second mode: 'incomplete' dispatch here would race with the - // `mode: 'new'` one AND bulk-clear sibling-group outputs (the incomplete - // bulk-clear wipes ALL targeted columns when any one column on the row - // is empty). - - const responseBody = { - success: true, - data: { - row: { - id: updatedRow.id, - data: wire.dataOut(updatedRow.data), - position: updatedRow.position, - createdAt: - updatedRow.createdAt instanceof Date - ? updatedRow.createdAt.toISOString() - : updatedRow.createdAt, - updatedAt: - updatedRow.updatedAt instanceof Date - ? updatedRow.updatedAt.toISOString() - : updatedRow.updatedAt, - }, - message: 'Row updated successfully', - }, - } - return createTableRowsResponse({ - request, - authType: authResult.authType, - userId: authResult.userId, - workspaceId: table.workspaceId, - body: responseBody, - rows: [updatedRow], - }) - } catch (error) { - const response = orchestrationErrorResponse(error) - if (response) return response +} - logger.error(`[${requestId}] Error updating row:`, error) - return NextResponse.json({ error: 'Failed to update row' }, { status: 500 }) - } +const rateLimit = internalRateLimits.none({ + reason: 'Preserve existing internal single-row table behavior', }) -/** DELETE /api/table/[tableId]/rows/[rowId] - Deletes a single row. */ -export const DELETE = withRouteHandler(async (request: NextRequest, context: RowRouteParams) => { - const requestId = generateRequestId() - - try { - const authResult = await checkSessionOrInternalAuth(request, { requireWorkflowId: false }) - if (!authResult.success || !authResult.userId) { - return NextResponse.json({ error: 'Authentication required' }, { status: 401 }) - } - - const parsed = await parseRequest(deleteTableRowContract, request, context, { - validationErrorResponse: (error) => validationErrorResponse(error), - }) - if (!parsed.success) return parsed.response - - const { tableId, rowId } = parsed.data.params - const validated = parsed.data.body - - const result = await checkAccess(tableId, authResult.userId, 'write') - if (!result.ok) return accessError(result, requestId, tableId) - - const { table } = result - - if (table.workspaceId !== validated.workspaceId) { - return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 }) - } +export const GET = defineInternalJsonRoute({ + contract: getTableRowContract, + operation: tableOperations.readRow, + auth: internalTableSessionOrExecutorAuth, + rateLimit, + errorPolicy: internalTableRowsErrorPolicy, + mapInput: ({ params, query }, { principal, request }) => ({ + tableId: params.tableId, + rowId: params.rowId, + assertedWorkspaceId: query.workspaceId, + includePersistedSecretProvenance: negotiateTableRowsProvenance(request, authTypeFor(principal)), + }), + useCase: readTableRow, + present: ({ table, row }, { principal }) => ({ + success: true as const, + data: { row: presentRow(row, table, principal) }, + }), + finalizeResponse: ({ result }) => finalizeTableRowsProvenance(result.secretProvenance), +}) - const outcome = await performDeleteTableRow({ table, rowId, requestId }) - if (!outcome.success) { - return orchestrationOutcomeErrorResponse(outcome, 'Failed to delete row') +export const PATCH = defineInternalJsonRoute({ + contract: updateTableRowContract, + operation: tableOperations.updateRow, + auth: internalTableSessionOrExecutorAuth, + rateLimit, + errorPolicy: internalTableRowsErrorPolicy, + mapInput: ({ params, body }, { principal, request }) => { + const authType = authTypeFor(principal) + return { + tableId: params.tableId, + rowId: params.rowId, + assertedWorkspaceId: body.workspaceId, + data: body.data as RowData, + dataKeying: keyingFor(authType), + strictWrite: false, + // Handed over unresolved: interpreting the selections needs the canonical + // schema, which this adapter must not load. + secretProvenanceEnvelope: readTableRowProvenanceEnvelope(request, body), + includePersistedSecretProvenance: negotiateTableRowsProvenance(request, authType), + actorClientId: readClientId(request), } + }, + useCase: updateTableRow, + present: ({ table, row }, { principal }) => ({ + success: true as const, + data: { + row: presentRow(row, table, principal), + message: 'Row updated successfully', + }, + }), + finalizeResponse: ({ result }) => finalizeTableRowsProvenance(result.secretProvenance), +}) - // Live-collab: tell open viewers the change landed so they refetch. - signalTableRowsChangedByActor(tableId, readClientId(request)) - - return NextResponse.json({ - success: true, - data: { - message: 'Row deleted successfully', - deletedCount: 1, - }, - }) - } catch (error) { - const lockError = tableLockErrorResponse(error) - if (lockError) return lockError - - const classified = orchestrationErrorResponse(error) - if (classified) return classified - - logger.error(`[${requestId}] Error deleting row:`, error) - return NextResponse.json({ error: 'Failed to delete row' }, { status: 500 }) - } +export const DELETE = defineInternalJsonRoute({ + contract: deleteTableRowContract, + operation: tableOperations.deleteRow, + auth: internalTableSessionOrExecutorAuth, + rateLimit, + errorPolicy: internalTableRowsErrorPolicy, + mapInput: ({ params, body }, { request }) => ({ + tableId: params.tableId, + rowId: params.rowId, + assertedWorkspaceId: body.workspaceId, + actorClientId: readClientId(request), + }), + useCase: deleteTableRow, + present: () => ({ + success: true as const, + data: { message: 'Row deleted successfully', deletedCount: 1 }, + }), }) diff --git a/apps/sim/app/api/table/row-secret-provenance.ts b/apps/sim/app/api/table/row-secret-provenance.ts index 9d72ce58990..5021b7b1de6 100644 --- a/apps/sim/app/api/table/row-secret-provenance.ts +++ b/apps/sim/app/api/table/row-secret-provenance.ts @@ -10,6 +10,10 @@ import { RESOLVED_SECRET_PROVENANCE_METADATA_V1, serializePrivateToolMetadataResponseEnvelope, } from '@/lib/execution/private-tool-metadata' +import { + type TableRowProvenanceEnvelope, + TableRowProvenanceError, +} from '@/lib/table/application/row-secret-provenance' import { loadTableRowSecretProvenance } from '@/lib/table/rows/secret-provenance' import { tableRowSecretProvenanceSelectionKey } from '@/lib/table/secret-provenance-selection' import type { RowData, TableRowSecretProvenanceWrite } from '@/lib/table/types' @@ -195,3 +199,57 @@ export async function createTableRowsResponse(options: { ) return NextResponse.json(envelope.body, { headers: envelope.headers }) } + +/** + * Reads the private provenance envelope off the request. Transport only — the + * selections are interpreted against the canonical schema inside the use case, + * by {@link resolveRowWriteProvenance}. + */ +export function readTableRowProvenanceEnvelope( + request: NextRequest, + payload: unknown +): TableRowProvenanceEnvelope { + const inspection = inspectPrivateSecretProvenanceRequest(request.headers, payload) + if (inspection.status === 'unsupported') return { kind: 'none' } + if (inspection.status !== 'verified') throw new TableRowProvenanceError() + return { kind: 'bundle', value: inspection.value } +} + +/** + * Whether this caller asked for persisted row provenance on the response. + * + * Only an authenticated internal caller that explicitly requested the capability + * gets it; every ordinary UI and API response keeps its existing wire shape. The + * answer feeds the use case's `includePersistedSecretProvenance`, so the load + * itself happens inside the authorized operation rather than in the adapter. + */ +export function negotiateTableRowsProvenance( + request: NextRequest, + authType: AuthTypeValue | undefined +): boolean { + const negotiation = negotiatePrivateToolMetadataResponse( + request.headers, + RESOLVED_SECRET_PROVENANCE_METADATA_V1, + authType === AuthType.INTERNAL_JWT + ) + if (negotiation.status === 'rejected') throw new TableRowProvenanceError() + return negotiation.status !== 'not-requested' +} + +/** + * The response half of the envelope, shaped for a declarative route's + * `finalizeResponse`: one sibling body field and one capability header, added + * only when the use case actually loaded provenance. + */ +export function finalizeTableRowsProvenance(provenance: unknown): { + bodyFields?: Record + headers?: HeadersInit +} { + if (provenance === undefined) return {} + const envelope = serializePrivateToolMetadataResponseEnvelope( + {}, + RESOLVED_SECRET_PROVENANCE_METADATA_V1, + provenance + ) + return { bodyFields: envelope.body, headers: envelope.headers } +} diff --git a/apps/sim/lib/api/contracts/tables.ts b/apps/sim/lib/api/contracts/tables.ts index 0ed44ed4543..9550d386b7a 100644 --- a/apps/sim/lib/api/contracts/tables.ts +++ b/apps/sim/lib/api/contracts/tables.ts @@ -350,6 +350,25 @@ export const rowDataSchema = domainObjectSchema() export const tableDefinitionSchema = domainObjectSchema() export const tableRowSchema = domainObjectSchema() +/** + * One row as the single-row routes actually emit it: the stored cells plus + * position, with timestamps already serialized. + * + * Deliberately not {@link tableRowSchema}. That one describes a `TableRow`, + * which carries the per-cell `executions` sidecar and `Date` objects — accurate + * for the list and query routes, which return exactly that, and wrong for the + * single-row routes, which have always projected a narrower object with ISO + * strings. Two shapes on the wire need two schemas; collapsing them would make + * one of the two lie to its clients. + */ +export const tableRowWireSchema = z.object({ + id: z.string(), + data: rowDataSchema, + position: z.number(), + createdAt: z.string(), + updatedAt: z.string(), +}) + /** * Plain-object base for the single-row insert body. Kept un-refined so callers * (e.g. the v1 public contract) can `.omit()` fields before applying @@ -1433,6 +1452,22 @@ export const upsertTableRowContract = defineRouteContract({ }, }) +/** + * Reads one row. The sibling of {@link updateTableRowContract} and + * {@link deleteTableRowContract}, which take their workspace scope from a body; + * a GET has none, so it is asserted on the query string instead. + */ +export const getTableRowContract = defineRouteContract({ + method: 'GET', + path: '/api/table/[tableId]/rows/[rowId]', + params: tableRowParamsSchema, + query: getTableQuerySchema, + response: { + mode: 'json', + schema: successResponseSchema(z.object({ row: tableRowWireSchema })), + }, +}) + export const updateTableRowContract = defineRouteContract({ method: 'PATCH', path: '/api/table/[tableId]/rows/[rowId]', @@ -1442,7 +1477,7 @@ export const updateTableRowContract = defineRouteContract({ mode: 'json', schema: successResponseSchema( z.object({ - row: tableRowSchema, + row: tableRowWireSchema, message: z.string(), }) ), diff --git a/apps/sim/lib/api/server/routes/internal-json-route.ts b/apps/sim/lib/api/server/routes/internal-json-route.ts index 8e362c35f66..c9898c2f291 100644 --- a/apps/sim/lib/api/server/routes/internal-json-route.ts +++ b/apps/sim/lib/api/server/routes/internal-json-route.ts @@ -199,14 +199,36 @@ type InternalJsonParseOptions = Pick< 'maxBodyBytes' | 'validationErrorResponse' > -type InternalJsonPresenter = [R] extends [ - ContractJsonResponse, -] +/** + * What a presenter may render from, beyond the use case's result. + * + * A surface that serves more than one caller kind can owe them different wire + * shapes for the same domain result — the internal table row routes answer a + * session in stable column ids and a workflow execution in column names. That is + * presentation, not domain, so it belongs in the adapter rather than the use + * case. {@link InternalJsonRouteOptions.responseHeaders} and + * {@link InternalJsonRouteOptions.finalizeResponse} already receive this pair; + * this closes the same gap for `present`. + */ +export interface InternalJsonPresenterContext { + principal: P + input: I +} + +type InternalJsonPresenter = [ + R, +] extends [ContractJsonResponse] ? { - present?(result: NoInfer): ContractJsonResponse | Promise> + present?( + result: NoInfer, + context: InternalJsonPresenterContext, P> + ): ContractJsonResponse | Promise> } : { - present(result: NoInfer): ContractJsonResponse | Promise> + present( + result: NoInfer, + context: InternalJsonPresenterContext, P> + ): ContractJsonResponse | Promise> } type InternalJsonRouteOptions< @@ -239,7 +261,7 @@ type InternalJsonRouteOptions< result: NoInfer body: ContractJsonResponse }): InternalJsonResponseFinalization | Promise -} & InternalJsonPresenter +} & InternalJsonPresenter function createJsonErrorResponse(descriptor: JsonErrorResponseDescriptor): NextResponse { return NextResponse.json(withRequestId(descriptor.body), { @@ -333,7 +355,7 @@ export function defineInternalJsonRoute< request, }) await options.onSuccess?.({ principal, input, result }) - const body = options.present ? await options.present(result) : result + const body = options.present ? await options.present(result, { principal, input }) : result const responseSchema = options.contract.response if (responseSchema.mode !== 'json') { throw new Error('Internal JSON route response mode changed after initialization') diff --git a/apps/sim/lib/table/api/index.ts b/apps/sim/lib/table/api/index.ts index e04cc23089e..22d2d977f04 100644 --- a/apps/sim/lib/table/api/index.ts +++ b/apps/sim/lib/table/api/index.ts @@ -1,5 +1,6 @@ export { internalTableErrorPolicies, + internalTableRowsErrorPolicy, internalTableSessionOrExecutorAuth, v2TableErrorPolicies, } from '@/lib/table/api/route-policies' diff --git a/apps/sim/lib/table/api/route-policies.ts b/apps/sim/lib/table/api/route-policies.ts index 679c0deb676..edb2fbd3ef1 100644 --- a/apps/sim/lib/table/api/route-policies.ts +++ b/apps/sim/lib/table/api/route-policies.ts @@ -9,6 +9,8 @@ import { } from '@/lib/api/server/routes' import { TABLE_DELEGATION_AUDIENCE } from '@/lib/table/application/authorization' import { TableOperationError } from '@/lib/table/application/errors' +import { TableRowProvenanceError } from '@/lib/table/application/row-secret-provenance' +import { TableRowsValidationError } from '@/lib/table/application/rows' import { TableLockedError } from '@/lib/table/mutation-locks' import { v2CaughtOrchestrationError, @@ -97,3 +99,23 @@ export const internalTableErrorPolicies = { notFoundMessage: 'Table export not found', }), } as const + +/** + * Row routes on the internal surface. The internal counterpart of + * {@link v2TableRowsErrorPolicy}: a row-shape complaint and a provenance + * envelope that does not authenticate are both the caller's to fix and answer + * 400; everything else conceals a cross-tenant table behind the same not-found + * wording the rest of the table surface uses. + */ +export const internalTableRowsErrorPolicy = extendInternalErrorPolicy( + internalTableErrorPolicies.concealTableAuthorization, + (error) => { + if (error instanceof TableRowsValidationError) { + return internalErrorResponse(400, { error: error.message }) + } + if (error instanceof TableRowProvenanceError) { + return internalErrorResponse(400, { error: error.message }) + } + return null + } +) diff --git a/apps/sim/lib/table/application/row-secret-provenance.test.ts b/apps/sim/lib/table/application/row-secret-provenance.test.ts new file mode 100644 index 00000000000..bd8e2a09046 --- /dev/null +++ b/apps/sim/lib/table/application/row-secret-provenance.test.ts @@ -0,0 +1,173 @@ +/** + * @vitest-environment node + * + * The provenance envelope moved out of the route adapter and into the domain, + * because interpreting a caller's selections requires the canonical schema and + * the adapter must not load it. These pin the semantics that move with it. + */ +import { describe, expect, it, vi } from 'vitest' + +const { mocks } = vi.hoisted(() => ({ + mocks: { scopeCompatible: vi.fn(() => true), isBundle: vi.fn(() => true) }, +})) + +vi.mock('@/lib/execution/durable-secret-provenance', () => ({ + isPrivateSecretProvenanceScopeCompatible: mocks.scopeCompatible, +})) + +vi.mock('@/lib/execution/model-input-provenance', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, isPrivateSecretProvenanceBundleV1: mocks.isBundle } +}) + +import { + resolveRowWriteProvenance, + TableRowProvenanceError, +} from '@/lib/table/application/row-secret-provenance' +import type { TableDefinition } from '@/lib/table/types' + +const TABLE = { + id: 'tbl_1', + workspaceId: 'workspace-1', + schema: { + columns: [ + { id: 'col_aaa', name: 'Name', type: 'string' }, + { id: 'col_bbb', name: 'Age', type: 'number' }, + ], + }, +} as unknown as TableDefinition + +const SESSION = { kind: 'session' as const, userId: 'user-1', sessionId: 'session-1' } +const EXECUTOR = { + kind: 'workflow_execution_delegated' as const, + userId: 'user-1', + workspaceId: 'workspace-1', + executionId: 'exec-1', +} + +function resolve(overrides: Partial[0]>) { + return resolveRowWriteProvenance({ + envelope: { kind: 'none' }, + principal: SESSION, + workspaceId: 'workspace-1', + table: TABLE, + keying: 'ids', + wireRows: [{ col_aaa: 'Ada' }], + storageRows: [{ col_aaa: 'Ada' }], + ...overrides, + }) +} + +describe('row write provenance', () => { + it('certifies an interactive write exact-empty over the columns it persists', () => { + const { stamps } = resolve({}) + + expect(stamps).toEqual([ + { complete: true, columns: { col_aaa: { version: 1, complete: true, entries: [] } } }, + ]) + }) + + it('leaves an internal caller that sent no envelope untracked', () => { + // Not exact-empty: stamping "this write introduced no secrets" on a runtime + // write that sent no envelope would be a false certification. + const { stamps } = resolve({ principal: EXECUTOR }) + + expect(stamps).toEqual([undefined]) + }) + + it('refuses a bundle from a session caller', () => { + expect(() => + resolve({ envelope: { kind: 'bundle', value: { complete: true, selections: [] } } }) + ).toThrow(TableRowProvenanceError) + }) + + it('refuses a bundle that is not a recognised envelope', () => { + mocks.isBundle.mockReturnValueOnce(false) + + expect(() => + resolve({ principal: EXECUTOR, envelope: { kind: 'bundle', value: { nope: true } } }) + ).toThrow(TableRowProvenanceError) + }) + + it('refuses a complete bundle that does not account for every written cell', () => { + expect(() => + resolve({ + principal: EXECUTOR, + envelope: { kind: 'bundle', value: { complete: true, selections: [] } }, + wireRows: [{ col_aaa: 'Ada', col_bbb: 36 }], + storageRows: [{ col_aaa: 'Ada', col_bbb: 36 }], + }) + ).toThrow(TableRowProvenanceError) + }) + + it('refuses a selection whose scope this principal may not read', () => { + mocks.scopeCompatible.mockReturnValueOnce(false) + + expect(() => + resolve({ + principal: EXECUTOR, + envelope: { + kind: 'bundle', + value: { + complete: true, + selections: [ + { key: JSON.stringify([0, 'col_aaa']), provenance: { scope: { kind: 'workspace' } } }, + ], + }, + }, + }) + ).toThrow(TableRowProvenanceError) + }) + + it('marks an incomplete bundle unknown rather than certifying it', () => { + const { stamps } = resolve({ + principal: EXECUTOR, + envelope: { kind: 'bundle', value: { complete: false, selections: [] } }, + }) + + expect(stamps).toEqual([{ complete: false, columns: {} }]) + }) + + it('keys a name-wire selection to the storage column it certifies', () => { + const { stamps } = resolve({ + principal: EXECUTOR, + keying: 'names', + wireRows: [{ Name: 'Ada' }], + storageRows: [{ col_aaa: 'Ada' }], + envelope: { + kind: 'bundle', + value: { + complete: true, + selections: [ + { key: JSON.stringify([0, 'Name']), provenance: { scope: { kind: 'workspace' } } }, + ], + }, + }, + }) + + expect(stamps[0]).toEqual({ + complete: true, + columns: { col_aaa: { scope: { kind: 'workspace' } } }, + }) + }) + + it('records nothing for a key that names no column, since it is never stored', () => { + const { stamps } = resolve({ + principal: EXECUTOR, + keying: 'names', + wireRows: [{ Nope: 'x' }], + storageRows: [{}], + envelope: { + kind: 'bundle', + value: { + complete: true, + selections: [ + { key: JSON.stringify([0, 'Nope']), provenance: { scope: { kind: 'workspace' } } }, + ], + }, + }, + }) + + expect(stamps[0]).toEqual({ complete: true, columns: {} }) + }) +}) diff --git a/apps/sim/lib/table/application/row-secret-provenance.ts b/apps/sim/lib/table/application/row-secret-provenance.ts new file mode 100644 index 00000000000..5174882c7c4 --- /dev/null +++ b/apps/sim/lib/table/application/row-secret-provenance.ts @@ -0,0 +1,143 @@ +import { type Principal, requirePrincipalSubjectUserId } from '@sim/auth/principal' +import { isPrivateSecretProvenanceScopeCompatible } from '@/lib/execution/durable-secret-provenance' +import { isPrivateSecretProvenanceBundleV1 } from '@/lib/execution/model-input-provenance' +import { buildColumnNameById, buildIdByName } from '@/lib/table/column-keys' +import { createExactEmptyTableRowSecretProvenance } from '@/lib/table/rows/secret-provenance' +import { tableRowSecretProvenanceSelectionKey } from '@/lib/table/secret-provenance-selection' +import type { RowData, TableDefinition, TableRowSecretProvenanceWrite } from '@/lib/table/types' + +/** + * The private provenance envelope exactly as it arrived on the wire. + * + * A surface adapter can read the header and the payload field — that is + * transport — but it cannot decide what the selections mean, because mapping a + * caller's column key to the storage column id it certifies requires the + * canonical schema. That resolution lives here, behind authorization, which is + * what lets the row routes stop loading the table for themselves. + */ +export type TableRowProvenanceEnvelope = { kind: 'none' } | { kind: 'bundle'; value: unknown } + +/** Raised when an envelope does not authenticate against the canonical table. */ +export class TableRowProvenanceError extends Error { + constructor(message = 'Invalid table row secret provenance') { + super(message) + this.name = 'TableRowProvenanceError' + } +} + +/** + * Storage column id for each key the caller wrote, or `null` where the key names + * no column and is therefore never persisted. Mirrors how the row data itself is + * normalized, so a key dropped from the write is dropped from its provenance. + */ +function storageKeyByWireKey( + row: RowData, + table: TableDefinition, + keying: 'names' | 'ids' +): Map { + const wireKeys = Object.keys(row) + if (keying === 'ids') { + // Keyed by `getColumnId`, so a legacy pre-backfill column — stored under its + // name because it has no id — is recognised rather than dropped. + const known = buildColumnNameById(table.schema.columns) + return new Map(wireKeys.map((key) => [key, known.has(key) ? key : null])) + } + const idByName = buildIdByName(table.schema) + return new Map(wireKeys.map((key) => [key, idByName.get(key) ?? null])) +} + +/** + * What a write should stamp on its provenance sidecar. + * + * `undefined` is not "nothing to record" — it means *deliberately untracked*, the + * legacy protocol for an internal caller that sent no envelope. Defaulting it to + * an exact-empty stamp would certify "this write introduced no secrets" on a + * runtime write that may well have introduced some, so the two must stay + * distinguishable all the way to the sidecar. + */ +export interface ResolvedRowWriteProvenance { + stamps: Array +} + +/** + * Resolves a wire envelope into per-row provenance stamps against the canonical + * table. + * + * An interactive caller (session) certifies exact-empty over the storage columns + * its write actually persists. An internal execution may submit an encrypted + * bundle, which must name exactly the columns the write touched and must carry a + * scope this principal is allowed to read. Anything else fails closed. + */ +export function resolveRowWriteProvenance(options: { + envelope: TableRowProvenanceEnvelope + principal: Principal + workspaceId: string + table: TableDefinition + keying: 'names' | 'ids' + wireRows: readonly RowData[] + storageRows: readonly RowData[] +}): ResolvedRowWriteProvenance { + const { envelope, principal, table, keying, wireRows, storageRows } = options + const isDelegated = principal.kind !== 'session' + + if (envelope.kind === 'none') { + // An internal caller that sent nothing stays untracked, as it always has. + if (isDelegated) return { stamps: wireRows.map(() => undefined) } + return { stamps: storageRows.map((row) => createExactEmptyTableRowSecretProvenance(row)) } + } + + if (!isDelegated || !isPrivateSecretProvenanceBundleV1(envelope.value)) { + throw new TableRowProvenanceError() + } + const bundle = envelope.value + + const columnIdBySelectionKey = new Map() + const rowKeyBySelectionKey = new Map() + wireRows.forEach((row, rowIndex) => { + const storageKeys = storageKeyByWireKey(row, table, keying) + for (const wireKey of Object.keys(row)) { + const selectionKey = tableRowSecretProvenanceSelectionKey(rowIndex, wireKey) + columnIdBySelectionKey.set(selectionKey, storageKeys.get(wireKey) ?? null) + rowKeyBySelectionKey.set(selectionKey, rowIndex) + } + }) + + // A complete bundle must account for every cell the write touched, and only + // those — otherwise a caller could certify a column it never wrote. + if ( + bundle.complete && + (bundle.selections.length !== columnIdBySelectionKey.size || + bundle.selections.some((selection) => !columnIdBySelectionKey.has(selection.key))) + ) { + throw new TableRowProvenanceError() + } + + if (!bundle.complete) { + return { stamps: wireRows.map(() => ({ complete: false, columns: {} })) } + } + + const stamps: TableRowSecretProvenanceWrite[] = wireRows.map(() => ({ + complete: true, + columns: {}, + })) + const subjectUserId = requirePrincipalSubjectUserId(principal) + for (const selection of bundle.selections) { + const rowIndex = rowKeyBySelectionKey.get(selection.key) + if ( + rowIndex === undefined || + !isPrivateSecretProvenanceScopeCompatible(selection.provenance.scope, { + userId: subjectUserId, + workspaceId: options.workspaceId, + }) + ) { + throw new TableRowProvenanceError() + } + const columnId = columnIdBySelectionKey.get(selection.key) + if (columnId === null || columnId === undefined) continue + if (Object.hasOwn(stamps[rowIndex].columns, columnId)) { + throw new TableRowProvenanceError() + } + stamps[rowIndex].columns[columnId] = selection.provenance + } + return { stamps } +} diff --git a/apps/sim/lib/table/application/rows.test.ts b/apps/sim/lib/table/application/rows.test.ts index 23d52dcb87c..2bfbeb337b9 100644 --- a/apps/sim/lib/table/application/rows.test.ts +++ b/apps/sim/lib/table/application/rows.test.ts @@ -19,6 +19,7 @@ const { mockResolveContext, mockResolvePermission, mockSignalRowsChanged, + mockSignalRowsChangedByActor, mockUpsertRow, mockWithLockedTable, mockInsertRow, @@ -41,6 +42,7 @@ const { mockResolveContext: vi.fn(), mockResolvePermission: vi.fn(), mockSignalRowsChanged: vi.fn(), + mockSignalRowsChangedByActor: vi.fn(), mockUpsertRow: vi.fn(), mockWithLockedTable: vi.fn(), mockInsertRow: vi.fn(), @@ -137,6 +139,7 @@ vi.mock('@/lib/table/application/context', () => ({ vi.mock('@/lib/table/events', () => ({ signalTableRowsChanged: mockSignalRowsChanged, + signalTableRowsChangedByActor: mockSignalRowsChangedByActor, })) import { diff --git a/apps/sim/lib/table/application/rows.ts b/apps/sim/lib/table/application/rows.ts index 2f00f7b9696..751e2aaae07 100644 --- a/apps/sim/lib/table/application/rows.ts +++ b/apps/sim/lib/table/application/rows.ts @@ -42,11 +42,15 @@ import { import { defineAuthorizedTableUseCase } from '@/lib/table/application/authorized-table-use-case' import { resolveActiveTableContext } from '@/lib/table/application/context' import { tableOperations } from '@/lib/table/application/operations' +import { + resolveRowWriteProvenance, + type TableRowProvenanceEnvelope, +} from '@/lib/table/application/row-secret-provenance' import { assertRowCapacity, notifyTableRowUsage } from '@/lib/table/billing' import { buildColumnNameById, buildIdByName, unknownColumnNames } from '@/lib/table/column-keys' import { columnTypeOf } from '@/lib/table/column-types' import { TableQueryValidationError } from '@/lib/table/errors' -import { signalTableRowsChanged } from '@/lib/table/events' +import { signalTableRowsChanged, signalTableRowsChangedByActor } from '@/lib/table/events' import { predicateToFilter } from '@/lib/table/query-builder/converters' import { validatePredicate, @@ -495,6 +499,15 @@ interface CreateSingleTableRowInput extends TableScopedInput { /** See {@link TableRowDataKeying}. Required so a new write surface must choose. */ dataKeying: TableRowDataKeying kind: 'single' + /** + * Tab that caused this write, when the calling surface knows it. Lets that tab + * skip refetching its own write — see {@link signalTableRowsChangedByActor}, + * whose soundness condition is that the caller's hook reconciles the write + * locally across every cached rows query. Only the single-row paths accept + * one: a batch or filter-scoped write genuinely needs the acting tab to + * refetch. Absent by default, which broadcasts to every subscriber as before. + */ + actorClientId?: string data: RowData position?: number afterRowId?: string @@ -597,9 +610,16 @@ export const createTableRows = defineAuthorizedTableUseCase({ ) return { kind: 'batch', table: context.table, rows: created } }, - afterSuccess: ({ context, result }) => { - const affected = result.kind === 'single' ? 1 : result.rows.length - if (affected > 0) signalTableRowsChanged(context.tableId) + afterSuccess: ({ context, input, result }) => { + // Narrowed on the input, not the result: only the single-row variant carries + // an actor, and the two discriminants always agree. + if (input.kind === 'single') { + signalTableRowsChangedByActor(context.tableId, input.actorClientId) + return + } + // A batch insert is not reconciled locally by the acting tab, so it must + // refetch like every other subscriber. + if (result.kind === 'batch' && result.rows.length > 0) signalTableRowsChanged(context.tableId) }, }) @@ -834,7 +854,23 @@ export interface UpdateTableRowInput extends TableScopedInput { rowId: string data: RowData secretProvenance?: TableRowSecretProvenanceWrite + /** + * Private provenance envelope as it arrived on the wire, resolved here against + * the canonical schema. Mutually exclusive with {@link secretProvenance}: a + * surface either resolves its own stamp or hands over the envelope for this + * use case to resolve, never both. + */ + secretProvenanceEnvelope?: TableRowProvenanceEnvelope includePersistedSecretProvenance?: boolean + /** + * Tab that caused this write, when the calling surface knows it. Lets that tab + * skip refetching its own write — see {@link signalTableRowsChangedByActor}, + * whose soundness condition is that the caller's hook reconciles the write + * locally across every cached rows query. Only the single-row paths accept + * one: a batch or filter-scoped write genuinely needs the acting tab to + * refetch. Absent by default, which broadcasts to every subscriber as before. + */ + actorClientId?: string } export interface UpdateTableRowResult extends TableResult { @@ -848,6 +884,17 @@ export const updateTableRow = defineAuthorizedTableUseCase({ resolveContext: ({ input }: { input: UpdateTableRowInput }) => resolveActiveTableContext(input), async execute({ principal, input, context }): Promise { const data = rowDataToStorage(input.data, context.table, input.dataKeying, input.strictWrite) + const secretProvenance = input.secretProvenanceEnvelope + ? resolveRowWriteProvenance({ + envelope: input.secretProvenanceEnvelope, + principal, + workspaceId: context.workspaceId, + table: context.table, + keying: input.dataKeying, + wireRows: [input.data], + storageRows: [data], + }).stamps[0] + : defaultedRowSecretProvenance(data, input.secretProvenance) const row = await updateRow( { tableId: context.tableId, @@ -855,7 +902,7 @@ export const updateTableRow = defineAuthorizedTableUseCase({ rowId: input.rowId, data, actorUserId: actorUserId(principal, context.billedAccountUserId), - secretProvenance: defaultedRowSecretProvenance(data, input.secretProvenance), + secretProvenance, }, context.table, requestId(input), @@ -874,8 +921,8 @@ export const updateTableRow = defineAuthorizedTableUseCase({ ), } }, - afterSuccess: ({ context, result }) => { - if (result.changed) signalTableRowsChanged(context.tableId) + afterSuccess: ({ context, input, result }) => { + if (result.changed) signalTableRowsChangedByActor(context.tableId, input.actorClientId) }, }) @@ -925,6 +972,15 @@ export const updateTableRows = defineAuthorizedTableUseCase({ export interface DeleteTableRowInput extends TableScopedInput { rowId: string + /** + * Tab that caused this write, when the calling surface knows it. Lets that tab + * skip refetching its own write — see {@link signalTableRowsChangedByActor}, + * whose soundness condition is that the caller's hook reconciles the write + * locally across every cached rows query. Only the single-row paths accept + * one: a batch or filter-scoped write genuinely needs the acting tab to + * refetch. Absent by default, which broadcasts to every subscriber as before. + */ + actorClientId?: string } export interface DeleteTableRowResult extends TableResult { @@ -938,7 +994,8 @@ export const deleteTableRow = defineAuthorizedTableUseCase({ await deleteRow(context.table, input.rowId, requestId(input)) return { table: context.table, deletedRowId: input.rowId } }, - afterSuccess: ({ context }) => signalTableRowsChanged(context.tableId), + afterSuccess: ({ context, input }) => + signalTableRowsChangedByActor(context.tableId, input.actorClientId), }) export type DeleteTableRowsInput = TableScopedInput & diff --git a/apps/sim/lib/table/events.attribution.test.ts b/apps/sim/lib/table/events.attribution.test.ts index e0f8c9ee448..351aa715f5e 100644 --- a/apps/sim/lib/table/events.attribution.test.ts +++ b/apps/sim/lib/table/events.attribution.test.ts @@ -9,16 +9,25 @@ import { describe, expect, it } from 'vitest' * `signalTableRowsChangedByActor` lets the acting tab skip its own refetch, which is only sound * where that tab's mutation hook already applies the server's answer to every cached rows query. * That invariant lives in `hooks/queries/tables.ts` — nothing in the type system ties it to the - * call site, so a well-meaning fourth call would silently strand that client on stale rows. + * call site, so a well-meaning extra call would silently strand that client on stale rows. * - * This pins the allowlist. If you are here because it failed: adding a call means proving the - * calling route's client hook reconciles locally, then adding it below. Removing one is always safe. + * Two lists are pinned, because a migrated single-row route now signals from inside its + * application use case rather than from the route. The call itself is no longer the decision: + * that use case is shared with `/api/v2` and Copilot, and it degrades to a broadcast whenever no + * actor is named. What actually selects the behavior is which surface supplies `actorClientId`, + * so that is pinned too and is the list to scrutinise. + * + * If you are here because it failed: adding a supplier means proving that surface's client hook + * reconciles the write locally across every cached rows query. Removing one is always safe. */ const ATTRIBUTED_CALL_SITES = [ 'app/api/table/[tableId]/rows/route.ts', - 'app/api/table/[tableId]/rows/[rowId]/route.ts', + 'lib/table/application/rows.ts', ] as const +/** Surfaces that name the acting tab. See the note above — this is the real allowlist. */ +const ACTOR_SUPPLYING_SURFACES = ['app/api/table/[tableId]/rows/[rowId]/route.ts'] as const + const APP_ROOT = join(import.meta.dirname, '../..') /** Declares the function; matching its own definition would say nothing about call sites. */ const DECLARING_MODULE = 'lib/table/events.ts' @@ -32,17 +41,37 @@ async function* walk(dir: string): AsyncGenerator { } } +async function filesContaining(needle: string, skip: (relative: string) => boolean = () => false) { + const found: string[] = [] + for await (const file of walk(APP_ROOT)) { + const source = await readFile(file, 'utf8') + if (!source.includes(needle)) continue + const relative = file.slice(APP_ROOT.length + 1) + if (skip(relative)) continue + found.push(relative) + } + return found.sort() +} + describe('signalTableRowsChangedByActor call sites', () => { it('is called only where the acting tab reconciles the write locally', async () => { - const callers: string[] = [] - for await (const file of walk(APP_ROOT)) { - const source = await readFile(file, 'utf8') - if (!source.includes('signalTableRowsChangedByActor(')) continue - const relative = file.slice(APP_ROOT.length + 1) - if (relative === DECLARING_MODULE) continue - callers.push(relative) - } - - expect(callers.sort()).toEqual([...ATTRIBUTED_CALL_SITES].sort()) + const callers = await filesContaining( + 'signalTableRowsChangedByActor(', + (relative) => relative === DECLARING_MODULE + ) + + expect(callers).toEqual([...ATTRIBUTED_CALL_SITES].sort()) + }) + + it('is given an actor only by surfaces whose client hook reconciles locally', async () => { + const suppliers = await filesContaining( + 'actorClientId:', + // These declare or forward the field rather than naming a tab. + (relative) => + relative === 'lib/table/application/rows.ts' || + relative === 'lib/table/application/row-secret-provenance.ts' + ) + + expect(suppliers).toEqual([...ACTOR_SUPPLYING_SURFACES].sort()) }) }) From ff80f2c970b398e3e4d145f46c83bcf8ade7d99c Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 17 Aug 2026 21:54:09 -0700 Subject: [PATCH 2/7] refactor(table): move the upsert route onto the application boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same shape as the single-row route: declares itself against upsertTableRow, hands the provenance envelope over unresolved, and derives its column keying from the principal rather than assuming one. The keying and presentation helpers the two routes shared are now in row-wire beside the translators they wrap, so a third route does not restate them. Two response details changed on purpose. The row now carries `position`, which the use case always had and this route alone omitted — every other single-row response already returned it, and the contract now describes one shape instead of three. The upsert result also carries read-back provenance, which the route previously assembled for itself. The surface had no route-level tests; it has six now, covering both caller keyings, both operations, and the envelope handover. --- .../api/table/[tableId]/rows/[rowId]/route.ts | 57 ++----- .../table/[tableId]/rows/upsert/route.test.ts | 144 ++++++++++++++++ .../api/table/[tableId]/rows/upsert/route.ts | 160 ++++++------------ apps/sim/app/api/table/row-wire.ts | 48 +++++- apps/sim/lib/api/contracts/tables.ts | 2 +- apps/sim/lib/table/application/rows.ts | 29 +++- 6 files changed, 287 insertions(+), 153 deletions(-) create mode 100644 apps/sim/app/api/table/[tableId]/rows/upsert/route.test.ts diff --git a/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts b/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts index 63780882463..5b623b4155e 100644 --- a/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts +++ b/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts @@ -1,4 +1,3 @@ -import type { Principal } from '@sim/auth/principal' import { readClientId } from '@/lib/api/client-id' import { deleteTableRowContract, @@ -6,50 +5,23 @@ import { updateTableRowContract, } from '@/lib/api/contracts/tables' import { defineInternalJsonRoute, internalRateLimits } from '@/lib/api/server/routes' -import { AuthType, type AuthTypeValue } from '@/lib/auth/hybrid' import { internalTableRowsErrorPolicy, internalTableSessionOrExecutorAuth } from '@/lib/table/api' import { tableOperations } from '@/lib/table/application/operations' -import type { TableRowDataKeying } from '@/lib/table/application/rows' import { deleteTableRow, readTableRow, updateTableRow } from '@/lib/table/application/rows' -import type { RowData, TableDefinition, TableRow } from '@/lib/table/types' +import type { RowData } from '@/lib/table/types' import { finalizeTableRowsProvenance, negotiateTableRowsProvenance, readTableRowProvenanceEnvelope, } from '@/app/api/table/row-secret-provenance' -import { rowWireTranslators } from '@/app/api/table/row-wire' +import { + authTypeForPrincipal, + presentRowForPrincipal, + rowKeyingForPrincipal, +} from '@/app/api/table/row-wire' export const dynamic = 'force-dynamic' -/** - * One path, two caller kinds, two column keyings. - * - * The first-party grid holds the schema it rendered and addresses cells by - * stable column id; a workflow tool execution speaks column names, because names - * are what tool enrichment surfaces to the model. The keying is a property of - * the caller rather than of the endpoint, which is why it is derived from the - * principal here and passed to the use case rather than assumed by it. - */ -function authTypeFor(principal: Principal): AuthTypeValue { - return principal.kind === 'session' ? AuthType.SESSION : AuthType.INTERNAL_JWT -} - -function keyingFor(authType: AuthTypeValue): TableRowDataKeying { - return authType === AuthType.INTERNAL_JWT ? 'names' : 'ids' -} - -/** The narrower projection these routes have always returned. */ -function presentRow(row: TableRow, table: TableDefinition, principal: Principal) { - const wire = rowWireTranslators(authTypeFor(principal), table.schema) - return { - id: row.id, - data: wire.dataOut(row.data), - position: row.position, - createdAt: row.createdAt instanceof Date ? row.createdAt.toISOString() : String(row.createdAt), - updatedAt: row.updatedAt instanceof Date ? row.updatedAt.toISOString() : String(row.updatedAt), - } -} - const rateLimit = internalRateLimits.none({ reason: 'Preserve existing internal single-row table behavior', }) @@ -64,12 +36,15 @@ export const GET = defineInternalJsonRoute({ tableId: params.tableId, rowId: params.rowId, assertedWorkspaceId: query.workspaceId, - includePersistedSecretProvenance: negotiateTableRowsProvenance(request, authTypeFor(principal)), + includePersistedSecretProvenance: negotiateTableRowsProvenance( + request, + authTypeForPrincipal(principal) + ), }), useCase: readTableRow, present: ({ table, row }, { principal }) => ({ success: true as const, - data: { row: presentRow(row, table, principal) }, + data: { row: presentRowForPrincipal(row, table.schema, principal) }, }), finalizeResponse: ({ result }) => finalizeTableRowsProvenance(result.secretProvenance), }) @@ -81,18 +56,20 @@ export const PATCH = defineInternalJsonRoute({ rateLimit, errorPolicy: internalTableRowsErrorPolicy, mapInput: ({ params, body }, { principal, request }) => { - const authType = authTypeFor(principal) return { tableId: params.tableId, rowId: params.rowId, assertedWorkspaceId: body.workspaceId, data: body.data as RowData, - dataKeying: keyingFor(authType), + dataKeying: rowKeyingForPrincipal(principal), strictWrite: false, // Handed over unresolved: interpreting the selections needs the canonical // schema, which this adapter must not load. secretProvenanceEnvelope: readTableRowProvenanceEnvelope(request, body), - includePersistedSecretProvenance: negotiateTableRowsProvenance(request, authType), + includePersistedSecretProvenance: negotiateTableRowsProvenance( + request, + authTypeForPrincipal(principal) + ), actorClientId: readClientId(request), } }, @@ -100,7 +77,7 @@ export const PATCH = defineInternalJsonRoute({ present: ({ table, row }, { principal }) => ({ success: true as const, data: { - row: presentRow(row, table, principal), + row: presentRowForPrincipal(row, table.schema, principal), message: 'Row updated successfully', }, }), diff --git a/apps/sim/app/api/table/[tableId]/rows/upsert/route.test.ts b/apps/sim/app/api/table/[tableId]/rows/upsert/route.test.ts new file mode 100644 index 00000000000..1b0c13690a1 --- /dev/null +++ b/apps/sim/app/api/table/[tableId]/rows/upsert/route.test.ts @@ -0,0 +1,144 @@ +/** + * @vitest-environment node + * + * The upsert surface had no route-level tests. These pin what it emits now that + * it runs on the shared internal route builder, including the dual-caller wire + * keying and the provenance envelope being handed over unresolved. + */ +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mocks } = vi.hoisted(() => ({ + mocks: { upsertRow: vi.fn(), authenticate: vi.fn() }, +})) + +vi.mock('@/lib/table/application/rows', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + upsertTableRow: { operation: { id: 'tables.rows.upsert' }, execute: mocks.upsertRow }, + } +}) + +vi.mock('@/lib/table/api', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, internalTableSessionOrExecutorAuth: { authenticate: mocks.authenticate } } +}) + +import { InternalUnauthenticatedError } from '@/lib/api/server/routes' +import { POST } from '@/app/api/table/[tableId]/rows/upsert/route' + +const TABLE_ID = 'tbl_1' +const WORKSPACE_ID = 'workspace-1' +const CREATED_AT = new Date('2024-01-01T00:00:00.000Z') +const UPDATED_AT = new Date('2024-02-02T00:00:00.000Z') + +const TABLE = { + id: TABLE_ID, + workspaceId: WORKSPACE_ID, + schema: { columns: [{ id: 'col_aaa', name: 'Name', type: 'string' as const }] }, +} +const ROW = { + id: 'row_1', + data: { col_aaa: 'Ada' }, + executions: {}, + position: 0, + createdAt: CREATED_AT, + updatedAt: UPDATED_AT, +} + +function routeContext() { + return { params: Promise.resolve({ tableId: TABLE_ID }) } +} + +function request(body: unknown) { + return new NextRequest(`http://localhost/api/table/${TABLE_ID}/rows/upsert`, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify(body), + }) +} + +const BODY = { workspaceId: WORKSPACE_ID, data: { col_aaa: 'Ada' }, conflictTarget: 'col_aaa' } + +beforeEach(() => { + vi.clearAllMocks() + mocks.authenticate.mockResolvedValue({ + kind: 'session', + userId: 'user-1', + sessionId: 'session-1', + }) + mocks.upsertRow.mockResolvedValue({ table: TABLE, row: ROW, operation: 'insert' }) +}) + +describe('POST /api/table/[tableId]/rows/upsert', () => { + it('returns 401 when the caller is not authenticated', async () => { + mocks.authenticate.mockRejectedValue(new InternalUnauthenticatedError()) + + const response = await POST(request(BODY), routeContext()) + + expect(response.status).toBe(401) + expect(mocks.upsertRow).not.toHaveBeenCalled() + }) + + it('names the operation it performed in the body and the message', async () => { + const response = await POST(request(BODY), routeContext()) + + expect(response.status).toBe(200) + await expect(response.json()).resolves.toEqual({ + success: true, + data: { + row: { + id: 'row_1', + data: { col_aaa: 'Ada' }, + position: 0, + createdAt: CREATED_AT.toISOString(), + updatedAt: UPDATED_AT.toISOString(), + }, + operation: 'insert', + message: 'Row inserted successfully', + }, + }) + }) + + it('says updated when the row already existed', async () => { + mocks.upsertRow.mockResolvedValue({ table: TABLE, row: ROW, operation: 'update' }) + + const body = await (await POST(request(BODY), routeContext())).json() + + expect(body.data.message).toBe('Row updated successfully') + }) + + it('tells the use case a session speaks column ids and forwards the conflict target', async () => { + await POST(request(BODY), routeContext()) + + expect(mocks.upsertRow.mock.calls[0][0].input).toMatchObject({ + tableId: TABLE_ID, + assertedWorkspaceId: WORKSPACE_ID, + dataKeying: 'ids', + strictWrite: false, + conflictTarget: 'col_aaa', + }) + }) + + it('tells the use case a workflow execution speaks column names', async () => { + mocks.authenticate.mockResolvedValue({ + kind: 'workflow_execution_delegated', + userId: 'user-1', + workspaceId: WORKSPACE_ID, + executionId: 'exec-1', + }) + + await POST(request({ ...BODY, data: { Name: 'Ada' }, conflictTarget: 'Name' }), routeContext()) + + expect(mocks.upsertRow.mock.calls[0][0].input).toMatchObject({ dataKeying: 'names' }) + }) + + it('hands the provenance envelope over unresolved rather than interpreting it', async () => { + await POST(request(BODY), routeContext()) + + expect(mocks.upsertRow.mock.calls[0][0].input.secretProvenanceEnvelope).toEqual({ + kind: 'none', + }) + }) +}) diff --git a/apps/sim/app/api/table/[tableId]/rows/upsert/route.ts b/apps/sim/app/api/table/[tableId]/rows/upsert/route.ts index abfbc7e0384..34f147fdb39 100644 --- a/apps/sim/app/api/table/[tableId]/rows/upsert/route.ts +++ b/apps/sim/app/api/table/[tableId]/rows/upsert/route.ts @@ -1,114 +1,56 @@ -import { createLogger } from '@sim/logger' -import { type NextRequest, NextResponse } from 'next/server' import { upsertTableRowContract } from '@/lib/api/contracts/tables' -import { parseRequest } from '@/lib/api/server' -import { isZodError, validationErrorResponse } from '@/lib/api/server/validation' -import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' -import { generateRequestId } from '@/lib/core/utils/request' -import { withRouteHandler } from '@/lib/core/utils/with-route-handler' -import type { RowData, TableSchema } from '@/lib/table' -import { upsertRow } from '@/lib/table' -import { signalTableRowsChanged } from '@/lib/table/events' +import { defineInternalJsonRoute, internalRateLimits } from '@/lib/api/server/routes' +import { internalTableRowsErrorPolicy, internalTableSessionOrExecutorAuth } from '@/lib/table/api' +import { tableOperations } from '@/lib/table/application/operations' +import { upsertTableRow } from '@/lib/table/application/rows' +import type { RowData } from '@/lib/table/types' import { - createTableRowsResponse, - createTableWriteProvenanceTargets, - resolveTableWriteSecretProvenance, + finalizeTableRowsProvenance, + negotiateTableRowsProvenance, + readTableRowProvenanceEnvelope, } from '@/app/api/table/row-secret-provenance' -import { rowWireTranslators } from '@/app/api/table/row-wire' -import { accessError, checkAccess, orchestrationErrorResponse } from '@/app/api/table/utils' - -const logger = createLogger('TableUpsertAPI') - -interface UpsertRouteParams { - params: Promise<{ tableId: string }> -} - -/** POST /api/table/[tableId]/rows/upsert - Inserts or updates based on unique columns. */ -export const POST = withRouteHandler(async (request: NextRequest, context: UpsertRouteParams) => { - const requestId = generateRequestId() - const { tableId } = await context.params - - try { - const authResult = await checkSessionOrInternalAuth(request, { requireWorkflowId: false }) - if (!authResult.success || !authResult.userId) { - return NextResponse.json({ error: 'Authentication required' }, { status: 401 }) - } - - const validation = await parseRequest(upsertTableRowContract, request, context) - if (!validation.success) return validation.response - const validated = validation.data.body - - const result = await checkAccess(tableId, authResult.userId, 'write') - if (!result.ok) return accessError(result, requestId, tableId) - - const { table } = result - - if (table.workspaceId !== validated.workspaceId) { - return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 }) - } - - const wire = rowWireTranslators(authResult.authType, table.schema as TableSchema) - const provenance = resolveTableWriteSecretProvenance({ - request, - payload: validated, - authType: authResult.authType, - userId: authResult.userId, - workspaceId: table.workspaceId, - targets: createTableWriteProvenanceTargets([validated.data as RowData], wire.dataIn), - rowKeys: ['0'], - }) - if (!provenance.success) return provenance.response - // conflictTarget passes through untranslated — upsertRow resolves it id-or-name. - const upsertResult = await upsertRow( - { - tableId, - workspaceId: validated.workspaceId, - data: wire.dataIn(validated.data as RowData), - userId: authResult.userId, - conflictTarget: validated.conflictTarget, - secretProvenance: provenance.provenanceByRowKey?.['0'], - }, - table, - requestId - ) - signalTableRowsChanged(tableId) - - const responseBody = { - success: true, - data: { - row: { - id: upsertResult.row.id, - data: wire.dataOut(upsertResult.row.data), - createdAt: - upsertResult.row.createdAt instanceof Date - ? upsertResult.row.createdAt.toISOString() - : upsertResult.row.createdAt, - updatedAt: - upsertResult.row.updatedAt instanceof Date - ? upsertResult.row.updatedAt.toISOString() - : upsertResult.row.updatedAt, - }, - operation: upsertResult.operation, - message: `Row ${upsertResult.operation === 'update' ? 'updated' : 'inserted'} successfully`, - }, - } - return createTableRowsResponse({ +import { + authTypeForPrincipal, + presentRowForPrincipal, + rowKeyingForPrincipal, +} from '@/app/api/table/row-wire' + +export const dynamic = 'force-dynamic' + +/** POST /api/table/[tableId]/rows/upsert — inserts or updates based on unique columns. */ +export const POST = defineInternalJsonRoute({ + contract: upsertTableRowContract, + operation: tableOperations.upsertRow, + auth: internalTableSessionOrExecutorAuth, + rateLimit: internalRateLimits.none({ + reason: 'Preserve existing internal table upsert behavior', + }), + errorPolicy: internalTableRowsErrorPolicy, + mapInput: ({ params, body }, { principal, request }) => ({ + tableId: params.tableId, + assertedWorkspaceId: body.workspaceId, + data: body.data as RowData, + dataKeying: rowKeyingForPrincipal(principal), + strictWrite: false, + // The conflict target follows the same keying as the data; the use case + // resolves it id-or-name against the canonical schema. + conflictTarget: body.conflictTarget, + // Handed over unresolved: interpreting the selections needs the canonical + // schema, which this adapter must not load. + secretProvenanceEnvelope: readTableRowProvenanceEnvelope(request, body), + includePersistedSecretProvenance: negotiateTableRowsProvenance( request, - authType: authResult.authType, - userId: authResult.userId, - workspaceId: table.workspaceId, - body: responseBody, - rows: [upsertResult.row], - }) - } catch (error) { - if (isZodError(error)) { - return validationErrorResponse(error) - } - - const response = orchestrationErrorResponse(error) - if (response) return response - - logger.error(`[${requestId}] Error upserting row:`, error) - return NextResponse.json({ error: 'Failed to upsert row' }, { status: 500 }) - } + authTypeForPrincipal(principal) + ), + }), + useCase: upsertTableRow, + present: ({ table, row, operation }, { principal }) => ({ + success: true as const, + data: { + row: presentRowForPrincipal(row, table.schema, principal), + operation, + message: `Row ${operation === 'update' ? 'updated' : 'inserted'} successfully`, + }, + }), + finalizeResponse: ({ result }) => finalizeTableRowsProvenance(result.secretProvenance), }) diff --git a/apps/sim/app/api/table/row-wire.ts b/apps/sim/app/api/table/row-wire.ts index f4c0a85d988..3c5c948e693 100644 --- a/apps/sim/app/api/table/row-wire.ts +++ b/apps/sim/app/api/table/row-wire.ts @@ -1,5 +1,15 @@ +import type { Principal } from '@sim/auth/principal' import { AuthType, type AuthTypeValue } from '@/lib/auth/hybrid' -import type { Filter, RowData, Sort, SortSpec, TablePredicate, TableSchema } from '@/lib/table' +import type { + Filter, + RowData, + Sort, + SortSpec, + TablePredicate, + TableRow, + TableSchema, +} from '@/lib/table' +import type { TableRowDataKeying } from '@/lib/table/application/rows' import { namedRowMapper } from '@/lib/table/cell-format' import { buildIdByName, @@ -61,3 +71,39 @@ export function rowWireTranslators( sortSpecIn: (sort) => sortSpecNamesToIds(sort, idByName), } } + +/** + * The internal table routes serve two caller kinds on the same paths, and they + * speak different column keyings: the first-party grid holds the schema it + * rendered and addresses cells by stable id, while a workflow tool execution + * speaks column names, because names are what tool enrichment surfaces to the + * model. Keying is therefore a property of the caller, not of the endpoint. + */ +export function authTypeForPrincipal(principal: Principal): AuthTypeValue { + return principal.kind === 'session' ? AuthType.SESSION : AuthType.INTERNAL_JWT +} + +/** See {@link authTypeForPrincipal}. Feeds the use case's `dataKeying`. */ +export function rowKeyingForPrincipal(principal: Principal): TableRowDataKeying { + return principal.kind === 'session' ? 'ids' : 'names' +} + +/** + * One row in the narrower projection the single-row and upsert routes return: + * the stored cells in the caller's keying, plus position, with timestamps + * already serialized. See `tableRowWireSchema`, which is its contract. + */ +export function presentRowForPrincipal( + row: Pick, + schema: TableSchema, + principal: Principal +) { + const wire = rowWireTranslators(authTypeForPrincipal(principal), schema) + return { + id: row.id, + data: wire.dataOut(row.data), + position: row.position, + createdAt: row.createdAt instanceof Date ? row.createdAt.toISOString() : String(row.createdAt), + updatedAt: row.updatedAt instanceof Date ? row.updatedAt.toISOString() : String(row.updatedAt), + } +} diff --git a/apps/sim/lib/api/contracts/tables.ts b/apps/sim/lib/api/contracts/tables.ts index 9550d386b7a..e5eeefac94e 100644 --- a/apps/sim/lib/api/contracts/tables.ts +++ b/apps/sim/lib/api/contracts/tables.ts @@ -1444,7 +1444,7 @@ export const upsertTableRowContract = defineRouteContract({ mode: 'json', schema: successResponseSchema( z.object({ - row: tableRowSchema, + row: tableRowWireSchema, operation: z.enum(['insert', 'update']), message: z.string(), }) diff --git a/apps/sim/lib/table/application/rows.ts b/apps/sim/lib/table/application/rows.ts index 751e2aaae07..adb9f59741a 100644 --- a/apps/sim/lib/table/application/rows.ts +++ b/apps/sim/lib/table/application/rows.ts @@ -1071,11 +1071,15 @@ export interface UpsertTableRowInput extends TableScopedInput { data: RowData conflictTarget?: string secretProvenance?: TableRowSecretProvenanceWrite + /** See {@link UpdateTableRowInput.secretProvenanceEnvelope}. */ + secretProvenanceEnvelope?: TableRowProvenanceEnvelope + includePersistedSecretProvenance?: boolean } export interface UpsertTableRowResult extends TableResult { row: TableRow operation: 'insert' | 'update' + secretProvenance?: TableRowsProvenance } export const upsertTableRow = defineAuthorizedTableUseCase({ @@ -1089,6 +1093,17 @@ export const upsertTableRow = defineAuthorizedTableUseCase({ ? (buildIdByName(context.table.schema).get(input.conflictTarget) ?? input.conflictTarget) : input.conflictTarget const data = rowDataToStorage(input.data, context.table, input.dataKeying, input.strictWrite) + const secretProvenance = input.secretProvenanceEnvelope + ? resolveRowWriteProvenance({ + envelope: input.secretProvenanceEnvelope, + principal, + workspaceId: context.workspaceId, + table: context.table, + keying: input.dataKeying, + wireRows: [input.data], + storageRows: [data], + }).stamps[0] + : defaultedRowSecretProvenance(data, input.secretProvenance) const result = await upsertRow( { tableId: context.tableId, @@ -1096,13 +1111,23 @@ export const upsertTableRow = defineAuthorizedTableUseCase({ data, conflictTarget, userId: actorUserId(principal, context.billedAccountUserId), - secretProvenance: defaultedRowSecretProvenance(data, input.secretProvenance), + secretProvenance, }, context.table, requestId(input), rowWriteOptions(input) ) - return { table: context.table, row: result.row, operation: result.operation } + return { + table: context.table, + row: result.row, + operation: result.operation, + secretProvenance: await loadAuthorizedRowsProvenance( + principal, + context.workspaceId, + [result.row], + input.includePersistedSecretProvenance + ), + } }, afterSuccess: ({ context }) => signalTableRowsChanged(context.tableId), }) From 1fa9afb7cbee704f6e48d3bbc4094010a773d28f Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 17 Aug 2026 21:57:07 -0700 Subject: [PATCH 3/7] refactor(table): move the enrichment-detail route onto the application boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The last internal adapter that queried the database for itself. It now runs through readTableRowEnrichmentDetail, a new use case that shares tableOperations.readRow — reading a cell's cascade breakdown is a projection of the same row under the same role, not a second semantic operation. Its tests move to the same seam and gain one the old suite could not express: a cross-tenant table now conceals rather than confirming it exists. --- .../enrichment/[groupId]/route.test.ts | 141 +++++++++--------- .../[rowId]/enrichment/[groupId]/route.ts | 66 +++----- apps/sim/lib/table/application/rows.ts | 32 ++++ 3 files changed, 126 insertions(+), 113 deletions(-) diff --git a/apps/sim/app/api/table/[tableId]/rows/[rowId]/enrichment/[groupId]/route.test.ts b/apps/sim/app/api/table/[tableId]/rows/[rowId]/enrichment/[groupId]/route.test.ts index cb59859af26..1ae9263975f 100644 --- a/apps/sim/app/api/table/[tableId]/rows/[rowId]/enrichment/[groupId]/route.test.ts +++ b/apps/sim/app/api/table/[tableId]/rows/[rowId]/enrichment/[groupId]/route.test.ts @@ -1,101 +1,102 @@ /** * @vitest-environment node + * + * The enrichment-detail surface after moving onto the shared internal route + * builder. It previously queried the database from the adapter; the assertions + * below are the same wire outcomes, now with the use case as the seam. */ -import { createTableDefinition, hybridAuthMockFns } from '@sim/testing' import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' -import type { EnrichmentRunDetail } from '@/lib/table' -const { mockCheckAccess, mockLoadEnrichmentDetail } = vi.hoisted(() => ({ - mockCheckAccess: vi.fn(), - mockLoadEnrichmentDetail: vi.fn(), +const { mocks } = vi.hoisted(() => ({ + mocks: { readDetail: vi.fn(), authenticate: vi.fn() }, })) -vi.mock('@/lib/table/rows/executions', () => ({ - loadEnrichmentDetail: mockLoadEnrichmentDetail, -})) -vi.mock('@/app/api/table/utils', async () => { - const { NextResponse } = await import('next/server') +vi.mock('@/lib/table/application/rows', async (importOriginal) => { + const actual = await importOriginal() return { - checkAccess: mockCheckAccess, - accessError: (result: { status: number }) => - NextResponse.json({ error: 'denied' }, { status: result.status }), + ...actual, + readTableRowEnrichmentDetail: { + operation: { id: 'tables.rows.read' }, + execute: mocks.readDetail, + }, } }) +vi.mock('@/lib/table/api', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, internalTableSessionOrExecutorAuth: { authenticate: mocks.authenticate } } +}) + +import { InternalUnauthenticatedError } from '@/lib/api/server/routes' +import { NoWorkspaceAccessError } from '@/lib/core/application' import { GET } from '@/app/api/table/[tableId]/rows/[rowId]/enrichment/[groupId]/route' -function makeRequest(tableId = 'tbl_1', rowId = 'row_1', groupId = 'grp_1') { - const req = new NextRequest( - `http://localhost:3000/api/table/${tableId}/rows/${rowId}/enrichment/${groupId}` - ) - return GET(req, { params: Promise.resolve({ tableId, rowId, groupId }) }) +const TABLE = { id: 'tbl_1', workspaceId: 'workspace-1', schema: { columns: [] } } +const DETAIL = { providers: [{ id: 'clearbit', status: 'hit' }], costUsd: 0.01 } + +function routeContext() { + return { + params: Promise.resolve({ tableId: 'tbl_1', rowId: 'row_1', groupId: 'grp_1' }), + } } -const detail: EnrichmentRunDetail = { - startedAt: '2026-06-18T00:00:00.000Z', - completedAt: '2026-06-18T00:00:01.000Z', - durationMs: 1000, - totalCost: 0.05, - matchedProvider: 'hunter', - aborted: false, - providers: [ - { - id: 'hunter', - label: 'Hunter', - toolId: 'hunter_find_email', - status: 'matched', - cost: 0.05, - durationMs: 1000, - error: null, - }, - ], +function request() { + return new NextRequest('http://localhost/api/table/tbl_1/rows/row_1/enrichment/grp_1', { + method: 'GET', + }) } +beforeEach(() => { + vi.clearAllMocks() + mocks.authenticate.mockResolvedValue({ + kind: 'session', + userId: 'user-1', + sessionId: 'session-1', + }) + mocks.readDetail.mockResolvedValue({ table: TABLE, detail: DETAIL }) +}) + describe('GET /api/table/[tableId]/rows/[rowId]/enrichment/[groupId]', () => { - beforeEach(() => { - vi.clearAllMocks() - hybridAuthMockFns.mockCheckSessionOrInternalAuth.mockResolvedValue({ - success: true, - userId: 'user-1', - authType: 'session', - }) - mockCheckAccess.mockResolvedValue({ ok: true, table: createTableDefinition({ rowCount: 1 }) }) + it('returns 401 when the caller is not authenticated', async () => { + mocks.authenticate.mockRejectedValue(new InternalUnauthenticatedError()) + + const response = await GET(request(), routeContext()) + + expect(response.status).toBe(401) + expect(mocks.readDetail).not.toHaveBeenCalled() }) it('returns the enrichment detail', async () => { - mockLoadEnrichmentDetail.mockResolvedValue(detail) - const res = await makeRequest() - expect(res.status).toBe(200) - const json = await res.json() - expect(json).toEqual({ success: true, data: { detail } }) - expect(mockLoadEnrichmentDetail).toHaveBeenCalledWith( - expect.anything(), - 'tbl_1', - 'row_1', - 'grp_1' - ) + const response = await GET(request(), routeContext()) + + expect(response.status).toBe(200) + await expect(response.json()).resolves.toEqual({ success: true, data: { detail: DETAIL } }) }) it('returns null when there is no recorded run', async () => { - mockLoadEnrichmentDetail.mockResolvedValue(null) - const res = await makeRequest() - expect(res.status).toBe(200) - const json = await res.json() - expect(json).toEqual({ success: true, data: { detail: null } }) + mocks.readDetail.mockResolvedValue({ table: TABLE, detail: null }) + + const body = await (await GET(request(), routeContext())).json() + + expect(body).toEqual({ success: true, data: { detail: null } }) }) - it('401s when unauthenticated', async () => { - hybridAuthMockFns.mockCheckSessionOrInternalAuth.mockResolvedValue({ success: false }) - const res = await makeRequest() - expect(res.status).toBe(401) - expect(mockLoadEnrichmentDetail).not.toHaveBeenCalled() + it('passes the row and group through to the use case', async () => { + await GET(request(), routeContext()) + + expect(mocks.readDetail.mock.calls[0][0].input).toMatchObject({ + tableId: 'tbl_1', + rowId: 'row_1', + groupId: 'grp_1', + }) }) - it('denies when access check fails', async () => { - mockCheckAccess.mockResolvedValue({ ok: false, status: 403 }) - const res = await makeRequest() - expect(res.status).toBe(403) - expect(mockLoadEnrichmentDetail).not.toHaveBeenCalled() + it('conceals a cross-tenant table rather than confirming it exists', async () => { + mocks.readDetail.mockRejectedValue(new NoWorkspaceAccessError()) + + const response = await GET(request(), routeContext()) + + expect(response.status).toBe(404) }) }) diff --git a/apps/sim/app/api/table/[tableId]/rows/[rowId]/enrichment/[groupId]/route.ts b/apps/sim/app/api/table/[tableId]/rows/[rowId]/enrichment/[groupId]/route.ts index 34a045f7677..be90d7a2fec 100644 --- a/apps/sim/app/api/table/[tableId]/rows/[rowId]/enrichment/[groupId]/route.ts +++ b/apps/sim/app/api/table/[tableId]/rows/[rowId]/enrichment/[groupId]/route.ts @@ -1,51 +1,31 @@ -import { db } from '@sim/db' -import { createLogger } from '@sim/logger' -import { type NextRequest, NextResponse } from 'next/server' import { getEnrichmentDetailContract } from '@/lib/api/contracts/tables' -import { parseRequest } from '@/lib/api/server' -import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' -import { generateRequestId } from '@/lib/core/utils/request' -import { withRouteHandler } from '@/lib/core/utils/with-route-handler' -import { loadEnrichmentDetail } from '@/lib/table/rows/executions' -import { accessError, checkAccess } from '@/app/api/table/utils' +import { defineInternalJsonRoute, internalRateLimits } from '@/lib/api/server/routes' +import { internalTableRowsErrorPolicy, internalTableSessionOrExecutorAuth } from '@/lib/table/api' +import { tableOperations } from '@/lib/table/application/operations' +import { readTableRowEnrichmentDetail } from '@/lib/table/application/rows' -const logger = createLogger('EnrichmentDetailAPI') - -interface RouteParams { - params: Promise<{ tableId: string; rowId: string; groupId: string }> -} +export const dynamic = 'force-dynamic' /** * GET /api/table/[tableId]/rows/[rowId]/enrichment/[groupId] * - * Returns the enrichment cascade breakdown (provider outcomes, cost, timing) - * for one enrichment cell. Read on demand by the enrichment details panel — - * this data is deliberately kept off the hot grid read. Returns `null` for - * cells with no recorded run or runs that predate the feature. + * The enrichment cascade breakdown — provider outcomes, cost, timing — for one + * enrichment cell. Read on demand by the details panel; this data is + * deliberately kept off the hot grid read. */ -export const GET = withRouteHandler(async (request: NextRequest, { params }: RouteParams) => { - const requestId = generateRequestId() - - const authResult = await checkSessionOrInternalAuth(request, { requireWorkflowId: false }) - if (!authResult.success || !authResult.userId) { - return NextResponse.json({ error: 'Authentication required' }, { status: 401 }) - } - - const parsed = await parseRequest(getEnrichmentDetailContract, request, { params }) - if (!parsed.success) return parsed.response - const { tableId, rowId, groupId } = parsed.data.params - - const result = await checkAccess(tableId, authResult.userId, 'read') - if (!result.ok) return accessError(result, requestId, tableId) - - const detail = await loadEnrichmentDetail(db, tableId, rowId, groupId) - - logger.info(`[${requestId}] Loaded enrichment detail`, { - tableId, - rowId, - groupId, - hasDetail: detail !== null, - }) - - return NextResponse.json({ success: true, data: { detail } }) +export const GET = defineInternalJsonRoute({ + contract: getEnrichmentDetailContract, + operation: tableOperations.readRow, + auth: internalTableSessionOrExecutorAuth, + rateLimit: internalRateLimits.none({ + reason: 'Preserve existing internal enrichment-detail behavior', + }), + errorPolicy: internalTableRowsErrorPolicy, + mapInput: ({ params }) => ({ + tableId: params.tableId, + rowId: params.rowId, + groupId: params.groupId, + }), + useCase: readTableRowEnrichmentDetail, + present: ({ detail }) => ({ success: true as const, data: { detail } }), }) diff --git a/apps/sim/lib/table/application/rows.ts b/apps/sim/lib/table/application/rows.ts index adb9f59741a..44012017bcc 100644 --- a/apps/sim/lib/table/application/rows.ts +++ b/apps/sim/lib/table/application/rows.ts @@ -1,6 +1,7 @@ import { isDeepStrictEqual } from 'node:util' import { AuditAction, AuditResourceType } from '@sim/audit' import { requirePrincipalSubjectUserId, resolvePrincipalAttribution } from '@sim/auth/principal' +import { db } from '@sim/db' import { getRequestContext } from '@sim/logger' import { generateId } from '@sim/utils/id' import { isPlainRecord } from '@sim/utils/object' @@ -59,6 +60,7 @@ import { validateStoragePredicate, } from '@/lib/table/query-builder/validate' import { assertCursorQueryBinding, decodeCursor } from '@/lib/table/rows/cursor' +import { loadEnrichmentDetail } from '@/lib/table/rows/executions' import { createExactEmptyTableRowSecretProvenance, createTableRowSecretProvenanceFromRegistry, @@ -493,6 +495,36 @@ export const readTableRow = defineAuthorizedTableUseCase({ }, }) +export interface ReadTableRowEnrichmentInput extends TableScopedInput { + rowId: string + groupId: string +} + +export interface ReadTableRowEnrichmentResult extends TableResult { + detail: Awaited> +} + +/** + * The enrichment cascade breakdown — provider outcomes, cost, timing — for one + * cell. Deliberately kept off the hot grid read and fetched on demand by the + * details panel; `null` for a cell with no recorded run, or a run predating the + * feature. + * + * Shares {@link tableOperations.readRow}: this is a projection of the same row, + * under the same role, so it is not a second semantic operation. + */ +export const readTableRowEnrichmentDetail = defineAuthorizedTableUseCase({ + operation: tableOperations.readRow, + resolveContext: ({ input }: { input: ReadTableRowEnrichmentInput }) => + resolveActiveTableContext(input), + async execute({ input, context }): Promise { + return { + table: context.table, + detail: await loadEnrichmentDetail(db, context.tableId, input.rowId, input.groupId), + } + }, +}) + interface CreateSingleTableRowInput extends TableScopedInput { /** See {@link rowWriteOptions}. Required so a new write surface must choose. */ strictWrite: boolean From 5915fe39b26c5a91af0f6519dd3baa30f78fff5c Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 17 Aug 2026 22:09:58 -0700 Subject: [PATCH 4/7] fix(table): mirror the storage rule when keying write provenance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit storageKeyByWireKey mapped an unrecognised id-keyed key to null, but rowDataToStorage persists that key when the caller is not writing strictly. A cell would have been written with no provenance recorded, under a stamp still marked complete — the same failure the bundle completeness check exists to prevent, arriving through the keying map instead of the selection set. The two wires genuinely differ and the code now says so: the name path drops an unrecognised key, so it maps to null; the id path stores what it is given, so every key it sends is a storage key. Unreachable today, since no delegated surface uses id keying and a session bundle is refused earlier. Fixed because the function's stated invariant — that it mirrors how the row data itself is normalized — was not true. Also corrects the delegated principal fixture in these tests, which used a kind that is not in the Principal union, so the subject-id branch was never actually exercised. It is now, and the scope check is asserted to receive the acting principal's own subject id. Verified to fail: restoring the schema-based lookup turns the covering test red. --- .../application/row-secret-provenance.test.ts | 49 +++++++++++++++++-- .../application/row-secret-provenance.ts | 18 +++---- 2 files changed, 55 insertions(+), 12 deletions(-) diff --git a/apps/sim/lib/table/application/row-secret-provenance.test.ts b/apps/sim/lib/table/application/row-secret-provenance.test.ts index bd8e2a09046..c43d7546dfe 100644 --- a/apps/sim/lib/table/application/row-secret-provenance.test.ts +++ b/apps/sim/lib/table/application/row-secret-provenance.test.ts @@ -39,10 +39,14 @@ const TABLE = { const SESSION = { kind: 'session' as const, userId: 'user-1', sessionId: 'session-1' } const EXECUTOR = { - kind: 'workflow_execution_delegated' as const, - userId: 'user-1', + kind: 'delegated' as const, + serviceId: 'executor' as const, + subjectUserId: 'user-1', workspaceId: 'workspace-1', - executionId: 'exec-1', + delegationId: 'delegation-1', + audience: 'table', + issuedAt: new Date('2026-01-01'), + expiresAt: new Date('2026-01-02'), } function resolve(overrides: Partial[0]>) { @@ -151,6 +155,45 @@ describe('row write provenance', () => { }) }) + it('checks the scope against the acting principal, not a billing owner', () => { + resolve({ + principal: EXECUTOR, + envelope: { + kind: 'bundle', + value: { + complete: true, + selections: [{ key: JSON.stringify([0, 'col_aaa']), provenance: { scope: {} } }], + }, + }, + }) + + expect(mocks.scopeCompatible).toHaveBeenCalledWith( + {}, + { userId: 'user-1', workspaceId: 'workspace-1' } + ) + }) + + it('records provenance for an id-keyed key the write persists, recognised or not', () => { + // The id wire stores what it is given, so every key it sends is a storage + // key. Mapping an unrecognised one to null would leave a written cell + // uncertified under a complete stamp. + const { stamps } = resolve({ + principal: EXECUTOR, + keying: 'ids', + wireRows: [{ 'col-unknown': 'x' }], + storageRows: [{ 'col-unknown': 'x' }], + envelope: { + kind: 'bundle', + value: { + complete: true, + selections: [{ key: JSON.stringify([0, 'col-unknown']), provenance: { scope: {} } }], + }, + }, + }) + + expect(stamps[0]).toEqual({ complete: true, columns: { 'col-unknown': { scope: {} } } }) + }) + it('records nothing for a key that names no column, since it is never stored', () => { const { stamps } = resolve({ principal: EXECUTOR, diff --git a/apps/sim/lib/table/application/row-secret-provenance.ts b/apps/sim/lib/table/application/row-secret-provenance.ts index 5174882c7c4..5f10c4b10a1 100644 --- a/apps/sim/lib/table/application/row-secret-provenance.ts +++ b/apps/sim/lib/table/application/row-secret-provenance.ts @@ -1,7 +1,7 @@ import { type Principal, requirePrincipalSubjectUserId } from '@sim/auth/principal' import { isPrivateSecretProvenanceScopeCompatible } from '@/lib/execution/durable-secret-provenance' import { isPrivateSecretProvenanceBundleV1 } from '@/lib/execution/model-input-provenance' -import { buildColumnNameById, buildIdByName } from '@/lib/table/column-keys' +import { buildIdByName } from '@/lib/table/column-keys' import { createExactEmptyTableRowSecretProvenance } from '@/lib/table/rows/secret-provenance' import { tableRowSecretProvenanceSelectionKey } from '@/lib/table/secret-provenance-selection' import type { RowData, TableDefinition, TableRowSecretProvenanceWrite } from '@/lib/table/types' @@ -27,8 +27,13 @@ export class TableRowProvenanceError extends Error { /** * Storage column id for each key the caller wrote, or `null` where the key names - * no column and is therefore never persisted. Mirrors how the row data itself is - * normalized, so a key dropped from the write is dropped from its provenance. + * no column and is therefore never persisted. + * + * This must mirror {@link rowDataToStorage} exactly, or a cell could be written + * with no provenance recorded under a `complete` stamp. The two wires differ in + * what they do with an unrecognised key: the name path drops it, so it gets + * `null`; the id path stores what it is given, so every key it sends is a + * storage key and none of them is `null`. */ function storageKeyByWireKey( row: RowData, @@ -36,12 +41,7 @@ function storageKeyByWireKey( keying: 'names' | 'ids' ): Map { const wireKeys = Object.keys(row) - if (keying === 'ids') { - // Keyed by `getColumnId`, so a legacy pre-backfill column — stored under its - // name because it has no id — is recognised rather than dropped. - const known = buildColumnNameById(table.schema.columns) - return new Map(wireKeys.map((key) => [key, known.has(key) ? key : null])) - } + if (keying === 'ids') return new Map(wireKeys.map((key) => [key, key])) const idByName = buildIdByName(table.schema) return new Map(wireKeys.map((key) => [key, idByName.get(key) ?? null])) } From ff6e61c873f731a9381da2f1366d251fc6b434bd Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 17 Aug 2026 22:16:57 -0700 Subject: [PATCH 5/7] fix(table): restore executor access to the migrated row routes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The migration swapped checkSessionOrInternalAuth for the delegation policy, and that broke every Table block call to these endpoints in two ways at once. The old policy accepted a legacy internal token. The new one requires a delegation token, which the executor only mints when the tool asks for it — and none of the four table row tools did, so get/update/delete/upsert row would each have failed with a 401. The knowledge tools already declare it, because their routes migrated first. Even with a valid token the operations denied the caller: readRow, updateRow, deleteRow and upsertRow ran under a policy whose delegatedServices is ['copilot'], so the executor got a 403. They now use the tool-facing policy that already existed for the group operations. Neither was visible to the route tests, which mock the auth policy wholesale — so the gap is closed at the layer that actually decides: one test pinning that each tool requests delegation and that its operation admits the executor, mutation-verified against both failure modes. Also fixes findings from the review pass: the read surfaces no longer load an executions sidecar none of them put on the wire (two readers rather than a flag, so a caller cannot silently read an empty one); the provenance name index is built once per batch instead of once per row; the uniqueness comment now names the concurrent-insert race as well as the retro-added constraint; and the presenter context gets NoInfer plus a note that the v2 builder passes something different. --- .../sim/app/api/table/table-tool-auth.test.ts | 45 +++++++++++ .../api/server/routes/internal-json-route.ts | 4 +- .../lib/table/__tests__/update-row.test.ts | 36 +++++++++ .../lib/table/application/operations.test.ts | 13 ++- apps/sim/lib/table/application/operations.ts | 17 +++- .../application/row-secret-provenance.ts | 16 ++-- apps/sim/lib/table/application/rows.ts | 12 ++- apps/sim/lib/table/rows/service.ts | 81 +++++++++++++------ apps/sim/tools/table/delete_row.ts | 1 + apps/sim/tools/table/get_row.ts | 1 + apps/sim/tools/table/update_row.ts | 1 + apps/sim/tools/table/upsert_row.ts | 1 + 12 files changed, 183 insertions(+), 45 deletions(-) create mode 100644 apps/sim/app/api/table/table-tool-auth.test.ts diff --git a/apps/sim/app/api/table/table-tool-auth.test.ts b/apps/sim/app/api/table/table-tool-auth.test.ts new file mode 100644 index 00000000000..9f2a6a3bede --- /dev/null +++ b/apps/sim/app/api/table/table-tool-auth.test.ts @@ -0,0 +1,45 @@ +/** + * @vitest-environment node + * + * The executor reaches the internal table row routes through the Table block's + * tools, and those routes now authenticate with the delegation policy rather + * than the legacy internal token. Two things have to line up for that to work, + * and neither is visible to a route test that mocks the auth policy: + * + * 1. the tool must ask the executor to mint a delegation token, and + * 2. the operation's policy must admit the `executor` delegated service. + * + * Both are pinned here because getting either wrong fails every workflow call + * to these endpoints — the first as a 401, the second as a 403 — while every + * route-level test keeps passing. + */ +import { describe, expect, it } from 'vitest' +import { tableOperations } from '@/lib/table/application/operations' +import { tableDeleteRowTool } from '@/tools/table/delete_row' +import { tableGetRowTool } from '@/tools/table/get_row' +import { tableUpdateRowTool } from '@/tools/table/update_row' +import { tableUpsertRowTool } from '@/tools/table/upsert_row' + +/** Tool → the operation its route runs under. */ +const EXECUTOR_ROW_TOOLS = [ + ['table_get_row', tableGetRowTool, tableOperations.readRow], + ['table_update_row', tableUpdateRowTool, tableOperations.updateRow], + ['table_delete_row', tableDeleteRowTool, tableOperations.deleteRow], + ['table_upsert_row', tableUpsertRowTool, tableOperations.upsertRow], +] as const + +describe('executor access to the migrated table row routes', () => { + it.each(EXECUTOR_ROW_TOOLS)('%s asks the executor for a delegation token', (_name, tool) => { + // Without this the executor mints a legacy internal token, which the + // delegation policy rejects outright. + expect(tool.request.internalAuth).toBe('executor_delegation') + }) + + it.each(EXECUTOR_ROW_TOOLS)( + '%s runs under an operation that admits the executor', + (_name, _tool, operation) => { + expect(operation.delegatedServices).toContain('executor') + expect(operation.principalKinds).toContain('delegated') + } + ) +}) diff --git a/apps/sim/lib/api/server/routes/internal-json-route.ts b/apps/sim/lib/api/server/routes/internal-json-route.ts index c9898c2f291..69b596c955f 100644 --- a/apps/sim/lib/api/server/routes/internal-json-route.ts +++ b/apps/sim/lib/api/server/routes/internal-json-route.ts @@ -221,13 +221,13 @@ type InternalJsonPresenter, - context: InternalJsonPresenterContext, P> + context: InternalJsonPresenterContext, NoInfer

> ): ContractJsonResponse | Promise> } : { present( result: NoInfer, - context: InternalJsonPresenterContext, P> + context: InternalJsonPresenterContext, NoInfer

> ): ContractJsonResponse | Promise> } diff --git a/apps/sim/lib/table/__tests__/update-row.test.ts b/apps/sim/lib/table/__tests__/update-row.test.ts index 46777d4b185..f2027de5445 100644 --- a/apps/sim/lib/table/__tests__/update-row.test.ts +++ b/apps/sim/lib/table/__tests__/update-row.test.ts @@ -8,6 +8,8 @@ import { deleteColumn, renameColumn } from '@/lib/table/columns/service' import { batchInsertRows, batchUpdateRows, + getRowById, + getRowSummaryById, insertRow, replaceTableRows, updateRow, @@ -658,3 +660,37 @@ describe('updateRow — uniqueness probe scoping', () => { expect(checkUniqueConstraintsDb).not.toHaveBeenCalled() }) }) + +/** + * The read surfaces never put the executions sidecar on the wire, so loading it + * for them is a query whose result is discarded. Two readers rather than a flag: + * a caller that forgets a flag reads an empty sidecar and cannot tell that from + * a row that has none, whereas here the field is not on the type. + */ +describe('row readers', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + dbChainMockFns.limit.mockResolvedValue([EXISTING_ROW]) + }) + + it('getRowSummaryById issues one select and returns no sidecar', async () => { + const row = await getRowSummaryById('tbl-1', 'row-1', 'ws-1') + + expect(row).not.toHaveProperty('executions') + expect(dbChainMockFns.select).toHaveBeenCalledTimes(1) + }) + + it('getRowById issues the extra select the sidecar needs', async () => { + const row = await getRowById('tbl-1', 'row-1', 'ws-1') + + expect(row).toHaveProperty('executions') + expect(dbChainMockFns.select).toHaveBeenCalledTimes(2) + }) + + it('getRowSummaryById returns null for a missing row', async () => { + dbChainMockFns.limit.mockResolvedValue([]) + + await expect(getRowSummaryById('tbl-1', 'nope', 'ws-1')).resolves.toBeNull() + }) +}) diff --git a/apps/sim/lib/table/application/operations.test.ts b/apps/sim/lib/table/application/operations.test.ts index 3b9dc95cc27..269719d8655 100644 --- a/apps/sim/lib/table/application/operations.test.ts +++ b/apps/sim/lib/table/application/operations.test.ts @@ -65,17 +65,26 @@ describe('table operation registry', () => { tableOperations.cancelExport.id, tableOperations.downloadExport.id, ]) - const sharedGroupOperations = new Set([ + // Reachable from the executor's Table block as well as from Copilot. The + // single-row operations joined this set when their routes moved onto the + // delegation auth policy: the Table block's get/update/delete/upsert row + // tools run under them, and a policy without `executor` fails every one of + // those calls with a 403 while every route test still passes. + const sharedToolOperations = new Set([ tableOperations.createGroup.id, tableOperations.updateGroup.id, tableOperations.deleteGroup.id, + tableOperations.readRow.id, + tableOperations.updateRow.id, + tableOperations.deleteRow.id, + tableOperations.upsertRow.id, ]) for (const operation of Object.values(tableOperations)) { expect(operation.delegatedServices).toEqual( executorOnlyOperations.has(operation.id) ? ['executor'] - : sharedGroupOperations.has(operation.id) + : sharedToolOperations.has(operation.id) ? ['copilot', 'executor'] : ['copilot'] ) diff --git a/apps/sim/lib/table/application/operations.ts b/apps/sim/lib/table/application/operations.ts index 42476b3783a..dfc7fc8e698 100644 --- a/apps/sim/lib/table/application/operations.ts +++ b/apps/sim/lib/table/application/operations.ts @@ -46,6 +46,15 @@ function toolWriteOperation(id: Id) { }) } +function toolReadOperation(id: Id) { + return defineWorkspaceOperation({ + id, + minimumRole: 'read', + workspaceApiKey: 'allow', + ...ALL_TABLE_TOOL_PRINCIPAL_POLICY, + }) +} + function internalExecutorReadOperation(id: Id) { return defineWorkspaceOperation({ id, @@ -104,14 +113,14 @@ export const tableOperations = { listRows: readOperation('tables.rows.list'), queryRows: readOperation('tables.rows.query'), findRows: readOperation('tables.rows.find'), - readRow: readOperation('tables.rows.read'), + readRow: toolReadOperation('tables.rows.read'), createRows: writeOperation('tables.rows.create'), replaceRows: writeOperation('tables.rows.replace'), - updateRow: writeOperation('tables.rows.update'), + updateRow: toolWriteOperation('tables.rows.update'), updateRows: writeOperation('tables.rows.update_many'), - deleteRow: writeOperation('tables.rows.delete'), + deleteRow: toolWriteOperation('tables.rows.delete'), deleteRows: writeOperation('tables.rows.delete_many'), - upsertRow: writeOperation('tables.rows.upsert'), + upsertRow: toolWriteOperation('tables.rows.upsert'), listViews: readOperation('tables.views.list'), readView: readOperation('tables.views.read'), createView: writeOperation('tables.views.create'), diff --git a/apps/sim/lib/table/application/row-secret-provenance.ts b/apps/sim/lib/table/application/row-secret-provenance.ts index 5f10c4b10a1..fba5bdca067 100644 --- a/apps/sim/lib/table/application/row-secret-provenance.ts +++ b/apps/sim/lib/table/application/row-secret-provenance.ts @@ -35,15 +35,15 @@ export class TableRowProvenanceError extends Error { * `null`; the id path stores what it is given, so every key it sends is a * storage key and none of them is `null`. */ -function storageKeyByWireKey( - row: RowData, +function storageKeyResolver( table: TableDefinition, keying: 'names' | 'ids' -): Map { - const wireKeys = Object.keys(row) - if (keying === 'ids') return new Map(wireKeys.map((key) => [key, key])) +): (wireKey: string) => string | null { + // Built once for the whole batch rather than once per row, matching how + // `rowsToStorage` hoists the same index. + if (keying === 'ids') return (wireKey) => wireKey const idByName = buildIdByName(table.schema) - return new Map(wireKeys.map((key) => [key, idByName.get(key) ?? null])) + return (wireKey) => idByName.get(wireKey) ?? null } /** @@ -91,13 +91,13 @@ export function resolveRowWriteProvenance(options: { } const bundle = envelope.value + const storageKeyFor = storageKeyResolver(table, keying) const columnIdBySelectionKey = new Map() const rowKeyBySelectionKey = new Map() wireRows.forEach((row, rowIndex) => { - const storageKeys = storageKeyByWireKey(row, table, keying) for (const wireKey of Object.keys(row)) { const selectionKey = tableRowSecretProvenanceSelectionKey(rowIndex, wireKey) - columnIdBySelectionKey.set(selectionKey, storageKeys.get(wireKey) ?? null) + columnIdBySelectionKey.set(selectionKey, storageKeyFor(wireKey)) rowKeyBySelectionKey.set(selectionKey, rowIndex) } }) diff --git a/apps/sim/lib/table/application/rows.ts b/apps/sim/lib/table/application/rows.ts index 44012017bcc..f8ba74583e1 100644 --- a/apps/sim/lib/table/application/rows.ts +++ b/apps/sim/lib/table/application/rows.ts @@ -26,13 +26,14 @@ import { deleteRowsByFilter, deleteRowsByIds, findRowMatches, - getRowById, + getRowSummaryById, insertRow, queryRows, replaceTableRows as replaceTableRowsPrimitive, rowDataNameToId, sortSpecNamesToIds, TABLE_LIMITS, + type TableRowSummary, updateRow, updateRowsByFilter, upsertRow, @@ -115,7 +116,9 @@ type TableRowsProvenance = Awaited[0], workspaceId: string, - rows: TableRow[], + // The loader reads only id, updatedAt and data, so a row without its + // executions sidecar is enough — see `TABLE_ROW_SIDECAR_SELECTION`. + rows: TableRowSummary[], include: boolean | undefined ): Promise { if (!include) return undefined @@ -472,7 +475,8 @@ export interface ReadTableRowInput extends TableScopedInput { } export interface ReadTableRowResult extends TableResult { - row: TableRow + /** Without the executions sidecar — no read surface puts it on the wire. */ + row: TableRowSummary secretProvenance?: TableRowsProvenance } @@ -480,7 +484,7 @@ export const readTableRow = defineAuthorizedTableUseCase({ operation: tableOperations.readRow, resolveContext: ({ input }: { input: ReadTableRowInput }) => resolveActiveTableContext(input), async execute({ principal, input, context }): Promise { - const row = await getRowById(context.tableId, input.rowId, context.workspaceId) + const row = await getRowSummaryById(context.tableId, input.rowId, context.workspaceId) if (!row) throw new OrchestrationError('not_found', 'Row not found') return { table: context.table, diff --git a/apps/sim/lib/table/rows/service.ts b/apps/sim/lib/table/rows/service.ts index 5495e7ae38f..24e82c6b330 100644 --- a/apps/sim/lib/table/rows/service.ts +++ b/apps/sim/lib/table/rows/service.ts @@ -1417,6 +1417,53 @@ async function fetchRowsBounded(params: BoundedFetchParams): Promise + +function selectRowRecord(tableId: string, rowId: string, workspaceId: string) { + return db + .select() + .from(userTableRows) + .where( + and( + eq(userTableRows.id, rowId), + eq(userTableRows.tableId, tableId), + eq(userTableRows.workspaceId, workspaceId) + ) + ) + .limit(1) +} + +function toRowSummary(row: Awaited>[number]): TableRowSummary { + return { + id: row.id, + data: row.data as RowData, + position: row.position, + orderKey: row.orderKey ?? undefined, + createdAt: row.createdAt, + updatedAt: row.updatedAt, + } +} + +/** + * One row without its executions sidecar, for the surfaces that never put + * executions on the wire — the single-row read routes and the Copilot row tool. + * Loading the sidecar for them is a query whose result is discarded. + * + * Deliberately a separate function rather than a flag on {@link getRowById}: a + * caller that forgets to pass the flag reads an empty sidecar and cannot tell + * that from a row with no executions, whereas here the field simply is not on + * the type. + */ +export async function getRowSummaryById( + tableId: string, + rowId: string, + workspaceId: string +): Promise { + const [row] = await selectRowRecord(tableId, rowId, workspaceId) + return row ? toRowSummary(row) : null +} + export async function getRowById( tableId: string, rowId: string, @@ -1427,32 +1474,13 @@ export async function getRowById( // round trip instead of two. A miss pays one redundant sidecar read, which is // the rare path and costs no extra wall time. const [results, executions] = await Promise.all([ - db - .select() - .from(userTableRows) - .where( - and( - eq(userTableRows.id, rowId), - eq(userTableRows.tableId, tableId), - eq(userTableRows.workspaceId, workspaceId) - ) - ) - .limit(1), + selectRowRecord(tableId, rowId, workspaceId), loadExecutionsForRow(db, rowId), ]) if (results.length === 0) return null - const row = results[0] - return { - id: row.id, - data: row.data as RowData, - executions, - position: row.position, - orderKey: row.orderKey ?? undefined, - createdAt: row.createdAt, - updatedAt: row.updatedAt, - } + return { ...toRowSummary(results[0]), executions } } /** @@ -1617,10 +1645,13 @@ export async function updateRow( // table that has any unique column this was several round trips on every // edit, including edits nowhere near one. // - // The one case this does not cover is a unique constraint added to a column - // that already held duplicates: such a row is no longer blocked from edits - // elsewhere in it. That is the intended outcome — an unrelated cell edit - // should not fail on data it did not write. + // What this does not cover is a duplicate that already exists — either from a + // constraint added to a column that already held one, or from two concurrent + // inserts both passing this probe, since uniqueness here is advisory (a + // SELECT, not a DB constraint). Such a row is no longer blocked from edits + // elsewhere in it. That is the intended outcome: an unrelated cell edit + // should not fail on data it did not write, and blocking it was never a + // repair mechanism. const patchedColumnIds = new Set(Object.keys(data.data)) const patchedUniqueColumns = getUniqueColumns(table.schema).filter((column) => patchedColumnIds.has(getColumnId(column)) diff --git a/apps/sim/tools/table/delete_row.ts b/apps/sim/tools/table/delete_row.ts index 47d46d699aa..fa339c07c49 100644 --- a/apps/sim/tools/table/delete_row.ts +++ b/apps/sim/tools/table/delete_row.ts @@ -23,6 +23,7 @@ export const tableDeleteRowTool: ToolConfig `/api/table/${params.tableId}/rows/${params.rowId}`, method: 'DELETE', headers: () => ({ diff --git a/apps/sim/tools/table/get_row.ts b/apps/sim/tools/table/get_row.ts index 7b76e605fda..010dcc53af5 100644 --- a/apps/sim/tools/table/get_row.ts +++ b/apps/sim/tools/table/get_row.ts @@ -23,6 +23,7 @@ export const tableGetRowTool: ToolConfig = }, request: { + internalAuth: 'executor_delegation', secretProvenance: { response: { incomplete: 'propagate' } }, url: (params: TableRowGetParams) => { const workspaceId = params._context?.workspaceId diff --git a/apps/sim/tools/table/update_row.ts b/apps/sim/tools/table/update_row.ts index c9792f95680..24f781cf042 100644 --- a/apps/sim/tools/table/update_row.ts +++ b/apps/sim/tools/table/update_row.ts @@ -38,6 +38,7 @@ export const tableUpdateRowTool: ToolConfig selectTableRowSecretProvenance([params.data]), response: { incomplete: 'propagate' }, diff --git a/apps/sim/tools/table/upsert_row.ts b/apps/sim/tools/table/upsert_row.ts index 70afc179872..7a62a79ef82 100644 --- a/apps/sim/tools/table/upsert_row.ts +++ b/apps/sim/tools/table/upsert_row.ts @@ -39,6 +39,7 @@ export const tableUpsertRowTool: ToolConfig selectTableRowSecretProvenance([params.data]), response: { incomplete: 'propagate' }, From 9b1b0d0f7c05a13fda683846da8229672a4138bb Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 17 Aug 2026 22:23:31 -0700 Subject: [PATCH 6/7] fix(table): keep the lock on a 423 and pin the remaining wire changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rows error policy was built on the concealment base rather than the lock-aware one, so a TableLockedError fell through to the generic handler and the response lost its `lock` field — the only thing that tells a client which lock to clear. A row write is exactly as lockable as a group mutation, so it now shares that base. Also pins the two wire changes the review found undocumented: a mismatched workspace assertion answers 404 rather than 400, which is a superset of the cross-tenant concealment already intended, and an unclassified failure answers the builder's shared "Internal server error" rather than the old per-route text. Both are consistent with the ~80 routes already on this builder; they are asserted so they read as decisions rather than drift. Verified to fail: reverting the policy base turns the lock test red. --- .../[tableId]/rows/[rowId]/route.test.ts | 28 +++++++++++++++++++ apps/sim/lib/table/api/route-policies.ts | 6 +++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.test.ts b/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.test.ts index 5d5f5e18537..752e18b4c11 100644 --- a/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.test.ts +++ b/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.test.ts @@ -43,6 +43,7 @@ vi.mock('@/lib/table/api', async (importOriginal) => { import { InternalUnauthenticatedError } from '@/lib/api/server/routes' import { NoWorkspaceAccessError } from '@/lib/core/application' import { OrchestrationError } from '@/lib/core/orchestration/types' +import { TableLockedError } from '@/lib/table/mutation-locks' import { DELETE, GET, PATCH } from '@/app/api/table/[tableId]/rows/[rowId]/route' const TABLE_ID = 'tbl_1' @@ -336,6 +337,33 @@ describe('deliberate wire changes', () => { expect(response.status).toBe(403) }) + it('keeps the lock on a 423 so the client knows which one to clear', async () => { + mocks.updateRow.mockRejectedValue(new TableLockedError('update')) + + const response = await PATCH( + bodyRequest('PATCH', { workspaceId: WORKSPACE_ID, data: { col_aaa: 'x' } }), + routeContext() + ) + + expect(response.status).toBe(423) + await expect(response.json()).resolves.toMatchObject({ lock: 'update' }) + }) + + it('answers a generic 500 message where the handler named the operation', async () => { + // The builder has one internal error envelope, shared with ~80 other + // migrated routes. The old per-route text ("Failed to update row") was more + // specific; consistency won, and the client only ever toasts the message. + mocks.updateRow.mockRejectedValue(new Error('boom')) + + const response = await PATCH( + bodyRequest('PATCH', { workspaceId: WORKSPACE_ID, data: { col_aaa: 'x' } }), + routeContext() + ) + + expect(response.status).toBe(500) + await expect(response.json()).resolves.toMatchObject({ error: 'Internal server error' }) + }) + it('conceals a mismatched workspace assertion as 404, where it used to answer 400', async () => { mocks.updateRow.mockRejectedValue(new OrchestrationError('not_found', 'Table not found')) diff --git a/apps/sim/lib/table/api/route-policies.ts b/apps/sim/lib/table/api/route-policies.ts index edb2fbd3ef1..d48e4655c6a 100644 --- a/apps/sim/lib/table/api/route-policies.ts +++ b/apps/sim/lib/table/api/route-policies.ts @@ -106,9 +106,13 @@ export const internalTableErrorPolicies = { * envelope that does not authenticate are both the caller's to fix and answer * 400; everything else conceals a cross-tenant table behind the same not-found * wording the rest of the table surface uses. + * + * Built on the lock-aware base so a 423 keeps carrying `lock` — the only field + * that tells a client which lock to clear. A row write is exactly as lockable + * as a group mutation. */ export const internalTableRowsErrorPolicy = extendInternalErrorPolicy( - internalTableErrorPolicies.concealTableAuthorization, + internalTableErrorPolicies.concealTableGroupAuthorization, (error) => { if (error instanceof TableRowsValidationError) { return internalErrorResponse(400, { error: error.message }) From 7f9a1dea4442099917f6d4c2489a4598f6a5f410 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 17 Aug 2026 22:37:56 -0700 Subject: [PATCH 7/7] refactor(table): apply the quality pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four parallel reviews (reuse, simplification, efficiency, altitude). The highest-value finding cut against the branch's own purpose: the rows error policy sat in the barrel-exported route-policies module, so its import of the 1,200-line row use-case graph was paid by every one of the ~28 table routes that can never throw a row error — the barrel's import cost went from ~1.1s to ~1.7s. row-route-policies.ts already existed for exactly this and is deliberately not re-exported; the policy now lives there with its v2 sibling. The upsert path still loaded an executions sidecar no surface puts on the wire, and did it inside the write transaction, holding it open for a discarded result. The read path got that fix earlier; the write path next to it did not. rowKeyingForPrincipal fell through to name keying for anything that was not a session. The operation policy admits API-key principals, so the first one to reach these routes would have had every id-keyed cell dropped and the write reported as successful. It is now an exhaustive switch over the two kinds the auth policy yields — which immediately failed three tests using a principal kind that exists nowhere in the repo, so those fixtures are real now too. Also: reuses toWireTimestamp and createUnknownTableRowSecretProvenance instead of re-inlining them; shares one helper for the provenance choice the update and upsert use cases both make; keeps one canonical actorClientId doc with two cross-references; merges two maps keyed by the same string into one; stops round-tripping the principal through the legacy AuthType enum; hoists the presenter function type out of both conditional branches; drops a subsumed test; and freezes the shared locks fixture so a mutating test cannot poison its siblings. --- .../[rowId]/enrichment/[groupId]/route.ts | 3 +- .../[tableId]/rows/[rowId]/route.test.ts | 18 ++-- .../api/table/[tableId]/rows/[rowId]/route.ts | 13 +-- .../table/[tableId]/rows/upsert/route.test.ts | 10 +- .../api/table/[tableId]/rows/upsert/route.ts | 11 +-- .../app/api/table/row-secret-provenance.ts | 4 +- apps/sim/app/api/table/row-wire.ts | 43 ++++++--- .../api/server/routes/internal-json-route.ts | 20 ++-- apps/sim/lib/table/api/index.ts | 1 - apps/sim/lib/table/api/route-policies.ts | 26 ----- apps/sim/lib/table/api/row-route-policies.ts | 28 +++++- .../application/row-secret-provenance.ts | 33 ++++--- apps/sim/lib/table/application/rows.ts | 95 +++++++++++-------- apps/sim/lib/table/rows/service.ts | 15 +-- apps/sim/lib/table/trigger.ts | 4 +- apps/sim/lib/table/types.ts | 6 +- .../testing/src/factories/table.factory.ts | 4 +- 17 files changed, 178 insertions(+), 156 deletions(-) diff --git a/apps/sim/app/api/table/[tableId]/rows/[rowId]/enrichment/[groupId]/route.ts b/apps/sim/app/api/table/[tableId]/rows/[rowId]/enrichment/[groupId]/route.ts index be90d7a2fec..3caf42247f5 100644 --- a/apps/sim/app/api/table/[tableId]/rows/[rowId]/enrichment/[groupId]/route.ts +++ b/apps/sim/app/api/table/[tableId]/rows/[rowId]/enrichment/[groupId]/route.ts @@ -1,6 +1,7 @@ import { getEnrichmentDetailContract } from '@/lib/api/contracts/tables' import { defineInternalJsonRoute, internalRateLimits } from '@/lib/api/server/routes' -import { internalTableRowsErrorPolicy, internalTableSessionOrExecutorAuth } from '@/lib/table/api' +import { internalTableSessionOrExecutorAuth } from '@/lib/table/api' +import { internalTableRowsErrorPolicy } from '@/lib/table/api/row-route-policies' import { tableOperations } from '@/lib/table/application/operations' import { readTableRowEnrichmentDetail } from '@/lib/table/application/rows' diff --git a/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.test.ts b/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.test.ts index 752e18b4c11..cf4935b3b1e 100644 --- a/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.test.ts +++ b/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.test.ts @@ -82,10 +82,14 @@ function sessionPrincipal() { function executorPrincipal() { mocks.authenticate.mockResolvedValue({ - kind: 'workflow_execution_delegated', - userId: 'user-1', + kind: 'delegated', + serviceId: 'executor', + subjectUserId: 'user-1', workspaceId: WORKSPACE_ID, - executionId: 'exec-1', + delegationId: 'delegation-1', + audience: 'table', + issuedAt: new Date('2026-01-01'), + expiresAt: new Date('2026-01-02'), }) } @@ -268,14 +272,6 @@ describe('PATCH /api/table/[tableId]/rows/[rowId]', () => { expect(response.status).toBe(409) }) - - it('falls back to 500 for an unclassified failure', async () => { - mocks.updateRow.mockRejectedValue(new Error('boom')) - - const response = await PATCH(bodyRequest('PATCH', patchBody), routeContext()) - - expect(response.status).toBe(500) - }) }) describe('DELETE /api/table/[tableId]/rows/[rowId]', () => { diff --git a/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts b/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts index 5b623b4155e..02a5a45cfa4 100644 --- a/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts +++ b/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts @@ -5,7 +5,8 @@ import { updateTableRowContract, } from '@/lib/api/contracts/tables' import { defineInternalJsonRoute, internalRateLimits } from '@/lib/api/server/routes' -import { internalTableRowsErrorPolicy, internalTableSessionOrExecutorAuth } from '@/lib/table/api' +import { internalTableSessionOrExecutorAuth } from '@/lib/table/api' +import { internalTableRowsErrorPolicy } from '@/lib/table/api/row-route-policies' import { tableOperations } from '@/lib/table/application/operations' import { deleteTableRow, readTableRow, updateTableRow } from '@/lib/table/application/rows' import type { RowData } from '@/lib/table/types' @@ -14,11 +15,7 @@ import { negotiateTableRowsProvenance, readTableRowProvenanceEnvelope, } from '@/app/api/table/row-secret-provenance' -import { - authTypeForPrincipal, - presentRowForPrincipal, - rowKeyingForPrincipal, -} from '@/app/api/table/row-wire' +import { presentRowForPrincipal, rowKeyingForPrincipal } from '@/app/api/table/row-wire' export const dynamic = 'force-dynamic' @@ -38,7 +35,7 @@ export const GET = defineInternalJsonRoute({ assertedWorkspaceId: query.workspaceId, includePersistedSecretProvenance: negotiateTableRowsProvenance( request, - authTypeForPrincipal(principal) + principal.kind !== 'session' ), }), useCase: readTableRow, @@ -68,7 +65,7 @@ export const PATCH = defineInternalJsonRoute({ secretProvenanceEnvelope: readTableRowProvenanceEnvelope(request, body), includePersistedSecretProvenance: negotiateTableRowsProvenance( request, - authTypeForPrincipal(principal) + principal.kind !== 'session' ), actorClientId: readClientId(request), } diff --git a/apps/sim/app/api/table/[tableId]/rows/upsert/route.test.ts b/apps/sim/app/api/table/[tableId]/rows/upsert/route.test.ts index 1b0c13690a1..2d07bd5e0d0 100644 --- a/apps/sim/app/api/table/[tableId]/rows/upsert/route.test.ts +++ b/apps/sim/app/api/table/[tableId]/rows/upsert/route.test.ts @@ -123,10 +123,14 @@ describe('POST /api/table/[tableId]/rows/upsert', () => { it('tells the use case a workflow execution speaks column names', async () => { mocks.authenticate.mockResolvedValue({ - kind: 'workflow_execution_delegated', - userId: 'user-1', + kind: 'delegated', + serviceId: 'executor', + subjectUserId: 'user-1', workspaceId: WORKSPACE_ID, - executionId: 'exec-1', + delegationId: 'delegation-1', + audience: 'table', + issuedAt: new Date('2026-01-01'), + expiresAt: new Date('2026-01-02'), }) await POST(request({ ...BODY, data: { Name: 'Ada' }, conflictTarget: 'Name' }), routeContext()) diff --git a/apps/sim/app/api/table/[tableId]/rows/upsert/route.ts b/apps/sim/app/api/table/[tableId]/rows/upsert/route.ts index 34f147fdb39..d34559cb66f 100644 --- a/apps/sim/app/api/table/[tableId]/rows/upsert/route.ts +++ b/apps/sim/app/api/table/[tableId]/rows/upsert/route.ts @@ -1,6 +1,7 @@ import { upsertTableRowContract } from '@/lib/api/contracts/tables' import { defineInternalJsonRoute, internalRateLimits } from '@/lib/api/server/routes' -import { internalTableRowsErrorPolicy, internalTableSessionOrExecutorAuth } from '@/lib/table/api' +import { internalTableSessionOrExecutorAuth } from '@/lib/table/api' +import { internalTableRowsErrorPolicy } from '@/lib/table/api/row-route-policies' import { tableOperations } from '@/lib/table/application/operations' import { upsertTableRow } from '@/lib/table/application/rows' import type { RowData } from '@/lib/table/types' @@ -9,11 +10,7 @@ import { negotiateTableRowsProvenance, readTableRowProvenanceEnvelope, } from '@/app/api/table/row-secret-provenance' -import { - authTypeForPrincipal, - presentRowForPrincipal, - rowKeyingForPrincipal, -} from '@/app/api/table/row-wire' +import { presentRowForPrincipal, rowKeyingForPrincipal } from '@/app/api/table/row-wire' export const dynamic = 'force-dynamic' @@ -40,7 +37,7 @@ export const POST = defineInternalJsonRoute({ secretProvenanceEnvelope: readTableRowProvenanceEnvelope(request, body), includePersistedSecretProvenance: negotiateTableRowsProvenance( request, - authTypeForPrincipal(principal) + principal.kind !== 'session' ), }), useCase: upsertTableRow, diff --git a/apps/sim/app/api/table/row-secret-provenance.ts b/apps/sim/app/api/table/row-secret-provenance.ts index 5021b7b1de6..d455a6617d8 100644 --- a/apps/sim/app/api/table/row-secret-provenance.ts +++ b/apps/sim/app/api/table/row-secret-provenance.ts @@ -225,12 +225,12 @@ export function readTableRowProvenanceEnvelope( */ export function negotiateTableRowsProvenance( request: NextRequest, - authType: AuthTypeValue | undefined + isInternalCaller: boolean ): boolean { const negotiation = negotiatePrivateToolMetadataResponse( request.headers, RESOLVED_SECRET_PROVENANCE_METADATA_V1, - authType === AuthType.INTERNAL_JWT + isInternalCaller ) if (negotiation.status === 'rejected') throw new TableRowProvenanceError() return negotiation.status !== 'not-requested' diff --git a/apps/sim/app/api/table/row-wire.ts b/apps/sim/app/api/table/row-wire.ts index 3c5c948e693..b20121a621e 100644 --- a/apps/sim/app/api/table/row-wire.ts +++ b/apps/sim/app/api/table/row-wire.ts @@ -1,4 +1,4 @@ -import type { Principal } from '@sim/auth/principal' +import type { SessionPrincipal, WorkflowExecutionDelegatedPrincipal } from '@sim/auth/principal' import { AuthType, type AuthTypeValue } from '@/lib/auth/hybrid' import type { Filter, @@ -19,6 +19,7 @@ import { sortSpecNamesToIds, } from '@/lib/table/column-keys' import { predicateToStorage, resolveFilterSelectValues } from '@/lib/table/select-values' +import { toWireTimestamp } from '@/lib/table/wire' export interface RowWireTranslators { /** Inbound row data: wire keys → storage column ids. */ @@ -72,6 +73,15 @@ export function rowWireTranslators( } } +/** + * The principal kinds the internal table row routes admit — the auth policy + * yields exactly these two. Typed as the union rather than `Principal` so a + * third kind becomes an exhaustiveness error here instead of silently taking + * the name-keyed branch, which would drop every id-keyed cell of a write and + * report success. + */ +type TableRowRoutePrincipal = SessionPrincipal | WorkflowExecutionDelegatedPrincipal + /** * The internal table routes serve two caller kinds on the same paths, and they * speak different column keyings: the first-party grid holds the schema it @@ -79,13 +89,13 @@ export function rowWireTranslators( * speaks column names, because names are what tool enrichment surfaces to the * model. Keying is therefore a property of the caller, not of the endpoint. */ -export function authTypeForPrincipal(principal: Principal): AuthTypeValue { - return principal.kind === 'session' ? AuthType.SESSION : AuthType.INTERNAL_JWT -} - -/** See {@link authTypeForPrincipal}. Feeds the use case's `dataKeying`. */ -export function rowKeyingForPrincipal(principal: Principal): TableRowDataKeying { - return principal.kind === 'session' ? 'ids' : 'names' +export function rowKeyingForPrincipal(principal: TableRowRoutePrincipal): TableRowDataKeying { + switch (principal.kind) { + case 'session': + return 'ids' + case 'delegated': + return 'names' + } } /** @@ -96,14 +106,21 @@ export function rowKeyingForPrincipal(principal: Principal): TableRowDataKeying export function presentRowForPrincipal( row: Pick, schema: TableSchema, - principal: Principal + principal: TableRowRoutePrincipal ) { - const wire = rowWireTranslators(authTypeForPrincipal(principal), schema) + // Only the outbound mapper is needed here; building the full translator set + // would also index the schema name→id for inbound paths a presenter cannot reach. + const dataOut = + rowKeyingForPrincipal(principal) === 'names' ? namedRowMapper(schema.columns) : identity return { id: row.id, - data: wire.dataOut(row.data), + data: dataOut(row.data), position: row.position, - createdAt: row.createdAt instanceof Date ? row.createdAt.toISOString() : String(row.createdAt), - updatedAt: row.updatedAt instanceof Date ? row.updatedAt.toISOString() : String(row.updatedAt), + createdAt: toWireTimestamp(row.createdAt), + updatedAt: toWireTimestamp(row.updatedAt), } } + +function identity(value: T): T { + return value +} diff --git a/apps/sim/lib/api/server/routes/internal-json-route.ts b/apps/sim/lib/api/server/routes/internal-json-route.ts index 69b596c955f..ae198a2684d 100644 --- a/apps/sim/lib/api/server/routes/internal-json-route.ts +++ b/apps/sim/lib/api/server/routes/internal-json-route.ts @@ -215,21 +215,17 @@ export interface InternalJsonPresenterContext { input: I } +type InternalJsonPresentFn = ( + result: NoInfer, + context: InternalJsonPresenterContext, NoInfer

> +) => ContractJsonResponse | Promise> + +/** The presenter is optional exactly when the result already is the response body. */ type InternalJsonPresenter = [ R, ] extends [ContractJsonResponse] - ? { - present?( - result: NoInfer, - context: InternalJsonPresenterContext, NoInfer

> - ): ContractJsonResponse | Promise> - } - : { - present( - result: NoInfer, - context: InternalJsonPresenterContext, NoInfer

> - ): ContractJsonResponse | Promise> - } + ? { present?: InternalJsonPresentFn } + : { present: InternalJsonPresentFn } type InternalJsonRouteOptions< C extends JsonApiRouteContract, diff --git a/apps/sim/lib/table/api/index.ts b/apps/sim/lib/table/api/index.ts index 22d2d977f04..e04cc23089e 100644 --- a/apps/sim/lib/table/api/index.ts +++ b/apps/sim/lib/table/api/index.ts @@ -1,6 +1,5 @@ export { internalTableErrorPolicies, - internalTableRowsErrorPolicy, internalTableSessionOrExecutorAuth, v2TableErrorPolicies, } from '@/lib/table/api/route-policies' diff --git a/apps/sim/lib/table/api/route-policies.ts b/apps/sim/lib/table/api/route-policies.ts index d48e4655c6a..679c0deb676 100644 --- a/apps/sim/lib/table/api/route-policies.ts +++ b/apps/sim/lib/table/api/route-policies.ts @@ -9,8 +9,6 @@ import { } from '@/lib/api/server/routes' import { TABLE_DELEGATION_AUDIENCE } from '@/lib/table/application/authorization' import { TableOperationError } from '@/lib/table/application/errors' -import { TableRowProvenanceError } from '@/lib/table/application/row-secret-provenance' -import { TableRowsValidationError } from '@/lib/table/application/rows' import { TableLockedError } from '@/lib/table/mutation-locks' import { v2CaughtOrchestrationError, @@ -99,27 +97,3 @@ export const internalTableErrorPolicies = { notFoundMessage: 'Table export not found', }), } as const - -/** - * Row routes on the internal surface. The internal counterpart of - * {@link v2TableRowsErrorPolicy}: a row-shape complaint and a provenance - * envelope that does not authenticate are both the caller's to fix and answer - * 400; everything else conceals a cross-tenant table behind the same not-found - * wording the rest of the table surface uses. - * - * Built on the lock-aware base so a 423 keeps carrying `lock` — the only field - * that tells a client which lock to clear. A row write is exactly as lockable - * as a group mutation. - */ -export const internalTableRowsErrorPolicy = extendInternalErrorPolicy( - internalTableErrorPolicies.concealTableGroupAuthorization, - (error) => { - if (error instanceof TableRowsValidationError) { - return internalErrorResponse(400, { error: error.message }) - } - if (error instanceof TableRowProvenanceError) { - return internalErrorResponse(400, { error: error.message }) - } - return null - } -) diff --git a/apps/sim/lib/table/api/row-route-policies.ts b/apps/sim/lib/table/api/row-route-policies.ts index 08dba0ffdda..4a93c4e74c6 100644 --- a/apps/sim/lib/table/api/row-route-policies.ts +++ b/apps/sim/lib/table/api/row-route-policies.ts @@ -1,5 +1,10 @@ -import type { V2ErrorPolicy } from '@/lib/api/server/routes' -import { v2TableErrorPolicies } from '@/lib/table/api/route-policies' +import { + extendInternalErrorPolicy, + internalErrorResponse, + type V2ErrorPolicy, +} from '@/lib/api/server/routes' +import { internalTableErrorPolicies, v2TableErrorPolicies } from '@/lib/table/api/route-policies' +import { TableRowProvenanceError } from '@/lib/table/application/row-secret-provenance' import { TableRowsValidationError } from '@/lib/table/application/rows' import { v2Error } from '@/app/api/v2/lib/response' @@ -11,3 +16,22 @@ export const v2TableRowsErrorPolicy = { return v2TableErrorPolicies.concealTableAuthorization.render(error) }, } satisfies V2ErrorPolicy + +/** + * Row routes on the internal surface. The internal counterpart of + * {@link v2TableRowsErrorPolicy}: a row-shape complaint and a provenance + * envelope that does not authenticate are both the caller's to fix and answer + * 400; everything else conceals a cross-tenant table behind the same not-found + * wording the rest of the table surface uses. + * + * Built on the lock-aware base so a 423 keeps carrying `lock` — the only field + * that tells a client which lock to clear. A row write is exactly as lockable + * as a group mutation. + */ +export const internalTableRowsErrorPolicy = extendInternalErrorPolicy( + internalTableErrorPolicies.concealTableGroupAuthorization, + (error) => + error instanceof TableRowsValidationError || error instanceof TableRowProvenanceError + ? internalErrorResponse(400, { error: error.message }) + : null +) diff --git a/apps/sim/lib/table/application/row-secret-provenance.ts b/apps/sim/lib/table/application/row-secret-provenance.ts index fba5bdca067..3911703ac42 100644 --- a/apps/sim/lib/table/application/row-secret-provenance.ts +++ b/apps/sim/lib/table/application/row-secret-provenance.ts @@ -2,7 +2,10 @@ import { type Principal, requirePrincipalSubjectUserId } from '@sim/auth/princip import { isPrivateSecretProvenanceScopeCompatible } from '@/lib/execution/durable-secret-provenance' import { isPrivateSecretProvenanceBundleV1 } from '@/lib/execution/model-input-provenance' import { buildIdByName } from '@/lib/table/column-keys' -import { createExactEmptyTableRowSecretProvenance } from '@/lib/table/rows/secret-provenance' +import { + createExactEmptyTableRowSecretProvenance, + createUnknownTableRowSecretProvenance, +} from '@/lib/table/rows/secret-provenance' import { tableRowSecretProvenanceSelectionKey } from '@/lib/table/secret-provenance-selection' import type { RowData, TableDefinition, TableRowSecretProvenanceWrite } from '@/lib/table/types' @@ -92,13 +95,14 @@ export function resolveRowWriteProvenance(options: { const bundle = envelope.value const storageKeyFor = storageKeyResolver(table, keying) - const columnIdBySelectionKey = new Map() - const rowKeyBySelectionKey = new Map() + /** Every cell this write touches, by the selection key a bundle must name. */ + const touchedBySelectionKey = new Map() wireRows.forEach((row, rowIndex) => { for (const wireKey of Object.keys(row)) { - const selectionKey = tableRowSecretProvenanceSelectionKey(rowIndex, wireKey) - columnIdBySelectionKey.set(selectionKey, storageKeyFor(wireKey)) - rowKeyBySelectionKey.set(selectionKey, rowIndex) + touchedBySelectionKey.set(tableRowSecretProvenanceSelectionKey(rowIndex, wireKey), { + rowIndex, + columnId: storageKeyFor(wireKey), + }) } }) @@ -106,14 +110,14 @@ export function resolveRowWriteProvenance(options: { // those — otherwise a caller could certify a column it never wrote. if ( bundle.complete && - (bundle.selections.length !== columnIdBySelectionKey.size || - bundle.selections.some((selection) => !columnIdBySelectionKey.has(selection.key))) + (bundle.selections.length !== touchedBySelectionKey.size || + bundle.selections.some((selection) => !touchedBySelectionKey.has(selection.key))) ) { throw new TableRowProvenanceError() } if (!bundle.complete) { - return { stamps: wireRows.map(() => ({ complete: false, columns: {} })) } + return { stamps: wireRows.map(() => createUnknownTableRowSecretProvenance()) } } const stamps: TableRowSecretProvenanceWrite[] = wireRows.map(() => ({ @@ -122,9 +126,9 @@ export function resolveRowWriteProvenance(options: { })) const subjectUserId = requirePrincipalSubjectUserId(principal) for (const selection of bundle.selections) { - const rowIndex = rowKeyBySelectionKey.get(selection.key) + const touched = touchedBySelectionKey.get(selection.key) if ( - rowIndex === undefined || + !touched || !isPrivateSecretProvenanceScopeCompatible(selection.provenance.scope, { userId: subjectUserId, workspaceId: options.workspaceId, @@ -132,12 +136,11 @@ export function resolveRowWriteProvenance(options: { ) { throw new TableRowProvenanceError() } - const columnId = columnIdBySelectionKey.get(selection.key) - if (columnId === null || columnId === undefined) continue - if (Object.hasOwn(stamps[rowIndex].columns, columnId)) { + if (touched.columnId === null) continue + if (Object.hasOwn(stamps[touched.rowIndex].columns, touched.columnId)) { throw new TableRowProvenanceError() } - stamps[rowIndex].columns[columnId] = selection.provenance + stamps[touched.rowIndex].columns[touched.columnId] = selection.provenance } return { stamps } } diff --git a/apps/sim/lib/table/application/rows.ts b/apps/sim/lib/table/application/rows.ts index f8ba74583e1..3e0a06710fa 100644 --- a/apps/sim/lib/table/application/rows.ts +++ b/apps/sim/lib/table/application/rows.ts @@ -1,6 +1,10 @@ import { isDeepStrictEqual } from 'node:util' import { AuditAction, AuditResourceType } from '@sim/audit' -import { requirePrincipalSubjectUserId, resolvePrincipalAttribution } from '@sim/auth/principal' +import { + type Principal, + requirePrincipalSubjectUserId, + resolvePrincipalAttribution, +} from '@sim/auth/principal' import { db } from '@sim/db' import { getRequestContext } from '@sim/logger' import { generateId } from '@sim/utils/id' @@ -273,6 +277,38 @@ function defaultedRowSecretProvenance( return provided ?? createExactEmptyTableRowSecretProvenance(storageData) } +/** + * The stamp a single-row write should carry: resolved from the caller's envelope + * when it handed one over, otherwise defaulted. Shared by the update and upsert + * use cases so the envelope contract has one implementation, not two. + */ +function singleRowWriteProvenance(options: { + principal: Principal + workspaceId: string + table: TableDefinition + input: { + dataKeying: TableRowDataKeying + data: RowData + secretProvenance?: TableRowSecretProvenanceWrite + secretProvenanceEnvelope?: TableRowProvenanceEnvelope + } + storageData: RowData +}): TableRowSecretProvenanceWrite | undefined { + const { principal, workspaceId, table, input, storageData } = options + if (!input.secretProvenanceEnvelope) { + return defaultedRowSecretProvenance(storageData, input.secretProvenance) + } + return resolveRowWriteProvenance({ + envelope: input.secretProvenanceEnvelope, + principal, + workspaceId, + table, + keying: input.dataKeying, + wireRows: [input.data], + storageRows: [storageData], + }).stamps[0] +} + function defaultedRowsSecretProvenance( storageRows: RowData[], provided: Array | undefined @@ -535,14 +571,7 @@ interface CreateSingleTableRowInput extends TableScopedInput { /** See {@link TableRowDataKeying}. Required so a new write surface must choose. */ dataKeying: TableRowDataKeying kind: 'single' - /** - * Tab that caused this write, when the calling surface knows it. Lets that tab - * skip refetching its own write — see {@link signalTableRowsChangedByActor}, - * whose soundness condition is that the caller's hook reconciles the write - * locally across every cached rows query. Only the single-row paths accept - * one: a batch or filter-scoped write genuinely needs the acting tab to - * refetch. Absent by default, which broadcasts to every subscriber as before. - */ + /** See {@link UpdateTableRowInput.actorClientId}. */ actorClientId?: string data: RowData position?: number @@ -920,17 +949,13 @@ export const updateTableRow = defineAuthorizedTableUseCase({ resolveContext: ({ input }: { input: UpdateTableRowInput }) => resolveActiveTableContext(input), async execute({ principal, input, context }): Promise { const data = rowDataToStorage(input.data, context.table, input.dataKeying, input.strictWrite) - const secretProvenance = input.secretProvenanceEnvelope - ? resolveRowWriteProvenance({ - envelope: input.secretProvenanceEnvelope, - principal, - workspaceId: context.workspaceId, - table: context.table, - keying: input.dataKeying, - wireRows: [input.data], - storageRows: [data], - }).stamps[0] - : defaultedRowSecretProvenance(data, input.secretProvenance) + const secretProvenance = singleRowWriteProvenance({ + principal, + workspaceId: context.workspaceId, + table: context.table, + input, + storageData: data, + }) const row = await updateRow( { tableId: context.tableId, @@ -1008,14 +1033,7 @@ export const updateTableRows = defineAuthorizedTableUseCase({ export interface DeleteTableRowInput extends TableScopedInput { rowId: string - /** - * Tab that caused this write, when the calling surface knows it. Lets that tab - * skip refetching its own write — see {@link signalTableRowsChangedByActor}, - * whose soundness condition is that the caller's hook reconciles the write - * locally across every cached rows query. Only the single-row paths accept - * one: a batch or filter-scoped write genuinely needs the acting tab to - * refetch. Absent by default, which broadcasts to every subscriber as before. - */ + /** See {@link UpdateTableRowInput.actorClientId}. */ actorClientId?: string } @@ -1113,7 +1131,8 @@ export interface UpsertTableRowInput extends TableScopedInput { } export interface UpsertTableRowResult extends TableResult { - row: TableRow + /** Without the executions sidecar — see {@link UpsertResult.row}. */ + row: TableRowSummary operation: 'insert' | 'update' secretProvenance?: TableRowsProvenance } @@ -1129,17 +1148,13 @@ export const upsertTableRow = defineAuthorizedTableUseCase({ ? (buildIdByName(context.table.schema).get(input.conflictTarget) ?? input.conflictTarget) : input.conflictTarget const data = rowDataToStorage(input.data, context.table, input.dataKeying, input.strictWrite) - const secretProvenance = input.secretProvenanceEnvelope - ? resolveRowWriteProvenance({ - envelope: input.secretProvenanceEnvelope, - principal, - workspaceId: context.workspaceId, - table: context.table, - keying: input.dataKeying, - wireRows: [input.data], - storageRows: [data], - }).stamps[0] - : defaultedRowSecretProvenance(data, input.secretProvenance) + const secretProvenance = singleRowWriteProvenance({ + principal, + workspaceId: context.workspaceId, + table: context.table, + input, + storageData: data, + }) const result = await upsertRow( { tableId: context.tableId, diff --git a/apps/sim/lib/table/rows/service.ts b/apps/sim/lib/table/rows/service.ts index 24e82c6b330..4857338e68a 100644 --- a/apps/sim/lib/table/rows/service.ts +++ b/apps/sim/lib/table/rows/service.ts @@ -791,12 +791,13 @@ export async function upsertRow( }) if (!updatedRow) throw new Error('Matched table row no longer exists') - const executions = await loadExecutionsForRow(trx, updatedRow.id) + // No executions sidecar: no upsert surface puts one on the wire, and + // loading it here would hold the write transaction open for a result that + // is discarded. See `getRowSummaryById` for the same reasoning on reads. return { row: { id: updatedRow.id, data: updatedRow.data as RowData, - executions, position: updatedRow.position, orderKey: updatedRow.orderKey ?? undefined, createdAt: updatedRow.createdAt, @@ -842,7 +843,6 @@ export async function upsertRow( row: { id: insertedRow.id, data: insertedRow.data as RowData, - executions: {}, position: insertedRow.position, orderKey: insertedRow.orderKey ?? undefined, createdAt: insertedRow.createdAt, @@ -1409,14 +1409,6 @@ async function fetchRowsBounded(params: BoundedFetchParams): Promise @@ -1464,6 +1456,7 @@ export async function getRowSummaryById( return row ? toRowSummary(row) : null } +/** One row with its executions sidecar, for the write and background paths. */ export async function getRowById( tableId: string, rowId: string, diff --git a/apps/sim/lib/table/trigger.ts b/apps/sim/lib/table/trigger.ts index cdb2b2a3169..a08ebc093a0 100644 --- a/apps/sim/lib/table/trigger.ts +++ b/apps/sim/lib/table/trigger.ts @@ -51,7 +51,9 @@ export async function fireTableTrigger( tableId: string, tableName: string, eventType: EventType, - rows: TableRow[], + // Accepts a row without its executions sidecar: the payload projects id and + // data only, and the upsert path deliberately does not load one. + rows: Array>, oldRows: Map | null, schema: TableSchema, requestId: string diff --git a/apps/sim/lib/table/types.ts b/apps/sim/lib/table/types.ts index 1e11fc1f857..b0950443756 100644 --- a/apps/sim/lib/table/types.ts +++ b/apps/sim/lib/table/types.ts @@ -718,7 +718,11 @@ export interface UpsertRowData { } export interface UpsertResult { - row: TableRow + /** + * Without the executions sidecar: no upsert surface puts one on the wire, and + * loading it would hold the write transaction open for a discarded result. + */ + row: Omit operation: 'insert' | 'update' previousData?: RowData } diff --git a/packages/testing/src/factories/table.factory.ts b/packages/testing/src/factories/table.factory.ts index 70e418618d5..65ccf7dbfde 100644 --- a/packages/testing/src/factories/table.factory.ts +++ b/packages/testing/src/factories/table.factory.ts @@ -120,12 +120,12 @@ export interface TableDefinitionFactoryOptions { updatedAt?: Date | string } -const UNLOCKED_TABLE_LOCKS: TableLocksFixture = { +const UNLOCKED_TABLE_LOCKS: TableLocksFixture = Object.freeze({ schemaLocked: false, insertLocked: false, updateLocked: false, deleteLocked: false, -} +}) /** * Creates a table definition fixture with sensible defaults — the shape route