Skip to content

fix(NPC,LBC): harden network policy and load balancer controllers#2042

Open
Aprazor wants to merge 2 commits intocloudnativelabs:masterfrom
Aprazor:fix/npc-lbc-harden-input-validation
Open

fix(NPC,LBC): harden network policy and load balancer controllers#2042
Aprazor wants to merge 2 commits intocloudnativelabs:masterfrom
Aprazor:fix/npc-lbc-harden-input-validation

Conversation

@Aprazor
Copy link
Copy Markdown

@Aprazor Aprazor commented Mar 23, 2026

What type of PR is this?

bug

What this PR does / why we need it:

Two defensive fixes (per @aauren's consolidation request on #2020):

  1. LBC allocateService() nil panic: When a service already has all requested IPs, err.Error() was called on a nil error. Added early return and safe per-family error collection.
  2. NPC iptables comment sanitization: Pod/namespace/policy names embedded directly in iptables-restore comments without control character filtering. Added sanitizeForComment() as defense-in-depth.

Supersedes: #2022, #2039

Was AI used during the creation of this PR?

  • What tool was used: Claude Code
  • To what extent was the tool used? Code review identified the bugs, human reviewed and confirmed each fix
  • If drafted, how detailed of a plan did you create for the AI? Detailed — each bug traced line-by-line
  • Help us understand if a human was in the loop or not for this PR? Yes

What, if any, amount of integration testing was done with this change in a Kubernetes environment?

Unit tests pass for both lballoc and netpol packages. No integration testing.

Does this PR introduce a breaking change?

NONE

Anything else the reviewer should know that wasn't already covered?

The LBC fix changes error message format from "unable to allocate address: no IPs left" to "unable to allocate address: IPv4: no IPs left" — log/error string change only, no API change.

Two defensive fixes:

1. LBC allocateService(): prevent nil pointer panic when no allocation needed
   - When a service already has all requested IPs, both err4 and err6 are
     nil, but err.Error() was called unconditionally. Add early return when
     no allocation was needed and collect per-family error messages safely.

2. NPC pod.go: sanitize pod and policy names before embedding in iptables comments
   - Pod names, namespace names, and policy names are concatenated directly
     into iptables-restore comment strings. Add sanitizeForComment() that
     strips control characters as a defense-in-depth measure.
Table-driven tests covering: clean strings, newline injection,
tabs, carriage returns, null bytes, normal K8s names, and empty
strings.
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.

1 participant