mirror of
https://github.com/simstudioai/sim.git
synced 2026-08-29 02:27:35 +08:00
fix(ci): make the utils gate see the wrapped forms of what it bans (#7047)
`check-utils-enforcement.ts` scanned line by line, and every idiom it bans is a
multi-token expression the formatter wraps at 100 columns. So it printed
`✓ No banned patterns found` while eleven files carried the wrapped form of
e instanceof Error ? e.message : fallback
which CLAUDE.md mandates `getErrorMessage` for. The same class as the two blind
spots already fixed in check-react-query-patterns.
Patterns now run against the whole file, with match offsets mapped back to line
numbers by binary search over the line-start table — verified against every
offset of a multi-line fixture.
Eight of the eleven are now `getErrorMessage(error, fallback)`.
`auto-layout-utils` collapses a redundant `instanceof ApiClientError` arm on the
way, since that class extends `Error`; `upgrade.ts` keeps its `rawBody ?? message`
arm, which the helper cannot express, and only its tail collapses.
The other three stay, because the helper genuinely does not fit, and they carry a
`// utils-lint-allow: <reason>` annotation — the same escape hatch
check-react-query-patterns already has, which this gate lacked:
- the two auth routes return the message to an unauthenticated caller, so a
non-Error throw must surface the fixed copy rather than its own text.
`getErrorMessage` passes a thrown string straight through, which is the
disclosure shape #7015 closed.
- `e2b.ts` probes E2B's own error shape — a record-like carrying `message` or
`value` — which has no equivalent.
An annotation with no reason does not suppress, so the hatch cannot be used to
silence a finding without saying why.
Also corrects the header, which claimed Biome's `noRestrictedImports` covers
"crypto named imports". It lists only `nanoid` and `uuid`. Named crypto imports
pass both gates deliberately — server code building cipher IVs wants node's
crypto, not the cross-context wrapper — and the comment asserting otherwise would
mislead the next person auditing this.
Verified the gate can fail in both directions: reintroducing a wrapped ternary
reports it, and emptying an annotation's reason reports it too.
This commit is contained in:
@@ -97,6 +97,9 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
|
||||
return NextResponse.json(
|
||||
{
|
||||
message:
|
||||
// utils-lint-allow: returned to an unauthenticated caller, so a non-Error throw
|
||||
// must surface the fixed copy rather than its own text — getErrorMessage would
|
||||
// pass a thrown string straight through.
|
||||
error instanceof Error
|
||||
? error.message
|
||||
: 'Failed to send password reset email. Please try again later.',
|
||||
|
||||
@@ -60,6 +60,9 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
|
||||
return NextResponse.json(
|
||||
{
|
||||
message:
|
||||
// utils-lint-allow: returned to an unauthenticated caller, so a non-Error throw
|
||||
// must surface the fixed copy rather than its own text — getErrorMessage would
|
||||
// pass a thrown string straight through.
|
||||
error instanceof Error
|
||||
? error.message
|
||||
: 'Failed to reset password. Please try again or request a new reset link.',
|
||||
|
||||
+1
-3
@@ -274,9 +274,7 @@ export function TeamManagement({
|
||||
portalWindow?.close()
|
||||
logger.error('Failed to open billing portal from transfer dialog', { error })
|
||||
setTransferPortalError(
|
||||
error instanceof Error
|
||||
? error.message
|
||||
: 'Failed to open Stripe billing portal. Please try again.'
|
||||
getErrorMessage(error, 'Failed to open Stripe billing portal. Please try again.')
|
||||
)
|
||||
},
|
||||
}
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
import { createLogger } from '@sim/logger'
|
||||
import { getErrorMessage } from '@sim/utils/errors'
|
||||
import type { Edge } from 'reactflow'
|
||||
import { ApiClientError } from '@/lib/api/client/errors'
|
||||
import { requestJson } from '@/lib/api/client/request'
|
||||
import {
|
||||
putWorkflowNormalizedStateContract,
|
||||
@@ -100,12 +99,7 @@ export async function applyAutoLayoutAndUpdateStore(
|
||||
},
|
||||
})
|
||||
} catch (error) {
|
||||
const errorMessage =
|
||||
error instanceof ApiClientError
|
||||
? error.message
|
||||
: error instanceof Error
|
||||
? error.message
|
||||
: 'Auto layout failed'
|
||||
const errorMessage = getErrorMessage(error, 'Auto layout failed')
|
||||
logger.error('Auto layout API call failed:', { error: errorMessage })
|
||||
return { success: false, error: errorMessage }
|
||||
}
|
||||
|
||||
@@ -211,9 +211,7 @@ export function useSubscriptionUpgrade() {
|
||||
error:
|
||||
transferError instanceof ApiClientError
|
||||
? (transferError.rawBody ?? transferError.message)
|
||||
: transferError instanceof Error
|
||||
? transferError.message
|
||||
: 'Unknown error',
|
||||
: getErrorMessage(transferError, 'Unknown error'),
|
||||
})
|
||||
}
|
||||
} catch (error) {
|
||||
|
||||
@@ -109,6 +109,8 @@ function isE2BExecutionTimeout(error: unknown): boolean {
|
||||
? error.name
|
||||
: ''
|
||||
const message =
|
||||
// utils-lint-allow: probes E2B's own error shape — a record-like carrying `message`
|
||||
// or `value` — which getErrorMessage cannot express.
|
||||
error instanceof Error
|
||||
? error.message
|
||||
: isRecordLike(error)
|
||||
|
||||
@@ -3,7 +3,7 @@ import { account } from '@sim/db/schema'
|
||||
import { createLogger } from '@sim/logger'
|
||||
import { safeCompare } from '@sim/security/compare'
|
||||
import { hmacSha256Base64 } from '@sim/security/hmac'
|
||||
import { toError } from '@sim/utils/errors'
|
||||
import { getErrorMessage, toError } from '@sim/utils/errors'
|
||||
import { isRecordLike } from '@sim/utils/object'
|
||||
import { eq } from 'drizzle-orm'
|
||||
import { type NextRequest, NextResponse } from 'next/server'
|
||||
@@ -733,9 +733,7 @@ export const microsoftTeamsHandler: WebhookProviderHandler = {
|
||||
error
|
||||
)
|
||||
throw new Error(
|
||||
error instanceof Error
|
||||
? error.message
|
||||
: 'Failed to create Teams subscription. Please try again.'
|
||||
getErrorMessage(error, 'Failed to create Teams subscription. Please try again.')
|
||||
)
|
||||
}
|
||||
},
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { db, webhook, workflowDeploymentVersion } from '@sim/db'
|
||||
import { createLogger } from '@sim/logger'
|
||||
import { getErrorMessage } from '@sim/utils/errors'
|
||||
import { and, eq, isNull, ne } from 'drizzle-orm'
|
||||
import { getNotificationUrl, getProviderConfig } from '@/lib/webhooks/provider-subscription-utils'
|
||||
import type {
|
||||
@@ -170,9 +171,7 @@ export const telegramHandler: WebhookProviderHandler = {
|
||||
error
|
||||
)
|
||||
throw new Error(
|
||||
error instanceof Error
|
||||
? error.message
|
||||
: 'Failed to create Telegram webhook. Please try again.'
|
||||
getErrorMessage(error, 'Failed to create Telegram webhook. Please try again.')
|
||||
)
|
||||
}
|
||||
},
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { createLogger } from '@sim/logger'
|
||||
import { safeCompare } from '@sim/security/compare'
|
||||
import { hmacSha256Base64 } from '@sim/security/hmac'
|
||||
import { getErrorMessage } from '@sim/utils/errors'
|
||||
import { getNotificationUrl, getProviderConfig } from '@/lib/webhooks/provider-subscription-utils'
|
||||
import type {
|
||||
DeleteSubscriptionContext,
|
||||
@@ -168,9 +169,7 @@ export const typeformHandler: WebhookProviderHandler = {
|
||||
error
|
||||
)
|
||||
throw new Error(
|
||||
error instanceof Error
|
||||
? error.message
|
||||
: 'Failed to create Typeform webhook. Please try again.'
|
||||
getErrorMessage(error, 'Failed to create Typeform webhook. Please try again.')
|
||||
)
|
||||
}
|
||||
},
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit'
|
||||
import { createLogger } from '@sim/logger'
|
||||
import { getPostgresErrorCode, toError } from '@sim/utils/errors'
|
||||
import { getErrorMessage, getPostgresErrorCode, toError } from '@sim/utils/errors'
|
||||
import { asOrchestrationError, type OrchestrationErrorCode } from '@/lib/core/orchestration/types'
|
||||
import { FolderPathError } from '@/lib/folders/paths'
|
||||
import { notifyWorkspaceFilesChanged } from '@/lib/realtime/notify'
|
||||
@@ -385,10 +385,10 @@ export async function performMoveWorkspaceFileItems(
|
||||
) {
|
||||
return {
|
||||
success: false,
|
||||
error:
|
||||
error instanceof Error
|
||||
? error.message
|
||||
: 'A file or folder with this name already exists in the destination folder',
|
||||
error: getErrorMessage(
|
||||
error,
|
||||
'A file or folder with this name already exists in the destination folder'
|
||||
),
|
||||
errorCode: 'conflict',
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { createLogger } from '@sim/logger'
|
||||
import { getErrorMessage } from '@sim/utils/errors'
|
||||
import { generateRandomHex } from '@sim/utils/random'
|
||||
import { create } from 'zustand'
|
||||
import { devtools } from 'zustand/middleware'
|
||||
@@ -206,10 +207,10 @@ export const useWorkflowRegistry = create<WorkflowRegistry>()(
|
||||
|
||||
logger.info(`Switched to workflow ${workflowId}`)
|
||||
} catch (error) {
|
||||
const message =
|
||||
error instanceof Error
|
||||
? error.message
|
||||
: `Failed to load workflow ${workflowId}: Unknown error`
|
||||
const message = getErrorMessage(
|
||||
error,
|
||||
`Failed to load workflow ${workflowId}: Unknown error`
|
||||
)
|
||||
logger.error(message)
|
||||
|
||||
const currentHydration = get().hydration
|
||||
|
||||
@@ -2,9 +2,17 @@
|
||||
/**
|
||||
* Enforces use of shared @sim/utils helpers over inline implementations.
|
||||
*
|
||||
* Biome's noRestrictedImports covers import-based bans (nanoid, uuid, crypto named imports).
|
||||
* This script catches patterns that static import analysis misses — global property access,
|
||||
* inline idioms, and reimplemented helpers that should live in @sim/utils.
|
||||
* Biome's noRestrictedImports covers the import-based bans it lists — today `nanoid` and
|
||||
* `uuid`. It does NOT cover named crypto imports; `import { randomBytes } from 'node:crypto'`
|
||||
* passes both gates, and deliberately so, since server code building cipher IVs and tokens
|
||||
* wants node's crypto rather than the cross-context wrapper in `@sim/utils/random`.
|
||||
*
|
||||
* This script catches what static import analysis misses — global property access, inline
|
||||
* idioms, and reimplemented helpers that should live in @sim/utils.
|
||||
*
|
||||
* Patterns are matched against the whole file, not line by line: every idiom banned here is a
|
||||
* multi-token expression that the formatter wraps at 100 columns, and a line-scoped scan sees
|
||||
* none of the wrapped forms. Deliberate exceptions carry `// utils-lint-allow: <reason>`.
|
||||
*/
|
||||
import { readdir, readFile } from 'node:fs/promises'
|
||||
import path from 'node:path'
|
||||
@@ -108,6 +116,48 @@ interface Violation {
|
||||
snippet: string
|
||||
}
|
||||
|
||||
/** Escape hatch for a deliberate use, mirroring `rq-lint-allow:` in check-react-query-patterns.ts. */
|
||||
const ALLOW = 'utils-lint-allow:'
|
||||
|
||||
/** Offset of the first character of each line, for mapping a match index back to a line number. */
|
||||
function buildLineStarts(content: string): number[] {
|
||||
const starts = [0]
|
||||
for (let i = 0; i < content.length; i++) {
|
||||
if (content[i] === '\n') starts.push(i + 1)
|
||||
}
|
||||
return starts
|
||||
}
|
||||
|
||||
/** 1-based line containing `offset`, by binary search over {@link buildLineStarts}. */
|
||||
function lineAt(lineStarts: number[], offset: number): number {
|
||||
let low = 0
|
||||
let high = lineStarts.length - 1
|
||||
while (low < high) {
|
||||
const mid = Math.ceil((low + high) / 2)
|
||||
if (lineStarts[mid] <= offset) low = mid
|
||||
else high = mid - 1
|
||||
}
|
||||
return low + 1
|
||||
}
|
||||
|
||||
/**
|
||||
* True if a `// utils-lint-allow: <reason>` annotation sits just above `line` (1-based).
|
||||
*
|
||||
* The reason must be non-empty: an annotation that does not say why is the thing this
|
||||
* check exists to prevent. Scans up to three comment lines above, so the annotation can
|
||||
* carry context lines with it.
|
||||
*/
|
||||
function hasAllow(lines: string[], line: number): boolean {
|
||||
for (let i = line - 2; i >= 0 && i >= line - 5; i--) {
|
||||
const text = lines[i]?.trim() ?? ''
|
||||
if (text.includes(ALLOW)) {
|
||||
return text.slice(text.indexOf(ALLOW) + ALLOW.length).trim().length > 0
|
||||
}
|
||||
if (text.length > 0 && !text.startsWith('//') && !text.startsWith('*')) break
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
async function main() {
|
||||
const allFiles: string[] = []
|
||||
for (const dir of SCAN_DIRS) {
|
||||
@@ -122,20 +172,20 @@ async function main() {
|
||||
|
||||
const content = await readFile(file, 'utf8')
|
||||
const lines = content.split('\n')
|
||||
const lineStarts = buildLineStarts(content)
|
||||
|
||||
for (let i = 0; i < lines.length; i++) {
|
||||
const line = lines[i]
|
||||
for (const { pattern, description, suggestion } of BANNED_PATTERNS) {
|
||||
pattern.lastIndex = 0
|
||||
if (pattern.test(line)) {
|
||||
violations.push({
|
||||
file: rel,
|
||||
line: i + 1,
|
||||
description,
|
||||
suggestion,
|
||||
snippet: line.trim(),
|
||||
})
|
||||
}
|
||||
for (const { pattern, description, suggestion } of BANNED_PATTERNS) {
|
||||
pattern.lastIndex = 0
|
||||
for (let match = pattern.exec(content); match !== null; match = pattern.exec(content)) {
|
||||
const line = lineAt(lineStarts, match.index)
|
||||
if (hasAllow(lines, line)) continue
|
||||
violations.push({
|
||||
file: rel,
|
||||
line,
|
||||
description,
|
||||
suggestion,
|
||||
snippet: (lines[line - 1] ?? '').trim(),
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user