chore(scripts): generate-federation-iac.go iacData has drifted out of sync with the templates it renders #153

Activity

1. cristim

   added

   triagedItem has been triagedItem has been triaged

   priority/p3Polish / idea / may never shipPolish / idea / may never ship

   severity/lowMinor harmMinor harm

   urgency/eventuallyNo deadlineNo deadline

   impact/internalTeam-internal onlyTeam-internal only

   effort/xsTrivial / one-linerTrivial / one-liner

   type/bugDefectDefect

   on Aug 3, 2026
2. cristim commented on Aug 3, 2026

   

   cristim

   on Aug 3, 2026

   MemberAuthor

   More actions

   Three follow-ups found while implementing the CodeRabbit fixes on LeanerCloud/cloud-commitments-cli#1691 (which closes LeanerCloud/cloud-commitments-cli#1640). All are pre-existing on origin/main and all were deliberately left out of LeanerCloud/cloud-commitments-cli#1691 to keep that PR scoped to the --oidc-subject-claim path. Filing here since this issue already tracks the drift between scripts/generate-federation-iac.go and the templates.

   ---

   1. iacData is also missing SourceAccountID, so the aws -> aws cross-account path is broken too

   The issue body lists CUDlyAPIURL, ContactEmail and OIDCIssuerHost. SourceAccountID is missing as well: internal/iacfiles/templates/aws-cross-account.tfvars.tmpl references it, so the cross-account combination fails the same way the WIF one does.

   Reproduced against origin/main's copy of the script (34d85bc1b):

   $ go run scripts/generate-federation-iac.go \
       --target aws --source aws \
       --account-name "tgt" --account-id "999888777666" --output -
   Error: render internal/iacfiles/templates/aws-cross-account.tfvars.tmpl: template: iac:16:23: executing "iac" at <.SourceAccountID>: can't evaluate field SourceAccountID in type main.iacData
   exit status 1
   

   Full set of fields the templates reference but iacData lacks: CUDlyAPIURL, ContactEmail, SourceAccountID, OIDCIssuerHost. Net effect: every .tfvars path of this script is unusable, plus --format bundle for all targets. --format cf-params is the only working mode.

   This is why LeanerCloud/cloud-commitments-cli#1691's TestGenerator_CrossAccountDoesNotRequireSubjectClaim asserts on stderr rather than on exitCode == 0; there is a comment in that test pointing back here.

   ---

   2. shellEscape does not escape }

   internal/api/handler_federation.go:399

   r := strings.NewReplacer(`\`, `\\`, `"`, `\"`, "`", "\\`", `$`, `\$`)

   It covers \ " $but not</code>}<code>. Several CLI templates interpolate values inside a ${VAR:-{{.Field}}}default, e.g.aws-wif-cli.sh.tmpl:21and:139, aws-cfn-deploy.sh.tmpl:56. A value containing }terminates the parameter expansion early, so the remainder of the value lands in the script as bare shell tokens rather than as part of the string. Escaping$alone does not prevent that, because the}` is structural to the surrounding expansion, not to the value.

   Not currently exploitable: every field that reaches those expansions is built by buildGenericIaCData from the validated target/source/config URL, or by populateSourceAccountID from the database. It is a latent gap in a helper whose stated contract is "safe use inside a double-quoted bash argument", which } breaks.

   ---

   3. Two render paths that emit shell/HCL bypass escaping entirely

   shellEscapeData is applied at only two call sites (handler_federation.go:274 for single-file cli, and :640 for cloudformation/deploy-cfn.sh). Two other renders take the raw struct:

   • handler_federation.go:532 renders templates/azure-wif-bicep-deploy.sh.tmpl with raw data. This is a shell script written into the Azure bicep/ARM zip, and it is the one deploy script in the codebase that does not get the shellEscapeData treatment its siblings get. Most likely an oversight rather than a decision.
   • handler_federation.go:592 renders the .auto.tfvars templates with raw data. Here escaping is not the right fix: shell escaping is wrong for HCL. HCL string values need their own treatment (", \, and ${/%{ interpolation sequences), or the values need validating at the boundary the way sec(iac/aws): require OIDC subject claim in the federation bundle generator cloud-commitments-cli#1691 now validates --oidc-subject-claim.

   Same caveat as LeanerCloud/cloud-commitments-cli#2: the fields are not attacker-controlled from the request today, so this is hardening, not an open hole.

   ---

   Suggested shape for whoever picks this up

   1. Add the four missing fields to iacData and wire them from flags (--cudly-api-url, --contact-email, --source-account-id); derive OIDCIssuerHost from OIDCIssuerURL as the server does.
   2. Add a test asserting iacData and federationIaCData stay field-compatible (reflection over field names is enough) so this cannot drift again silently. Better still, render every template in internal/iacfiles/templates with Option("missingkey=error") against iacData in a test, which catches a missing field the moment a template starts using it.
   3. Decide per sink rather than per struct: shell templates get shellEscapeData (with } added to shellEscape), HCL templates get an HCL-safe path or boundary validation, JSON templates go through encoding/json rather than string interpolation.
3. cristim

   added 2 commits that reference this issue on Aug 3, 2026

   sec(scripts): validate --oidc-subject-claim before any rendering

   ...

   68a47c0

   sec(iac/aws): require OIDC subject claim in the federation bundle gen…

   ...

   6427675
4. cristim

   transferred this issue fromLeanerCloud/cloud-commitments-clion Sep 27, 2026