chore(utils): migrate to shared random/ID utilities and add enforcement linting#4622
chore(utils): migrate to shared random/ID utilities and add enforcement linting#4622waleedlatif1 wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview It then migrates callsites across the repo away from Finally, it adds enforcement: Biome Reviewed by Cursor Bugbot for commit 4ad2f31. Configure here. |
Greptile SummaryThis PR consolidates all randomness and ID generation across 72 files into two shared packages —
Confidence Score: 4/5Safe to merge — the migration is mechanically correct across all 72 call sites, but the enforcement layer has a gap that allows the rules to be bypassed via All migrated call sites are functionally correct: OTP generation, token/key derivation, and Twilio HMAC signing all produce equivalent outputs with the shared utilities. The
Important Files Changed
Reviews (1): Last reviewed commit: "chore: remove stray files" | Re-trigger Greptile |
| "noInferrableTypes": "error", | ||
| "noUselessElse": "error" | ||
| "noUselessElse": "error", | ||
| "noRestrictedImports": { | ||
| "level": "error", | ||
| "options": { | ||
| "paths": { | ||
| "nanoid": "Use generateShortId() from @sim/utils/id instead", | ||
| "uuid": "Use generateId() from @sim/utils/id instead", | ||
| "crypto": { | ||
| "importNames": ["randomUUID", "randomBytes"], | ||
| "message": "Use generateId()/generateShortId() from @sim/utils/id or generateRandomBytes()/generateRandomHex() from @sim/utils/random instead" | ||
| } |
There was a problem hiding this comment.
"crypto" but not "node:crypto"
Biome treats "crypto" and "node:crypto" as distinct module specifiers. The restriction only fires for the bare "crypto" form, so import { randomBytes } from 'node:crypto' silently passes the linter. The project already uses the node: prefix convention for legitimate crypto imports (see twilio-voice.ts in this PR), so the gap is live. The check-utils-enforcement.ts script only catches crypto.randomBytes( as a property-access pattern; a direct call like randomBytes(16) after a destructured node:crypto import bypasses both enforcement layers simultaneously.
Adding a parallel "node:crypto" entry to the paths object would close the gap.
| description: 'Math.random()', | ||
| suggestion: 'randomInt / randomFloat / randomItem from @sim/utils/random', | ||
| }, | ||
| { | ||
| pattern: /\bcrypto\.randomUUID\s*\(/g, | ||
| description: 'crypto.randomUUID()', | ||
| suggestion: 'generateId() or generateShortId() from @sim/utils/id', | ||
| }, | ||
| { | ||
| pattern: /\bcrypto\.randomBytes\s*\(/g, | ||
| description: 'crypto.randomBytes()', | ||
| suggestion: 'generateRandomBytes() or generateRandomHex() from @sim/utils/random', | ||
| }, | ||
| ] | ||
|
|
||
| async function walk(dir: string, results: string[] = []): Promise<string[]> { | ||
| let entries | ||
| try { | ||
| entries = await readdir(dir, { withFileTypes: true }) | ||
| } catch { | ||
| return results |
There was a problem hiding this comment.
node:crypto imports
The BANNED_PATTERNS match crypto.randomUUID( and crypto.randomBytes( as property-access calls. A developer who writes import { randomBytes } from 'node:crypto' and calls randomBytes(16) directly would bypass this check (no crypto. prefix). This is the same gap as in the Biome rule — the two enforcement mechanisms fail in the same way for node:crypto destructured imports, so the gap isn't recovered by running the script.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4ad2f31. Configure here.
| "importNames": ["randomUUID", "randomBytes"], | ||
| "message": "Use generateId()/generateShortId() from @sim/utils/id or generateRandomBytes()/generateRandomHex() from @sim/utils/random instead" | ||
| } | ||
| } |
There was a problem hiding this comment.
Biome lint rule misses node:crypto restricted imports
Low Severity
The noRestrictedImports rule only restricts "crypto" but not "node:crypto". Biome treats these as distinct module specifiers, so import { randomBytes } from 'node:crypto' completely bypasses the lint rule. The codebase already uses node:crypto exclusively for all remaining crypto imports (e.g., createHmac, createHash), making it the natural pattern a developer would follow when adding a new import. The companion check-utils-enforcement.ts script only catches crypto.randomBytes(...) call-site patterns, so a destructured randomBytes(...) call from node:crypto would evade both enforcement mechanisms.
Reviewed by Cursor Bugbot for commit 4ad2f31. Configure here.
Summary
Math.random(),crypto.randomUUID(),crypto.randomBytes(),nanoid, anduuidusages across 72 files with the shared@sim/utils/randomand@sim/utils/idhelpersnoRestrictedImportsrule to catchnanoid,uuid, andcrypto.randomUUID/randomBytesimports at lint time (errors, not warnings)scripts/check-utils-enforcement.tsto catchMath.randomandcrypto.*global property access — patterns that import analysis alone can't catchcheck:utilsscript topackage.json.claude/worktreesfrom Biome scanning to prevent false positives from agent worktreesType of Change
Testing
Tested manually —
bun run check:utilspasses,bunx biome lintreports nonoRestrictedImportsviolations on the actual codebaseChecklist