Skip to content

Add configurable IP forward header for Varnish purge ACL#114

Open
peterjaap with Copilot wants to merge 4 commits into
masterfrom
copilot/add-ip-forward-header-field
Open

Add configurable IP forward header for Varnish purge ACL#114
peterjaap with Copilot wants to merge 4 commits into
masterfrom
copilot/add-ip-forward-header-field

Conversation

Copilot AI commented Nov 20, 2025

Copy link
Copy Markdown
Contributor

Adds backend configuration for specifying which HTTP header (e.g., X-Forwarded-For, X-Real-IP) to use when checking purge request authorization against the ACL.

Changes

Configuration

  • Added ip_forward_header field in admin panel (defaults to X-Forwarded-For)
  • Added getIpForwardHeader() method to Model/Config.php

VCL Template
Updated purge ACL checks to use configurable header with first-IP extraction:

# Before
if (client.ip ~ purge) {

# After  
if (std.ip(regsub(req.http.{{var ip_forward_header}}, "^([^,]+),?.*$", "\1"), client.ip) ~ purge) {

The regsub() extracts the first IP from comma-separated lists (common when requests traverse multiple proxies), falling back to client.ip if header is absent or unparseable.

Testing

  • Added ip_forward_header.vtc test case
  • Updated test parser with default value
Original prompt

This section details on the original issue you should resolve

<issue_title>Add backend field for IP Forward Header</issue_title>
<issue_description>For example X-Forwarded-For or X-Real-IP.

X-Forwarded-For should be the default.

And update the VCL to use that var instead of if (client.ip ~ purge) { in vcl_recv;

if (std.ip({{var ip_forwarded_header}}, client.ip) ~ purge) {

</issue_description>

<agent_instructions>The field should be added in etc/adminhtml/system.xml

With the default setting in etc/config.xml

The etc/varnish6.vcl should then be updated with the change mentioned in the issue.

The Model/Config.php should have a method for it, like already exists for the sslOffloadedHeader.

It should be added to the Console/Command/GenerateVclCommand.php file, like sslOffloadedHeader is.

It should also be added to Model/Varnish/VCLGenerator.php.</agent_instructions>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 3 commits November 20, 2025 14:31
- Added ip_forward_header field in etc/adminhtml/system.xml
- Set default value X-Forwarded-For in etc/config.xml
- Added getIpForwardHeader() method to Model/Config.php
- Updated Console/Command/GenerateVclCommand.php to include ipForwardHeader
- Updated Model/Varnish/VCLGenerator.php to include ip_forward_header variable
- Updated etc/varnish6.vcl to use std.ip() with the configurable header

Co-authored-by: peterjaap <431360+peterjaap@users.noreply.github.com>
Co-authored-by: peterjaap <431360+peterjaap@users.noreply.github.com>
- Modified etc/varnish6.vcl to use regsub to extract first IP from comma-separated list
- Added test case to verify IP forward header functionality
- All existing tests still pass

Co-authored-by: peterjaap <431360+peterjaap@users.noreply.github.com>
@peterjaap

Copy link
Copy Markdown
Contributor

@toonvd did it get it right in one go?!

I got mad prompt engineering skills.

@peterjaap peterjaap requested a review from toonvd November 20, 2025 14:40
Copilot AI changed the title [WIP] Add backend field for IP Forward Header Add configurable IP forward header for Varnish purge ACL Nov 20, 2025
Copilot AI requested a review from peterjaap November 20, 2025 14:41
@peterjaap peterjaap marked this pull request as ready for review November 20, 2025 14:42
# Set up ACL with a single whitelisted IP that is not the test client
shell {
export ACCESS_LIST="whitelisted"
export WHITELISTED_IP="10.0.0.1"

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.

export SERVER1_IP="10.0.0.1"

# Generate the VCL file based on included variables and write it to output.vcl
# Set up ACL with a single whitelisted IP that is not the test client
shell {
export ACCESS_LIST="whitelisted"

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.

should not be set as SERVER1_IP overrides it

@toonvd

toonvd commented Nov 20, 2025

Copy link
Copy Markdown
Collaborator

@peterjaap looks good, I have to add that this can be solved without a regex with Varnish in proxy mode.
https://info.varnish-software.com/blog/failure-to-purge-a-story-about-client.ip-and-proxies

@peterjaap

peterjaap commented Nov 20, 2025

Copy link
Copy Markdown
Contributor

@peterjaap looks good, I have to add that this can be solved without a regex with Varnish in proxy mode. https://info.varnish-software.com/blog/failure-to-purge-a-story-about-client.ip-and-proxies

Huh, just noticed Guillaime also uses the 0.0.0.0 as a fallback there?

@toonvd

toonvd commented Dec 2, 2025

Copy link
Copy Markdown
Collaborator

In Belgium we say, if Guillaume jumps from a bridge... :3

@tdgroot

tdgroot commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

I like this change. @toonvd is right, but I think this is also a good option to have.

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.

Add backend field for IP Forward Header

4 participants