feat(agents): issue-992-p1-workflow-parameters-are-interpolated

[mctl-agents]

Implements accepted proposal mctl-gitops/issue-992-p1-workflow-parameters-are-interpolated.

Spec: https://github.com/mctlhq/mctl-gitops/tree/main/platform-gitops/agents-state/mctl-gitops/proposals/issue-992-p1-workflow-parameters-are-interpolated/

Generated by mctl-agents Tier 2 implementer. Review carefully — this PR was opened automatically and the spec may have gaps.

Refs #992

---

Scope note (operator): this PR is the first slice of #992 (proposal tasks 1-5: injection test harness, tpl-validate-tenant, tpl-git-commit, tpl-vault-write, assert-attempt). Still open on #992: task 6 (wft-smoke-test), task 7 (create/delete/deploy/provision/retire sites), task 8 (validator ratchet becomes a ban), task 10 (live smoke). The API-side follow-up from task 9 is mctlhq/mctl-api#559. #992 stays open.

[claude]

Claude finished @mctl-agents[bot]'s task in 6m 11s —— View job

---

Reviewing PR #1812

• Fetch context and diff (git diff 2189c29f...06b87eba1)
• Analyze Argo Workflow template changes (tpl-git-commit, tpl-validate-tenant, tpl-vault-write, assert-attempt in the 4 cwft-mctl-agents-* files)
• Analyze validation script changes (scripts/validate-shell-param-interpolation.py baseline shrink)
• Analyze test coverage (new tests/test_shell_param_injection.py, updated tests/test_tpl_git_commit_yq.py)
• Post findings / verdict

Summary

This converts the remaining {{inputs.parameters.X}} direct interpolations into env: bindings read as "$PARAM_X", closing the shell-injection class tracked in #992 for tpl-git-commit, tpl-validate-tenant, tpl-vault-write, and the assert-attempt template in all four cwft-mctl-agents-* files.

Verified:

• The heredocs switched from quoted (<< 'ENVEOF', literal param text embedded) to unquoted (<<ENVEOF, body is just $PARAM_X). This is safe: a heredoc's terminator is matched against the literal script text at parse time, before the $PARAM_X parameter expansion happens — so a value containing a line like ENVEOF can no longer truncate the heredoc early.
• Every $PARAM_* read in a script has a matching env: entry in that same template (no missing bindings across any of the 7 edited templates).
• tpl-vault-write.yaml's write-platform-secret adds a case guard rejecting empty/absolute/../non-allowlisted-char vault_path values before they reach the Vault URL, and json_data is now piped to curl via stdin (--data-binary @-) instead of spliced into the command line.
• New test tests/test_shell_param_injection.py extracts each template's real prelude/loop text and runs quote-breakout, $()/backtick, and terminator-line payloads against it, plus a control case proving the harness actually detects injection on the old (pre-fix) shape.
• test_tpl_git_commit_yq.py gained a case (5b) proving a terminator-line value in env_vars is treated as data, not shell.

One P3 posted inline (non-blocking): the new vault_path guard in tpl-vault-write.yaml allows /-sequences like a//b or a trailing / through (not an injection — / is intentionally allowlisted — just a minor hygiene nit).

No P1/P2 findings. Approved.

[github-actions]

Antigravity (agy) review — informational

Model: gemini-3.8-flash-medium · commit 06b87eb.

No significant issues found.

[claude]

P3: the *..* / char-class guard rejects path traversal and shell/URL metacharacters, but still accepts a leading/embedded/trailing / sequence like a//b or a/. Not an injection (the character / is in the allowlist by design, since vault_path is itself a sub-path), just a minor hygiene gap — Vault will likely just normalize it, but consider also rejecting //* or a leading/trailing / if that matters for the secret/data/${PARAM_VAULT_PATH} URL this feeds.

[claude]

No P1/P2 findings (1 P3). Converts the remaining direct {{inputs.parameters.X}} interpolations in tpl-git-commit/tpl-validate-tenant/tpl-vault-write and the assert-attempt templates to env-var bindings read as "$PARAM_X" — closing the shell-injection class tracked in #992. Verified: unquoted heredocs (<<ENVEOF etc.) still can't be terminator-injected because the shell matches the delimiter against the literal script text before parameter expansion runs, every $PARAM_* read has a matching env: entry, the new vault_path case-guard correctly blocks empty/absolute/traversal/non-allowlisted-char paths before it reaches the Vault URL, and json_data now goes through curl via stdin instead of being spliced into the command line. Test coverage (tests/test_shell_param_injection.py, plus the terminator-line case added to test_tpl_git_commit_yq.py) exercises quote-breakout, $()/backtick, and terminator-line payloads against the actual extracted template prelude/loop text, with a control case proving the harness detects injection on the old shape. Good to merge.