Update qa tests to v1.0.12 by bitterpanda63 · Pull Request #280 · AikidoSec/firewall-java

bitterpanda63 · 2026-04-13T14:22:10Z

Summary by Aikido

Security Issues: 0	🔍 Quality Issues: 2	Resolved Issues: 0

🚀 New Features

Added thread-local bypassed context store and integrated bypass checks.
Introduced DangerousBodyException and JWT/body size and depth safeguards.

⚡ Enhancements

Normalized hostnames using IDN and compared Unicode and punycode equivalents.
Adjusted DNS and request collectors to respect bypassed context behavior.
Updated QA workflow to use firewall-tester-action v1.0.12 and removed skips.

🔧 Refactors

Renamed IP block message and changed outbound blocking wording accordingly.

^{More info}

codecov · 2026-04-13T14:26:49Z

Codecov Report

❌ Patch coverage is 70.17544% with 17 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...erabilities/outbound_blocking/OutboundDomains.java	0.00%	8 Missing ⚠️
...ikido/agent_api/helpers/patterns/LooksLikeJWT.java	55.55%	4 Missing ⚠️
.../agent_api/helpers/extraction/StringExtractor.java	85.71%	2 Missing ⚠️
.../dev/aikido/agent_api/vulnerabilities/Scanner.java	0.00%	2 Missing ⚠️
...aikido/agent_api/storage/ServiceConfiguration.java	0.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

aikido-pr-checks · 2026-04-14T12:01:24Z


            // Block if the hostname is in the blocked domains list
-            if (ServiceConfigStore.shouldBlockOutgoingRequest(hostname)) {
+            if (ServiceConfigStore.shouldBlockOutgoingRequest(hostname) && !BypassedContextStore.isBypassed()) {


report(...) now includes enforcement via BypassedContextStore.isBypassed(), mixing reporting and blocking. Rename/split the method or document the enforcement behavior.

Details

✨ AI Reasoning
The report(...) method in the DNSRecordCollector previously performed DNS-related reporting and SSRF detection. The recent change adds a bypass check and conditional blocking via BypassedContextStore.isBypassed(), introducing enforcement behavior into a method named 'report'. This increases mixing of responsibilities and makes the method's purpose less self-evident, especially when callers expect reporting-only side effects. The problematic change occurs where the blocking condition was tightened to include the bypass check.

🔧 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}

…ests

draft: add DangerousBodyException

aikido-pr-checks · 2026-04-14T14:07:47Z

+            if (!bypassed) {
+                // Bypassed IPs are trusted — don't report their outbound hostnames in heartbeats.
+                if (!ports.isEmpty()) {
+                    for (int port : ports) {
+                        HostnamesStore.incrementHits(hostname, port);
+                    }
+                } else {
+                    HostnamesStore.incrementHits(hostname, 0);
                }


The if (!bypassed) block in report adds nesting around the main hostname-hit logic; invert the condition or use an early return/guard to flatten the method for clarity.

Show fix

Suggested change

if (!bypassed) {

// Bypassed IPs are trusted — don't report their outbound hostnames in heartbeats.

if (!ports.isEmpty()) {

for (int port : ports) {

HostnamesStore.incrementHits(hostname, port);

}

} else {

HostnamesStore.incrementHits(hostname, 0);

}

if (bypassed) {

// Bypassed IPs are trusted — don't report their outbound hostnames in heartbeats.

} else if (!ports.isEmpty()) {

for (int port : ports) {

HostnamesStore.incrementHits(hostname, port);

}

} else {

HostnamesStore.incrementHits(hostname, 0);

Details

✨ AI Reasoning
The report method now computes a bypassed flag and then wraps hostname hit-incrementing logic inside an if (!bypassed) else branch, increasing nesting. This buries the normal-path behavior and makes the flow harder to follow; using an early return or inverting the condition would flatten the logic and improve readability.

_{Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.}
_{Reply @AikidoSec ignore: [REASON] to ignore this issue.}
_{More info}

aikido-pr-checks · 2026-04-16T12:56:41Z

+        // before the real lookup runs. Without this, getAllByName throws UnknownHostException
+        // and OnMethodExit never fires — meaning DNSRecordCollector can't block or track
+        // the hostname.
+        @Advice.OnMethodEnter(suppress = Throwable.class)


Converting and replacing the original 'hostname' argument (IDN.toASCII) before lookup can obscure the original domain and enable bypasses. Preserve or log the original hostname (or avoid in-place mutation) so downstream checks see the true input.

Details

✨ AI Reasoning
The change adds an entry advice that converts non-ASCII hostnames to ASCII (Punycode) before DNS lookup. Transforming inputs prior to resolution can hide the original domain representation from later inspection (e.g., logs or collectors) and may be used to bypass domain-based detection or allowlist checks. The transformation is done silently (catching IllegalArgumentException and returning) and replaces the hostname argument in place, which can obscure the original value from downstream code. This behavior changes how hostnames are presented to the rest of the system and therefore can degrade transparency and enable hiding malicious domains.

🔧 How do I fix it?
Ensure code is transparent and not intentionally obfuscated. Avoid hiding functionality from code review. Focus on intent and deception, not specific patterns.

_{Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.}
_{Reply @AikidoSec ignore: [REASON] to ignore this issue.}
_{More info}

aikido-pr-checks · 2026-04-16T12:56:42Z

+                if (hostname.charAt(i) > 0x7F) {
+                    try {
+                        hostname = IDN.toASCII(hostname, IDN.ALLOW_UNASSIGNED);
+                    } catch (IllegalArgumentException ignored) {


Empty catch of IllegalArgumentException in the hostname normalization block silently swallows errors; add logging or explicit handling so failures are observable.

Suggested change

} catch (IllegalArgumentException ignored) {

} catch (IllegalArgumentException e) {

// IDN conversion failed; log and continue with original hostname

System.out.println("AIKIDO: Failed to convert hostname to ASCII: " + e.getMessage());

Details

✨ AI Reasoning
A new OnMethodEnter wrapper was added to convert internationalized hostnames to ASCII. Inside that addition, an empty catch block catches IllegalArgumentException and does nothing. Silently swallowing this exception hides parsing/conversion failures and makes debugging harder. The catch contains no handling, logging, or comment explaining intent; it neither rethrows nor records the error. This harms maintainability and can obscure failures in hostname normalization during DNS resolution.

_{Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.}
_{Reply @AikidoSec ignore: [REASON] to ignore this issue.}
_{More info}

Update qa tests to v1.0.12

632062a

bitterpanda63 added 7 commits April 14, 2026 13:32

Merge branch 'main' into update-qa-tests

6698e27

rename an outbound request to an oubtound connection for qa tests

2b37ebf

rename not in allowlist -> not allowed for qa tests

e2e233e

IPList: Check for ipv4 mapped ipv6 addresses

8e49f49

Add a bypassed ip store for stored ssrf

afeba2d

remove the skip_tests to view work

7177e50

Outbound domain blocking: CHeck bypassed ip store

64b2cc2

aikido-pr-checks Bot reviewed Apr 14, 2026

View reviewed changes

bitterpanda63 added 4 commits April 14, 2026 14:40

Merge branch 'fix-ipv4-mapped-ipv6-for-bypassed-ips' into update-qa-t…

cdc06ae

…ests

draft: add DangerousBodyException

b595c3e

Update dns record collector + test cases for outbound issues

31e9c70

Merge pull request #283 from AikidoSec/dangerous-bodies-update

7caa870

draft: add DangerousBodyException

aikido-pr-checks Bot reviewed Apr 14, 2026

View reviewed changes

bitterpanda63 added 2 commits April 16, 2026 13:51

Merge branch 'main' into update-qa-tests

2364aea

Add helper for non-ascii hostnames

49c169d

aikido-pr-checks Bot reviewed Apr 16, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update qa tests to v1.0.12#280

Update qa tests to v1.0.12#280
bitterpanda63 wants to merge 14 commits intomainfrom
update-qa-tests

bitterpanda63 commented Apr 13, 2026 •

edited by aikido-pr-checks Bot

Loading

Uh oh!

codecov Bot commented Apr 13, 2026 •

edited

Loading

Uh oh!

aikido-pr-checks Bot Apr 14, 2026

Uh oh!

aikido-pr-checks Bot Apr 14, 2026

Uh oh!

aikido-pr-checks Bot Apr 16, 2026

Uh oh!

aikido-pr-checks Bot Apr 16, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

-                    } catch (IllegalArgumentException ignored) {
+                    } catch (IllegalArgumentException e) {
+                        // IDN conversion failed; log and continue with original hostname
+                        System.out.println("AIKIDO: Failed to convert hostname to ASCII: " + e.getMessage());

Conversation

bitterpanda63 commented Apr 13, 2026 • edited by aikido-pr-checks Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by Aikido

Uh oh!

codecov Bot commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

aikido-pr-checks Bot Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

aikido-pr-checks Bot Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

aikido-pr-checks Bot Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

aikido-pr-checks Bot Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

bitterpanda63 commented Apr 13, 2026 •

edited by aikido-pr-checks Bot

Loading

codecov Bot commented Apr 13, 2026 •

edited

Loading

aikido-pr-checks Bot Apr 16, 2026 •

edited

Loading