OCPBUGS-64943: Make GetUserFromRequestContext more nil safe #15711

TheRealJon · 2025-11-11T14:40:24Z

No description provided.

openshift-ci-robot · 2025-11-11T14:40:32Z

@TheRealJon: This pull request references Jira Issue OCPBUGS-64943, 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 (4.21.0) matches configured target version for branch (4.21.0)
bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @yapei

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

In response to this:

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.

Leo6Leo · 2025-11-14T16:05:33Z

/lgtm

openshift-ci · 2025-11-14T16:05:52Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Leo6Leo, TheRealJon

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~pkg/OWNERS~~ [TheRealJon]

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

Leo6Leo · 2025-11-17T14:58:18Z

QE verification
/assign @yapei

yapei · 2025-11-18T05:40:20Z

I'm still seeing the runtime errors

I'm on correct commit

$ git log -1
commit 23f29e60616a967be47b7f4315ded9b30e7d1e90 (HEAD -> OCPBUGS-64943, origin/OCPBUGS-64943)
Author: Jon Jackson <[email protected]>
Date:   Tue Nov 11 09:39:54 2025 -0500

    OCPBUGS-64943: Make GetUserFromRequestContext more nil safe

and running local bridge

$ ./clean.sh
$ ./build.sh
$ ./bin/bridge
W1118 13:38:04.321880   27042 authoptions.go:112] Flag inactivity-timeout is set to less then 300 seconds and will be ignored!
W1118 13:38:04.322957   27042 authoptions.go:265] running with AUTHENTICATION DISABLED -- for development use only!
I1118 13:38:04.324446   27042 main.go:779] Binding to 0.0.0.0:9000...
I1118 13:38:04.324451   27042 main.go:781] Not using TLS
I1118 13:38:07.325459   27042 metrics.go:133] serverconfig.Metrics: Update ConsolePlugin metrics...
I1118 13:38:08.067917   27042 metrics.go:143] serverconfig.Metrics: Update ConsolePlugin metrics: &map[monitoring:map[disabled:1] networking:map[disabled:1]] (took 742.380583ms)
I1118 13:38:09.324825   27042 metrics.go:80] usage.Metrics: Count console users...
I1118 13:38:10.067567   27042 metrics.go:156] usage.Metrics: Update console users metrics: 1 kubeadmin, 0 cluster-admins, 0 developers, 0 unknown/errors (took 742.6695ms)
2025/11/18 13:38:10 http: panic serving [::1]:56694: runtime error: invalid memory address or nil pointer dereference
goroutine 84 [running]:
net/http.(*conn).serve.func1()
	/opt/homebrew/Cellar/go/1.24.4/libexec/src/net/http/server.go:1947 +0xb0
panic({0x1059c4c80?, 0x107a56360?})
	/opt/homebrew/Cellar/go/1.24.4/libexec/src/runtime/panic.go:792 +0x124
net/http.(*Request).Context(...)
	/opt/homebrew/Cellar/go/1.24.4/libexec/src/net/http/request.go:353
github.com/openshift/console/pkg/auth.GetUserFromRequestContext(0x1400046ebd0?)
	/Users/yapei/go/src/github.com/openshift/TheRealJon-console/pkg/auth/user.go:12 +0x18
github.com/openshift/console/pkg/olm.(*OLMHandler).getK8sClientConfig(0x14000968930, 0x38?)
	/Users/yapei/go/src/github.com/openshift/TheRealJon-console/pkg/olm/utils.go:30 +0x24
github.com/openshift/console/pkg/olm.(*OLMHandler).getClientWithScheme(0x14000968930, 0x0)
	/Users/yapei/go/src/github.com/openshift/TheRealJon-console/pkg/olm/utils.go:49 +0x8c
github.com/openshift/console/pkg/olm.(*OLMHandler).checkPackageManifestHandler(0x14000968930, {0x105f1acd0, 0x140006d01c0}, 0x102f38dcc?)
	/Users/yapei/go/src/github.com/openshift/TheRealJon-console/pkg/olm/handler.go:112 +0x258
net/http.HandlerFunc.ServeHTTP(0x14000383bc0?, {0x105f1acd0?, 0x140006d01c0?}, 0x12fc44f08?)
	/opt/homebrew/Cellar/go/1.24.4/libexec/src/net/http/server.go:2294 +0x38
net/http.(*ServeMux).ServeHTTP(0x130?, {0x105f1acd0, 0x140006d01c0}, 0x14000572280)
	/opt/homebrew/Cellar/go/1.24.4/libexec/src/net/http/server.go:2822 +0x1b4
github.com/openshift/console/pkg/olm.(*OLMHandler).ServeHTTP(0x130?, {0x105f1acd0?, 0x140006d01c0?}, 0x10324cf48?)
	/Users/yapei/go/src/github.com/openshift/TheRealJon-console/pkg/olm/handler.go:42 +0x28
net/http.HandlerFunc.ServeHTTP(0x14000572000?, {0x105f1acd0?, 0x140006d01c0?}, 0x105eeea90?)
	/opt/homebrew/Cellar/go/1.24.4/libexec/src/net/http/server.go:2294 +0x38
github.com/openshift/console/pkg/server.(*Server).HTTPHandler.(*Server).HTTPHandler.func4.AuthMiddleware.func77({0x105f1acd0, 0x140006d01c0}, 0x14000572000)
	/Users/yapei/go/src/github.com/openshift/TheRealJon-console/pkg/middleware/middleware.go:33 +0x1a4
net/http.HandlerFunc.ServeHTTP(0x1400037ea00?, {0x105f1acd0?, 0x140006d01c0?}, 0x102f398ac?)
	/opt/homebrew/Cellar/go/1.24.4/libexec/src/net/http/server.go:2294 +0x38
github.com/openshift/console/pkg/server.(*Server).HTTPHandler.(*Server).HTTPHandler.func4.AuthMiddleware.(*CSRFVerifier).WithCSRFVerification.func112({0x105f1acd0, 0x140006d01c0}, 0x14000572000)
	/Users/yapei/go/src/github.com/openshift/TheRealJon-console/pkg/auth/csrfverifier/csrf.go:52 +0x90
net/http.HandlerFunc.ServeHTTP(0x140003826c0?, {0x105f1acd0?, 0x140006d01c0?}, 0xf?)
	/opt/homebrew/Cellar/go/1.24.4/libexec/src/net/http/server.go:2294 +0x38
net/http.(*ServeMux).ServeHTTP(0x14000a08270?, {0x105f1acd0, 0x140006d01c0}, 0x14000572000)
	/opt/homebrew/Cellar/go/1.24.4/libexec/src/net/http/server.go:2822 +0x1b4
github.com/openshift/console/pkg/server.(*Server).HTTPHandler.WithSecurityHeaders.func57({0x105f1acd0, 0x140006d01c0}, 0x14000572000)
	/Users/yapei/go/src/github.com/openshift/TheRealJon-console/pkg/middleware/middleware.go:131 +0x100
net/http.HandlerFunc.ServeHTTP(0x14000748020?, {0x105f1acd0?, 0x140006d01c0?}, 0x14000157b60?)
	/opt/homebrew/Cellar/go/1.24.4/libexec/src/net/http/server.go:2294 +0x38
net/http.serverHandler.ServeHTTP({0x1400090b440?}, {0x105f1acd0?, 0x140006d01c0?}, 0x6?)
	/opt/homebrew/Cellar/go/1.24.4/libexec/src/net/http/server.go:3301 +0xbc
net/http.(*conn).serve(0x140008fa000, {0x105f337b8, 0x14000969620})
	/opt/homebrew/Cellar/go/1.24.4/libexec/src/net/http/server.go:2102 +0x52c
created by net/http.(*Server).Serve in goroutine 1
	/opt/homebrew/Cellar/go/1.24.4/libexec/src/net/http/server.go:3454 +0x3d8
2025/11/18 13:38:10 http: panic serving [::1]:56716: runtime error: invalid memory address or nil pointer dereference
goroutine 199 [running]:
net/http.(*conn).serve.func1()
	/opt/homebrew/Cellar/go/1.24.4/libexec/src/net/http/server.go:1947 +0xb0
panic({0x1059c4c80?, 0x107a56360?})
	/opt/homebrew/Cellar/go/1.24.4/libexec/src/runtime/panic.go:792 +0x124
net/http.(*Request).Context(...)
	/opt/homebrew/Cellar/go/1.24.4/libexec/src/net/http/request.go:353
github.com/openshift/console/pkg/auth.GetUserFromRequestContext(0x1400046ebd0?)
	/Users/yapei/go/src/github.com/openshift/TheRealJon-console/pkg/auth/user.go:12 +0x18
github.com/openshift/console/pkg/olm.(*OLMHandler).getK8sClientConfig(0x14000968930, 0x38?)
	/Users/yapei/go/src/github.com/openshift/TheRealJon-console/pkg/olm/utils.go:30 +0x24
github.com/openshift/console/pkg/olm.(*OLMHandler).getClientWithScheme(0x14000968930, 0x0)
	/Users/yapei/go/src/github.com/openshift/TheRealJon-console/pkg/olm/utils.go:49 +0x8c
github.com/openshift/console/pkg/olm.(*OLMHandler).checkPackageManifestHandler(0x14000968930, {0x105f1acd0, 0x140006d0380}, 0x102f38dcc?)
	/Users/yapei/go/src/github.com/openshift/TheRealJon-console/pkg/olm/handler.go:112 +0x258
net/http.HandlerFunc.ServeHTTP(0x14000383bc0?, {0x105f1acd0?, 0x140006d0380?}, 0x12fc44f08?)
	/opt/homebrew/Cellar/go/1.24.4/libexec/src/net/http/server.go:2294 +0x38
net/http.(*ServeMux).ServeHTTP(0x130?, {0x105f1acd0, 0x140006d0380}, 0x140005723c0)
	/opt/homebrew/Cellar/go/1.24.4/libexec/src/net/http/server.go:2822 +0x1b4
github.com/openshift/console/pkg/olm.(*OLMHandler).ServeHTTP(0x130?, {0x105f1acd0?, 0x140006d0380?}, 0x10324cf48?)
	/Users/yapei/go/src/github.com/openshift/TheRealJon-console/pkg/olm/handler.go:42 +0x28
net/http.HandlerFunc.ServeHTTP(0x14001130000?, {0x105f1acd0?, 0x140006d0380?}, 0x105eeea90?)
	/opt/homebrew/Cellar/go/1.24.4/libexec/src/net/http/server.go:2294 +0x38
github.com/openshift/console/pkg/server.(*Server).HTTPHandler.(*Server).HTTPHandler.func4.AuthMiddleware.func77({0x105f1acd0, 0x140006d0380}, 0x14001130000)
	/Users/yapei/go/src/github.com/openshift/TheRealJon-console/pkg/middleware/middleware.go:33 +0x1a4
net/http.HandlerFunc.ServeHTTP(0x1400037ea00?, {0x105f1acd0?, 0x140006d0380?}, 0x102f398ac?)
	/opt/homebrew/Cellar/go/1.24.4/libexec/src/net/http/server.go:2294 +0x38
github.com/openshift/console/pkg/server.(*Server).HTTPHandler.(*Server).HTTPHandler.func4.AuthMiddleware.(*CSRFVerifier).WithCSRFVerification.func112({0x105f1acd0, 0x140006d0380}, 0x14001130000)
	/Users/yapei/go/src/github.com/openshift/TheRealJon-console/pkg/auth/csrfverifier/csrf.go:52 +0x90
net/http.HandlerFunc.ServeHTTP(0x140003826c0?, {0x105f1acd0?, 0x140006d0380?}, 0xf?)
	/opt/homebrew/Cellar/go/1.24.4/libexec/src/net/http/server.go:2294 +0x38
net/http.(*ServeMux).ServeHTTP(0x140009fdef0?, {0x105f1acd0, 0x140006d0380}, 0x14001130000)
	/opt/homebrew/Cellar/go/1.24.4/libexec/src/net/http/server.go:2822 +0x1b4
github.com/openshift/console/pkg/server.(*Server).HTTPHandler.WithSecurityHeaders.func57({0x105f1acd0, 0x140006d0380}, 0x14001130000)
	/Users/yapei/go/src/github.com/openshift/TheRealJon-console/pkg/middleware/middleware.go:131 +0x100
net/http.HandlerFunc.ServeHTTP(0x14000749120?, {0x105f1acd0?, 0x140006d0380?}, 0x14000868b60?)
	/opt/homebrew/Cellar/go/1.24.4/libexec/src/net/http/server.go:2294 +0x38
net/http.serverHandler.ServeHTTP({0x14000a16960?}, {0x105f1acd0?, 0x140006d0380?}, 0x6?)
	/opt/homebrew/Cellar/go/1.24.4/libexec/src/net/http/server.go:3301 +0xbc
net/http.(*conn).serve(0x1400097a000, {0x105f337b8, 0x14000969620})
	/opt/homebrew/Cellar/go/1.24.4/libexec/src/net/http/server.go:2102 +0x52c
created by net/http.(*Server).Serve in goroutine 1
	/opt/homebrew/Cellar/go/1.24.4/libexec/src/net/http/server.go:3454 +0x3d8
I1118 13:38:13.708751   27042 proxy.go:240] CheckOrigin: Proxy has no configured Origin. Allowing origin [http://localhost:9000] to wss://api.qe-uidaily-1118.qe.azure.devcluster.openshift.com:6443/apis/console.openshift.io/v1/consolenotifications?watch=true&resourceVersion=100667

Leo6Leo · 2025-11-19T21:21:45Z

2025/11/18 13:38:10 http: panic serving [::1]:56694: runtime error: invalid memory address or nil pointer dereference
goroutine 84 [running]:

Hello @yapei , I think this error is not caused by @TheRealJon 's PR, since I can reproduce it on other branches.

I have identified the issue, and opened a PR. #15746
PTAL, thanks

yapei · 2025-11-20T08:34:56Z

@Leo6Leo Is this nil pointer dereference error in https://issues.redhat.com//browse/OCPBUGS-64943 the same with https://issues.redhat.com//browse/OCPBUGS-65804? (they look pretty similar even same)

openshift-ci · 2025-11-20T19:33:12Z

New changes are detected. LGTM label has been removed.

coderabbitai · 2025-11-20T19:33:26Z

Walkthrough

Refactored GetUserFromRequestContext in pkg/auth/user.go to add explicit nil checks and stepwise context value retrieval, performing a safe type assertion and returning nil on any missing or mismatched context data. Signature unchanged.

Changes

Cohort / File(s)	Summary
Defensive context handling refactor `pkg/auth/user.go`	Rewrote `GetUserFromRequestContext` to: check for nil request context, fetch `UserContextKey` value explicitly, verify presence, perform a type assertion with an explicit ok check, and return nil on failure. Flow is more defensive while keeping the same function signature.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Single-file, single-function change with localized scope
Pay attention to the exact UserContextKey lookup and type assertion branch coverage
Verify there are no callers relying on previous implicit behaviors (though signature is unchanged)

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between c18e689 and 7baea63.

📒 Files selected for processing (1)

pkg/auth/user.go (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

pkg/auth/user.go

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

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 86c7b34 and c18e689.

📒 Files selected for processing (1)

pkg/auth/user.go (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

pkg/auth/user.go

🔇 Additional comments (2)

pkg/auth/user.go (2)

17-20: Good defensive check for missing context value.

The nil check for userValue properly handles the case where UserContextKey is not present in the context.

22-25: LGTM: Explicit type assertion prevents panics.

The explicit type assertion with the ok check safely handles cases where the context value is not of type *User, preventing potential panics from failed type assertions.

pkg/auth/user.go

TheRealJon · 2025-11-20T19:41:29Z

@yapei I believe the comment has been addressed, I'm not sure how we're ever getting nil passed into that function, but apparently it's happening!

Leo6Leo · 2025-11-20T20:18:50Z

Thanks for catching it @yapei, you are right! I didn't read deeper into that function. I figured that the call stack is:

checkPackageManifestHandler pass in a nil pointer and calls:

o.getClientWithScheme(nil) calls
- o.getK8sClientConfig(nil) calls
  - auth.GetUserFromRequestContext(nil), which is the function that Jon's improving.

The PR #15746 I opened only solves the surface checkPackageManifestHandler.

But that PR fix still apply, as nil pointer shouldn't be passed in checkPackageManifestHandler. I will update the corresponding jira and description of the PR.

openshift-ci · 2025-11-21T01:16:40Z

@TheRealJon: all tests passed!

Full PR test history. Your PR dashboard.

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.

openshift-ci-robot added jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. labels Nov 11, 2025

openshift-ci bot requested review from Leo6Leo, jhadvig and yapei November 11, 2025 14:40

openshift-ci bot added component/backend Related to backend approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 11, 2025

TheRealJon force-pushed the OCPBUGS-64943 branch from afff9b4 to 23f29e6 Compare November 13, 2025 14:49

openshift-ci bot assigned Leo6Leo Nov 14, 2025

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 14, 2025

openshift-ci bot assigned yapei Nov 17, 2025

Leo6Leo mentioned this pull request Nov 19, 2025

OCPBUGS-65804: Update client initialization in checkPackageManifestHandler #15746

Open

TheRealJon force-pushed the OCPBUGS-64943 branch from 23f29e6 to c18e689 Compare November 20, 2025 19:33

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 20, 2025

coderabbitai bot reviewed Nov 20, 2025

View reviewed changes

pkg/auth/user.go Outdated Show resolved Hide resolved

OCPBUGS-64943: Make GetUserFromRequestContext more nil safe

7baea63

TheRealJon force-pushed the OCPBUGS-64943 branch from c18e689 to 7baea63 Compare November 20, 2025 19:39

OCPBUGS-64943: Make GetUserFromRequestContext more nil safe #15711

Are you sure you want to change the base?

OCPBUGS-64943: Make GetUserFromRequestContext more nil safe #15711

Conversation

TheRealJon commented Nov 11, 2025

Uh oh!

openshift-ci-robot commented Nov 11, 2025

Uh oh!

Leo6Leo commented Nov 14, 2025

Uh oh!

openshift-ci bot commented Nov 14, 2025

Uh oh!

Leo6Leo commented Nov 17, 2025

Uh oh!

yapei commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Leo6Leo commented Nov 19, 2025

Uh oh!

yapei commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented Nov 20, 2025

Uh oh!

coderabbitai bot commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

TheRealJon commented Nov 20, 2025

Uh oh!

Leo6Leo commented Nov 20, 2025

Uh oh!

openshift-ci bot commented Nov 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

yapei commented Nov 18, 2025 •

edited

Loading

yapei commented Nov 20, 2025 •

edited

Loading

coderabbitai bot commented Nov 20, 2025 •

edited

Loading