Skip to content

Move attack wave detection to WebResponseCollector to capture user info #248

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
PopoviciMarian wants to merge 2 commits into from
Closed

Move attack wave detection to WebResponseCollector to capture user info #248

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ff6d2f0
Move attack wave detection to WebResponseCollector to capture user info
PopoviciMarian Nov 20, 2025
219a545
clean
PopoviciMarian Nov 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions agent_api/src/main/java/dev/aikido/agent_api/collectors/WebRequestCollector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,13 @@
package dev.aikido.agent_api.collectors;

import dev.aikido.agent_api.background.Endpoint;
import dev.aikido.agent_api.background.cloud.api.events.DetectedAttackWave;
import dev.aikido.agent_api.context.Context;
import dev.aikido.agent_api.context.ContextObject;
import dev.aikido.agent_api.context.RouteMetadata;
import dev.aikido.agent_api.helpers.logging.LogManager;
import dev.aikido.agent_api.helpers.logging.Logger;
import dev.aikido.agent_api.storage.AttackQueue;
import dev.aikido.agent_api.storage.ServiceConfigStore;
import dev.aikido.agent_api.storage.ServiceConfiguration;
import dev.aikido.agent_api.storage.attack_wave_detector.AttackWaveDetectorStore;
import dev.aikido.agent_api.storage.statistics.StatisticsStore;

import java.util.List;
Expand Down Expand Up @@ -60,14 +57,6 @@ public static Res report(ContextObject newContext) {
if (blockedUARes != null)
return blockedUARes;

// Check for attack waves
if (AttackWaveDetectorStore.check(newContext)) {
AttackQueue.add(
DetectedAttackWave.createAPIEvent(newContext)
);
StatisticsStore.incrementAttackWavesDetected();
}

return null;
}

Expand Down
21 changes: 20 additions & 1 deletion agent_api/src/main/java/dev/aikido/agent_api/collectors/WebResponseCollector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
package dev.aikido.agent_api.collectors;

import dev.aikido.agent_api.api_discovery.APISpec;
import dev.aikido.agent_api.background.cloud.api.events.DetectedAttackWave;
import dev.aikido.agent_api.context.Context;
import dev.aikido.agent_api.context.ContextObject;
import dev.aikido.agent_api.context.RouteMetadata;
import dev.aikido.agent_api.storage.AttackQueue;
import dev.aikido.agent_api.storage.attack_wave_detector.AttackWaveDetectorStore;
import dev.aikido.agent_api.storage.routes.RoutesStore;
import dev.aikido.agent_api.storage.statistics.StatisticsStore;

import static dev.aikido.agent_api.api_discovery.GetApiInfo.getApiInfo;
import static dev.aikido.agent_api.helpers.url.IsUsefulRoute.isUsefulRoute;
Expand All @@ -23,6 +27,15 @@ public static void report(int statusCode) {
if (statusCode <= 0 || context == null) {
return; // Status code below or equal to zero: Invalid request
}

// Check for attack waves (after request is complete and user has been set)
if (AttackWaveDetectorStore.check(context)) {

@aikido-pr-checks aikido-pr-checks Bot Nov 20, 2025 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling shared/static stores (AttackWaveDetectorStore.check, AttackQueue.add, StatisticsStore.incrementAttackWavesDetected) from concurrent request threads may introduce race conditions if those stores are not thread-safe.

Details

✨ AI Reasoning
​​1) The new code invokes shared/static stores: AttackWaveDetectorStore.check(context), AttackQueue.add(...), and StatisticsStore.incrementAttackWavesDetected() from a static reporter that can run concurrently across requests.
​2) If these store methods access or mutate shared state without internal synchronization or using concurrent-safe data structures, this introduces potential race conditions (check-then-act, concurrent modifications) that can cause data races or lost updates.
​3) This harms maintainability and correctness in a multithreaded web server context because the reporter runs on request-completion threads; ensuring atomicity or using thread-safe APIs is necessary. Therefore this change introduces potential thread-safety issues 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.

AttackQueue.add(
DetectedAttackWave.createAPIEvent(context)
);
StatisticsStore.incrementAttackWavesDetected();
}

RouteMetadata routeMetadata = context.getRouteMetadata();
if (routeMetadata == null || !isUsefulRoute(statusCode, context.getRoute(), context.getMethod())) {
return;
Expand All @@ -37,4 +50,10 @@ public static void report(int statusCode) {
RoutesStore.updateApiSpec(routeMetadata, apiSpec);
}
}
}
}
//do we run @Marian Popovici’s tests already on Ruby?
// we do, but most of them fail because there are some differences in the implementation, like sending post intead of POST in event, or /test_ratelimiting_1(.:format) instead of /test_ratelimiting_1

// Other porbless:
// path traversal bypas - https://aikido-security.slack.com/archives/C07F482T4JG/p1754669410362449
//
40 changes: 0 additions & 40 deletions agent_api/src/test/java/collectors/WebRequestCollectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,8 @@
import dev.aikido.agent_api.background.Endpoint;
import dev.aikido.agent_api.background.cloud.api.APIResponse;
import dev.aikido.agent_api.background.cloud.api.ReportingApi;
import dev.aikido.agent_api.background.cloud.api.events.DetectedAttackWave;
import dev.aikido.agent_api.collectors.WebRequestCollector;
import dev.aikido.agent_api.context.Context;
import dev.aikido.agent_api.context.ContextObject;
import dev.aikido.agent_api.storage.AttackQueue;
import dev.aikido.agent_api.storage.ServiceConfigStore;
import dev.aikido.agent_api.storage.statistics.StatisticsStore;
Expand All @@ -15,7 +13,6 @@
import utils.EmptySampleContextObject;

import java.util.List;
import java.util.Map;

import static dev.aikido.agent_api.helpers.UnixTimeMS.getUnixTimeMS;
import static org.junit.jupiter.api.Assertions.*;
Expand Down Expand Up @@ -261,41 +258,4 @@ void testReport_ipNotAllowedUsingLists_Ip_Bypassed() {
assertNull(Context.get());
}

@Test
void testReport_WithAttackWaveContext() throws InterruptedException {
ContextObject attackWaveCtx = new EmptySampleContextObject("/wp-config.php", "BADMETHOD", Map.of());

WebRequestCollector.Res response = WebRequestCollector.report(attackWaveCtx);
assertNull(response);
assertEquals(0, AttackQueue.getSize());
assertEquals(0, StatisticsStore.getStatsRecord().requests().attackWaves().total());

// 2...14
WebRequestCollector.report(attackWaveCtx);
WebRequestCollector.report(attackWaveCtx);
WebRequestCollector.report(attackWaveCtx);
WebRequestCollector.report(attackWaveCtx);
WebRequestCollector.report(attackWaveCtx);
WebRequestCollector.report(attackWaveCtx);
WebRequestCollector.report(attackWaveCtx);
WebRequestCollector.report(attackWaveCtx);
WebRequestCollector.report(attackWaveCtx);
WebRequestCollector.report(attackWaveCtx);
WebRequestCollector.report(attackWaveCtx);
WebRequestCollector.report(attackWaveCtx);
WebRequestCollector.report(attackWaveCtx);

WebRequestCollector.Res response2 = WebRequestCollector.report(attackWaveCtx);
assertNull(response2);
assertEquals(1, AttackQueue.getSize());
DetectedAttackWave.DetectedAttackWaveEvent event = (DetectedAttackWave.DetectedAttackWaveEvent) AttackQueue.get();
assertEquals("192.168.1.1", event.request().ipAddress());
assertEquals("web", event.request().source());
assertEquals(null, event.request().userAgent());
assertEquals("detected_attack_wave", event.type());
assertEquals(null, event.attack().user());
assertEquals(0, event.attack().metadata().size());
// check stats changed
assertEquals(1, StatisticsStore.getStatsRecord().requests().attackWaves().total());
}
}
88 changes: 88 additions & 0 deletions agent_api/src/test/java/collectors/WebResponseCollectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,27 @@
package collectors;

import dev.aikido.agent_api.background.cloud.api.events.DetectedAttackWave;
import dev.aikido.agent_api.collectors.WebRequestCollector;
import dev.aikido.agent_api.collectors.WebResponseCollector;
import dev.aikido.agent_api.context.Context;
import dev.aikido.agent_api.context.ContextObject;
import dev.aikido.agent_api.context.RouteMetadata;
import dev.aikido.agent_api.context.User;
import dev.aikido.agent_api.storage.AttackQueue;
import dev.aikido.agent_api.storage.ServiceConfigStore;
import dev.aikido.agent_api.storage.routes.RoutesStore;
import dev.aikido.agent_api.storage.statistics.StatisticsStore;
import org.junit.jupiter.api.*;
import org.junit.jupiter.api.Test;
import utils.EmptySampleContextObject;

import java.sql.SQLException;
import java.util.HashMap;
import java.util.Map;

import static org.junit.jupiter.api.Assertions.*;
import static utils.EmptyAPIResponses.emptyAPIListsResponse;
import static utils.EmptyAPIResponses.emptyAPIResponse;


public class WebResponseCollectorTest {
Expand Down Expand Up @@ -43,6 +53,8 @@ public static void clean() {
public void tearDown() throws SQLException {
Context.set(null);
RoutesStore.clear();
AttackQueue.clear();
StatisticsStore.clear();
}

@Test
Expand Down Expand Up @@ -92,5 +104,81 @@ public void testResponseCollectorWithInvalidMethodOrStatusCode() throws SQLExcep
WebResponseCollector.report(-200);
assertEquals(0, RoutesStore.getRoutesAsList().length);
}

@Test
public void testAttackWaveDetectionWithUserSet() throws SQLException, InterruptedException {
// Setup
ServiceConfigStore.updateFromAPIResponse(emptyAPIResponse);
ServiceConfigStore.updateFromAPIListsResponse(emptyAPIListsResponse);

// Create attack wave context (unusual route/method that triggers attack wave detection)
ContextObject attackWaveCtx = new EmptySampleContextObject("/wp-config.php", "BADMETHOD", Map.of());

// Set a user in the context (simulating SetUser.setUser being called during request)
User testUser = new User("user123", "John Doe", "192.168.1.1", System.currentTimeMillis());
attackWaveCtx.setUser(testUser);

// Simulate the request flow: first WebRequestCollector, then trigger attack wave detection
// We need to trigger attack waves by making multiple requests
for (int i = 0; i < 15; i++) {
Context.set(attackWaveCtx);
WebRequestCollector.report(attackWaveCtx);
}

// Now call WebResponseCollector which should detect the attack wave WITH user info
Context.set(attackWaveCtx);
WebResponseCollector.report(200);

// Verify attack wave was detected
assertTrue(AttackQueue.getSize() > 0, "Attack wave should be detected");
assertEquals(1, StatisticsStore.getStatsRecord().requests().attackWaves().total());

// Get the attack wave event and verify user information is captured
DetectedAttackWave.DetectedAttackWaveEvent event =
(DetectedAttackWave.DetectedAttackWaveEvent) AttackQueue.get();

assertNotNull(event);
assertEquals("detected_attack_wave", event.type());
assertEquals("192.168.1.1", event.request().ipAddress());
assertEquals("web", event.request().source());

// The key assertion: user information should be present
assertNotNull(event.attack().user(), "User should be captured in attack wave event");
assertEquals("user123", event.attack().user().id());
assertEquals("John Doe", event.attack().user().name());
}

@Test
public void testAttackWaveDetectionWithoutUser() throws SQLException, InterruptedException {
// Setup
ServiceConfigStore.updateFromAPIResponse(emptyAPIResponse);
ServiceConfigStore.updateFromAPIListsResponse(emptyAPIListsResponse);

// Create attack wave context without user
ContextObject attackWaveCtx = new EmptySampleContextObject("/wp-config.php", "BADMETHOD", Map.of());

// Trigger attack waves by making multiple requests
for (int i = 0; i < 15; i++) {
Context.set(attackWaveCtx);
WebRequestCollector.report(attackWaveCtx);
}

// Call WebResponseCollector which should detect the attack wave without user info
Context.set(attackWaveCtx);
WebResponseCollector.report(200);

// Verify attack wave was detected
assertTrue(AttackQueue.getSize() > 0, "Attack wave should be detected");

// Get the attack wave event
DetectedAttackWave.DetectedAttackWaveEvent event =
(DetectedAttackWave.DetectedAttackWaveEvent) AttackQueue.get();

assertNotNull(event);
assertEquals("detected_attack_wave", event.type());

// User should be null when not set
assertNull(event.attack().user(), "User should be null when not set during request");
}
}

Loading