Skip to content

Commit 52f8b4d

Browse files
committed
improvement(audits): share the directive classifier and simplify check:utils
- move leadingDirective/directiveOn into scripts/source-kind.ts; check:utils uses it instead of its own 'use client' regex, and multi-line block-comment headers now parse - replace the deployment-shape allowlist with a client-boundary-allow annotation on the panel store's isChatEnabled import - exempt all of packages/utils/src by prefix (drops the stale retry.test.ts entry) - build both truncate patterns from one shared fragment and prefilter - add literal prefilters to isRecordLike, fromEntries, and useRef patterns and memoize prefilter results per file - trim the deployment-shape rationale to a CLAUDE.md pointer
1 parent fb900db commit 52f8b4d

4 files changed

Lines changed: 74 additions & 79 deletions

File tree

‎apps/sim/stores/panel/store.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import { create } from 'zustand'
22
import { persist } from 'zustand/middleware'
3+
// client-boundary-allow: the default tab is picked at module init, before any surface
4+
// seeds the deployment shape; moving it to the reader is a product call
35
import { isChatEnabled } from '@/lib/core/config/env-flags'
46
import { PANEL_WIDTH } from '@/stores/constants'
57
import type { PanelState, PanelTab } from '@/stores/panel/types'

‎scripts/check-client-boundary-imports.ts‎

Lines changed: 8 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -49,20 +49,19 @@
4949
* ## Deployment-shape flags in client code
5050
*
5151
* Client code — `stores/`, `hooks/`, `blocks/`, and any `'use client'` module or hook
52-
* under the workspace, organization, or standalone settings surfaces — must not read
53-
* `isHosted`, `isBillingEnabled`, `isChatEnabled`, or the enterprise feature flags from
54-
* `env-flags`, by named or namespace import: those freeze at module init from the root
55-
* layout's `NEXT_PUBLIC_*` transport, which a recovered 404 or `global-error` tab never
56-
* ran, so Sim Cloud renders as self-hosted. Read them through `useDeploymentShape()` /
57-
* `getDeploymentShape()` from `@/lib/core/config/deployment-shape` (CLAUDE.md). The flag
58-
* list is read from that module's own `env-flags` import, so it cannot drift.
52+
* under the workspace, organization, or standalone settings surfaces — must not import the
53+
* deployment-shape flags from `env-flags` (named or namespace); it reads them through
54+
* `@/lib/core/config/deployment-shape`. The flag list is that module's own `env-flags`
55+
* import, so it cannot drift. Same escape hatch as above. Why: CLAUDE.md "Deployment flags
56+
* in the browser".
5957
*
6058
* Usage:
6159
* bun run scripts/check-client-boundary-imports.ts # report
6260
* bun run scripts/check-client-boundary-imports.ts --check # CI gate (fail on any)
6361
*/
6462
import { readdir, readFile } from 'node:fs/promises'
6563
import path from 'node:path'
64+
import { directiveOn, leadingDirective } from './source-kind'
6665

6766
const ROOT = path.resolve(import.meta.dir, '..')
6867
const APP_DIR = path.join(ROOT, 'apps/sim')
@@ -81,16 +80,6 @@ function isServerSurface(rel: string): boolean {
8180
const ENV_FLAGS = '@/lib/core/config/env-flags'
8281
const DEPLOYMENT_SHAPE_MODULE = path.join(APP_DIR, 'lib/core/config/deployment-shape.ts')
8382

84-
/**
85-
* Known deployment-shape violations awaiting a product decision (paths relative to apps/sim).
86-
* Each entry names why it cannot simply move to the reader.
87-
*/
88-
const DEPLOYMENT_SHAPE_ALLOWLIST = new Set([
89-
// Picks the panel's default tab from `isChatEnabled` at module init, before any surface
90-
// seeds the shape; moving it to the reader changes the first-render tab, a product call.
91-
'stores/panel/store.ts',
92-
])
93-
9483
/** Surfaces whose shell seeds the server-resolved deployment shape (paths relative to apps/sim). */
9584
function isDeploymentShapeSurface(rel: string): boolean {
9685
return /^(?:app\/(?:workspace|o|account|selfhost\/settings)|components\/settings|ee)\//.test(rel)
@@ -156,34 +145,6 @@ async function listFiles(dir: string): Promise<string[]> {
156145
return out
157146
}
158147

159-
/**
160-
* Drops a trailing `//` or `/* *\/` comment from an already-trimmed line. A
161-
* directive keeps its meaning when a note follows it on the same line, so the
162-
* comment has to come off before the directive is matched.
163-
*/
164-
function stripTrailingComment(line: string): string {
165-
return line.replace(/(?:\/\/.*|\/\*.*?\*\/)\s*$/, '').trim()
166-
}
167-
168-
/** A lone directive statement, e.g. `'use server'` or `"use client";`. */
169-
const DIRECTIVE_STATEMENT = /^(['"])(use [a-z-]+)\1\s*;?$/
170-
171-
/**
172-
* Returns the module's leading directive prologue string, if any. A directive
173-
* must be the first statement; comments and blank lines may precede it.
174-
*/
175-
function leadingDirective(content: string): string | null {
176-
for (const raw of content.split('\n')) {
177-
const line = raw.trim()
178-
if (line === '' || line.startsWith('//') || line.startsWith('/*') || line.startsWith('*')) {
179-
continue
180-
}
181-
const match = DIRECTIVE_STATEMENT.exec(stripTrailingComment(line))
182-
return match ? match[2] : null
183-
}
184-
return null
185-
}
186-
187148
const useClientCache = new Map<string, boolean>()
188149

189150
async function isUseClientModule(absFile: string): Promise<boolean> {
@@ -206,8 +167,7 @@ async function findUseServerDirectives(files: readonly string[]): Promise<string
206167
for (const absFile of files) {
207168
const lines = (await readSource(absFile)).split('\n')
208169
for (let i = 0; i < lines.length; i++) {
209-
const match = DIRECTIVE_STATEMENT.exec(stripTrailingComment(lines[i].trim()))
210-
if (match?.[2] === 'use server') {
170+
if (directiveOn(lines[i]) === 'use server') {
211171
found.push(`${path.relative(ROOT, absFile)}:${i + 1}`)
212172
}
213173
}
@@ -383,9 +343,7 @@ async function main() {
383343
for (const absFile of allFiles) {
384344
if (!absFile.startsWith(`${APP_DIR}${path.sep}`)) continue
385345
const rel = path.relative(APP_DIR, absFile)
386-
if (DEPLOYMENT_SHAPE_ALLOWLIST.has(rel) || !(await isDeploymentShapeClient(rel, absFile))) {
387-
continue
388-
}
346+
if (!(await isDeploymentShapeClient(rel, absFile))) continue
389347
const content = await readSource(absFile)
390348
for (const imp of parseImports(content)) {
391349
if (imp.specifier !== ENV_FLAGS || !importsAValue(imp.clause)) continue
@@ -403,7 +361,6 @@ async function main() {
403361
failed = true
404362
console.error(
405363
`\n✗ ${shapeViolations.length} client module(s) read deployment-shape flags from env-flags.\n` +
406-
` Those constants freeze from the root layout's NEXT_PUBLIC_* transport, so a recovered 404/global-error tab renders Sim Cloud as self-hosted.\n` +
407364
` Read them via useDeploymentShape() (components) or getDeploymentShape() (helpers) from @/lib/core/config/deployment-shape.\n`
408365
)
409366
for (const v of shapeViolations) {

‎scripts/check-utils-enforcement.ts‎

Lines changed: 39 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -18,29 +18,19 @@
1818
*/
1919
import { readdir, readFile } from 'node:fs/promises'
2020
import path from 'node:path'
21+
import { leadingDirective } from './source-kind'
2122

2223
const ROOT = path.resolve(import.meta.dir, '..')
2324

2425
const SCAN_DIRS = [path.join(ROOT, 'apps'), path.join(ROOT, 'packages')]
2526

2627
const SKIP_DIRS = new Set(['node_modules', 'dist', '.next', '.turbo', 'coverage', 'bundles'])
2728

28-
/** Files that implement the utilities themselves — allowed to use the underlying primitives. */
29+
/** `@sim/utils` implements the helpers, so it may use the primitives they replace. */
30+
const UTILS_SOURCE = 'packages/utils/src/'
31+
32+
/** Other files allowed to use the underlying primitives. */
2933
const ALLOWLISTED_FILES = new Set([
30-
'packages/utils/src/errors.ts',
31-
'packages/utils/src/helpers.ts',
32-
'packages/utils/src/random.ts',
33-
'packages/utils/src/id.ts',
34-
'packages/utils/src/object.ts',
35-
'packages/utils/src/retry.ts',
36-
'packages/utils/src/string.ts',
37-
'packages/utils/src/errors.test.ts',
38-
'packages/utils/src/helpers.test.ts',
39-
'packages/utils/src/random.test.ts',
40-
'packages/utils/src/id.test.ts',
41-
'packages/utils/src/object.test.ts',
42-
'packages/utils/src/retry.test.ts',
43-
'packages/utils/src/string.test.ts',
4434
// Published standalone CLIs: `@sim/utils` is private, so they carry local
4535
// copies rather than a dependency that only resolves inside the monorepo.
4636
'packages/cli/src/index.ts',
@@ -53,9 +43,6 @@ const ALLOWLISTED_FILES = new Set([
5343
'packages/testing/src/factories/id.ts',
5444
])
5545

56-
/** Leading `'use client'` directive, after any comment header. */
57-
const USE_CLIENT_PROLOGUE = /^(?:\s*(?:\/\/[^\n]*|\/\*[\s\S]*?\*\/))*\s*['"]use client['"]/
58-
5946
/** App Router files Next only ever evaluates on the server, plus `*.server.ts` modules. */
6047
const SERVER_ONLY_FILE =
6148
/\/(?:route|sitemap|robots|manifest)\.[jt]sx?$|\/(?:opengraph-image|twitter-image|icon|apple-icon)\.[^/]+$|\.server\.[jt]sx?$/
@@ -71,17 +58,25 @@ function isClientRenderPath(rel: string, content: string): boolean {
7158
/^apps\/sim\/(app|components|hooks|stores)\//.test(rel) ||
7259
/^apps\/docs\/(app|components)\//.test(rel) ||
7360
/^packages\/(emcn|workflow-renderer)\/src\//.test(rel) ||
74-
USE_CLIENT_PROLOGUE.test(content)
61+
leadingDirective(content) === 'use client'
7562
)
7663
}
7764

65+
/** `s.slice(0, n)` plus a suffix, by `+` or in a template literal; `\1` is `s` and `\2` is `n`. */
66+
const TRUNCATED = String.raw`(?:\`\$\{\1\.(?:slice|substring)\(\s*0\s*,\s*\2\s*\)\}[^\`$]*\`|\1\.(?:slice|substring)\(\s*0\s*,\s*\2\s*\)\s*\+\s*(?:'[^']*'|"[^"]*"|\w+))`
67+
/** Literal gate for both truncate patterns, whose backreferences are slow over every file. */
68+
const TRUNCATE_PREFILTER = /\.(?:slice|substring)\(\s*0\s*,[^)]*\)\s*(?:\}|\+)/
69+
70+
/** Literal gate shared by the `filterUndefined` and `omit` patterns. */
71+
const FROM_ENTRIES = /Object\.fromEntries\(/
72+
7873
const BANNED_PATTERNS: Array<{
7974
pattern: RegExp
8075
description: string
8176
suggestion: string
8277
/** Restricts the pattern to matching files; unrestricted patterns apply everywhere. */
8378
appliesTo?: (rel: string, content: string) => boolean
84-
/** Cheap literal test that gates a backreference-heavy pattern, which is slow over every file. */
79+
/** Cheap literal test that skips the pattern on files that cannot match; memoized per file. */
8580
prefilter?: RegExp
8681
}> = [
8782
// Randomness / ID generation — global property access that import bans miss
@@ -130,31 +125,38 @@ const BANNED_PATTERNS: Array<{
130125
/typeof\s+([\w.]+)\s*===\s*'object'\s*&&\s*\1\s*!==\s*null\s*&&\s*!Array\.isArray\(\s*\1\s*\)/g,
131126
description: "typeof v === 'object' && v !== null && !Array.isArray(v)",
132127
suggestion: 'isRecordLike(v) from @sim/utils/object',
128+
prefilter: /!Array\.isArray\(/,
133129
},
134130
{
135131
pattern:
136132
/Object\.fromEntries\(\s*Object\.entries\([^()]*\)\s*\.filter\(\s*\(\[\s*\w*\s*,\s*(\w+)\s*\]\)\s*=>\s*\1\s*!==?\s*undefined\s*\)\s*,?\s*\)/g,
137133
description: 'Object.fromEntries(Object.entries(obj).filter(([, v]) => v !== undefined))',
138134
suggestion: 'filterUndefined(obj) from @sim/utils/object',
135+
prefilter: FROM_ENTRIES,
139136
},
140137
{
141138
pattern:
142139
/Object\.fromEntries\(\s*Object\.entries\([^()]*\)\s*\.filter\(\s*\(\[\s*(\w+)\s*\]\)\s*=>\s*\1\s*!==\s*[\w.'"]+\s*\)\s*,?\s*\)/g,
143140
description: 'Object.fromEntries(Object.entries(obj).filter(([k]) => k !== key))',
144141
suggestion: 'omit(obj, [key]) from @sim/utils/object',
142+
prefilter: FROM_ENTRIES,
145143
},
146144
{
147-
pattern:
148-
/\b([\w.]+)\.length\s*>\s*([\w.]+)\s*\?\s*(?:`\$\{\1\.(?:slice|substring)\(\s*0\s*,\s*\2\s*\)\}[^`$]*`|\1\.(?:slice|substring)\(\s*0\s*,\s*\2\s*\)\s*\+\s*(?:'[^']*'|"[^"]*"|\w+))\s*:\s*\1\b(?!\.)/g,
145+
pattern: new RegExp(
146+
String.raw`\b([\w.]+)\.length\s*>\s*([\w.]+)\s*\?\s*${TRUNCATED}\s*:\s*\1\b(?!\.)`,
147+
'g'
148+
),
149149
description: 's.length > n ? s.slice(0, n) + suffix : s',
150-
prefilter: /\.(?:slice|substring)\(\s*0\s*,[^)]*\)\s*(?:\}|\+)/,
150+
prefilter: TRUNCATE_PREFILTER,
151151
suggestion: "truncate(s, n, suffix?) from @sim/utils/string (suffix defaults to '...')",
152152
},
153153
{
154-
pattern:
155-
/\b([\w.]+)\.length\s*<=\s*([\w.]+)\s*\?\s*\1\s*:\s*(?:`\$\{\1\.(?:slice|substring)\(\s*0\s*,\s*\2\s*\)\}[^`$]*`|\1\.(?:slice|substring)\(\s*0\s*,\s*\2\s*\)\s*\+\s*(?:'[^']*'|"[^"]*"|\w+))/g,
154+
pattern: new RegExp(
155+
String.raw`\b([\w.]+)\.length\s*<=\s*([\w.]+)\s*\?\s*\1\s*:\s*${TRUNCATED}`,
156+
'g'
157+
),
156158
description: 's.length <= n ? s : s.slice(0, n) + suffix',
157-
prefilter: /\.(?:slice|substring)\(\s*0\s*,[^)]*\)\s*(?:\}|\+)/,
159+
prefilter: TRUNCATE_PREFILTER,
158160
suggestion: "truncate(s, n, suffix?) from @sim/utils/string (suffix defaults to '...')",
159161
},
160162
{
@@ -173,6 +175,7 @@ const BANNED_PATTERNS: Array<{
173175
pattern: /\buseRef(?:<(?:[^<>]|<[^<>]*>)*>)?\(\s*new\s+[A-Z]\w*/g,
174176
description: 'useRef(new X()) allocates a throwaway X on every render',
175177
suggestion: 'useRef<X | null>(null), then `ref.current ??= new X()` before first use',
178+
prefilter: /useRef/,
176179
},
177180
{
178181
pattern:
@@ -275,7 +278,7 @@ async function main() {
275278

276279
for (const file of allFiles) {
277280
const rel = path.relative(ROOT, file)
278-
if (ALLOWLISTED_FILES.has(rel)) continue
281+
if (rel.startsWith(UTILS_SOURCE) || ALLOWLISTED_FILES.has(rel)) continue
279282

280283
const content = await readFile(file, 'utf8')
281284
const matches: Array<{
@@ -284,9 +287,17 @@ async function main() {
284287
suggestion: string
285288
}> = []
286289

290+
const prefilterHits = new Map<RegExp, boolean>()
287291
for (const { pattern, description, suggestion, appliesTo, prefilter } of BANNED_PATTERNS) {
288292
if (appliesTo && !appliesTo(rel, content)) continue
289-
if (prefilter && !prefilter.test(content)) continue
293+
if (prefilter) {
294+
let hit = prefilterHits.get(prefilter)
295+
if (hit === undefined) {
296+
hit = prefilter.test(content)
297+
prefilterHits.set(prefilter, hit)
298+
}
299+
if (!hit) continue
300+
}
290301
pattern.lastIndex = 0
291302
for (let match = pattern.exec(content); match !== null; match = pattern.exec(content)) {
292303
matches.push({ index: match.index, description, suggestion })

‎scripts/source-kind.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/** A lone directive statement, e.g. `'use server'` or `"use client";`. */
2+
const DIRECTIVE_STATEMENT = /^(['"])(use [a-z-]+)\1\s*;?$/
3+
4+
/**
5+
* The directive a single source line states, e.g. `use client`, or null. A note may follow the
6+
* directive on the same line, so a trailing `//` or `/* *\/` comment comes off before matching.
7+
*/
8+
export function directiveOn(line: string): string | null {
9+
const statement = line
10+
.trim()
11+
.replace(/(?:\/\/.*|\/\*.*?\*\/)\s*$/, '')
12+
.trim()
13+
return DIRECTIVE_STATEMENT.exec(statement)?.[2] ?? null
14+
}
15+
16+
/** Comments and whitespace ahead of a module's first statement. */
17+
const LEADING_COMMENTS = /^(?:\s*(?:\/\/[^\n]*|\/\*[\s\S]*?\*\/))*\s*/
18+
19+
/**
20+
* The module's leading directive prologue, if any. A directive must be the first statement;
21+
* comments and blank lines may precede it.
22+
*/
23+
export function leadingDirective(content: string): string | null {
24+
return directiveOn(content.replace(LEADING_COMMENTS, '').split('\n', 1)[0])
25+
}

0 commit comments

Comments
 (0)