feat(account-directory): Clerk-verified machine directory Worker#814
Conversation
New Cloudflare Worker (apps/account-directory) backing "see all my machines": a signed-in client registers its machine and lists machines with online/offline state. Verifies the caller's Clerk token via JWKS (jose, RS256, issuer-bound, 5s skew), requires sub, and accepts both the daemon OAuth token (aud/azp = client_id) and native session tokens (absent aud). D1 `machines` table scoped by user_id taken from the JWT (never the client); register upsert doubles as heartbeat + reachable- endpoint self-heal; list returns offline machines too (90s online window, online-first). 14 tests, typecheck, wrangler dry-run green. Deploy (D1 create + Worker vars + wrangler deploy) is a pending setup step. Phase 2 of the accounts/machine-directory initiative. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughAdds a Cloudflare Worker account directory backed by D1. It verifies Clerk JWTs, supports machine registration, listing, and deletion, computes online status, and includes migrations, configuration, documentation, and tests. ChangesAccount directory Worker
Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
@copilot review but do not make fixes |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@apps/account-directory/src/directory.ts`:
- Around line 127-139: Update parseRegisterInput to enforce explicit maximum
lengths for every registration string field and a maximum count for
reachableEndpoints, returning null for oversized payloads. In the registration
flow around the machine persistence logic, count existing machines for the
authenticated user and reject new registrations once the per-user machine quota
is reached, including unique machine keys; ensure these checks occur before
writing to D1.
- Around line 190-197: Update authenticate and handleRequest so verification
infrastructure failures from verifyCallerToken are propagated or classified
separately from invalid tokens, preserving 401 only for credential failures and
returning an appropriate 5xx response for JWKS, configuration, or key-fetch
outages. Do not convert all authenticate errors to null.
- Around line 156-167: Update isAllowedCallerToken to validate aud and azp
independently: require OAuth tokens with an audience to match the allowed
audience, without allowing a matching azp to bypass a mismatched aud. For tokens
with no aud, replace unconditional acceptance with an explicit allowlist of
permitted origin-based azp values, while preserving support for valid native
session-token shapes.
In `@apps/account-directory/wrangler.jsonc`:
- Around line 8-10: Add the required Clerk environment variables CLERK_JWKS_URL,
CLERK_ISSUER, and CLERK_OAUTH_CLIENT_ID to the vars configuration alongside
ONLINE_WINDOW_MS, using suitable development defaults or descriptive
placeholders that satisfy the Env contract and support local wrangler
development.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4fed74e5-49c3-4baa-a98d-4c62c321cb7a
⛔ Files ignored due to path filters (1)
apps/account-directory/package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.json
📒 Files selected for processing (11)
.gitignoreapps/account-directory/.gitignoreapps/account-directory/README.mdapps/account-directory/migrations/0001_machines.sqlapps/account-directory/package.jsonapps/account-directory/src/directory.tsapps/account-directory/src/index.tsapps/account-directory/test/directory.test.tsapps/account-directory/tsconfig.jsonapps/account-directory/wrangler.jsoncpackage.json
| function parseRegisterInput(value: unknown): RegisterInput | null { | ||
| if (!isRecord(value)) return null; | ||
| const machineKey = requiredString(value, "machineKey"); | ||
| const deviceId = requiredString(value, "deviceId"); | ||
| const name = requiredString(value, "name"); | ||
| const platform = requiredString(value, "platform"); | ||
| const deviceType = requiredString(value, "deviceType"); | ||
| const pubkey = optionalString(value, "pubkey"); | ||
| const reachableEndpoints = parseReachableEndpoints(value.reachableEndpoints); | ||
| if (!machineKey || !deviceId || !name || !platform || !deviceType || pubkey === undefined || !reachableEndpoints) { | ||
| return null; | ||
| } | ||
| return { machineKey, deviceId, name, platform, deviceType, pubkey, reachableEndpoints }; |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Bound per-user machine count and registration payload sizes.
A user can create unlimited unique machine keys and persist unbounded strings or endpoint arrays in the shared D1 database. Add per-user quotas plus limits for field lengths and endpoint count to prevent one authenticated account exhausting storage and degrading every tenant.
Also applies to: 255-280
🤖 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 `@apps/account-directory/src/directory.ts` around lines 127 - 139, Update
parseRegisterInput to enforce explicit maximum lengths for every registration
string field and a maximum count for reachableEndpoints, returning null for
oversized payloads. In the registration flow around the machine persistence
logic, count existing machines for the authenticated user and reject new
registrations once the per-user machine quota is reached, including unique
machine keys; ensure these checks occur before writing to D1.
| function isAllowedCallerToken(payload: JWTPayload, oauthClientId: string): boolean { | ||
| // Clerk's native session tokens have no audience. Their `azp` may be absent, | ||
| // empty, or origin-based, so `azp` alone must not reject that token shape. | ||
| if (payload.aud === undefined) return true; | ||
|
|
||
| // OAuth access tokens are audience/authorized-party bound to the ADE client. | ||
| // Future fixed audiences (for example `ade-relay`) can be added to this list. | ||
| const allowedAudiences = [oauthClientId]; | ||
| return allowedAudiences.some((allowed) => | ||
| audienceIncludes(payload.aud, allowed) || payload.azp === allowed | ||
| ); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the file around the referenced lines and locate related token-validation helpers.
FILE='apps/account-directory/src/directory.ts'
echo '--- file outline ---'
ast-grep outline "$FILE" --view expanded || true
echo '--- relevant slice ---'
sed -n '120,210p' "$FILE"
echo '--- search for audienceIncludes / azp / JWTPayload usage ---'
rg -n "audienceIncludes|azp|JWTPayload|isAllowedCallerToken|oauthClientId" apps/account-directory/srcRepository: arul28/ADE
Length of output: 7237
🌐 Web query:
Clerk session tokens audience azp authorized-party access token audience meaning docs
💡 Result:
In Clerk's authentication system, the azp and aud claims in session tokens serve distinct, specific purposes for security and validation. The azp (Authorized Party) claim is a standard JWT claim used to identify the authorized party to which the token was issued [1][2]. In the context of Clerk, this claim typically contains the origin (e.g., https://example.com or http://localhost:3000) of the frontend application that generated the session [3][4][5]. When verifying a session token on your backend, validating the azp claim is a security best practice to prevent subdomain cookie leaking and cross-site request forgery (CSRF) attacks [3][6][2]. By configuring an allowlist of permitted origins (using the authorizedParties option in Clerk's verification utilities), you ensure that your application only accepts tokens originating from trusted domains [3][6][2]. The aud (Audience) claim identifies the intended recipients of the JWT [6][7]. While Clerk's default session tokens may not always require an audience check, this claim becomes relevant if you use Clerk to generate custom JWTs (e.g., for integrating with third-party services like MongoDB Atlas) that require specific audience values [4][7][8]. When an aud claim is present in a token, it is a recommended security practice to validate it against your expected audience values on the backend to ensure the token is intended for your specific service [6][9][8]. In summary: - azp (Authorized Party): Specifies the origin of the frontend application. It is primarily used to protect against unauthorized origin usage [3][2]. - aud (Audience): Specifies the intended recipient of the token. It is used to ensure the token was generated for your specific service or integration [6][7]. When verifying tokens using Clerk SDKs (such as verifyToken), you can pass both authorizedParties (for azp validation) and audience (for aud validation) to ensure robust security [3][6][10].
Citations:
- 1: https://clerk.com/docs/guides/sessions/manual-jwt-verification
- 2: https://github.com/clerk/clerk-docs/blob/126650ba/docs/backend-requests/manual-jwt.mdx
- 3: https://clerk.com/docs/reference/backend/verify-token
- 4: https://clerk.com/docs/guides/sessions/jwt-templates
- 5: https://clerk.com/docs/reference/backend/types/auth-object
- 6: https://github.com/clerk/javascript/blob/4fae43c0/packages/backend/src/jwt/verifyJwt.ts
- 7: feat(backend): Propagate audience option to verifyToken clerk/javascript#978
- 8: If custom "aud" claim is added to session token, getAuth() fails in getServerSideProps clerk/javascript#1507
- 9: If custom "aud" claim is added to session token, getAuth() fails in getServerSideProps clerk/javascript#1507
- 10: https://github.com/clerk/clerk-sdk-python/blob/e3637603/src/clerk_backend_api/security/types.py
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE='apps/account-directory/src/directory.ts'
python3 - <<'PY'
from pathlib import Path
p = Path('apps/account-directory/src/directory.ts')
text = p.read_text()
lines = text.splitlines()
for i in range(140, 190):
if i <= len(lines):
print(f"{i:4}: {lines[i-1]}")
PY
echo '--- related symbols ---'
rg -n "function audienceIncludes|function isAllowedCallerToken|payload\.aud|payload\.azp|oauthClientId" apps/account-directory/src/directory.tsRepository: arul28/ADE
Length of output: 3117
Separate aud and azp validation. azp must not satisfy a mismatched OAuth audience, and session tokens without aud need an explicit origin allowlist instead of unconditional acceptance.
🤖 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 `@apps/account-directory/src/directory.ts` around lines 156 - 167, Update
isAllowedCallerToken to validate aud and azp independently: require OAuth tokens
with an audience to match the allowed audience, without allowing a matching azp
to bypass a mismatched aud. For tokens with no aud, replace unconditional
acceptance with an explicit allowlist of permitted origin-based azp values,
while preserving support for valid native session-token shapes.
| async function authenticate(request: Request, env: Env): Promise<string | null> { | ||
| const token = readBearerToken(request); | ||
| if (!token) return null; | ||
| try { | ||
| return await verifyCallerToken(token, env); | ||
| } catch { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map relevant files
git ls-files 'apps/account-directory/src/*' | sed -n '1,120p'
printf '\n--- outline directory.ts ---\n'
ast-grep outline apps/account-directory/src/directory.ts --view expanded || true
printf '\n--- relevant grep ---\n'
rg -n "async function authenticate|verifyCallerToken|JWKS|jwks|Unauthorized|401|return null|throw" apps/account-directory/src -SRepository: arul28/ADE
Length of output: 5301
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the relevant function and dependencies with line numbers.
sed -n '1,280p' apps/account-directory/src/directory.ts | cat -n
printf '\n--- verifyCallerToken references ---\n'
rg -n "function verifyCallerToken|const verifyCallerToken|async function verifyCallerToken|verifyCallerToken\(" apps/account-directory/src -SRepository: arul28/ADE
Length of output: 11442
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Focus on the authentication flow and the caller that turns a null result into 401.
sed -n '180,260p' apps/account-directory/src/directory.ts | cat -n
printf '\n--- request handlers using authenticate() ---\n'
rg -n "authenticate\(" apps/account-directory/src/directory.ts apps/account-directory/src -SRepository: arul28/ADE
Length of output: 3845
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '320,350p' apps/account-directory/src/directory.ts | cat -nRepository: arul28/ADE
Length of output: 1072
Do not collapse verification outages into 401
authenticate() catches every verifyCallerToken() error, and handleRequest() turns the resulting null into 401 Unauthorized. That makes JWKS/config/key-fetch failures look like bad credentials; return 5xx for those cases so clients can retry.
🤖 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 `@apps/account-directory/src/directory.ts` around lines 190 - 197, Update
authenticate and handleRequest so verification infrastructure failures from
verifyCallerToken are propagated or classified separately from invalid tokens,
preserving 401 only for credential failures and returning an appropriate 5xx
response for JWKS, configuration, or key-fetch outages. Do not convert all
authenticate errors to null.
| "vars": { | ||
| "ONLINE_WINDOW_MS": "90000" | ||
| }, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add required Clerk environment variables to vars.
Based on the Env interface contract, the worker requires CLERK_JWKS_URL, CLERK_ISSUER, and CLERK_OAUTH_CLIENT_ID to verify incoming tokens. Since they are omitted here, local development via wrangler dev will fail during authentication unless the developer manually adds them to a .dev.vars file.
Consider defining them in this configuration with development defaults or descriptive placeholders to document the expected local environment surface clearly.
⚙️ Proposed addition
"vars": {
- "ONLINE_WINDOW_MS": "90000"
+ "ONLINE_WINDOW_MS": "90000",
+ "CLERK_JWKS_URL": "...",
+ "CLERK_ISSUER": "...",
+ "CLERK_OAUTH_CLIENT_ID": "..."
},📝 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.
| "vars": { | |
| "ONLINE_WINDOW_MS": "90000" | |
| }, | |
| "vars": { | |
| "ONLINE_WINDOW_MS": "90000", | |
| "CLERK_JWKS_URL": "...", | |
| "CLERK_ISSUER": "...", | |
| "CLERK_OAUTH_CLIENT_ID": "..." | |
| }, |
🤖 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 `@apps/account-directory/wrangler.jsonc` around lines 8 - 10, Add the required
Clerk environment variables CLERK_JWKS_URL, CLERK_ISSUER, and
CLERK_OAUTH_CLIENT_ID to the vars configuration alongside ONLINE_WINDOW_MS,
using suitable development defaults or descriptive placeholders that satisfy the
Env contract and support local wrangler development.
|
Ship review disposition — CI green (32 pass / 1 skip), Greptile + Copilot clean, coordinator already reviewed security + correctness. CodeRabbit's 4 comments are all non-blocking (
Package validated locally (CI does not exercise |
Phase 2 · Relay machine directory (Cloudflare Worker + D1)
Part of "sign in once → see all your machines." A signed-in ADE machine registers itself; a signed-in client lists its machines with online/offline state. Standalone new Worker — no runtime dependency on the identity PR (it only verifies Clerk-issued tokens).
What it does
apps/account-directory(modeled onapps/webhook-relay):POST /account/machines/register,GET /account/machines,DELETE /account/machines/:machineKey,GET /health.jose, RS256, issuer-bound, 5s clock tolerance, mandatorysub). Accepts both token shapes: the daemon OAuth access token (aud/azp === client_id) and native session tokens (absentaud); one-line seam to add anade-relayaudience later.machinestable scoped byuser_idtaken from the verified JWT, never the client (no IDOR). Register is an upsert that doubles as heartbeat + reachable-endpoint self-heal.lastSeenAt+ reachable endpoints), computed from a 90s window (ONLINE_WINDOW_MS), sorted online-first. Leaves a seam for a future hosted always-on runtime.Testing
14 unit tests (JWKS verify happy/expired/bad-sig/missing-sub/aud-allowlist; register upsert + cross-user isolation; online computation + offline-still-listed; delete scoping). Typecheck +
wranglerdry-run build green.Deploy (pending setup)
D1 database creation, Worker vars (
CLERK_JWKS_URL/CLERK_ISSUER/CLERK_OAUTH_CLIENT_ID), applying the migration, andwrangler deployare follow-up setup steps (need Cloudflare account access).🤖 Generated with Claude Code
Greptile Summary
This PR adds a standalone account directory Worker for Clerk-authenticated machine discovery. The main changes are:
apps/account-directoryfor machine registration, listing, deletion, and health checks.machinestable keyed byuser_idandmachine_keywith account-scoped queries.last_seen_atandONLINE_WINDOW_MS.Confidence Score: 5/5
Safe to merge with minimal risk.
The Worker is self-contained and uses verified JWT subjects for data scoping. D1 access uses parameterized queries, and tests cover the main auth, isolation, registration, listing, and deletion paths. No blocking correctness or security issues were identified.
No files require special attention.
What T-Rex did
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Client as ADE client/machine participant Worker as account-directory Worker participant Clerk as Clerk JWKS participant D1 as D1 machines table Client->>Worker: "Request /account/machines* with Bearer JWT" Worker->>Clerk: Verify RS256 token via configured JWKS/issuer Clerk-->>Worker: Verified payload with sub alt POST /account/machines/register Worker->>D1: "Upsert machine row scoped by user_id=sub" D1-->>Worker: Stored machine record Worker-->>Client: Machine record else GET /account/machines Worker->>D1: "Select machines where user_id=sub" D1-->>Worker: Machine rows Worker-->>Client: Machines sorted online-first else DELETE /account/machines/:machineKey Worker->>D1: "Delete where user_id=sub and machine_key" D1-->>Worker: Delete result Worker-->>Client: "{ ok: true, machineKey }" end%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant Client as ADE client/machine participant Worker as account-directory Worker participant Clerk as Clerk JWKS participant D1 as D1 machines table Client->>Worker: "Request /account/machines* with Bearer JWT" Worker->>Clerk: Verify RS256 token via configured JWKS/issuer Clerk-->>Worker: Verified payload with sub alt POST /account/machines/register Worker->>D1: "Upsert machine row scoped by user_id=sub" D1-->>Worker: Stored machine record Worker-->>Client: Machine record else GET /account/machines Worker->>D1: "Select machines where user_id=sub" D1-->>Worker: Machine rows Worker-->>Client: Machines sorted online-first else DELETE /account/machines/:machineKey Worker->>D1: "Delete where user_id=sub and machine_key" D1-->>Worker: Delete result Worker-->>Client: "{ ok: true, machineKey }" endReviews (1): Last reviewed commit: "feat(account-directory): Clerk-verified ..." | Re-trigger Greptile
Summary by CodeRabbit
New Features
Documentation