feat: support _gateway prop for cloud auth in Slice/FFmpeg/Probe resolvers#213
Open
SecurityQQ wants to merge 1 commit into
Open
feat: support _gateway prop for cloud auth in Slice/FFmpeg/Probe resolvers#213SecurityQQ wants to merge 1 commit into
SecurityQQ wants to merge 1 commit into
Conversation
Contributor
📝 Walkthroughwalkthroughgateway credential resolution now supports injecting a changes
estimated code review effort🎯 2 (simple) | ⏱️ ~10 minutes poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/react/types.ts (1)
422-423: extract a sharedgatewayconfiginterface for_gatewaysame shape repeats 3 times (lines 423, 434, 441) — pulling it into one interface keeps this internal contract consistent and easier to evolve. aligns with ts guideline to use interfaces for object types.
suggested diff
+export interface GatewayConfig { + apiKey: string; + baseUrl: string; +} + export interface SliceProps { @@ - _gateway?: { apiKey: string; baseUrl: string }; + _gateway?: GatewayConfig; } @@ export interface FFmpegProps { @@ - _gateway?: { apiKey: string; baseUrl: string }; + _gateway?: GatewayConfig; } @@ export interface ProbeProps { @@ - _gateway?: { apiKey: string; baseUrl: string }; + _gateway?: GatewayConfig; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/types.ts` around lines 422 - 423, Create a shared interface named GatewayConfig (e.g., interface GatewayConfig { apiKey: string; baseUrl: string; }) and replace the inline object type used for the _gateway property with _gateway?: GatewayConfig in every occurrence (the _gateway declarations in this file). Preserve the existing `@internal` JSDoc on the _gateway property and keep the interface unexported if it’s meant to remain internal so the internal contract stays consistent and easy to evolve.src/react/resolve.ts (1)
912-913: use gateway-aware interface instead of double-caststhe
as unknown as Record<string, unknown>pattern at lines 998, 1076, and 1100 weakens type safety. introduce dedicated interfaces for the gateway prop structure to eliminate unsafe casts and align with typescript guidelines.suggested refactor
+interface GatewayConfig { + apiKey: string; + baseUrl: string; +} + +interface GatewayAwareProps { + _gateway?: GatewayConfig; +} + function getGatewayConfig( - props?: Record<string, unknown>, -): { apiKey: string; baseUrl: string } | null { + props?: GatewayAwareProps, +): GatewayConfig | null { @@ async function gatewayJobRequest( path: string, body: Record<string, unknown>, - props?: Record<string, unknown>, + props?: GatewayAwareProps, ): Promise<Record<string, unknown>> { @@ - props as unknown as Record<string, unknown>, + props, @@ - props as unknown as Record<string, unknown>, + props, @@ - const config = getGatewayConfig(props as unknown as Record<string, unknown>); + const config = getGatewayConfig(props);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/react/resolve.ts` around lines 912 - 913, The props parameter is typed as Record<string, unknown> and three places use unsafe casts (the `as unknown as Record<string, unknown>` pattern) — introduce a dedicated interface (e.g., GatewayProps or GatewayGatewayProp) that models the gateway prop shape (fields like apiKey?: string, baseUrl?: string, etc.), replace the loose props?: Record<string, unknown> with props?: GatewayProps (or narrow props.gateway to GatewayProps), and update the three call sites (the uses currently double-cast) to either use a proper type assertion to GatewayProps or, better, a runtime type-guard that narrows props.gateway before reading apiKey/baseUrl; update the function signature that returns { apiKey: string; baseUrl: string } | null to accept the new typed props and remove all `as unknown as Record<string, unknown>` casts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/react/resolve.ts`:
- Around line 914-917: The current check silently falls back to env auth when
props._gateway is present but malformed; update the logic in resolve.ts (the
block that reads props?._gateway and the similar block around lines 918-923) to
validate the gateway shape (e.g., ensure gw.apiKey and gw.baseUrl are present
and valid) and, if props._gateway exists but is invalid, throw a clear error (or
return a validation failure) instead of falling back to environment credentials;
reference the gw variable and the props._gateway checks so you alter those
branches to enforce strict validation and fail-fast on malformed gateway
objects.
---
Nitpick comments:
In `@src/react/resolve.ts`:
- Around line 912-913: The props parameter is typed as Record<string, unknown>
and three places use unsafe casts (the `as unknown as Record<string, unknown>`
pattern) — introduce a dedicated interface (e.g., GatewayProps or
GatewayGatewayProp) that models the gateway prop shape (fields like apiKey?:
string, baseUrl?: string, etc.), replace the loose props?: Record<string,
unknown> with props?: GatewayProps (or narrow props.gateway to GatewayProps),
and update the three call sites (the uses currently double-cast) to either use a
proper type assertion to GatewayProps or, better, a runtime type-guard that
narrows props.gateway before reading apiKey/baseUrl; update the function
signature that returns { apiKey: string; baseUrl: string } | null to accept the
new typed props and remove all `as unknown as Record<string, unknown>` casts.
In `@src/react/types.ts`:
- Around line 422-423: Create a shared interface named GatewayConfig (e.g.,
interface GatewayConfig { apiKey: string; baseUrl: string; }) and replace the
inline object type used for the _gateway property with _gateway?: GatewayConfig
in every occurrence (the _gateway declarations in this file). Preserve the
existing `@internal` JSDoc on the _gateway property and keep the interface
unexported if it’s meant to remain internal so the internal contract stays
consistent and easy to evolve.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8f0e61c1-2e8b-46e7-8d45-433eb0c57511
📒 Files selected for processing (2)
src/react/resolve.tssrc/react/types.ts
Comment on lines
+914
to
+917
| if (props?._gateway) { | ||
| const gw = props._gateway as { apiKey: string; baseUrl: string }; | ||
| if (gw.apiKey) return gw; | ||
| } |
Contributor
There was a problem hiding this comment.
don’t silently fall back when _gateway exists but is invalid
right now, _gateway being present but malformed still drops to env auth. that breaks the “fallback only when _gateway is absent” behavior and can route jobs with unintended creds.
suggested diff
function getGatewayConfig(
props?: Record<string, unknown>,
): { apiKey: string; baseUrl: string } | null {
- if (props?._gateway) {
- const gw = props._gateway as { apiKey: string; baseUrl: string };
- if (gw.apiKey) return gw;
+ if (props?._gateway) {
+ const gw = props._gateway as { apiKey: string; baseUrl: string };
+ if (!gw.apiKey || !gw.baseUrl) {
+ throw new Error("invalid _gateway config: apiKey and baseUrl are required");
+ }
+ return gw;
}
const apiKey = process.env.VARG_API_KEY;Also applies to: 918-923
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/react/resolve.ts` around lines 914 - 917, The current check silently
falls back to env auth when props._gateway is present but malformed; update the
logic in resolve.ts (the block that reads props?._gateway and the similar block
around lines 918-923) to validate the gateway shape (e.g., ensure gw.apiKey and
gw.baseUrl are present and valid) and, if props._gateway exists but is invalid,
throw a clear error (or return a validation failure) instead of falling back to
environment credentials; reference the gw variable and the props._gateway checks
so you alter those branches to enforce strict validation and fail-fast on
malformed gateway objects.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
getGatewayConfig()now checks for a_gatewayprop (injected byvarg.slice()/varg.probe()/varg.ffmpeg()) before falling back toprocess.env.VARG_API_KEY_gatewayfield toSliceProps,FFmpegProps,ProbeProps(internal, not user-facing)gatewayJobRequest()andgetGatewayConfig()in all 3 resolversStandalone
Slice()/FFmpeg()/Probe()continue working via env var for local CLI. Cloud rendering uses the injected_gatewayfromvarg.slice()etc.Part of the varg.slice/probe/ffmpeg auth fix (see companion PRs in gateway and render repos).