feat(nextjs) Environment Drift Telemetry Event for Keyless Applications #6522
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
heatlikeheatwave
changed the title
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
📝 WalkthroughWalkthroughIntroduces a new server-only module packages/nextjs/src/server/keyless-telemetry.ts that exports detectKeylessEnvDrift(): Promise. It reads keyless configuration and environment variables, compares values to detect drift, and, if detected, emits a single Clerk telemetry event per app instance using a file-based guard. The function uses dynamic imports, filesystem checks/writes, and wraps operations in error handling. Additionally, packages/nextjs/src/app-router/keyless-actions.ts now invokes detectKeylessEnvDrift() on the non-matching-key path within createOrReadKeylessAction, without changing exported signatures. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
packages/nextjs/src/server/keyless-telemetry.ts (5)
62-70: JSDoc mentions a NOT_DETECTED event that isn’t implementedEither remove the NOT_DETECTED reference or implement the complementary event to match docs.
Suggested change:
- * If there's a mismatch, it fires a `KEYLESS_ENV_DRIFT_DETECTED` event. - * For local testing purposes, it also fires a `KEYLESS_ENV_DRIFT_NOT_DETECTED` event when - * keys exist and match the environment variables. + * If there's a mismatch, it fires a `KEYLESS_ENV_DRIFT_DETECTED` event.
2-3: Use node: scheme for built-ins (optional)Prefer node:fs and node:path for clarity and tree-shaking friendliness in modern Node/TS setups.
Apply:
-import { promises as fs } from 'fs'; -import { join } from 'path'; +import { promises as fs } from 'node:fs'; +import { join } from 'node:path';
7-9: Narrow constant typesMark event name and sampling rate as const to keep literal types where helpful.
Apply:
-const EVENT_KEYLESS_ENV_DRIFT_DETECTED = 'KEYLESS_ENV_DRIFT_DETECTED'; -const EVENT_SAMPLING_RATE = 1; // 100% sampling rate +const EVENT_KEYLESS_ENV_DRIFT_DETECTED = 'KEYLESS_ENV_DRIFT_DETECTED' as const; +const EVENT_SAMPLING_RATE = 1 as const; // 100% sampling rate
145-148: Use the package’s logger (if available) for consistent diagnosticsConsole.warn works, but prefer the internal logger to integrate with existing log levels and silencing rules.
1-150: Tests missing for the new telemetry behaviorPlease add tests covering:
- drift detected -> emits event once (atomic flag ensures single emission),
- no drift -> no event,
- env missing + keyless file has keys -> early return,
- missing keys in keyless file -> safe no-op,
- masking behavior on keys.
Do you want me to scaffold unit tests (with fs and keyless-node mocks) for this module?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/nextjs/src/app-router/keyless-actions.ts(2 hunks)packages/nextjs/src/server/keyless-telemetry.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/nextjs/src/app-router/keyless-actions.tspackages/nextjs/src/server/keyless-telemetry.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/nextjs/src/app-router/keyless-actions.tspackages/nextjs/src/server/keyless-telemetry.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/nextjs/src/app-router/keyless-actions.tspackages/nextjs/src/server/keyless-telemetry.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/nextjs/src/app-router/keyless-actions.tspackages/nextjs/src/server/keyless-telemetry.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/nextjs/src/app-router/keyless-actions.tspackages/nextjs/src/server/keyless-telemetry.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/nextjs/src/app-router/keyless-actions.tspackages/nextjs/src/server/keyless-telemetry.ts
**/*
⚙️ CodeRabbit Configuration File
If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.
Files:
packages/nextjs/src/app-router/keyless-actions.tspackages/nextjs/src/server/keyless-telemetry.ts
🧬 Code Graph Analysis (2)
packages/nextjs/src/app-router/keyless-actions.ts (1)
packages/nextjs/src/server/keyless-telemetry.ts (1)
detectKeylessEnvDrift(73-149)
packages/nextjs/src/server/keyless-telemetry.ts (3)
packages/nextjs/src/server/keyless-node.ts (1)
safeParseClerkFile(52-66)packages/nextjs/src/server/createClerkClient.ts (1)
createClerkClientWithOptions(34-35)packages/types/src/telemetry.ts (1)
TelemetryEventRaw(40-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep/ci
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
packages/nextjs/src/app-router/keyless-actions.ts (1)
65-67: Mismatch with summary: this runs unconditionally after result exists, not only on a “non-matching key” pathThe AI summary states the call happens only on the “non-matching-key path,” but here it executes for every successful create/read. Confirm this is intended.
packages/nextjs/src/server/keyless-telemetry.ts (1)
1-150: No action required: Event name and env-var usage are compatible
- TelemetryEvent.event is defined as a string, so
KEYLESS_ENV_DRIFT_DETECTEDis accepted by the telemetry system.- Across Next.js packages, environment variables are consistently read via
process.env.*(e.g. inmergeNextClerkPropsWithEnv.tsandserver/constants.ts); there is no shared utility to replace.All checks pass—no changes needed.
| async function hasTelemetryEventBeenFired(): Promise<boolean> { | ||
| try { | ||
| await fs.access(getTelemetryFlagFilePath()); | ||
| return true; | ||
| } catch { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Race condition: two-phase “check then mark” can emit duplicate events under concurrency
Between hasTelemetryEventBeenFired and markTelemetryEventAsFired, concurrent calls can both emit before the flag is written. Use an atomic “create-if-not-exists” guard and emit only if you successfully create the flag.
Minimal restructure:
- Replace the two helpers with a single atomic one that attempts to create the flag file using { flag: 'wx' }.
- In detectKeylessEnvDrift, after hasDrift is computed, attempt to acquire the flag atomically; only the winner emits.
Apply:
-async function hasTelemetryEventBeenFired(): Promise<boolean> {
- try {
- await fs.access(getTelemetryFlagFilePath());
- return true;
- } catch {
- return false;
- }
-}
+async function tryMarkTelemetryEventAsFired(): Promise<boolean> {
+ try {
+ const dir = join(process.cwd(), '.clerk/.tmp');
+ await fs.mkdir(dir, { recursive: true });
+ const flagData = {
+ firedAt: new Date().toISOString(),
+ event: EVENT_KEYLESS_ENV_DRIFT_DETECTED,
+ };
+ await fs.writeFile(getTelemetryFlagFilePath(), JSON.stringify(flagData, null, 2), { flag: 'wx' });
+ return true; // we created the flag; we are the single emitter
+ } catch (error: unknown) {
+ // EEXIST -> someone else won the race; silently ignore
+ return false;
+ }
+}And:
- // Check if telemetry event has already been fired
- if (await hasTelemetryEventBeenFired()) {
- return;
- }
+ // No-op here; atomic flag is taken right before emitting to avoid racesAnd:
- if (hasDrift) {
- // Fire drift detected event
+ if (hasDrift) {
+ // Acquire single-emission lock (atomic file create). Only the winner should emit.
+ const shouldEmit = await tryMarkTelemetryEventAsFired();
+ if (!shouldEmit) {
+ return;
+ }
+ // Fire drift detected event
const driftDetectedEvent: TelemetryEventRaw<EventKeylessEnvDriftPayload> = {
event: EVENT_KEYLESS_ENV_DRIFT_DETECTED,
eventSamplingRate: EVENT_SAMPLING_RATE,
payload,
};
-
- clerkClient.telemetry?.record(driftDetectedEvent);
-
- // Mark the telemetry event as fired to prevent future executions
- await markTelemetryEventAsFired();
+ clerkClient.telemetry?.record(driftDetectedEvent);
}Also applies to: 42-59, 132-145
🤖 Prompt for AI Agents
In packages/nextjs/src/server/keyless-telemetry.ts around lines 30-37 (and also
apply same pattern to 42-59 and 132-145), replace the current two-phase
hasTelemetryEventBeenFired + markTelemetryEventAsFired pattern with a single
atomic helper that tries to create the telemetry flag file using
fs.writeFile(getTelemetryFlagFilePath(), '', { flag: 'wx' }) (or the equivalent
atomic create API); return true only if the write succeeded (meaning this
process won the race) and false if it failed due to EEXIST; then update
detectKeylessEnvDrift to compute hasDrift and immediately call the new atomic
helper and emit the telemetry event only when the helper indicates it
successfully created the flag (winner), ensuring no duplicate emits under
concurrency.
| const envPublishableKey = process.env.NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY; | ||
| const envSecretKey = process.env.CLERK_SECRET_KEY; | ||
|
|
||
| // Check if environment variables are missing, and keys exist in keyless file | ||
| const envVarsMissing = !envPublishableKey && !envSecretKey; | ||
| const keylessFileHasKeys = Boolean(keylessFile?.publishableKey && keylessFile?.secretKey); | ||
|
|
||
| if (envVarsMissing && keylessFileHasKeys) { |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Support all recognized Clerk env var names for publishable key
Per guidelines, support multiple env var names. At minimum, also check CLERK_PUBLISHABLE_KEY.
Apply:
-const envPublishableKey = process.env.NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY;
+const envPublishableKey =
+ process.env.NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY ?? process.env.CLERK_PUBLISHABLE_KEY;Optionally, centralize via an existing env util if the repo has one.
Add support for CLERK_PUBLISHABLE_KEY alongside NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY
To ensure we recognize both official env var names for the publishable key, update the assignment in packages/nextjs/src/server/keyless-telemetry.ts (lines 96–103):
- const envPublishableKey = process.env.NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY;
+ const envPublishableKey =
+ process.env.NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY ??
+ process.env.CLERK_PUBLISHABLE_KEY;• File: packages/nextjs/src/server/keyless-telemetry.ts
• Lines: 96–103
There isn’t an existing environment‐variable helper in this package, so inlining the fallback here is fine. If you’d prefer to centralize, you could introduce an env utility (e.g. packages/nextjs/src/server/env.ts) to handle all Clerk-related vars.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const envPublishableKey = process.env.NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY; | |
| const envSecretKey = process.env.CLERK_SECRET_KEY; | |
| // Check if environment variables are missing, and keys exist in keyless file | |
| const envVarsMissing = !envPublishableKey && !envSecretKey; | |
| const keylessFileHasKeys = Boolean(keylessFile?.publishableKey && keylessFile?.secretKey); | |
| if (envVarsMissing && keylessFileHasKeys) { | |
| const envPublishableKey = | |
| process.env.NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY ?? | |
| process.env.CLERK_PUBLISHABLE_KEY; | |
| const envSecretKey = process.env.CLERK_SECRET_KEY; | |
| // Check if environment variables are missing, and keys exist in keyless file | |
| const envVarsMissing = !envPublishableKey && !envSecretKey; | |
| const keylessFileHasKeys = Boolean(keylessFile?.publishableKey && keylessFile?.secretKey); | |
| if (envVarsMissing && keylessFileHasKeys) { |
🤖 Prompt for AI Agents
In packages/nextjs/src/server/keyless-telemetry.ts around lines 96–103, the code
only reads NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY for the publishable key; change the
assignment to accept either NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY or
CLERK_PUBLISHABLE_KEY (i.e. use the latter as a fallback), keeping the existing
secret key lookup and subsequent envVarsMissing/keylessFileHasKeys logic
unchanged; inlining this fallback is fine since there is no shared env helper in
this package.
| const payload: EventKeylessEnvDriftPayload = { | ||
| publicKeyMatch, | ||
| secretKeyMatch, | ||
| envVarsMissing, | ||
| keylessFileHasKeys, | ||
| keylessPublishableKey: keylessFile.publishableKey, | ||
| envPublishableKey: envPublishableKey as string, | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Avoid casting undefined to string; consider masking publishable key before sending
envPublishableKey can be undefined. Casting hides a real mismatch and violates the declared payload type. Also, avoid sending full keys in telemetry; include a masked version.
Apply:
-type EventKeylessEnvDriftPayload = {
+type EventKeylessEnvDriftPayload = {
publicKeyMatch: boolean;
secretKeyMatch: boolean;
envVarsMissing: boolean;
keylessFileHasKeys: boolean;
- keylessPublishableKey: string;
- envPublishableKey: string;
+ keylessPublishableKeyMasked?: string;
+ envPublishableKeyMasked?: string;
};And:
-const payload: EventKeylessEnvDriftPayload = {
+const payload: EventKeylessEnvDriftPayload = {
publicKeyMatch,
secretKeyMatch,
envVarsMissing,
keylessFileHasKeys,
- keylessPublishableKey: keylessFile.publishableKey,
- envPublishableKey: envPublishableKey as string,
+ keylessPublishableKeyMasked: maskKeyForTelemetry(keylessFile.publishableKey),
+ envPublishableKeyMasked: maskKeyForTelemetry(envPublishableKey),
};Add helper (outside the ranges):
function maskKeyForTelemetry(key?: string): string | undefined {
if (!key) return undefined;
// Keep a small prefix/suffix for correlation; adjust length as needed
const head = key.slice(0, 8);
const tail = key.slice(-4);
return `${head}…${tail}`;
}🏁 Script executed:
#!/bin/bash
set -e
# Locate the type definition for EventKeylessEnvDriftPayload
rg -C3 "type EventKeylessEnvDriftPayload" -n packages/nextjs/src/server/keyless-telemetry.ts
# Show the payload instantiation to verify casting
rg -C3 "const payload: EventKeylessEnvDriftPayload" -n packages/nextjs/src/server/keyless-telemetry.ts
# Check if a masking helper already exists
rg "maskKeyForTelemetry" -n packages/nextjs/src/server/keyless-telemetry.ts || echo "maskKeyForTelemetry helper not found"Length of output: 910
Mask publishable keys and handle undefined in Telemetry payload
Casting envPublishableKey to string can hide real mismatches and violates the declared payload type. We also shouldn’t send full publishable keys in telemetry. Please:
• Update the EventKeylessEnvDriftPayload type in packages/nextjs/src/server/keyless-telemetry.ts
– Replace
ts keylessPublishableKey: string; envPublishableKey: string;
with
ts keylessPublishableKeyMasked?: string; envPublishableKeyMasked?: string;
• Change the payload instantiation (around line 117) to use a masking helper:
const payload: EventKeylessEnvDriftPayload = {
publicKeyMatch,
secretKeyMatch,
envVarsMissing,
keylessFileHasKeys,
- keylessPublishableKey: keylessFile.publishableKey,
- envPublishableKey: envPublishableKey as string,
+ keylessPublishableKeyMasked: maskKeyForTelemetry(keylessFile.publishableKey),
+ envPublishableKeyMasked: maskKeyForTelemetry(envPublishableKey),
};• Add a helper function (e.g. at the bottom of the file) to mask keys:
function maskKeyForTelemetry(key?: string): string | undefined {
if (!key) return undefined;
const head = key.slice(0, 8);
const tail = key.slice(-4);
return `${head}…${tail}`;
}This ensures we don’t send raw keys and correctly represent missing values.
🤖 Prompt for AI Agents
In packages/nextjs/src/server/keyless-telemetry.ts around lines 117 to 124, the
telemetry payload currently casts envPublishableKey to string and sends raw
publishable keys; update the EventKeylessEnvDriftPayload type to replace
keylessPublishableKey and envPublishableKey string fields with optional
keylessPublishableKeyMasked?: string and envPublishableKeyMasked?: string,
change the payload instantiation to stop casting and instead assign masked
values using a masking helper (e.g., keylessPublishableKeyMasked:
maskKeyForTelemetry(keylessFile.publishableKey), envPublishableKeyMasked:
maskKeyForTelemetry(envPublishableKey)), and add the helper function at the
bottom of the file that returns undefined for falsy input and otherwise returns
the first 8 chars + ellipsis + last 4 chars; ensure no raw keys are logged or
sent and types match the updated payload.
| const clerkClient = createClerkClientWithOptions({ | ||
| publishableKey: keylessFile.publishableKey, | ||
| secretKey: keylessFile.secretKey, | ||
| }); | ||
|
|
There was a problem hiding this comment.
Guard against missing keys in keyless file before creating the client
safeParseClerkFile can parse objects without both keys. Avoid passing undefined to the client.
Apply:
-// Create a clerk client to access telemetry
-const clerkClient = createClerkClientWithOptions({
- publishableKey: keylessFile.publishableKey,
- secretKey: keylessFile.secretKey,
-});
+if (!keylessFile.publishableKey || !keylessFile.secretKey) {
+ return;
+}
+// Create a clerk client to access telemetry
+const clerkClient = createClerkClientWithOptions({
+ publishableKey: keylessFile.publishableKey,
+ secretKey: keylessFile.secretKey,
+});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const clerkClient = createClerkClientWithOptions({ | |
| publishableKey: keylessFile.publishableKey, | |
| secretKey: keylessFile.secretKey, | |
| }); | |
| if (!keylessFile.publishableKey || !keylessFile.secretKey) { | |
| return; | |
| } | |
| // Create a clerk client to access telemetry | |
| const clerkClient = createClerkClientWithOptions({ | |
| publishableKey: keylessFile.publishableKey, | |
| secretKey: keylessFile.secretKey, | |
| }); |
Description
KEYLESS_ENV_DRIFT_DETECTEDto detect drift between publishable and secret keys in keyless apps and values in the.envfile.Example event payload:
{ "envPublishableKey":"pk_test_[redacted]", "envVarsMissing":false, "keylessFileHasKeys":true, "keylessPublishableKey":"pk_test_[redacted]", "publicKeyMatch":false, "secretKeyMatch":false }.clerk/.tmp/keyless-telemetry-fired.jsonfile) to prevent noiseChecklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Chores