Skip to content

OCM-22871 | fix: update Go version to 1.25.8 in order to fix#3229

Draft
olucasfreitas wants to merge 1 commit intoopenshift:masterfrom
olucasfreitas:OCM-22871
Draft

OCM-22871 | fix: update Go version to 1.25.8 in order to fix#3229
olucasfreitas wants to merge 1 commit intoopenshift:masterfrom
olucasfreitas:OCM-22871

Conversation

@olucasfreitas
Copy link
Copy Markdown
Contributor

@olucasfreitas olucasfreitas commented Mar 31, 2026

PR Summary

Remediate CVE-2026-25679 (incorrect parsing of IPv6 host literals in net/url) by upgrading ROSA to Go 1.25.8 and hardening URL parsing.

Detailed Description of the Issue

Go's net/url.Parse insufficiently validated the host/authority component and accepted some invalid URLs by treating garbage before an IPv6 literal as ignorable (for example, http://example.com[::1]:8080 was accepted instead of rejected). This affects Parse, ParseRequestURI, JoinPath, URL.Parse, and URL.UnmarshalBinary.

Upstream fixed this in Go 1.25.8 and Go 1.26.1. This PR now targets Go 1.25.8 specifically because it fixes the net/url issue tracked in OCM-22871 while avoiding OCM-22870 / CVE-2026-27137, which upstream states only affects Go 1.26.

ROSA CLI uses url.Parse and url.ParseRequestURI extensively for validating user-supplied URLs (OIDC endpoints, proxy URLs, IDP issuer URLs, gateway URLs). An attacker could potentially craft a malformed IPv6 URL that bypasses validation.

Related Issues and PRs

Type of Change

  • fix - resolves an incorrect behavior or bug.

Previous Behavior

  • go.mod declared a vulnerable Go version line for the net/url issue.
  • Production code called net/url.Parse / ParseRequestURI directly without additional host validation.
  • cmd/dlt/oidcprovider/cmd.go silently ignored URL parse errors, risking nil pointer dereferences.
  • Builder Dockerfiles used floating go-toolset:latest tags with GOTOOLCHAIN=auto, which could make container builds depend on toolchain downloads.

Behavior After This Change

  • ROSA now targets Go 1.25.8, which includes the upstream net/url fix for CVE-2026-25679.
  • This avoids moving ROSA onto the Go 1.26 line, which is the only affected line for CVE-2026-27137.
  • Production URL parsing goes through pkg/helper/url.Parse / ParseRequestURI wrappers that independently validate IPv6 host literal placement.
  • URL parse errors in cmd/dlt/oidcprovider/cmd.go are now reported and cause the command to exit.
  • UBI builder images are pinned to registry.access.redhat.com/ubi9/go-toolset:1.25.8, so container builds no longer depend on GOTOOLCHAIN=auto inside those Dockerfiles.

How to Test (Step-by-Step)

  1. make lint
  2. make test
  3. make rosa
  4. goreleaser-v2.15.1 check --config .goreleaser.yaml
  5. podman run --rm registry.access.redhat.com/ubi9/go-toolset:1.25.8 go version
  6. podman build -f Dockerfile .
  7. podman build -f images/Dockerfile.konflux .

Expected Results

All commands above exit 0. images/Dockerfile.e2e still requires authenticated access to registry.ci.openshift.org for a local build.

Breaking Changes

  • No breaking changes

Developer Verification Checklist

  • Commit subject/title follows [JIRA-TICKET] | [TYPE]: <MESSAGE>.
  • PR description clearly explains both what changed and why.
  • Relevant Jira/GitHub issues and related PRs are linked.
  • Tests were added/updated where appropriate.
  • I manually tested the change.
  • make test passes.
  • make lint passes.
  • make rosa passes.
  • Documentation was added/updated where appropriate.
  • Any risk, limitation, or follow-up work is documented.

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci bot commented Mar 31, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: olucasfreitas

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 31, 2026
@olucasfreitas olucasfreitas marked this pull request as draft March 31, 2026 17:29
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 31, 2026
.golangci.yml Outdated
version: "2"
run:
go: "1.24"
go: "1.26"

This comment was marked as resolved.

CLAUDE.md Outdated

### Dependencies and Modules
- Go 1.24.0 minimum version
- Go 1.26.1 minimum version

This comment was marked as resolved.

@olucasfreitas olucasfreitas marked this pull request as ready for review March 31, 2026 17:48
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 31, 2026
@olucasfreitas
Copy link
Copy Markdown
Contributor Author

/retest

Dockerfile Outdated
USER root
COPY . .

ENV GOTOOLCHAIN=auto
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the base image’s Go is older than the version in go.mod, GOTOOLCHAIN=auto lets the Go command download and use the required toolchain. That implies internet access during docker build. For Konflux, do our build tasks allow that egress for the official Go toolchain download?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think this is a risk since the pipeline can fail due lack of network (image Go version < go.mod) if a newer version is required or (image Go verion > go.mod) silently build with a newer version since go.mod sets the minimum.
Can we change the FROM to a pinned image version?
Similar:
https://github.com/openshift/cluster-ingress-operator/blob/master/Dockerfile

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated this direction in the latest push. The branch now targets Go 1.25.8 and pins the UBI builder images to registry.access.redhat.com/ubi9/go-toolset:1.25.8, so the container builds no longer rely on GOTOOLCHAIN=auto inside those Dockerfiles.

That also lines up better with the security tickets: upstream fixed CVE-2026-25679 in Go 1.25.8 and 1.26.1, while CVE-2026-27137 / OCM-22870 only affects Go 1.26, so staying on the latest 1.25.x line fixes the tracked net/url issue without taking on the separate Go 1.26-only vuln.

Dockerfile Outdated
@@ -1,6 +1,8 @@
FROM registry.access.redhat.com/ubi9/go-toolset:latest AS builder
USER root
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why root required?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No longer required in the current branch. I removed USER root and switched the builder stage to COPY --chown=1001:0 . ., which still lets make release succeed. The builder image is now also pinned to registry.access.redhat.com/ubi9/go-toolset:1.25.8.

@olucasfreitas
Copy link
Copy Markdown
Contributor Author

Follow-up update:

  • addressed the review feedback to keep go.mod as the source of truth by removing the explicit Go version from .golangci.yml and changing CLAUDE.md to reference the go directive instead
  • removed USER root from the main Dockerfile; the builder now uses COPY --chown=1001:0 . ., which still lets make release succeed locally
  • updated images/Dockerfile.e2e to use the existing rhel-9-golang-1.25-openshift-4.21 CI builder stream while keeping GOTOOLCHAIN=auto

The remaining red Prow jobs are blocked on external CI config rather than the repo diff itself. The earlier openshift/release bump landed on a 1.26 builder tag that does not exist in CI, so I opened a follow-up draft PR to correct that and force GOTOOLCHAIN=auto in the Go-based ROSA jobs:

Local verification after the follow-up commit:

  • make lint
  • make test
  • podman build -f Dockerfile .

podman build -f images/Dockerfile.e2e . now reaches the existing 1.25 builder tag locally and only stops on local auth to registry.ci.openshift.org, rather than manifest unknown for a nonexistent tag.

@@ -18,6 +18,7 @@ package url

import (
"fmt"
neturl "net/url"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no blocker: other packages use the file name helpers.go and helpers_test.go. Might be interesting follow same pattern.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aligned this in the latest push. The files are now pkg/helper/url/helpers.go and pkg/helper/url/helpers_test.go to match the helper naming pattern used elsewhere in the repo.

@olucasfreitas olucasfreitas marked this pull request as draft April 1, 2026 17:08
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 1, 2026
@olucasfreitas olucasfreitas changed the title OCM-22871 | chore: update Go version to 1.26.1 in order to fix OCM-22871 | fix: update Go version to 1.25.8 in order to fix Apr 1, 2026
@olucasfreitas olucasfreitas marked this pull request as ready for review April 2, 2026 12:00
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 2, 2026
@olucasfreitas
Copy link
Copy Markdown
Contributor Author

/retest-required

@olucasfreitas olucasfreitas marked this pull request as draft April 2, 2026 12:22
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 2, 2026
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci bot commented Apr 2, 2026

@olucasfreitas: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/build d0efcd7 link true /test build
ci/prow/test d0efcd7 link true /test test

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants