Adding `Valve` workaround for `GHSA-95v2-fvxr-qg83` by iannesbitt · Pull Request #12 · DataONEorg/d1_common_java

iannesbitt · 2026-07-10T19:53:19Z

This is a workaround for a soon-to-be-released advisory GHSA-95v2-fvxr-qg83 to be enabled in tomcat's server.xml with versions of metacat < 3.5.0. It uses a conservative URL pattern denial strategy to catch and deny potential filesystem path traversals in a request prior to being forwarded to the webapp. It is a temporary fix for affected installations (Metacat 3.4.2 and below) and is intended to be an intermediate step towards upgrading Metacat as soon as possible.

Installation

Enable this module for the Metacat host by inserting the following line into server.xml (in the <Host ...> element):

     <Valve className="org.dataone.security.TemporaryMitigationValve" />

Restart tomcat after the Valve is in place:

systemctl restart tomcat9

… suggestion for metacat<3.5.0

Copilot

Pull request overview

This PR introduces a Tomcat Valve implementation intended as a temporary mitigation for GHSA-95v2-fvxr-qg83 by rejecting requests whose URI/query contain path-traversal-style patterns (including across multiple URL-decoding rounds) before forwarding to the webapp.

Changes:

Add TemporaryMitigationValve (Tomcat ValveBase) that inspects request targets and denies suspicious patterns with HTTP 400.
Implement multi-round URL decoding to catch double-encoded traversal attempts.

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

… `null` when an illegal sequence is found

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.

iannesbitt · 2026-07-10T20:30:55Z

+    private static final Pattern SUSPICIOUS = Pattern.compile(
+        "(?i)(\\.{2}|%2e|%2f|%5c|\\\\|/\\./|/\\.\\./|%252e|%252f|%255c)"
+    );


Leaning towards erring on the stricter side unless others disagree

Copilot · 2026-07-10T20:25:59Z

+    private boolean isSuspicious(String input) {
+        String current = input;
+        for (int i = 0; i < MAX_DECODE_ROUNDS; i++) {
+            if (SUSPICIOUS.matcher(current).find()) {
+                return true;
+            }
+            String decoded = decodeOnce(current);
+            if (decoded == null) {
+                return true;
+            }
+            if (decoded.equals(current)) {
+                break;
+            }
+            current = decoded;
+        }
+        return SUSPICIOUS.matcher(current).find();
+    }


Implemented in f7c7825 by adding focused JUnit coverage for TemporaryMitigationValve that exercises representative accepted and rejected request targets, including plain/encoded traversal and malformed % encoding cases.

asked copilot to add unit tests

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

…ity declarations in MIME multipart data, and adding accompanying tests

Copilot

Pull request overview

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

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

artntek

part-reviewed; will continue after dependencies fixed

artntek · 2026-08-11T18:08:32Z

+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletResponse;


I believe jakarta.servlet.* is specific to Tomcat v10+. For Tomcat 9, these should instead be javax.servlet.*

I've changed these in bb952c7

artntek · 2026-08-11T18:11:04Z

+    private final XmlSecurityValidationValve valve = new XmlSecurityValidationValve();
+
+    @Test
+    public void identifiesXmlFromContentType() {


can we also assert some expected failures?

Adding these shortly...

Added tests with broken xml in 41a1852

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

iannesbitt · 2026-08-11T20:06:28Z

@artntek it's compiling cleanly for me now, and I added a couple of negative tests to the test suite. Let me know what else you find. Thank you! 🙏

artntek

looks good! One minor simplification

artntek · 2026-08-11T20:56:56Z

-                ByteArrayOutputStream baos = new ByteArrayOutputStream();
-                byte[] buffer = new byte[1024];
+                ByteArrayOutputStream baos = new ByteArrayOutputStream(
+                        declaredLength > 0 && declaredLength <= Integer.MAX_VALUE ? (int) declaredLength : 1024);


declaredLength <= Integer.MAX_VALUE is always true (so can be omitted, to simplify), because you already checked for if (declaredLength > maxRequestBytes) on line 46

adding Valve workaround for GHSA-95v2-fvxr-qg83 with implementation…

57cd920

… suggestion for metacat<3.5.0

iannesbitt requested review from artntek, Copilot, mbjones and taojing2002 July 10, 2026 19:53

iannesbitt self-assigned this Jul 10, 2026

Copilot started reviewing on behalf of iannesbitt July 10, 2026 19:53 View session

Copilot AI reviewed Jul 10, 2026

View reviewed changes

Comment thread src/main/java/org/dataone/security/TemporaryMitigationValve.java Outdated

Comment thread src/main/java/org/dataone/security/TemporaryMitigationValve.java Outdated

Comment thread src/main/java/org/dataone/security/TemporaryMitigationValve.java Outdated

iannesbitt added 3 commits July 10, 2026 12:58

correcting filename

896e359

adding license statement

c88caa3

addressing suggestion not to return a magic string but instead return…

347f2d7

… `null` when an illegal sequence is found

iannesbitt requested a review from Copilot July 10, 2026 20:09

Copilot started reviewing on behalf of iannesbitt July 10, 2026 20:09 View session

Copilot AI reviewed Jul 10, 2026

View reviewed changes

adding logging

705c789

Copilot started work on behalf of iannesbitt July 10, 2026 20:23 View session

test: add focused TemporaryMitigationValve regression coverage

f7c7825

Copilot finished work on behalf of iannesbitt July 10, 2026 20:26

iannesbitt and others added 2 commits July 10, 2026 13:32

Implementing docstring suggestion for explicit code references

d0d106c

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

changing security module to exclude external DTD declarations and ent…

d7f6b69

…ity declarations in MIME multipart data, and adding accompanying tests

iannesbitt requested a review from Copilot July 13, 2026 13:26

Copilot started reviewing on behalf of iannesbitt July 13, 2026 13:32 View session

Copilot AI reviewed Jul 13, 2026

View reviewed changes

iannesbitt and others added 3 commits July 13, 2026 07:30

correcting imports

f1ab1eb

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

correcting class name in docstring

267965e

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

avoiding case sensitivity issue

63bf0c6

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

artntek requested changes Aug 11, 2026

View reviewed changes

iannesbitt added 3 commits August 11, 2026 12:07

removing license statement

e02379d

fixing imports

bb952c7

adding tomcat to pom.xml imports

57d018b

iannesbitt and others added 2 commits August 11, 2026 12:56

Apply suggestions from code review

a103c2e

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

adding negative tests

41a1852

iannesbitt requested a review from artntek August 11, 2026 20:05

artntek requested changes Aug 11, 2026

View reviewed changes

		import jakarta.servlet.ServletException;
		import jakarta.servlet.http.HttpServletResponse;

Uh oh!

Conversation

iannesbitt commented Jul 10, 2026

Installation

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

artntek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iannesbitt commented Aug 11, 2026

Uh oh!

artntek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants