OCPBUGS-122075: don't log authorization header

[simonpasquier]

This commit refactors the parsing of the authorization header to avoid the exposure of the header value in the logs.

[openshift-merge-bot]

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

[openshift-ci-robot]

@simonpasquier: This pull request references Jira Issue OCPBUGS-122075, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target version (5.1.0) matches configured target version for branch (5.1.0)
• bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

This commit refactors the parsing of the authorization header to avoid the exposure of the header value in the logs.

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 openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: simonpasquier

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:

• OWNERS [simonpasquier]

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

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 11e6ed13-0b47-4029-a00a-acf3c78df7d2

📥 Commits

Reviewing files that changed from the base of the PR and between 6387f64 and 74efd6f.

📒 Files selected for processing (1)

• pkg/authorize/handler.go

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.

---

📝 Walkthrough

Walkthrough

The authorization handler now requires exactly two whitespace-separated header fields before checking the scheme. It returns distinct 401 Unauthorized responses and debug reasons for malformed headers and unsupported schemes. Bearer scheme matching remains case-insensitive.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 74efd

Authorization header parsing now rejects invalid formats without exposing header values in logs. No current merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (14 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the primary change: preventing authorization header values from being logged. It is concise and specific.
Description check	✅ Passed	The description directly explains that authorization-header parsing was refactored to prevent header values from appearing in logs.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	The pull request changes only pkg/authorize/handler.go. The authoritative diff contains no test files, Ginkgo declarations, or test-title changes. Therefore, it introduces no unstable or overly spec…
Test Structure And Quality	✅ Passed	PASS: The pull request changes only pkg/authorize/handler.go (+7/-7) and changes no test-like file. The patch contains no Ginkgo It, setup, cleanup, wait, or assertion code. Therefore, the listed …
Microshift Test Compatibility	✅ Passed	The pull request changes only pkg/authorize/handler.go. The diff adds no Ginkgo e2e tests or test declarations such as It, Describe, Context, or When. Therefore, the MicroShift test compatib…
Single Node Openshift (Sno) Test Compatibility	✅ Passed	PASS: The pull request changes only pkg/authorize/handler.go. The authoritative diff adds no Ginkgo e2e tests, test files, or It(), Describe(), Context(), or When() constructs. Therefore, th…
Topology-Aware Scheduling Compatibility	✅ Passed	PASS. The pull request changes only authorization-header parsing in pkg/authorize/handler.go. The diff adds no deployment manifests, operator/controller scheduling logic, replicas, affinity, topolog…
Ote Binary Stdout Contract	✅ Passed	PASS. The pull request changes only pkg/authorize/handler.go. The changed code runs inside an HTTP request handler and contains no fmt.Print*, log.Print*, os.Stdout, or other stdout write. Its…
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	The pull request changes only pkg/authorize/handler.go. The authoritative diff adds no Ginkgo tests and introduces no IPv4 assumptions or external connectivity requirements. The custom check is ther…
No-Weak-Crypto	✅ Passed	The pull request changes only pkg/authorize/handler.go and replaces header parsing with strings.Fields. The changed code adds no MD5, SHA1, DES, 3DES, RC4, Blowfish, or ECB usage, and it adds no c…
Container-Privileges	✅ Passed	PASS: The pull request changes only pkg/authorize/handler.go. The diff adds no container or Kubernetes manifest changes and no added lines contain privileged, hostPID, hostNetwork, hostIPC, …
No-Sensitive-Data-In-Logs	✅ Passed	The pull request removes sensitive authorization-header values from the changed log paths. The base code logged auth[0] and the full auth slice. The head code logs only the field count and fixed r…

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

Some tools did not complete. Review the errors below.

🔧 golangci-lint (2.13.2)

level=error msg="Running error: context loading failed: failed to load packages: failed to load packages: failed to load with go/packages: err: exit status 1: stderr: go: inconsistent vendoring in :\n\tgithub.com/bradfitz/gomemcache@v0.0.0-20230905024940-24af94b03874: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/coreos/go-oidc/v3@v3.9.0: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/efficientgo/core@v1.0.0-rc.2: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/go-chi/chi@v4.0.4+incompatible: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/go-jose/go-jose/v3@v3.0.5: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/go-kit/log@v0.2.1: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/gogo/protobuf@v

... [truncated 5756 characters] ...

org/x/text@v0.37.0: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgoogle.golang.org/genproto/googleapis/api@v0.0.0-20260526163538-3dc84a4a5aaa: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgoogle.golang.org/genproto/googleapis/rpc@v0.0.0-20260526163538-3dc84a4a5aaa: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgoogle.golang.org/grpc@v1.81.1: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgoogle.golang.org/protobuf@v1.36.11: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\n\tTo ignore the vendor directory, use -mod=readonly or -mod=mod.\n\tTo sync the vendor directory, run:\n\t\tgo mod vendor\n"

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

pkg/authorize/handler.go (1)

28-28: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

Handle errors from level.Debug(logger).Log.

Each of the three calls discards the returned error. Handle these errors according to the repository's Go error-handling rule.

Source: Path instructions

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 76579ecf-7af5-484f-b83a-127eabafa6b8

📥 Commits

Reviewing files that changed from the base of the PR and between a47a32b and 6387f64.

📒 Files selected for processing (1)

• pkg/authorize/handler.go

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Reject headers with extra parts before authorization.

SplitN(..., 2) accepts Bearer token extra as two entries. It passes token extra to AuthorizeClient instead of returning the malformed-header response. Use strings.Fields and require exactly two fields.

Proposed fix

-		auth := strings.SplitN(req.Header.Get("Authorization"), " ", 2)
+		auth := strings.Fields(req.Header.Get("Authorization"))

[simonpasquier]

/cc @jan--f

[simonpasquier]

/verified by CI
/pipeline required

[openshift-merge-bot]

Scheduling required tests:
/test e2e-aws-ovn
/test e2e-aws-upgrade

[openshift-ci-robot]

@simonpasquier: This PR has been marked as verified by CI.

Details

In response to this:

/verified by CI
/pipeline required

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 openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

@simonpasquier: all tests passed!

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.