Skip to content

feat(infra): add dev SSM sync and dev credential users#303

Open
tataihono wants to merge 12 commits intomainfrom
feat/300-dev-ssm-sync-credentials
Open

feat(infra): add dev SSM sync and dev credential users#303
tataihono wants to merge 12 commits intomainfrom
feat/300-dev-ssm-sync-credentials

Conversation

@tataihono
Copy link
Contributor

@tataihono tataihono commented Mar 9, 2026

Summary

  • add a dedicated forge-dev-credentials IAM group with MFA enforcement, SSM read, KMS decrypt, and self access-key management
  • add one list-driven Terraform user file to create <github>-dev-credentials users without touching existing IAM users
  • add pnpm sync:ssm:dev to pull String + SecureString parameters into .env.development.local, and document usage

Resolves #300

Contracts Changed

  • yes
  • no

Regeneration Required

  • yes
  • no

Validation

  • Contracts validated
  • Generated code verified (no manual edits)
  • Tests and build passed
  • Terraform plan reviewed (if infra change)

Notes:

  • terraform -chdir=infra/aws init -backend=false && terraform -chdir=infra/aws validate blocked locally by invalid AWS token.
  • node scripts/sync-ssm-dev.mjs blocked locally by invalid AWS token.
  • terraform -chdir=infra/aws/iam fmt -recursive passed.

Made with Cursor

Summary by CodeRabbit

  • New Features

    • Developers can fetch development secrets locally via a new workspace task and npm scripts (e.g., pnpm fetch-secrets). Fetched secrets write to .env.development.local for CMS or Web projects.
    • CMS and Web development SSM parameters can be optionally provisioned for dev environments.
  • Security / Access

    • Introduced a scoped dev-secrets IAM group with MFA enforcement and self-service account support for contributors.
  • Documentation

    • Added a dev secrets onboarding guide with step-by-step setup and sync instructions.

Add a dedicated dev credentials group and list-driven contributor users.
Keep all existing IAM users unchanged.

Made-with: Cursor
Add a sync script and package command to pull dev SSM parameters,
including SecureString decryption, into .env.development.local.

Made-with: Cursor
@coderabbitai
Copy link

coderabbitai bot commented Mar 9, 2026

Walkthrough

Adds Terraform-managed dev IAM users and a least-privilege "forge-dev-secrets" group, conditional dev SSM parameters for CMS and Web, a TypeScript script to sync SSM into .env.development.local, and npm/Turbo scripts to run the fetch workflow locally or via Turbo.

Changes

Cohort / File(s) Summary
Docs
infra/aws/README.md
Adds "Dev secrets + SSM sync" subsection documenting SSM paths, contributor refresh command, onboarding flow, and fetch-secrets usage.
IAM Groups
infra/aws/iam/groups/main.tf, infra/aws/iam/groups/dev_secrets/*
Adds module "dev_secrets" and new forge-dev-secrets group, IAM policy document granting scoped SSM & KMS decrypt permissions, and MFA policy attachment.
IAM Users
infra/aws/iam/users/main.tf, infra/aws/iam/users/dev_secrets/*
Adds list-driven Terraform user provisioning creating <github-handle>-dev-secrets users via for_each, group membership resources, variables, and an onboarding README.
SSM Parameters — CMS
infra/aws/modules/cms/main.tf
Adds conditional dev SSM support: dev KMS alias, multiple random_passwords, and dev SSM parameters created only when the dev flag is true.
SSM Parameters — Web
infra/aws/modules/platform/web_dev_ssm.tf
Adds conditional web dev SSM parameters, random token generation, and KMS alias, gated by environment flag and using count-based creation.
Secrets Sync Script
scripts/fetch-secrets.ts
New TypeScript script fetching parameters by path (cms
NPM / Turbo Integration
package.json, apps/cms/package.json, apps/web/package.json, turbo.json
Adds root and per-app fetch-secrets scripts (using tsx), adds tsx devDependency, and a Turbo task fetch-secrets (cache:false, outputs .env.development.local).

Sequence Diagram

sequenceDiagram
    actor Dev as Developer
    participant CLI as pnpm / turbo
    participant Script as fetch-secrets.ts
    participant AWS as AWS SSM & KMS
    participant FS as Filesystem

    Dev->>CLI: pnpm fetch-secrets (or app-specific)
    CLI->>Script: run tsx scripts/fetch-secrets.ts --project <cms|web>
    Script->>AWS: GetParametersByPath(/forge/aws/<project>/dev/*) (with Decryption, paginate)
    AWS-->>Script: Parameters (may include SecureString)
    Script->>AWS: KMS decrypt (handled via AWS SDK/SSM)
    AWS-->>Script: Decrypted values
    Script->>Script: Normalize names, detect duplicates, sort
    Script->>FS: Write .env.development.local
    FS-->>Script: File created/updated
    Script-->>CLI: exit 0 / exit 1 (error)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: adding dev SSM sync functionality and dev credential users as IAM resources.
Linked Issues check ✅ Passed The PR implements all acceptance criteria from #300: preserves existing IAM users, creates a dev-credentials IAM group with SSM/KMS permissions, provides list-driven Terraform user file, includes fetch-secrets sync command, and includes documentation.
Out of Scope Changes check ✅ Passed All changes align with #300 scope: IAM group/users, SSM/KMS resources for dev, fetch-secrets script and tooling, and documentation are all directly related to the stated objectives.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/300-dev-ssm-sync-credentials

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

github-actions bot commented Mar 9, 2026

❌ Terraform plan (aws/prod) — Pre-plan step failed (format/validate/init)

Changes: n/a
Time:

TZ Time
UTC 2026-03-09, 2:33:33 a.m.
NZ 2026-03-09, 3:33:33 p.m.
PT 2026-03-08, 7:33:33 p.m.
ET 2026-03-08, 10:33:33 p.m.

Run: https://github.com/JesusFilm/forge/actions/runs/22835962190

=== Format check ===
=== Init ===
�[0m�[1mInitializing the backend...�[0m
�[0m�[32m
Successfully configured the backend "s3"! Terraform will automatically
use this backend unless the backend configuration changes.�[0m
�[0m�[1mInitializing modules...�[0m
- github in github
- iam in iam
- iam.groups in iam/groups
- iam.groups.admin_readonly in iam/groups/admin_readonly
- iam.groups.billing in iam/groups/billing
- iam.groups.dev_secrets in iam/groups/dev_secrets
- iam.groups.login_profile in iam/groups/login_profile
- iam.groups.require_mfa in iam/groups/require_mfa
- iam.users in iam/users
- iam.users.tataihono in iam/users/tataihono
- platform in modules/platform
- platform.application in modules/cms
- platform.assets in modules/assets
- vercel in vercel
�[0m�[1mInitializing provider plugins...�[0m
- Reusing previous version of hashicorp/aws from the dependency lock file
- Reusing previous version of hashicorp/random from the dependency lock file
- Installing hashicorp/random v3.8.1...
- Installed hashicorp/random v3.8.1 (signed by HashiCorp)
- Installing hashicorp/aws v6.34.0...
- Installed hashicorp/aws v6.34.0 (signed by HashiCorp)
Terraform has made some changes to the provider dependency selections recorded
in the .terraform.lock.hcl file. Review those changes and commit them to your
version control system if they represent changes you intended to make.

�[0m�[1m�[32mTerraform has been successfully initialized!�[0m�[32m�[0m
�[0m�[32m
You may now begin working with Terraform. Try running "terraform plan" to see
any changes that are required for your infrastructure. All Terraform commands
should now work.

If you ever set or change modules or backend configuration for Terraform,
rerun this command to reinitialize your working directory. If you forget, other
commands will detect it and remind you to do so if necessary.�[0m
=== Validate ===
�[32m�[1mSuccess!�[0m The configuration is valid.
�[0m

@github-actions
Copy link

github-actions bot commented Mar 9, 2026

❌ Terraform plan (aws/stage) — Pre-plan step failed (format/validate/init)

Changes: n/a
Time:

TZ Time
UTC 2026-03-09, 2:33:32 a.m.
NZ 2026-03-09, 3:33:32 p.m.
PT 2026-03-08, 7:33:32 p.m.
ET 2026-03-08, 10:33:32 p.m.

Run: https://github.com/JesusFilm/forge/actions/runs/22835962190

=== Format check ===
=== Init ===
�[0m�[1mInitializing the backend...�[0m
�[0m�[32m
Successfully configured the backend "s3"! Terraform will automatically
use this backend unless the backend configuration changes.�[0m
�[0m�[1mInitializing modules...�[0m
- github in github
- iam in iam
- iam.groups in iam/groups
- iam.groups.admin_readonly in iam/groups/admin_readonly
- iam.groups.billing in iam/groups/billing
- iam.groups.dev_secrets in iam/groups/dev_secrets
- iam.groups.login_profile in iam/groups/login_profile
- iam.groups.require_mfa in iam/groups/require_mfa
- iam.users in iam/users
- iam.users.tataihono in iam/users/tataihono
- platform in modules/platform
- platform.application in modules/cms
- platform.assets in modules/assets
- vercel in vercel
�[0m�[1mInitializing provider plugins...�[0m
- Reusing previous version of hashicorp/aws from the dependency lock file
- Reusing previous version of hashicorp/random from the dependency lock file
- Installing hashicorp/aws v6.34.0...
- Installed hashicorp/aws v6.34.0 (signed by HashiCorp)
- Installing hashicorp/random v3.8.1...
- Installed hashicorp/random v3.8.1 (signed by HashiCorp)
Terraform has made some changes to the provider dependency selections recorded
in the .terraform.lock.hcl file. Review those changes and commit them to your
version control system if they represent changes you intended to make.

�[0m�[1m�[32mTerraform has been successfully initialized!�[0m�[32m�[0m
�[0m�[32m
You may now begin working with Terraform. Try running "terraform plan" to see
any changes that are required for your infrastructure. All Terraform commands
should now work.

If you ever set or change modules or backend configuration for Terraform,
rerun this command to reinitialize your working directory. If you forget, other
commands will detect it and remind you to do so if necessary.�[0m

Use a shared sync script with project-specific execution for cms and web,
writing each app's own .env.development.local file.

Made-with: Cursor
Add per-app fetch-secrets scripts for cms and web, and wire a Turbo task
so one command fetches secrets for both app projects.

Made-with: Cursor
Rename the shared dev secret script and update package/app/docs commands
to use fetch-secrets naming consistently.

Made-with: Cursor
Migrate the shared fetch-secrets script to TypeScript and run it with tsx
from app-level Turbo tasks.

Made-with: Cursor
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@infra/aws/iam/users/dev_credentials_users.tf`:
- Around line 1-3: The refresh hint comment currently recommending "git shortlog
-sne --since=\"12 months ago\" --all" is incorrect because it yields author
names/emails and doesn't exclude bots; replace that single-line hint with either
(a) a short documented bash one-liner or script that enumerates the exact GitHub
handles used in the IAM usernames (including steps to map author emails/names to
GitHub handles and to filter known bot accounts), or (b) a generated-list
command that directly prints the exact "<github>-dev-credentials" handles
expected; update the comment to include the script name or the exact command and
a brief note about filtering/mapping rules so future refreshes produce valid IAM
usernames.

In `@infra/aws/README.md`:
- Around line 22-28: The README shows SSM paths users can sync that the
dev-credentials IAM group does not permit; update the examples so they only
reference the permitted path (/forge/aws/cms/stage/) or add an explicit note
that syncing web/prod paths requires a different AWS principal, and point to the
IAM policy in infra/aws/iam/groups/dev_credentials/main.tf (which currently
grants SSM/KMS access only via alias/forge-cms-stage-ssm) so readers know why
they’ll get access-denied unless using elevated credentials.

In `@scripts/sync-ssm-dev.mjs`:
- Around line 75-77: The toEnvKey function currently collapses SSM paths to
their final segment (e.g., DATABASE_URL) which allows silent collisions when
merging multiple paths; update the logic used where keys are merged (refer to
toEnvKey and the merge at the code that assigns into the env map around the
later-value-wins block) to detect duplicate env keys and fail fast: when
computing key = toEnvKey(parameterName), check if that key already exists and if
so throw an error that includes both source parameter names; alternatively
provide a namespacing option (e.g., include parent path segment) behind a
flag—implement the collision check and error throw in the merge code so
duplicates are rejected unless an explicit namespace option is enabled.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 883910a1-ebca-4459-8c7b-8c097ce059e7

📥 Commits

Reviewing files that changed from the base of the PR and between 604f01d and 60ecceb.

📒 Files selected for processing (7)
  • infra/aws/README.md
  • infra/aws/iam/groups/dev_credentials/main.tf
  • infra/aws/iam/groups/dev_credentials/variables.tf
  • infra/aws/iam/groups/main.tf
  • infra/aws/iam/users/dev_credentials_users.tf
  • package.json
  • scripts/sync-ssm-dev.mjs

Rename the IAM group, user naming, and Terraform module/file paths from
"dev credentials" to "dev secrets" for consistency.

Made-with: Cursor
Switch dev-secrets access and fetch defaults to /dev paths, simplify docs and
root command usage, and harden fetch-secrets key collision handling.

Made-with: Cursor
@tataihono
Copy link
Contributor Author

Review feedback addressed (eb9e6f1)

Handled:

  • [no filter.]: root command now uses turbo run fetch-secrets.
  • [cms path suggestion]: IAM SSM scope updated to /forge/aws/cms/dev/*.
  • [kms alias suggestion]: alias updated to alias/forge-cms-dev-ssm.
  • [cms/web defaultPaths suggestions]: script defaults now use /forge/aws/cms/dev/ and /forge/aws/web/dev/.
  • [can cwd provide this?]: output now writes to cwd .env.development.local.
  • [README command suggestions/drop]: docs simplified to pnpm fetch-secrets, pnpm fetch-secrets:web, and turbo run fetch-secrets; optional custom path section removed.
  • [no sync paths variable]: removed SSM_SYNC_PATHS* overrides; paths are fixed by project.
  • [CodeRabbit key collision]: added duplicate env-key detection with fail-fast error.
  • [CodeRabbit refresh hint]: replaced contributor refresh hint with a GitHub-handle command.

Declined:

  • None.

Questions / blocked:

  • None.

Create CMS and web dev SSM parameters, move dev-secrets users into a
submodule, and add developer-first IAM onboarding documentation.

Made-with: Cursor
Remove extra root helper scripts and redundant source-path header output
to keep fetch-secrets usage minimal and consistent.

Made-with: Cursor
@tataihono
Copy link
Contributor Author

Review feedback addressed (00548cb)

Handled:

  • [drop this. since its just /forge/aws/(key)/dev/]: removed source-path header line from generated env output.
  • [drop]: removed root fetch-secrets:cms and fetch-secrets:web scripts.

Declined:

  • None.

Questions / blocked:

  • None.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@infra/aws/iam/users/dev_secrets/main.tf`:
- Around line 1-3: The header comment currently uses the contributors API
command "gh api repos/JesusFilm/forge/contributors --paginate --jq '.[].login' |
sort -u" which returns all-time contributors and can include bots; replace that
line with a command that instead queries recent commits (using the commits API
with a --since filter for the last 12 months), extracts author logins, and
filters out bot accounts (e.g., exclude authors with type 'Bot' or logins that
end with "bot") so the regenerated list only contains recent human contributors;
update the header comment text to describe the new regeneration command and its
intent.
- Around line 14-16: Remove the data source lookup
data.aws_iam_group.dev_secrets and instead parameterize the module with a string
variable: add variable "group_name" { type = string } to the users module
(infra/aws/iam/users/dev_secrets/main.tf), update the membership resource to use
var.group_name (replace any data.aws_iam_group.dev_secrets.group_name usage,
e.g., in the aws_iam_group_membership or related resource), and update the
module invocation in infra/aws/iam/users/main.tf to pass group_name =
"forge-dev-secrets"; finally delete the data "aws_iam_group" "dev_secrets" block
so the plan no longer depends on an existing group.

In `@infra/aws/modules/cms/main.tf`:
- Around line 158-239: The dev SSM parameters (aws_ssm_parameter.dev_app_keys,
dev_admin_jwt_secret, dev_jwt_secret, dev_api_token_salt,
dev_transfer_token_salt, dev_encryption_key) are pointed at the existing
ephemeral.random_password.* results for stage; create a dedicated set of
ephemeral.random_password resources for dev (e.g.,
ephemeral.random_password.dev_app_key_1..4,
ephemeral.random_password.dev_admin_jwt_secret,
ephemeral.random_password.dev_jwt_secret,
ephemeral.random_password.dev_api_token_salt,
ephemeral.random_password.dev_transfer_token_salt,
ephemeral.random_password.dev_encryption_key) and update each
aws_ssm_parameter.value_wo (and the join(...) for dev_app_keys) to use the
corresponding ephemeral.random_password.dev_*.result so the /dev namespace mints
independent secrets. Ensure names and dependencies reference these new
dev-specific random_password resources.

In `@infra/aws/modules/platform/web_dev_ssm.tf`:
- Around line 24-39: The SSM parameter
aws_ssm_parameter.web_dev_strapi_api_token is currently set to a literal
placeholder and has lifecycle.ignore_changes on value, which causes the real
secret to be managed in the console (drift); replace the hardcoded value with a
Terraform-managed sensitive input (e.g., value = var.web_dev_strapi_api_token),
declare that variable as sensitive in variables.tf, and remove value from
lifecycle.ignore_changes so Terraform manages and records the true secret state;
apply the same change to the other aws_ssm_parameter resources in this file that
currently use the placeholder + ignore_changes pattern.

In `@scripts/fetch-secrets.ts`:
- Around line 87-113: fetchAllByPath sometimes returns an empty parameters
array; detect this immediately after const parameters =
fetchAllByPath(sourcePath) and throw an Error (including sourcePath and project)
if parameters.length === 0 so the script fails fast instead of writing an empty
outputFile and printing the success message; place the guard before iterating
the parameters (before the for loop) so writeFileSync and the subsequent
process.stdout.write call never run when no secrets were returned.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4fdc2695-26a3-46db-842c-a8516b3c9254

📥 Commits

Reviewing files that changed from the base of the PR and between 60ecceb and 3d7cf2b.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (15)
  • apps/cms/package.json
  • apps/web/package.json
  • infra/aws/README.md
  • infra/aws/iam/groups/dev_secrets/main.tf
  • infra/aws/iam/groups/dev_secrets/variables.tf
  • infra/aws/iam/groups/main.tf
  • infra/aws/iam/users/dev_secrets/README.md
  • infra/aws/iam/users/dev_secrets/main.tf
  • infra/aws/iam/users/dev_secrets/variables.tf
  • infra/aws/iam/users/main.tf
  • infra/aws/modules/cms/main.tf
  • infra/aws/modules/platform/web_dev_ssm.tf
  • package.json
  • scripts/fetch-secrets.ts
  • turbo.json
✅ Files skipped from review due to trivial changes (2)
  • infra/aws/iam/users/dev_secrets/README.md
  • infra/aws/iam/groups/dev_secrets/variables.tf

Comment on lines +14 to +16
data "aws_iam_group" "dev_secrets" {
group_name = "forge-dev-secrets"
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== users module lookup =="
rg -n -C2 'data "aws_iam_group" "dev_secrets"|group_name = "forge-dev-secrets"' infra/aws/iam/users/dev_secrets

echo
echo "== group resource definition =="
rg -n -C2 'resource "aws_iam_group" "dev_secrets"|name = "forge-dev-secrets"' infra/aws/iam/groups/dev_secrets

echo
echo "== module wiring / outputs =="
rg -n -C3 'module ".*dev.*secrets.*"|output ".*dev.*secrets.*"|depends_on' infra/aws/iam

Repository: JesusFilm/forge

Length of output: 1677


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== infra/aws/iam/users/main.tf (full) =="
cat -n infra/aws/iam/users/main.tf

echo
echo "== infra/aws/iam/users/dev_secrets/main.tf (full) =="
cat -n infra/aws/iam/users/dev_secrets/main.tf

echo
echo "== Check for outputs in groups module =="
if [ -f infra/aws/iam/groups/dev_secrets/outputs.tf ]; then
  echo "=== infra/aws/iam/groups/dev_secrets/outputs.tf ==="
  cat -n infra/aws/iam/groups/dev_secrets/outputs.tf
else
  echo "No outputs.tf found"
fi

echo
echo "== infra/aws/iam/groups/main.tf (full) =="
cat -n infra/aws/iam/groups/main.tf

Repository: JesusFilm/forge

Length of output: 2454


Remove the data source lookup and use the group reference directly.

The dev_secrets module reads forge-dev-secrets via data "aws_iam_group" (lines 14–16), but the same stack creates that group in infra/aws/iam/groups/dev_secrets/main.tf. On a fresh account, the plan fails before Terraform gets a chance to create the group. The depends_on = [module.groups] at the parent level ensures creation order but does not prevent the data lookup from failing during the plan phase.

Since the group name is already hardcoded as "forge-dev-secrets", pass it as a module input variable instead of reading it via data source:

  • Add variable "group_name" { type = string } to infra/aws/iam/users/dev_secrets/main.tf
  • Update the users module call in infra/aws/iam/users/main.tf to pass group_name = "forge-dev-secrets"
  • Use var.group_name in the membership resource (line 35) instead of data.aws_iam_group.dev_secrets.group_name
  • Remove the data source block entirely

This removes the circular dependency and aligns with the guideline: "Keep Terraform modules explicit and environment inputs parameterized."

Also applies to lines 29–36, where the membership resource depends on the same data source.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@infra/aws/iam/users/dev_secrets/main.tf` around lines 14 - 16, Remove the
data source lookup data.aws_iam_group.dev_secrets and instead parameterize the
module with a string variable: add variable "group_name" { type = string } to
the users module (infra/aws/iam/users/dev_secrets/main.tf), update the
membership resource to use var.group_name (replace any
data.aws_iam_group.dev_secrets.group_name usage, e.g., in the
aws_iam_group_membership or related resource), and update the module invocation
in infra/aws/iam/users/main.tf to pass group_name = "forge-dev-secrets"; finally
delete the data "aws_iam_group" "dev_secrets" block so the plan no longer
depends on an existing group.

Replace manual console value for /forge/aws/web/dev/STRAPI_API_TOKEN with
Terraform-generated random password output.

Made-with: Cursor
Address latest CodeRabbit comments by removing IAM group data lookup,
separating dev secret generation from stage, and managing web dev
tokens in Terraform. Add fail-fast handling for empty SSM paths and
update contributor refresh guidance to 12-month human-only commits.

Made-with: Cursor
@tataihono
Copy link
Contributor Author

Review feedback addressed (cc45d42)

Fixed:

  • Removed data "aws_iam_group" lookup from infra/aws/iam/users/dev_secrets/main.tf, added group_name module input, and wired group_name = "forge-dev-secrets" from infra/aws/iam/users/main.tf to avoid plan-time dependency on pre-existing group.
  • Added dedicated ephemeral.random_password.dev_* resources in infra/aws/modules/cms/main.tf and switched all /forge/aws/cms/dev/* SSM values to those dev-specific secrets.
  • Replaced manual placeholder + ignore_changes tokens in infra/aws/modules/platform/web_dev_ssm.tf by Terraform-managed random_password values for STRAPI_REVALIDATE_TOKEN and STRAPI_PREVIEW_TOKEN (API token was already Terraform-managed).
  • Added fail-fast guard in scripts/fetch-secrets.ts when fetchAllByPath() returns zero parameters, including path + project in the error message.
  • Updated contributor refresh hint in infra/aws/iam/users/dev_secrets/main.tf to use recent commits (--since 12 months) and bot filtering.

Validation:

  • terraform -chdir=infra/aws/iam init -backend=false
  • terraform -chdir=infra/aws/iam validate (success)

If you want, I can also switch the contributor refresh command to a Linux-compatible date variant in a follow-up commit.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
infra/aws/modules/platform/web_dev_ssm.tf (2)

5-24: Use ephemeral random_password for consistency with CMS pattern.

The CMS module uses ephemeral "random_password" resources combined with value_wo / value_wo_version to prevent secrets from being stored in Terraform state (per Terraform 1.11+ write-only attribute support). This file uses regular resource "random_password" with the standard value attribute, which stores the secret values in state.

Consider aligning with the CMS approach for consistent security posture across both modules.

♻️ Proposed refactor to use ephemeral passwords and value_wo
-resource "random_password" "web_dev_strapi_api_token" {
+ephemeral "random_password" "web_dev_strapi_api_token" {
   count = local.create_web_dev_ssm_parameters ? 1 : 0
 
   length  = 64
   special = false
 }
 
-resource "random_password" "web_dev_strapi_revalidate_token" {
+ephemeral "random_password" "web_dev_strapi_revalidate_token" {
   count = local.create_web_dev_ssm_parameters ? 1 : 0
 
   length  = 64
   special = false
 }
 
-resource "random_password" "web_dev_strapi_preview_token" {
+ephemeral "random_password" "web_dev_strapi_preview_token" {
   count = local.create_web_dev_ssm_parameters ? 1 : 0
 
   length  = 64
   special = false
 }

Then update the SSM parameters to use value_wo (see next comment).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@infra/aws/modules/platform/web_dev_ssm.tf` around lines 5 - 24, The three
random_password resources (random_password.web_dev_strapi_api_token,
random_password.web_dev_strapi_revalidate_token,
random_password.web_dev_strapi_preview_token) currently create persistent
secrets in state; change them to use the ephemeral provider pattern by declaring
them as ephemeral "random_password" so they do not emit a readable value into
state and then wire their outputs into SSM using the write-only attributes
(value_wo / value_wo_version) instead of value. Update the resource declarations
for those three identifiers to the ephemeral form and ensure downstream SSM
parameter resources reference the ephemeral outputs via
value_wo/value_wo_version.

45-88: SecureString parameters should use value_wo to avoid state exposure.

Continuing from above, the SSM parameters should use value_wo and value_wo_version instead of value to prevent secrets from being stored in Terraform state.

♻️ Proposed refactor for SSM parameters
 resource "aws_ssm_parameter" "web_dev_strapi_api_token" {
   count = local.create_web_dev_ssm_parameters ? 1 : 0
 
   name   = "/forge/aws/web/dev/STRAPI_API_TOKEN"
   type   = "SecureString"
   key_id = module.application.ssm_kms_key_arn
-  value  = random_password.web_dev_strapi_api_token[0].result
+  value_wo         = ephemeral.random_password.web_dev_strapi_api_token[0].result
+  value_wo_version = var.web_ssm_secret_version  # Add variable for rotation
 
   tags = merge(local.tags, {
     Environment = "dev"
     Service     = "web"
   })
-
 }
 
 resource "aws_ssm_parameter" "web_dev_strapi_revalidate_token" {
   count = local.create_web_dev_ssm_parameters ? 1 : 0
 
   name   = "/forge/aws/web/dev/STRAPI_REVALIDATE_TOKEN"
   type   = "SecureString"
   key_id = module.application.ssm_kms_key_arn
-  value  = random_password.web_dev_strapi_revalidate_token[0].result
+  value_wo         = ephemeral.random_password.web_dev_strapi_revalidate_token[0].result
+  value_wo_version = var.web_ssm_secret_version
 
   tags = merge(local.tags, {
     Environment = "dev"
     Service     = "web"
   })
-
 }
 
 resource "aws_ssm_parameter" "web_dev_strapi_preview_token" {
   count = local.create_web_dev_ssm_parameters ? 1 : 0
 
   name   = "/forge/aws/web/dev/STRAPI_PREVIEW_TOKEN"
   type   = "SecureString"
   key_id = module.application.ssm_kms_key_arn
-  value  = random_password.web_dev_strapi_preview_token[0].result
+  value_wo         = ephemeral.random_password.web_dev_strapi_preview_token[0].result
+  value_wo_version = var.web_ssm_secret_version
 
   tags = merge(local.tags, {
     Environment = "dev"
     Service     = "web"
   })
-
 }

Note: You'll need to add a web_ssm_secret_version variable similar to cms_ssm_secret_version for rotation support.

Based on learnings: "using value_wo and value_wo_version on aws_ssm_parameter resources is valid for write-only secrets (Terraform 1.11+). These fields prevent secrets from being stored in the Terraform state."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@infra/aws/modules/platform/web_dev_ssm.tf` around lines 45 - 88, Replace the
use of the value attribute on aws_ssm_parameter resources with the write-only
fields to avoid storing secrets in state: in
aws_ssm_parameter.web_dev_strapi_api_token,
aws_ssm_parameter.web_dev_strapi_revalidate_token, and
aws_ssm_parameter.web_dev_strapi_preview_token remove the value =
random_password... assignments and instead use value_wo =
random_password...result together with value_wo_version =
var.web_ssm_secret_version (add a web_ssm_secret_version variable similar to
cms_ssm_secret_version for rotation support); keep the existing name, type,
key_id and tags unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@infra/aws/iam/users/dev_secrets/main.tf`:
- Around line 1-3: The refresh command uses a BSD/macOS-only date flag (`date -u
-v-12m`) which breaks on GNU/Linux; update the comment to use a portable
approach by either replacing the command with a portable helper that detects the
date implementation (e.g., use `date -d "-12 months"` on GNU and `date -v-12m`
on BSD) or provide both GNU and BSD variants in the refresh instructions so
contributors on both platforms can run the command; ensure the updated text in
the header comment clearly shows both commands or the detection suggestion so
the contributor list can be refreshed cross-platform.
- Around line 14-23: The aws_iam_user resource "aws_iam_user.dev_secrets" can
fail to be destroyed if users retain credentials; add force_destroy = true to
the aws_iam_user "dev_secrets" resource so Terraform will remove users even when
they have access keys/MFA artifacts, ensuring removals of entries from
local.dev_secret_contributors succeed without manual cleanup. Locate resource
aws_iam_user.dev_secrets and insert force_destroy = true in its block; keep
existing tags and name logic unchanged.

---

Nitpick comments:
In `@infra/aws/modules/platform/web_dev_ssm.tf`:
- Around line 5-24: The three random_password resources
(random_password.web_dev_strapi_api_token,
random_password.web_dev_strapi_revalidate_token,
random_password.web_dev_strapi_preview_token) currently create persistent
secrets in state; change them to use the ephemeral provider pattern by declaring
them as ephemeral "random_password" so they do not emit a readable value into
state and then wire their outputs into SSM using the write-only attributes
(value_wo / value_wo_version) instead of value. Update the resource declarations
for those three identifiers to the ephemeral form and ensure downstream SSM
parameter resources reference the ephemeral outputs via
value_wo/value_wo_version.
- Around line 45-88: Replace the use of the value attribute on aws_ssm_parameter
resources with the write-only fields to avoid storing secrets in state: in
aws_ssm_parameter.web_dev_strapi_api_token,
aws_ssm_parameter.web_dev_strapi_revalidate_token, and
aws_ssm_parameter.web_dev_strapi_preview_token remove the value =
random_password... assignments and instead use value_wo =
random_password...result together with value_wo_version =
var.web_ssm_secret_version (add a web_ssm_secret_version variable similar to
cms_ssm_secret_version for rotation support); keep the existing name, type,
key_id and tags unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 342592e6-b012-4ea4-94ef-f72c85d99ae6

📥 Commits

Reviewing files that changed from the base of the PR and between 00548cb and cc45d42.

📒 Files selected for processing (6)
  • infra/aws/iam/users/dev_secrets/main.tf
  • infra/aws/iam/users/dev_secrets/variables.tf
  • infra/aws/iam/users/main.tf
  • infra/aws/modules/cms/main.tf
  • infra/aws/modules/platform/web_dev_ssm.tf
  • scripts/fetch-secrets.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • scripts/fetch-secrets.ts
  • infra/aws/iam/users/dev_secrets/variables.tf
  • infra/aws/iam/users/main.tf

Comment on lines +1 to +3
# Generated from recent human contributors, bots excluded.
# Refresh source list with:
# gh api repos/JesusFilm/forge/commits --paginate -f since="$(date -u -v-12m +%Y-%m-%dT%H:%M:%SZ)" --jq '.[] | select(.author != null and .author.type != "Bot" and (.author.login | test("bot$") | not)) | .author.login' | sort -u
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

date -v flag portability BSD macOS GNU Linux

💡 Result:

date -v is a BSD/macOS-specific extension (it’s not POSIX), so it won’t work on GNU/Linux systems that use GNU coreutils date. (hackmd.io)

GNU/Linux equivalent

On GNU date, the common replacement is -d/--date with a relative date string, e.g.: (gnu.org)

  • BSD/macOS: date -v+1d +"%F"
  • GNU/Linux: date -d "+1 day" +"%F" (gnu.org)

Portability options

  • If you need one script for both: detect GNU vs BSD date and choose -d vs -v.
  • If you want GNU behavior on macOS: install GNU coreutils and use gdate (common convention; avoids BSD date differences).
  • If you want strict POSIX portability: avoid date arithmetic in date (POSIX date doesn’t standardize -v/-d-style adjustments). (man7.org)

Citations:


Make the contributor refresh command portable.

Line 3 uses date -u -v-12m, which is BSD/macOS-only. Contributors on GNU/Linux will get a failing refresh command and may leave this list stale. Switch to a portable helper (e.g., date -d "-12 months" for GNU or detect the date variant) or document both GNU and BSD variants.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@infra/aws/iam/users/dev_secrets/main.tf` around lines 1 - 3, The refresh
command uses a BSD/macOS-only date flag (`date -u -v-12m`) which breaks on
GNU/Linux; update the comment to use a portable approach by either replacing the
command with a portable helper that detects the date implementation (e.g., use
`date -d "-12 months"` on GNU and `date -v-12m` on BSD) or provide both GNU and
BSD variants in the refresh instructions so contributors on both platforms can
run the command; ensure the updated text in the header comment clearly shows
both commands or the detection suggestion so the contributor list can be
refreshed cross-platform.

Comment on lines +14 to +23
resource "aws_iam_user" "dev_secrets" {
for_each = local.dev_secret_contributors

name = "${each.value}-dev-secrets"
path = "/"
tags = merge(var.tags, {
ManagedBy = "terraform"
Role = "dev-secrets"
})
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's find and examine the file mentioned
find . -path "*infra/aws/iam/users/dev_secrets/main.tf" -type f

Repository: JesusFilm/forge

Length of output: 100


🏁 Script executed:

# Search for all aws_iam_user resources in the infra directory
rg "resource \"aws_iam_user\"" infra/ -A 10

Repository: JesusFilm/forge

Length of output: 1421


🏁 Script executed:

# Check if there are other IAM user resources with force_destroy
rg "force_destroy" infra/ -B 3 -A 3

Repository: JesusFilm/forge

Length of output: 41


🌐 Web query:

aws_iam_user force_destroy terraform

💡 Result:

In Terraform, aws_iam_user.force_destroy controls whether Terraform is allowed to delete an IAM user even if the user still has “extra” IAM artifacts that Terraform is not managing, such as:

  • access keys created outside Terraform
  • a console login profile
  • MFA devices

If force_destroy = false (default), terraform destroy (or a replace) can fail because AWS won’t delete a user that still has those items. With force_destroy = true, Terraform will try to remove them and proceed with deletion. [1]

Also: if you attach policies via the legacy aws_iam_policy_attachment and then change the user’s name or path (which forces replacement), you must set force_destroy = true and apply it before the change, otherwise you can hit DeleteConflict. The recommended aws_iam_user_policy_attachment doesn’t have this requirement. [1]

Example:

resource "aws_iam_user" "example" {
  name          = "my-user"
  force_destroy = true
}

Source: Terraform AWS Provider docs for aws_iam_user. [1]


Add force_destroy = true to the managed IAM users.

These users are expected to self-manage access keys and likely MFA artifacts as well. Without force_destroy = true, removing someone from local.dev_secret_contributors will fail with a DeleteConflict error because AWS refuses to delete users that still have credentials attached, which pushes cleanup into manual IAM work.

Suggested change
 resource "aws_iam_user" "dev_secrets" {
   for_each = local.dev_secret_contributors

-  name = "${each.value}-dev-secrets"
-  path = "/"
+  name          = "${each.value}-dev-secrets"
+  path          = "/"
+  force_destroy = true
   tags = merge(var.tags, {
     ManagedBy = "terraform"
     Role      = "dev-secrets"
   })
 }

This keeps infrastructure changes within Terraform as required by the coding guidelines.

📝 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.

Suggested change
resource "aws_iam_user" "dev_secrets" {
for_each = local.dev_secret_contributors
name = "${each.value}-dev-secrets"
path = "/"
tags = merge(var.tags, {
ManagedBy = "terraform"
Role = "dev-secrets"
})
}
resource "aws_iam_user" "dev_secrets" {
for_each = local.dev_secret_contributors
name = "${each.value}-dev-secrets"
path = "/"
force_destroy = true
tags = merge(var.tags, {
ManagedBy = "terraform"
Role = "dev-secrets"
})
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@infra/aws/iam/users/dev_secrets/main.tf` around lines 14 - 23, The
aws_iam_user resource "aws_iam_user.dev_secrets" can fail to be destroyed if
users retain credentials; add force_destroy = true to the aws_iam_user
"dev_secrets" resource so Terraform will remove users even when they have access
keys/MFA artifacts, ensuring removals of entries from
local.dev_secret_contributors succeed without manual cleanup. Locate resource
aws_iam_user.dev_secrets and insert force_destroy = true in its block; keep
existing tags and name logic unchanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(infra): add dev SSM sync and dev credential users

1 participant