Realtime: improve scope access to runs with tags and batches #1511
Conversation
… or are in the batch - useTaskTrigger can now submit options - auto-generated batch trigger public access tokens no longer need each individual run ID scope
🦋 Changeset detectedLatest commit: bd509cb 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 |
WalkthroughThis pull request includes significant changes across multiple files, primarily focused on the restructuring of presenters and API routes related to batch and run task retrieval. The Changes
Suggested reviewers
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
apps/webapp/app/routes/api.v3.runs.$runId.tsOops! Something went wrong! :( ESLint: 8.45.0 ESLint couldn't find the config "custom" to extend from. Please check that the name of the config is correct. The config "custom" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 9
🧹 Outside diff range and nitpick comments (18)
apps/webapp/app/routes/api.v3.runs.$runId.ts (2)
28-30: Consider adding type annotations for better type safetyWhile the code works correctly, adding explicit type annotations would improve maintainability and catch potential issues earlier.
- async ({ authentication, resource }) => { + async ({ authentication, resource }: { authentication: AuthEnvironment; resource: Run }) => {
Line range hint
1-40: Well-designed implementation of scoped accessThe overall architecture effectively implements the new scoped access system while maintaining backward compatibility. The separation between resource retrieval, authorization, and response handling creates a maintainable and extensible solution.
Consider documenting these access patterns in the API documentation to help users understand the new capabilities for accessing runs via tags and batches.
references/nextjs-realtime/src/components/TriggerButton.tsx (2)
31-32: Fix typo in city name"San Fransico" is misspelled. It should be "San Francisco".
- "Based on the temperature, will I need to wear extra clothes today in San Fransico? Please be detailed.", + "Based on the temperature, will I need to wear extra clothes today in San Francisco? Please be detailed.",
34-36: Consider making the user ID dynamicThe user ID is currently hardcoded as "user:1234". Consider making this dynamic by:
- Accepting it as a prop
- Using a user context
- Or fetching it from an authentication service
This would make the component more reusable and maintainable.
Example implementation:
interface TriggerButtonProps { accessToken: string; userId: string; } export default function TriggerButton({ accessToken, userId }: TriggerButtonProps) { // ... existing code ... // Then use it in submit: submit( { /* ... */ }, { tags: [`user:${userId}`], } ); }apps/webapp/app/routes/api.v1.packets.$.ts (1)
Line range hint
50-67: Consider standardizing error response structureThe error response structure in the loader differs slightly from the action function. Consider standardizing the error response format across both functions for consistency.
- return json({ error: "Failed to generate presigned URL" }, { status: 500 }); + return json( + { error: "Failed to generate presigned URL" }, + { status: 500, headers: { "Content-Type": "application/json" } } + );apps/webapp/app/routes/api.v1.runs.$runParam.reschedule.ts (3)
64-71: Consider optimizing the double run retrieval and improving error messagesThe current implementation performs two separate database queries for the same run. Additionally, the error messages could be more specific.
Consider these improvements:
- const run = await ApiRetrieveRunPresenter.findRun( - updatedRun.friendlyId, - authenticationResult.environment - ); - - if (!run) { - return json({ error: "Run not found" }, { status: 404 }); - } + // Reuse the updatedRun or add specific error context + const run = await ApiRetrieveRunPresenter.findRun( + updatedRun.friendlyId, + authenticationResult.environment + ); + + if (!run) { + return json({ error: "Failed to retrieve the rescheduled run" }, { status: 404 }); + }
Line range hint
74-78: Optimize presenter usage and streamline error handlingThe error check after
presenter.callmight be redundant since we've already verified the run exists. Also, consider reusing the presenter instance.Consider this improvement:
- const presenter = new ApiRetrieveRunPresenter(); - const result = await presenter.call(run, authenticationResult.environment); - - if (!result) { - return json({ error: "Run not found" }, { status: 404 }); - } - - return json(result); + const result = await new ApiRetrieveRunPresenter().call( + run, + authenticationResult.environment + ); + return json(result);
Authentication implementation needs to be updated for new scope requirements
Based on the verification results, the current authentication and authorization system needs attention:
- The code is using a deprecated
authenticateApiRequestfunction that doesn't properly handle failure cases. It should be updated to useauthenticateApiRequestWithFailureinstead.- The authorization system supports granular run-level scopes (e.g., "read:runs:run_1234"), but the rescheduling endpoint isn't leveraging this capability.
Required changes:
- Update to use
authenticateApiRequestWithFailurefor proper error handling- Add appropriate scope checks using the authorization service to verify run-level access permissions
- Implement proper authorization checks after authentication using
checkAuthorizationwith the correct run scope🔗 Analysis chain
Line range hint
19-24: Verify authorization checks align with new scope access requirementsGiven that the PR objectives mention improvements to scope access for runs with tags and batches, we should verify that the authentication checks are properly handling these new access patterns.
Let's verify the authentication implementation:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the implementation of authenticateApiRequest to verify scope checks ast-grep --pattern 'export async function authenticateApiRequest($$$) { $$$ }' # Look for related test files fd -e test.ts -e spec.ts authenticateApiRequestLength of output: 132
Script:
#!/bin/bash # Let's try a broader search for the authentication function rg -l "authenticateApiRequest" # Also search for any authentication-related files fd -e ts auth # Look for scope-related patterns in authentication files rg -i "scope.*run" $(fd -e ts auth)Length of output: 6904
Script:
#!/bin/bash # Let's check the implementation of authenticateApiRequest in the apiAuth service rg -A 10 "authenticateApiRequest" apps/webapp/app/services/apiAuth.server.ts # Also check the authorization service for run-related scope checks rg -A 10 "scope.*run" apps/webapp/app/services/authorization.server.ts # Look at the implementation in the API builder since it's used across routes rg -A 10 "authenticateApiRequest" apps/webapp/app/services/routeBuilders/apiBuilder.server.tsLength of output: 4451
references/v3-catalog/package.json (1)
13-15: Consider standardizing script execution approachThere's inconsistent usage of
tsxvsts-nodeacross different scripts. For example:
managementandclientusetsxqueuesusests-nodetriggerWithLargePayloadnow usestsxConsider standardizing the approach across all scripts for better maintainability.
"scripts": { "dev:trigger": "trigger dev", "deploy": "trigger deploy --self-hosted --load-image", "management": "tsx -r dotenv/config ./src/management.ts", - "queues": "ts-node -r dotenv/config -r tsconfig-paths/register ./src/queues.ts", + "queues": "tsx -r dotenv/config ./src/queues.ts", "build:client": "tsup-node ./src/clientUsage.ts --format esm,cjs", "client": "tsx -r dotenv/config ./src/clientUsage.ts", "triggerWithLargePayload": "tsx -r dotenv/config ./src/triggerWithLargePayload.ts",apps/webapp/app/services/authorization.server.ts (2)
97-99: Consider optimizing permission check order for performance.The current implementation checks both specific and general permissions for each resource. For better performance, consider checking general permissions first, as they would grant access to all resources of that type.
- if (entity.scopes.includes(specificPermission) || entity.scopes.includes(generalPermission)) { + // Check general permission first as it's more broad + if (entity.scopes.includes(generalPermission) || entity.scopes.includes(specificPermission)) { return { authorized: true }; }
104-112: Improve error message formatting for better readability.While the error message is informative, the formatting could be more concise.
- reason: `Public Access Token is missing required permissions. Token has the following permissions: ${entity.scopes - .map((s) => `'${s}'`) - .join( - ", " - )}. See https://trigger.dev/docs/frontend/overview#authentication for more information.`, + reason: `Public Access Token is missing required permissions. Token has the following permissions: ${entity.scopes.map(s => `'${s}'`).join(", ")}. See https://trigger.dev/docs/frontend/overview#authentication for more information.`,apps/webapp/app/routes/api.v1.tasks.batch.ts (1)
Line range hint
89-106: Enhance error messages for security-related failures.While the error handling is comprehensive, consider adding more specific error messages for security-related failures to help with debugging and auditing.
Consider this enhancement:
} catch (error) { logger.error("Batch trigger error", { error: { message: (error as Error).message, stack: (error as Error).stack, + type: error.constructor.name, // Add error type for better debugging }, }); if (error instanceof ServiceValidationError) { - return json({ error: error.message }, { status: 422 }); + return json({ + error: error.message, + code: 'VALIDATION_ERROR' + }, { status: 422 }); } else if (error instanceof OutOfEntitlementError) { - return json({ error: error.message }, { status: 422 }); + return json({ + error: error.message, + code: 'ENTITLEMENT_ERROR' + }, { status: 422 }); }packages/react-hooks/src/hooks/useTaskTrigger.ts (1)
29-29: Update interface documentation to include the new options parameter.The interface has been correctly updated to support optional trigger options, but the documentation comment above the interface should be updated to reflect this new parameter.
references/v3-catalog/src/management.ts (1)
274-288: Consider reducing the initial delay for practical testingThe 1-hour initial delay makes this example impractical for testing purposes. Consider using a shorter initial delay (e.g., "10s") to make the example more developer-friendly while still demonstrating the rescheduling functionality.
- const run = await simpleChildTask.trigger({ message: "Hello, World!" }, { delay: "1h" }); + const run = await simpleChildTask.trigger({ message: "Hello, World!" }, { delay: "10s" });Additionally, consider adding validation to ensure the reschedule was successful:
const reschedule = await runs.reschedule(run.id, { delay: "1s", }); console.log("reschedule", reschedule); + + // Validate reschedule response + if (!reschedule?.id) { + throw new Error("Failed to reschedule run"); + } const rescheduledRun = await waitForRunToComplete(reschedule.id);apps/webapp/test/authorization.test.ts (1)
68-68: Consider extracting the documentation URL to a constantThe documentation URL "https://trigger.dev/docs/frontend/overview#authentication" is duplicated across multiple error messages. Consider extracting it to a constant at the top of the file to improve maintainability.
import { describe, it, expect } from "vitest"; import { checkAuthorization, AuthorizationEntity } from "../app/services/authorization.server"; + +const AUTH_DOCS_URL = "https://trigger.dev/docs/frontend/overview#authentication";Also applies to: 244-244, 281-281, 314-314
apps/webapp/app/routes/api.v1.batches.$batchId.ts (1)
3-3: Ensure correct import path for$replica.At line 3,
$replicais imported from"~/db.server". Verify that this path is correct and that$replicais properly initialized. Misconfigured imports can lead to runtime errors.Confirm that the import aligns with the project's structure and adjust the path if necessary.
-import { $replica } from "~/db.server"; +import { $replica } from "~/path/to/db.server";apps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.ts (1)
27-41: Optimize database query by selecting only necessary fieldsIn the
findResourcefunction, consider selecting only the fields required for authorization and response. This optimization reduces data transfer and improves query performance.Apply this diff to select necessary fields:
return $replica.taskRun.findFirst({ where: { friendlyId: params.runId, runtimeEnvironmentId: auth.environment.id, }, + select: { + friendlyId: true, + runTags: true, + batch: { + select: { + friendlyId: true, + }, + }, + }, });apps/webapp/app/presenters/v3/ApiRetrieveRunPresenter.server.ts (1)
18-18: Confirm the necessity of importing both$replicaandprismaYou have imported both
$replicaandprisma. Since$replicais used for read operations andprismafor write operations, ensure that this distinction is intentional and necessary within this file.If
prismais not used elsewhere in the file, consider removing it to clean up unused imports.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (20)
apps/webapp/app/presenters/v3/ApiRetrieveBatchPresenter.server.ts(0 hunks)apps/webapp/app/presenters/v3/ApiRetrieveRunPresenter.server.ts(2 hunks)apps/webapp/app/routes/api.v1.batches.$batchId.ts(2 hunks)apps/webapp/app/routes/api.v1.packets.$.ts(1 hunks)apps/webapp/app/routes/api.v1.runs.$runParam.reschedule.ts(1 hunks)apps/webapp/app/routes/api.v1.runs.ts(1 hunks)apps/webapp/app/routes/api.v1.tasks.batch.ts(1 hunks)apps/webapp/app/routes/api.v3.runs.$runId.ts(1 hunks)apps/webapp/app/routes/realtime.v1.batches.$batchId.ts(1 hunks)apps/webapp/app/routes/realtime.v1.runs.$runId.ts(1 hunks)apps/webapp/app/routes/realtime.v1.runs.ts(1 hunks)apps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.ts(1 hunks)apps/webapp/app/services/authorization.server.ts(1 hunks)apps/webapp/app/services/routeBuilders/apiBuilder.server.ts(6 hunks)apps/webapp/test/authorization.test.ts(6 hunks)packages/core/src/v3/apiClient/index.ts(1 hunks)packages/react-hooks/src/hooks/useTaskTrigger.ts(2 hunks)references/nextjs-realtime/src/components/TriggerButton.tsx(1 hunks)references/v3-catalog/package.json(1 hunks)references/v3-catalog/src/management.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/webapp/app/presenters/v3/ApiRetrieveBatchPresenter.server.ts
🔇 Additional comments (36)
apps/webapp/app/routes/api.v1.runs.ts (1)
15-17: Verify the authorization scope changes
The changes to the resource function align with the PR objective of improving run accessibility via public access tokens. However, let's verify that this implementation correctly handles the new tag and batch-based access patterns mentioned in the PR objectives.
✅ Verification successful
Authorization resource function implementation is consistent with codebase patterns
The resource function implementation is correct and aligns with established patterns in the codebase:
- The three-parameter signature
(_, __, searchParams)matches other route handlers likerealtime.v1.runs.ts - Task filtering via
filter[taskIdentifier]is consistently used across the codebase, appearing in API client, presenters, and routes - The superScopes
["read:runs", "read:all", "admin"]align with other run-related endpoints
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent authorization patterns across related files
# Look for similar authorization patterns in other run-related routes
echo "Checking authorization patterns in run-related routes..."
rg -A 5 "authorization.*resource.*tasks" "apps/webapp/app/routes"
# Check for tag and batch-related authorization logic
echo "Checking tag and batch authorization implementations..."
rg -A 10 "authorization.*resource.*(tag|batch)" "apps/webapp/app/routes"
Length of output: 382
Script:
#!/bin/bash
# Let's try a different approach to verify the authorization changes
# 1. First, let's find all files that might contain authorization configurations
echo "Files with authorization configurations:"
rg -l "authorization.*superScopes" --type ts
# 2. Let's examine the resource function patterns in these files
echo -e "\nResource function patterns:"
rg -B2 -A2 "resource:\s*\([^)]*\)\s*=>" --type ts
# 3. Let's also check for any task-related filters in the codebase
echo -e "\nTask filter usage patterns:"
rg "filter\[taskIdentifier\]" --type ts
Length of output: 4532
apps/webapp/app/routes/realtime.v1.runs.ts (2)
19-19: Clarify the purpose of the dummy findResource implementation
The comment indicates this is a dummy value that's not used. However, given the PR's objective to improve scope access, this seems like it could be a missed opportunity or incomplete implementation.
Let's check how findResource is implemented in related files:
22-24: Verify the authorization implementation for tag-based access
The resource function now properly receives searchParams which contain tags, aligning with the PR's objective to enable tag-based access. However, we should verify that the superScopes array properly covers all the access patterns described in the PR objectives.
Let's analyze the scope implementation across the codebase:
✅ Verification successful
Let me gather more information about the tag-based access implementation to ensure complete verification.
Let me check one more aspect of the authorization implementation to complete the verification.
Let me check the complete authorization implementation to understand how tag-based scopes are validated.
Let me check one final piece to understand the complete scope of resource types.
The authorization implementation correctly supports tag-based access control
The implementation is verified to be correct for the following reasons:
- The
superScopesarray containing["read:runs", "read:all", "admin"]provides the necessary high-level access patterns - The authorization system supports granular tag-based access through:
- Tags are included in the
ResourceTypesconstant - The
AuthorizationResourcestype properly handles tag resources - The
checkAuthorizationfunction validates both super scopes and specific resource-level permissions
- Tags are included in the
- Test coverage confirms the implementation with examples of tag-specific scopes like
read:tags:tag_5678
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other implementations of superScopes to ensure consistency
rg "superScopes:\s+\[" "apps/webapp/app/routes/"
# Search for scope-related tests
rg -g "*.test.*" "read:runs|read:all|admin" "apps/webapp/"
Length of output: 2276
Script:
#!/bin/bash
# Search for tag-based access implementation in authorization code
rg -A 5 "read:runs:" "apps/webapp/"
# Look for tag-related scope implementations
ast-grep --pattern 'function $_($$_) {
$$$
read:runs
$$$
}'
# Check for any tag-based authorization tests
rg -g "*.test.*" "read:tags|read:runs:[a-zA-Z0-9]" "apps/webapp/"
Length of output: 3078
Script:
#!/bin/bash
# Look for the authorization check implementation
rg -A 10 "function checkAuthorization" "apps/webapp/"
# Search for any tag-based scope validation logic
rg -A 5 "superScopes.*\[.*read:runs" "apps/webapp/app/services/"
Length of output: 974
Script:
#!/bin/bash
# Get the full implementation of the authorization check
rg -A 50 "export function checkAuthorization" "apps/webapp/app/services/authorization.server.ts"
# Look for tag-based scope definitions
rg "type.*AuthorizationResources" -A 10 "apps/webapp/"
Length of output: 2808
Script:
#!/bin/bash
# Look for ResourceTypes definition
rg "ResourceTypes.*=.*\[" -A 5 "apps/webapp/"
# Check for any tag-specific resource type handling
rg "tags.*:.*string" "apps/webapp/"
Length of output: 782
apps/webapp/app/routes/api.v3.runs.$runId.ts (2)
15-17: LGTM! Good separation of concerns
The findResource function cleanly separates resource retrieval from authorization logic, following the single responsibility principle.
19-24: Verify the completeness of authorization scopes
The new authorization structure properly implements tag and batch-based access as intended. However, let's verify that all necessary access patterns are covered.
✅ Verification successful
Based on the results, I can now generate the final response as I have sufficient information about the scope configurations across the codebase.
Authorization scopes are properly configured and consistent
The authorization structure is complete and well-aligned across the codebase:
- All run-related endpoints consistently use the same scope structure (runs, tags, batch) and superScopes ["read:runs", "read:all", "admin"]
- The authorization service properly handles both regular scopes and superScopes
- Test coverage confirms the correct behavior of scope-based authorization
- Other related endpoints (batches, tasks) follow the same pattern with appropriate superScopes
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any other scope-related configurations that might need updating
# Look for other files defining scopes or authorization patterns
rg -A 3 "superScopes|resource: \(" --type ts
# Look for any scope-related tests
rg -A 3 "describe.*scope" --type ts
Length of output: 11593
references/nextjs-realtime/src/components/TriggerButton.tsx (1)
28-37: Verify the new submit options implementation
The changes align with the PR objectives by implementing tag-based run access. Let's verify the implementation across the codebase.
✅ Verification successful
Let me gather more information about the submit implementation and its usage patterns with a different search strategy.
Based on the search results, I can now provide a final response about the tag-based run access implementation:
Tag-based run access is correctly implemented across the codebase
The implementation in TriggerButton.tsx aligns with the established patterns across the codebase:
- The
useTaskTriggerhook properly supports tags in its options (packages/react-hooks/src/hooks/useTaskTrigger.ts) - Tag handling is consistently implemented in the backend services (apps/webapp/app/services/)
- The authorization system correctly validates tag-based access (apps/webapp/test/authorization.test.ts)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent implementation of the new submit options pattern
# and verify the integration with run presenters
# Check for other useTaskTrigger usage patterns
echo "Checking useTaskTrigger usage patterns:"
rg "useTaskTrigger.*submit.*tags" -A 5
# Check for related presenter implementations
echo "Checking run presenter implementations:"
ast-grep --pattern 'class $presenter {
$$$
findRun($$$) {
$$$
}
$$$
}'
Length of output: 303
Script:
#!/bin/bash
# Search for useTaskTrigger hook usage
echo "Checking useTaskTrigger hook usage:"
rg "useTaskTrigger" -A 3
# Search for submit function implementations and calls
echo "Checking submit function implementations:"
ast-grep --pattern 'submit($$$) {
$$$
}'
# Search for any tag-related implementations
echo "Checking tag-related patterns:"
rg "tags:" -A 2
Length of output: 30823
apps/webapp/app/routes/api.v1.packets.$.ts (1)
48-48: Verify security implications of the dummy findResource function
The dummy implementation that always returns 1 appears to bypass resource validation. While this might be intentional as part of the broader scope access improvements mentioned in the PR, we should verify that this doesn't inadvertently create security vulnerabilities.
Let's check how findResource is implemented in related routes:
✅ Verification successful
Dummy findResource function is safe in this context
After reviewing the implementation, the dummy findResource function that returns 1 is safe because:
- The route is specifically for handling presigned URL generation for packets
- The
createLoaderApiRoutestill enforces authentication via JWT/API key throughauthenticateApiRequestWithFailure - The actual authorization is handled through the presigned URL mechanism, not through the resource lookup
- Other routes in the codebase (e.g.,
realtime.v1.runs.ts) also use dummyfindResourcefunctions when resource lookup isn't needed
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other implementations of findResource to understand the pattern
rg -A 5 "findResource.*=.*async" --type typescript
# Search for createLoaderApiRoute usage to understand the security context
ast-grep --pattern 'createLoaderApiRoute({
$$$
})'
Length of output: 142
Script:
#!/bin/bash
# Search for findResource implementations without file type restriction
rg -A 5 "findResource.*=.*async"
# Search for createLoaderApiRoute usage
rg -A 5 "createLoaderApiRoute"
# Look for similar route files handling packets
fd "packets" --type f
Length of output: 7927
Script:
#!/bin/bash
# Let's check the apiBuilder.server.ts to understand the security model
cat apps/webapp/app/services/routeBuilders/apiBuilder.server.ts
# Check the implementation of packets.ts utility
cat apps/webapp/app/utils/packets.ts
Length of output: 21305
references/v3-catalog/package.json (1)
15-15: Verify module resolution after removing tsconfig-paths
The script no longer uses tsconfig-paths/register. Please ensure that all imports in triggerWithLargePayload.ts are using correct paths and the module resolution still works as expected.
✅ Verification successful
Path resolution is working correctly without tsconfig-paths
The file references/v3-catalog/src/triggerWithLargePayload.ts uses standard package imports from @trigger.dev/sdk/v3 and a local relative import ./trigger/simple.js. The removal of tsconfig-paths/register does not impact the imports because:
- The
@trigger.devimports are resolved through the package resolution system - The local import uses a relative path which doesn't require path alias resolution
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for path aliases usage in the target file
# Look for potential path aliases (imports starting with @) in the file
echo "Checking for path aliases in triggerWithLargePayload.ts:"
rg "^import.*from ['\"]@" ./src/triggerWithLargePayload.ts
# Look for tsconfig.json path aliases configuration
echo -e "\nChecking for path aliases configuration in tsconfig:"
fd tsconfig.json --exec cat {} | jq '.compilerOptions.paths'
Length of output: 3067
Script:
#!/bin/bash
# Let's first find the correct location of triggerWithLargePayload.ts
echo "Finding triggerWithLargePayload.ts:"
fd triggerWithLargePayload.ts
# Once we find it, let's check its imports
echo -e "\nChecking all imports in the file:"
fd triggerWithLargePayload.ts --exec rg "^import" {}
# Let's also check the directory structure to understand the file location
echo -e "\nChecking directory structure:"
fd --type d --max-depth 3
Length of output: 2577
apps/webapp/app/services/authorization.server.ts (1)
Line range hint 1-112: Verify the implementation covers all access scenarios.
The changes align well with the PR objectives for improving scope access. Let's verify that all access patterns work as expected.
✅ Verification successful
Let me analyze the test coverage specifically for the authorization implementation.
Implementation verified with comprehensive test coverage
The authorization implementation has been thoroughly tested, covering:
- Tag and batch resource types in the
ResourceTypesconstant - All entity types (PRIVATE, PUBLIC, PUBLIC_JWT)
- Both specific and general permission patterns
- Array-based resource access
- Super scope functionality
- Edge cases including empty resources and undefined scopes
- Error scenarios with detailed messages
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the authorization implementation handles all required access patterns
# Check for test coverage of new access patterns
echo "Checking test coverage for tag and batch access patterns..."
rg -l "test|spec|describe.*tag.*scope|batch.*scope" --type ts
# Verify error scenarios are properly tested
echo "Checking error scenario test coverage..."
rg -l "test|spec|describe.*unauthorized|permission.*denied" --type ts
# Look for any hardcoded scope patterns that might conflict
echo "Checking for conflicting scope patterns..."
rg "scope.*tag|scope.*batch|permission.*tag|permission.*batch" --type ts
Length of output: 23223
Script:
#!/bin/bash
# Check the authorization test file content
echo "Checking authorization test coverage..."
cat apps/webapp/test/authorization.test.ts
# Look for any authorization-related test files we might have missed
echo -e "\nChecking for additional authorization test files..."
rg -l "describe.*authorization|test.*authorization" --type ts
Length of output: 12580
apps/webapp/app/routes/api.v1.tasks.batch.ts (1)
136-136: LGTM! Verify authorization checks for batch-scoped access.
The simplified scope approach using read:batch:${batch.id} is a good improvement that:
- Reduces JWT complexity by eliminating individual run scopes
- Maintains security through proper scope hierarchy
- Aligns with the PR objective of improving run accessibility
Let's verify that batch-scoped authorization is properly enforced:
packages/react-hooks/src/hooks/useTaskTrigger.ts (2)
97-99: LGTM! Implementation correctly handles trigger options.
The changes properly integrate the new options parameter with the existing mutation system, maintaining type safety and backward compatibility. This implementation supports the PR's objective of improving scope access through tags.
29-29: Verify tag handling in the codebase.
The changes support tag-based scope access as intended. Let's verify the integration with the rest of the codebase.
Also applies to: 97-99
✅ Verification successful
Based on the gathered information, I can now provide a final response:
Tag handling is properly integrated in the codebase
The TriggerOptions type in packages/core/src/v3/types/tasks.ts includes tags support, and the implementation in useTaskTrigger.ts correctly passes tags through to the API client. The hook properly handles tags in the options parameter and propagates them during task submission, as seen in the implementation:
options: {
...
tags: options?.tags,
...
}🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the integration of tag-based scope access.
# Check for tag-related types and interfaces
echo "Checking for tag-related type definitions..."
rg -t typescript "interface.*TriggerOptions" -A 10
# Check for tag usage in API routes and handlers
echo "Checking for tag handling in API routes..."
rg -t typescript "tags.*TriggerOptions"
# Check for tag-related tests
echo "Checking for tag-related tests..."
fd -e test.ts -e spec.ts | xargs rg "tags.*TriggerOptions"
Length of output: 496
Script:
#!/bin/bash
# Let's try a different approach to verify tag handling
# Search for TriggerOptions interface definition
echo "Searching for TriggerOptions interface..."
rg "interface.*TriggerOptions"
# Look for tag-related properties or methods
echo "Searching for tag-related code..."
rg "tags.*:" -A 2
# Check for tag usage in function parameters
echo "Checking tag usage in parameters..."
rg "options.*TriggerOptions"
# Search for any tag-related implementations
echo "Searching for tag implementations..."
ast-grep --pattern 'interface TriggerOptions {
$$$
}'
Length of output: 39624
Script:
#!/bin/bash
# Let's find the TriggerOptions interface definition and its usage
# Search for TriggerOptions interface definition
echo "Searching for TriggerOptions interface..."
ast-grep --pattern 'interface TriggerOptions {
$$$
}'
# Look for the specific file where TriggerOptions is defined
echo "Looking for TriggerOptions definition file..."
fd -e ts -e tsx | xargs rg "interface.*TriggerOptions" -A 5
# Check for tag-related imports and usage in useTaskTrigger
echo "Checking useTaskTrigger implementation..."
rg -A 10 "useTaskTrigger" packages/react-hooks/src/hooks/useTaskTrigger.ts
Length of output: 2168
Script:
#!/bin/bash
# Let's find the core TriggerOptions type definition
# Search in core package
echo "Searching in core package..."
rg "interface.*TriggerOptions" packages/core/src/v3/types/ -A 10
# Search in trigger-sdk package
echo "Searching in trigger-sdk package..."
rg "interface.*TriggerOptions" packages/trigger-sdk/src/ -A 10
# Look for the specific implementation of tags in TriggerOptions
echo "Looking for tags in TriggerOptions..."
ast-grep --pattern 'interface TriggerOptions {
$$$
tags?: $_
$$$
}'
Length of output: 440
Script:
#!/bin/bash
# Let's try a different approach to find TriggerOptions
# Look for type or interface definitions of TriggerOptions
echo "Searching for TriggerOptions type definition..."
rg "export (type|interface) TriggerOptions" -A 10
# Look for the file content directly
echo "Looking at useTaskTrigger.ts content..."
cat packages/react-hooks/src/hooks/useTaskTrigger.ts
# Check for any imports of TriggerOptions
echo "Checking TriggerOptions imports..."
rg "import.*TriggerOptions"
Length of output: 8317
references/v3-catalog/src/management.ts (1)
293-296: Verify test coverage for batch and scope access features
While testing the reschedule functionality is important, the PR objectives mention improvements to scope access for runs with tags and batches. Consider uncommenting doBatchTrigger and adding new test cases to verify:
- Access to runs using tag-scoped tokens
- Access to runs using batch-scoped tokens
- Auto-generated batch trigger public access token functionality
Let's verify if there are any existing tests for these features:
apps/webapp/test/authorization.test.ts (1)
100-101:
Breaking Change: Authorization behavior for nonexistent resource types
The test now expects unauthorized access for nonexistent resource types, which is a breaking change from the previous behavior. This change should be:
- Documented in the PR description
- Added to the changelog
- Communicated to users as it may affect existing implementations
Let's verify if this is used in the codebase:
packages/core/src/v3/apiClient/index.ts (2)
Line range hint 150-190: Implementation consistently handles scoped access across features.
The triggerTask method's scope handling (combining run and tag scopes) provides a good precedent for the batch scope implementation. This consistency in access control patterns across different features enhances maintainability.
258-258: LGTM: Simplified batch access scope aligns with PR objectives.
The change to only include read:batch:${data.id} in the JWT scopes aligns with the PR's goal of improving run accessibility through batch scopes. This simplification is secure as it maintains proper access control while reducing token size.
Let's verify that this change is consistent with the batch access patterns in the codebase:
✅ Verification successful
LGTM: Batch scope implementation is consistent across the codebase
The verification confirms that:
- The
read:batch:<batchId>scope is consistently implemented across the codebase - No individual run scopes are used in batch contexts
- Documentation in
docs/realtime/subscribe-to-batch.mdxaligns with the implementation - The scope pattern is consistently used in both API client and server-side routes
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for batch access patterns and scope usage
# Expected: Only batch-level scope checks, no individual run scope checks for batch access
# Check for batch scope validation in auth middleware or handlers
rg -A 5 "read:batch:"
# Check for any remaining individual run scope checks in batch contexts
ast-grep --pattern 'read:runs:$_'
Length of output: 1310
apps/webapp/app/routes/realtime.v1.batches.$batchId.ts (1)
29-29: Function signature updated appropriately to accept batchRun
The async function now accepts resource: batchRun, aligning with the updated structure and providing direct access to the batchRun resource retrieved by findResource.
apps/webapp/app/routes/realtime.v1.runs.$runId.ts (3)
16-30: Correct implementation of findResource function
The findResource function accurately retrieves the taskRun based on friendlyId and runtimeEnvironmentId, including the necessary batch information for authorization.
33-37: Proper construction of the authorization resource
The resource object correctly includes runs, tags, and batch, ensuring that access control is properly enforced based on these attributes.
Line range hint 41-46: Updated handler function aligns with the new resource retrieval
The handler function now correctly accepts { authentication, request, resource: run } and utilizes realtimeClient.streamRun with run.id, maintaining consistency with the refactored resource handling.
apps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.ts (2)
52-54: Function signature update aligns with resource usage
The updated function signature now accepts resource: run, which matches the output of findResource. This change ensures the run object is properly utilized in the response.
44-48: Verify access control implications of 'tags' and 'batch' in authorization resource
Adding tags and batch to the authorization resource might affect access control. Ensure this change aligns with your security policies and that scopes are correctly enforced.
Run the following script to verify usage of tags and batch in access control checks:
✅ Verification successful
Let's analyze the authorization patterns more comprehensively to understand the security implications.
Let's analyze the actual authorization implementation and how resources are handled in the security context.
Adding 'tags' and 'batch' to authorization resource is safe and properly handled
Based on the authorization implementation in authorization.server.ts, both 'tags' and 'batch' are explicitly included in the ResourceTypes constant and are properly handled by the authorization system. The implementation:
- Validates these resource types against a predefined list
- Supports both specific and general permission patterns (e.g.,
read:tags:specific_tagorread:tags) - Has comprehensive test coverage for similar resource types
- Properly handles array-based resources
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for access control checks involving 'tags' and 'batch'.
# Search for authorization usage of 'tags'
rg 'authorize.*tags' -A 5
# Search for authorization usage of 'batch'
rg 'authorize.*batch' -A 5
Length of output: 56
Script:
#!/bin/bash
# Let's try a broader search for authorization patterns and resource definitions
# Search for authorization-related files and patterns
rg -l "authorize|permission|resource" --type ts
# Search for resource definitions in authorization contexts
ast-grep --pattern 'resource: $_'
# Search specifically in the realtime routes directory for authorization
fd realtime.v1 --type f --exec rg -l "authorize|permission|resource" {}
Length of output: 5949
Script:
#!/bin/bash
# Let's examine the authorization implementation and resource usage
# Check the authorization service implementation
cat apps/webapp/app/services/authorization.server.ts
# Look at the realtime stream route implementation
cat apps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.ts
# Check authorization tests for resource patterns
cat apps/webapp/test/authorization.test.ts
Length of output: 16386
apps/webapp/app/presenters/v3/ApiRetrieveRunPresenter.server.ts (2)
97-100:
Update method callers to match new call signature
The call method signature has changed to accept taskRun: FoundRun instead of friendlyId: string. Ensure that all callers of ApiRetrieveRunPresenter.call are updated to pass the taskRun object returned from findRun.
Run the following script to find all invocations of call and check that they pass a taskRun object:
✅ Verification successful
All callers have been updated to match the new signature
Based on the search results, I found all usages of ApiRetrieveRunPresenter.call() in the codebase:
- In
apps/webapp/app/routes/api.v3.runs.$runId.ts:
const presenter = new ApiRetrieveRunPresenter();
const result = await presenter.call(resource, authentication.environment);- In
apps/webapp/app/routes/api.v1.runs.$runParam.reschedule.ts:
const presenter = new ApiRetrieveRunPresenter();
const result = await presenter.call(run, authenticationResult.environment);In both cases, the callers are correctly passing a taskRun object (named as resource and run respectively) that was obtained from ApiRetrieveRunPresenter.findRun(). The signatures match the new method definition and no updates are needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify all `call` method invocations are updated to pass `taskRun`.
# Test: Search for `call` method usages. Expect: First argument is a `taskRun` object.
rg 'ApiRetrieveRunPresenter\.call\(' -A 1
Length of output: 43
Script:
#!/bin/bash
# Let's try a broader search to find usages of ApiRetrieveRunPresenter
rg "ApiRetrieveRunPresenter" -A 5
Length of output: 8749
Script:
#!/bin/bash
# Let's also search for any imports of this presenter
rg "import.*ApiRetrieveRunPresenter"
Length of output: 458
Script:
#!/bin/bash
# Let's use ast-grep to find class instantiation and method calls
ast-grep --pattern 'new ApiRetrieveRunPresenter'
Length of output: 266
Script:
#!/bin/bash
# Let's check the file itself to understand the class better
cat "apps/webapp/app/presenters/v3/ApiRetrieveRunPresenter.server.ts"
Length of output: 9996
65-95:
Handle potential null return from findRun method
The findRun method uses findFirst, which may return null if no matching taskRun is found. However, the FoundRun type is defined using NonNullable, implying that taskRun is always non-null when passed to the call method. This could lead to runtime errors if null is passed inadvertently.
Ensure that all callers of findRun properly handle the possibility of a null return value before invoking call. Consider throwing an error within findRun when no run is found to enforce non-nullability.
Run the following script to verify that all usages of findRun handle the null case appropriately:
apps/webapp/app/services/routeBuilders/apiBuilder.server.ts (12)
24-25: Addition of 'TResource' generic parameter enhances flexibility
The introduction of TResource with a default of never provides a way to specify the resource type, improving the type safety and flexibility of the code.
32-35: Implementation of 'findResource' function enables resource retrieval
Adding the findResource function allows dynamic retrieval of resources based on parameters and authentication, enhancing modularity and reusability.
53-54: Inclusion of 'TResource' in 'ApiKeyHandlerFunction'
Extending ApiKeyHandlerFunction with TResource ensures that handlers receive a properly typed resource, improving type safety.
63-63: Handler function now receives 'resource' parameter
Including resource in the handler arguments provides direct access to the retrieved resource within the handler, enhancing functionality.
69-70: Adding 'TResource' to 'createLoaderApiRoute' generic parameters
Incorporating TResource into the generic parameters of createLoaderApiRoute allows for consistent typing throughout the API route creation process.
72-73: Passing 'TResource' to options and handler
Propagating TResource through options and handler ensures that resource typing is maintained across the API route, improving code consistency.
83-83: Including 'findResource' in options destructuring
Destructuring findResource from options facilitates resource retrieval within the loader function, enhancing code readability.
159-169: Resource retrieval and 404 handling added
The loader now calls findResource to retrieve the resource and returns a 404 response if not found. This improves error handling and provides clearer feedback to API consumers.
171-177: Authorization now utilizes the retrieved resource
The authorization logic now uses the retrieved resource in checks, enhancing security by ensuring permissions are correctly enforced on the specific resource.
181-181: Logging updated to include authorization resource
Including $authResource in the debug logs provides better traceability and aids in debugging authorization issues.
189-189: Authorization check now uses '$authResource'
Passing $authResource to checkAuthorization ensures accurate permission verification against the specific resource, enhancing security.
216-216: Handler receives the 'resource'
Passing the resource to the handler function allows business logic to access the resource directly, improving the handler's capability to process requests effectively.
| authorization: { | ||
| action: "read", | ||
| resource: (params) => ({ batch: params.batchId }), | ||
| resource: (batch) => ({ batch: batch.friendlyId }), |
There was a problem hiding this comment.
Ensure safe access to batch.friendlyId in authorization resource
In the authorization block, the resource function accesses batch.friendlyId. If batch is null, this will cause a runtime error. Verify that the authorization logic handles null resources appropriately to prevent potential exceptions.
Consider adjusting the resource function or ensuring upstream that batch cannot be null at this point.
@trigger.dev/build
@trigger.dev/react-hooks
@trigger.dev/rsc
trigger.dev
@trigger.dev/sdk
@trigger.dev/core
commit: |
Also removed scopes for tags when auto-generating a public access token as that could be dangerous.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
apps/webapp/app/routes/realtime.v1.runs.$runId.ts (1)
16-30: Consider adding explicit error handling for missing runsThe database query is well-structured and properly scoped to the environment. However, consider handling the case where no run is found explicitly, rather than letting it propagate to the default error handler.
findResource: async (params, authentication) => { const run = await $replica.taskRun.findFirst({ where: { friendlyId: params.runId, runtimeEnvironmentId: authentication.environment.id, }, include: { batch: { select: { friendlyId: true, }, }, }, }); + if (!run) { + throw json({ error: `Run ${params.runId} not found` }, { status: 404 }); + } return run; },docs/frontend/overview.mdx (2)
19-19: Add missing comma after "By default"For better readability and correct grammar, add a comma after "By default".
-By default a Public Access Token has no permissions. +By default, a Public Access Token has no permissions.
149-149: Consider adding more detail about default scopes.While the documentation mentions the read scope for the specific run, it would be helpful to explicitly document what default scopes are included for batch-triggered runs, especially given the PR's focus on improving batch trigger public access tokens.
Consider adding a note like:
-By default, tokens returned from the `trigger` function expire after 15 minutes and have a read scope for that specific run. +By default, tokens returned from the `trigger` function expire after 15 minutes and have a read scope for that specific run. For batch-triggered runs, the token automatically includes read scope for all runs in the batch.docs/frontend/react-hooks.mdx (2)
147-147: Enhance security documentation for token handlingThe comment about token scope could be more detailed to align with the PR's improvements to scope access. Consider expanding it to clarify:
- That the token can now access runs via tags and batches
- The security implications of storing tokens in cookies
Update the comment to be more comprehensive:
- cookies().set("publicAccessToken", handle.publicAccessToken); // ✅ this token only has access to read this run + cookies().set("publicAccessToken", handle.publicAccessToken, { + httpOnly: true, // Prevents JavaScript access + secure: true, // Only sent over HTTPS + sameSite: 'strict' // CSRF protection + }); // ✅ This token has scoped access to read this run and associated runs via tags/batchesAdditionally, consider adding a note about token expiration and rotation best practices.
Line range hint
1-24: Add examples demonstrating new token scope capabilitiesThe documentation would benefit from additional examples that showcase the PR's improvements to token scoping, particularly:
- How to generate and use tokens scoped to tags
- How to use batch-scoped tokens
- Examples of auto-generated batch trigger tokens
Consider adding a new section under "Authentication" with examples like:
// Example of generating a tag-scoped token const tagScopedToken = auth.createPublicToken({ scopes: { read: { tags: ["deployment-123"] // Access all runs with this tag } } }); // Example of generating a batch-scoped token const batchScopedToken = auth.createPublicToken({ scopes: { read: { batches: ["batch-123"] // Access all runs in this batch } } });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
.changeset/purple-snakes-divide.md(1 hunks)apps/webapp/app/routes/api.v3.runs.$runId.ts(1 hunks)apps/webapp/app/routes/realtime.v1.runs.$runId.ts(1 hunks)apps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.ts(1 hunks)docs/frontend/overview.mdx(7 hunks)docs/frontend/react-hooks.mdx(1 hunks)packages/core/src/v3/apiClient/index.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/webapp/app/routes/api.v3.runs.$runId.ts
- packages/core/src/v3/apiClient/index.ts
🧰 Additional context used
🪛 LanguageTool
docs/frontend/overview.mdx
[uncategorized] ~18-~18: Did you mean: “By default,”?
Context: ... so is pretty useless! ``` ### Scopes By default a Public Access Token has no permission...
(BY_DEFAULT_COMMA)
🔇 Additional comments (10)
.changeset/purple-snakes-divide.md (2)
1-4: Verify the semantic versioning impact.
The patch version bumps suggest this is a backwards-compatible enhancement. However, since this modifies access control behavior, we should verify that existing applications won't be impacted negatively.
✅ Verification successful
Based on the search results, I can see that there are significant changes in the access control code, particularly in packages/trigger-sdk/src/v3/auth.ts which shows new functionality for public access tokens with scopes for accessing runs via tags and batches. This aligns with the changeset's purpose. The changes appear to be additive and backwards-compatible, as they're adding new capabilities rather than modifying existing ones.
Patch version bump is appropriate for this enhancement.
The changes add new scoping capabilities to public access tokens while maintaining backward compatibility:
- New scope options for accessing runs via tags and batches
- No breaking changes or removals of existing functionality
- Purely additive feature that enhances access control granularity
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for breaking changes in the codebase
# Look for any removed or renamed functions/methods that might indicate breaking changes
# Search for removed or modified function signatures
rg -g '!{dist,node_modules}/**' -A 2 'export (function|const|class|interface)'
# Look for potentially breaking changes in types
ast-grep --pattern 'export type $_ = $_'
Length of output: 100202
Script:
#!/bin/bash
# Look for changes in access control related code
rg -g '!{dist,node_modules}/**' -A 5 'scope|access|permission|token' packages/trigger-sdk/src/
# Look for any breaking changes in the SDK
rg -g '!{dist,node_modules}/**' -B 2 'BREAKING CHANGE' packages/trigger-sdk/CHANGELOG.md
# Check for any changes in public API interfaces
ast-grep --pattern 'export interface $_ {
$$$
}'
Length of output: 112052
Line range hint 6-7: Security review needed for access control changes.
This change broadens access control by allowing tokens with tag/batch scopes to access related runs. While this improves usability, we should verify:
- That runs can only be accessed through their associated tags/batches
- That there's no potential for privilege escalation
- That proper access validation is in place
✅ Verification successful
Let me analyze the access control implementation further by looking specifically at the authorization logic for tags and batches.
Let me analyze the authorization implementation for tags and batches further by looking at the core authorization logic.
Let me check the specific authorization logic for tag and batch access.
Let me try a different approach to find the authorization logic.
Based on the authorization implementation shown in the code, I can now provide a final assessment of the security changes for tag/batch access:
Access control implementation is properly secured for tags and batches
The authorization system implements proper validation for both tag and batch access:
- Tags and batches are explicitly defined as valid resource types in
ResourceTypes = ["tasks", "tags", "runs", "batch"] - Access is granted only if:
- The token has a specific permission (e.g.,
read:tags:tag_123orread:batch:batch_456) - The token has a general permission for the resource type (e.g.,
read:tagsorread:batch) - The token has a super scope like
read:runs,read:all, oradmin
- The token has a specific permission (e.g.,
- The system validates both individual and array-based resource access
- Proper error messages are returned when access is denied
- No access is granted if the token has no permissions or invalid scopes
The implementation ensures that runs can only be accessed through their associated tags/batches when the token has the appropriate scopes, with no potential for privilege escalation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify access control implementation
# Look for authorization checks and token validation
# Search for authorization checks
rg -g '!{dist,node_modules}/**' -B 2 -A 5 'authorize|authenticate|isAuthorized|checkAccess'
# Look for token validation logic
rg -g '!{dist,node_modules}/**' -B 2 -A 5 'validateToken|verifyToken|checkToken'
# Search for scope-related code
rg -g '!{dist,node_modules}/**' -B 2 -A 5 'scope|permission|access'
Length of output: 154570
Script:
#!/bin/bash
# Search for tag/batch related authorization checks
rg -g '!{dist,node_modules}/**' -B 5 -A 5 'tags:|batch:|read:tags|read:batch'
# Look for scope validation related to tags/batches
ast-grep --pattern 'if ($expr.includes("read:tags") || $expr.includes("read:batch")) {
$$$
}'
# Search for tag/batch access validation
rg -g '!{dist,node_modules}/**' -B 5 -A 5 'checkAuthorization.*tags|checkAuthorization.*batch'
Length of output: 127070
Script:
#!/bin/bash
# Search for the core authorization implementation
rg -g '!{dist,node_modules}/**' -B 10 -A 10 'export function checkAuthorization'
# Look for tag/batch scope validation in authorization
ast-grep --pattern 'function checkAuthorization($$$) {
$$$
if ($expr.scopes?.includes($scope)) {
$$$
}
$$$
}'
Length of output: 1994
Script:
#!/bin/bash
# Look for the specific resource authorization checks in the authorization service
rg -g '!{dist,node_modules}/**' -B 5 -A 15 'function isAuthorizedForResource|function checkResourceAuthorization' apps/webapp/app/services/authorization.server.ts
# Search for tag/batch specific scope validation
rg -g '!{dist,node_modules}/**' -B 5 -A 15 'read:tags:|read:batch:|tags:|batch:' apps/webapp/app/services/authorization.server.ts
Length of output: 296
Script:
#!/bin/bash
# Search for the authorization implementation in the authorization service file
cat apps/webapp/app/services/authorization.server.ts
# Also look for any test cases related to tag/batch authorization
rg -g '!{dist,node_modules}/**' -B 5 -A 10 'it\(".*tag.*"\)|it\(".*batch.*"\)' apps/webapp/test/authorization.test.ts
Length of output: 4121
apps/webapp/app/routes/realtime.v1.runs.$runId.ts (2)
33-38: LGTM! Well-structured authorization with improved scope access
The authorization configuration successfully implements the enhanced scope access described in the PR objectives. The resource function properly extracts all relevant identifiers (runs, tags, batch, tasks) enabling flexible access control through public tokens.
Line range hint 42-48: Verify streamRun implementation handles the new scope access
The loader implementation is clean and properly utilizes the retrieved run resource. Let's verify that the streamRun function in realtimeClient properly handles the new scope-based access.
✅ Verification successful
Implementation properly handles scope-based access through SQL where clauses
The streamRun implementation correctly handles scope access by:
- Converting the run ID into a proper SQL where clause (
id='${runId}') - Using the environment ID in the request to Electric
- Implementing proper concurrency controls per environment
- Maintaining backward compatibility with older clients through header rewrites
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for streamRun implementation to verify scope handling
ast-grep --pattern 'streamRun($$$) {
$$$
}'
# Also check for any tests covering this functionality
rg -l "streamRun.*test" --type ts
Length of output: 83
Script:
#!/bin/bash
# Let's try a broader search for streamRun implementation
rg "streamRun" -A 5 --type ts
# Also search for any realtime client implementation
fd "realtime.*client" --type ts --exec cat {}
Length of output: 7211
Script:
#!/bin/bash
# Let's examine the implementation of #streamRunsWhere which is the core function
cat apps/webapp/app/services/realtimeClient.server.ts
Length of output: 8286
apps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.ts (2)
27-41: Handle null cases in findResource function
The function may return null if no matching taskRun is found, which needs to be handled appropriately downstream.
44-49: LGTM! Authorization scopes properly expanded
The changes align well with the PR objectives, expanding access to runs via tags and batches while maintaining security boundaries.
✅ Verification successful
Based on the search results, I can now generate the final response. The code shows that these properties (runTags, taskIdentifier, and friendlyId) are well-established throughout the codebase, particularly in the TaskRun model and related schemas. The authorization resource function is correctly using these properties.
Properties verified as safe and properly used in authorization context
The properties used in the authorization resource function are valid and secure:
friendlyId: Core identifier for runs, used consistently across the codebaserunTags: Standard property for run tagging, properly typed as string arraytaskIdentifier: Well-established property for task identificationbatch?.friendlyId: Safe optional chaining for batch reference
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify that these properties exist on the TaskRun model
ast-grep --pattern 'model TaskRun {
$$$
runTags: $_
$$$
taskIdentifier: $_
$$$
}'
Length of output: 93
Script:
#!/bin/bash
# Let's try to find the model definition using ripgrep
rg "class TaskRun|interface TaskRun|type TaskRun" -A 10
# Also search for property usage to verify their existence
rg "runTags|taskIdentifier|friendlyId" --type typescript
Length of output: 32008
Script:
#!/bin/bash
# Let's search for these properties in TypeScript files
rg "runTags|taskIdentifier|friendlyId" --type ts
# Also search for the TaskRun model/interface definition
rg "interface TaskRun|type TaskRun =|class TaskRun" --type ts -A 5
Length of output: 87796
docs/frontend/overview.mdx (4)
14-14: LGTM! Clear warning about default token behavior.
The comment effectively warns users that a token without scopes is useless, encouraging proper scope configuration.
Line range hint 25-77: LGTM! Well-structured progression of scoping examples.
The examples effectively demonstrate the progression from broad to specific scoping, with clear indicators (❌/✅) of best practices. The new capabilities for tag and batch-based scoping align perfectly with the PR objectives.
Line range hint 79-89: LGTM! Clear demonstration of scope combination.
The example effectively shows how to combine different scope types, enhancing the flexibility of the access control system.
108-119: LGTM! Excellent security recommendation.
The documentation strongly emphasizes the security best practice of using short-lived tokens for write operations, with a clear example of implementation.
Public access token scopes with just tags or just a batch can now access runs that have those tags or are in the batch. Previously, the only way to access a run was to have a specific scope for that exact run.