Skip to content

Commit 4d1d130

Browse files
committed
test: cover four untested behaviors and drop five tests that cannot fail
Adds coverage that goes red when the behavior is reverted: - `rejectDuplicateQueryValues` through `parseRequest`, not just the pure helper — the existing blank-query tests stay green even when parseRequest ignores the flag entirely. - `failUndispatchedDocumentProcessing`'s pending + not-deleted WHERE guard, asserted on the condition tree so removing it fails. - The widened `present(result, request)` signature, so dropping the second argument stops being a silent no-op. - The NUL scan on `readFormDataWithLimit`'s content-length branch — the branch every ordinary browser and curl upload takes, and the one the existing multipart tests never reached. Removes tests verified incapable of failing: the credentials projection row (the outbound `.parse()` strips unknown keys either way), the per-document 413 sweep (vacuous on two of three documents, subsumed by the sweep in scripts/openapi/documents.test.ts), the two upload-session rows that assert their own `generateWorkspaceFileKey` stub, the storage-key row whose 20-byte name never reaches the budget, and the views-lock assertion against a function `views/service.ts` does not import.
1 parent 586e246 commit 4d1d130

9 files changed

Lines changed: 255 additions & 155 deletions

File tree

apps/sim/app/api/v2/credentials/route.test.ts

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -201,32 +201,6 @@ describe('GET /api/v2/credentials', () => {
201201
expect(JSON.stringify(body)).not.toContain('createdBy')
202202
})
203203

204-
/**
205-
* The projection is an explicit field-by-field copy, which is what makes a
206-
* column added to the credential table later inert here: a field nobody wrote
207-
* into `toV2Credential` is simply never read. The outbound response `.parse()`
208-
* strips whatever survives, so a leak needs two independent mistakes. This
209-
* pins the pairing against a row carrying a column the projection has never
210-
* heard of.
211-
*/
212-
it('withholds a credential column the projection was never taught to publish', async () => {
213-
mocks.execute.mockResolvedValueOnce({
214-
credentials: [{ ...credential, encryptedFutureSecret: 'MUST_NOT_LEAK_EITHER' }],
215-
nextCursorKeys: null,
216-
sortBy: 'createdAt',
217-
sortOrder: 'desc',
218-
})
219-
220-
const response = await GET(
221-
new NextRequest(`http://localhost:3000/api/v2/credentials?workspaceId=${WORKSPACE_ID}`)
222-
)
223-
const body = await response.json()
224-
225-
expect(response.status).toBe(200)
226-
expect(JSON.stringify(body)).not.toContain('encryptedFutureSecret')
227-
expect(JSON.stringify(body)).not.toContain('MUST_NOT_LEAK_EITHER')
228-
})
229-
230204
it('hides repository errors that may contain secret details', async () => {
231205
mocks.execute.mockRejectedValueOnce(new Error('encryptedServiceAccountKey failed'))
232206

apps/sim/lib/api/contracts/v2/openapi/resources.test.ts

Lines changed: 0 additions & 54 deletions
This file was deleted.

apps/sim/lib/api/server/blank-query-values.test.ts

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,17 @@
11
/**
22
* @vitest-environment node
33
*/
4+
import { NextRequest } from 'next/server'
45
import { describe, expect, it } from 'vitest'
5-
import { blankQueryValueValidationError } from '@/lib/api/server/blank-query-values'
6+
import { z } from 'zod'
7+
import { defineRouteContract } from '@/lib/api/contracts'
8+
import {
9+
blankQueryValueValidationError,
10+
duplicateQueryValueValidationError,
11+
} from '@/lib/api/server/blank-query-values'
612
import { V2_PARSE_DEFAULTS } from '@/lib/api/server/routes/v2-json-route'
13+
import { parseRequest } from '@/lib/api/server/validation'
14+
import { v2ValidationError } from '@/app/api/v2/lib/response'
715

816
/**
917
* A query parameter that is present but blank is a different request from one
@@ -51,3 +59,71 @@ describe('blank query values', () => {
5159
expect(V2_PARSE_DEFAULTS.rejectBlankQueryValues).toBe(true)
5260
})
5361
})
62+
63+
const listContract = defineRouteContract({
64+
method: 'GET',
65+
path: '/api/v2/widgets',
66+
query: z.object({ workspaceId: z.string().min(1, 'Workspace ID is required') }),
67+
response: { mode: 'json', schema: z.object({ data: z.array(z.string()) }) },
68+
})
69+
70+
function listRequest(search: string): NextRequest {
71+
return new NextRequest(`http://localhost/api/v2/widgets?${search}`, { method: 'GET' })
72+
}
73+
74+
async function parseListRequest(search: string) {
75+
return parseRequest(
76+
listContract,
77+
listRequest(search),
78+
{},
79+
{
80+
...V2_PARSE_DEFAULTS,
81+
validationErrorResponse: v2ValidationError,
82+
}
83+
)
84+
}
85+
86+
/**
87+
* A repeated parameter reaches the schema as an array, and no v2 query param is
88+
* declared as one — so without this rule the caller is told the param is
89+
* *missing* for a request that plainly sent it twice.
90+
*/
91+
describe('duplicate query values', () => {
92+
it('names the duplication rather than the schema type failure', () => {
93+
const error = duplicateQueryValueValidationError({ workspaceId: ['w-1', 'w-1'] })
94+
95+
expect(error?.issues[0]).toMatchObject({
96+
path: ['workspaceId'],
97+
message: 'workspaceId was sent 2 times; send it at most once',
98+
})
99+
})
100+
101+
it('accepts a query where every parameter appears once', () => {
102+
expect(duplicateQueryValueValidationError({ workspaceId: 'w-1', limit: '10' })).toBeNull()
103+
})
104+
105+
it('rejects a repeated parameter through parseRequest under the v2 defaults', async () => {
106+
const parsed = await parseListRequest('workspaceId=w-1&workspaceId=w-1')
107+
108+
expect(parsed.success).toBe(false)
109+
if (parsed.success) return
110+
expect(parsed.response.status).toBe(400)
111+
await expect(parsed.response.json()).resolves.toMatchObject({
112+
error: expect.objectContaining({
113+
message: expect.stringContaining('workspaceId was sent 2 times; send it at most once'),
114+
}),
115+
})
116+
})
117+
118+
it('lets a query sending each parameter once through parseRequest', async () => {
119+
const parsed = await parseListRequest('workspaceId=w-1')
120+
121+
expect(parsed.success).toBe(true)
122+
if (!parsed.success) return
123+
expect(parsed.data.query).toEqual({ workspaceId: 'w-1' })
124+
})
125+
126+
it('is on for every v2 route through the shared parse defaults', () => {
127+
expect(V2_PARSE_DEFAULTS.rejectDuplicateQueryValues).toBe(true)
128+
})
129+
})

apps/sim/lib/api/server/routes/v2-json-route.test.ts

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -644,3 +644,73 @@ describe('defineV2JsonRoute HEAD on a route that is not head-safe', () => {
644644
).rejects.toThrow(/authorize/)
645645
})
646646
})
647+
648+
const presenterContract = defineRouteContract({
649+
method: 'POST',
650+
path: '/api/v2/widgets/[widgetId]/pages',
651+
params: z.object({ widgetId: z.string() }).strict(),
652+
query: z.object({ sort: z.string(), workspaceId: z.string() }).strict(),
653+
body: z.object({ value: z.string() }).strict(),
654+
response: {
655+
mode: 'json',
656+
status: 201,
657+
schema: z.object({ data: z.object({ value: z.string() }), nextCursor: z.string() }),
658+
},
659+
})
660+
661+
/**
662+
* A `nextCursor` is stamped with the sort and filters the page was read under,
663+
* and those live in the request rather than the domain result — so a presenter
664+
* that cannot see the parsed request forces the use case to carry an HTTP
665+
* cursor-encoding concern back out.
666+
*/
667+
describe('defineV2JsonRoute presentation', () => {
668+
beforeEach(() => {
669+
vi.clearAllMocks()
670+
v2RouteMocks.authenticate.mockResolvedValue(auth)
671+
v2RouteMocks.gate.mockResolvedValue(null)
672+
v2RouteMocks.preauthRate.mockResolvedValue({ allowed: true, remaining: 599, resetAt })
673+
v2RouteMocks.operationRate.mockResolvedValue(allowedRate)
674+
})
675+
676+
it('hands the presenter the parsed request alongside the result', async () => {
677+
const present = vi.fn((result: Result, parsed: ParsedRequest<typeof presenterContract>) => ({
678+
data: result,
679+
nextCursor: `${parsed.params.widgetId}:${parsed.query.sort}:${parsed.body.value}`,
680+
}))
681+
682+
const handler = defineV2JsonRoute({
683+
contract: presenterContract,
684+
auth: v2ApiKeyAuth,
685+
operation,
686+
rateLimit: v2RateLimits.publicApi,
687+
errorPolicy: v2OrchestrationErrorPolicy,
688+
mapInput: ({ body }) => body,
689+
useCase: { operation, execute: async ({ input }) => input },
690+
present,
691+
})
692+
693+
const response = await handler(
694+
new NextRequest('http://localhost/api/v2/widgets/widget-1/pages?sort=asc&workspaceId=ws-1', {
695+
method: 'POST',
696+
headers: { 'content-type': 'application/json', 'x-api-key': 'secret' },
697+
body: JSON.stringify({ value: 'ok' }),
698+
}),
699+
{ params: Promise.resolve({ widgetId: 'widget-1' }) }
700+
)
701+
702+
expect(response.status).toBe(201)
703+
await expect(response.json()).resolves.toEqual({
704+
data: { value: 'ok' },
705+
nextCursor: 'widget-1:asc:ok',
706+
})
707+
expect(present).toHaveBeenCalledWith(
708+
{ value: 'ok' },
709+
expect.objectContaining({
710+
params: { widgetId: 'widget-1' },
711+
query: { sort: 'asc', workspaceId: 'ws-1' },
712+
body: { value: 'ok' },
713+
})
714+
)
715+
})
716+
})

apps/sim/lib/core/utils/stream-limits.test.ts

Lines changed: 47 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -36,20 +36,37 @@ function streamFromChunks(chunks: Uint8Array[]): ReadableStream<Uint8Array> {
3636
* a filename on serialization, so a hand-written part is the only way to put
3737
* the byte on the wire exactly as a real client can.
3838
*/
39-
function multipartRequest(disposition: string, value: string): Request {
39+
function multipartRequest(
40+
disposition: string,
41+
value: string,
42+
options: { declareContentLength?: boolean } = {}
43+
): Request {
4044
const boundary = 'streamlimitsboundary'
4145
const body =
4246
`--${boundary}\r\n` +
4347
`Content-Disposition: form-data; ${disposition}\r\n` +
4448
`Content-Type: text/plain\r\n\r\n${value}\r\n` +
4549
`--${boundary}--\r\n`
50+
const bytes = new TextEncoder().encode(body)
51+
const requestHeaders = new Headers({
52+
'content-type': `multipart/form-data; boundary=${boundary}`,
53+
})
54+
if (options.declareContentLength) {
55+
requestHeaders.set('content-length', String(bytes.byteLength))
56+
}
4657
return new Request('http://localhost/upload', {
4758
method: 'POST',
48-
headers: { 'content-type': `multipart/form-data; boundary=${boundary}` },
49-
body: new TextEncoder().encode(body),
59+
headers: requestHeaders,
60+
body: bytes,
5061
})
5162
}
5263

64+
const NUL_MULTIPART_PARTS = [
65+
['a NUL in a file name', 'name="file"; filename="apitest_\u0000x.txt"', 'hello'],
66+
['a NUL in a text field value', 'name="label"', 'apitest_\u0000x'],
67+
['a NUL in a field name', 'name="apitest_\u0000x"', 'hello'],
68+
] as const
69+
5370
function headers(contentLength?: string): Headers {
5471
const headers = new Headers()
5572
if (contentLength !== undefined) headers.set('content-length', contentLength)
@@ -217,18 +234,33 @@ describe('stream limits', () => {
217234
expect(formData.get('name')).toBe('example')
218235
})
219236

220-
it.each([
221-
['a NUL in a file name', 'name="file"; filename="apitest_\u0000x.txt"', 'hello'],
222-
['a NUL in a text field value', 'name="label"', 'apitest_\u0000x'],
223-
['a NUL in a field name', 'name="apitest_\u0000x"', 'hello'],
224-
])('rejects multipart form data carrying %s', async (_label, disposition, value) => {
225-
await expect(
226-
readFormDataWithLimit(multipartRequest(disposition, value), {
227-
maxBytes: 1024 * 1024,
228-
label: 'multipart body',
229-
})
230-
).rejects.toBeInstanceOf(MultipartFieldValidationError)
231-
})
237+
it.each(NUL_MULTIPART_PARTS)(
238+
'rejects a streamed multipart body carrying %s',
239+
async (_label, disposition, value) => {
240+
await expect(
241+
readFormDataWithLimit(multipartRequest(disposition, value), {
242+
maxBytes: 1024 * 1024,
243+
label: 'multipart body',
244+
})
245+
).rejects.toBeInstanceOf(MultipartFieldValidationError)
246+
}
247+
)
248+
249+
/**
250+
* A declared `content-length` takes the reader's other branch — the one every
251+
* ordinary browser and curl upload takes — and it scans fields separately.
252+
*/
253+
it.each(NUL_MULTIPART_PARTS)(
254+
'rejects a content-length multipart body carrying %s',
255+
async (_label, disposition, value) => {
256+
const request = multipartRequest(disposition, value, { declareContentLength: true })
257+
expect(request.headers.get('content-length')).not.toBeNull()
258+
259+
await expect(
260+
readFormDataWithLimit(request, { maxBytes: 1024 * 1024, label: 'multipart body' })
261+
).rejects.toBeInstanceOf(MultipartFieldValidationError)
262+
}
263+
)
232264

233265
it('rejects multipart streams without content-length once bytes exceed the limit', async () => {
234266
const request = new Request('http://localhost/upload', {

0 commit comments

Comments
 (0)