chore(audits): check raw-route parseRequest contracts and enforce surface-neutral application imports by waleedlatif1 · Pull Request #8883 · simstudioai/sim · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .agents/skills/migrate-application-operation/SKILL.md
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ packages/
- Every protected read, write, canonical resource lookup, or authorization-sensitive reference resolution enters through an authorized application use case.
- 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.
- 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.
- 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.
- 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.
- Copilot is a surface adapter. Use `createCopilotApplicationAdapter` and the domain's registered operation object; never a Copilot-only authorization or business implementation.
- Protected compound mutations belong in one top-level semantic application operation, never a sequence of independently committing mutations in a route or tool adapter.
- 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.
Expand Down
3 changes: 2 additions & 1 deletion apps/sim/app/api/knowledge/sim-search/sources/route.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { listSearchSourcesContract } from '@/lib/api/contracts/knowledge'
import { cursorRoute } from '@/lib/api/cursor-binding'
import {
defineInternalJsonRoute,
internalRateLimits,
Expand All @@ -16,7 +17,7 @@ export const GET = defineInternalJsonRoute({
reason: 'Workspace source summaries for the Search page and indexing status polling',
}),
errorPolicy: internalKnowledgeErrorPolicies.connectors,
mapInput: ({ query }) => query,
mapInput: ({ query }) => ({ ...query, cursorRoute: cursorRoute(listSearchSourcesContract) }),
useCase: listSearchSources,
present: (page) => ({ success: true as const, data: page }),
staticResponseHeaders: { 'Cache-Control': 'private, no-store' },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ const sourceIds = Array.from({ length: 105 }, () => generateId())
.sort()
.reverse()
const olderSourceId = generateId()
const input = { workspaceId: ids.workspaceId }
const cursorRoute = { method: 'GET', path: '/api/knowledge/sim-search/sources' }
const input = { workspaceId: ids.workspaceId, cursorRoute }

beforeAll(async () => {
await seedKnowledgeAclFixture(ids)
Expand Down Expand Up @@ -157,8 +158,10 @@ describe('bounded live Search source configuration pagination', () => {
connectorType: 'google_drive',
approved: false,
})
const owner = { organizationId: ids.organizationId }
const summary = await listSearchSources.execute({ principal: alice, input: owner })
const summary = await listSearchSources.execute({
principal: alice,
input: { organizationId: ids.organizationId, cursorRoute },
})
expect(summary.sources[0]).toMatchObject({
connectorId: approvalSourceId,
approved: false,
Expand Down
7 changes: 4 additions & 3 deletions apps/sim/lib/knowledge/application/search-sources.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ workspaceAuthzMockFns.mockPermissionSatisfies.mockImplementation(
)

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

function source(id: string, connectorType = 'google_drive', accessMode = 'admin') {
Expand Down Expand Up @@ -156,7 +157,7 @@ describe('organization Search source summaries', () => {
seed([source('drive')])
const result = await listSearchSources.execute({
principal,
input: { organizationId: 'org-1' },
input: { organizationId: 'org-1', cursorRoute },
})
expect(result.sources[0]).toMatchObject({
connectorId: 'drive',
Expand All @@ -174,7 +175,7 @@ describe('organization Search source summaries', () => {
})
queueTableRows(member, [])
await expect(
listSearchSources.execute({ principal, input: { organizationId: 'org-1' } })
listSearchSources.execute({ principal, input: { organizationId: 'org-1', cursorRoute } })
).rejects.toThrow('Organization not found')
})
})
Expand Down
11 changes: 5 additions & 6 deletions apps/sim/lib/knowledge/application/search-sources.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,8 @@ import { db } from '@sim/db'
import { knowledgeBase, knowledgeConnector } from '@sim/db/schema'
import { toRecord } from '@sim/utils/object'
import { and, desc, eq, inArray, isNull, lt, ne, or, sql } from 'drizzle-orm'
import {
listSearchSourcesContract,
searchSourceCursorSchema,
} from '@/lib/api/contracts/knowledge/connectors'
import { cursorRoute, cursorScopeKey } from '@/lib/api/cursor-binding'
import { searchSourceCursorSchema } from '@/lib/api/contracts/knowledge/connectors'
import { type CursorScopeRoute, cursorScopeKey } from '@/lib/api/cursor-binding'
import { OrchestrationError } from '@/lib/core/orchestration/types'
import { type ResourceOwner, resourceScopeFromOwner } from '@/lib/core/resource-scope'
import { resourceScopeCondition } from '@/lib/core/resource-scope.server'
Expand All @@ -24,6 +21,8 @@ import { describeSearchSource } from '@/lib/sim-search/source-identity'
import { getConnectorMeta } from '@/connectors/registry'

export interface ListSearchSourcesInput extends ResourceOwner {
/** The list endpoint the adapter's cursors belong to; binds every cursor to it. */
cursorRoute: CursorScopeRoute
cursor?: string
connectorId?: string
connectorType?: string
Expand All @@ -41,7 +40,7 @@ export const listSearchSources = defineAuthorizedKnowledgeUseCase({
const search = input.search?.trim().toLowerCase() ?? ''
const connectorType = input.connectorType?.trim()
const excludeConnectorType = input.excludeConnectorType?.trim()
const cursorScope = cursorScopeKey(cursorRoute(listSearchSourcesContract), {
const cursorScope = cursorScopeKey(input.cursorRoute, {
workspaceId: context.workspaceId,
organizationId: context.organizationId,
userId: userId,
Expand Down
22 changes: 20 additions & 2 deletions apps/sim/lib/mothership/tools/server/search-sources.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,15 @@ describe('Search source direct tool', () => {
audience: 'sim:knowledge',
resourceScope: { chatId: 'actual-chat' },
}),
input: { organizationId: 'actual-org', cursor: 'previous' },
input: {
organizationId: 'actual-org',
cursor: 'previous',
cursorRoute: {
method: 'GET',
path: '/api/knowledge/sim-search/sources',
params: undefined,
},
},
})
expect(mocks.chat).toHaveBeenCalledBefore(mocks.list)
})
Expand All @@ -75,7 +83,17 @@ describe('Search source direct tool', () => {
tool.execute({ action: 'get', connectorId: 'foreign' }, context)
).rejects.toMatchObject({ code: 'not_found' })
expect(mocks.list).toHaveBeenCalledWith(
expect.objectContaining({ input: { organizationId: 'actual-org', connectorId: 'foreign' } })
expect.objectContaining({
input: {
organizationId: 'actual-org',
connectorId: 'foreign',
cursorRoute: {
method: 'GET',
path: '/api/knowledge/sim-search/sources',
params: undefined,
},
},
})
)
})
it('returns existing setup UI without creating an index, connecting or changing approval', async () => {
Expand Down
11 changes: 9 additions & 2 deletions apps/sim/lib/mothership/tools/server/search-sources.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { listSearchSourcesContract } from '@/lib/api/contracts/knowledge/connectors'
import {
type OrganizationSearchSourcesInput,
type OrganizationSearchSourcesOutput,
organizationSearchSourcesInputSchema,
organizationSearchSourcesOutputSchema,
} from '@/lib/api/contracts/mothership-search-sources'
import { cursorRoute } from '@/lib/api/cursor-binding'
import { OrchestrationError } from '@/lib/core/orchestration/types'
import { knowledgeDelegationPolicy } from '@/lib/knowledge/application/authorization'
import {
Expand Down Expand Up @@ -47,18 +49,23 @@ export const organizationSearchSourcesServerTool: BaseServerTool<
context?.userStopSignal?.throwIfAborted()
const input = organizationSearchSourcesInputSchema.parse(args)
const owner = { organizationId: trusted.organizationId }
// Shares the Search page's cursor namespace, so a page cursor stays valid on either surface.
const sourcesCursorRoute = cursorRoute(listSearchSourcesContract)
switch (input.action) {
case 'list': {
const { action, ...filters } = input
return {
action,
...(await listSearchSources.execute({ principal, input: { ...owner, ...filters } })),
...(await listSearchSources.execute({
principal,
input: { ...owner, ...filters, cursorRoute: sourcesCursorRoute },
})),
}
}
case 'get': {
const page = await listSearchSources.execute({
principal,
input: { ...owner, connectorId: input.connectorId },
input: { ...owner, connectorId: input.connectorId, cursorRoute: sourcesCursorRoute },
})
const source = page.sources[0]
if (!source) throw new OrchestrationError('not_found', 'Search source not found')
Expand Down
64 changes: 64 additions & 0 deletions scripts/check-monorepo-boundaries.test.ts
Loading
Loading