Add outbound blocking - #264
Conversation
also adds test cases for DNSRecordCollector
![aikido-pr-checks[bot]](https://avatars.githubusercontent.com/in/898896?s=60&v=4){kind=small}
| StatisticsStore.registerCall("java.net.InetAddress.getAllByName", OperationKind.OUTGOING_HTTP_OP); | ||
|
|
||
| // Block if the hostname is in the blocked domains list | ||
| if (ServiceConfigStore.shouldBlockOutgoingRequest(hostname)) { |
There was a problem hiding this comment.
DNSRecordCollector.report now performs enforcement (may throw BlockedOutboundException) in addition to reporting. Rename or document the method to reflect enforcement, or move blocking logic to a separate method.
Details
✨ AI Reasoning
The DNSRecordCollector.report method was changed to both report DNS records and enforce blocking by throwing BlockedOutboundException when a hostname is blocked. The method name 'report' no longer fully conveys that it can perform enforcement side-effects. Mixing collection/reporting with blocking logic makes the purpose less self-evident and can surprise callers expecting a pure reporter method. This harms readability and makes error handling assumptions unclear to callers.
🔧 How do I fix it?
Use descriptive verb-noun function names, add docstrings explaining the function's purpose, or provide meaningful return type hints.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
The clear in the Context.set was clearing this bridge and breaking redirect protectiosn
| ServiceConfiguration config = getConfig(); | ||
| Context.reset(); // clear context | ||
|
|
||
| // clear context |
There was a problem hiding this comment.
report() now calls PendingHostnamesStore.clear(), introducing an unexpected side-effect and mixing responsibilities. Move this clearing to a clearly named method or document/rename report() to reflect the additional behavior.
Details
✨ AI Reasoning
The WebRequestCollector.report method previously handled initial request context setup and basic blocking checks. The change adds a call to clear a global/thread-local PendingHostnamesStore, which introduces a side-effect unrelated to the method's documented purpose. This mixes responsibilities and makes the function's purpose less self-evident: a caller expecting only context/reporting behavior may be surprised that pending hostnames are flushed here. The doc comment for report was not updated to reflect this new behavior, so the purpose is now unclear without reading implementation.
🔧 How do I fix it?
Use descriptive verb-noun function names, add docstrings explaining the function's purpose, or provide meaningful return type hints.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
…l site Private IP literal hostnames with no pending port (resolver bootstrap noise, network capability probing, etc.) flood the outbound-connections dashboard - skip recording those specifically. Outbound domain blocking and SSRF detection stay fully unconditional either way. Known-port HTTP client calls now record their dashboard hit directly at the call site (URLCollector), matching every other Zen agent (Go, Node, Ruby, .NET, PHP) and restoring the original architecture from #264. DNS-resolution-time recording is now only a fallback for hostnames that never reached an instrumented HTTP client (JDBC, raw sockets, etc.) - which is also the only place the private-IP noise gate applies.
2 participants
Summary by Aikido
🚀 New Features
⚡ Enhancements
🔧 Refactors
More info