Count 404 requests with foreign extensions as attack wave scans#300
Count 404 requests with foreign extensions as attack wave scans#300bitterpanda63 wants to merge 1 commit into
Conversation
Ports AikidoSec/firewall-node#1041 to Java. Requests to foreign-platform extensions (php, php3, php4, php5, phtml) are only counted as attack wave scan hits when the response is 404 — a 200 may indicate the app proxies to another backend. jsp/jspx are excluded since Java apps can legitimately serve JSP files. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![aikido-pr-checks[bot]](https://avatars.githubusercontent.com/in/898896?s=60&v=4){kind=small}
| * @return true if an attack wave is detected and should be reported. | ||
| */ | ||
| public boolean check(ContextObject ctx) { | ||
| public boolean check(ContextObject ctx, int statusCode) { |
There was a problem hiding this comment.
Method 'check(ContextObject ctx, int statusCode)' still uses the ambiguous 'check' prefix; the name doesn't convey its side effects (mutating internal LRU caches and sending events) or returned semantic.
Details
✨ AI Reasoning
The PR modified the AttackWaveDetector.check method signature to add a statusCode parameter. The method name still begins with 'check', which is the pattern the rule discourages. The change occurred at the method declaration line where the signature was updated. This keeps the same ambiguous 'check' prefix while altering the method's behavior/contract, which maintains and slightly worsens the clarity problem because callers must now pass an extra parameter to a method whose name doesn't indicate its side-effects or return semantics.
🔧 How do I fix it?
Replace 'check' with more descriptive verbs that indicate the function's action or purpose. Use 'validate' for validation, 'get' for retrieval, or 'is' for boolean checks. Ensure the name clearly communicates the function's intent and return type.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
| } | ||
|
|
||
| public static boolean check(ContextObject ctx) { | ||
| public static boolean check(ContextObject ctx, int statusCode) { |
There was a problem hiding this comment.
Method 'check(ContextObject ctx, int statusCode)' uses the ambiguous 'check' prefix; its behavior (locking, forwarding to detector which mutates caches and may record events) isn't communicated by the name.
Details
✨ AI Reasoning
The PR updated AttackWaveDetectorStore.check to accept a statusCode and forward it to the detector. The method name begins with 'check', which is discouraged: it doesn't indicate whether it performs queries, mutations, or reporting. The change touched the method signature and body, keeping the unclear name while changing its contract, thus worsening the clarity slightly by expanding the method's responsibilities without renaming.
🔧 How do I fix it?
Replace 'check' with more descriptive verbs that indicate the function's action or purpose. Use 'validate' for validation, 'get' for retrieval, or 'is' for boolean checks. Ensure the name clearly communicates the function's intent and return type.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1 participant
Summary
Ports AikidoSec/firewall-node#1041 to Java.
php,php3,php4,php5,phtml) are only counted as attack wave scan hits when the HTTP response is 404jsp/jspxare intentionally excluded since Java apps can legitimately serve JSP filesChanges
PathChecker: addedFOREIGN_EXTENSIONSset;isWebScanPath(String)→isWebScanPath(String, int statusCode)with the new 404-gated checkWebScanDetector:isWebScanner(ctx)→isWebScanner(ctx, statusCode), threadsstatusCodetoisWebScanPathAttackWaveDetector/AttackWaveDetectorStore:check(ctx)→check(ctx, statusCode), threadsstatusCodetoisWebScannerWebResponseCollector: passes the already-availablestatusCodeparameter toAttackWaveDetectorStore.checkTest plan
PathCheckerTest.testForeignExtensions_404_ReturnsTrue— php/php3/php4/php5/phtml with 404 →truePathCheckerTest.testForeignExtensions_200_ReturnsFalse— same extensions with 200 →falsePathCheckerTest,WebScanDetectorTest,AttackWaveDetectorTest, and benchmark tests pass with updated signatures./gradlew :agent_api:testpasses green🤖 Generated with Claude Code
Summary by Aikido
⚡ Enhancements
More info