[iacobdaniel]

Replaces pytricia and the Python fallback with the Rust matcher from zen-internals.

[aikido-pr-checks]

🟠 High - Missing native matcher disables SSRF and IP-list enforcement

When the bundled zen-internals library is missing or cannot be loaded, IPMatcher has no handle and its has() method returns false for every address. The static IMDS/private-IP matchers and configured global or route IP lists then produce no matches, allowing DNS resolutions to metadata/private addresses and bypassing configured IP blocklists and allowlists.

Show fix

Retain a functional pure-Python matcher fallback for load failures, or fail closed and make the application refuse to enable these security controls unless all required native matcher symbols are available. Ensure every supported deployment target ships and loads the matching artifact before accepting IP-based security configuration.

_{More info - Reply on this comment to give feedback or ignore the issue.}

[iacobdaniel]

On platforms that have no zen-internals build (for example 32-bit Linux), Zen fails open and logs a warning, the same way SQL injection detection already behaves there on main.

[aikido-pr-checks]

🔴 We were not able to ignore this issue because of the following reason:

You do not have the permission to ignore issues.

[iacobdaniel]

Reminder: this points at the v0.1.75-beta.2 pre-release of zen-internals. Switch BASE_URL to the first stable release that contains the IP matcher.

[hansott]

any way we can avoid this? e.g. use a function create_ip_matcher that returns None if no IPMatcher could be created?

[iacobdaniel]

I tried it. create_ip_matcher() returning None removes the if for route allowlists, but every other place that uses a matcher then needs a None check: the blocklist, bypass list and both SSRF lists. That's 5 checks instead of 2, and any new caller has to remember it. So I would keep it as it is.

[marksmith]

Do those call sites need to check whether the IP matcher was built?

[iacobdaniel]

No. A matcher that isn't built matches no IP. Only an allowlist would then block everyone, so only the two allowlists check is_built(). For the other lists, matching nothing just means that check is skipped, the same as when the library isn't loaded.

[aikido-pr-checks]

The finalizer may receive None as its callback when the library lacks ip_matcher_free; registering it can cause a runtime exception during finalization.

Details

✨ AI Reasoning
​The create binding can be present while the free binding is absent. In that case, construction succeeds and registers a finalizer with a nullable callback, which can fail when the object is finalized.

🔧 How do I fix it?
Check values for null before use with guards, early returns, or null-coalescing. Declare nullable return types for methods that can return null, and avoid silencing nullable warnings with the null-forgiving operator (!) unless the value's non-nullness is guaranteed.

_{Reply on this comment to give feedback or ignore the issue.
More info}

[iacobdaniel]

Fixed. Without ip_matcher_free, ip_matcher_create returns None.

[marksmith]

What if the allowed IPs are currently set? Should a currently failing attempt to set the allowed IPs override a previously successful attempt to set the allowed IPs by clearing the allowed IPs entirely?

[iacobdaniel]

Right, good point. Pushed a fix to keep previous allowlist.

[marksmith]

An IP list that fails to build clears the allowed list. Does an empty allowed IPs list not block visitors that it should allow?

[iacobdaniel]

No. An empty allowed list means allow mode is off, so all visitors are allowed (is_allowed_ip returns True when the list is empty).

[aikido-pr-checks]

🟡 Medium - Malformed allowlist entry suppresses valid blocklist refresh

A successful firewall-list response with valid blocked IP entries but a malformed allowed-list entry causes FirewallLists.update to fail while parsing the allowlist (Trigger). The new atomic update builds both lists before assigning either, and the caller swallows the exception, so the valid blocklist is not installed (Mechanism). Requests from IPs that the response intended to block then pass the global blocklist until a later successful refresh, or indefinitely when this occurs during initial setup (Consequence).

Show fix

Suggested change

	blocked_ips = list(map(parse_ip_entry, blocked_ip_entries))
	allowed_ips = list(map(parse_ip_entry, allowed_ip_entries))
	blocked_ips = list(map(parse_ip_entry, blocked_ip_entries))
	try:
	allowed_ips = list(map(parse_ip_entry, allowed_ip_entries))
	except (KeyError, TypeError, ValueError) as e:
	logger.debug("Exception parsing allowed IP lists: %s", e)
	self.blocked_ips = blocked_ips
	self.set_blocked_user_agents(blocked_user_agents)
	return

_{More info - Reply on this comment to give feedback or ignore the issue.}

[iacobdaniel]

Valid, fixed differently: a missing ips or description no longer raises, so the update still applies everything together.

[aikido-pr-checks]

This issue remains because FirewallLists.update still maps parse_ip_entry at aikido_zen/storage/firewall_lists.py:33 without validating each entry: a non-mapping entry (or non-iterable ips) raises before assignments at lines 58–63, and update_firewall_lists.py:40 swallows it, so the valid blocklist is not installed; the newer .get/IPMatcher filtering only handles some malformed shapes.

[iacobdaniel]

Fixed: ips that isn't a list is now treated as empty, so a malformed allowlist entry is skipped and the rest of the update still applies.

[aikido-pr-checks]

This issue remains because FirewallLists.update still maps every allowlist entry through parse_ip_entry at aikido_zen/storage/firewall_lists.py:28, while parse_ip_entry calls entry.get("ips") at line 159 without validating that entry is a mapping; an entry such as None still aborts the atomic update, and update_firewall_lists swallows that exception. The new non-list ips handling is only a partial fix.

[aikido-pr-checks]

Partially fixed in commit 8017344.

This issue remains because aikido_zen/storage/firewall_lists.py:29-36 returns from FirewallLists.update when any allowed entry has a non-list ips value, before assigning self.blocked_ips at line 69, so a valid blocklist is still not installed; the new validation avoids the parse exception but preserves the atomic suppression.

[iacobdaniel]

Intentional: null (and a missing or empty ips) means the list has no IPs, which is how realtime sends an emptied list, so the blocklist is correctly cleared.

[aikido-pr-checks]

🟡 Medium - Missing IP field installs an empty global allowlist

A firewall-list response with an allowlist entry whose ips field is missing or null is accepted by parse_ip_entry. The resulting empty IPMatcher is still treated as a built, non-empty allowlist entry, so is_allowed_ip rejects every public client address; the same normalization on a malformed blocked entry can also replace a previously effective blocklist with a matcher that matches nothing.

Show fix

Suggested change

	blocked_ips = list(map(parse_ip_entry, blocked_ip_entries))
	allowed_ips = list(map(parse_ip_entry, allowed_ip_entries))
	blocked_ips = (
	self.blocked_ips
	if any(
	not isinstance(entry, dict)
	or not isinstance(entry.get("ips"), list)
	for entry in blocked_ip_entries
	)
	else list(map(parse_ip_entry, blocked_ip_entries))
	)
	allowed_ips = [
	parse_ip_entry(entry)
	for entry in allowed_ip_entries
	if isinstance(entry, dict) and isinstance(entry.get("ips"), list)
	]

_{More info - Reply on this comment to give feedback or ignore the issue.}

[iacobdaniel]

Valid for the allowlist, fixed: allowlist entries without IPs are skipped, so they can't block every visitor.

[aikido-pr-checks]

This issue remains because FirewallLists.update still maps every blocked entry through parse_ip_entry at aikido_zen/storage/firewall_lists.py:17, whose IPMatcher(entry.get("ips")) at line 97 turns missing/null ips into a built empty matcher and installs it, replacing an effective prior block; the new empty-entry filter only protects allowlists at lines 19–23.

[iacobdaniel]

Intentional: realtime sends "ips": null when a list has no IPs, so an empty blocklist is the correct update. Empty allowlist entries are skipped instead, so they never restrict access.

[aikido-pr-checks]

🔴 We were not able to ignore this issue because of the following reason:

You do not have the permission to ignore issues.

[aikido-pr-checks]

This issue remains because update maps blocked entries at firewall_lists.py:28 and installs them at line 58; parse_ip_entry turns missing/null ips into [] at lines 159-165, and IPMatcher([]) is built when its handle exists, so a malformed response can replace an effective blocklist with a matcher matching nothing. Lines 30-35 fix only the empty-allowlist case.

[aikido-pr-checks]

Partially fixed in commit 8017344.

This issue remains because FirewallLists.update only rejects non-None non-list ips at aikido_zen/storage/firewall_lists.py:30; a blocked entry with missing or null ips still reaches parse_ip_entry at line 37, which constructs an empty matcher at line 176, and that replacement is committed to self.blocked_ips at line 69. The allowlist symptom is mitigated by the empty-entry filter at lines 40–44, but malformed blocked entries can still erase effective blocks.

[iacobdaniel]

Intentional: null (and a missing or empty ips) means the list has no IPs, which is how realtime sends an emptied list, so the blocklist is correctly cleared.

[aikido-pr-checks]

🔴 We were not able to ignore this issue because of the following reason:

You do not have the permission to ignore issues.

[aikido-pr-checks]

networks is rebound to a new list, obscuring the caller-provided value. Use a separate local for the filtered input.

Show fix

Suggested change

	networks = list(networks or [])
	self._networks = [
	network
	for network in networks
	if isinstance(network, str) and network.isascii() and "\n" not in network
	]
	self._skipped_count = len(networks) - len(self._networks)
	networks_list = list(networks or [])
	self._networks = [
	network
	for network in networks_list
	if isinstance(network, str) and network.isascii() and "\n" not in network
	]
	self._skipped_count = len(networks_list) - len(self._networks)

Details

✨ AI Reasoning
​The constructor replaces the caller-provided networks value with a newly constructed list, so the parameter no longer represents the original argument. Keeping the copy in a separate local would preserve that distinction and make the code easier to follow.

_{Reply on this comment to give feedback or ignore the issue.
More info}

[iacobdaniel]

Intentional, as before: the original value isn't used after this line, so the name is reused for its list form.

[aikido-pr-checks]

⛔ You don't have permission to ignore issues.