Two items surfaced during review of LeanerCloud/cloud-commitments-cli#1866 (the fix for LeanerCloud/cloud-commitments-cli#1661). Both are pre-existing and were deliberately left out of that PR to keep its diff on the reported defects. Recorded here so they do not live only in a review thread.
1. CUDLY_ISSUER_URL is unvalidated in the served GCP script, while the ARM script validates the same destination
internal/iacfiles/templates/gcp-wif-cli.sh.tmpl:30:
CUDLY_ISSUER_URL="${CUDLY_ISSUER_URL:-{{.CUDlyAPIURL}}/oidc}"
It is environment-overridable and flows straight into --issuer-uri on the provider create. arm/CUDly-CrossSubscription/setup-gcp-wif.sh validates the same value:
validate_principal_value "--issuer-uri" "$ISSUER_URI" '^https://[a-zA-Z0-9.-]+(:[0-9]+)?(/[-a-zA-Z0-9._~/]*)?$'
The served path has no equivalent. Whoever controls the issuer that ends up configured can mint a token with any subject, and GCP will fetch their /.well-known/openid-configuration and trust it. A typo or a pasted wrong URL is enough; there is no attacker-controlled input path, so this is a misconfiguration vector rather than a live bypass.
The asymmetry is the reason to close this deliberately. Two implementations of one control that disagree is the shape behind LeanerCloud/cloud-commitments-cli#1592 → LeanerCloud/cloud-commitments-cli#1820 → LeanerCloud/cloud-commitments-cli#1821. LeanerCloud/cloud-commitments-cli#1866 hardened the subject on this path (CUDLY_FEDERATED_SUBJECT) and left the issuer as the one value on the same line of defence still unguarded.
Suggested fix: apply the ARM script's regex to CUDLY_ISSUER_URL in the template, before the first gcloud call, alongside the subject guard added in LeanerCloud/cloud-commitments-cli#1866. Reject plain http:// explicitly. internal/iacfiles/templates_test.go has the harness for it (renders the script and runs it under bash against a recording gcloud stub); a reject case must assert zero gcloud calls, and an accept case must keep a normal run working.
2. normalize_mapping aborts with unbound variable on bash 3.2 when the mapping is empty
arm/CUDly-CrossSubscription/setup-gcp-wif.sh:
normalize_mapping() {
local mapping="$1" pair_sep="$2"
local -a pairs
IFS="$pair_sep" read -ra pairs <<<"$mapping"
printf '%s\n' "${pairs[@]}" | sort
}
"${pairs[@]}" on an empty array is an error under set -u in bash < 4.4, which is macOS's /bin/bash (3.2.57) and therefore the shell many operators will run this with. Reproduced: normalize_mapping "" ";" emits pairs[@]: unbound variable.
It is reached when an existing provider has an empty attribute mapping. The path still fails closed — the substitution runs inside [[ ]], so the empty result mismatches and die fires — but the operator sees a bash internal error instead of the "different attribute mapping" diagnostic that was written for exactly this case, and a reader cannot tell the closed outcome is accidental.
Suggested fix: printf '%s\n' ${pairs[@]+"${pairs[@]}"} | sort, or return early on [[ ${#pairs[@]} -eq 0 ]].
Triage note
Filed as one issue because both are small, both sit in the same pair of onboarding artifacts, and both were found in the same review. Split if they end up with different owners. Labels take the higher severity across the two and the union of types, per the multi-concern rule: item 1 drives severity/medium and type/security, item 2 is type/bug and low on its own.
Impact is impact/few rather than impact/all-users deliberately: the missing guard is on a path every GCP onboarding uses, but harm only materialises for an operator who supplies a wrong issuer, which is a small population. If that reading is wrong, the priority should go up with it.
3. CUDLY_FEDERATED_SUBJECT is overridable but unsatisfiable
Added after review of LeanerCloud/cloud-commitments-cli#1866 found it from the Go side rather than the script side.
internal/credentials/gcp_federated.go:20:
// gcpFederatedSubject is the fixed JWT subject CUDly uses when the
// target GCP project has a Workload Identity Pool provider bound to
// CUDly's own OIDC issuer. Changing this string is an incompatible
// change that requires every existing WIF provider's
// attribute_condition to be recreated.
const gcpFederatedSubject = "cudly-controller"
It is a compile-time constant, used directly as the sub claim at line 37, with no per-account override anywhere in the codebase. But internal/iacfiles/templates/gcp-wif-cli.sh.tmpl:31 honours an environment override:
CUDLY_FEDERATED_SUBJECT="${CUDLY_FEDERATED_SUBJECT:-cudly-controller}"
and feeds it into both the provider's attribute condition and the impersonation grant. An operator who sets it gets a provider and a grant pinned to a subject CUDly will never present. The script prints === Done ===, and with CUDlyAPIURL set it auto-registers the account. So the override is not merely unvalidated, it is unsatisfiable: no value other than the default can ever work.
The same shape exists on the Azure path: azure-wif-cli.sh.tmpl:26 and azure-wif-deploy.sh.tmpl:61 against the equivalent constant in internal/credentials/azure_federated.go:22. Whatever is decided here should be applied there in the same change.
Options, roughly in increasing order of cost: warn when the value differs from the default; reject the override outright; or make the subject genuinely per-account, which is the change the constant's own doc comment warns is incompatible with every existing provider.
LeanerCloud/cloud-commitments-cli#1866 deliberately did not touch the knob: it predates that range, appears only as diff context, and the Azure duplication puts it beyond that PR's scope. What LeanerCloud/cloud-commitments-cli#1866 did do is stop its own test suite from implying the override works, since a test asserting that google-oauth2|1234567890 configures a project end to end read as an endorsement of a configuration that cannot authenticate.
Why these three are one issue
All three items here are the same shape: a script promise the rest of the system does not keep. The served script accepts an issuer nobody validates, reports a mapping comparison it cannot always render, and offers a subject knob the backend cannot honour. Each is individually small; together they are a pattern worth fixing in one pass over the onboarding templates.
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-033 (medium)
Same bash 3.2 class as the normalize_mapping half of this issue, in a second script. scripts/gcp-import-dev-state.sh:151 uses declare -A SECRET_TF_ADDR with no BASH_VERSINFO guard anywhere in the file; measured, /bin/bash 3.2.57 rejects declare -A with exit 2 and set -euo pipefail (line 17) turns that into an abort. The damage is that line 70 has already run terraform init -backend-config=... -reconfigure against the live GCS state, so a Mac operator is left with a reconfigured local backend and zero imports. scripts/lib/code-scan-awk.sh:74 and check-azure-role-parity.sh:118 both avoid bash-4 constructs for this reason. Finding A14-033.
Two items surfaced during review of LeanerCloud/cloud-commitments-cli#1866 (the fix for LeanerCloud/cloud-commitments-cli#1661). Both are pre-existing and were deliberately left out of that PR to keep its diff on the reported defects. Recorded here so they do not live only in a review thread.
1.
CUDLY_ISSUER_URLis unvalidated in the served GCP script, while the ARM script validates the same destinationinternal/iacfiles/templates/gcp-wif-cli.sh.tmpl:30:CUDLY_ISSUER_URL="${CUDLY_ISSUER_URL:-{{.CUDlyAPIURL}}/oidc}"It is environment-overridable and flows straight into
--issuer-urion the provider create.arm/CUDly-CrossSubscription/setup-gcp-wif.shvalidates the same value:The served path has no equivalent. Whoever controls the issuer that ends up configured can mint a token with any subject, and GCP will fetch their
/.well-known/openid-configurationand trust it. A typo or a pasted wrong URL is enough; there is no attacker-controlled input path, so this is a misconfiguration vector rather than a live bypass.The asymmetry is the reason to close this deliberately. Two implementations of one control that disagree is the shape behind LeanerCloud/cloud-commitments-cli#1592 → LeanerCloud/cloud-commitments-cli#1820 → LeanerCloud/cloud-commitments-cli#1821. LeanerCloud/cloud-commitments-cli#1866 hardened the subject on this path (
CUDLY_FEDERATED_SUBJECT) and left the issuer as the one value on the same line of defence still unguarded.Suggested fix: apply the ARM script's regex to
CUDLY_ISSUER_URLin the template, before the firstgcloudcall, alongside the subject guard added in LeanerCloud/cloud-commitments-cli#1866. Reject plainhttp://explicitly.internal/iacfiles/templates_test.gohas the harness for it (renders the script and runs it under bash against a recordinggcloudstub); a reject case must assert zero gcloud calls, and an accept case must keep a normal run working.2.
normalize_mappingaborts withunbound variableon bash 3.2 when the mapping is emptyarm/CUDly-CrossSubscription/setup-gcp-wif.sh:"${pairs[@]}"on an empty array is an error underset -uin bash < 4.4, which is macOS's/bin/bash(3.2.57) and therefore the shell many operators will run this with. Reproduced:normalize_mapping "" ";"emitspairs[@]: unbound variable.It is reached when an existing provider has an empty attribute mapping. The path still fails closed — the substitution runs inside
[[ ]], so the empty result mismatches anddiefires — but the operator sees a bash internal error instead of the "different attribute mapping" diagnostic that was written for exactly this case, and a reader cannot tell the closed outcome is accidental.Suggested fix:
printf '%s\n' ${pairs[@]+"${pairs[@]}"} | sort, or return early on[[ ${#pairs[@]} -eq 0 ]].Triage note
Filed as one issue because both are small, both sit in the same pair of onboarding artifacts, and both were found in the same review. Split if they end up with different owners. Labels take the higher severity across the two and the union of types, per the multi-concern rule: item 1 drives
severity/mediumandtype/security, item 2 istype/bugand low on its own.Impact is
impact/fewrather thanimpact/all-usersdeliberately: the missing guard is on a path every GCP onboarding uses, but harm only materialises for an operator who supplies a wrong issuer, which is a small population. If that reading is wrong, the priority should go up with it.3.
CUDLY_FEDERATED_SUBJECTis overridable but unsatisfiableAdded after review of LeanerCloud/cloud-commitments-cli#1866 found it from the Go side rather than the script side.
internal/credentials/gcp_federated.go:20:It is a compile-time constant, used directly as the
subclaim at line 37, with no per-account override anywhere in the codebase. Butinternal/iacfiles/templates/gcp-wif-cli.sh.tmpl:31honours an environment override:CUDLY_FEDERATED_SUBJECT="${CUDLY_FEDERATED_SUBJECT:-cudly-controller}"and feeds it into both the provider's attribute condition and the impersonation grant. An operator who sets it gets a provider and a grant pinned to a subject CUDly will never present. The script prints
=== Done ===, and withCUDlyAPIURLset it auto-registers the account. So the override is not merely unvalidated, it is unsatisfiable: no value other than the default can ever work.The same shape exists on the Azure path:
azure-wif-cli.sh.tmpl:26andazure-wif-deploy.sh.tmpl:61against the equivalent constant ininternal/credentials/azure_federated.go:22. Whatever is decided here should be applied there in the same change.Options, roughly in increasing order of cost: warn when the value differs from the default; reject the override outright; or make the subject genuinely per-account, which is the change the constant's own doc comment warns is incompatible with every existing provider.
LeanerCloud/cloud-commitments-cli#1866 deliberately did not touch the knob: it predates that range, appears only as diff context, and the Azure duplication puts it beyond that PR's scope. What LeanerCloud/cloud-commitments-cli#1866 did do is stop its own test suite from implying the override works, since a test asserting that
google-oauth2|1234567890configures a project end to end read as an endorsement of a configuration that cannot authenticate.Why these three are one issue
All three items here are the same shape: a script promise the rest of the system does not keep. The served script accepts an issuer nobody validates, reports a mapping comparison it cannot always render, and offers a subject knob the backend cannot honour. Each is individually small; together they are a pattern worth fixing in one pass over the onboarding templates.
Findings from the 2026-09-02 codebase audit
Added by an automated audit of
3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd(tip oforigin/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-033 (medium)
Same bash 3.2 class as the normalize_mapping half of this issue, in a second script. scripts/gcp-import-dev-state.sh:151 uses declare -A SECRET_TF_ADDR with no BASH_VERSINFO guard anywhere in the file; measured, /bin/bash 3.2.57 rejects declare -A with exit 2 and set -euo pipefail (line 17) turns that into an abort. The damage is that line 70 has already run terraform init -backend-config=... -reconfigure against the live GCS state, so a Mac operator is left with a reconfigured local backend and zero imports. scripts/lib/code-scan-awk.sh:74 and check-azure-role-parity.sh:118 both avoid bash-4 constructs for this reason. Finding A14-033.