Setup IaC for staging environment #1059
Conversation
WalkthroughReplaces HEAD requests with GET Range (bytes=0-0) for video availability and URL resolution. Adds Vercel OIDC-based AWS credentials fallback across web and backend S3 utilities. Updates env schema to make AWS keys optional and add VERCEL_AWS_ROLE_ARN. Infra config adjusts Vercel provider version, IAM policies, OIDC/identity lookups, and environment wiring. Adds @vercel/functions dependencies. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Player as CapVideoPlayer
participant Server as Video Host
Note over Player: Preflight URL resolution (changed)
User->>Player: Load video
Player->>Server: GET /video Range: bytes=0-0
alt 2xx
Server-->>Player: 206/200 + final URL
Player->>Player: Determine finalUrl, CORS flags
Player-->>User: Initialize playback
else error
Player->>Player: Fallback / error handling
Player-->>User: Show error/fallback
end
sequenceDiagram
autonumber
participant App as App (web/backend)
participant Env as Environment
participant VercelOIDC as @vercel/functions/oidc
participant S3 as AWS S3
Note over App: S3 credentials resolution
App->>Env: Read CAP_AWS_ACCESS_KEY/SECRET, VERCEL_AWS_ROLE_ARN, REGION, ENDPOINT
alt Static keys present
App->>S3: Create client with accessKeyId/secretAccessKey
else No static keys
App->>VercelOIDC: awsCredentialsProvider(roleArn)
VercelOIDC-->>App: Temporary AWS credentials
App->>S3: Create client with OIDC credentials
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx (1)
310-315: Duplicate loadedmetadata listener causes double handlers.This will fire twice and can flip state unexpectedly.
Remove the duplicate add:
video.addEventListener("loadedmetadata", handleLoadedMetadataWithTracks); video.addEventListener("load", handleLoad); video.addEventListener("play", handlePlay); video.addEventListener("error", handleError as EventListener); - video.addEventListener("loadedmetadata", handleLoadedMetadataWithTracks);packages/web-backend/src/S3Buckets/index.ts (1)
78-79: Remove logging of S3 credentials.This logs secrets (accessKeyId/secretAccessKey). Remove immediately.
- console.log({ config });infra/sst-env.d.ts (1)
8-12: Env typings updated — DISCORD_BOT_TOKEN & GITHUB_APP_PRIVATE_KEY missing but still referenced.infra/sst-env.d.ts only exposes DATABASE_URL; restore the two missing env entries or remove/update their usages to avoid compile/runtime failures.
Affected locations:
- infra/sst-env.d.ts
- infra/sst.config.ts (DiscordBotScript secret setup)
- apps/discord-bot/src/index.ts (c.env.DISCORD_BOT_TOKEN, env.GITHUB_APP_PRIVATE_KEY)
- apps/discord-bot/worker-configuration.d.ts
- apps/discord-bot/wrangler.jsonc
🧹 Nitpick comments (8)
apps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx (1)
66-66: Use portable timeout type in browser contexts.NodeJS.Timeout conflicts in DOM; prefer ReturnType.
- const retryTimeout = useRef<NodeJS.Timeout | null>(null); + const retryTimeout = useRef<ReturnType<typeof setTimeout> | null>(null);apps/web/lib/transcribe.ts (1)
83-86: Header case + naming nit.Header is case-insensitive; capitalize and rename var for clarity.
- const headResponse = await fetch(videoUrl, { - method: "GET", - headers: { range: "bytes=0-0" }, - }); - if (!headResponse.ok) { + const probeResponse = await fetch(videoUrl, { + method: "GET", + headers: { Range: "bytes=0-0" }, + }); + if (!probeResponse.ok) {apps/web/utils/s3.ts (1)
67-81: Avoid constructing empty-string credentials; prefer role first, then keys.Empty strings can break auth; only set credentials when present.
- return { + const credentials = + env.VERCEL_AWS_ROLE_ARN + ? awsCredentialsProvider({ roleArn: env.VERCEL_AWS_ROLE_ARN }) + : env.CAP_AWS_ACCESS_KEY && env.CAP_AWS_SECRET_KEY + ? { + accessKeyId: env.CAP_AWS_ACCESS_KEY, + secretAccessKey: env.CAP_AWS_SECRET_KEY, + } + : undefined; + + return { endpoint: internal ? (env.S3_INTERNAL_ENDPOINT ?? env.CAP_AWS_ENDPOINT) : (env.S3_PUBLIC_ENDPOINT ?? env.CAP_AWS_ENDPOINT), region: env.CAP_AWS_REGION, - credentials: env.VERCEL_AWS_ROLE_ARN - ? awsCredentialsProvider({ roleArn: env.VERCEL_AWS_ROLE_ARN }) - : { - accessKeyId: env.CAP_AWS_ACCESS_KEY ?? "", - secretAccessKey: env.CAP_AWS_SECRET_KEY ?? "", - }, + ...(credentials ? { credentials } : {}), forcePathStyle: env.S3_PATH_STYLE, };packages/web-backend/src/S3Buckets/index.ts (1)
28-39: Prefer role-based creds when available; fall back to static keys.Aligns with least-privilege and web utils behavior.
- credentials: yield* Config.string("CAP_AWS_ACCESS_KEY").pipe( - Effect.zip(Config.string("CAP_AWS_SECRET_KEY")), - Effect.map(([accessKeyId, secretAccessKey]) => ({ - accessKeyId, - secretAccessKey, - })), - Effect.catchAll(() => - Config.string("VERCEL_AWS_ROLE_ARN").pipe( - Effect.map((arn) => awsCredentialsProvider({ roleArn: arn })), - ), - ), - ), + credentials: yield* Config.string("VERCEL_AWS_ROLE_ARN").pipe( + Effect.map((arn) => awsCredentialsProvider({ roleArn: arn })), + Effect.catchAll(() => + Config.string("CAP_AWS_ACCESS_KEY").pipe( + Effect.zip(Config.string("CAP_AWS_SECRET_KEY")), + Effect.map(([accessKeyId, secretAccessKey]) => ({ + accessKeyId, + secretAccessKey, + })), + ), + ), + ),infra/sst.config.ts (4)
46-49: Remove commented CloudFront lookup (dead code).Keeps the config lean and avoids future confusion.
52-71: Helper for Vercel env vars is good; avoid hard‑coding customEnvironmentIds.Resolve the staging environment ID programmatically or via config/secret instead of a literal to prevent drift.
93-97: Remove commented CloudFront env var block.Dead code; if needed later, track via an issue/ADR instead.
170-184: Remove the commented Planetscale block.Keeps the config focused and reduces maintenance noise.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
apps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx(1 hunks)apps/web/lib/transcribe.ts(1 hunks)apps/web/package.json(1 hunks)apps/web/utils/s3.ts(2 hunks)infra/sst-env.d.ts(1 hunks)infra/sst.config.ts(3 hunks)packages/env/server.ts(2 hunks)packages/web-backend/package.json(1 hunks)packages/web-backend/src/S3Buckets/index.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and data fetching in the web app
Web mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData rather than broad invalidations
Client code should use useEffectQuery/useEffectMutation and useRpcClient from apps/web/lib/EffectRuntime.ts; do not create ManagedRuntime inside components
Files:
apps/web/utils/s3.tsapps/web/lib/transcribe.tsapps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
apps/web/utils/s3.tspackages/env/server.tsapps/web/lib/transcribe.tsapps/web/app/s/[videoId]/_components/CapVideoPlayer.tsxinfra/sst-env.d.tsinfra/sst.config.tspackages/web-backend/src/S3Buckets/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/web/utils/s3.tspackages/env/server.tsapps/web/lib/transcribe.tsapps/web/app/s/[videoId]/_components/CapVideoPlayer.tsxinfra/sst-env.d.tsinfra/sst.config.tspackages/web-backend/src/S3Buckets/index.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/web/utils/s3.tspackages/env/server.tsapps/web/lib/transcribe.tsapps/web/app/s/[videoId]/_components/CapVideoPlayer.tsxinfra/sst-env.d.tsinfra/sst.config.tspackages/web-backend/src/S3Buckets/index.ts
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/utils/s3.tsapps/web/lib/transcribe.tsapps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx
apps/web/app/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer Server Components for initial data in the Next.js App Router and pass initialData to client components
Files:
apps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx
🧬 Code graph analysis (1)
apps/web/utils/s3.ts (1)
packages/env/server.ts (1)
serverEnv(83-87)
⏰ 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: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (12)
packages/env/server.ts (2)
24-26: Make CAP_AWS keys optional — LGTM.Matches OIDC fallback strategy.
68-69: Add VERCEL_AWS_ROLE_ARN — LGTM.Enables role-based creds.
apps/web/utils/s3.ts (1)
43-43: Import of Vercel OIDC provider — LGTM.Correct import path for role-based creds.
packages/web-backend/src/S3Buckets/index.ts (1)
6-6: Import OIDC provider — LGTM.Matches infra changes.
packages/web-backend/package.json (1)
20-20: Add @vercel/functions dependency — LGTM; verify workspace alignment.
Present in packages/web-backend/package.json; sandbox rg/jq scans failed to fully verify — confirm no other packages import it and that workspace package.json entries use a consistent version.apps/web/package.json (1)
71-71: Ensure server-only usage of @vercel/functions.This dep should not leak into client bundles. Validate no client module ("use client") imports it or transitively imports utils that do.
infra/sst.config.ts (6)
22-22: Confirm Vercel provider version pin.Ensure "3.15.1" matches the provider version available in your stack runner to avoid preview/apply failures.
32-36: Stage-based WEB_URL wiring looks good; check non-staging flows.If you rely on Vercel preview/dev, confirm those flows won’t need WEB_URL/NEXTAUTH_URL.
73-76: DATABASE_URL Secret wiring to Vercel looks good.
78-91: WEB_URL / NEXT_PUBLIC_WEB_URL / NEXTAUTH_URL wiring looks good.Confirm NEXTAUTH_URL expectations for preview/dev if those flows are used.
98-107: Bucket name exposure: verify this is intended.NEXT_PUBLIC_CAP_AWS_BUCKET exposes the bucket name to clients. Fine if access is controlled via presigned URLs; otherwise reconsider.
161-164: Role ARN wiring to Vercel env looks good.
| const response = await fetch(urlWithTimestamp, { | ||
| method: "GET", | ||
| headers: { range: "bytes=0-0" }, | ||
| }); |
There was a problem hiding this comment.
Avoid CORS preflight regressions: try HEAD first, then GET Range as fallback.
Range header can trigger preflight in browsers; retain HEAD-first to reduce failures.
Apply:
- const response = await fetch(urlWithTimestamp, {
- method: "GET",
- headers: { range: "bytes=0-0" },
- });
+ let response: Response;
+ try {
+ response = await fetch(urlWithTimestamp, { method: "HEAD" });
+ if (!response.ok) throw new Error("HEAD not ok");
+ } catch {
+ response = await fetch(urlWithTimestamp, {
+ method: "GET",
+ headers: { Range: "bytes=0-0" },
+ });
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const response = await fetch(urlWithTimestamp, { | |
| method: "GET", | |
| headers: { range: "bytes=0-0" }, | |
| }); | |
| let response: Response; | |
| try { | |
| response = await fetch(urlWithTimestamp, { method: "HEAD" }); | |
| if (!response.ok) throw new Error("HEAD not ok"); | |
| } catch { | |
| response = await fetch(urlWithTimestamp, { | |
| method: "GET", | |
| headers: { Range: "bytes=0-0" }, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In apps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx around lines 91–94,
the code currently issues a GET with a Range header which can trigger CORS
preflight; change it to first perform a HEAD request to the same URL (no Range
header) and treat a successful HEAD (response.ok) as confirmation the resource
is available, and only if HEAD fails or indicates the server doesn't support
HEAD, fall back to a GET with headers { range: "bytes=0-0" }; ensure you catch
network/errors from HEAD and proceed to the GET-range fallback so the behavior
remains resilient.
| const vercelUser = new aws.iam.User("VercelUser", { forceDestroy: false }); | ||
|
|
There was a problem hiding this comment.
Remove unused IAM User to avoid long‑lived credentials.
This user isn’t referenced elsewhere and expands your attack surface unnecessarily.
Apply this diff to remove it:
- const vercelUser = new aws.iam.User("VercelUser", { forceDestroy: false });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const vercelUser = new aws.iam.User("VercelUser", { forceDestroy: false }); |
🤖 Prompt for AI Agents
In infra/sst.config.ts around lines 50-51, the new aws.iam.User("VercelUser", {
forceDestroy: false }) is unused and creates long‑lived credentials; remove that
resource declaration line, delete any local variable or identifier related to
VercelUser, and clean up any now-unused imports or references in the file so
there are no dangling symbols or compile errors.
| const vercelOidc = aws.iam.getOpenIdConnectProviderOutput({ | ||
| url: `https://oidc.vercel.com/${VERCEL_TEAM_SLUG}`, | ||
| }); |
There was a problem hiding this comment.
Use the looked‑up OIDC provider ARN (or manage the provider via IaC).
You fetch the provider but don’t use it; also, relying on a hard‑coded ARN format can drift. Prefer the looked‑up ARN. If the provider may not exist, manage an aws.iam.OpenIdConnectProvider resource to make this reproducible.
Apply this diff to trust the looked‑up provider:
- Federated: $interpolate`arn:aws:iam::${awsAccount.id}:oidc-provider/oidc.vercel.com/${VERCEL_TEAM_SLUG}`,
+ Federated: vercelOidc.arn,Also applies to: 121-121
🤖 Prompt for AI Agents
In infra/sst.config.ts around lines 108-110 (and also apply same fix at line
121), you call aws.iam.getOpenIdConnectProviderOutput to look up the Vercel OIDC
provider but never use its returned ARN and instead rely on a hard-coded ARN
format; update the trust setup to reference the looked-up provider. Replace any
hard-coded provider ARN uses with the value returned by
getOpenIdConnectProviderOutput (e.g., provider.arn) or, if the provider may not
exist in the account, create and manage an aws.iam.OpenIdConnectProvider
resource and reference its ARN in the role trust policy so the ARN is
deterministic and maintained by IaC. Ensure the role assume-role/trust policy
uses that ARN variable rather than a constructed string.
| inlinePolicies: [ | ||
| { | ||
| name: "VercelAWSAccessPolicy", | ||
| policy: recordingsBucket.arn.apply((arn) => | ||
| JSON.stringify({ | ||
| Version: "2012-10-17", | ||
| Statement: [ | ||
| { | ||
| Effect: "Allow", | ||
| Action: ["s3:*"], | ||
| Resource: `${arn}/*`, | ||
| }, | ||
| { | ||
| Effect: "Allow", | ||
| Action: ["s3:*"], | ||
| Resource: `${arn}`, | ||
| }, | ||
| ], | ||
| }), | ||
| ), | ||
| }, | ||
| ], |
There was a problem hiding this comment.
Tighten S3 permissions for the Vercel role (principle of least privilege).
s3:* is overly broad. Restrict to the minimal set required (e.g., ListBucket on the bucket; Get/Put/DeleteObject on objects). Expand only as needed for your flows (e.g., multipart).
Apply this diff to narrow the policy:
- inlinePolicies: [
+ inlinePolicies: [
{
name: "VercelAWSAccessPolicy",
policy: recordingsBucket.arn.apply((arn) =>
JSON.stringify({
Version: "2012-10-17",
Statement: [
{
Effect: "Allow",
- Action: ["s3:*"],
+ Action: ["s3:GetObject", "s3:PutObject", "s3:DeleteObject"],
Resource: `${arn}/*`,
},
{
Effect: "Allow",
- Action: ["s3:*"],
+ Action: ["s3:ListBucket"],
Resource: `${arn}`,
},
],
}),
),
},
],📝 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.
| inlinePolicies: [ | |
| { | |
| name: "VercelAWSAccessPolicy", | |
| policy: recordingsBucket.arn.apply((arn) => | |
| JSON.stringify({ | |
| Version: "2012-10-17", | |
| Statement: [ | |
| { | |
| Effect: "Allow", | |
| Action: ["s3:*"], | |
| Resource: `${arn}/*`, | |
| }, | |
| { | |
| Effect: "Allow", | |
| Action: ["s3:*"], | |
| Resource: `${arn}`, | |
| }, | |
| ], | |
| }), | |
| ), | |
| }, | |
| ], | |
| inlinePolicies: [ | |
| { | |
| name: "VercelAWSAccessPolicy", | |
| policy: recordingsBucket.arn.apply((arn) => | |
| JSON.stringify({ | |
| Version: "2012-10-17", | |
| Statement: [ | |
| { | |
| Effect: "Allow", | |
| Action: ["s3:GetObject", "s3:PutObject", "s3:DeleteObject"], | |
| Resource: `${arn}/*`, | |
| }, | |
| { | |
| Effect: "Allow", | |
| Action: ["s3:ListBucket"], | |
| Resource: `${arn}`, | |
| }, | |
| ], | |
| }), | |
| ), | |
| }, | |
| ], |
🤖 Prompt for AI Agents
In infra/sst.config.ts around lines 137 to 158 the inline policy grants s3:*
which is too broad; replace the wildcard with least-privilege actions: on the
bucket resource (arn) allow "s3:ListBucket" (and optional "s3:GetBucketLocation"
if needed); on objects (arn/*) allow "s3:GetObject", "s3:PutObject",
"s3:DeleteObject" and, if your flows use multipart, also include
"s3:AbortMultipartUpload", "s3:ListMultipartUploadParts" and
"s3:CreateMultipartUpload"; update the two Resource entries accordingly and
remove s3:* so the policy only contains these specific actions.
Summary by CodeRabbit