Skip to content

jellarr: set known proxy for nginx#184

Merged
booxter merged 1 commit intomasterfrom
jellarr-proxy-setting
Mar 4, 2026
Merged

jellarr: set known proxy for nginx#184
booxter merged 1 commit intomasterfrom
jellarr-proxy-setting

Conversation

@booxter
Copy link
Owner

@booxter booxter commented Mar 3, 2026

No description provided.

Copilot AI review requested due to automatic review settings March 3, 2026 00:58
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the jellarr NixOS service configuration to trust an nginx reverse proxy when determining the real client IP.

Changes:

  • Adds a knownProxies allow-list entry for localhost (127.0.0.1) so proxied requests can be trusted.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

enabled = true;
}
];
knownProxies = [ "127.0.0.1" ];
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

This only trusts the IPv4 loopback. If nginx connects to the upstream over IPv6 (source ::1), the proxy may not be recognized and client IP handling can be wrong. Consider including \"::1\" as well (or using a CIDR that covers both loopback families, if supported by this option).

Suggested change
knownProxies = [ "127.0.0.1" ];
knownProxies = [ "127.0.0.1" "::1" ];

Copilot uses AI. Check for mistakes.
Copilot AI review requested due to automatic review settings March 3, 2026 01:34
@booxter booxter force-pushed the jellarr-proxy-setting branch from eb25dfb to 1b3c7aa Compare March 3, 2026 01:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 2 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

};
};
network = {
knownProxies = [ "127.0.0.1" ];
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

If nginx connects to the backend over IPv6 loopback, requests will originate from ::1 rather than 127.0.0.1, and the proxy headers may not be trusted as intended. Consider including \"::1\" in knownProxies as well (or otherwise ensure nginx always uses IPv4 loopback).

Suggested change
knownProxies = [ "127.0.0.1" ];
knownProxies = [ "127.0.0.1" "::1" ];

Copilot uses AI. Check for mistakes.
processThreads = 10;
};
};
network = {
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

This introduces a security-relevant trust boundary (which sources are allowed to supply forwarded client IP headers). Add a short comment explaining why localhost is correct here (e.g., nginx is colocated and the backend is only reachable from localhost), to prevent future changes (like exposing the port or running behind a different proxy) from silently relying on an outdated assumption.

Suggested change
network = {
network = {
# Only trust X-Forwarded-For from the local reverse proxy (nginx),
# which is colocated on this host and reaches Jellyfin only via localhost.

Copilot uses AI. Check for mistakes.
@booxter booxter merged commit 71c52aa into master Mar 4, 2026
28 checks passed
@booxter booxter deleted the jellarr-proxy-setting branch March 4, 2026 01:54
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