[Bug] isAllowedClient() only checks the first valid CIDR (fixes #105)

[wollomatic]

see #105

Summary by CodeRabbit

• Bug Fixes
  • Fixed allow-list evaluation logic to correctly assess all configured entries when determining client access permissions, ensuring requests matching any configured rule are properly accepted rather than being prematurely rejected.

[coderabbitai]

Walkthrough

The isAllowedClient function in the socket proxy now evaluates all allowFromItem entries by continuing to the next item when a CIDR doesn't contain the IP, rather than immediately returning false. This allows multiple CIDR entries to be aggregated for acceptance, broadening the matching criteria.

Changes

Cohort / File(s)	Summary
CIDR Evaluation Logic cmd/socket-proxy/handlehttprequest.go	Modified isAllowedClient to continue iterating through allowFromItem entries on non-matching CIDR checks instead of returning early, enabling cumulative allowlist evaluation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A proxy hops through allowlists with grace,
No longer stopping at the first CIDR case,
Each entry now dances in sequence complete,
Multiple paths make the acceptance more sweet!

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and specifically describes the bug fix: isAllowedClient() only checking the first valid CIDR, which matches the core change in the pull request.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch 105-bug-isallowedclient-only-checks-the-first-valid-cidr

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

cmd/socket-proxy/handlehttprequest.go (1)

89-92: Add a regression test for the multi-entry allow list.

This loop fix looks right, but there is still no coverage for the case where Line 89 misses one CIDR and a later AllowFrom entry should match. Please add a small table-driven test for something like ["10.0.0.0/24", "192.168.1.0/24"] with a 192.168.1.x client so this bug does not creep back in.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cmd/socket-proxy/handlehttprequest.go` around lines 89 - 92, Add a
table-driven unit test that exercises the multi-entry AllowFrom check so we
cover the case where the first CIDR does not match but a later one does: create
a test (e.g. TestHandleHTTPRequestAllowListMultiEntry) that supplies AllowFrom =
["10.0.0.0/24", "192.168.1.0/24"] and a client IP like "192.168.1.5" and assert
the check returns allowed; ensure the test hits the code path using
allowedIPNet.Contains over the AllowFrom list so a non-match on the first entry
and a match on a later entry is validated and cannot regress.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@cmd/socket-proxy/handlehttprequest.go`:
- Around line 89-92: Add a table-driven unit test that exercises the multi-entry
AllowFrom check so we cover the case where the first CIDR does not match but a
later one does: create a test (e.g. TestHandleHTTPRequestAllowListMultiEntry)
that supplies AllowFrom = ["10.0.0.0/24", "192.168.1.0/24"] and a client IP like
"192.168.1.5" and assert the check returns allowed; ensure the test hits the
code path using allowedIPNet.Contains over the AllowFrom list so a non-match on
the first entry and a match on a later entry is validated and cannot regress.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f4ac3da2-6602-4857-8e33-5cc3e6809ec5

📥 Commits

Reviewing files that changed from the base of the PR and between e5b6e50 and faff8e3.

📒 Files selected for processing (1)

• cmd/socket-proxy/handlehttprequest.go