Skip to content

Commit 093680d

Browse files
committed
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.
1 parent 9f803f8 commit 093680d

4 files changed

Lines changed: 65 additions & 2 deletions

File tree

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,9 @@ describe('package boundary command', () => {
8888
["import { presentWidget } from '@/lib/api/server/widget-presenters'", useCase, false],
8989
["import { run } from '@/lib/mothership/tools/handlers/run-code'", useCase, false],
9090
["const tool = import('@/lib/mothership/tools/server/widgets')", useCase, false],
91+
["import { listWidgetsContract } from '../../api/contracts/widgets'", useCase, false],
92+
["import { GET } from '../../../app/api/widgets/route'", useCase, false],
93+
["import type { ListWidgetsContract } from '../../api/contracts/widgets'", useCase, true],
9194
["import type { ListWidgetsContract } from '@/lib/api/contracts/widgets'", useCase, true],
9295
["import { type listWidgetsContract } from '@/lib/api/contracts/widgets'", useCase, true],
9396
[

‎scripts/check-monorepo-boundaries.ts‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,19 @@ function matchesModule(specifier: string, module: string): boolean {
3636
return specifier === module || specifier.startsWith(`${module}/`)
3737
}
3838

39+
/**
40+
* A relative specifier rewritten into its `@/…` form against the importing file
41+
* (the `apps/sim` root is `@/`), so the application rules see one spelling.
42+
* Bare and already-aliased specifiers, and relative ones leaving `apps/sim`,
43+
* come back unchanged.
44+
*/
45+
function aliasedSpecifier(file: string, specifier: string): string {
46+
if (!specifier.startsWith('.')) return specifier
47+
const target = path.relative(SIM_DIR, path.resolve(path.dirname(file), specifier))
48+
if (target.startsWith('..') || path.isAbsolute(target)) return specifier
49+
return target ? `@/${target.split(path.sep).join('/')}` : '@'
50+
}
51+
3952
/**
4053
* Why an application file may not take this import edge, or `null` when it may.
4154
* An empty `bindings` list is an edge the compiler erases.
@@ -228,7 +241,7 @@ async function main() {
228241
continue
229242
offenders.push(
230243
...(await scanImports(file, ({ module, bindings }) =>
231-
applicationViolation(module.text, bindings)
244+
applicationViolation(aliasedSpecifier(file, module.text), bindings)
232245
))
233246
)
234247
}

‎scripts/check-route-verbs.test.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,27 @@ export { handler as PUT }`
7373
})
7474
})
7575

76+
describe('aliased parseRequest imports', () => {
77+
it('finds the contract parsed through an aliased parseRequest, so a wrong verb is compared', async () => {
78+
const source = `import { parseRequest as parse } from '@/lib/api/server'
79+
import { updateContract } from '@/lib/api/contracts/things'
80+
export const PUT = withRouteHandler((request) => parse(updateContract, request, {}))`
81+
const route = {
82+
relative: 'route.ts',
83+
bindings: importedNames(source),
84+
expectedPath: '/api/things',
85+
}
86+
const loadModule = () =>
87+
Promise.resolve({ updateContract: { method: 'PATCH', path: '/api/things' } })
88+
const failures: string[] = []
89+
for (const { verb, identifier } of rawRouteContractSites(source)) {
90+
await checkSite(route, 'raw', verb, identifier, failures, false, loadModule)
91+
}
92+
expect(failures).toHaveLength(1)
93+
expect(failures[0]).toContain('which declares PATCH /api/things')
94+
})
95+
})
96+
7697
describe('aliased contract imports', () => {
7798
const source = `import { updateContract as contract } from '@/lib/api/contracts/things'`
7899
const route = {

‎scripts/check-route-verbs.ts‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,30 @@ export function wrappedRouteSites(source: string): Array<{ verb: string; options
189189
return sites
190190
}
191191

192+
/** Modules that export the server-side `parseRequest`. */
193+
const PARSE_REQUEST_MODULES = new Set(['@/lib/api/server', '@/lib/api/server/validation'])
194+
195+
/**
196+
* Local names a module binds to `parseRequest`: `parseRequest` itself, plus
197+
* each alias of an `import { parseRequest as … }` from its real module.
198+
*/
199+
function parseRequestNames(statements: Statements): Set<string> {
200+
const names = new Set(['parseRequest'])
201+
for (const statement of statements) {
202+
if (statement.type !== 'ImportDeclaration') continue
203+
if (!PARSE_REQUEST_MODULES.has(statement.source.value)) continue
204+
for (const specifier of statement.specifiers) {
205+
if (specifier.type !== 'ImportSpecifier') continue
206+
const imported =
207+
specifier.imported.type === 'Identifier'
208+
? specifier.imported.name
209+
: specifier.imported.value
210+
if (imported === 'parseRequest') names.add(specifier.local.name)
211+
}
212+
}
213+
return names
214+
}
215+
192216
/**
193217
* The `parseRequest(<identifier>, …)` contract arguments reachable from each
194218
* exported verb of a raw route, following same-file functions and consts the
@@ -199,6 +223,7 @@ export function wrappedRouteSites(source: string): Array<{ verb: string; options
199223
export function rawRouteContractSites(source: string): Array<{ verb: string; identifier: string }> {
200224
const statements = parse(source, { sourceType: 'module', plugins: ['typescript'] }).program.body
201225
const { locals, exports } = topLevelBindings(statements)
226+
const parsers = parseRequestNames(statements)
202227
const exportedLocals = new Set(exports.map(({ local }) => local))
203228
const sites: Array<{ verb: string; identifier: string }> = []
204229
for (const { exported: verb, local } of exports) {
@@ -217,7 +242,8 @@ export function rawRouteContractSites(source: string): Array<{ verb: string; ide
217242
const [first] = node.arguments as Array<{ type?: string; name?: string }>
218243
if (
219244
callee.type === 'Identifier' &&
220-
callee.name === 'parseRequest' &&
245+
callee.name !== undefined &&
246+
parsers.has(callee.name) &&
221247
first?.type === 'Identifier' &&
222248
first.name
223249
) {

0 commit comments

Comments
 (0)