Skip to content

Make the portal-admin secret suffix-aware, and give the deploy-cli runner its environment - #35

Open
zzkamzn wants to merge 1 commit into
aws-samples:mainfrom
zzkamzn:fix/portal-admin-secret-suffix
Open

zzkamzn wants to merge 1 commit into
aws-samples:mainfrom
zzkamzn:fix/portal-admin-secret-suffix

Conversation

@zzkamzn

@zzkamzn zzkamzn commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Summary

The schedule-runner Lambda signs in as a portal admin to delegate pipeline:{name} schedules to the backend API. portal-admin was the one platform secret whose name was not suffix-aware, and nothing set PLATFORM_PORTAL_ADMIN_SECRET, so config.py's unsuffixed default won everywhere. The credential is a user in one stack's Cognito pool, so two stacks in an account cannot share it — which is exactly what name_suffix / SUFFIX exist for (terraform/README and the CLI runbook both recommend it for coexisting deployments).

  • Terraform with a suffix: both stacks read one secret; only one stack's password matches its own pool → the other's pipeline schedules fail with NotAuthorizedException.
  • deploy-cli with a SUFFIX: the runner role was granted the suffixed name while the code read the unsuffixed one → AccessDenied on every pipeline schedule.

Changes

  • terraform/modules/portal: one local.portal_admin_secret drives the IAM grant, the Lambda env, and a new portal_admin_secret_name output so the operator does not type the name.
  • deploy-cli had a larger defect, found while auditing it for the same fix: the runner Lambda was created with no environment at all — so on a suffixed deployment it ran against the unsuffixed table (present, healthy, wrong data) — and PLATFORM_PORTAL_API_URL was set on the backend instead of the runner, inverting the delegation path (schedule_service._run_pipeline). Phase 6 now fills the runner env; the variable is off the backend.
  • deploy-cli/tests/verify.sh: three new L1 assertions (runner table matches the deployment, admin-secret name is suffix-aware, runner points at the portal API).
  • Docs: what the secret is for, that its name carries the suffix, how to create it from the output.

Legacy CDK has no suffix concept, so its unsuffixed name stays correct and is untouched.

Not verified live

No AWS credentials or Terraform binary in the authoring environment. bash -n passes and the HCL is terraform fmt-shaped, but terraform validate / plan and a real suffixed apply have not been run. Please terraform plan before merging.

Scanner impact

Static check against the README suppression list: no new findings expected. The one gitleaks-shaped line (PLATFORM_PORTAL_ADMIN_SECRET = local.portal_admin_secret) has the same shape as the already-clean PLATFORM_SERVICE_ENTRY_SECRET_NAME = aws_secretsmanager_secret.service_entry.name.

🤖 Generated with Claude Code

…vironment

The schedule-runner Lambda signs in as a portal admin to delegate
`pipeline:{name}` schedules to the backend API (workflow scripts need Node,
which the Lambda has not got). It reads the credential from Secrets Manager,
and `portal-admin` was the one platform secret whose name was not suffix-aware:
`llm-gateway-key`, `service-entry`, `mcp-hub` and the team-auth secrets all
carry `name_suffix`, this one did not, and nothing set
`PLATFORM_PORTAL_ADMIN_SECRET` either, so `config.py`'s unsuffixed default won
everywhere.

That is wrong because the credential is a user in *one* stack's Cognito pool.
Two stacks in an account cannot share it — which is exactly what `name_suffix`
and `SUFFIX` exist to support, and what terraform/README and the CLI runbook
both recommend for coexisting deployments. The symptom differed per path:

* Terraform with a suffix: no AccessDenied, because grant and default agreed —
  but both stacks read one secret, and only one stack's password matches its
  own pool, so the other's pipeline schedules fail on NotAuthorizedException.
* deploy-cli with a SUFFIX: the runner role was granted
  `agent-platform${SUFFIX}/portal-admin*` while the code asked for the
  unsuffixed name, so every pipeline schedule died on AccessDenied.

So: one `local.portal_admin_secret` in the portal module now names the secret,
the IAM grant and the Lambda's `PLATFORM_PORTAL_ADMIN_SECRET` from a single
place, and `terraform output -raw portal_admin_secret_name` tells the operator
what to create. The legacy CDK stacks have no suffix concept at all, so their
unsuffixed name stays correct and is left alone.

Auditing the CLI port for the same fix turned up a larger defect in it: the
schedule-runner Lambda was created with **no environment at all**, and
`PLATFORM_PORTAL_API_URL` was set on the backend instead. Both are inverted
relative to Terraform, and both matter:

* An empty environment leaves the runner on `config.py` defaults, `dynamo_table
  = "agent-platform"` above all — so in a suffixed deployment every scheduled
  run reads and writes the wrong table. Present, correct-looking, wrong data.
* `PLATFORM_PORTAL_API_URL` is what selects the delegation path in
  `schedule_service._run_pipeline`. On the backend it made the backend delegate
  pipelines to itself and then fail, since only the runner role carries the
  portal-admin grant; on the runner, whose only path this is, it was missing.

Phase 6 now fills the runner's environment (it cannot happen in phase 5 —
the CloudFront domain does not exist yet) and the variable is off the backend.
`tests/verify.sh` gained three L1 assertions for it: the runner's table matches
the deployment, its admin-secret name is suffix-aware, and it points at the
portal API. That is the shape of check the suite exists for — every resource was
present and healthy while this was broken.

Docs: deployment.md and the CLI runbook now say what the secret is for, that
its name carries the suffix, and how to create it from the new output;
permissions.md's `PortalAdminSecret` row names the suffixed resource and notes
that the backend task role deliberately has no such grant.

Not verified against a live deployment: no AWS credentials or Terraform binary
in this environment. Shell syntax is checked (`bash -n`) and the Terraform
edits are formatted to be `terraform fmt`-stable, but `terraform validate`,
`plan` and an actual suffixed apply have not been run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant