Skip to content

Commit 3e3d860

Browse files
authored
fix(uploads): drop the stray 'use server' directive that enables Server Actions app-wide (#6335)
* 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. * fix(scripts): match boundary directives that carry a trailing comment A directive keeps its meaning when a note follows it on the same line, so strip a trailing '//' or block comment before matching. Shared by the 'use client' and 'use server' detectors.
1 parent 9b47930 commit 3e3d860

2 files changed

Lines changed: 100 additions & 29 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: 100 additions & 27 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,67 @@ async function listFiles(dir: string): Promise<string[]> {
6990
return out
7091
}
7192

72-
const useClientCache = new Map<string, boolean>()
93+
/**
94+
* Drops a trailing `//` or `/* *\/` comment from an already-trimmed line. A
95+
* directive keeps its meaning when a note follows it on the same line, so the
96+
* comment has to come off before the directive is matched.
97+
*/
98+
function stripTrailingComment(line: string): string {
99+
return line.replace(/(?:\/\/.*|\/\*.*?\*\/)\s*$/, '').trim()
100+
}
73101

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
102+
/** A lone directive statement, e.g. `'use server'` or `"use client";`. */
103+
const DIRECTIVE_STATEMENT = /^(['"])(use [a-z-]+)\1\s*;?$/
104+
105+
/**
106+
* Returns the module's leading directive prologue string, if any. A directive
107+
* must be the first statement; comments and blank lines may precede it.
108+
*/
109+
function leadingDirective(content: string): string | null {
86110
for (const raw of content.split('\n')) {
87111
const line = raw.trim()
88112
if (line === '' || line.startsWith('//') || line.startsWith('/*') || line.startsWith('*')) {
89113
continue
90114
}
91-
isClient = line === "'use client'" || line === '"use client"'
92-
break
115+
const match = DIRECTIVE_STATEMENT.exec(stripTrailingComment(line))
116+
return match ? match[2] : null
93117
}
118+
return null
119+
}
120+
121+
const useClientCache = new Map<string, boolean>()
122+
123+
async function isUseClientModule(absFile: string): Promise<boolean> {
124+
const cached = useClientCache.get(absFile)
125+
if (cached !== undefined) return cached
126+
let isClient = false
127+
try {
128+
isClient = leadingDirective(await readFile(absFile, 'utf8')) === 'use client'
129+
} catch {}
94130
useClientCache.set(absFile, isClient)
95131
return isClient
96132
}
97133

134+
/**
135+
* Locations declaring `'use server'` — module prologue or inline in a function
136+
* body. Either form registers Server Actions app-wide.
137+
*/
138+
async function findUseServerDirectives(): Promise<string[]> {
139+
const found: string[] = []
140+
for (const dir of DIRECTIVE_SCAN_DIRS) {
141+
for (const absFile of await listFiles(dir)) {
142+
const lines = (await readFile(absFile, 'utf8')).split('\n')
143+
for (let i = 0; i < lines.length; i++) {
144+
const match = DIRECTIVE_STATEMENT.exec(stripTrailingComment(lines[i].trim()))
145+
if (match?.[2] === 'use server') {
146+
found.push(`${path.relative(ROOT, absFile)}:${i + 1}`)
147+
}
148+
}
149+
}
150+
}
151+
return found
152+
}
153+
98154
/** Resolve an import specifier to an absolute source file, or null if external/unresolved. */
99155
async function resolveSpecifier(spec: string, fromFile: string): Promise<string | null> {
100156
let base: string
@@ -188,6 +244,22 @@ interface Violation {
188244

189245
async function main() {
190246
const checkMode = process.argv.includes('--check')
247+
let failed = false
248+
249+
const serverDirectives = await findUseServerDirectives()
250+
if (serverDirectives.length === 0) {
251+
console.log("✓ No 'use server' directives (Server Actions stay disabled).")
252+
} else {
253+
failed = true
254+
console.error(
255+
`\n✗ ${serverDirectives.length} 'use server' directive(s) found.\n` +
256+
` These enable Next's Server Action handling app-wide, which turns any unauthenticated\n` +
257+
` multipart/form-data POST to any App Router path into a 500, and exposes every export\n` +
258+
` as an unauthenticated endpoint. Use a '.server.ts' module called from a route handler.\n`
259+
)
260+
for (const location of serverDirectives) console.error(` ${location}`)
261+
}
262+
191263
const allFiles = await listFiles(APP_DIR)
192264
const violations: Violation[] = []
193265

@@ -212,19 +284,20 @@ async function main() {
212284
console.log(
213285
"✓ Client-boundary import check passed (no server file imports a value from a 'use client' module)."
214286
)
215-
return
287+
} else {
288+
failed = true
289+
console.error(
290+
`\n✗ ${violations.length} server file(s) import a runtime value from a 'use client' module.\n` +
291+
` On the server these resolve to client-reference stubs and throw when called (e.g. 'X.list is not a function').\n` +
292+
` Move the imported factory/fetcher/constant into a non-'use client' module (hooks/queries/utils/*-keys.ts or fetch-*.ts).\n` +
293+
` See .claude/rules/sim-queries.md. Escape hatch: // ${ALLOW_DIRECTIVE}: <reason> above the import.\n`
294+
)
295+
for (const v of violations) {
296+
console.error(` ${v.file}:${v.line} imports from '${v.specifier}'`)
297+
}
216298
}
217299

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)
300+
if (failed && checkMode) process.exit(1)
228301
}
229302

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

0 commit comments

Comments
 (0)