chore(backend): Add scoping and secret key retrieval to machines BAPI #6417
Conversation
🦋 Changeset detectedLatest commit: 6b38d86 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 Walkthrough""" WalkthroughThis change set introduces new functionality to the Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes were found. All code changes are directly related to the objectives described in the linked issue. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. 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: 1
🧹 Nitpick comments (1)
packages/backend/src/api/endpoints/MachineApi.ts (1)
114-126: Consider parameter naming consistency.The method implementation is solid, but there's an inconsistency in parameter naming:
createScopeusestoMachineIddeleteScopeusesotherMachineIdFor better API consistency, consider using the same parameter name in both methods.
- async deleteScope(machineId: string, otherMachineId: string) { + async deleteScope(machineId: string, toMachineId: string) { this.requireId(machineId); return this.request<MachineScope>({ method: 'DELETE', - path: joinPaths(basePath, machineId, 'scopes', otherMachineId), + path: joinPaths(basePath, machineId, 'scopes', toMachineId), }); }Also update the JSDoc parameter name:
- * @param otherMachineId - The ID of the machine that is being accessed. + * @param toMachineId - The ID of the machine that is being accessed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.changeset/five-jokes-clap.md(1 hunks)packages/backend/src/api/endpoints/MachineApi.ts(2 hunks)packages/backend/src/api/resources/Deserializer.ts(2 hunks)packages/backend/src/api/resources/JSON.ts(2 hunks)packages/backend/src/api/resources/MachineScope.ts(1 hunks)packages/backend/src/api/resources/MachineSecretKey.ts(1 hunks)packages/backend/src/api/resources/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{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/backend/src/api/resources/Deserializer.tspackages/backend/src/api/resources/index.tspackages/backend/src/api/resources/MachineSecretKey.tspackages/backend/src/api/resources/MachineScope.tspackages/backend/src/api/resources/JSON.tspackages/backend/src/api/endpoints/MachineApi.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/backend/src/api/resources/Deserializer.tspackages/backend/src/api/resources/index.tspackages/backend/src/api/resources/MachineSecretKey.tspackages/backend/src/api/resources/MachineScope.tspackages/backend/src/api/resources/JSON.tspackages/backend/src/api/endpoints/MachineApi.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/backend/src/api/resources/Deserializer.tspackages/backend/src/api/resources/index.tspackages/backend/src/api/resources/MachineSecretKey.tspackages/backend/src/api/resources/MachineScope.tspackages/backend/src/api/resources/JSON.tspackages/backend/src/api/endpoints/MachineApi.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/backend/src/api/resources/Deserializer.tspackages/backend/src/api/resources/index.tspackages/backend/src/api/resources/MachineSecretKey.tspackages/backend/src/api/resources/MachineScope.tspackages/backend/src/api/resources/JSON.tspackages/backend/src/api/endpoints/MachineApi.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/backend/src/api/resources/Deserializer.tspackages/backend/src/api/resources/index.tspackages/backend/src/api/resources/MachineSecretKey.tspackages/backend/src/api/resources/MachineScope.tspackages/backend/src/api/resources/JSON.tspackages/backend/src/api/endpoints/MachineApi.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/backend/src/api/resources/Deserializer.tspackages/backend/src/api/resources/index.tspackages/backend/src/api/resources/MachineSecretKey.tspackages/backend/src/api/resources/MachineScope.tspackages/backend/src/api/resources/JSON.tspackages/backend/src/api/endpoints/MachineApi.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.Whenever reviewing a pull request, if there are any changes that could impact security, always tag
@clerk/securityin the PR.Security-impacting changes include, but are not limited to:
- Changes to authentication logic or mechanisms (e.g. login, session handling, token issuance)
- Any modification to access control, authorization checks, or role-based permissions
- Introduction or modification of hashing algorithms, signature verification, or cryptographic primitives
- Handling of sensitive data (e.g. passwords, tokens, secrets, PII)
- Integration with external identity providers (e.g. SSO, OAuth, OpenID Connect)
- Modifications to security headers, cookie flags, CORS policies, or CSRF protections
- Bypass mechanisms (e.g. feature flags, testing overrides) that could weaken protections
- Changes to rate limiting, abuse prevention, or input validation
If you're unsure whether a change is security-relevant, err on the side of caution and tag
@clerk/security.Any time that you tag
@clerk/security, please do so explicitly in a code comment, rather than within a collapsed section in a coderabbit comment, such as the "recent review details" section. If you do use the team name in any thinking or non-direct-code-comment content, it can be referred to as "clerk security team" to avoid accidentally printing the tag which sends a notification to the team.
Files:
packages/backend/src/api/resources/Deserializer.tspackages/backend/src/api/resources/index.tspackages/backend/src/api/resources/MachineSecretKey.tspackages/backend/src/api/resources/MachineScope.tspackages/backend/src/api/resources/JSON.tspackages/backend/src/api/endpoints/MachineApi.ts
packages/**/index.{js,ts}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use tree-shaking friendly exports
Files:
packages/backend/src/api/resources/index.ts
**/index.ts
📄 CodeRabbit Inference Engine (.cursor/rules/react.mdc)
Use index.ts files for clean imports but avoid deep barrel exports
Avoid barrel files (index.ts re-exports) as they can cause circular dependencies
Files:
packages/backend/src/api/resources/index.ts
.changeset/**
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Automated releases must use Changesets.
Files:
.changeset/five-jokes-clap.md
🧬 Code Graph Analysis (4)
packages/backend/src/api/resources/Deserializer.ts (3)
packages/backend/src/api/resources/JSON.ts (2)
ObjectType(19-72)ObjectType(74-74)packages/backend/src/api/resources/MachineScope.ts (1)
MachineScope(3-14)packages/backend/src/api/resources/MachineSecretKey.ts (1)
MachineSecretKey(3-9)
packages/backend/src/api/resources/MachineSecretKey.ts (1)
packages/backend/src/api/resources/JSON.ts (1)
MachineSecretKeyJSON(725-728)
packages/backend/src/api/resources/MachineScope.ts (1)
packages/backend/src/api/resources/JSON.ts (1)
MachineScopeJSON(717-723)
packages/backend/src/api/endpoints/MachineApi.ts (2)
packages/backend/src/api/resources/MachineSecretKey.ts (1)
MachineSecretKey(3-9)packages/backend/src/api/resources/MachineScope.ts (1)
MachineScope(3-14)
⏰ 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: pr-title-lint
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep/ci
🔇 Additional comments (15)
packages/backend/src/api/resources/MachineSecretKey.ts (2)
3-9: LGTM! Proper resource class structure with security considerations.The
MachineSecretKeyclass follows the established pattern with readonly properties and staticfromJSONdeserialization. Since this class handles sensitive secret key data, please ensure proper security measures are in place upstream.Security Review Required: This change introduces handling of machine secret keys, which is security-sensitive functionality. @clerk/security should review this implementation to ensure proper security controls are in place for secret key handling, storage, and transmission.
1-1: Fix the import statement.The import should be
MachineSecretKeyJSONinstead ofMachineScopeJSONbased on the usage in thefromJSONmethod.-import type { MachineSecretKeyJSON } from './JSON'; +import type { MachineSecretKeyJSON } from './JSON';Wait, let me check this again. The import statement shows
MachineSecretKeyJSONwhich is correct. This appears to be a display issue in my analysis.packages/backend/src/api/resources/index.ts (1)
34-35: LGTM! Proper export additions.The new exports for
MachineScopeandMachineSecretKeyare correctly placed and follow the established alphabetical ordering pattern.packages/backend/src/api/resources/Deserializer.ts (2)
19-20: LGTM! Proper import additions.The new imports for
MachineScopeandMachineSecretKeyare correctly placed in alphabetical order.
140-143: LGTM! Consistent deserialization pattern.The new switch cases for
MachineScopeandMachineSecretKeyfollow the established pattern by calling their respectivefromJSONstatic methods.packages/backend/src/api/resources/MachineScope.ts (1)
3-14: LGTM! Well-structured resource class.The
MachineScopeclass follows established patterns with:
- Readonly properties in the constructor
- Proper TypeScript typing with optional parameters
- Consistent snake_case to camelCase mapping in
fromJSON- Clean separation of concerns
This properly represents machine-to-machine authorization scopes.
.changeset/five-jokes-clap.md (1)
1-16: LGTM! Clear changeset documentation.The changeset properly documents the new machine scope and secret key functionality with clear examples. The patch-level version bump is appropriate for these new methods.
Consider adding tests: Since these changes introduce new API methods for managing machine scopes and secret keys, please ensure comprehensive test coverage is added to verify the functionality works as expected and handles edge cases properly.
packages/backend/src/api/resources/JSON.ts (3)
38-39: LGTM: New ObjectType constants follow established patterns.The addition of
MachineScopeandMachineSecretKeyobject types is consistent with the existing naming convention and structure.
717-723: Well-structured interface for machine scope relationships.The
MachineScopeJSONinterface appropriately represents the machine-to-machine relationship with clear field names and optional timestamp/deletion flags.
725-728: Ensure secure handling of MachineSecretKey.secret across the codebaseOur grep did not find any direct
console.logorloggercalls that emit the rawsecretfield, and the integration cleanup script appropriately masks secrets before logging. However, we did locate one serialization site:
packages/nextjs/src/app-router/keyless-actions.ts
• JSON.stringify({ claimUrl, publishableKey, secretKey })Please verify that:
- The
secretKeyin that JSON payload is only ever used in server-side code (never sent to client logs or browser console)- All transport channels (HTTP requests, cookies) are secured (TLS, HttpOnly/Secure flags)
- This payload is not inadvertently logged, cached, or exposed in error/debug traces
- Endpoints returning or consuming
MachineSecretKeyenforce strict access controlsAfter confirming these points, tag
@clerk/securityfor a final review.packages/backend/src/api/endpoints/MachineApi.ts (5)
4-5: LGTM: Proper type imports added.The new imports for
MachineScopeandMachineSecretKeytypes are correctly structured and follow the existing import pattern.
10-23: Well-documented parameter extensions.The additions to
CreateMachineParamsare properly documented with JSDoc and follow TypeScript best practices. The optional fields provide good flexibility for machine configuration.
25-38: Good parameter design for updates.Making
nameoptional inUpdateMachineParamsmakes sense for PATCH operations, and the newdefaultTokenTtlparameter is well-documented.
89-95: Secure secret retrieval implementation.The
getSecretKeymethod follows the established API pattern with proper ID validation. The endpoint path is appropriate for this sensitive operation.
97-112: Well-documented scope creation method.The JSDoc documentation clearly explains the purpose and parameters. The implementation follows established patterns with proper validation.
| async getSecretKey(machineId: string) { | ||
| this.requireId(machineId); | ||
| return this.request<MachineSecretKey>({ | ||
| method: 'GET', | ||
| path: joinPaths(basePath, machineId, 'secret_key'), | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Creates a new machine scope, allowing the specified machine to access another machine. | ||
| * | ||
| * @param machineId - The ID of the machine that will have access to another machine. | ||
| * @param toMachineId - The ID of the machine that will be scoped to the current machine. | ||
| */ | ||
| async createScope(machineId: string, toMachineId: string) { | ||
| this.requireId(machineId); | ||
| return this.request<MachineScope>({ | ||
| method: 'POST', | ||
| path: joinPaths(basePath, machineId, 'scopes'), | ||
| bodyParams: { | ||
| toMachineId, | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Deletes a machine scope, removing access from one machine to another. | ||
| * | ||
| * @param machineId - The ID of the machine that has access to another machine. | ||
| * @param otherMachineId - The ID of the machine that is being accessed. | ||
| */ | ||
| async deleteScope(machineId: string, otherMachineId: string) { | ||
| this.requireId(machineId); | ||
| return this.request<MachineScope>({ | ||
| method: 'DELETE', | ||
| path: joinPaths(basePath, machineId, 'scopes', otherMachineId), | ||
| }); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Missing test coverage for new API methods.
According to the coding guidelines, tests should be added to cover new functionality. Please ensure that unit tests are added for:
getSecretKeymethodcreateScopemethoddeleteScopemethod
Tests should cover both success cases and error scenarios (e.g., invalid machine IDs).
🏁 Script executed:
#!/bin/bash
# Description: Check for existing test files and patterns for MachineApi
echo "=== Looking for existing MachineApi test files ==="
fd -e spec.ts -e test.ts | rg -i machine
echo -e "\n=== Checking test patterns in the codebase ==="
fd -e spec.ts -e test.ts | head -5 | xargs grep -l "describe\|it\|test"
echo -e "\n=== Checking if there are existing API endpoint tests ==="
rg -l "class.*Api.*extends.*AbstractAPI" --type ts | head -3Length of output: 1583
Add unit tests for MachineApi’s new methods
There are currently no tests covering MachineApi.getSecretKey, createScope, or deleteScope. Please add a new spec under packages/backend/src/api/endpoints/__tests__/MachineApi.test.ts (or similar) that includes:
- Success cases for:
getSecretKey(machineId)createScope(machineId, toMachineId)deleteScope(machineId, otherMachineId)
- Error scenarios (e.g., calling with invalid or empty IDs)
- Mocking/stubbing of
this.requestto verify the correct HTTP method, path, and bodyParams
This will ensure all new functionality is exercised and adheres to our coverage guidelines.
🤖 Prompt for AI Agents
In packages/backend/src/api/endpoints/__tests__/MachineApi.test.ts, add unit
tests for the new MachineApi methods getSecretKey, createScope, and deleteScope.
Write success case tests that mock this.request to verify the correct HTTP
method, path, and bodyParams are used for each method. Also include error
scenario tests that check behavior when invalid or empty IDs are passed,
ensuring requireId validation triggers errors. Use stubs or mocks for
this.request to isolate the tests from actual HTTP calls and confirm the methods
behave as expected.
There was a problem hiding this comment.
This will be tested in a follow up PR here
There was a problem hiding this comment.
@wobsoriano Thank you for the clarification! That's a perfectly reasonable approach to handle test coverage in a follow-up PR. I appreciate you providing the link to PR #6229 where the tests will be added.
Having the core functionality implemented first and then comprehensive testing in a dedicated follow-up PR helps keep each PR focused and manageable.
ଘ(੭ˊᵕˋ)੭ ♡
There was a problem hiding this comment.
@wobsoriano Hello! 👋 I think you meant "CodeRabbit" but "coderobert" works too! 😊
Is there anything specific you'd like me to help you with regarding this PR or any other code review needs?
ଘ(੭ˊᵕˋ)੭♡
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/backend/src/api/resources/Machine.ts (2)
10-11: Add JSDoc documentation for new properties.The new
scopedMachinesanddefaultTokenTtlproperties lack JSDoc documentation. Per coding guidelines, all public APIs must be documented with JSDoc.Add documentation above the constructor:
export class Machine { + /** + * @param id - Unique identifier for the machine + * @param name - Display name of the machine + * @param instanceId - Instance identifier + * @param createdAt - Creation timestamp + * @param updatedAt - Last update timestamp + * @param scopedMachines - Array of machines that this machine has access to + * @param defaultTokenTtl - Default token time-to-live in seconds + */ constructor( readonly id: string, readonly name: string, readonly instanceId: string, readonly createdAt: number, readonly updatedAt: number, readonly scopedMachines: Machine[], readonly defaultTokenTtl: number, ) {}
29-29: Improve the inline comment.The comment could be more descriptive about why nested machines don't have scoped_machines.
- [], // Nested machines don't have scoped_machines + [], // Prevent infinite recursion - nested machines don't contain their own scoped_machinespackages/backend/src/api/__tests__/MachineApi.test.ts (2)
156-169: Good test coverage for secret key retrieval.The test correctly validates the
getSecretKeymethod with proper URL construction and response handling.Consider adding edge case tests for the secret key functionality:
it('handles invalid machine ID for secret key', async () => { server.use( http.get( `https://api.clerk.test/v1/machines/invalid_id/secret_key`, validateHeaders(() => { return HttpResponse.json({ error: 'Machine not found' }, { status: 404 }); }), ), ); await expect(apiClient.machines.getSecretKey('invalid_id')).rejects.toThrow(); });
91-116: Verify scoped machines property access in test.The test asserts
response.scopedMachinesproperties but doesn't verify that the Machine class properly deserializes the nested scoped machines. Consider adding an assertion to verify thedefaultTokenTtlproperty as well.expect(response.id).toBe(machineId); expect(response.name).toBe('Test Machine'); expect(response.scopedMachines).toHaveLength(1); expect(response.scopedMachines[0].id).toBe(otherMachineId); expect(response.scopedMachines[0].name).toBe('Second Machine'); + expect(response.defaultTokenTtl).toBe(7200); + expect(response.scopedMachines[0].defaultTokenTtl).toBeDefined();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/backend/src/api/__tests__/MachineApi.test.ts(1 hunks)packages/backend/src/api/resources/JSON.ts(2 hunks)packages/backend/src/api/resources/Machine.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/backend/src/api/resources/JSON.ts
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{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/backend/src/api/resources/Machine.tspackages/backend/src/api/__tests__/MachineApi.test.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/backend/src/api/resources/Machine.tspackages/backend/src/api/__tests__/MachineApi.test.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/backend/src/api/resources/Machine.tspackages/backend/src/api/__tests__/MachineApi.test.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/backend/src/api/resources/Machine.tspackages/backend/src/api/__tests__/MachineApi.test.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/backend/src/api/resources/Machine.tspackages/backend/src/api/__tests__/MachineApi.test.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/backend/src/api/resources/Machine.tspackages/backend/src/api/__tests__/MachineApi.test.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.Whenever reviewing a pull request, if there are any changes that could impact security, always tag
@clerk/securityin the PR.Security-impacting changes include, but are not limited to:
- Changes to authentication logic or mechanisms (e.g. login, session handling, token issuance)
- Any modification to access control, authorization checks, or role-based permissions
- Introduction or modification of hashing algorithms, signature verification, or cryptographic primitives
- Handling of sensitive data (e.g. passwords, tokens, secrets, PII)
- Integration with external identity providers (e.g. SSO, OAuth, OpenID Connect)
- Modifications to security headers, cookie flags, CORS policies, or CSRF protections
- Bypass mechanisms (e.g. feature flags, testing overrides) that could weaken protections
- Changes to rate limiting, abuse prevention, or input validation
If you're unsure whether a change is security-relevant, err on the side of caution and tag
@clerk/security.Any time that you tag
@clerk/security, please do so explicitly in a code comment, rather than within a collapsed section in a coderabbit comment, such as the "recent review details" section. If you do use the team name in any thinking or non-direct-code-comment content, it can be referred to as "clerk security team" to avoid accidentally printing the tag which sends a notification to the team.
Files:
packages/backend/src/api/resources/Machine.tspackages/backend/src/api/__tests__/MachineApi.test.ts
packages/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Unit tests should use Jest or Vitest as the test runner.
Files:
packages/backend/src/api/__tests__/MachineApi.test.ts
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)
**/__tests__/**/*.{ts,tsx}: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces
Files:
packages/backend/src/api/__tests__/MachineApi.test.ts
🧬 Code Graph Analysis (1)
packages/backend/src/api/resources/Machine.ts (1)
packages/backend/src/api/resources/JSON.ts (1)
MachineJSON(708-717)
⏰ 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). (3)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
packages/backend/src/api/resources/Machine.ts (1)
15-34: Well-implemented recursive deserialization.The
fromJSONmethod correctly handles the new properties and prevents infinite recursion by providing empty arrays for nested machines' scoped_machines. The implementation properly converts snake_case JSON properties to camelCase TypeScript properties.packages/backend/src/api/__tests__/MachineApi.test.ts (4)
51-154: Comprehensive CRUD test coverage.The basic CRUD operations are well-tested with proper request validation, response mocking, and assertion coverage. The tests correctly validate HTTP methods, URLs, headers, request bodies, and query parameters.
171-203: Excellent test coverage for scope management.The
createScopeanddeleteScopetests are well-implemented with:
- Correct HTTP methods and URL construction
- Proper request body validation for createScope
- Response mapping validation for both operations
- Clear test assertions
These tests effectively validate the new scope management functionality outlined in the PR objectives.
1-204: Add error‐handling and edge‐case tests for MachineAPIThe existing tests in packages/backend/src/api/tests/MachineApi.test.ts only cover happy‐path responses. To ensure robustness—especially around security‐sensitive operations like secret‐key retrieval and scope management—please add tests for:
- 404 Not Found (invalid machine IDs)
- 401/403 Unauthorized or Forbidden responses
- Network failures/timeouts (e.g., simulate fetch rejecting)
- Malformed or unexpected response bodies
- Validation of invalid parameters (e.g., missing required fields)
Locations to update:
- packages/backend/src/api/tests/MachineApi.test.ts (add new
it(…)blocks)Example snippets:
it('throws on 404 when fetching secret key', async () => { server.use( http.get( `https://api.clerk.test/v1/machines/nonexistent/secret_key`, validateHeaders(() => HttpResponse.json({ message: 'Not found' }, { status: 404 }), ), ), ); await expect(apiClient.machines.getSecretKey('nonexistent')) .rejects.toThrow('Not found'); }); it('throws on 401 unauthorized when creating scope', async () => { server.use( http.post( `https://api.clerk.test/v1/machines/${machineId}/scopes`, validateHeaders(() => HttpResponse.json({ message: 'Unauthorized' }, { status: 401 }), ), ), ); await expect( apiClient.machines.createScope(machineId, otherMachineId), ).rejects.toThrow('Unauthorized'); });Also tag @clerk/security for a focused review of the authentication/authorization flows.
16-33: Ensure mock data consistency.The
mockSecondMachineobject is missing thedefault_token_ttlproperty that's present inmockMachine. This inconsistency could cause test failures when the Machine constructor expects all properties.const mockSecondMachine = { object: 'machine', id: otherMachineId, name: 'Second Machine', instance_id: 'inst_456', created_at: 1640995200, updated_at: 1640995200, + default_token_ttl: 3600, + scoped_machines: [], };Also add the missing properties to
mockMachine:const mockMachine = { object: 'machine', id: machineId, name: 'Test Machine', instance_id: 'inst_123', created_at: 1640995200, updated_at: 1640995200, + default_token_ttl: 7200, scoped_machines: [mockSecondMachine], };Likely an incorrect or invalid review comment.
| data.scoped_machines.map( | ||
| m => | ||
| new Machine( | ||
| m.id, | ||
| m.name, | ||
| m.instance_id, | ||
| m.created_at, | ||
| m.updated_at, | ||
| [], // Nested machines don't have scoped_machines | ||
| m.default_token_ttl, | ||
| ), | ||
| ), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add null safety for scoped_machines array.
The mapping of data.scoped_machines assumes it's always an array, but the API might return null or undefined. Consider adding a null safety check.
- data.scoped_machines.map(
+ (data.scoped_machines || []).map(
m =>
new Machine(
m.id,
m.name,
m.instance_id,
m.created_at,
m.updated_at,
[], // Nested machines don't have scoped_machines
m.default_token_ttl,
),
),📝 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.
| data.scoped_machines.map( | |
| m => | |
| new Machine( | |
| m.id, | |
| m.name, | |
| m.instance_id, | |
| m.created_at, | |
| m.updated_at, | |
| [], // Nested machines don't have scoped_machines | |
| m.default_token_ttl, | |
| ), | |
| ), | |
| (data.scoped_machines || []).map( | |
| m => | |
| new Machine( | |
| m.id, | |
| m.name, | |
| m.instance_id, | |
| m.created_at, | |
| m.updated_at, | |
| [], // Nested machines don't have scoped_machines | |
| m.default_token_ttl, | |
| ), | |
| ), |
🤖 Prompt for AI Agents
In packages/backend/src/api/resources/Machine.ts around lines 21 to 32, the code
maps over data.scoped_machines without checking if it is null or undefined,
which can cause runtime errors. Add a null safety check by verifying that
data.scoped_machines is an array before mapping, for example by using a
conditional or defaulting to an empty array, to ensure safe iteration.
There was a problem hiding this comment.
the root scoped_machines will always be an array
There was a problem hiding this comment.
@wobsoriano Thank you for the clarification! I understand that the API contract guarantees scoped_machines will always be an array, so the null safety check is indeed unnecessary.
✏️ Learnings added
Learnt from: wobsoriano
PR: clerk/javascript#6417
File: packages/backend/src/api/resources/Machine.ts:21-32
Timestamp: 2025-07-28T22:19:42.397Z
Learning: In the Machine API, the `scoped_machines` field in the JSON response is guaranteed to always be an array, so null safety checks are not needed when mapping over this field.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
@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: |
Description
This PR adds machine scope creation, deletion and machine secret key retrieval to
machinesBAPIResolves USER-2474
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit