Skip to content

Commit 1250943

Browse files
committed
fix(uploads): drop the stray 'use server' directive that enables Server Actions app-wide
`file-utils.server.ts` was the repo's only `'use server'` module, and the sole reason Next's `hasServerActions()` returned true. With actions registered, Next loses its early-404 escape hatch for Server Action requests — and it classifies a request as an action from headers alone, with no body inspection and no auth. Any unauthenticated `POST` with `Content-Type: multipart/form-data` to any App Router path therefore took the non-fetch action path, which bare-throws and surfaces as an HTTP 500. Nothing invokes these functions as Server Actions: every one of the ~77 importers is server-side, with zero `'use client'` importers. The directive was a misuse of `'use server'` where "server-only module" was meant — the `.server.ts` suffix already carries that convention. Extends check-client-boundary-imports.ts to fail on any `'use server'` directive so this cannot regress.
1 parent 2ba4556 commit 1250943

2 files changed

Lines changed: 88 additions & 30 deletions

File tree

apps/sim/lib/uploads/utils/file-utils.server.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
'use server'
2-
31
import { createLogger, type Logger } from '@sim/logger'
42
import { getErrorMessage } from '@sim/utils/errors'
53
import { getMaxExecutionTimeout } from '@/lib/core/execution-limits'

scripts/check-client-boundary-imports.ts

Lines changed: 88 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,25 @@
11
#!/usr/bin/env bun
22
/**
3-
* Guards against the Next.js `'use client'` server-import foot-gun.
3+
* Guards the two Next.js boundary directives: `'use client'` imports and any
4+
* `'use server'` module.
5+
*
6+
* ## `'use server'`
7+
*
8+
* A single `'use server'` module anywhere in the graph flips Next's
9+
* `hasServerActions()` to true, which removes the early 404 for Server Action
10+
* requests. Next classifies a request as a Server Action from HEADERS ALONE —
11+
* no body inspection, no auth — so once actions exist, ANY unauthenticated
12+
* `POST` with `Content-Type: multipart/form-data` to ANY App Router path takes
13+
* the non-fetch action path, which bare-`throw`s and surfaces as an HTTP 500.
14+
* A trickle of such requests is enough to trip the ALB 5xx alarm. Every export
15+
* of a `'use server'` module is also a remotely invocable, unauthenticated
16+
* endpoint.
17+
*
18+
* Sim has no Server Actions — server-only modules use the `.server.ts` suffix
19+
* and are called directly from route handlers. If you genuinely need a Server
20+
* Action, remove this check deliberately and wrap every export in auth.
21+
*
22+
* ## `'use client'`
423
*
524
* Next.js rewrites EVERY export of a `'use client'` module into a client
625
* reference in the server bundle. Server-evaluated code can only *render* such
@@ -36,6 +55,8 @@ import path from 'node:path'
3655

3756
const ROOT = path.resolve(import.meta.dir, '..')
3857
const APP_DIR = path.join(ROOT, 'apps/sim')
58+
/** Everything Next compiles into the app's module graph. */
59+
const DIRECTIVE_SCAN_DIRS = [path.join(ROOT, 'apps'), path.join(ROOT, 'packages')]
3960

4061
/** Server-evaluated, non-JSX surfaces. A file matches if its path passes one. */
4162
function isServerSurface(rel: string): boolean {
@@ -69,32 +90,54 @@ async function listFiles(dir: string): Promise<string[]> {
6990
return out
7091
}
7192

72-
const useClientCache = new Map<string, boolean>()
73-
74-
async function isUseClientModule(absFile: string): Promise<boolean> {
75-
const cached = useClientCache.get(absFile)
76-
if (cached !== undefined) return cached
77-
let content: string
78-
try {
79-
content = await readFile(absFile, 'utf8')
80-
} catch {
81-
useClientCache.set(absFile, false)
82-
return false
83-
}
84-
// The directive must be the first statement (comments/blank lines may precede it).
85-
let isClient = false
93+
/**
94+
* Returns the module's leading directive prologue string, if any. A directive
95+
* must be the first statement; comments and blank lines may precede it.
96+
*/
97+
function leadingDirective(content: string): string | null {
8698
for (const raw of content.split('\n')) {
8799
const line = raw.trim()
88100
if (line === '' || line.startsWith('//') || line.startsWith('/*') || line.startsWith('*')) {
89101
continue
90102
}
91-
isClient = line === "'use client'" || line === '"use client"'
92-
break
103+
const match = /^(['"])(use [a-z-]+)\1;?$/.exec(line)
104+
return match ? match[2] : null
93105
}
106+
return null
107+
}
108+
109+
const useClientCache = new Map<string, boolean>()
110+
111+
async function isUseClientModule(absFile: string): Promise<boolean> {
112+
const cached = useClientCache.get(absFile)
113+
if (cached !== undefined) return cached
114+
let isClient = false
115+
try {
116+
isClient = leadingDirective(await readFile(absFile, 'utf8')) === 'use client'
117+
} catch {}
94118
useClientCache.set(absFile, isClient)
95119
return isClient
96120
}
97121

122+
/**
123+
* Locations declaring `'use server'` — module prologue or inline in a function
124+
* body. Either form registers Server Actions app-wide.
125+
*/
126+
async function findUseServerDirectives(): Promise<string[]> {
127+
const found: string[] = []
128+
for (const dir of DIRECTIVE_SCAN_DIRS) {
129+
for (const absFile of await listFiles(dir)) {
130+
const lines = (await readFile(absFile, 'utf8')).split('\n')
131+
for (let i = 0; i < lines.length; i++) {
132+
if (/^(['"])use server\1;?$/.test(lines[i].trim())) {
133+
found.push(`${path.relative(ROOT, absFile)}:${i + 1}`)
134+
}
135+
}
136+
}
137+
}
138+
return found
139+
}
140+
98141
/** Resolve an import specifier to an absolute source file, or null if external/unresolved. */
99142
async function resolveSpecifier(spec: string, fromFile: string): Promise<string | null> {
100143
let base: string
@@ -188,6 +231,22 @@ interface Violation {
188231

189232
async function main() {
190233
const checkMode = process.argv.includes('--check')
234+
let failed = false
235+
236+
const serverDirectives = await findUseServerDirectives()
237+
if (serverDirectives.length === 0) {
238+
console.log("✓ No 'use server' directives (Server Actions stay disabled).")
239+
} else {
240+
failed = true
241+
console.error(
242+
`\n✗ ${serverDirectives.length} 'use server' directive(s) found.\n` +
243+
` These enable Next's Server Action handling app-wide, which turns any unauthenticated\n` +
244+
` multipart/form-data POST to any App Router path into a 500, and exposes every export\n` +
245+
` as an unauthenticated endpoint. Use a '.server.ts' module called from a route handler.\n`
246+
)
247+
for (const location of serverDirectives) console.error(` ${location}`)
248+
}
249+
191250
const allFiles = await listFiles(APP_DIR)
192251
const violations: Violation[] = []
193252

@@ -212,19 +271,20 @@ async function main() {
212271
console.log(
213272
"✓ Client-boundary import check passed (no server file imports a value from a 'use client' module)."
214273
)
215-
return
274+
} else {
275+
failed = true
276+
console.error(
277+
`\n✗ ${violations.length} server file(s) import a runtime value from a 'use client' module.\n` +
278+
` On the server these resolve to client-reference stubs and throw when called (e.g. 'X.list is not a function').\n` +
279+
` Move the imported factory/fetcher/constant into a non-'use client' module (hooks/queries/utils/*-keys.ts or fetch-*.ts).\n` +
280+
` See .claude/rules/sim-queries.md. Escape hatch: // ${ALLOW_DIRECTIVE}: <reason> above the import.\n`
281+
)
282+
for (const v of violations) {
283+
console.error(` ${v.file}:${v.line} imports from '${v.specifier}'`)
284+
}
216285
}
217286

218-
console.error(
219-
`\n✗ ${violations.length} server file(s) import a runtime value from a 'use client' module.\n` +
220-
` On the server these resolve to client-reference stubs and throw when called (e.g. 'X.list is not a function').\n` +
221-
` Move the imported factory/fetcher/constant into a non-'use client' module (hooks/queries/utils/*-keys.ts or fetch-*.ts).\n` +
222-
` See .claude/rules/sim-queries.md. Escape hatch: // ${ALLOW_DIRECTIVE}: <reason> above the import.\n`
223-
)
224-
for (const v of violations) {
225-
console.error(` ${v.file}:${v.line} imports from '${v.specifier}'`)
226-
}
227-
if (checkMode) process.exit(1)
287+
if (failed && checkMode) process.exit(1)
228288
}
229289

230290
main().catch((error) => {

0 commit comments

Comments
 (0)