Describe the bug
--enable-cross-namespace=false does not block cross-namespace Secret writes. The flag is enforced only on the read path, so the ACM and ACMPCA "export a certificate/key into a Secret" features still write into any namespace the user names, even with the flag off.
The gate lives in SecretValueFromReference (runtime/pkg/runtime/reconciler.go#L172), which calls ValidateCrossNamespaceReferenceString and returns a terminal error when the reference crosses a namespace and the flag is disabled.
Its counterpart WriteToSecret (runtime/pkg/runtime/reconciler.go#L231) takes a raw namespace string and validates nothing — no namespace check, and no Secret type check either (the read path accepts only Opaque).
Affected export paths, all of which pass a user-supplied namespace straight through:
This is the higher-impact half of the asymmetry, because what crosses the namespace boundary on a write is material the controller just created: an exported certificate chain and, for acm Certificate, the decrypted private key. A tenant who cannot read a Secret in another namespace can still cause one to be written there.
For contrast, acm AcmeExternalAccountBinding (spec.credentialsOutput) does validate, but only because the hook calls ValidateCrossNamespaceReferenceString itself before writing (acme_external_account_binding/hooks.go#L86). That per-hook workaround is what suggests the check belongs in the runtime instead.
Steps to reproduce
- Run the
acm controller with --enable-cross-namespace=false (Helm: enableCrossNamespace: false).
- Create an empty
Opaque Secret exported-cert in namespace team-b.
- In namespace
team-a, create a Certificate with:
spec:
exportTo:
name: exported-cert
namespace: team-b
key: tls.crt
- Wait for the certificate to be issued.
Observed: team-b/exported-cert is populated with the certificate chain and tls.key. The resource reports Synced=True, no ACK.Terminal, and no ACK.Advisory / CrossNamespaceOptInRequired condition.
The same applies to acmpca Certificate and CertificateAuthorityActivation via spec.certificateOutput.secretReference.namespace.
For comparison, a cross-namespace read on the same controller (spec.certificate / spec.privateKey / spec.certificateChain pointing at a Secret in another namespace) is correctly rejected with a terminal error.
(Note: the above is derived from reading the code paths, not from a cluster run — the call sites take the namespace verbatim and there is no validation between them and the Patch. Happy to confirm on a kind cluster if useful.)
Expected outcome
With --enable-cross-namespace=false, a write to a Secret in a namespace other than the resource's own should be refused with a terminal error, the same as a read. With the flag enabled (Phase 1 default), it should proceed and set the ACK.Advisory / CrossNamespaceOptInRequired deprecation notice, so operators get the same visibility they get for references and field exports before the default flips.
Suggested fix: move the check into WriteToSecret in the runtime, rather than repeating it in every hook that exports a value. That covers all three ACM/ACMPCA paths and any future controller that writes to a Secret, and it keeps the write path from drifting from the read path again. Validating the target Secret's type there too would close the matching gap (today a write into a non-Opaque Secret succeeds and the next read of it fails).
Because this affects a security control rather than an ordinary bug, it would be worth confirming whether the fix should land before the default flips to false — otherwise flipping the default silently leaves these three export paths unchanged.
Environment
- Kubernetes version: any
- Using EKS (yes/no), if so version? not EKS-specific
- AWS service targeted: ACM, ACM PCA (the runtime gap applies to any controller calling
WriteToSecret)
- ACK runtime:
v0.60.0 and later (v0.63.0 current); --enable-cross-namespace was introduced in v0.60.0
- Controller versions:
acm v1.4.1+ (latest v1.8.1), acmpca v1.3.1+ (latest v1.5.0)
Describe the bug
--enable-cross-namespace=falsedoes not block cross-namespace Secret writes. The flag is enforced only on the read path, so the ACM and ACMPCA "export a certificate/key into a Secret" features still write into any namespace the user names, even with the flag off.The gate lives in
SecretValueFromReference(runtime/pkg/runtime/reconciler.go#L172), which callsValidateCrossNamespaceReferenceStringand returns a terminal error when the reference crosses a namespace and the flag is disabled.Its counterpart
WriteToSecret(runtime/pkg/runtime/reconciler.go#L231) takes a rawnamespace stringand validates nothing — no namespace check, and no Secrettypecheck either (the read path accepts onlyOpaque).Affected export paths, all of which pass a user-supplied namespace straight through:
acmCertificatespec.exportTo.namespace(cert chain +tls.key)acmpcaCertificatespec.certificateOutput.secretReference.namespaceacmpcaCertificateAuthorityActivationspec.certificateOutput.secretReference.namespaceThis is the higher-impact half of the asymmetry, because what crosses the namespace boundary on a write is material the controller just created: an exported certificate chain and, for
acmCertificate, the decrypted private key. A tenant who cannot read a Secret in another namespace can still cause one to be written there.For contrast,
acmAcmeExternalAccountBinding(spec.credentialsOutput) does validate, but only because the hook callsValidateCrossNamespaceReferenceStringitself before writing (acme_external_account_binding/hooks.go#L86). That per-hook workaround is what suggests the check belongs in the runtime instead.Steps to reproduce
acmcontroller with--enable-cross-namespace=false(Helm:enableCrossNamespace: false).OpaqueSecretexported-certin namespaceteam-b.team-a, create aCertificatewith:Observed:
team-b/exported-certis populated with the certificate chain andtls.key. The resource reportsSynced=True, noACK.Terminal, and noACK.Advisory/CrossNamespaceOptInRequiredcondition.The same applies to
acmpcaCertificateandCertificateAuthorityActivationviaspec.certificateOutput.secretReference.namespace.For comparison, a cross-namespace read on the same controller (
spec.certificate/spec.privateKey/spec.certificateChainpointing at a Secret in another namespace) is correctly rejected with a terminal error.(Note: the above is derived from reading the code paths, not from a cluster run — the call sites take the namespace verbatim and there is no validation between them and the
Patch. Happy to confirm on a kind cluster if useful.)Expected outcome
With
--enable-cross-namespace=false, a write to a Secret in a namespace other than the resource's own should be refused with a terminal error, the same as a read. With the flag enabled (Phase 1 default), it should proceed and set theACK.Advisory/CrossNamespaceOptInRequireddeprecation notice, so operators get the same visibility they get for references and field exports before the default flips.Suggested fix: move the check into
WriteToSecretin the runtime, rather than repeating it in every hook that exports a value. That covers all three ACM/ACMPCA paths and any future controller that writes to a Secret, and it keeps the write path from drifting from the read path again. Validating the target Secret'stypethere too would close the matching gap (today a write into a non-OpaqueSecret succeeds and the next read of it fails).Because this affects a security control rather than an ordinary bug, it would be worth confirming whether the fix should land before the default flips to
false— otherwise flipping the default silently leaves these three export paths unchanged.Environment
WriteToSecret)v0.60.0and later (v0.63.0current);--enable-cross-namespacewas introduced inv0.60.0acmv1.4.1+ (latestv1.8.1),acmpcav1.3.1+ (latestv1.5.0)