fix: -allow and -deny flags #2232
Conversation
- Add missing flag integration in createNetworkpolicyInstance() - Fixes broken IP filtering where -allow and -deny flags were ignored - Add test coverage for Allow/Deny flag validation The NetworkPolicy instance was created without the Allow/Deny flag values, causing all IP filtering to be bypassed regardless of command-line flags
WalkthroughAdds ASN-aware expansion for Allow and Deny entries when building network policy options and a unit test validating Allow/Deny behavior across multiple scenarios. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as CreateNetworkpolicyInstance
participant Options as CLI Options
participant Helper as appendToList
participant ASN as asn package
participant CIDR as CIDR expander
participant NPOpts as npOptions
participant NP as NetworkPolicy
Caller->>Options: read Allow/Deny/Exclude flags
Caller->>NPOpts: populate from Exclude and other flags
Caller->>Helper: appendToList(NPOpts.AllowList, options.Allow...)
Helper->>ASN: IsASN? / GetCIDRsForASNNum
ASN->>CIDR: return CIDRs
CIDR->>Helper: expand CIDR -> IPs
Helper-->>NPOpts: return augmented AllowList
Caller->>Helper: appendToList(NPOpts.DenyList, options.Deny...)
Helper-->>NPOpts: return augmented DenyList
Caller->>NP: New(NetworkPolicy, NPOpts)
NP-->>Caller: instance / error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these settings in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
runner/runner_test.go (1)
226-258: Solid test coverage for Allow and Deny flags.The test comprehensively validates both the
-allowand-denyflag functionality by:
- Verifying that IPs outside the allowed range are blocked when using
-allow- Confirming that IPs within a denied range are blocked when using
-deny- Testing that appropriate IPs are permitted in each scenario
Consider adding a test case that validates the behavior when both
-allowand-denyflags are used together, as mentioned in the PR objectives that "the flags work both independently and in combination."+ // Test Allow and Deny flags together + options = &Options{} + options.Allow = []string{"192.168.0.0/16"} + options.Deny = []string{"192.168.1.0/24"} + + np, err = runner.createNetworkpolicyInstance(options) + require.Nil(t, err, "could not create networkpolicy instance") + + // Should block IP in denied range even if in allowed range + allowed = np.Validate("192.168.1.10") + require.False(t, allowed, "IP in denied range should be blocked even if in allowed range") + + // Should allow IP in allowed range but not in denied range + allowed = np.Validate("192.168.2.10") + require.True(t, allowed, "IP in allowed range but not denied range should be allowed")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
runner/runner.go(1 hunks)runner/runner_test.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: noaho
PR: projectdiscovery/httpx#2227
File: runner/runner.go:1308-1329
Timestamp: 2025-08-01T06:38:23.971Z
Learning: In the httpx codebase, normal IP inputs go through network policy validation via skip() method which validates URL.Host, but vhost-input format bypasses this validation since skip() only validates the hostname portion (target.Host) and not the actual target IP (target.CustomIP), creating a potential security bypass.
Learnt from: noaho
PR: projectdiscovery/httpx#2227
File: runner/runner.go:1308-1329
Timestamp: 2025-08-01T06:18:09.303Z
Learning: In the httpx codebase, IP validation occurs at multiple layers: parseVhostInput for format validation, and later via network policy validation in the skip() method. The codebase pattern extensively uses iputil utility functions for input validation, suggesting early IP validation is appropriate and consistent with the architecture.
🔇 Additional comments (1)
runner/runner.go (1)
415-419: LGTM! Clean integration of Allow and Deny flags.The fix correctly integrates the
-allowand-denycommand-line flags into the network policy by appendingoptions.Allowandoptions.Denyto the respectivenpOptionslists before creating the network policy instance. This ensures the flags are properly enforced, addressing the bug where they were parsed but not applied.
- Add test cases for simultaneous use of -allow and -deny flags - Validate deny rules take precedence over allow rules - Test multiple allow/deny ranges with overlapping scenarios - Ensure IPs outside allow ranges are blocked when allow list exists
|
I added extra tests for flags being used together. |
|
@coderabbitai resolve |
✅ Actions performedComments resolved. |
dogancanbakir
left a comment
dogancanbakir
left a comment
There was a problem hiding this comment.
LGTM; good catch - left a comment for you,
|
@coderabbitai resolve |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
runner/runner.go(1 hunks)
🔇 Additional comments (1)
runner/runner.go (1)
415-418: Wiring -allow/-deny into NetworkPolicy is correct and fixes the bugHooking Options.Allow and Options.Deny into npOptions fixes the root cause where flags were parsed but ignored. This aligns with the PR objective and associated tests.
✅ Actions performedComments resolved. |
3 participants
Summary
Fixes non-functional IP filtering in httpx where
-allowand-denyflags were being parsed but never applied to theNetworkPolicy instance.
Changes
createNetworkpolicyInstance()Problem
The
-allowand-denycommand-line flags were not being passed to the NetworkPolicy instance during initialization,causing all IP filtering to be ignored. IPs that should have been blocked were allowed through.
Testing
Added
TestCreateNetworkpolicyInstance_AllowDenyFlagswhich validates:Examples
Summary by CodeRabbit
New Features
Tests