Spotted while chasing #2955, which turned out to be a logging defect. This one is real, though it stops at the namespace boundary.
When CreateSecret returns AlreadyExists, we reuse whatever secret already carries that name:
https://github.com/tektoncd/pipelines-as-code/blob/main/pkg/reconciler/reconciler.go#L306-L318
ee9da52 added that branch for a genuine case: slow etcd makes the API server retry a create for the same PipelineRun. But it matches on the name alone. It never checks that the existing secret belongs to this PipelineRun or this Repository.
We then call UpdateSecretWithOwnerRef on it, which replaces the owner references wholesale:
https://github.com/tektoncd/pipelines-as-code/blob/main/pkg/kubeinteraction/secrets.go#L52
The first consequence is a run bound to a secret it does not own. Secret names are pac-gitauth- plus 6 random letters, lowercased by GenerateBasicAuthSecretName, so the space is 26^6. On a collision inside a namespace, a PipelineRun for repo A proceeds with the token minted for repo B and the only signal is a warning that carries the wrong context (#2955).
The second is worse: PaC can be talked into adopting an arbitrary secret. The reconciler acts on annotations that sit on the PipelineRun, so anyone who can create a PipelineRun in a namespace can write:
metadata:
annotations:
pipelinesascode.tekton.dev/repository: <an existing Repository CR in that namespace>
pipelinesascode.tekton.dev/git-auth-secret: <any existing secret in that namespace>
pipelinesascode.tekton.dev/secret-created: "false"CreateSecret fails with AlreadyExists, the reuse branch tolerates it, and UpdateSecretWithOwnerRef stamps that unrelated secret with an owner reference to the attacker's PipelineRun. Delete the PipelineRun and the secret gets garbage collected with it. CleanupPipelines deletes by the same annotation value:
https://github.com/tektoncd/pipelines-as-code/blob/main/pkg/kubeinteraction/cleanups.go#L52-L59
It needs create-PipelineRun rights in the target namespace, so no cross-tenant escape. Still a destructive primitive we hand out for free, and it applies to the PaC install namespace too wherever users can create PipelineRuns there.
Suggested fix
Reuse the existing secret only when it provably belongs to this PipelineRun or Repository: owner UID matching the PipelineRun, or matching pipelinesascode.tekton.dev/url-org, url-repository and sha annotations. That covers the etcd-retry case the branch was written for and rejects the rest, which should then fail closed with a Warning Event on the Repository CR instead of running with someone else's credential.
UpdateSecretWithOwnerRef should also stop flattening OwnerReferences, and refuse a secret already owned by another object.
While in there: GenerateBasicAuthSecretName lowercases the generated string, dropping the space from 52^6 to 26^6 for nothing.
Goes back at least to v0.48.1 (reconciler.go:226-238 and kubeinteraction/secrets.go:51 there).
Spotted while chasing #2955, which turned out to be a logging defect. This one is real, though it stops at the namespace boundary.
When
CreateSecretreturnsAlreadyExists, we reuse whatever secret already carries that name:https://github.com/tektoncd/pipelines-as-code/blob/main/pkg/reconciler/reconciler.go#L306-L318
ee9da52 added that branch for a genuine case: slow etcd makes the API server retry a create for the same PipelineRun. But it matches on the name alone. It never checks that the existing secret belongs to this PipelineRun or this Repository.
We then call
UpdateSecretWithOwnerRefon it, which replaces the owner references wholesale:https://github.com/tektoncd/pipelines-as-code/blob/main/pkg/kubeinteraction/secrets.go#L52
The first consequence is a run bound to a secret it does not own. Secret names are
pac-gitauth-plus 6 random letters, lowercased byGenerateBasicAuthSecretName, so the space is 26^6. On a collision inside a namespace, a PipelineRun for repo A proceeds with the token minted for repo B and the only signal is a warning that carries the wrong context (#2955).The second is worse: PaC can be talked into adopting an arbitrary secret. The reconciler acts on annotations that sit on the PipelineRun, so anyone who can create a PipelineRun in a namespace can write:
CreateSecretfails withAlreadyExists, the reuse branch tolerates it, andUpdateSecretWithOwnerRefstamps that unrelated secret with an owner reference to the attacker's PipelineRun. Delete the PipelineRun and the secret gets garbage collected with it.CleanupPipelinesdeletes by the same annotation value:https://github.com/tektoncd/pipelines-as-code/blob/main/pkg/kubeinteraction/cleanups.go#L52-L59
It needs create-PipelineRun rights in the target namespace, so no cross-tenant escape. Still a destructive primitive we hand out for free, and it applies to the PaC install namespace too wherever users can create PipelineRuns there.
Suggested fix
Reuse the existing secret only when it provably belongs to this PipelineRun or Repository: owner UID matching the PipelineRun, or matching
pipelinesascode.tekton.dev/url-org,url-repositoryandshaannotations. That covers the etcd-retry case the branch was written for and rejects the rest, which should then fail closed with a Warning Event on the Repository CR instead of running with someone else's credential.UpdateSecretWithOwnerRefshould also stop flatteningOwnerReferences, and refuse a secret already owned by another object.While in there:
GenerateBasicAuthSecretNamelowercases the generated string, dropping the space from 52^6 to 26^6 for nothing.Goes back at least to v0.48.1 (reconciler.go:226-238 and kubeinteraction/secrets.go:51 there).