Skip to content

ascanrulesBeta: Clarify Proxy Disclosure alert details#7407

Open
kingthorin wants to merge 1 commit into
zaproxy:mainfrom
kingthorin:proxy-det-fixes
Open

ascanrulesBeta: Clarify Proxy Disclosure alert details#7407
kingthorin wants to merge 1 commit into
zaproxy:mainfrom
kingthorin:proxy-det-fixes

Conversation

@kingthorin

@kingthorin kingthorin commented May 29, 2026

Copy link
Copy Markdown
Member

Overview

I decided to tackle these separate from 5718 as it was too busy/fragmented.

Improves Proxy Disclosure scan rule (40025) alert quality per Issue 8556:

  • Evidence — Sets evidence to matched proxy request header from TRACE body (e.g. X-Forwarded-For: 10.0.0.1)
  • Other Info — For unfingerprinted ("Unknown") nodes, notes which header triggered detection
  • Attack field — No longer populated (not literal attack payload, was assembled string previously)
  • Description — Fixes broken grammar in alert description text
  • Docs/tests — Updates primary Messages.properties, English help, example alerts, and unit tests

Related Issues

@thc202 thc202 changed the title Clarify Proxy Disclosure alert details ascanrulesBeta: Clarify Proxy Disclosure alert details May 29, 2026
@psiinon

psiinon commented May 29, 2026

Copy link
Copy Markdown
Member

Logo
Checkmarx One – Scan Summary & Detailsb0880232-b44c-490f-bfa3-30e043a1f458

Great job! No new security vulnerabilities introduced in this pull request


Use @Checkmarx to interact with Checkmarx PR Assistant.
Examples:
@Checkmarx how are you able to help me?
@Checkmarx rescan this PR

@kingthorin

Copy link
Copy Markdown
Member Author

Sorry for the additional pushes, I noticed a few things needed cleaning up. Should be stable now.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the quality and clarity of the Proxy Disclosure active scan rule (40025) alerts in the ascanrulesBeta add-on, aligning the alert output with the intended meaning of the findings (and Issue 8556).

Changes:

  • Populate Evidence with the matched proxy request header echoed in TRACE responses, and stop populating the Attack field.
  • Improve Other Info by indicating which proxy request header triggered detection when a node is unfingerprinted/unknown.
  • Update associated documentation/help text, i18n strings, example alerts, and unit tests to reflect the new alert details.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
addOns/ascanrulesBeta/src/main/java/org/zaproxy/zap/extension/ascanrulesBeta/ProxyDisclosureScanRule.java Captures proxy header evidence and improves Other Info generation; removes Attack population; updates example alerts.
addOns/ascanrulesBeta/src/main/resources/org/zaproxy/zap/extension/ascanrulesBeta/resources/Messages.properties Updates Proxy Disclosure description text and adds a new Other Info i18n string.
addOns/ascanrulesBeta/src/main/javahelp/org/zaproxy/zap/extension/ascanrulesBeta/resources/help/contents/ascanbeta.html Updates Proxy Disclosure help content to describe new evidence/other-info behavior.
addOns/ascanrulesBeta/src/test/java/org/zaproxy/zap/extension/ascanrulesBeta/ProxyDisclosureScanRuleUnitTest.java Adjusts example-alert assertions for evidence/attack/description/other-info changes.
addOns/ascanrulesBeta/CHANGELOG.md Documents the change under Unreleased.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread addOns/ascanrulesBeta/CHANGELOG.md Outdated
@kingthorin kingthorin force-pushed the proxy-det-fixes branch 2 times, most recently from cef694e to 4412ff4 Compare May 29, 2026 15:51
Signed-off-by: kingthorin <kingthorin@users.noreply.github.com>
@kingthorin

Copy link
Copy Markdown
Member Author

Now with copilot items addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants