fix(cms): bootstrap internal api token and stage/prod routing#302
fix(cms): bootstrap internal api token and stage/prod routing#302
Conversation
Ensure Strapi creates or rotates a managed read-only internal API token from env at startup. Wire SSM-backed stage/prod token routing so only Vercel production uses prod while preview and GitHub builds stay on stage. Resolves #299. Made-with: Cursor
|
Caution Review failedPull request was closed or merged during review Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughBootstraps and enforces a read-only internal Strapi API token from STRAPI_INTERNAL_API_TOKEN at CMS startup; adds SSM parameter and ECS secret wiring in AWS, and routes stage/prod tokens to Vercel and GitHub via new Terraform data/resources. Changes
Sequence DiagramsequenceDiagram
participant App as CMS App (startup)
participant Env as Env (STRAPI_INTERNAL_API_TOKEN)
participant Bootstrap as ensureInternalApiToken
participant Strapi as Strapi admin api-token service
participant DB as Strapi DB
App->>Env: read STRAPI_INTERNAL_API_TOKEN
App->>Bootstrap: invoke with token
Bootstrap->>Strapi: resolve api-token service
alt service unavailable
Strapi-->>Bootstrap: null -> log & exit
else service available
Bootstrap->>DB: acquire advisory lock
Bootstrap->>Strapi: query token by name
Strapi->>DB: read tokens
DB-->>Strapi: token record?
Strapi-->>Bootstrap: existing token or null
alt no existing token
Bootstrap->>Strapi: create read-only token (name=internal)
Strapi->>DB: insert token
DB-->>Bootstrap: created
else existing token present
Bootstrap->>Strapi: compare stored vs env value (hash/check)
alt match & read-only
Bootstrap-->>App: done (release lock)
else mismatch or wrong type
Bootstrap->>Strapi: create pending token
Strapi->>DB: insert pending
Bootstrap->>Strapi: delete existing token
Strapi->>DB: remove existing
Bootstrap->>Strapi: promote pending -> internal name
Strapi->>DB: update token record
Bootstrap-->>App: rotated token (release lock)
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
❌ Terraform plan (vercel) — Plan failedChanges: n/a
Run: https://github.com/JesusFilm/forge/actions/runs/22925676470 |
❌ Terraform plan (github/prod) — Plan failedChanges: n/a
Run: https://github.com/JesusFilm/forge/actions/runs/22925676470 |
🟨 Terraform plan (aws/stage) — Changes detectedChanges: +17 ~4 -1
Run: https://github.com/JesusFilm/forge/actions/runs/22925676470 |
🟨 Terraform plan (aws/prod) — Changes detectedChanges: +15 ~6 -1
Run: https://github.com/JesusFilm/forge/actions/runs/22925676470 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/cms/src/index.ts (2)
84-95: Add logging when rotating an existing token for auditability.The rotation case (token exists but doesn't match or isn't read-only) silently deletes and recreates the token. Adding a log here would improve operational visibility and align with the guideline to keep transitions auditable.
📝 Suggested logging addition
const isReadOnly = existingToken.type === "read-only" if (matches && isReadOnly) return + strapi.log.info( + `Rotating internal API token: match=${matches}, readOnly=${isReadOnly}` + ) await strapi.db.query("admin::api-token").delete({ where: { id: existingToken.id }, })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cms/src/index.ts` around lines 84 - 95, Add an info log when an existing API token is being rotated: inside the branch after determining matches and isReadOnly (just before deleting and recreating), emit a concise audit log via the application's logger (e.g., strapi.log.info) mentioning the existingToken.id, existingToken.type and the accessKey (or masked form) to record the rotation action, then proceed to call strapi.db.query("admin::api-token").delete(...) and createReadOnlyToken(strapi, apiTokenService, accessKey).
7-17: Consider adding thegetBymethod to the type definition.The code queries tokens via
strapi.db.query()at line 74 rather than using a potential service method. If theapi-tokenservice has agetByorfindOnemethod, using it would be more consistent with the service abstraction pattern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cms/src/index.ts` around lines 7 - 17, The ApiTokenService type is missing a method used elsewhere; add a getBy (or findOne) method signature to the ApiTokenService type so callers can use the service instead of strapi.db.query; e.g., extend the type with getBy: (params: { where?: Record<string, any>; populate?: string[] }) => Promise<unknown> (or a more specific token shape), or add findOne: (where: Record<string, any>) => Promise<unknown>, keeping the same return type conventions as create/check/hash so callers referencing ApiTokenService (and code that uses strapi.db.query) can switch to service.getBy/getOne.infra/aws/vercel/ssm.tf (1)
46-70: Consider adding tags for resource management consistency.The KMS key includes tags (lines 17-19) but the SSM parameters don't. Adding
tags = local.vercel_ssm_tagsto SSM parameters would improve resource discoverability and cost tracking. This matches the existingapi_tokenresource pattern, so it's optional.📋 Suggested tags addition
resource "aws_ssm_parameter" "strapi_api_token_stage" { count = local.create_vercel_ssm_resources ? 1 : 0 name = "/forge/vercel/strapi_api_token_stage" type = "SecureString" key_id = aws_kms_key.vercel_ssm[0].arn value = "manually set in AWS console" + tags = local.vercel_ssm_tags lifecycle { ignore_changes = [value] } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infra/aws/vercel/ssm.tf` around lines 46 - 70, Add consistent tagging to the two SSM parameter resources by adding tags = local.vercel_ssm_tags to aws_ssm_parameter.strapi_api_token_stage and aws_ssm_parameter.strapi_api_token_prod so they match the KMS key and the existing api_token resource pattern; place the tags attribute alongside the other top-level attributes (name/type/key_id/value) in each resource block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/cms/src/index.ts`:
- Around line 92-95: The current non-atomic rotation deletes the existing
managed token via strapi.db.query("admin::api-token").delete(...) before calling
createReadOnlyToken(strapi, apiTokenService, accessKey), which risks losing all
tokens if creation fails; change the flow to create-and-verify the new token
first (e.g., call createReadOnlyToken with a temporary name/marker and confirm
it exists), only then delete the old token, or wrap createReadOnlyToken in a
try/catch that on failure logs prominently and restores the old token if
deleted; reference the delete call and createReadOnlyToken when implementing the
create-then-delete or rollback logic to ensure token rotation is
atomic/defensive.
In `@infra/github/data.tf`:
- Around line 55-59: The Terraform data source data "aws_ssm_parameter"
"strapi_api_token_stage" fails during plan because the SSM parameter it reads is
created conditionally by the SSM resource in the github module (created only
when environment == "prod"); either make the lookup conditional (wrap data
"aws_ssm_parameter" "strapi_api_token_stage" with the same environment == "prod"
guard or use count/for_each to only reference it in prod) or ensure the
infra/aws/github SSM resource is applied before running terraform plan for
infra/github (and document this ordering in the README or CI workflow); update
the CI workflow to apply the github SSM module first or add documentation
explaining the dependency.
---
Nitpick comments:
In `@apps/cms/src/index.ts`:
- Around line 84-95: Add an info log when an existing API token is being
rotated: inside the branch after determining matches and isReadOnly (just before
deleting and recreating), emit a concise audit log via the application's logger
(e.g., strapi.log.info) mentioning the existingToken.id, existingToken.type and
the accessKey (or masked form) to record the rotation action, then proceed to
call strapi.db.query("admin::api-token").delete(...) and
createReadOnlyToken(strapi, apiTokenService, accessKey).
- Around line 7-17: The ApiTokenService type is missing a method used elsewhere;
add a getBy (or findOne) method signature to the ApiTokenService type so callers
can use the service instead of strapi.db.query; e.g., extend the type with
getBy: (params: { where?: Record<string, any>; populate?: string[] }) =>
Promise<unknown> (or a more specific token shape), or add findOne: (where:
Record<string, any>) => Promise<unknown>, keeping the same return type
conventions as create/check/hash so callers referencing ApiTokenService (and
code that uses strapi.db.query) can switch to service.getBy/getOne.
In `@infra/aws/vercel/ssm.tf`:
- Around line 46-70: Add consistent tagging to the two SSM parameter resources
by adding tags = local.vercel_ssm_tags to
aws_ssm_parameter.strapi_api_token_stage and
aws_ssm_parameter.strapi_api_token_prod so they match the KMS key and the
existing api_token resource pattern; place the tags attribute alongside the
other top-level attributes (name/type/key_id/value) in each resource block.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ddbab034-3229-40af-b0aa-da2273f25780
📒 Files selected for processing (11)
apps/cms/.env.exampleapps/cms/src/index.tsinfra/aws/github/ssm.tfinfra/aws/modules/cms/main.tfinfra/aws/vercel/ssm.tfinfra/github/README.mdinfra/github/actions.tfinfra/github/data.tfinfra/vercel/README.mdinfra/vercel/data.tfinfra/vercel/main.tf
Generate STRAPI internal tokens in the CMS module as random SecureStrings controlled by ssm_secret_version for rotation. Point Vercel and GitHub token reads to those CMS parameters and remove duplicate token params from vercel/github namespaces. Made-with: Cursor
Move internal API token logic behind a single bootstrap call and implement create-verify-promote rotation to avoid delete-first risk. This keeps startup entrypoint small while preserving audit logs. Made-with: Cursor
Allow github plan/apply stack roles to read and decrypt both stage and prod CMS token parameters, and document cms-parameter dependency for github/vercel plans. Made-with: Cursor
Review feedback addressed (c25dbe0)Handled:
Declined:
Questions / blocked:
|
Guard section keys using an id-property check and relax exhaustive section rendering fallback so generated union additions do not break Next.js production build typechecking. Made-with: Cursor
Review feedback addressed (bbcbefa)Handled:
Declined:
Questions / blocked:
|
Use `Core.Strapi` for the bootstrap argument type to keep the entrypoint signature aligned with the Strapi app contract. Made-with: Cursor
Review feedback addressed (5fbf06c)Handled:
Declined:
Questions / blocked:
|
Document the Strapi internal typing rule for future agents and add an inline note at the local cast site in token bootstrap logic. Made-with: Cursor
…al-api-token-routing
Review feedback addressed (b09c49b)Fixed:
Not changed:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/cms/src/internal-api-token.ts (1)
19-26: Avoid readingDATABASE_CLIENTdirectly inapps/cmsruntime code.This hard-codes env-specific branching into app logic and duplicates the database-client resolution that already lives in Strapi config. Prefer deriving capability/client information from the resolved Strapi DB object instead of
process.envhere.As per coding guidelines "Do not embed env-specific branching in app logic."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cms/src/internal-api-token.ts` around lines 19 - 26, Don't read process.env.DATABASE_CLIENT in apps/cms; instead derive DB type/capabilities from the resolved Strapi DB object. Remove the isPostgres check and rely on the inspected strapi.db object (use dbWithRaw, dbWithRaw.connection?.raw and/or the DB client's config/dialect on strapi.db or strapi.db.connection) to decide behavior—i.e., check for the presence of a raw function or the client's dialect/config field rather than using process.env.DATABASE_CLIENT. Update the conditional around raw and any code paths that depended on isPostgres to use that derived capability from strapi.db.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/cms/src/internal-api-token.ts`:
- Around line 15-36: The advisory lock calls in withTokenBootstrapLock use
raw(...) twice which may pull different pooled connections so
pg_advisory_lock/pg_advisory_unlock can run on different sessions; modify
withTokenBootstrapLock to acquire a single connection/session for the lifetime
of the lock (either by wrapping the lock, run(), and unlock calls inside a
single Knex transaction or by manually acquiring a connection from the Knex pool
and releasing it after unlock) so both SELECT pg_advisory_lock(?) and SELECT
pg_advisory_unlock(?) run on the same session; ensure you still call the
provided run() callback inside that transaction/connection and release/commit in
a finally block, referencing TOKEN_BOOTSTRAP_LOCK_ID and the existing raw
function usage in withTokenBootstrapLock.
---
Nitpick comments:
In `@apps/cms/src/internal-api-token.ts`:
- Around line 19-26: Don't read process.env.DATABASE_CLIENT in apps/cms; instead
derive DB type/capabilities from the resolved Strapi DB object. Remove the
isPostgres check and rely on the inspected strapi.db object (use dbWithRaw,
dbWithRaw.connection?.raw and/or the DB client's config/dialect on strapi.db or
strapi.db.connection) to decide behavior—i.e., check for the presence of a raw
function or the client's dialect/config field rather than using
process.env.DATABASE_CLIENT. Update the conditional around raw and any code
paths that depended on isPostgres to use that derived capability from strapi.db.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3d7f5669-aad4-47b5-9f9c-9685dbf534da
📒 Files selected for processing (2)
apps/cms/AGENTS.mdapps/cms/src/internal-api-token.ts
✅ Files skipped from review due to trivial changes (1)
- apps/cms/AGENTS.md
Run advisory lock and unlock on the same acquired Postgres session so concurrent CMS startups cannot bypass the token bootstrap mutex. Made-with: Cursor
Review feedback addressed (aadbb82)Fixed:
Not changed:
|
Summary
STRAPI_INTERNAL_API_TOKEN, rotating when value/type changesContracts Changed
Regeneration Required
Validation
Resolves #299
Summary by CodeRabbit
New Features
Chores
Documentation