The remote connector in lab/connector/ went through three adversarial security reviews
before it ever ran against real Google. Every finding is listed here with what was done
about it and the test that now pins it, so the reasoning survives the session that did
the work. SCSP’s connector will be built from this one, so most of these lessons are
SCSP’s too (see ../HANDOFF.md, “Facts learned”).
How each round worked. Reviewers each took one angle and reported findings with a concrete failure scenario. Duplicates were merged. Each finding then faced three skeptical verifiers, told to refute it: one tried to reproduce it, one judged its real impact, and one checked every factual claim against the code, the MCP SDK’s source and the specs. A finding counted as confirmed when at least two of the three could not refute it. A final critic looked for what the reviewers had missed. Rounds two and three also drove real headless Chrome and Edge through the app over CDP, read Claude Code 2.1.263’s own binary to see how it really behaves, and broke each protection on purpose in scratch copies to see whether a test noticed (mutation testing). Scratch work stayed outside the repo.
| Round | Reviewed | Agents | Findings (raw → distinct → confirmed) | Fixed in |
|---|---|---|---|---|
| 1 | The first draft, 6499278 |
97 | 56 → 30 → 30 | ebcae39 |
| 2 | The fixed code, ebcae39 |
78 | 35 → 23 → 22 | 74f726a |
| 3 | Round two’s changes, 74f726a |
57 | 17 → 17 → 16 | 48d8e97 |
After round three, the whole sign-in works in real Chrome and Edge over http and https, and each of 11 regressions applied on purpose to scratch copies made a test fail.
| # | Severity | Finding | What was done; the test that pins it |
|---|---|---|---|
| 0 | medium | Redirects to the client carried no iss (RFC 9207), so a client using several servers could be mixed up |
Every redirect carries iss equal to the metadata’s issuer, and the metadata says so. test_signed_in_person_gets_a_code_with_exactly_one_iss, test_metadata_advertises_iss |
| 1 | medium | The refresh grace window forked token chains, and reuse never revoked anything | Sessions: one current refresh token; a retry within 60 s gets the same answer; any other reuse ends the whole session. test_a_lost_response_can_be_retried_for_a_minute, test_reuse_after_the_minute_ends_the_whole_session |
| 2 | medium | Refresh tokens had no absolute lifetime and never sent anyone back to Google | Sessions last 7 days from the Google sign-in; no token outlives its session. test_session_lasts_seven_days_then_google_again |
| 3 | medium | Revocation did nothing for Claude: the SDK’s /revoke needs a client_secret, only one token died, and nothing was durable | Our own /revoke takes public clients and ends the whole session. test_claude_can_revoke_and_it_ends_the_session, test_revoking_the_access_token_ends_the_session_too |
| 4 | medium | OAuth state lived in process memory, the one-instance condition had been dropped, and the docstring overstated it | Fails closed: tokens need a live session in the store, so a restart signs everyone out; one instance is documented; a shared store is Session C’s decision. test_a_restart_signs_everyone_out |
| 5 | low | Google’s answer was accepted without the hd check and keyed on the email, not Google’s stable ID |
hd must equal the email’s domain; Google’s sub is required and carried in codes, sessions and the record. test_google_answer_must_check_out |
| 6 | low | No consent of our own, and Google’s round trip was not tied to the browser (the MCP spec’s confused deputy) | A CSRF-protected consent page, then a cookie that only the approving browser holds. test_consent_needs_this_browser_and_this_page, test_callback_needs_the_browser_that_approved |
| 7 | low | Non-ASCII input crashed the signature check with a 500 | Non-ASCII values are refused before any work. test_odd_bytes_are_refused_not_a_crash, test_state_must_be_ours_and_fresh |
| 8 | medium | Google’s error parameter went into the log raw, so anyone could forge audit lines |
Google errors map to a fixed set, and every audit line is one JSON object. test_google_error_text_never_reaches_the_log_verbatim, test_on_cloud_run_each_line_is_one_json_object |
| 9 | low | /diag told anyone the tool list and how many people were allowed | /diag answers liveness only. test_diag_says_nothing_about_tools_or_people |
| 10 | low | Nothing tested refresh, revocation, reuse, the audience check or the OAuth refusals | tests/test_connector_tokens.py (19 tests) and the sign-in tests |
| 11 | nit | A client’s registered grant types were ignored, and the metadata hid the “none” method | Grant types are signed into the client ID and enforced; “none” is advertised. test_registered_grant_types_are_kept, test_only_the_methods_that_work_are_offered |
| 12 | low | A redirect URI with no host crashed the refusal’s own log line | Hosts are recorded safely (bounded in round 2). test_registration_refusals_are_recorded_small |
| 13 | medium | A failed tool’s traceback leaked request content (a Gmail query inside an HTTP error) | Failures log the exception type and file:line only. test_every_call_is_recorded_without_content |
| 14 | low | The untrusted-data frame markers could be faked from inside a result | Line breaks and the marker itself are escaped the JSON way, so the data reads back unchanged. test_text_inside_a_result_cannot_end_the_frame |
| 15 | low | Anyone allowed to sign in got the owner’s data, and the instructions said “your own” | Data tools go to the owner only, until step 6’s policy. test_the_owner_gets_the_data_tools_and_others_do_not, test_a_hidden_tool_cannot_be_called_by_name |
| 16 | low | The signing key was only checked for length, and /register gives an offline guessing oracle | The key must come from the generator (checksummed since round 2). test_signing_key_must_come_from_the_generator |
| 17 | low | Secrets showed in repr() |
Secret fields are hidden from repr. test_secrets_never_show_in_repr |
| 18 | low | Refresh exchanges left no record | token_refreshed is recorded, with whether it was a retry. test_refresh_is_recorded |
| 19 | nit | A refused call recorded whatever “tool name” the client sent | Recorded only if it looks like a tool name, otherwise <invalid>. test_every_call_is_recorded_without_content |
| 20 | nit | An expired gcloud login gave a raw traceback instead of the fix | It says to run gcloud auth application-default login. test_an_expired_gcloud_login_says_how_to_fix_it |
| 21 | low | Google’s ID token was checked with no clock skew allowed | 30 seconds of skew. test_clocks_may_differ_by_thirty_seconds |
| 22 | low | Real ID-token verification and the startup code were untested | The fake Google signs real RS256 tokens; tests/test_connector_main.py covers startup |
| 23 | low | Per-person filtering, error-path records, the 2026-07-28 protocol path and Host checks were untested | test_a_hidden_tool_cannot_be_called_by_name, test_mcp_needs_sign_in_on_the_2026_07_28_path_too, test_requests_for_another_host_or_from_a_web_page_are_refused |
| 24 | low | The record miscounted results for single-record tools | Each tool names the list it counts. test_results_are_counted_by_what_each_tool_returns, test_tools_count_their_results |
| 25 | low | whoami ran on the event loop for a reason that was wrong | Every tool runs in the tool threads with the request’s context. test_whoami_reports_the_google_identity |
| 26 | low | DNS-rebinding protection was switched off instead of pinned | Protection on, with the Host pinned to the connector’s hostname. test_requests_for_another_host_or_from_a_web_page_are_refused |
| 27 | low | Oversized registrations or state failed only after Google, as “expired” | Input bounds (completed in round 2). test_signed_values_stay_under_the_size_limit, test_a_too_large_request_is_refused_before_google |
| 28 | low | The record of use was not structured on Cloud Run and split across two loggers | One record() helper; one JSON object per line on Cloud Run. test_sign_in_and_tool_calls_share_one_record |
| 29 | nit | Dead or duplicated settings and code | The project comes from lab/config.py, only a Web client is accepted, the path is required. test_google_client_must_be_a_web_client, test_load_config_from_the_environment |
Round 1’s critic:
test_each_google_call_builds_its_own_clienttest_arguments_are_checked_before_the_tool_runstest_every_other_route_needs_a_token).allValues: ALLOW).test_listens_on_this_machine_unless_on_cloud_runPR #193’s reviewer:
github_pull_request. Fixed:
test_a_later_comment_does_not_hide_an_approval, test_reviews_are_read_page_by_page, test_a_review_list_cut_short_says_so
test_a_tool_call_goes_all_the_way_throughtest_calendar_days_are_the_owners_not_the_servers| # | Severity | Finding | What was done; the test that pins it |
|---|---|---|---|
| 0 | high | The consent page’s Referrer-Policy: no-referrer made browsers post its own form with Origin: null, and the Origin check refused every sign-in. Unit tests could not see it; real Chrome and Edge did |
same-origin. The test kit now sends the Origin a browser would, and a real MCP client drives the journey. test_consent_page_lets_the_browser_name_its_origin, test_a_real_mcp_client_signs_in_and_calls_a_tool |
| 1 | medium | A loopback redirect registered with ?iss=... put a second, attacker-chosen iss first, which Claude Code reads |
Loopback redirects take no query and no fragment. test_registration_refuses_anything_else, test_signed_in_person_gets_a_code_with_exactly_one_iss |
| 2 | medium | A Google answer refused in another browser left Google’s code unspent, and the request log handed it to whoever started the flow | The first arrival claims the approval (a boot ID and no access log were added in round 3). test_a_refused_arrival_cannot_be_replayed_from_the_log |
| 3 | medium | The record-of-use test never read the audit fields, where everything is written | Tests read the record as production writes it. test_every_call_is_recorded_without_content, test_sign_in_never_writes_a_secret |
| 4 | low | Refusals the SDK’s /authorize sends itself carried no iss |
A wrapper adds iss and records them. test_the_sdks_own_refusals_carry_iss_and_are_recorded |
| 5 | low | Revoking an access token past its hour did nothing | Revocation reads tokens whatever their expiry. test_revoking_an_expired_access_token_still_ends_the_session |
| 6 | low | After a proven return from Google, a refusal page never told the client | The page links back to the client with error, state and iss. test_refusals_after_google_offer_the_way_back_to_claude |
| 7 | low | Two sign-ins in one browser broke each other | Cookies are named for their own sign-in. test_two_sign_ins_in_one_browser_both_finish |
| 8 | low | A call the client abandoned ran on but left no record | Recorded as cancelled; Google calls time out after 20 s. test_a_call_the_client_abandons_is_still_recorded |
| 9 | low | An anonymous registration could write megabytes of hostnames into the record | Hosts are bounded. test_registration_refusals_are_recorded_small |
| 10 | low | Typed passphrases passed the key check | Keys carry a checksum and come from python -m lab.connector.signer. test_signing_key_must_come_from_the_generator |
| 11 | low | Refuted. A fake marker with a zero-width character inside it survives framing | Not changed: the model reads the bytes, not a rendering, and two of three verifiers found no impact |
| 12 | low | The browser-binding test accepted any cookie and never checked its attributes | A cross-flow test, and attribute assertions. test_a_cookie_from_another_sign_in_does_not_count, test_approval_sends_the_browser_to_google_with_pkce_and_nonce |
| 13 | low | No test checked the refusal events | test_every_refusal_is_recorded and the event checks across the sign-in tests |
| 14 | low | The 7-day session, the 60 s window and the 30 s skew were not pinned | test_the_session_rules_are_the_documented_ones, test_clocks_may_differ_by_thirty_seconds |
| 15 | low | The route walk skipped anything that was not a plain route | It descends into mounts (and counts bare ones, round 3). test_every_other_route_needs_a_token, test_the_route_walk_sees_inside_and_around_mounts |
| 16 | low | The identity-parameter check was an exact-name list over three modules | Every registered source, matched on identity words inside names. test_no_tool_lets_the_model_choose_whose_data, test_every_module_with_tools_is_registered |
| 17 | low | The read-only checks missed httpx, new modules and several Google write methods | Every module is scanned, and Google’s write methods come from its own discovery documents. test_tools_reach_the_network_only_through_one_get, test_google_connector_calls_no_write_method |
| 18 | low | Size bounds counted characters, but signed values grow with escaping, so a sign-in could still fail after Google | state must be printable ASCII, and a request too large to carry through Google is refused at /authorize. test_odd_requests_are_refused_up_front_with_iss, test_a_too_large_request_is_refused_before_google |
| 19 | nit | client_secret_basic was advertised but could not work |
No longer offered. test_only_the_methods_that_work_are_offered |
| 20 | nit | The issuer was normalised but the token audience and allowed Host kept the raw URL | One normalised URL for all of them. test_urls_are_normalised_once_for_everything |
| 21 | nit | The owner’s calendar time zone is cached for the life of the process | Left as is. Step 3 must cache it per person (HANDOFF) |
| 22 | nit | Revocation by a client_secret_basic client was untested |
Moot: that method was removed (#19) |
Round 2’s critic:
/mcp/ was redirected to plain http behind Cloud Run. Slash redirects are off. test_a_trailing_slash_is_not_redirectedop run) dropped audit lines it could not encode. stdout is UTF-8. test_a_windows_pipe_does_not_lose_linestest_a_real_mcp_client_signs_in_and_calls_a_toolhd may name the organization’s primary domain. Unproven. It gets checked on the first live sign-in, where failures now record both domains.| # | Severity | Finding | What was done; the test that pins it |
|---|---|---|---|
| 0 | low | The one-time claim lived in memory, so a restart could revive a refused Google answer | Every sign-in carries the process’s boot ID; the connector keeps no access log. test_a_restart_ends_sign_ins_in_flight, test_the_server_keeps_no_access_log |
| 1 | low | The route walk missed a mount of a bare app | It counts the mount’s path. test_the_route_walk_sees_inside_and_around_mounts |
| 2 | low | A multipart consent answer with odd bytes caused a 500 | Non-ASCII form values are refused, and ref() never raises. test_odd_form_bytes_are_refused_not_a_crash |
| 3 | low | Refuted. The printable-ASCII state check could crash | Not reproducible as described |
| 4 | low | /authorize refusals answered with a page (unknown client, unregistered redirect) left no record | Recorded. test_every_refusal_is_recorded |
| 5 | low | A double-clicked consent button ended on a refusal, and a double-clicked Cancel never reached Claude Code | The CSRF cookie lives until it expires; the same page opened twice shares one secret. test_double_clicks_and_twin_tabs_still_work |
| 6 | medium | Untested: a __Host- cookie on plain http would break every real browser on localhost |
A test, and the kit’s browsers now refuse any cookie a real browser would drop. test_on_plain_http_the_cookies_drop_the_host_prefix |
| 7 | low | Untested: the size margin for Google’s state | test_any_request_accepted_at_authorize_fits_through_google |
| 8 | low | Untested: the allowed Host from the raw URL | The normalised-URL test calls /mcp. test_urls_are_normalised_once_for_everything |
| 9 | low | Untested: the body limits | test_oversized_bodies_are_refused_before_anything_reads_them |
| 10 | low | Untested: a client signing in twice | test_a_client_can_sign_in_again |
| 11 | low | The network guardrail missed from requests import post and requests.api.post |
test_the_network_guardrail_sees_the_other_spellings |
| 12 | low | Untested: the Google tools’ timeout value | test_each_google_call_builds_its_own_client |
| 13 | nit | Untested: the sign-in Google client’s timeout | test_calls_to_google_time_out |
| 14 | nit | Untested: a tool error that echoes its input reaching the record | test_every_call_is_recorded_without_content |
| 15 | nit | Untested: Google answering with neither a code nor an error | test_no_code_from_google_still_tells_the_client |
| 16 | nit | Untested: escaping the link back to Claude | test_the_way_back_is_escaped |
Round 3’s critic:
hd and email domains. test_sign_in_failures_say_whytest_a_result_with_a_lone_surrogate_can_still_be_sentconsent_refused and token_refused. test_every_consent_refusal_is_recorded, test_token_refusals_are_recordedtest_the_server_keeps_no_access_log