fix(gchat): per-shape verification + dataclass field order + test env isolation (codex review)

Addresses three findings from Codex's review of #130.

1. Per-shape webhook verification (P1, security).

   handle_webhook accepts BOTH direct Google Chat webhooks AND Pub/Sub push
   shapes on a single endpoint. The constructor's "at least one verifier OR
   disable_signature_verification" check is a coarse gate: an operator who
   configured only pubsub_audience left the direct-webhook shape going through
   the warned-but-unverified path (and symmetrically for project_number vs
   Pub/Sub). An attacker could pick the unconfigured shape to bypass the
   configured verifier.

   handle_webhook now REJECTS (401) any shape whose verifier isn't configured,
   unless disable_signature_verification is explicitly set -- in which case
   the existing warn-and-process path is preserved. Mirrors upstream
   adapter-gchat/src/index.ts (vercel/chat) faithfully; upstream solves this
   the same way (per-path require, construct-time stays any-of).

2. GoogleChatAdapterConfig.disable_signature_verification field order (P2).

   The PR inserted the new field in the MIDDLE of the dataclass, shifting
   every later positional arg. A caller like
       GoogleChatAdapterConfig("creds", "audience", impersonate, logger)
   would silently route `impersonate` into `disable_signature_verification`.
   Moved the field to the END so existing positional callers keep working
   (the field has a default so trailing positional callers are unaffected).
   Added a regression test that pins the field's position via
   dataclasses.fields().

3. Env-var leakage in fail-closed tests (P2, test isolation).

   The env-fallback tests used a manual try/finally that only restored env
   vars SET before the test. Setting
   GOOGLE_CHAT_DISABLE_SIGNATURE_VERIFICATION=true in a test where the var
   was originally absent leaked it to later tests, silently satisfying the
   fail-closed gate in unrelated construction tests.

   Converted to pytest's monkeypatch fixture, which restores both was-set
   and was-absent cases at teardown. Added a load-bearing leak-detection
   test that exercises the previously-leaky pattern within a single test.

All three findings have load-bearing tests added under tests/test_gchat_verification.py:
- TestPerShapeVerificationRejection (3 tests; first two would 200 instead of 401 pre-fix)
- TestDisableSignatureVerificationFieldOrder (2 tests; both fail pre-fix)
- TestDisableSignatureVerificationEnvFallback::test_env_does_not_leak_to_subsequent_construction

Validation: uv run ruff check, ruff format --check, audit_test_quality.py
(0 hard failures), full pytest (4063 passed, 3 skipped).

https://claude.ai/code/session_01FyMxQn2BEAzmwKS1GZczKj

Author: claude

src/chat_sdk/adapters/google_chat/adapter.py

@@ -878,31 +878,59 @@ async def handle_webhook(
             and parsed["message"].get("data")
             and parsed.get("subscription")
         ):
-            # Verify Pub/Sub JWT if audience is configured
+            # Verify Pub/Sub JWT if audience is configured. The two transports
+            # share a single endpoint, so each shape must be independently
+            # verified -- otherwise an attacker could send the unconfigured
+            # shape to bypass the configured verifier. The constructor's "at
+            # least one verifier OR disable_signature_verification" check is
+            # not sufficient here for that reason. Mirrors upstream
+            # adapter-gchat/src/index.ts.
             if self._pubsub_audience:
                 valid = await self._verify_bearer_token(request, self._pubsub_audience)
                 if not valid:
                     return {"body": "Unauthorized", "status": 401}
-            elif not self._warned_no_pubsub_verification:
-                self._warned_no_pubsub_verification = True
+            elif self._disable_signature_verification:
+                if not self._warned_no_pubsub_verification:
+                    self._warned_no_pubsub_verification = True
+                    self._logger.warn(
+                        "Pub/Sub webhook verification is disabled. "
+                        "Set GOOGLE_CHAT_PUBSUB_AUDIENCE or pubsubAudience to verify incoming requests."
+                    )
+            else:
+                # Direct-webhook verifier may be configured but Pub/Sub
+                # verifier is not -- reject rather than silently process an
+                # unverified payload that a peer transport's config does
+                # nothing to authenticate.
                 self._logger.warn(
-                    "Pub/Sub webhook verification is disabled. "
-                    "Set GOOGLE_CHAT_PUBSUB_AUDIENCE or pubsubAudience to verify incoming requests."
+                    "Rejected Pub/Sub webhook: pubsub_audience is not configured. "
+                    "Set GOOGLE_CHAT_PUBSUB_AUDIENCE, or set "
+                    "disable_signature_verification to accept unverified Pub/Sub payloads."
                 )
+                return {"body": "Unauthorized", "status": 401}
             return self._handle_pub_sub_message(parsed, options)
 
-        # Verify direct Google Chat webhook JWT if project number is configured
+        # Verify direct Google Chat webhook JWT if project number is configured.
+        # Same reasoning as the Pub/Sub branch: each shape requires its own
+        # verifier (or explicit opt-out) to prevent cross-transport bypass.
         if self._google_chat_project_number:
             valid = await self._verify_bearer_token(request, self._google_chat_project_number)
             if not valid:
                 return {"body": "Unauthorized", "status": 401}
-        elif not self._warned_no_webhook_verification:
-            self._warned_no_webhook_verification = True
+        elif self._disable_signature_verification:
+            if not self._warned_no_webhook_verification:
+                self._warned_no_webhook_verification = True
+                self._logger.warn(
+                    "Google Chat webhook verification is disabled. "
+                    "Set GOOGLE_CHAT_PROJECT_NUMBER or googleChatProjectNumber "
+                    "to verify incoming requests."
+                )
+        else:
             self._logger.warn(
-                "Google Chat webhook verification is disabled. "
-                "Set GOOGLE_CHAT_PROJECT_NUMBER or googleChatProjectNumber "
-                "to verify incoming requests."
+                "Rejected direct Google Chat webhook: google_chat_project_number "
+                "is not configured. Set GOOGLE_CHAT_PROJECT_NUMBER, or set "
+                "disable_signature_verification to accept unverified payloads."
             )
+            return {"body": "Unauthorized", "status": 401}
 
         # Treat as a direct Google Chat webhook event
         event: dict[str, Any] = parsed

src/chat_sdk/adapters/google_chat/types.py

@@ -301,15 +301,6 @@ class GoogleChatAdapterConfig:
     # Google Cloud project number for verifying direct webhook JWTs
     google_chat_project_number: str | None = None
 
-    # Explicit opt-in to disable webhook signature verification. Required to
-    # construct the adapter when neither google_chat_project_number nor
-    # pubsub_audience is configured. Without this flag the constructor raises
-    # ValidationError -- fail-closed by default. Only enable in development or
-    # when an upstream layer (e.g. authenticated Cloud Run invocations) provides
-    # equivalent guarantees. Falls back to the
-    # GOOGLE_CHAT_DISABLE_SIGNATURE_VERIFICATION env var when left unset (None).
-    disable_signature_verification: bool | None = None
-
     # User email to impersonate for Workspace Events API calls
     impersonate_user: str | None = None
 
@@ -324,3 +315,16 @@ class GoogleChatAdapterConfig:
 
     # Override bot username
     user_name: str | None = None
+
+    # Explicit opt-in to disable webhook signature verification. Required to
+    # construct the adapter when neither google_chat_project_number nor
+    # pubsub_audience is configured. Without this flag the constructor raises
+    # ValidationError -- fail-closed by default. Only enable in development or
+    # when an upstream layer (e.g. authenticated Cloud Run invocations) provides
+    # equivalent guarantees. Falls back to the
+    # GOOGLE_CHAT_DISABLE_SIGNATURE_VERIFICATION env var when left unset (None).
+    #
+    # Kept at the END of the field list intentionally: GoogleChatAdapterConfig
+    # is a positional-args dataclass, so inserting a new field in the middle
+    # would silently shift every later positional arg for existing callers.
+    disable_signature_verification: bool | None = None