fix(aws): gate /api/scheduled/* with bearer mode + Secrets Manager#203
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds optional scheduled-task bearer secret creation and propagation: a Secrets Manager secret is conditionally created and exported; compute modules (Lambda and Fargate) receive new inputs and set environment variables to enable bearer-mode scheduled-task auth; IAM permissions are expanded to allow reading the scheduled-task secret. Changes
Sequence Diagram(s)sequenceDiagram
participant SecrMod as Secrets Module (Terraform)
participant TF as Root Terraform
participant CompMod as Compute Modules (Lambda/Fargate)
participant IAM as IAM Policy
participant Runtime as Lambda / Fargate Runtime
participant SM as AWS Secrets Manager
SecrMod->>TF: create secret (conditional) & outputs ARN/name
TF->>CompMod: pass secret ARN/name (or "")
CompMod->>IAM: include secret ARN in GetSecretValue/DescribeSecret policy
TF->>Runtime: set env vars SCHEDULED_TASK_AUTH_MODE, SCHEDULED_TASK_SECRET_NAME
Runtime->>SM: DescribeSecret / GetSecretValue using secret name (at startup)
SM-->>Runtime: secret value (bearer token)
Runtime->>Runtime: use bearer token for /api/scheduled/* auth
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
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 unit tests (beta)
Review rate limit: 4/5 reviews remaining, refill in 12 minutes. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@terraform/modules/compute/aws/lambda/main.tf`:
- Around line 58-70: The module currently sets SCHEDULED_TASK_AUTH_MODE =
"disabled", which leaves the public Lambda URL unprotected; change
SCHEDULED_TASK_AUTH_MODE to "bearer" and wire SCHEDULED_TASK_SECRET to a Secrets
Manager value (ensure you reference the secret resource used in this module,
e.g., the aws_secretsmanager_secret / aws_secretsmanager_secret_version you're
using) so bearer token validation is enforced, or alternatively remove/exclude
the public Lambda URL exposure for this function; update any related references
to SCHEDULED_TASK_SECRET and the Lambda URL configuration to ensure the URL is
not publicly reachable if you cannot immediately use bearer mode.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e842fc79-6f27-4210-b998-625370aea0e2
📒 Files selected for processing (1)
terraform/modules/compute/aws/lambda/main.tf
PR #161 made SCHEDULED_TASK_AUTH_MODE required at startup but the env var was only wired on GCP and Azure. AWS Lambda was missed and the post-merge deploy crashed with: scheduledauth: invalid configuration: SCHEDULED_TASK_AUTH_MODE is unset (set to oidc, bearer, or disabled) (See https://github.com/LeanerCloud/CUDly/actions/runs/25148590105 .) The "obvious" fix — wire SCHEDULED_TASK_AUTH_MODE=disabled to mirror the pre-#161 reality where the bearer check was a no-op when the secret was empty — is wrong. ModeDisabled lets every /api/scheduled/* request through, and the Lambda Function URL has authorization_type = NONE (the same comment in this module's main.tf explicitly notes the URL is public). That would ship a production auth bypass. Instead, run AWS in bearer mode with the secret resolved from Secrets Manager at runtime — same model Azure already uses. Changes: * terraform/modules/secrets/aws — provision a random_password + aws_secretsmanager_secret + version for the scheduled-task bearer (gated by var.create_scheduled_task_secret, default true). Include it in the secret_read policy and expose ARN + name outputs. * terraform/modules/compute/aws/lambda — set SCHEDULED_TASK_AUTH_MODE=bearer + SCHEDULED_TASK_SECRET_NAME=var.scheduled_task_secret_name on the Lambda env block. Add the secret ARN to the secrets_access IAM policy. Add the corresponding scheduled_task_secret_arn / scheduled_task_secret_name variables. * terraform/modules/compute/aws/fargate — same wiring on the task definition env block + IAM policy + variables. Fargate isn't the default platform but should not regress when selected. * terraform/environments/aws/compute.tf — pass module.secrets.scheduled_task_secret_arn / _name into both compute_lambda and compute_fargate. Runtime path (already in place from #161): app.go reads SCHEDULED_TASK_SECRET_NAME, resolveScheduledTaskSecret fetches the plaintext from AWS Secrets Manager via the SecretResolver, and buildScheduledAuth overrides saCfg.Bearer with that value before calling scheduledauth.New. The plaintext never lives in Lambda env or Terraform state. Validation: terraform validate clean for all four touched modules. Once deployed, /api/scheduled/* on the public Lambda URL will reject every request that doesn't carry the resolved bearer secret. AWS schedulers (EventBridge -> direct Lambda invocation) bypass the HTTP middleware entirely, so they are unaffected.
37efe20 to
f001f83
Compare
|
Force-pushed @coderabbitai review |
|
✅ Actions performedReview triggered.
|
Hotfix for the failing Lambda health check on
feat/multicloud-web-frontendafter #161 merged: https://github.com/LeanerCloud/CUDly/actions/runs/25148590105/job/73714185103.Root cause
#161 made
SCHEDULED_TASK_AUTH_MODErequired at startup (fail-closedLoadConfig) but only wired the env var on the GCP and Azure compute modules. AWS Lambda was missed → boot fails:Why not
SCHEDULED_TASK_AUTH_MODE = "disabled"That was my first attempt (force-pushed away). It would have shipped a production auth bypass:
ModeDisabledlets every/api/scheduled/*request through, and the Lambda Function URL hasauthorization_type = NONE(the same module documents this). Even though pre-#161 the bearer check was a no-op when the secret was empty, codifying that state under the new validator is actively wrong.Fix: bearer mode + Secrets Manager (Azure-pattern)
terraform/modules/secrets/aws/— provision arandom_password+aws_secretsmanager_secret+ version for the scheduled-task bearer (gated byvar.create_scheduled_task_secret, defaulttrue). Include insecret_readpolicy. Exposescheduled_task_secret_arn+_nameoutputs.terraform/modules/compute/aws/lambda/— setSCHEDULED_TASK_AUTH_MODE = "bearer"+SCHEDULED_TASK_SECRET_NAME = var.scheduled_task_secret_nameon the Lambda env block. Add the secret ARN to thesecrets_accessIAM policy. Add the corresponding variables.terraform/modules/compute/aws/fargate/— same wiring on the task definition env block + IAM policy + variables, so Fargate doesn't regress when selected.terraform/environments/aws/compute.tf— passmodule.secrets.scheduled_task_secret_arn/_nameinto bothcompute_lambdaandcompute_fargate.Runtime path (already in place from #161)
internal/server/app.goreadsSCHEDULED_TASK_SECRET_NAME,resolveScheduledTaskSecretfetches the plaintext from AWS Secrets Manager via theSecretResolver, andbuildScheduledAuthoverridessaCfg.Bearerwith that value before callingscheduledauth.New. The plaintext never lives in Lambda env or Terraform state.Result
/api/scheduled/*on the public Lambda URL rejects every request that doesn't carry the resolved bearer secret.lambda:InvokeFunction) bypass the HTTP middleware entirely and are unaffected.secretsmanager:GetSecretValueon the new secret only — least privilege.Validation
terraform validateclean forterraform/modules/{secrets/aws, compute/aws/lambda, compute/aws/fargate}andterraform/environments/aws/fix/aws-scheduled-auth-mode-disabledfor PR continuity, but the title now accurately reflects the implemented fixSummary by CodeRabbit