fix(audits): keep the HEAD-on-GET allowance off defineScimRoute and catch extension-suffixed presenter imports

Author: waleedlatif1

scripts/check-monorepo-boundaries.test.ts

@@ -86,6 +86,7 @@ describe('package boundary command', () => {
         ["export { listWidgetsContract } from '@/lib/api/contracts/widgets'", useCase, false],
         ["import * as contracts from '@/lib/api/contracts/widgets'", useCase, false],
         ["import { presentWidget } from '@/lib/api/server/widget-presenters'", useCase, false],
+        ["import { presentWidget } from '@/lib/api/server/widget-presenters.ts'", useCase, false],
         ["import { run } from '@/lib/mothership/tools/handlers/run-code'", useCase, false],
         ["import {} from '@/lib/mothership/tools/handlers/run-code'", useCase, false],
         ["export {} from '@/lib/api/server/widget-presenters'", useCase, false],

scripts/check-monorepo-boundaries.ts

@@ -67,7 +67,7 @@ function applicationViolation(
   ) {
     return `application code imports a Copilot handler: ${specifier}`
   }
-  if (specifier.startsWith('@/') && /(^|[/-])presenters?$/.test(specifier)) {
+  if (specifier.startsWith('@/') && /(^|[/-])presenters?(\.[cm]?[jt]sx?)?$/.test(specifier)) {
     return `application code imports a presenter: ${specifier}`
   }
   if (matchesModule(specifier, '@/lib/api/contracts')) {

scripts/check-route-verbs.test.ts

@@ -15,7 +15,7 @@ describe('traced route declarations', () => {
 export const POST = (request: NextRequest, context?: Parameters<typeof handler>[1]) =>
   withIncomingGoSpan(request.headers, span, undefined, () => handler(request, context))`
     expect(wrappedRouteSites(source)).toEqual([
-      { verb: 'POST', optionsStart: source.indexOf('{') + 1 },
+      { verb: 'POST', builder: 'defineInternalJsonRoute', optionsStart: source.indexOf('{') + 1 },
     ])
   })
   it('preserves a wrong verb so the contract comparison rejects it', () => {
@@ -30,8 +30,12 @@ const fallback = defineInternalJsonRoute({
 })
 export const POST = enabled ? handler : fallback`
     expect(wrappedRouteSites(source)).toEqual([
-      { verb: 'POST', optionsStart: source.indexOf('{') + 1 },
-      { verb: 'POST', optionsStart: source.indexOf('{', source.indexOf('const fallback')) + 1 },
+      { verb: 'POST', builder: 'defineInternalJsonRoute', optionsStart: source.indexOf('{') + 1 },
+      {
+        verb: 'POST',
+        builder: 'defineInternalJsonRoute',
+        optionsStart: source.indexOf('{', source.indexOf('const fallback')) + 1,
+      },
     ])
   })
   it('checks a builder handler exported through an export list', () => {
@@ -40,7 +44,7 @@ export const POST = enabled ? handler : fallback`
 })
 export { GET }`
     expect(wrappedRouteSites(source)).toEqual([
-      { verb: 'GET', optionsStart: source.indexOf('{') + 1 },
+      { verb: 'GET', builder: 'defineInternalJsonRoute', optionsStart: source.indexOf('{') + 1 },
     ])
   })
 })
@@ -132,6 +136,11 @@ describe('HEAD handlers', () => {
     await checkSite(route, 'raw', 'HEAD', 'readContract', failures, false, loadModule)
     expect(failures).toEqual([])
   })
+  it('rejects HEAD on a GET contract for defineScimRoute, which requires an exact method', async () => {
+    const failures: string[] = []
+    await checkSite(route, 'scim', 'HEAD', 'readContract', failures, false, loadModule)
+    expect(failures).toHaveLength(1)
+  })
   it('still rejects any other verb on a GET contract', async () => {
     const failures: string[] = []
     await checkSite(route, 'raw', 'POST', 'readContract', failures, false, loadModule)

scripts/check-route-verbs.ts

@@ -147,9 +147,11 @@ function topLevelBindings(statements: Statements): {
 }
 
 /** Resolves exported handler references and tracing callbacks without importing server code. */
-export function wrappedRouteSites(source: string): Array<{ verb: string; optionsStart: number }> {
+export function wrappedRouteSites(
+  source: string
+): Array<{ verb: string; builder: string; optionsStart: number }> {
   const statements = parse(source, { sourceType: 'module', plugins: ['typescript'] }).program.body
-  const handlers = new Map<string, number>()
+  const handlers = new Map<string, { builder: string; optionsStart: number }>()
   for (const statement of statements) {
     if (statement.type !== 'VariableDeclaration' || statement.kind !== 'const') continue
     for (const declaration of statement.declarations) {
@@ -159,15 +161,18 @@ export function wrappedRouteSites(source: string): Array<{ verb: string; options
         continue
       const options = call.arguments[0]
       if (options?.type === 'ObjectExpression' && typeof options.start === 'number') {
-        handlers.set(declaration.id.name, options.start + 1)
+        handlers.set(declaration.id.name, {
+          builder: call.callee.name,
+          optionsStart: options.start + 1,
+        })
       }
     }
   }
   const { locals, exports } = topLevelBindings(statements)
-  const sites: Array<{ verb: string; optionsStart: number }> = []
+  const sites: Array<{ verb: string; builder: string; optionsStart: number }> = []
   for (const { exported: verb, local } of exports) {
     if (!isVerb(verb)) continue
-    const found = new Set<number>()
+    const found = new Set<{ builder: string; optionsStart: number }>()
     const direct = handlers.get(local)
     if (direct !== undefined) found.add(direct)
     const visit = (value: unknown): void => {
@@ -178,13 +183,13 @@ export function wrappedRouteSites(source: string): Array<{ verb: string; options
       }
       const node = value as Record<string, unknown>
       if (node.type === 'Identifier' && typeof node.name === 'string') {
-        const start = handlers.get(node.name)
-        if (start !== undefined) found.add(start)
+        const handler = handlers.get(node.name)
+        if (handler !== undefined) found.add(handler)
       }
       Object.values(node).forEach(visit)
     }
     if (direct === undefined) visit(locals.get(local))
-    for (const optionsStart of found) sites.push({ verb, optionsStart })
+    for (const handler of found) sites.push({ verb, ...handler })
   }
   return sites
 }
@@ -347,8 +352,12 @@ function derivedPath(file: string): string {
   return ['/api', ...segments].join('/')
 }
 
-/** Where a contract identifier is bound in a route: a builder's `contract:` key or a raw `parseRequest`. */
-type SiteKind = 'builder' | 'raw'
+/**
+ * Where a contract identifier is bound in a route: a builder's `contract:` key or a raw
+ * `parseRequest`. `scim` is `defineScimRoute`, the one builder that rejects a `HEAD` request
+ * against a `GET` contract instead of serving it through `methodMatchesContract`.
+ */
+type SiteKind = 'builder' | 'scim' | 'raw'
 
 /** One route file, as every site in it is checked against. */
 interface RouteFile {
@@ -426,13 +435,13 @@ export async function checkSite(
   }
 
   const subject =
-    kind === 'builder'
+    kind !== 'raw'
       ? `export const ${verb} is built from \`${identifier}\``
       : `export const ${verb} parses with \`${identifier}\``
   const method = contract.method.toUpperCase()
-  if (method !== verb && !(verb === 'HEAD' && method === 'GET')) {
+  if (method !== verb && !(verb === 'HEAD' && method === 'GET' && kind !== 'scim')) {
     failures.push(
-      kind === 'builder'
+      kind !== 'raw'
         ? `${relative}: ${subject}, which declares ${contract.method} ${contract.path}. Next routes by the exported symbol, so ${verb} requests 500 and ${contract.method} requests 404.`
         : `${relative}: ${subject}, which declares ${contract.method} ${contract.path}. Clients calling through the contract send ${contract.method} and get a 405.`
     )
@@ -493,13 +502,14 @@ async function main() {
 
     const directSites = [...source.matchAll(EXPORT_RE)].map((match) => ({
       verb: match[1],
+      builder: match[2],
       optionsStart: (match.index ?? 0) + match[0].length,
     }))
     const sites =
       directSites.length === builderCalls
         ? directSites
         : [...directSites, ...wrappedRouteSites(source)]
-    for (const { verb, optionsStart } of sites) {
+    for (const { verb, builder, optionsStart } of sites) {
       sitesInFile += 1
       const options = source.slice(optionsStart, optionsStart + OPTIONS_SCAN_CHARS)
 
@@ -509,7 +519,8 @@ async function main() {
         continue
       }
       const identifier = contractKey[1]
-      if (await checkSite(route, 'builder', verb, identifier, failures, verbose)) checked += 1
+      const kind = builder === 'defineScimRoute' ? 'scim' : 'builder'
+      if (await checkSite(route, kind, verb, identifier, failures, verbose)) checked += 1
     }
 
     if (sitesInFile < builderCalls) {