Skip to content

fix(scripts): --account-id has no per-target format validation, so a malformed value reaches the generated tfvars #164

Description

@cristim

--account-id in scripts/generate-federation-iac.go is checked for presence but never for format, so a malformed value flows into the generated tfvars and surfaces later as a confusing Terraform or provider error far from its cause.

This is the same defect class CodeRabbit found on --source-account-id in LeanerCloud/cloud-commitments-cli#1710, one field over. That one is now validated as exactly 12 ASCII digits; --account-id has no format check on any target.

Why it was not fixed in LeanerCloud/cloud-commitments-cli#1710

--account-id is polymorphic — it means a different thing per --target:

--target meaning shape
aws AWS account ID 12 digits
azure Azure subscription ID GUID
gcp GCP project ID 6-30 chars, lowercase letter first, letters/digits/hyphens, no trailing hyphen

So there is no single regex, which is why LeanerCloud/cloud-commitments-cli#1710 correctly validated only --source-account-id (unambiguously an AWS account ID) and left this alone rather than inventing a one-size check. Flagged by that PR's implementer rather than silently skipped.

Suggested fix

Validate --account-id per target, after --target is parsed and before the value reaches iacData:

  • aws -> ^[0-9]{12}$
  • azure -> GUID, case-insensitive
  • gcp -> the documented project-ID rule

Reuse the existing sourceAccountIDRE for the AWS arm rather than declaring a second 12-digit regex; two copies of one rule drift.

Error messages should name the flag, the rejected value and the expected shape for that target, matching the style already used by --source-account-id:

--source-account-id %q is not a valid AWS account ID: it must be exactly 12 digits

Reject rather than normalise. Do not trim whitespace or lowercase the value into validity — a normaliser exists to make different strings equal, which is the opposite of what a generator producing customer IaC wants.

Tests

One malformed case per target arm, asserting a non-zero exit and an error naming the flag, plus a positive control per target proving valid values still pass. LeanerCloud/cloud-commitments-cli#1710 added 8 subtests (4 malformed shapes x 2 routes) for --source-account-id; mirror that shape.

Scope

Keep it to --account-id. Do not extend validation to other flags in this pass, and do not change what any flag means. --tenant-id, --project-id and --service-account-email may deserve the same treatment, but each is a separate decision and bundling them makes the change harder to review.

Found while reviewing LeanerCloud/cloud-commitments-cli#1710.

Findings from the 2026-09-02 codebase audit

Added by an automated audit of 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd (tip of origin/main). Each item below was reported by one reviewer and independently confirmed by a second that did not write it. Full report: docs/audits/codebase-audit-2026-09-02.md.

A14-027 (medium)

Six sibling fields with the same gap, from finding A14-027. This issue is scoped to --account-id's format, but the underlying problem is that only OIDCSubjectClaim is validated at all (validateOIDCSubjectClaim, scripts/generate-federation-iac.go:441), while AccountName, AccountExternalID, TenantID, ProjectID, ServiceAccountEmail, CUDlyAPIURL and ContactEmail reach the same templates raw. Demonstrated: --account-name 'Acme"\nevil_var = "pwned' renders a second HCL attribute into the generated tfvars, and --cudly-api-url 'https://x$(id)' passes through verbatim into aws-cfn-deploy.sh.tmpl:78's Bash. The server-side renderer of the same templates escapes every field (internal/api/handler_federation.go:285-299), so the two callers of one template set disagree. The Bash sink is unreachable today because bundle mode dies earlier, but the HCL sink is reachable. Worth fixing as one validation pass over iacData rather than field by field.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions