Skip to content

CLEANUP/MINOR: annotations: skip empty elements in allow-list and deny-list - #849

Merged
haproxyci merged 1 commit into
haproxytech:masterfrom
locker95:fix-accesslist-empty-elements
Sep 10, 2026
Merged

haproxyci merged 1 commit into
haproxytech:masterfrom
locker95:fix-accesslist-empty-elements

Conversation

@locker95

@locker95 locker95 commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

A trailing comma in the allow-list annotation, e.g. haproxy.org/allow-list: "10.0.0.0/16,", makes strings.Split produce an empty element. net.ParseCIDR("") then fails and the whole annotation is discarded with incorrect address '' in allow-list annotation, so the ingress silently ends up open to all traffic instead of being restricted. The same applies to deny-list and the deprecated whitelist/blacklist names.

This patch skips elements that are empty after trimming, so trailing commas, consecutive commas or stray whitespace no longer invalidate the list. A value with no valid element at all is still rejected as before: adding the rule with an empty map would flip an allow-list into denying everyone, which is just as surprising as allowing everyone.

Added a table-driven test for AccessControl.Process covering trailing/consecutive commas, whitespace-only elements, the deprecated whitelist name, deny-list, pattern files and the existing error paths.

Fixes #848

Comment thread pkg/annotations/ingress/accessControl.go
@locker95

locker95 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

switched the split to FieldsFunc so trailing/double commas drop out. still trim spaces so 1.2.3.4, 5.6.7.8 works.

@oktalz
oktalz force-pushed the fix-accesslist-empty-elements branch from 0b722a6 to b52a720 Compare September 9, 2026 14:34
…y-list

A trailing comma in an allow-list annotation makes strings.Split
produce an empty element, net.ParseCIDR("") fails and the whole
annotation is discarded, leaving the ingress open to all traffic.
Split the value with strings.FieldsFunc so consecutive or trailing
commas yield no element, and skip elements that are blank after
trimming. A value with no valid element at all is still rejected:
adding the rule with an empty map would flip an allow-list into
denying everyone.

Signed-off-by: Dean Chen <862469039@qq.com>
@oktalz
oktalz force-pushed the fix-accesslist-empty-elements branch from b52a720 to 8506feb Compare September 9, 2026 14:53
@oktalz oktalz changed the title BUG/MINOR: annotations: skip empty elements in allow-list and deny-list CLEANUP/MINOR: annotations: skip empty elements in allow-list and deny-list Sep 9, 2026
@oktalz

oktalz commented Sep 9, 2026

Copy link
Copy Markdown
Member

as mentioned in #848 I needed to change overall commit message I hope that change is fine by you ?

I pushed the change already on a PR

@locker95 is that OK for you?

@locker95

locker95 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

yeah that's fine, thanks for fixing it up.

@haproxyci
haproxyci merged commit 96f153c into haproxytech:master Sep 10, 2026
8 checks passed
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.

Trailing comma in haproxy.org/allow-list results in access being allowed from anywhere

3 participants