Skip to content

[management] Handle missing NetworkAddresses in peer network range posture check#5806

Open
MichaelUray wants to merge 1 commit intonetbirdio:mainfrom
MichaelUray:fix/posture-check-empty-network-addresses
Open

[management] Handle missing NetworkAddresses in peer network range posture check#5806
MichaelUray wants to merge 1 commit intonetbirdio:mainfrom
MichaelUray:fix/posture-check-empty-network-addresses

Conversation

@MichaelUray
Copy link
Copy Markdown

@MichaelUray MichaelUray commented Apr 6, 2026

Summary

Peers with empty NetworkAddresses (e.g., older mobile clients that don't report network interfaces) were incorrectly handled by the peer_network_range_check posture check. The check returned an error, which blocked the peer entirely.

Fix: For deny action, allow the peer through (can't confirm it IS in the denied range). For allow action, deny the peer (can't confirm it IS in the allowed range).

Includes updated unit tests.

Checklist

  • Bug fix
  • Create tests that fail without the change: updated existing tests to match new behavior
  • Documentation not needed — behavioral fix for edge case

By submitting this pull request, I confirm that I have read and agree to the terms of the Contributor License Agreement.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed network posture validation to properly handle peers without configured network addresses. The system now applies the configured policy rules correctly instead of returning errors, improving the reliability of network access checks.

…e posture check

Peers with empty NetworkAddresses (e.g., older mobile clients) were
blocked by deny-action posture checks. Allow them through since we
cannot confirm they ARE in the denied range.

Update tests to match new behavior.
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 6, 2026

CLA assistant check
All committers have signed the CLA.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 6, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 06a81aa6-25f2-4e79-9954-347b43a50197

📥 Commits

Reviewing files that changed from the base of the PR and between decb5dd and 42f255a.

📒 Files selected for processing (2)
  • management/server/posture/network.go
  • management/server/posture/network_test.go

📝 Walkthrough

Walkthrough

The PeerNetworkRangeCheck.Check method now handles empty network addresses differently. Instead of returning an error, it returns a boolean outcome based on the action type: CheckActionDeny returns true (allow), while CheckActionAllow returns false (deny). Test cases were updated accordingly.

Changes

Cohort / File(s) Summary
Network Range Check Logic
management/server/posture/network.go
Modified empty network address handling to return action-based outcomes instead of errors: returns true for CheckActionDeny and false for CheckActionAllow.
Test Expectations
management/server/posture/network_test.go
Updated test assertions for empty peer scenarios: allow case expects invalid but no error; deny case expects valid and no error.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested reviewers

  • crn4

Poem

🐰 Hops through networks empty and bare,
No addresses found, but we don't despair!
Allow says no, Deny says yes—
Logic flipped with such finesse!
Empty checks now find their way. 🌐

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: handling missing NetworkAddresses in the peer network range posture check.
Description check ✅ Passed The description includes a clear summary of the problem and fix, checked the bug fix checkbox, confirmed tests were updated, and noted documentation is not needed. All critical template sections are addressed.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Apr 6, 2026

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