Skip to content

Commit b6476cc

Browse files
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
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ Do not call shared authorization, principal audit attribution, or `recordAudit`
226226

227227
Inspect legacy orchestration before reusing it. If it already authorizes, audits, notifies, or captures analytics, call a lower-level primitive or remove duplicate responsibility for migrated callers.
228228

229-
Application code must remain surface-neutral. It must not import `app/api/**`, `next/server`, internal/v1/v2 contracts or presenters, or Copilot tool handlers. Return domain values and let each surface presenter project its own wire result.
229+
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. Return domain values and let each surface presenter project its own wire result.
230230

231231
## Adapt internal APIs
232232

‎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
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,68 @@ describe('package boundary command', () => {
6666
rmSync(fixture, { recursive: true, force: true })
6767
}
6868
}, 30_000)
69+
70+
it('keeps application code surface-neutral while allowing shared contract vocabulary', () => {
71+
const fixture = mkdtempSync(path.join(ROOT, 'node_modules/.boundary-audit-'))
72+
try {
73+
mkdirSync(path.join(fixture, 'scripts'))
74+
mkdirSync(path.join(fixture, 'packages'))
75+
mkdirSync(path.join(fixture, 'apps/sim/lib/widgets/application'), { recursive: true })
76+
copyFileSync(
77+
path.join(ROOT, 'scripts/check-monorepo-boundaries.ts'),
78+
path.join(fixture, 'scripts/check-monorepo-boundaries.ts')
79+
)
80+
const useCase = 'apps/sim/lib/widgets/application/use-case.ts'
81+
const cases = [
82+
["import { NextResponse } from 'next/server'", useCase, false],
83+
["import type { NextRequest } from 'next/server'", useCase, false],
84+
["import { GET } from '@/app/api/widgets/route'", useCase, false],
85+
["import { listWidgetsContract } from '@/lib/api/contracts/widgets'", useCase, false],
86+
["export { listWidgetsContract } from '@/lib/api/contracts/widgets'", useCase, false],
87+
["import * as contracts from '@/lib/api/contracts/widgets'", useCase, false],
88+
["import { presentWidget } from '@/lib/api/server/widget-presenters'", useCase, false],
89+
["import { presentWidget } from '@/lib/api/server/widget-presenters.ts'", useCase, false],
90+
["import { run } from '@/lib/mothership/tools/handlers/run-code'", useCase, false],
91+
["import {} from '@/lib/mothership/tools/handlers/run-code'", useCase, false],
92+
["export {} from '@/lib/api/server/widget-presenters'", useCase, false],
93+
["const tool = import('@/lib/mothership/tools/server/widgets')", useCase, false],
94+
["import { listWidgetsContract } from '../../api/contracts/widgets'", useCase, false],
95+
["import { GET } from '../../../app/api/widgets/route'", useCase, false],
96+
["import type { ListWidgetsContract } from '../../api/contracts/widgets'", useCase, true],
97+
["import type { ListWidgetsContract } from '@/lib/api/contracts/widgets'", useCase, true],
98+
["import { type listWidgetsContract } from '@/lib/api/contracts/widgets'", useCase, true],
99+
[
100+
"import { widgetBodySchema, MAX_WIDGETS } from '@/lib/api/contracts/widgets'",
101+
useCase,
102+
true,
103+
],
104+
[
105+
"import type { ServerToolContext } from '@/lib/mothership/tools/server/base-tool'",
106+
useCase,
107+
true,
108+
],
109+
[
110+
"import { NextResponse } from 'next/server'",
111+
'apps/sim/lib/widgets/application/use-case.test.ts',
112+
true,
113+
],
114+
["import { NextResponse } from 'next/server'", 'apps/sim/lib/widgets/routes.ts', true],
115+
] as const
116+
117+
for (const [source, file, allowed] of cases) {
118+
rmSync(path.join(fixture, 'apps/sim/lib/widgets'), { recursive: true, force: true })
119+
mkdirSync(path.join(fixture, 'apps/sim/lib/widgets/application'), { recursive: true })
120+
writeFileSync(path.join(fixture, file), `${source}\n`)
121+
const result = spawnSync(
122+
'bun',
123+
[path.join(fixture, 'scripts/check-monorepo-boundaries.ts')],
124+
{ cwd: fixture, encoding: 'utf8' }
125+
)
126+
expect.soft(result.status, `${file}: ${source}\n${result.stderr}`).toBe(allowed ? 0 : 1)
127+
if (!allowed) expect.soft(result.stderr).toContain(`${file}:1`)
128+
}
129+
} finally {
130+
rmSync(fixture, { recursive: true, force: true })
131+
}
132+
}, 30_000)
69133
})

0 commit comments

Comments
 (0)