Skip to content

fix(ci): make the utils gate see the wrapped forms of what it bans - #7047

Merged
waleedlatif1 merged 1 commit into
stagingfrom
fix-utils-gate-linescope
Aug 24, 2026
Merged

fix(ci): make the utils gate see the wrapped forms of what it bans#7047
waleedlatif1 merged 1 commit into
stagingfrom
fix-utils-gate-linescope

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

The gate was green while 11 files violated it

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. Same class as the two blind spots fixed in check-react-query-patterns (#7020).

Patterns now run against the whole file, with match offsets mapped back to line numbers by binary search over a line-start table — verified against every offset of a multi-line fixture, not just a sample.

The 11, handled individually

8 → getErrorMessage(error, fallback). Two were not straight swaps:

  • auto-layout-utils also collapses a redundant instanceof ApiClientError arm, since that class extends Error
  • upgrade.ts keeps its rawBody ?? message arm — the helper can't express it — and only the tail collapses

3 stay, annotated with why. This gate had no escape hatch, unlike its siblings (rq-lint-allow:, boundary-raw-fetch:), so I added the same one: // utils-lint-allow: <reason>.

Site Why the helper doesn't fit
forget-password/route.ts message returns to an unauthenticated caller
reset-password/route.ts same
e2b.ts probes E2B's error shape — a record-like carrying message or value

The auth pair matters: getErrorMessage(v, fallback) returns the value when it's a non-empty string, where the ternary returns the fallback. For a response body sent to an unauthenticated caller, that is the disclosure shape #7015 closed. Those two ternaries are deliberate, and there is no way to express "non-Error → always the fixed copy" with the mandated helper.

An annotation with no reason does not suppress, so the hatch can't be used to silence a finding without saying why.

A comment that was actively misleading

The header claimed Biome's noRestrictedImports covers "crypto named imports". It lists only nanoid and uuid. import { randomBytes } from 'node:crypto' passes both gates — deliberately, since server code building cipher IVs wants node's crypto rather than the cross-context wrapper in @sim/utils/random. Corrected, because the next person auditing this would otherwise assume a ban that doesn't exist.

(An earlier read of this flagged 12 "violations" here. CLAUDE.md bans crypto.randomUUID(), not randomBytes, and packages/security/src/encryption.ts using it for cipher IVs is correct. Not a violation set — a wrong comment.)

Verified it can fail, both ways

reintroduce a wrapped ternary   → Found 1 banned pattern(s)   exit 1
empty an annotation's reason    → Found 1 banned pattern(s)   exit 1
restore both                    → ✓ No banned patterns found  exit 0

All 26 workspaces type-check, lint:check exits 0, all 33 repo audits pass, and 2777 tests across lib/webhooks, stores/workflows, lib/workspace-files, lib/execution, app/api/auth and lib/billing.

`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.
@vercel

vercel Bot commented Aug 24, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview Aug 24, 2026 8:25pm

Request Review

@cursor

cursor Bot commented Aug 24, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Mostly CI and consistent error messaging; auth routes explicitly preserve safe 500 copy for unauthenticated callers instead of adopting getErrorMessage.

Overview
Fixes a blind spot in check-utils-enforcement where banned multi-line idioms (especially instanceof Error ? … : fallback) never matched because the script only inspected single lines. Matching now runs over full file contents with offset→line mapping, and deliberate exceptions use // utils-lint-allow: <reason> (empty reasons do not suppress).

Brings ~11 call sites in line with the gate, mostly replacing inline error ternaries with getErrorMessage from @sim/utils/errors (webhooks, workflow registry, auto-layout, billing portal errors, workspace file moves, etc.). A few spots stay on custom logic with annotations: unauthenticated password reset/forget responses (must not surface thrown strings), and E2B timeout probing (message vs value on record-like errors). upgrade.ts still prefers ApiClientError.rawBody when present.

The script header comment is corrected so it no longer implies Biome bans all node:crypto named imports.

Reviewed by Cursor Bugbot for commit 85284e6. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR changes the shared-utils audit to detect formatter-wrapped banned patterns across complete files, maps matches back to source lines, and adds reason-required exception annotations. It also replaces eligible inline error extraction with getErrorMessage while preserving specialized behavior in authentication and E2B paths.

  • Adds whole-file matching and binary-search line-number mapping to the utils enforcement script.
  • Introduces utils-lint-allow: annotations with mandatory reasons.
  • Migrates eight error-handling sites to the shared helper.
  • Retains fixed unauthenticated error responses and E2B-specific error-shape probing.

Confidence Score: 5/5

The PR appears safe to merge with no concrete blocking or independently actionable non-blocking issue identified.

The multiline enforcement logic correctly maps match offsets to source lines, the changed exception annotations are narrowly placed and reasoned, and the helper migrations preserve behavior for the established Error-shaped failure paths.

Important Files Changed

Filename Overview
scripts/check-utils-enforcement.ts Reworks enforcement to detect multiline patterns and adds documented, reason-required local exceptions without an accepted correctness issue.
apps/sim/app/api/auth/forget-password/route.ts Documents why the public route intentionally preserves fixed fallback behavior for non-Error throws.
apps/sim/app/api/auth/reset-password/route.ts Documents the same deliberate fixed-fallback behavior for unauthenticated reset failures.
apps/sim/lib/execution/remote-sandbox/e2b.ts Annotates E2B-specific probing that must inspect record-like message and value fields.
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/auto-layout-utils.ts Simplifies equivalent Error and ApiClientError message extraction through the shared helper.
apps/sim/stores/workflows/registry/store.ts Replaces inline workflow hydration error extraction with the shared utility without altering established Error behavior.
apps/sim/lib/webhooks/providers/microsoft-teams.ts Standardizes fallback extraction for Teams subscription failures.
apps/sim/lib/webhooks/providers/telegram.ts Standardizes fallback extraction for Telegram subscription failures.
apps/sim/lib/webhooks/providers/typeform.ts Standardizes fallback extraction for Typeform subscription failures.

Reviews (1): Last reviewed commit: "fix(ci): make the utils gate see the wra..." | Re-trigger Greptile

@waleedlatif1
waleedlatif1 merged commit 3251bf1 into staging Aug 24, 2026
30 checks passed
@waleedlatif1
waleedlatif1 deleted the fix-utils-gate-linescope branch August 24, 2026 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant