-
Notifications
You must be signed in to change notification settings - Fork 1.1k
last few staging things #1190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
last few staging things #1190
Conversation
WalkthroughIntroduces a new AwsCredentials service and replaces hard-coded/static AWS credentials with dynamic, effect-driven retrieval across S3 and CloudFront usages. Updates dependency wiring to provide AwsCredentials, adjusts infra typings and configuration, re-exports the new service, and includes a minor formatting change in a Stripe webhook route. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as App code (web/server)
participant Aws as AwsCredentials (Effect service)
participant Env as Env Secrets (CAP_AWS_ACCESS_KEY/SECRET)
participant Role as Vercel Role ARN
participant SSO as Dev SSO Profile
participant Meta as Container Metadata
participant AWS as AWS SDK Clients (S3/CloudFront)
App->>Aws: request credentials (Effect)
alt Env secrets present
Aws->>Env: read access/secret
Env-->>Aws: credentials
else Role ARN present
Aws->>Role: assume role (provider)
Role-->>Aws: credentials
else Dev environment
Aws->>SSO: fromSSO(profile)
SSO-->>Aws: credentials
else Fallback
Aws->>Meta: fromContainerMetadata()
Meta-->>Aws: credentials
end
Aws-->>App: { credentials }
App->>AWS: instantiate clients with credentials
AWS-->>App: perform S3/CloudFront operations
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/actions/video/upload.ts (1)
54-66: CloudFront invalidation condition is inverted and can skip/over-invalidate; also make it best-effort
- Invalidate when using the default bucket, not when using a different custom bucket.
- Wrap client construction in try/catch so invalidation failures don’t block presign flow.
Apply this diff:
- if ( - !customBucket || - !s3Config || - customBucket.bucketName !== serverEnv().CAP_AWS_BUCKET - ) { + if (!customBucket || customBucket.bucketName === serverEnv().CAP_AWS_BUCKET) { const distributionId = serverEnv().CAP_CLOUDFRONT_DISTRIBUTION_ID; if (distributionId) { - const cloudfront = new CloudFrontClient({ - region: serverEnv().CAP_AWS_REGION || "us-east-1", - credentials: await runPromise( - Effect.map(AwsCredentials, (c) => c.credentials), - ), - }); - - const pathToInvalidate = "/" + fileKey; - - try { - await cloudfront.send( + try { + const cloudfront = new CloudFrontClient({ + region: serverEnv().CAP_AWS_REGION || "us-east-1", + credentials: await runPromise( + Effect.map(AwsCredentials, (c) => c.credentials), + ), + }); + const pathToInvalidate = "/" + fileKey; + await cloudfront.send( new CreateInvalidationCommand({ DistributionId: distributionId, InvalidationBatch: { CallerReference: `${Date.now()}`, Paths: { Quantity: 1, Items: [pathToInvalidate], }, }, }), ); } catch (error) { console.error("Failed to create CloudFront invalidation:", error); } } }Also applies to: 68-86
apps/web/app/api/upload/[...route]/signed.ts (1)
65-79: Same invalidation condition problem; also wrap client construction in try/catch
- Only invalidate when using the default bucket.
- Make invalidation non-blocking by including client creation in try/catch.
Apply this diff:
- if ( - !customBucket || - !s3Config || - customBucket.bucketName !== serverEnv().CAP_AWS_BUCKET - ) { + if (!customBucket || customBucket.bucketName === serverEnv().CAP_AWS_BUCKET) { const distributionId = serverEnv().CAP_CLOUDFRONT_DISTRIBUTION_ID; if (distributionId) { console.log("Creating CloudFront invalidation for", fileKey); - - const cloudfront = new CloudFrontClient({ - region: serverEnv().CAP_AWS_REGION || "us-east-1", - credentials: await runPromise( - Effect.map(AwsCredentials, (c) => c.credentials), - ), - }); - - const pathToInvalidate = "/" + fileKey; - - try { - const invalidation = await cloudfront.send( + try { + const cloudfront = new CloudFrontClient({ + region: serverEnv().CAP_AWS_REGION || "us-east-1", + credentials: await runPromise( + Effect.map(AwsCredentials, (c) => c.credentials), + ), + }); + const pathToInvalidate = "/" + fileKey; + const invalidation = await cloudfront.send( new CreateInvalidationCommand({ DistributionId: distributionId, InvalidationBatch: { CallerReference: `${Date.now()}`, Paths: { Quantity: 1, Items: [pathToInvalidate], }, }, }), ); console.log("CloudFront invalidation created:", invalidation); } catch (error) { console.error("Failed to create CloudFront invalidation:", error); } } }Also applies to: 81-100
🧹 Nitpick comments (3)
packages/web-backend/src/Aws.ts (2)
15-31: Support optional session token for static keysHandle CAP_AWS_SESSION_TOKEN when access keys are temporary STS creds.
- const accessKeys = yield* Config.option( + const accessKeys = yield* Config.option( Config.all([ Config.string("CAP_AWS_ACCESS_KEY"), Config.string("CAP_AWS_SECRET_KEY"), ]), ); + const sessionToken = yield* Config.option( + Config.string("CAP_AWS_SESSION_TOKEN"), + ); @@ - if (Option.isSome(accessKeys)) { + if (Option.isSome(accessKeys)) { const [accessKeyId, secretAccessKey] = accessKeys.value; yield* Effect.log("Using CAP_AWS_ACCESS_KEY and CAP_AWS_SECRET_KEY"); - credentials = { accessKeyId, secretAccessKey }; + credentials = Option.isSome(sessionToken) + ? { accessKeyId, secretAccessKey, sessionToken: sessionToken.value } + : { accessKeyId, secretAccessKey }; } else if (Option.isSome(vercelAwsRole)) {
32-35: Avoid passing undefined profile to fromSSOPass profile only when set to prevent subtle resolution issues.
- } else if (process.env.NODE_ENV === "development") { + } else if (process.env.NODE_ENV === "development") { yield* Effect.log("Using AWS_DEFAULT_PROFILE"); - credentials = fromSSO({ profile: process.env.AWS_DEFAULT_PROFILE }); + const profile = process.env.AWS_DEFAULT_PROFILE; + credentials = profile ? fromSSO({ profile }) : fromSSO(); } else {infra/sst.config.ts (1)
132-155: Tighten IAM policy: avoid s3: on bucket*Limit to least privilege for bucket-level and object-level actions.
- policy: $resolve([ - recordingsBucket.arn, - cloudfrontDistribution?.arn, - ] as const).apply(([bucketArn, cloudfrontArn]) => - JSON.stringify({ - Version: "2012-10-17", - Statement: [ - { - Effect: "Allow", - Action: ["s3:*"], - Resource: `${bucketArn}/*`, - }, - { - Effect: "Allow", - Action: ["s3:*"], - Resource: bucketArn, - }, - cloudfrontArn && { - Effect: "Allow", - Action: ["cloudfront:CreateInvalidation"], - Resource: cloudfrontArn, - }, - ].filter(Boolean), - }), - ), + policy: $resolve([ + recordingsBucket.arn, + cloudfrontDistribution?.arn, + ] as const).apply(([bucketArn, cloudfrontArn]) => + JSON.stringify({ + Version: "2012-10-17", + Statement: [ + // Bucket-level permissions + { + Effect: "Allow", + Action: ["s3:ListBucket"], + Resource: bucketArn, + }, + // Object-level permissions + { + Effect: "Allow", + Action: [ + "s3:GetObject", + "s3:PutObject", + "s3:DeleteObject", + "s3:AbortMultipartUpload" + ], + Resource: `${bucketArn}/*`, + }, + // CloudFront invalidation (prod only) + cloudfrontArn && { + Effect: "Allow", + Action: ["cloudfront:CreateInvalidation"], + Resource: cloudfrontArn, + }, + ].filter(Boolean), + }), + ),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/web/actions/video/upload.ts(2 hunks)apps/web/app/api/upload/[...route]/signed.ts(2 hunks)apps/web/lib/server.ts(2 hunks)infra/sst-env.d.ts(1 hunks)infra/sst.config.ts(6 hunks)packages/web-backend/src/Aws.ts(1 hunks)packages/web-backend/src/S3Buckets/index.ts(4 hunks)packages/web-backend/src/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.Use strict TypeScript and avoid any; leverage shared types
Files:
apps/web/actions/video/upload.tspackages/web-backend/src/index.tsinfra/sst-env.d.tsapps/web/lib/server.tspackages/web-backend/src/Aws.tsapps/web/app/api/upload/[...route]/signed.tspackages/web-backend/src/S3Buckets/index.tsinfra/sst.config.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/actions/video/upload.tspackages/web-backend/src/index.tsinfra/sst-env.d.tsapps/web/lib/server.tspackages/web-backend/src/Aws.tsapps/web/app/api/upload/[...route]/signed.tspackages/web-backend/src/S3Buckets/index.tsinfra/sst.config.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/actions/video/upload.tsapps/web/lib/server.tsapps/web/app/api/upload/[...route]/signed.ts
apps/web/actions/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
All Groq/OpenAI calls must execute in Next.js Server Actions under apps/web/actions; do not invoke AI providers elsewhere
Files:
apps/web/actions/video/upload.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and fetching in the web app
Mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData
Run server-side effects via the ManagedRuntime from apps/web/lib/server.ts using EffectRuntime.runPromise/runPromiseExit; do not create runtimes ad hoc
Client code should use helpers from apps/web/lib/EffectRuntime.ts (useEffectQuery, useEffectMutation, useRpcClient); never call ManagedRuntime.make inside components
Files:
apps/web/actions/video/upload.tsapps/web/lib/server.tsapps/web/app/api/upload/[...route]/signed.ts
apps/web/app/api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/app/api/**/*.{ts,tsx}: Prefer Server Actions for API surface; when routes are necessary, implement under app/api and export only the handler from apiToHandler(ApiLive)
Construct API routes with @effect/platform HttpApi/HttpApiBuilder, declare contracts with Schema, and only export the handler
Use HttpAuthMiddleware for required auth and provideOptionalAuth for guests; avoid duplicating session lookups
Map domain errors to transport with HttpApiError.* and keep translation exhaustive (catchTags/tapErrorCause)
Inside HttpApiBuilder.group, acquire services with Effect.gen and provide dependencies via Layer.provide instead of manual provideService
Files:
apps/web/app/api/upload/[...route]/signed.ts
apps/web/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Server components needing Effect services must call EffectRuntime.runPromise(effect.pipe(provideOptionalAuth))
Files:
apps/web/app/api/upload/[...route]/signed.ts
🧠 Learnings (1)
📚 Learning: 2025-09-24T07:24:21.449Z
Learnt from: Brendonovich
PR: CapSoftware/Cap#1060
File: apps/web/components/forms/server.ts:85-97
Timestamp: 2025-09-24T07:24:21.449Z
Learning: In the S3BucketAccess service, bucketName is an Effect that must be yielded (using yield*) to get the actual string value before use in string interpolation or other operations.
Applied to files:
packages/web-backend/src/S3Buckets/index.ts
🧬 Code graph analysis (4)
apps/web/actions/video/upload.ts (2)
apps/web/lib/server.ts (1)
runPromise(124-136)packages/web-backend/src/Aws.ts (1)
AwsCredentials(9-43)
apps/web/lib/server.ts (1)
packages/web-backend/src/Aws.ts (1)
AwsCredentials(9-43)
apps/web/app/api/upload/[...route]/signed.ts (2)
apps/web/lib/server.ts (1)
runPromise(124-136)packages/web-backend/src/Aws.ts (1)
AwsCredentials(9-43)
packages/web-backend/src/S3Buckets/index.ts (2)
packages/web-backend/src/Aws.ts (1)
AwsCredentials(9-43)packages/web-backend/src/Database.ts (1)
Database(5-15)
⏰ 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 (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (12)
infra/sst-env.d.ts (1)
7-50: Auto-generated types look fineResource shapes and module augmentation are consistent; no changes needed here.
Also applies to: 54-55
packages/web-backend/src/index.ts (1)
2-2: Expose Aws surfaceGood call re-exporting Aws.ts to make AwsCredentials available to consumers.
apps/web/lib/server.ts (1)
5-16: AwsCredentials wired into server runtimeIncluding AwsCredentials.Default in Dependencies is correct and aligns with downstream usage.
Also applies to: 100-111
apps/web/actions/video/upload.ts (1)
63-66: Credential sourcing via AwsCredentials looks goodPassing either identity or provider into CloudFrontClient is valid. Nice centralization.
apps/web/app/api/upload/[...route]/signed.ts (1)
76-79: Credential centralization is correctUsing AwsCredentials via runPromise is appropriate here.
packages/web-backend/src/S3Buckets/index.ts (2)
8-9: Credential wiring via AwsCredentials looks correctDependency injection and S3Client config now consume the provider. Nice consolidation.
Also applies to: 17-18, 29-29
183-187: Good: dependencies updated to include AwsCredentials.DefaultEnsures service resolution at runtime.
infra/sst.config.ts (5)
48-52: Retain S3 bucket on delete: LGTMPrevents accidental data loss across stage transitions.
66-69: Prod-only CloudFront resolutionLooks good. Verify no staging code paths require invalidations.
95-103: OIDC provider handling per stage makes senseReuse in prod/staging, create otherwise. Ensure provider exists in target accounts for those stages.
161-165: Staging-only workflow clusterReasonable scoping; avoids infra drift in prod.
166-194: Env var creation loop: safe filtering and stage-specific targetsLooks good. Values using Outputs are preserved; staging customEnvironmentIds applied only when needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
infra/sst.config.ts (3)
13-14: Hard-coded AXIOM API token committed to source. Move to a secret.Avoid committing tokens; store in Secret and reference via
secrets. Also ensures Pulumi state doesn’t contain cleartext.Apply this diff here to remove the hard-coded token:
-const AXIOM_API_TOKEN = "xaat-c0704be6-e942-4935-b068-3b491d7cc00f"; +// AXIOM_API_TOKEN will be provided via Secrets()Then, add AXIOM_API_TOKEN to Secrets and update usages:
- In
function Secrets()add:AXIOM_API_TOKEN: new sst.Secret("AXIOM_API_TOKEN"),
- Update env var wiring (examples):
// vercelVariables entry { key: "NEXT_PUBLIC_AXIOM_TOKEN", value: secrets.AXIOM_API_TOKEN.value } // commonEnvironment AXIOM_API_TOKEN: secrets.AXIOM_API_TOKEN.value
200-210: Add AXIOM_API_TOKEN to Secrets to replace the in-code literal.Centralize the token with other secrets.
Apply this diff inside
Secrets():return { DATABASE_URL_MYSQL: new sst.Secret("DATABASE_URL_MYSQL"), CAP_AWS_ACCESS_KEY: new sst.Secret("CAP_AWS_ACCESS_KEY"), CAP_AWS_SECRET_KEY: new sst.Secret("CAP_AWS_SECRET_KEY"), GITHUB_PAT: new sst.Secret("GITHUB_PAT"), + AXIOM_API_TOKEN: new sst.Secret("AXIOM_API_TOKEN"), WORKFLOWS_RPC_SECRET: new random.RandomString("WORKFLOWS_RPC_SECRET", { length: 48, }), };
300-309: Use secret-backed AXIOM token in service environment.Avoid embedding tokens in code and Pulumi state.
- AXIOM_API_TOKEN, + AXIOM_API_TOKEN: secrets.AXIOM_API_TOKEN.value,Additionally, consider wrapping with
pulumi.secret(...)if the provider doesn’t already mark it as a secret Output.
🧹 Nitpick comments (3)
packages/web-backend/src/S3Buckets/index.ts (1)
8-8: LGTM! Well-executed credential refactor.The service now correctly depends on
AwsCredentialsfor credential management, eliminating duplicated credential resolution logic. The credentials are properly extracted (line 17) and integrated into the default client configuration (line 29), with the dependency correctly declared (lines 183-187).Also applies to: 17-17, 29-29, 183-187
infra/sst.config.ts (2)
71-71: Remove unused IAM user.
VercelUseris created but never used; drop it to reduce surface area.-const vercelUser = new aws.iam.User("VercelUser", { forceDestroy: false });
169-182: Avoid NonNullable cast; type the collection up-front and treat secrets as Pulumi secrets.Minor: instead of casting inside
forEach, build a typed array ofvercel.ProjectEnvironmentVariableArgs. Also mark sensitive values (likeWORKFLOWS_RPC_SECRET) as Pulumi secrets to avoid plaintext in state.Example:
const vars: Array<Omit<vercel.ProjectEnvironmentVariableArgs, "projectId">> = [ ...vercelVariables, ...(workflowCluster ? [ { key: "WORKFLOWS_RPC_URL", value: workflowCluster.api.url }, { key: "WORKFLOWS_RPC_SECRET", value: pulumi.secret(secrets.WORKFLOWS_RPC_SECRET.result) }, ] : []), { key: "VERCEL_AWS_ROLE_ARN", value: vercelAwsAccessRole.arn }, ]; vars.forEach((v) => new vercel.ProjectEnvironmentVariable(`VercelEnv${v.key}`, { ...v, projectId: vercelProject.id, /* ... */ }));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/web/actions/video/upload.ts(2 hunks)apps/web/app/api/upload/[...route]/signed.ts(2 hunks)apps/web/app/api/webhooks/stripe/route.ts(1 hunks)apps/web/lib/server.ts(2 hunks)infra/sst-env.d.ts(1 hunks)infra/sst.config.ts(6 hunks)packages/web-backend/src/Aws.ts(1 hunks)packages/web-backend/src/S3Buckets/index.ts(4 hunks)packages/web-backend/src/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.Use strict TypeScript and avoid any; leverage shared types
Files:
packages/web-backend/src/index.tspackages/web-backend/src/Aws.tsapps/web/app/api/webhooks/stripe/route.tsapps/web/lib/server.tspackages/web-backend/src/S3Buckets/index.tsinfra/sst.config.tsapps/web/actions/video/upload.tsapps/web/app/api/upload/[...route]/signed.tsinfra/sst-env.d.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:
packages/web-backend/src/index.tspackages/web-backend/src/Aws.tsapps/web/app/api/webhooks/stripe/route.tsapps/web/lib/server.tspackages/web-backend/src/S3Buckets/index.tsinfra/sst.config.tsapps/web/actions/video/upload.tsapps/web/app/api/upload/[...route]/signed.tsinfra/sst-env.d.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/app/api/webhooks/stripe/route.tsapps/web/lib/server.tsapps/web/actions/video/upload.tsapps/web/app/api/upload/[...route]/signed.ts
apps/web/app/api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/app/api/**/*.{ts,tsx}: Prefer Server Actions for API surface; when routes are necessary, implement under app/api and export only the handler from apiToHandler(ApiLive)
Construct API routes with @effect/platform HttpApi/HttpApiBuilder, declare contracts with Schema, and only export the handler
Use HttpAuthMiddleware for required auth and provideOptionalAuth for guests; avoid duplicating session lookups
Map domain errors to transport with HttpApiError.* and keep translation exhaustive (catchTags/tapErrorCause)
Inside HttpApiBuilder.group, acquire services with Effect.gen and provide dependencies via Layer.provide instead of manual provideService
Files:
apps/web/app/api/webhooks/stripe/route.tsapps/web/app/api/upload/[...route]/signed.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and fetching in the web app
Mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData
Run server-side effects via the ManagedRuntime from apps/web/lib/server.ts using EffectRuntime.runPromise/runPromiseExit; do not create runtimes ad hoc
Client code should use helpers from apps/web/lib/EffectRuntime.ts (useEffectQuery, useEffectMutation, useRpcClient); never call ManagedRuntime.make inside components
Files:
apps/web/app/api/webhooks/stripe/route.tsapps/web/lib/server.tsapps/web/actions/video/upload.tsapps/web/app/api/upload/[...route]/signed.ts
apps/web/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Server components needing Effect services must call EffectRuntime.runPromise(effect.pipe(provideOptionalAuth))
Files:
apps/web/app/api/webhooks/stripe/route.tsapps/web/app/api/upload/[...route]/signed.ts
apps/web/actions/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
All Groq/OpenAI calls must execute in Next.js Server Actions under apps/web/actions; do not invoke AI providers elsewhere
Files:
apps/web/actions/video/upload.ts
🧠 Learnings (1)
📚 Learning: 2025-09-24T07:24:21.449Z
Learnt from: Brendonovich
PR: CapSoftware/Cap#1060
File: apps/web/components/forms/server.ts:85-97
Timestamp: 2025-09-24T07:24:21.449Z
Learning: In the S3BucketAccess service, bucketName is an Effect that must be yielded (using yield*) to get the actual string value before use in string interpolation or other operations.
Applied to files:
packages/web-backend/src/S3Buckets/index.ts
🧬 Code graph analysis (5)
apps/web/app/api/webhooks/stripe/route.ts (2)
packages/env/server.ts (1)
serverEnv(84-88)packages/env/index.ts (1)
serverEnv(2-2)
apps/web/lib/server.ts (1)
packages/web-backend/src/Aws.ts (1)
AwsCredentials(9-43)
packages/web-backend/src/S3Buckets/index.ts (2)
packages/web-backend/src/Aws.ts (1)
AwsCredentials(9-43)packages/web-backend/src/Database.ts (1)
Database(5-15)
apps/web/actions/video/upload.ts (2)
apps/web/lib/server.ts (1)
runPromise(124-136)packages/web-backend/src/Aws.ts (1)
AwsCredentials(9-43)
apps/web/app/api/upload/[...route]/signed.ts (2)
apps/web/lib/server.ts (1)
runPromise(124-136)packages/web-backend/src/Aws.ts (1)
AwsCredentials(9-43)
⏰ 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 (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (11)
apps/web/app/api/webhooks/stripe/route.ts (1)
118-121: LGTM! Improved readability.The ternary expression is now more readable with the multi-line format while preserving the same logic.
infra/sst-env.d.ts (1)
1-1: Auto-generated file - no manual changes needed.This file is auto-generated by SST (line 1 comment). The new resource definitions reflect infrastructure changes and are correctly formatted.
packages/web-backend/src/index.ts (1)
2-2: LGTM! AwsCredentials correctly exposed.The new export makes AwsCredentials available to package consumers, enabling the credential refactor across the codebase.
apps/web/lib/server.ts (1)
5-5: LGTM! AwsCredentials integrated into dependency layer.The new service is properly imported and added to the merged dependencies, making it available throughout the application's Effect runtime.
Also applies to: 108-108
apps/web/app/api/upload/[...route]/signed.ts (1)
9-9: LGTM! Dynamic credentials correctly implemented.The CloudFrontClient now uses dynamic credentials from AwsCredentials via the Effect runtime, replacing static environment-based configuration. The pattern using
runPromiseandEffect.mapis correct and consistent with other changes in the PR.Also applies to: 76-78
apps/web/actions/video/upload.ts (1)
13-13: LGTM! Consistent credential usage pattern.The implementation matches the pattern used in
apps/web/app/api/upload/[...route]/signed.ts, maintaining consistency across CloudFront client initialization throughout the codebase.Also applies to: 63-65
packages/web-backend/src/Aws.ts (1)
1-43: LGTM! Well-designed credential provider.The
AwsCredentialsservice implements a robust credential resolution strategy with a clear precedence order:
- CAP_AWS_ACCESS_KEY / CAP_AWS_SECRET_KEY (static credentials)
- VERCEL_AWS_ROLE_ARN (Vercel OIDC provider)
- AWS_DEFAULT_PROFILE (SSO in development)
- ECS metadata (fallback for container environments)
The implementation correctly:
- Uses
Config.optionfor safe environment variable access- Returns appropriate credential types compatible with AWS SDK clients
- Logs the credential source for debugging without exposing secrets
- Follows the Effect.Service pattern for dependency injection
infra/sst.config.ts (4)
75-80: LGTM: conditional WEB_URL envs.This guard is fine and avoids empty values.
95-103: OIDC provider lookup vs. creation looks good.Pattern aligns with staged/prod reuse and ephemeral env creation.
Ensure the production/staging OIDC provider already exists at
https://oidc.vercel.com/${VERCEL_TEAM_SLUG}; otherwise stack will fail duringgetOpenIdConnectProviderOutput.
161-165: LGTM: staging-only WorkflowCluster.Creation is gated to staging; clean separation.
66-70: Directly use.arnfrom getDistributionOutput
aws.cloudfront.getDistributionOutputexposes anarnproperty, so you can referencecloudfrontDistribution.arndirectly.
| const recordingsBucket = new aws.s3.BucketV2( | ||
| "RecordingsBucket", | ||
| {}, | ||
| { retainOnDelete: true }, | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
S3 bucket lacks security hardening (public access block, SSE, ownership controls).
Harden defaults for the recordings bucket.
Keep the bucket instantiation, and add these resources below it:
new aws.s3.BucketPublicAccessBlock("RecordingsBucketPublicAccess", {
bucket: recordingsBucket.id,
blockPublicAcls: true,
ignorePublicAcls: true,
blockPublicPolicy: true,
restrictPublicBuckets: true,
});
new aws.s3.BucketOwnershipControls("RecordingsBucketOwnership", {
bucket: recordingsBucket.id,
rule: { objectOwnership: "BucketOwnerEnforced" },
});
new aws.s3.BucketServerSideEncryptionConfigurationV2("RecordingsBucketSSE", {
bucket: recordingsBucket.id,
rules: [
{
applyServerSideEncryptionByDefault: { sseAlgorithm: "AES256" },
bucketKeyEnabled: true,
},
],
});
// Optional but recommended:
new aws.s3.BucketVersioningV2("RecordingsBucketVersioning", {
bucket: recordingsBucket.id,
versioningConfiguration: { status: "Enabled" },
});🤖 Prompt for AI Agents
In infra/sst.config.ts around lines 48 to 52, the RecordingsBucket is created
without S3 security hardening; add resources immediately after the bucket to
enforce Block Public Access (blockPublicAcls, ignorePublicAcls,
blockPublicPolicy, restrictPublicBuckets), set Bucket Ownership Controls to
BucketOwnerEnforced, configure Server-Side Encryption (AES256 with bucket key
enabled), and optionally enable Versioning; reference recordingsBucket.id for
each new aws.s3 resource so the bucket gets public access blocked, ownership
enforced, SSE configured, and versioning enabled.
| const vercelVariables = [ | ||
| { key: "NEXT_PUBLIC_AXIOM_TOKEN", value: AXIOM_API_TOKEN }, | ||
| { key: "NEXT_PUBLIC_AXIOM_DATASET", value: AXIOM_DATASET }, | ||
| { key: "CAP_AWS_BUCKET", value: recordingsBucket.bucket }, | ||
| { key: "NEXT_PUBLIC_CAP_AWS_BUCKET", value: recordingsBucket.bucket }, | ||
| { key: "DATABASE_URL", value: secrets.DATABASE_URL_MYSQL.value }, | ||
| ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Use secret-backed AXIOM token for Vercel env like other sensitive values.
Replace the direct constant usage with the Secret value.
- { key: "NEXT_PUBLIC_AXIOM_TOKEN", value: AXIOM_API_TOKEN },
+ { key: "NEXT_PUBLIC_AXIOM_TOKEN", value: secrets.AXIOM_API_TOKEN.value },Note: Although labeled “NEXT_PUBLIC”, avoid committing the token in code; source-of-truth via Secret prevents accidental reuse and rotation issues.
📝 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 vercelVariables = [ | |
| { key: "NEXT_PUBLIC_AXIOM_TOKEN", value: AXIOM_API_TOKEN }, | |
| { key: "NEXT_PUBLIC_AXIOM_DATASET", value: AXIOM_DATASET }, | |
| { key: "CAP_AWS_BUCKET", value: recordingsBucket.bucket }, | |
| { key: "NEXT_PUBLIC_CAP_AWS_BUCKET", value: recordingsBucket.bucket }, | |
| { key: "DATABASE_URL", value: secrets.DATABASE_URL_MYSQL.value }, | |
| ]; | |
| const vercelVariables = [ | |
| { key: "NEXT_PUBLIC_AXIOM_TOKEN", value: secrets.AXIOM_API_TOKEN.value }, | |
| { key: "NEXT_PUBLIC_AXIOM_DATASET", value: AXIOM_DATASET }, | |
| { key: "CAP_AWS_BUCKET", value: recordingsBucket.bucket }, | |
| { key: "DATABASE_URL", value: secrets.DATABASE_URL_MYSQL.value }, | |
| ]; |
🤖 Prompt for AI Agents
In infra/sst.config.ts around lines 54 to 59, the Vercel env currently uses the
plain AXIOM_API_TOKEN constant; replace that with the secret-backed value (e.g.,
secrets.AXIOM_API_TOKEN.value) so the token is sourced from the Secrets object
like DATABASE_URL_MYSQL.value, preventing accidental commit and enabling
rotation — keep the key name as "NEXT_PUBLIC_AXIOM_TOKEN" but use the secret
value reference instead of the constant.
| StringLike: { | ||
| [`${oidc.url}:sub`]: [ | ||
| `owner:${VERCEL_TEAM_SLUG}:project:*:environment:staging`, | ||
| `owner:${VERCEL_TEAM_SLUG}:project:*:environment:${$app.stage}`, | ||
| ], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Narrow OIDC sub condition to least privilege (avoid project:*).
Scope sub to specific project(s) to reduce blast radius.
If the project name is cap-web, prefer:
- [`${oidc.url}:sub`]: [`owner:${VERCEL_TEAM_SLUG}:project:*:environment:${$app.stage}`]
+ [`${oidc.url}:sub`]: [`owner:${VERCEL_TEAM_SLUG}:project:cap-web:environment:${$app.stage}`]Verify the exact sub format per Vercel OIDC docs for role assumptions.
🌐 Web query:
What is the exact `sub` (subject) format emitted by Vercel OIDC for deployments? How to scope IAM role trust to a single project and environment?
💡 Result:
Exact sub format
- sub = "owner:[TEAM_SLUG]:project:[PROJECT_NAME]:environment:[ENVIRONMENT]". Example: "owner:acme:project:acme_website:environment:production". [1]
How to scope an IAM role trust to a single Vercel project + environment
- Create the OIDC provider for Vercel (issuer URL depends on issuer mode: global https://oidc.vercel.com or team https://oidc.vercel.com/[TEAM_SLUG]) and set the audience to https://vercel.com/[TEAM_SLUG]. [2]
- In the IAM role trust policy use the provider ARN as Principal and require both the audience and the exact sub claim using Condition -> StringEquals. Example (team issuer mode):
{
"Version":"2012-10-17",
"Statement":[
{
"Effect":"Allow",
"Principal":{
"Federated":"arn:aws:iam::[ACCOUNT_ID]:oidc-provider/oidc.vercel.com/[TEAM_SLUG]"
},
"Action":"sts:AssumeRoleWithWebIdentity",
"Condition":{
"StringEquals":{
"oidc.vercel.com/[TEAM_SLUG]:aud":"https://vercel.com/[TEAM_SLUG]",
"oidc.vercel.com/[TEAM_SLUG]:sub":"owner:[TEAM_SLUG]:project:[PROJECT_NAME]:environment:[ENVIRONMENT]"
}
}
}
]
}
- Notes: use StringLike with wildcards if you want to allow multiple projects/environments; for team→global issuer differences see Vercel docs. [1][2]
Sources
- Vercel OIDC reference and AWS guide. [1][2]
Narrow OIDC sub to Specific Project
Replace the wildcard with the exact project name to enforce least privilege. For example, for cap-web:
- [`${oidc.url}:sub`]: [`owner:${VERCEL_TEAM_SLUG}:project:*:environment:${$app.stage}`]
+ [`${oidc.url}:sub`]: [`owner:${VERCEL_TEAM_SLUG}:project:cap-web:environment:${$app.stage}`]📝 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.
| StringLike: { | |
| [`${oidc.url}:sub`]: [ | |
| `owner:${VERCEL_TEAM_SLUG}:project:*:environment:staging`, | |
| `owner:${VERCEL_TEAM_SLUG}:project:*:environment:${$app.stage}`, | |
| ], | |
| StringLike: { | |
| [`${oidc.url}:sub`]: [ | |
| `owner:${VERCEL_TEAM_SLUG}:project:cap-web:environment:${$app.stage}`, | |
| ], |
🤖 Prompt for AI Agents
In infra/sst.config.ts around lines 120 to 123, the OIDC StringLike claim
currently uses a wildcard for the Vercel project which is overly broad; replace
the `project:*` segment with the exact project slug (e.g., `project:cap-web`) or
with a specific env var like `project:${VERCEL_PROJECT_NAME}` so the claim
becomes `owner:${VERCEL_TEAM_SLUG}:project:cap-web:environment:${$app.stage}`
(or equivalent using the env var), and verify the project slug matches the
actual Vercel project name.
| policy: $resolve([ | ||
| recordingsBucket.arn, | ||
| cloudfrontDistribution?.arn, | ||
| ] as const).apply(([bucketArn, cloudfrontArn]) => | ||
| JSON.stringify({ | ||
| Version: "2012-10-17", | ||
| Statement: [ | ||
| { | ||
| Effect: "Allow", | ||
| Action: ["s3:*"], | ||
| Resource: `${arn}/*`, | ||
| Resource: `${bucketArn}/*`, | ||
| }, | ||
| { | ||
| Effect: "Allow", | ||
| Action: ["s3:*"], | ||
| Resource: `${arn}`, | ||
| Resource: bucketArn, | ||
| }, | ||
| ], | ||
| cloudfrontArn && { | ||
| Effect: "Allow", | ||
| Action: ["cloudfront:CreateInvalidation"], | ||
| Resource: cloudfrontArn, | ||
| }, | ||
| ].filter(Boolean), | ||
| }), | ||
| ), | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
IAM policy hardening: principle of least privilege and CloudFront resource scoping.
- S3 permissions are
s3:*on bucket and objects. Limit to required actions (e.g.,GetObject,PutObject,DeleteObject,ListBucket) to reduce risk. - Confirm whether
cloudfront:CreateInvalidationsupports resource-level ARNs. If not, setResource: "*"(typical for CloudFront invalidations).
Apply this diff to scope actions; adjust CloudFront after verification:
- Action: ["s3:*"],
+ Action: ["s3:ListBucket"],
Resource: ${bucketArn},
},
{
Effect: "Allow",
- Action: ["s3:*"],
+ Action: ["s3:GetObject","s3:PutObject","s3:DeleteObject"],
Resource: `${bucketArn}/*`,
},
- cloudfrontArn && {
- Effect: "Allow",
- Action: ["cloudfront:CreateInvalidation"],
- Resource: cloudfrontArn,
- },
+ // After verification, either keep ARN if supported or use "*"
+ cloudfrontArn && {
+ Effect: "Allow",
+ Action: ["cloudfront:CreateInvalidation"],
+ Resource: cloudfrontArn, // replace with "*" if ARNs aren't supported
+ },Also, ensure $resolve([recordingsBucket.arn, cloudfrontDistribution?.arn]) tolerates undefined. If not, wrap with an explicit nullable Output or branch before $resolve.
🌐 Web query:
Does AWS IAM support resource-level permissions for `cloudfront:CreateInvalidation`, or must `Resource` be `"*"`?
💡 Result:
Yes — cloudfront:CreateInvalidation supports resource-level permissions. The Service Authorization Reference shows CreateInvalidation requires the distribution resource (arn:...:distribution/...) so you can restrict Resource to that distribution ARN instead of "*". [1][2]
Sources:
- AWS Service Authorization Reference — CloudFront (CreateInvalidation → resource type: distribution*). [1]
- CloudFront IAM / Developer Guide (how resource-level permissions work for CloudFront). [2]
Scope S3 actions and narrow CloudFront invalidation resource
- Restrict S3 to
s3:ListBucketon${bucketArn}ands3:GetObject,s3:PutObject,s3:DeleteObjecton${bucketArn}/*. - CloudFront invalidation supports distribution ARNs—keep
Resource: cloudfrontArn. - Ensure
$resolve([recordingsBucket.arn, cloudfrontDistribution?.arn])handlesundefined.
{
Effect: "Allow",
- Action: ["s3:*"],
+ Action: ["s3:ListBucket"],
Resource: ${bucketArn},
},
{
Effect: "Allow",
- Action: ["s3:*"],
+ Action: ["s3:GetObject", "s3:PutObject", "s3:DeleteObject"],
Resource: `${bucketArn}/*`,
},
- cloudfrontArn && {
- Effect: "Allow",
- Action: ["cloudfront:CreateInvalidation"],
- Resource: cloudfrontArn,
- },
+ cloudfrontArn && {
+ Effect: "Allow",
+ Action: ["cloudfront:CreateInvalidation"],
+ Resource: cloudfrontArn,
+ },Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In infra/sst.config.ts around lines 132 to 157, the IAM policy is overly
permissive and may break when cloudfrontDistribution?.arn is undefined; update
the policy to (1) replace the broad "s3:*" permissions with two statements: one
granting "s3:ListBucket" on the bucket ARN (Resource: bucketArn) and another
granting "s3:GetObject","s3:PutObject","s3:DeleteObject" on the objects path
(Resource: `${bucketArn}/*`), (2) keep the CloudFront invalidation statement as
Effect: "Allow", Action: ["cloudfront:CreateInvalidation"], Resource:
cloudfrontArn (do not convert to wildcard), and (3) ensure the $resolve([...])
call and its apply callback handle undefined cloudfrontArn safely (e.g., include
cloudfrontArn in the array and only push the CloudFront statement when
cloudfrontArn is truthy) so the policy JSON never contains undefined.
Summary by CodeRabbit