Skip to content

Commit 8ba9755

Browse files
committed
fix(v2): bound the bulk update echo, close the search leak, and make the docs true
B3: a selectAll bulk document update echoed every changed identifier, which the request does not bound — a 100k-document knowledge base produced a multi-megabyte array, materialized and then element-wise validated. The use case now reports whether the selection was unbounded and the presenter omits the echo. A1: the knowledge search presenter spread the whole use-case result, which also carries userId, workspaceId, a cost breakdown and a live secret-trace registry. Only Zod's default key-stripping kept them off the wire. Projected explicitly. P1-a: GET /knowledge/{id}/tags advertised all 17 slots while the document PATCH accepted only the seven text ones. The writer already coerces every slot type, so the PATCH now takes all 17 in their declared types, with a 400 where a malformed value used to silently clear the tag. P1-b: both new PATCHes deny workspace API keys and now say so. P1-c: the two table query reads declare maxBodyBytes and now document the 413. P1-d: getWorkflowDeploymentV2 loses its legacy suffix. C3: deletes two orchestration error mappers with no callers that mapped 'forbidden' with no details. D2: a stored null in table_views.config survived the pick and failed the response schema. Also folds the six 'bounded set' paraphrases onto one FULL_SET_LIST constant, shares the run-window date bound between the logs and runs lists so their documented parity is enforced rather than asserted, adds the missing barrel export for FORBIDDEN_DETAIL_CODE_DESCRIPTIONS, and strictens two response schemas whose peers were already strict. Migrates 40 v2 route tests onto the shared @sim/testing harness: 26 asserted a rateLimitSubjectIds shape v2 auth never returns, 26 asserted the wrong refillRate, 33 could not exercise their 401 path at all, and 6 hard-wired the rollout gate to null.
1 parent 860c4c8 commit 8ba9755

70 files changed

Lines changed: 1491 additions & 1431 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

apps/docs/content/docs/en/api-reference/(generated)/workflows/meta.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
"getWorkflowVersionV2",
1010
"exportWorkflow",
1111
"importWorkflow",
12-
"getWorkflowDeploymentV2",
12+
"getWorkflowDeployment",
1313
"deployWorkflow",
1414
"undeployWorkflow",
1515
"rollbackWorkflow",

apps/docs/openapi-v2-knowledge.json

Lines changed: 48 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -543,7 +543,7 @@
543543
"get": {
544544
"operationId": "listKnowledgeTags",
545545
"summary": "List Tags",
546-
"description": "List the knowledge base's tag vocabulary: each tag's display name, the slot it is stored in, and its field type. Display names are what tag filters and the tag values on document reads use; slots are what document writes set (`tag1`..`tag7`). The vocabulary is bounded by the fixed slot table, so the whole set is returned in one response and `nextCursor` is always null.",
546+
"description": "List the knowledge base's tag vocabulary: each tag's display name, the slot it is stored in, and its field type. Display names are what tag filters and the tag values on document reads use; slots are what document writes set. Every slot listed here is writable, in its declared type: `tag1`..`tag7` take a string, `number1`..`number5` a number, `date1`..`date2` a `YYYY-MM-DD` string, and `boolean1`..`boolean3` a boolean. The vocabulary is bounded by the fixed slot table. The bounded set is returned in one page with `nextCursor` always null; there is no second page to fetch.",
547547
"tags": ["Knowledge Bases"],
548548
"parameters": [
549549
{
@@ -784,7 +784,7 @@
784784
"patch": {
785785
"operationId": "bulkUpdateKnowledgeDocuments",
786786
"summary": "Bulk Enable or Disable Documents",
787-
"description": "Enable or disable many documents in one request, either by identifier (up to 100) or, with `selectAll`, every document in the knowledge base optionally narrowed by `enabledFilter`. Disabling keeps a document indexed but excludes it from search. Bulk delete is deliberately not offered: the bulk path records no audit entries, so deletions go through `DELETE /api/v2/knowledge/{id}/documents/{documentId}`, which audits each one.",
787+
"description": "Enable or disable many documents in one request, either by identifier (up to 100) or, with `selectAll`, every document in the knowledge base optionally narrowed by `enabledFilter`. Disabling keeps a document indexed but excludes it from search. Bulk delete is deliberately not offered: the bulk path records no audit entries, so deletions go through `DELETE /api/v2/knowledge/{id}/documents/{documentId}`, which audits each one. An identifier request echoes the documents it changed in `documentIds`; a `selectAll` request omits that field because the selection is unbounded, and reports `updatedCount` alone. A workspace API key cannot call this operation and is rejected with `403`; use a personal API key.",
788788
"tags": ["Knowledge Bases"],
789789
"parameters": [
790790
{
@@ -1441,7 +1441,7 @@
14411441
"patch": {
14421442
"operationId": "updateKnowledgeDocument",
14431443
"summary": "Update Document",
1444-
"description": "Rename a document, enable or disable it for search, set its tag slots, or requeue it for processing. Absent fields are unchanged. Only caller-owned fields are accepted: derived indexing state (`chunkCount`, `tokenCount`, `characterCount`, `processingStatus`, `processingError`) is written by the processing pipeline and cannot be asserted here. `retryProcessing: true` re-queues a failed or stuck document and must be sent on its own — it runs instead of, not alongside, the field updates — and it answers with a queue acknowledgement rather than the document. Otherwise the updated document is returned; it omits the connector provenance the detail read carries, so re-read with GET when that is needed.",
1444+
"description": "Rename a document, enable or disable it for search, set any of its 17 tag slots, or requeue it for processing. A tag slot takes its declared type — a string for `tag1`..`tag7`, a number for `number1`..`number5`, a `YYYY-MM-DD` string for `date1`..`date2`, a boolean for `boolean1`..`boolean3` — and a value that is not valid for the slot is a `400` rather than a silently cleared tag. Resolve a display name to its slot with `GET /api/v2/knowledge/{id}/tags`. Absent fields are unchanged. Only caller-owned fields are accepted: derived indexing state (`chunkCount`, `tokenCount`, `characterCount`, `processingStatus`, `processingError`) is written by the processing pipeline and cannot be asserted here. `retryProcessing: true` re-queues a failed or stuck document and must be sent on its own — it runs instead of, not alongside, the field updates — and it answers with a queue acknowledgement rather than the document. Otherwise the updated document is returned; it omits the connector provenance the detail read carries, so re-read with GET when that is needed. A workspace API key cannot call this operation and is rejected with `403`; use a personal API key.",
14451445
"tags": ["Knowledge Bases"],
14461446
"parameters": [
14471447
{
@@ -3031,14 +3031,14 @@
30313031
"examples": [42]
30323032
},
30333033
"documentIds": {
3034+
"description": "Identifiers of the documents the operation changed. Present only for an explicit `documentIds` request, which is bounded to 100 documents; a `selectAll` request omits it because the selection is unbounded, and reports `updatedCount` instead.",
30343035
"type": "array",
30353036
"items": {
30363037
"type": "string"
3037-
},
3038-
"description": "Identifiers of the documents the operation changed."
3038+
}
30393039
}
30403040
},
3041-
"required": ["operation", "updatedCount", "documentIds"],
3041+
"required": ["operation", "updatedCount"],
30423042
"additionalProperties": false,
30433043
"title": "Bulk knowledge document update data",
30443044
"description": "Outcome of a bulk enable or disable across knowledge documents."
@@ -3912,6 +3912,48 @@
39123912
"type": "string",
39133913
"maxLength": 1000
39143914
},
3915+
"number1": {
3916+
"description": "New value for number tag slot 1.",
3917+
"type": "number"
3918+
},
3919+
"number2": {
3920+
"description": "New value for number tag slot 2.",
3921+
"type": "number"
3922+
},
3923+
"number3": {
3924+
"description": "New value for number tag slot 3.",
3925+
"type": "number"
3926+
},
3927+
"number4": {
3928+
"description": "New value for number tag slot 4.",
3929+
"type": "number"
3930+
},
3931+
"number5": {
3932+
"description": "New value for number tag slot 5.",
3933+
"type": "number"
3934+
},
3935+
"date1": {
3936+
"description": "New value for date tag slot 1, formatted YYYY-MM-DD.",
3937+
"type": "string",
3938+
"pattern": "^\\d{4}-\\d{2}-\\d{2}$"
3939+
},
3940+
"date2": {
3941+
"description": "New value for date tag slot 2, formatted YYYY-MM-DD.",
3942+
"type": "string",
3943+
"pattern": "^\\d{4}-\\d{2}-\\d{2}$"
3944+
},
3945+
"boolean1": {
3946+
"description": "New value for boolean tag slot 1.",
3947+
"type": "boolean"
3948+
},
3949+
"boolean2": {
3950+
"description": "New value for boolean tag slot 2.",
3951+
"type": "boolean"
3952+
},
3953+
"boolean3": {
3954+
"description": "New value for boolean tag slot 3.",
3955+
"type": "boolean"
3956+
},
39153957
"retryProcessing": {
39163958
"description": "Requeue the document for processing. Send it alone: no other field may accompany it.",
39173959
"type": "boolean",

apps/docs/openapi-v2-resources.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -623,7 +623,7 @@
623623
"get": {
624624
"operationId": "listMcpServerTools",
625625
"summary": "List MCP Server Tools",
626-
"description": "Connect to a registered MCP server and return the tools it exposes. Unlike most reads this one has side effects: it opens a live connection to the third-party server and writes `connectionStatus`, `toolCount`, `lastError`, and `lastToolsRefresh` on the server resource, so registering a server and then calling this completes onboarding without opening the Sim UI. A `HEAD` request runs the same discovery pass and is answered without a body, so do not use one as a cheap liveness probe. Results are served from a short-lived per-workspace cache, so an uncached call reflects whichever workspace member last ran discovery; pass `refresh=true` to reconnect under your own credentials and pick up tools added since the last pass, at the cost of a live round trip to the server. The set is bounded by discovery itself — at most 1,000 tools and 5 MB of tool payload per server — so it is returned as a single page with `nextCursor` always null. An unreachable, slow, or cooling-down server is a `503`; a server whose stored OAuth grant no longer works is a `409` with `error.details.code` `MCP_SERVER_REAUTHORIZATION_REQUIRED`, meaning the registration is intact but a human must reauthorize it in Sim — your API key is fine and re-issuing it changes nothing. A workspace API key cannot call this operation and is rejected with `403`; use a personal API key. Discovery resolves the calling user's own OAuth credentials for the server, which a workspace key cannot supply — so a workspace key that can register a server cannot list its tools.",
626+
"description": "Connect to a registered MCP server and return the tools it exposes. Unlike most reads this one has side effects: it opens a live connection to the third-party server and writes `connectionStatus`, `toolCount`, `lastError`, and `lastToolsRefresh` on the server resource, so registering a server and then calling this completes onboarding without opening the Sim UI. Because the pass is not a safe read, a `HEAD` request is answered with an empty `200` without connecting or writing, so it reports only that the endpoint exists and the caller is authorized. Results are served from a short-lived per-workspace cache, so an uncached call reflects whichever workspace member last ran discovery; pass `refresh=true` to reconnect under your own credentials and pick up tools added since the last pass, at the cost of a live round trip to the server. The set is bounded by discovery itself — at most 1,000 tools and 5 MB of tool payload per server. The bounded set is returned in one page with `nextCursor` always null; there is no second page to fetch. An unreachable, slow, or cooling-down server is a `503`; a server whose stored OAuth grant no longer works is a `409` with `error.details.code` `MCP_SERVER_REAUTHORIZATION_REQUIRED`, meaning the registration is intact but a human must reauthorize it in Sim — your API key is fine and re-issuing it changes nothing. A workspace API key cannot call this operation and is rejected with `403`; use a personal API key. Discovery resolves the calling user's own OAuth credentials for the server, which a workspace key cannot supply — so a workspace key that can register a server cannot list its tools.",
627627
"tags": ["MCP Servers"],
628628
"parameters": [
629629
{

apps/docs/openapi-v2-tables.json

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1459,7 +1459,7 @@
14591459
"post": {
14601460
"operationId": "countTableRows",
14611461
"summary": "Count Rows",
1462-
"description": "Count the rows matching a typed predicate across the entire table. The paged reads carry no total, and rowCount on the table resource counts every row rather than the predicate matches. Omit the predicate to count the whole table.",
1462+
"description": "Count the rows matching a typed predicate across the entire table. The paged reads carry no total, and rowCount on the table resource counts every row rather than the predicate matches. Omit the predicate to count the whole table. A predicate larger than the request-body ceiling is a `413`.",
14631463
"tags": ["Tables"],
14641464
"parameters": [
14651465
{
@@ -1519,6 +1519,9 @@
15191519
"404": {
15201520
"$ref": "#/components/responses/NotFound"
15211521
},
1522+
"413": {
1523+
"$ref": "#/components/responses/PayloadTooLarge"
1524+
},
15221525
"429": {
15231526
"$ref": "#/components/responses/RateLimited"
15241527
},

apps/docs/openapi-v2-workflows.json

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -644,7 +644,7 @@
644644
},
645645
"/api/v2/workflows/{id}/deployment": {
646646
"get": {
647-
"operationId": "getWorkflowDeploymentV2",
647+
"operationId": "getWorkflowDeployment",
648648
"summary": "Get Workflow Deployment",
649649
"description": "Read the current deployment state of a workflow: whether a version is live, when it went live, the most recent deployment attempt with its readiness and failure payload, and whether the editable draft has since diverged from the live version. This is the only place `needsRedeployment` is published — the deploy, undeploy, and rollback responses cannot carry it, because they answer at the moment the draft and the live version are equal.",
650650
"tags": ["Workflows"],
@@ -1281,24 +1281,24 @@
12811281
"name": "startDate",
12821282
"in": "query",
12831283
"required": false,
1284-
"description": "Include runs started at or after this ISO 8601 timestamp.",
1284+
"description": "Only include runs started at or after this UTC ISO 8601 timestamp, e.g. `2026-08-06T00:00:00Z`. A date without a time, or a timestamp carrying a UTC offset instead of `Z`, is rejected.",
12851285
"schema": {
1286-
"description": "Include runs started at or after this ISO 8601 timestamp.",
12871286
"type": "string",
12881287
"format": "date-time",
1289-
"pattern": "^(?:(?:\\d\\d[2468][048]|\\d\\d[13579][26]|\\d\\d0[48]|[02468][048]00|[13579][26]00)-02-29|\\d{4}-(?:(?:0[13578]|1[02])-(?:0[1-9]|[12]\\d|3[01])|(?:0[469]|11)-(?:0[1-9]|[12]\\d|30)|(?:02)-(?:0[1-9]|1\\d|2[0-8])))T(?:(?:[01]\\d|2[0-3]):[0-5]\\d(?::[0-5]\\d(?:\\.\\d+)?)?(?:Z))$"
1288+
"pattern": "^(?:(?:\\d\\d[2468][048]|\\d\\d[13579][26]|\\d\\d0[48]|[02468][048]00|[13579][26]00)-02-29|\\d{4}-(?:(?:0[13578]|1[02])-(?:0[1-9]|[12]\\d|3[01])|(?:0[469]|11)-(?:0[1-9]|[12]\\d|30)|(?:02)-(?:0[1-9]|1\\d|2[0-8])))T(?:(?:[01]\\d|2[0-3]):[0-5]\\d(?::[0-5]\\d(?:\\.\\d+)?)?(?:Z))$",
1289+
"description": "Only include runs started at or after this UTC ISO 8601 timestamp, e.g. `2026-08-06T00:00:00Z`. A date without a time, or a timestamp carrying a UTC offset instead of `Z`, is rejected."
12901290
}
12911291
},
12921292
{
12931293
"name": "endDate",
12941294
"in": "query",
12951295
"required": false,
1296-
"description": "Include runs started at or before this ISO 8601 timestamp.",
1296+
"description": "Only include runs started at or before this UTC ISO 8601 timestamp, e.g. `2026-08-06T00:00:00Z`. A date without a time, or a timestamp carrying a UTC offset instead of `Z`, is rejected.",
12971297
"schema": {
1298-
"description": "Include runs started at or before this ISO 8601 timestamp.",
12991298
"type": "string",
13001299
"format": "date-time",
1301-
"pattern": "^(?:(?:\\d\\d[2468][048]|\\d\\d[13579][26]|\\d\\d0[48]|[02468][048]00|[13579][26]00)-02-29|\\d{4}-(?:(?:0[13578]|1[02])-(?:0[1-9]|[12]\\d|3[01])|(?:0[469]|11)-(?:0[1-9]|[12]\\d|30)|(?:02)-(?:0[1-9]|1\\d|2[0-8])))T(?:(?:[01]\\d|2[0-3]):[0-5]\\d(?::[0-5]\\d(?:\\.\\d+)?)?(?:Z))$"
1300+
"pattern": "^(?:(?:\\d\\d[2468][048]|\\d\\d[13579][26]|\\d\\d0[48]|[02468][048]00|[13579][26]00)-02-29|\\d{4}-(?:(?:0[13578]|1[02])-(?:0[1-9]|[12]\\d|3[01])|(?:0[469]|11)-(?:0[1-9]|[12]\\d|30)|(?:02)-(?:0[1-9]|1\\d|2[0-8])))T(?:(?:[01]\\d|2[0-3]):[0-5]\\d(?::[0-5]\\d(?:\\.\\d+)?)?(?:Z))$",
1301+
"description": "Only include runs started at or before this UTC ISO 8601 timestamp, e.g. `2026-08-06T00:00:00Z`. A date without a time, or a timestamp carrying a UTC offset instead of `Z`, is rejected."
13021302
}
13031303
},
13041304
{

apps/sim/app/api/v2/files/[fileId]/content/route.test.ts

Lines changed: 26 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,21 @@
11
/**
22
* @vitest-environment node
33
*/
4+
import {
5+
MockV2ApiKeyUnauthenticatedError,
6+
V2_OPERATION_RATE_LIMIT_ALLOWED,
7+
V2_PREAUTH_RATE_LIMIT_ALLOWED,
8+
v2ApiKeyAuthModuleMock,
9+
v2GateModuleMock,
10+
v2RateLimiterModuleMock,
11+
v2RouteMocks,
12+
} from '@sim/testing'
413
import { NextRequest } from 'next/server'
514
import { beforeEach, describe, expect, it, vi } from 'vitest'
615

716
const mocks = vi.hoisted(() => ({
817
admit: vi.fn(),
918
updateContent: vi.fn(),
10-
authenticateV2ApiKey: vi.fn(),
11-
checkRateLimitDirect: vi.fn(),
12-
checkRateLimitDirectOrThrow: vi.fn(),
1319
getUserEmailsByIds: vi.fn(),
1420
}))
1521

@@ -25,22 +31,9 @@ vi.mock('@/lib/workspace-files/application/update-workspace-file-content', () =>
2531
},
2632
}))
2733

28-
vi.mock('@/lib/api/server/routes/v2-api-key-auth', () => ({
29-
authenticateV2ApiKey: mocks.authenticateV2ApiKey,
30-
V2ApiKeyUnauthenticatedError: class V2ApiKeyUnauthenticatedError extends Error {},
31-
}))
32-
33-
vi.mock('@/lib/core/rate-limiter', () => ({
34-
getRateLimit: () => ({ maxTokens: 100, refillRate: 50, refillIntervalMs: 60_000 }),
35-
RateLimiter: class RateLimiter {
36-
checkRateLimitDirect = mocks.checkRateLimitDirect
37-
checkRateLimitDirectOrThrow = mocks.checkRateLimitDirectOrThrow
38-
},
39-
}))
40-
41-
vi.mock('@/app/api/v2/lib/gate', () => ({
42-
v2ApiGateError: vi.fn().mockResolvedValue(null),
43-
}))
34+
vi.mock('@/lib/api/server/routes/v2-api-key-auth', () => v2ApiKeyAuthModuleMock)
35+
vi.mock('@/lib/core/rate-limiter', () => v2RateLimiterModuleMock)
36+
vi.mock('@/app/api/v2/lib/gate', () => v2GateModuleMock)
4437

4538
vi.mock('@/lib/users/queries', () => ({
4639
getUserEmailsByIds: mocks.getUserEmailsByIds,
@@ -94,17 +87,10 @@ const callPut = (body: unknown, contentLength?: number) =>
9487
describe('PUT /api/v2/files/[fileId]/content', () => {
9588
beforeEach(() => {
9689
vi.clearAllMocks()
97-
mocks.authenticateV2ApiKey.mockResolvedValue(auth)
98-
mocks.checkRateLimitDirect.mockResolvedValue({
99-
allowed: true,
100-
remaining: 599,
101-
resetAt: new Date('2024-01-01T01:00:00Z'),
102-
})
103-
mocks.checkRateLimitDirectOrThrow.mockResolvedValue({
104-
allowed: true,
105-
remaining: 99,
106-
resetAt: new Date('2024-01-01T01:00:00Z'),
107-
})
90+
v2RouteMocks.authenticate.mockResolvedValue(auth)
91+
v2RouteMocks.gate.mockResolvedValue(null)
92+
v2RouteMocks.preauthRate.mockResolvedValue(V2_PREAUTH_RATE_LIMIT_ALLOWED)
93+
v2RouteMocks.operationRate.mockResolvedValue(V2_OPERATION_RATE_LIMIT_ALLOWED)
10894
mocks.admit.mockResolvedValue(undefined)
10995
mocks.updateContent.mockResolvedValue({ file: record })
11096
mocks.getUserEmailsByIds.mockResolvedValue(new Map([['user-1', 'ada@example.com']]))
@@ -120,6 +106,15 @@ describe('PUT /api/v2/files/[fileId]/content', () => {
120106
expect(mocks.updateContent).not.toHaveBeenCalled()
121107
})
122108

109+
it('rejects an unauthenticated request', async () => {
110+
v2RouteMocks.authenticate.mockRejectedValueOnce(new MockV2ApiKeyUnauthenticatedError())
111+
112+
const response = await callPut({ workspaceId: WORKSPACE_ID, content: 'id,name\n' })
113+
114+
expect(response.status).toBe(401)
115+
expect((await response.json()).error.code).toBe('UNAUTHORIZED')
116+
})
117+
123118
it('validates body fields after admission', async () => {
124119
const response = await callPut({ workspaceId: WORKSPACE_ID })
125120

@@ -172,7 +167,7 @@ describe('PUT /api/v2/files/[fileId]/content', () => {
172167
},
173168
request,
174169
})
175-
expect(mocks.checkRateLimitDirectOrThrow).toHaveBeenCalledWith(
170+
expect(v2RouteMocks.operationRate).toHaveBeenCalledWith(
176171
'v2:files.update_content:api-key:key-1',
177172
expect.anything()
178173
)

0 commit comments

Comments
 (0)