chore(audits): check raw-route parseRequest contracts and enforce sur… · simstudioai/sim@b6476cc · GitHub
Skip to content

Commit b6476cc

Browse files
waleedlatif1waleedlatif1
authored
chore(audits): check raw-route parseRequest contracts and enforce surface-neutral application imports (#8883)
* chore(audits): check raw-route parseRequest contracts and enforce surface-neutral application imports check:route-verbs now resolves the contract each raw withRouteHandler route passes to parseRequest and checks the exported verb and path against it (322 sites across 258 files, previously unchecked). check:boundaries now bans next/server and app/api imports, and runtime route-contract, presenter and Copilot-handler imports, from application code. listSearchSources takes its cursor route from the adapter instead of importing the route contract. * fix(tests): pass cursorRoute in the org source-summary integration case * fix(audits): resolve aliased contract imports and export-list verbs in check:route-verbs Read a contract by the name its module exports, not the local alias, and read handler verbs from export lists (export { GET }, export { h as GET }) as well as inline exports. Both builder and raw parseRequest sites share the fix. * fix(audits): follow aliased parseRequest imports and relative application imports check:route-verbs matched raw parseRequest calls only by the literal name, so `import { parseRequest as parse }` left the contract unchecked; it now matches every local name bound to parseRequest from @/lib/api/server(/validation). check:boundaries applied the application rules only to @/ specifiers, so a relative import of a contract object, presenter, Copilot handler or app/api module passed; relative specifiers are now normalized to their @/ form first. * fix(audits): accept HEAD on a GET contract and treat empty import/export clauses as runtime edges * fix(audits): keep the HEAD-on-GET allowance off defineScimRoute and catch extension-suffixed presenter imports * fix(audits): follow exported non-verb helpers when resolving raw-route contracts
1 parent b2cd2a9 commit b6476cc

12 files changed

Lines changed: 755 additions & 176 deletions

File tree

‎.agents/skills/migrate-application-operation/SKILL.md‎

Lines changed: 1 addition & 1 deletion

‎CLAUDE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ packages/
7272
- Every protected read, write, canonical resource lookup, or authorization-sensitive reference resolution enters through an authorized application use case.
7373
- Define one stable semantic operation with its minimum role, workspace-key policy, allowed principal kinds, and delegated services. Internal APIs, v2 APIs, Copilot, and trusted tools call the same use case when the domain behavior is the same.
7474
- Surface adapters authenticate and construct a `Principal`, apply request-rate policy, parse contracts, map input, and present their own result. They never query protected data, decide resource authorization, implement business transactions, or record semantic audit.
75-
- Application use cases load canonical context, compare asserted scope, authorize current access, execute managers/repositories, project semantic audit, and trigger shared domain effects. Managers accept canonical IDs and scope, never credentials or principals. Application code stays surface-neutral: it never imports `app/api/**`, `next/server`, route contracts/presenters, or Copilot handlers.
75+
- Application use cases load canonical context, compare asserted scope, authorize current access, execute managers/repositories, project semantic audit, and trigger shared domain effects. Managers accept canonical IDs and scope, never credentials or principals. Application code stays surface-neutral (`check:boundaries`): never `next/server` or `app/api/**`, and never a runtime import of a route contract object, presenter, or Copilot handler (contract types, schemas, and constants are fine); a surface fact, such as a cursor's route, comes in as input.
7676
- Copilot is a surface adapter. Use `createCopilotApplicationAdapter` and the domain's registered operation object; never a Copilot-only authorization or business implementation.
7777
- Protected compound mutations belong in one top-level semantic application operation, never a sequence of independently committing mutations in a route or tool adapter.
7878
- Never substitute a billing owner, uploader, creator, or API-key owner for the acting principal. Fail fast when the identity model or operation policy cannot express the caller.

‎apps/sim/app/api/knowledge/sim-search/sources/route.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { listSearchSourcesContract } from '@/lib/api/contracts/knowledge'
2+
import { cursorRoute } from '@/lib/api/cursor-binding'
23
import {
34
defineInternalJsonRoute,
45
internalRateLimits,
@@ -16,7 +17,7 @@ export const GET = defineInternalJsonRoute({
1617
reason: 'Workspace source summaries for the Search page and indexing status polling',
1718
}),
1819
errorPolicy: internalKnowledgeErrorPolicies.connectors,
19-
mapInput: ({ query }) => query,
20+
mapInput: ({ query }) => ({ ...query, cursorRoute: cursorRoute(listSearchSourcesContract) }),
2021
useCase: listSearchSources,
2122
present: (page) => ({ success: true as const, data: page }),
2223
staticResponseHeaders: { 'Cache-Control': 'private, no-store' },

‎apps/sim/lib/knowledge/__integration__/search-source-pagination.integration.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ const sourceIds = Array.from({ length: 105 }, () => generateId())
2323
.sort()
2424
.reverse()
2525
const olderSourceId = generateId()
26-
const input = { workspaceId: ids.workspaceId }
26+
const cursorRoute = { method: 'GET', path: '/api/knowledge/sim-search/sources' }
27+
const input = { workspaceId: ids.workspaceId, cursorRoute }
2728

2829
beforeAll(async () => {
2930
await seedKnowledgeAclFixture(ids)
@@ -157,8 +158,10 @@ describe('bounded live Search source configuration pagination', () => {
157158
connectorType: 'google_drive',
158159
approved: false,
159160
})
160-
const owner = { organizationId: ids.organizationId }
161-
const summary = await listSearchSources.execute({ principal: alice, input: owner })
161+
const summary = await listSearchSources.execute({
162+
principal: alice,
163+
input: { organizationId: ids.organizationId, cursorRoute },
164+
})
162165
expect(summary.sources[0]).toMatchObject({
163166
connectorId: approvalSourceId,
164167
approved: false,

‎apps/sim/lib/knowledge/application/search-sources.test.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,8 @@ workspaceAuthzMockFns.mockPermissionSatisfies.mockImplementation(
4747
)
4848

4949
const principal = createSessionPrincipal({ userId: 'reader', sessionId: 'session' })
50-
const input = { workspaceId: 'workspace' }
50+
const cursorRoute = { method: 'GET', path: '/api/knowledge/sim-search/sources' }
51+
const input = { workspaceId: 'workspace', cursorRoute }
5152
const LAST_SYNC = new Date('2026-09-05T12:00:00.000Z')
5253

5354
function source(id: string, connectorType = 'google_drive', accessMode = 'admin') {
@@ -156,7 +157,7 @@ describe('organization Search source summaries', () => {
156157
seed([source('drive')])
157158
const result = await listSearchSources.execute({
158159
principal,
159-
input: { organizationId: 'org-1' },
160+
input: { organizationId: 'org-1', cursorRoute },
160161
})
161162
expect(result.sources[0]).toMatchObject({
162163
connectorId: 'drive',
@@ -174,7 +175,7 @@ describe('organization Search source summaries', () => {
174175
})
175176
queueTableRows(member, [])
176177
await expect(
177-
listSearchSources.execute({ principal, input: { organizationId: 'org-1' } })
178+
listSearchSources.execute({ principal, input: { organizationId: 'org-1', cursorRoute } })
178179
).rejects.toThrow('Organization not found')
179180
})
180181
})

‎apps/sim/lib/knowledge/application/search-sources.ts‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,8 @@ import { db } from '@sim/db'
33
import { knowledgeBase, knowledgeConnector } from '@sim/db/schema'
44
import { toRecord } from '@sim/utils/object'
55
import { and, desc, eq, inArray, isNull, lt, ne, or, sql } from 'drizzle-orm'
6-
import {
7-
listSearchSourcesContract,
8-
searchSourceCursorSchema,
9-
} from '@/lib/api/contracts/knowledge/connectors'
10-
import { cursorRoute, cursorScopeKey } from '@/lib/api/cursor-binding'
6+
import { searchSourceCursorSchema } from '@/lib/api/contracts/knowledge/connectors'
7+
import { type CursorScopeRoute, cursorScopeKey } from '@/lib/api/cursor-binding'
118
import { OrchestrationError } from '@/lib/core/orchestration/types'
129
import { type ResourceOwner, resourceScopeFromOwner } from '@/lib/core/resource-scope'
1310
import { resourceScopeCondition } from '@/lib/core/resource-scope.server'
@@ -24,6 +21,8 @@ import { describeSearchSource } from '@/lib/sim-search/source-identity'
2421
import { getConnectorMeta } from '@/connectors/registry'
2522

2623
export interface ListSearchSourcesInput extends ResourceOwner {
24+
/** The list endpoint the adapter's cursors belong to; binds every cursor to it. */
25+
cursorRoute: CursorScopeRoute
2726
cursor?: string
2827
connectorId?: string
2928
connectorType?: string
@@ -41,7 +40,7 @@ export const listSearchSources = defineAuthorizedKnowledgeUseCase({
4140
const search = input.search?.trim().toLowerCase() ?? ''
4241
const connectorType = input.connectorType?.trim()
4342
const excludeConnectorType = input.excludeConnectorType?.trim()
44-
const cursorScope = cursorScopeKey(cursorRoute(listSearchSourcesContract), {
43+
const cursorScope = cursorScopeKey(input.cursorRoute, {
4544
workspaceId: context.workspaceId,
4645
organizationId: context.organizationId,
4746
userId: userId,

‎apps/sim/lib/mothership/tools/server/search-sources.test.ts‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,15 @@ describe('Search source direct tool', () => {
6868
audience: 'sim:knowledge',
6969
resourceScope: { chatId: 'actual-chat' },
7070
}),
71-
input: { organizationId: 'actual-org', cursor: 'previous' },
71+
input: {
72+
organizationId: 'actual-org',
73+
cursor: 'previous',
74+
cursorRoute: {
75+
method: 'GET',
76+
path: '/api/knowledge/sim-search/sources',
77+
params: undefined,
78+
},
79+
},
7280
})
7381
expect(mocks.chat).toHaveBeenCalledBefore(mocks.list)
7482
})
@@ -77,7 +85,17 @@ describe('Search source direct tool', () => {
7785
tool.execute({ action: 'get', connectorId: 'foreign' }, context)
7886
).rejects.toMatchObject({ code: 'not_found' })
7987
expect(mocks.list).toHaveBeenCalledWith(
80-
expect.objectContaining({ input: { organizationId: 'actual-org', connectorId: 'foreign' } })
88+
expect.objectContaining({
89+
input: {
90+
organizationId: 'actual-org',
91+
connectorId: 'foreign',
92+
cursorRoute: {
93+
method: 'GET',
94+
path: '/api/knowledge/sim-search/sources',
95+
params: undefined,
96+
},
97+
},
98+
})
8199
)
82100
})
83101
it('returns existing setup UI without creating an index, connecting or changing approval', async () => {

‎apps/sim/lib/mothership/tools/server/search-sources.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
1+
import { listSearchSourcesContract } from '@/lib/api/contracts/knowledge/connectors'
12
import {
23
type OrganizationSearchSourcesInput,
34
type OrganizationSearchSourcesOutput,
45
organizationSearchSourcesInputSchema,
56
organizationSearchSourcesOutputSchema,
67
} from '@/lib/api/contracts/mothership-search-sources'
8+
import { cursorRoute } from '@/lib/api/cursor-binding'
79
import { OrchestrationError } from '@/lib/core/orchestration/types'
810
import { knowledgeDelegationPolicy } from '@/lib/knowledge/application/authorization'
911
import {
@@ -47,18 +49,23 @@ export const organizationSearchSourcesServerTool: BaseServerTool<
4749
context?.userStopSignal?.throwIfAborted()
4850
const input = organizationSearchSourcesInputSchema.parse(args)
4951
const owner = { organizationId: trusted.organizationId }
52+
// Shares the Search page's cursor namespace, so a page cursor stays valid on either surface.
53+
const sourcesCursorRoute = cursorRoute(listSearchSourcesContract)
5054
switch (input.action) {
5155
case 'list': {
5256
const { action, ...filters } = input
5357
return {
5458
action,
55-
...(await listSearchSources.execute({ principal, input: { ...owner, ...filters } })),
59+
...(await listSearchSources.execute({
60+
principal,
61+
input: { ...owner, ...filters, cursorRoute: sourcesCursorRoute },
62+
})),
5663
}
5764
}
5865
case 'get': {
5966
const page = await listSearchSources.execute({
6067
principal,
61-
input: { ...owner, connectorId: input.connectorId },
68+
input: { ...owner, connectorId: input.connectorId, cursorRoute: sourcesCursorRoute },
6269
})
6370
const source = page.sources[0]
6471
if (!source) throw new OrchestrationError('not_found', 'Search source not found')

‎scripts/check-monorepo-boundaries.test.ts‎

Lines changed: 64 additions & 0 deletions

0 commit comments

Comments
 (0)