瀏覽代碼

fix: scrub raw JWTs and short Bearers in log output (SEC_REVIEW F65)

The Monolog `SecretScrubbingProcessor` (mirrored on api and ui)
caught Bearer tokens with `[A-Za-z0-9._\-]{20,}` and irdb-format
tokens with their own pattern. Two gaps the SEC_REVIEW called out:

  1. A short Bearer (< 20 chars) like `Bearer abc12345` slipped
     through because the value-pattern floor was {20,}.
  2. A raw JWT logged under any key not in the sensitive-key list
     (e.g. `'jwt' => '...'`, `'id_token' => '...'`,
     `'access_token' => '...'` if those exact names ever drifted)
     escaped the scrubber entirely.

Add two value patterns to both processors:

  - `\beyJ[A-Za-z0-9_-]{4,}\.[A-Za-z0-9_-]{4,}\.[A-Za-z0-9_-]{4,}\b`
    → `eyJ***`. Anchored on `eyJ` because every JWT header is the
    base64url of a JSON object starting with `{"…`, which always
    encodes to `eyJ…`. Anchoring eliminates false positives like
    `192.168.1.1` (dotted IP) or `lib.so.6` (shared-object name)
    or any other `a.b.c`-shaped prose. The per-segment `{4,}`
    floor skips pathologically short matches.
  - Bearer floor lowered from {20,} to {8,}.

The replacement keeps a triage breadcrumb in the output:
`Bearer ***` for short Bearers, `eyJ***` for raw JWTs (the prefix
identifies "this was a JWT" without leaking the secret half).

Regression tests added on both sides:

  - api: `testRawJwtInValueIsScrubbed`,
    `testRawJwtInMessageIsScrubbed`,
    `testShortBearerTokenIsScrubbed`, plus
    `testIpAddressDoesNotMatchJwtRegex` as a false-positive guard
    (a dotted-quad IP and a `lib.so.6`-style version string must
    NOT trigger the JWT pattern because they don't start with
    `eyJ`).
  - ui: `testRawJwtInValueIsScrubbed`, `testShortBearerIsScrubbed`.

All existing scrubber tests still pass on both sides.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chiappa 3 天之前
父節點
當前提交
c439ce1db3

+ 15 - 1
api/src/Infrastructure/Logging/SecretScrubbingProcessor.php

@@ -67,7 +67,21 @@ final class SecretScrubbingProcessor implements ProcessorInterface
         // Bearer header value, with or without the keyword. Replaces the
         // value but keeps the kind prefix as a triage breadcrumb.
         ['/(Bearer\s+irdb_(?:rep|con|adm|svc)_)[A-Z2-7]{32}/', '$1***'],
-        ['/(Bearer\s+)[A-Za-z0-9._\-]{20,}/', '$1***'],
+        // SEC_REVIEW F65: Bearer with any non-trivial value. The
+        // floor was {20,} which let a < 20-char Bearer slip through;
+        // dropped to {8,} which still excludes the common literal
+        // strings without false-positive matching prose.
+        ['/(Bearer\s+)[A-Za-z0-9._\-]{8,}/', '$1***'],
+        // SEC_REVIEW F65: raw JWT (`header.payload.signature`)
+        // anywhere in the message or value. Anchored on `eyJ`
+        // because every JWT header is the base64url encoding of a
+        // JSON object that starts with `{"…`, which is `eyJ…`.
+        // Anchoring eliminates false positives like `192.168.1.1`
+        // or `lib.so.6` — those don't start with `eyJ`. Each
+        // segment requires ≥4 chars to skip pathological short
+        // matches. The replacement keeps the `eyJ` prefix as a
+        // triage breadcrumb.
+        ['/\beyJ[A-Za-z0-9_-]{4,}\.[A-Za-z0-9_-]{4,}\.[A-Za-z0-9_-]{4,}\b/', 'eyJ***'],
         // Bare irdb_<kind>_<32 base32> tokens that aren't preceded by Bearer.
         ['/\birdb_(rep|con|adm|svc)_[A-Z2-7]{32}\b/', 'irdb_$1_***'],
         // Argon2 password hashes.

+ 61 - 0
api/tests/Unit/Logging/SecretScrubbingProcessorTest.php

@@ -106,6 +106,67 @@ final class SecretScrubbingProcessorTest extends TestCase
         self::assertSame('user search hit', $out->message);
     }
 
+    public function testRawJwtInValueIsScrubbed(): void
+    {
+        // SEC_REVIEW F65: a JWT logged under any key (not in the
+        // sensitive-key list) must still be scrubbed by the value
+        // pattern. JWTs always start `eyJ` (base64url of `{"`), so the
+        // pattern is anchored on that.
+        $processor = new SecretScrubbingProcessor();
+        $jwt = 'eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiIxMjM0NTYifQ.qj8u_Mt1ZyT5PfksI91X4-3aBwQ-_Pwm';
+        $record = $this->record('upstream returned id_token', [
+            'jwt' => $jwt,
+        ]);
+
+        $out = $processor($record);
+
+        self::assertSame('eyJ***', $out->context['jwt']);
+    }
+
+    public function testRawJwtInMessageIsScrubbed(): void
+    {
+        $processor = new SecretScrubbingProcessor();
+        $jwt = 'eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiI3In0.q-MT_OmW-fakesigvaluehere';
+        $record = $this->record("decoded payload from {$jwt} ok", []);
+
+        $out = $processor($record);
+
+        self::assertStringContainsString('eyJ***', $out->message);
+        self::assertStringNotContainsString($jwt, $out->message);
+    }
+
+    public function testShortBearerTokenIsScrubbed(): void
+    {
+        // SEC_REVIEW F65: the previous floor of {20,} let a short
+        // Bearer slip through. The new floor is {8,}.
+        $processor = new SecretScrubbingProcessor();
+        $record = $this->record('outbound', [
+            'auth' => 'Bearer abc12345',
+        ]);
+
+        $out = $processor($record);
+
+        // 'auth' isn't in the key-name list, so we exercise the value
+        // pattern. The Bearer prefix stays as a triage breadcrumb.
+        self::assertSame('Bearer ***', $out->context['auth']);
+    }
+
+    public function testIpAddressDoesNotMatchJwtRegex(): void
+    {
+        // False-positive guard: dotted-quad IP looks like
+        // `int.int.int` but doesn't start with `eyJ`, so it's
+        // outside the JWT pattern.
+        $processor = new SecretScrubbingProcessor();
+        $record = $this->record('handling 192.168.1.1 request', [
+            'ip' => '203.0.113.42',
+        ]);
+
+        $out = $processor($record);
+
+        self::assertSame('203.0.113.42', $out->context['ip']);
+        self::assertStringContainsString('192.168.1.1', $out->message);
+    }
+
     public function testNestedContextIsScrubbedRecursively(): void
     {
         $processor = new SecretScrubbingProcessor();

+ 5 - 1
ui/src/Logging/SecretScrubbingProcessor.php

@@ -41,7 +41,11 @@ final class SecretScrubbingProcessor implements ProcessorInterface
      */
     private const VALUE_PATTERNS = [
         ['/(Bearer\s+irdb_(?:rep|con|adm|svc)_)[A-Z2-7]{32}/', '$1***'],
-        ['/(Bearer\s+)[A-Za-z0-9._\-]{20,}/', '$1***'],
+        // SEC_REVIEW F65: Bearer floor lowered from {20,} to {8,}.
+        ['/(Bearer\s+)[A-Za-z0-9._\-]{8,}/', '$1***'],
+        // SEC_REVIEW F65: raw JWTs anchored on `eyJ` (base64url of
+        // `{"`); see api-side processor for the full rationale.
+        ['/\beyJ[A-Za-z0-9_-]{4,}\.[A-Za-z0-9_-]{4,}\.[A-Za-z0-9_-]{4,}\b/', 'eyJ***'],
         ['/\birdb_(rep|con|adm|svc)_[A-Z2-7]{32}\b/', 'irdb_$1_***'],
         ['/\$argon2(?:i|id|d)\$[^\s\'"]+/', '$argon2***'],
         ['/\$2[aby]?\$\d{2}\$[A-Za-z0-9.\/]{53}/', '$2***'],

+ 22 - 0
ui/tests/Unit/Logging/SecretScrubbingProcessorTest.php

@@ -60,6 +60,28 @@ final class SecretScrubbingProcessorTest extends TestCase
         self::assertSame('203.0.113.42', $out->context['q']);
     }
 
+    public function testRawJwtInValueIsScrubbed(): void
+    {
+        // SEC_REVIEW F65: JWTs anchored on `eyJ` (base64url of `{"`)
+        // are scrubbed regardless of the key they're logged under.
+        $processor = new SecretScrubbingProcessor();
+        $jwt = 'eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiI3In0.qj8u_Mt1ZyT5PfksI91X4Aaa';
+        $record = $this->record('oidc id_token captured', ['jwt' => $jwt]);
+
+        $out = $processor($record);
+        self::assertSame('eyJ***', $out->context['jwt']);
+    }
+
+    public function testShortBearerIsScrubbed(): void
+    {
+        // SEC_REVIEW F65: Bearer floor lowered from {20,} to {8,}.
+        $processor = new SecretScrubbingProcessor();
+        $record = $this->record('outbound', ['auth' => 'Bearer abcd1234']);
+
+        $out = $processor($record);
+        self::assertSame('Bearer ***', $out->context['auth']);
+    }
+
     /**
      * @param array<string, mixed> $context
      */