Skip to content

Use the X-Forwarded-For address in response logs if set.#881

Open
rolandjitsu wants to merge 1 commit intobuchgr:masterfrom
mujin:master
Open

Use the X-Forwarded-For address in response logs if set.#881
rolandjitsu wants to merge 1 commit intobuchgr:masterfrom
mujin:master

Conversation

@rolandjitsu
Copy link
Copy Markdown

When running on k8s and using a proxy such as ingress-nginx, the printed host in logs is not the user's IP. This patch will read the X-Forwarded-For header and use that instead of the request r.RemoteAddr if it's set.

@rolandjitsu rolandjitsu force-pushed the master branch 7 times, most recently from 26f7162 to dc3c477 Compare January 15, 2026 06:10
Comment thread docker/push_to_dockerhub
@mostynb
Copy link
Copy Markdown
Collaborator

mostynb commented Jan 15, 2026

Hi, this looks like two different changes in one PR. Could you drop the container image changes from this PR and create a new PR for that?

@rolandjitsu
Copy link
Copy Markdown
Author

Hi, this looks like two different changes in one PR. Could you drop the container image changes from this PR and create a new PR for that?

Will do.

@rolandjitsu
Copy link
Copy Markdown
Author

Hi, this looks like two different changes in one PR. Could you drop the container image changes from this PR and create a new PR for that?

Moved the change to #882.

Comment thread server/http.go
if xff := r.Header.Get("X-Forwarded-For"); xff != "" {
// XFF format: client, proxy1, proxy2
parts := strings.Split(xff, ",")
remoteAddr = strings.TrimSpace(parts[0])
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Since this is untrustworthy data, I wonder if we should log it differently?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Let me know what you recommend. The input could be sanitized before logging.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

How about if the X-Forwarded-For header is set, instead of logging r.RemoteAddr by itself we log r.RemoteAddr (X-Forwarded-For?) with a question mark to indicate that the value is uncertain?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants