[DNM] fix(security): backport v0.48.0 fixes to v0.43.x#2760
Conversation
Validate GitHub App webhook payloads before minting tokens and avoid trusting a forged enterprise host header. Also include the upstream App token scoping and remote-resource deep-copy fixes needed for the same supported release stream. Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
Download the branch-compatible golangci-lint binary into tmp and use it from make lint-go. Pin GOTOOLCHAIN for local make targets and PAC Go steps so linting does not drift with the host Go installation. Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
Signed-off-by: Shubham Bhardwaj <shubbhar@redhat.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several security and caching improvements, including webhook signature and enterprise host validation before minting GitHub App tokens, scoping of App tokens, and deep-copying of cached remote pipelines and tasks to prevent mutation leaks. It also updates various dependencies and build configurations. However, several critical issues were identified: the Makefile and Tekton workflows reference non-existent versions of Go (1.25.0) and golangci-lint (v2.12.2), which will cause build failures; comparing URL Host fields directly during enterprise host validation can fail due to port mismatches and should use Hostname(); and a potential nil pointer dereference panic exists in the remote resource resolver if a referenced pipeline is not found before calling DeepCopy().
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| GOLANGCI_LINT_VERSION ?= v2.12.2 | ||
| GO_TOOLCHAIN ?= go1.25.0 |
There was a problem hiding this comment.
The pinned versions for GOLANGCI_LINT_VERSION (v2.12.2) and GO_TOOLCHAIN (go1.25.0) do not exist. golangci-lint does not have a v2.x release (it is currently on v1.x), and Go 1.25.0 is not a released version of Go. Using these non-existent versions will cause local builds and linter runs to fail.
GOLANGCI_LINT_VERSION ?= v1.62.2
GO_TOOLCHAIN ?= go1.23.0
| ./codecov -P $GITHUB_PULL_REQUEST_ID -C {{revision}} -v | ||
| - name: lint | ||
| image: docker.io/golangci/golangci-lint:v2.10.1 | ||
| image: docker.io/golang:1.25 |
There was a problem hiding this comment.
The Docker image docker.io/golang:1.25 does not exist because Go 1.25 is not a released version. This will cause the Tekton lint task to fail to pull the image and run. Please use a valid, released Go version image such as docker.io/golang:1.23.
image: docker.io/golang:1.23| if !strings.EqualFold(enterpriseURL.Host, repoURL.Host) { | ||
| return fmt.Errorf("github enterprise host %q does not match repository host %q", enterpriseURL.Host, repoURL.Host) | ||
| } |
There was a problem hiding this comment.
Comparing enterpriseURL.Host and repoURL.Host directly can cause false validation failures if one of the URLs contains a port number (e.g., github.company.com:8443) and the other does not. Using Hostname() instead of Host ensures that only the hostnames are compared, ignoring any port differences.
| if !strings.EqualFold(enterpriseURL.Host, repoURL.Host) { | |
| return fmt.Errorf("github enterprise host %q does not match repository host %q", enterpriseURL.Host, repoURL.Host) | |
| } | |
| if !strings.EqualFold(enterpriseURL.Hostname(), repoURL.Hostname()) { | |
| return fmt.Errorf("github enterprise host %q does not match repository host %q", enterpriseURL.Hostname(), repoURL.Hostname()) | |
| } |
| // if PipelineRef is used then, first resolve pipeline and replace all taskRef{Finally/Task} of Pipeline, then put inlinePipeline in PipelineRun | ||
| if pipelinerun.Spec.PipelineRef != nil && pipelinerun.Spec.PipelineRef.Resolver == "" { | ||
| pipelineResolved := fetchedResourcesForPipelineRun.Pipeline | ||
| pipelineResolved := fetchedResourcesForPipelineRun.Pipeline.DeepCopy() |
There was a problem hiding this comment.
If fetchedResourcesForPipelineRun.Pipeline is nil (which can happen if the referenced pipeline is not found in annotations or local resources), calling DeepCopy() on it will cause a nil pointer dereference panic, crashing the controller. A nil check should be added before calling DeepCopy().
if fetchedResourcesForPipelineRun.Pipeline == nil {
return nil, fmt.Errorf("pipeline %s not found", pipelinerun.Spec.PipelineRef.Name)
}
pipelineResolved := fetchedResourcesForPipelineRun.Pipeline.DeepCopy()2 participants
📝 Description of the Change
[DNM] Backport of the security fixes released in v0.48.0 to release-v0.43.x.
This PR backports the public security hardening from v0.48.0:
?secret=values are not written to stdout.How this backport was done:
release-v0.43.xin a dedicated worktree.make lint-gouse the same known-good linter binary fromtmp/.go-jose/v3,go-jose/v4, andtektoncd/pipeline.👨🏻 Linked Jira
N/A
🔗 Linked GitHub Issue
N/A
🧪 Testing Strategy
Validation run locally:
make testmake lint-gogo test ./pkg/adapterafter the incoming webhook log redaction backportgit diff --checkFull pre-push hooks were not used for this branch because
check-linksfailed on externaldocs.openshift.com403 responses unrelated to this backport.🤖 AI Assistance
If you have used AI assistance, please provide the following details:
Which LLM was used?
Extent of AI Assistance:
✅ Submitter Checklist
fix:,feat:) matches the "Type of Change" I selected above.make testandmake lintlocally to check for and fix anyissues. For an efficient workflow, I have considered installing
pre-commit and running
pre-commit installtoautomate these checks.