Surfaced during the verification of LeanerCloud/cloud-commitments-cli#1817 (which closes LeanerCloud/cloud-commitments-cli#1621), by the implementer rather than the reviewer. Not a regression, and deliberately not fixed in LeanerCloud/cloud-commitments-cli#1817. Companion to #198.
What
LeanerCloud/cloud-commitments-cli#1817 grants the AKS workload identity Key Vault Secrets User on the vault, which is read-only (dataActions getSecret + readMetadata). That is the exact equivalent of the secret_permissions = ["Get", "List"] access policy it replaced, so it correctly grants no more than before.
But the AKS deployment has a genuine Key Vault write path:
AzureResolver.PutSecret calls azsecrets.SetSecret (internal/secrets/azure_resolver.go:116-121)
- reached from
buildAdminPasswordSyncCallback (internal/server/app.go:876-886)
- that callback returns non-nil only when both
ADMIN_PASSWORD_SECRET and ADMIN_EMAIL are set
- the AKS deployment sets both:
ADMIN_EMAIL at terraform/modules/compute/azure/aks/main.tf:343, ADMIN_PASSWORD_SECRET at :348
Verified by reading each site.
The sibling container-apps path sets both too, and is granted Key Vault Secrets Officer by default via writeable_secrets_role (terraform/modules/secrets/azure/main.tf:299), whose variable description names this admin-password sync as the reason.
Consequence
On AKS, an admin password change will 403 on the vault write. It fails soft: logging.Warnf("Failed to sync admin password to secret manager: %v", err). The password change still commits to the database; only the Key Vault copy goes stale. So the two copies silently diverge, which matters for any recovery path that reads the vault copy.
Upgrading AKS to Secrets Officer there would have granted strictly more than the access policy did, breaking that PR's no-more-privilege property, which was the basis for its safety argument. That was the right call: the grant-model fix and the grant-widening decision are separate, and the second deserves its own review.
Bounds worth keeping in mind when prioritising
Fix direction
Decide deliberately rather than by symmetry with container-apps:
- Grant AKS
Key Vault Secrets Officer, matching container-apps, accepting the write privilege; or
- Add a
writeable_secrets_role-style variable to the AKS module so the grant is explicit and downgradable, mirroring the existing pattern; or
- Leave AKS read-only and stop setting
ADMIN_PASSWORD_SECRET / ADMIN_EMAIL in that module, so the callback never activates and the failure cannot occur.
Option 3 is worth genuine consideration rather than defaulting to 1: a write grant that exists only to keep a secondary copy in sync is a real privilege for a modest benefit.
Verification bar
Whichever option, assert both directions: the admin-password sync path succeeds under the chosen grant, AND nothing broader than intended is granted. Note the standing caveat that the role's dataActions have been corroborated from documentation and in-tree usage, not from an az role definition list response.
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.
A13c-004 (high)
The container-apps half of this is not actually covered. This issue states the sibling path "is granted Key Vault Secrets Officer by default via writeable_secrets_role (terraform/modules/secrets/azure/main.tf:299)", but that role assignment is count = 0 in every environment: container_app_identity_principal_id defaults to null (secrets/azure/variables.tf:140) and terraform/environments/azure/secrets.tf:20-54 never passes it. The only grant the container app receives is the read-only Key Vault Secrets User at terraform/environments/azure/compute.tf:205, while compute.tf:81 still sets admin_password_secret_name, so buildAdminPasswordSyncCallback arms and PutSecret 403s exactly as on AKS. Unlike AKS this path is deployed today, since all three Azure environments select container-apps. It fails soft the same way (logging.Warnf at app.go:894), so the vault copy diverges silently. Finding A13c-004.
Surfaced during the verification of LeanerCloud/cloud-commitments-cli#1817 (which closes LeanerCloud/cloud-commitments-cli#1621), by the implementer rather than the reviewer. Not a regression, and deliberately not fixed in LeanerCloud/cloud-commitments-cli#1817. Companion to #198.
What
LeanerCloud/cloud-commitments-cli#1817 grants the AKS workload identity
Key Vault Secrets Useron the vault, which is read-only (dataActionsgetSecret+readMetadata). That is the exact equivalent of thesecret_permissions = ["Get", "List"]access policy it replaced, so it correctly grants no more than before.But the AKS deployment has a genuine Key Vault write path:
AzureResolver.PutSecretcallsazsecrets.SetSecret(internal/secrets/azure_resolver.go:116-121)buildAdminPasswordSyncCallback(internal/server/app.go:876-886)ADMIN_PASSWORD_SECRETandADMIN_EMAILare setADMIN_EMAILatterraform/modules/compute/azure/aks/main.tf:343,ADMIN_PASSWORD_SECRETat:348Verified by reading each site.
The sibling container-apps path sets both too, and is granted
Key Vault Secrets Officerby default viawriteable_secrets_role(terraform/modules/secrets/azure/main.tf:299), whose variable description names this admin-password sync as the reason.Consequence
On AKS, an admin password change will 403 on the vault write. It fails soft:
logging.Warnf("Failed to sync admin password to secret manager: %v", err). The password change still commits to the database; only the Key Vault copy goes stale. So the two copies silently diverge, which matters for any recovery path that reads the vault copy.Why it was not fixed in LeanerCloud/cloud-commitments-cli#1817
Upgrading AKS to
Secrets Officerthere would have granted strictly more than the access policy did, breaking that PR's no-more-privilege property, which was the basis for its safety argument. That was the right call: the grant-model fix and the grant-widening decision are separate, and the second deserves its own review.Bounds worth keeping in mind when prioritising
count = var.compute_platform == "aks" ? 1 : 0and all three Azure environments selectcontainer-apps, so nothing deploys it today.workload_identity_enabled, no federated credential). Until fix(iac/azure): AKS workload identity is not wired up, so the UAI is unassumable by any pod #198 is fixed, neither the reads nor this write happen. Fixing this issue alone changes nothing observable.Fix direction
Decide deliberately rather than by symmetry with container-apps:
Key Vault Secrets Officer, matching container-apps, accepting the write privilege; orwriteable_secrets_role-style variable to the AKS module so the grant is explicit and downgradable, mirroring the existing pattern; orADMIN_PASSWORD_SECRET/ADMIN_EMAILin that module, so the callback never activates and the failure cannot occur.Option 3 is worth genuine consideration rather than defaulting to 1: a write grant that exists only to keep a secondary copy in sync is a real privilege for a modest benefit.
Verification bar
Whichever option, assert both directions: the admin-password sync path succeeds under the chosen grant, AND nothing broader than intended is granted. Note the standing caveat that the role's dataActions have been corroborated from documentation and in-tree usage, not from an
az role definition listresponse.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.A13c-004 (high)
The container-apps half of this is not actually covered. This issue states the sibling path "is granted Key Vault Secrets Officer by default via writeable_secrets_role (terraform/modules/secrets/azure/main.tf:299)", but that role assignment is count = 0 in every environment: container_app_identity_principal_id defaults to null (secrets/azure/variables.tf:140) and terraform/environments/azure/secrets.tf:20-54 never passes it. The only grant the container app receives is the read-only Key Vault Secrets User at terraform/environments/azure/compute.tf:205, while compute.tf:81 still sets admin_password_secret_name, so buildAdminPasswordSyncCallback arms and PutSecret 403s exactly as on AKS. Unlike AKS this path is deployed today, since all three Azure environments select container-apps. It fails soft the same way (logging.Warnf at app.go:894), so the vault copy diverges silently. Finding A13c-004.