From 72a78f63d47bdccf78bd272bce8c83038439ce43 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Mon, 27 Apr 2026 16:15:54 +0000
Subject: [PATCH 1/2] Initial plan


From 7bcd1ab1f586005f7f144a1b649f7bb27df296ac Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Mon, 27 Apr 2026 16:35:14 +0000
Subject: [PATCH 2/2] Tighten regression tests for oauth2 decrypt token fix

- Match unique error message 'plaintext is not a valid header value' instead of
  generic 'failed to decrypt token' prefix in both DecryptTokenSpuriousSuccessReturnsOriginalInput
  and GarbagePlaintextCookieDoesNotCrash tests
- Add ASSERT_TRUE(cookies.contains('RefreshToken')) before cookies.at() in
  GarbagePlaintextCookieDoesNotCrash to avoid out_of_range on cookie drop
- Add EXPECT_CALL for 302 redirect in GarbagePlaintextCookieDoesNotCrash using
  Truly() lambda checking Status=='302' and Location non-empty

Agent-Logs-Url: https://github.com/envoyproxy/envoy/sessions/5d01f403-a691-47e4-8316-e8f2b7d29d5d

Co-authored-by: phlax <454682+phlax@users.noreply.github.com>
---
 .../filters/http/oauth2/filter_test.cc        | 26 +++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/test/extensions/filters/http/oauth2/filter_test.cc b/test/extensions/filters/http/oauth2/filter_test.cc
index 10a41e7dda8f1..434f348d9f872 100644
--- a/test/extensions/filters/http/oauth2/filter_test.cc
+++ b/test/extensions/filters/http/oauth2/filter_test.cc
@@ -5476,7 +5476,12 @@ TEST_F(OAuth2Test, DecryptTokenSpuriousSuccessReturnsOriginalInput) {
   // key, so EVP_DecryptFinal_ex succeeds, but the resulting plaintext fails headerValueIsValid.
   const std::string binary_plaintext("\x00\x01\x02\xff", 4);
   const std::string ciphertext = filter_->encryptToken(binary_plaintext);
-  EXPECT_LOG_CONTAINS("error", "failed to decrypt token",
+  // Tighter assertion: match on the message that is unique to the new headerValueIsValid branch
+  // ("plaintext is not a valid header value"), rather than the generic "failed to decrypt token"
+  // prefix that is shared with the pre-existing EVP_DecryptFinal_ex failure path. A regression
+  // that re-introduced the crash via the old error path would emit a different message and fail
+  // this test.
+  EXPECT_LOG_CONTAINS("error", "plaintext is not a valid header value",
                       { EXPECT_EQ(filter_->decryptToken(ciphertext), ciphertext); });
 }
 
@@ -5508,10 +5513,24 @@ TEST_F(OAuth2Test, GarbagePlaintextCookieDoesNotCrash) {
   EXPECT_CALL(*validator_, isValid()).WillOnce(Return(false));
   EXPECT_CALL(*validator_, canUpdateTokenByRefreshToken()).WillOnce(Return(false));
 
+  // Assert the redirect actually happened: filter took the redirect path (302 with Location)
+  // rather than crashing or returning a 401. We only check Status=="302" and Location is set
+  // to avoid brittleness from Set-Cookie values, nonce, and CSRF in the full header map.
+  // redirectToOAuthServer calls decoder_callbacks_->encodeHeaders with end_stream=true.
+  EXPECT_CALL(decoder_callbacks_,
+              encodeHeaders_(testing::Truly([](const Http::ResponseHeaderMap& headers) {
+                               return headers.getStatusValue() == "302" &&
+                                      !headers.getLocationValue().empty();
+                             }),
+                             true));
+
+  // Tighter assertion: match on the message that is unique to the new headerValueIsValid branch.
+  // A regression that re-introduced the crash via the old EVP_DecryptFinal_ex-failure path would
+  // emit a different message and cause this test to fail.
   // Before the fix this line would trigger a crash (ENVOY_BUG assertion in HeaderStringValidator).
   // After the fix it should complete without crashing and redirect to the OAuth server.
   EXPECT_LOG_CONTAINS(
-      "error", "failed to decrypt token",
+      "error", "plaintext is not a valid header value",
       {
         EXPECT_EQ(Http::FilterHeadersStatus::StopIteration,
                   filter_->decodeHeaders(request_headers, false));
@@ -5520,6 +5539,9 @@ TEST_F(OAuth2Test, GarbagePlaintextCookieDoesNotCrash) {
   // After the fix, the Cookie header should contain the original (valid ASCII) ciphertext,
   // not the binary garbage that decryption produced.
   auto cookies = Http::Utility::parseCookies(request_headers);
+  // Use ASSERT_TRUE first so the test fails clearly (not with out_of_range) if the cookie were
+  // dropped instead of preserved.
+  ASSERT_TRUE(cookies.contains("RefreshToken"));
   EXPECT_EQ(cookies.at("RefreshToken"), garbage_ciphertext);
 }