fix(beps): generate API tokens with crypto randomness#3619
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
@xuyua9 is attempting to deploy a commit to the Boundary Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR refactors the API token generation logic in the user service. The implementation replaces ChangesAPI Token Generation Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@typescript/apps/beps/convex/users.ts`:
- Around line 314-333: The generateApiToken function uses non-deterministic
crypto.getRandomValues but is invoked inside Convex mutation handlers
getOrCreateApiToken and regenerateApiToken; move the non-deterministic token
generation out of mutations by creating a new Convex action (e.g.,
createApiTokenAction) that calls generateApiToken and returns the token, then
update getOrCreateApiToken and regenerateApiToken to accept a token parameter
and perform only the deterministic mutation (or alternatively replace
generateApiToken with a deterministic, seeded Math.random-based generator and
keep calls in the mutations); ensure references to generateApiToken,
getOrCreateApiToken, and regenerateApiToken are updated accordingly so mutations
no longer call crypto.getRandomValues directly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5847021b-e4e7-4f03-897e-885535c68ce4
📒 Files selected for processing (1)
typescript/apps/beps/convex/users.ts
| function generateApiToken(): string { | ||
| const chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"; | ||
| const prefix = "bep_"; | ||
| let token = prefix; | ||
| for (let i = 0; i < 32; i++) { | ||
| token += chars.charAt(Math.floor(Math.random() * chars.length)); | ||
| const randomBytes = new Uint8Array(API_TOKEN_RANDOM_LENGTH); | ||
| let token = API_TOKEN_PREFIX; | ||
|
|
||
| while (token.length < API_TOKEN_PREFIX.length + API_TOKEN_RANDOM_LENGTH) { | ||
| crypto.getRandomValues(randomBytes); | ||
|
|
||
| for (const byte of randomBytes) { | ||
| if (byte >= API_TOKEN_MAX_BYTE) { | ||
| continue; | ||
| } | ||
|
|
||
| token += API_TOKEN_ALPHABET[byte % API_TOKEN_ALPHABET.length]; | ||
| if (token.length === API_TOKEN_PREFIX.length + API_TOKEN_RANDOM_LENGTH) { | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return token; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="typescript/apps/beps/convex/users.ts"
echo "## File: $FILE"
ls -l "$FILE"
echo "## generateApiToken definition area"
rg -n "function[[:space:]]+generateApiToken|generateApiToken\\s*\\(" "$FILE" || true
start=250; end=380
sed -n "${start},${end}p" "$FILE" | nl -ba | sed -n '1,200p'
echo "## Callers of generateApiToken"
rg -n "\\bgenerateApiToken\\s*\\(" "$FILE" || true
echo "## getOrCreateApiToken and regenerateApiToken definitions"
rg -n "function[[:space:]]+getOrCreateApiToken|function[[:space:]]+regenerateApiToken|getOrCreateApiToken\\s*:\\s*mutation|regenerateApiToken\\s*:\\s*mutation|\\bgetOrCreateApiToken\\b|\\bregenerateApiToken\\b" "$FILE" || true
for sym in getOrCreateApiToken regenerateApiToken; do
l=$(rg -n "function[[:space:]]+$sym|\\bconst[[:space:]]+$sym\\b|\\b$sym\\b\\s*:\\s*(mutation|query)" "$FILE" | head -n1 | cut -d: -f1 || true)
if [ -n "${l:-}" ]; then
echo "---- $sym around line $l ----"
sed -n "$((l-80)),$((l+160))p" "$FILE" | nl -ba | sed -n '1,260p'
fi
done
echo "## Any crypto.getRandomValues usage in file"
rg -n "crypto\\.getRandomValues|getRandomValues\\b" "$FILE" || true
echo "## Any Convex helper runtime/determinism notes in file (near helper)"
rg -n "determin|Math\\.random|Web Crypto|runtim" "$FILE" || trueRepository: BoundaryML/baml
Length of output: 416
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="typescript/apps/beps/convex/users.ts"
echo "## File: $FILE"
sed -n '280,430p' "$FILE" | cat -n
echo "## Call sites of generateApiToken()"
rg -n "\\bgenerateApiToken\\s*\\(" "$FILE" || true
echo "## Mutation declarations for getOrCreateApiToken / regenerateApiToken"
rg -n "\\bgetOrCreateApiToken\\b|\\bregenerateApiToken\\b" "$FILE" || true
echo "## WebCrypto usage"
rg -n "crypto\\.getRandomValues|getRandomValues\\b|Math\\.random" "$FILE" || trueRepository: BoundaryML/baml
Length of output: 5499
🌐 Web query:
Convex queries and mutations determinism Math.random seeded Web Crypto crypto.getRandomValues allowed?
💡 Result:
Convex requires queries and mutations to be deterministic to ensure they can be reliably executed, cached, and retried by the system [1][2][3]. Because of this requirement, standard JavaScript randomness sources are handled specifically in the Convex runtime: 1. Math.random: Convex provides a seeded, pseudo-random number generator at Math.random [3]. This allows you to use it within queries and mutations while maintaining determinism [3]. Multiple calls within a single function execution will return different random values, but the generator's seed is managed implicitly by Convex [3]. It is important to note that because the runtime does not re-evaluate modules on every call, any Math.random value assigned to a variable in the global scope of a file will be fixed once the module is loaded and will not change between function runs [3]. 2. crypto.getRandomValues: The Web Crypto API function crypto.getRandomValues is allowed and supported in the Convex runtime [4][5]. Unlike Math.random, which is seeded for determinism, crypto.getRandomValues provides cryptographically secure random numbers [6]. It is the recommended approach for security-sensitive contexts, such as generating tokens, API keys, or session identifiers [4][7]. In summary, use Math.random for general-purpose, non-security-critical randomness where you rely on Convex's seeded determinism, and use crypto.getRandomValues when you require cryptographically secure randomness [3][6][4].
Citations:
- 1: https://docs.convex.dev/functions/mutation-functions
- 2: https://medium.com/@roneymoon77/goodbye-crud-hello-deterministic-functions-how-convex-rewrites-the-backend-mental-model-3d91e1d16aab
- 3: https://docs.convex.dev/functions/runtimes
- 4: https://labs.convex.dev/auth/config/passwords
- 5: https://gist.github.com/fayimora/5d323cac32ce95929c7c254ffbfc03d5
- 6: https://security.stackexchange.com/questions/181580/why-is-math-random-not-designed-to-be-cryptographically-secure
- 7: https://dev.to/ofri-peretz/mathrandom-is-not-random-enough-i-found-it-building-api-keys-in-a-57k-star-repo-2pl1
🌐 Web query:
site:docs.convex.dev crypto.getRandomValues deterministic
💡 Result:
The function crypto.getRandomValues is, by design and definition, non-deterministic. It is a Cryptographically Secure Pseudo-Random Number Generator (CSPRNG) provided by the Web Crypto API, intended to produce unpredictable, high-entropy values suitable for cryptographic use cases such as generating keys, initialization vectors, or nonces. Because it relies on high-entropy sources (such as hardware noise or OS-level entropy pools) to ensure unpredictability, calling crypto.getRandomValues with the same inputs will produce different outputs every time it is executed. It is not intended to be used in environments where deterministic or reproducible results are required. In contrast, deterministic systems (such as the server-side runtime of certain platforms like Convex, which requires functions to be deterministic to support automatic retries and consistency) explicitly restrict or forbid the use of non-deterministic functions like crypto.getRandomValues within their core query and mutation handlers. These systems often provide alternative, seeded pseudo-random number generators (such as a seeded Math.random) to maintain reproducibility.
Citations:
- 1: https://docs.convex.dev/functions/runtimes
- 2: https://docs.convex.dev/api/classes/browser.ConvexHttpClient
- 3: https://docs.convex.dev/testing/convex-test
- 4: https://docs.convex.dev/api/modules/values
- 5: https://docs.convex.dev/api/modules/browser
🌐 Web query:
site:docs.convex.dev "crypto.getRandomValues" "deterministic" "runtimes"
💡 Result:
There isn’t an inherent “deterministic” guarantee for JavaScript’s crypto.getRandomValues across runtimes; instead, determinism concerns typically arise in systems that need replayability. For example, Convex restricts what can be done inside query/mutation functions: they must be deterministic so the system can safely retry them with the same arguments and obtain identical side effects/returns [1]. In Convex, the system provides a seeded pseudo-random generator for Math.random to support determinism within that runtime [1]. So if your question is about “crypto.getRandomValues” being deterministic in some runtime layer: in the context of deterministic-replay runtimes (like Convex queries/mutations), using nondeterministic randomness APIs like getRandomValues would conflict with the runtime’s determinism model; the runtime instead uses its own deterministic randomness mechanism for allowed sources such as Math.random [1]. What’s missing from the sources I found: I did not retrieve official Web/MDN/WHATWG documentation text stating that crypto.getRandomValues is deterministic or not deterministic “per runtime,” nor any runtime-specific contract keyed to deterministic replay. The Convex documentation I found does, however, clearly state the determinism requirement for queries/mutations and that Convex guarantees determinism for randomness by seeding Math.random, not by making system randomness deterministic [1].
Citations:
🌐 Web query:
site:docs.convex.dev getRandomValues mutation crypto
💡 Result:
In the context of Convex, which uses server-side runtimes for its functions, crypto.getRandomValues() is generally not supported or available within query and mutation functions because these functions must remain deterministic to allow for consistent retries and state management [1]. Convex enforces determinism by controlling sources of randomness and time within its execution environment [1]. Specifically: - Determinism Requirement: Queries and mutations must be deterministic [1]. - Randomness: Instead of standard APIs like crypto.getRandomValues() or Math.random() (in its standard browser/Node form), Convex provides a seeded pseudo-random number generator at Math.random() that is deterministic for the duration of a function's execution [1]. - Actions: If you require non-deterministic behavior, such as high-entropy random values from crypto.getRandomValues(), you must perform this logic within a Convex Action, which is specifically designed to handle non-deterministic tasks, external API calls, and standard Node.js libraries [2][1]. You can then pass the results of the action back to a mutation using runMutation if state changes are required [2]. If you were referring to a different technical environment or a specific library's mutation handling, please clarify, as the term "mutation" is heavily used within the Convex ecosystem to refer to database write operations [3][4].
Citations:
- 1: https://docs.convex.dev/functions/runtimes
- 2: https://docs.convex.dev/api/interfaces/server.GenericActionCtx
- 3: https://docs.convex.dev/functions/mutation-functions
- 4: https://docs.convex.dev/api/interfaces/server.GenericMutationCtx
Avoid crypto.getRandomValues inside Convex mutation token generation
generateApiToken() in typescript/apps/beps/convex/users.ts uses crypto.getRandomValues(...) (~line 319) and is called from the Convex mutation handlers getOrCreateApiToken and regenerateApiToken (~lines 342-112). Convex requires queries/mutations to be deterministic for safe retries; non-deterministic Web Crypto randomness in these handlers can break that contract. If cryptographic randomness is required, generate the token in a Convex action and write it back via an internal mutation (or use Convex’s seeded Math.random if acceptable).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@typescript/apps/beps/convex/users.ts` around lines 314 - 333, The
generateApiToken function uses non-deterministic crypto.getRandomValues but is
invoked inside Convex mutation handlers getOrCreateApiToken and
regenerateApiToken; move the non-deterministic token generation out of mutations
by creating a new Convex action (e.g., createApiTokenAction) that calls
generateApiToken and returns the token, then update getOrCreateApiToken and
regenerateApiToken to accept a token parameter and perform only the
deterministic mutation (or alternatively replace generateApiToken with a
deterministic, seeded Math.random-based generator and keep calls in the
mutations); ensure references to generateApiToken, getOrCreateApiToken, and
regenerateApiToken are updated accordingly so mutations no longer call
crypto.getRandomValues directly.
1 participant
Summary
Math.random()withcrypto.getRandomValues().bep_prefix and 32-character alphanumeric token format.Testing
git diff --checknodesmoke test for token format/uniqueness using the same generator logicpnpm -C typescript/apps/beps exec tsc --noEmit --project convex/tsconfig.json(blocked locally: dependencies are not installed in this worktree, so Convex/Node modules cannot be resolved)Summary by CodeRabbit