Clarify outbound pr

[bitterpanda63]

Summary by Aikido

⚡ Enhancements

• Added should_block_outbound_domain helper to record hostnames and evaluate blocking
• Normalized hostname in socket.getaddrinfo and enforced blocking by raising error

🔧 Refactors

• Renamed domains field and update methods to outbound_domains across codebase

[aikido-pr-checks]

Calling should_block_outbound_domain(...) from socket.getaddrinfo path causes unsynchronized mutation of shared cache; invoke only with proper synchronization or use a thread-safe API.

Details

✨ AI Reasoning
​The socket getaddrinfo after-wrapper (_getaddrinfo_after) was modified to call should_block_outbound_domain(hostname, port). That call can mutate process_cache.hostnames (see added function). Because socket.getaddrinfo is commonly invoked from multiple threads, invoking a function that mutates shared state without synchronization increases the risk of concurrent modifications and related race conditions. The change moved from the previous report_and_check_hostname behavior to storing hostname and performing a blocking check that now performs mutation; this worsens thread-safety at the call site.

🔧 How do I fix it?
Use locks, concurrent collections, or atomic operations when accessing shared mutable state. Avoid modifying collections during iteration. Use proper synchronization primitives like mutex, lock, or thread-safe data structures.

_{More info - Comment @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.}

[aikido-pr-checks]

process_cache.hostnames.add(...) mutates shared cache without synchronization; protect hostnames updates with a lock or use a thread-safe collection.

Details

✨ AI Reasoning
​The new function should_block_outbound_domain (a newly added file) calls process_cache.hostnames.add(hostname, port) which mutates shared state retrieved from get_cache(). There's no locking or thread-safe API evident around hostnames modification. This introduces a potential race where multiple threads calling socket.getaddrinfo (which now invokes this code) may concurrently modify process_cache.hostnames, causing data races or runtime errors if the underlying collection is not thread-safe. The call site in aikido_zen/sinks/socket/init.py now invokes should_block_outbound_domain from the socket path used in many threads, making the race practical. Previously the code path used report_and_check_hostname (removed) — the new behavior increases risk because of the explicit mutation added.

🔧 How do I fix it?
Use locks, concurrent collections, or atomic operations when accessing shared mutable state. Avoid modifying collections during iteration. Use proper synchronization primitives like mutex, lock, or thread-safe data structures.

_{More info - Comment @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.}

[aikido-pr-checks]

should_block_outbound_domain both records hostnames (side-effect) and returns a boolean. Rename or separate recording from the predicate, or add clear docstring describing the side-effect.

Details

✨ AI Reasoning
​The new function should_block_outbound_domain (a predicate-style name) both records the hostname/port into process_cache.hostnames and returns a boolean decision. A caller reading the name would reasonably expect a pure check. The inline comments document the recording behavior, but the side-effect remains surprising and mixes responsibilities: (1) mutation of process_cache (recording hostnames), and (2) policy evaluation (blocking). This reduces clarity and can lead to misuse or unexpected state changes when callers invoke a 'should_' function solely to inspect blocking status.

🔧 How do I fix it?
Use descriptive verb-noun function names, add docstrings explaining the function's purpose, or provide meaningful return type hints.

_{More info - Comment @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.}