Repository navigation
fix: tunnel a Google login to the picture host only, not to googleusercontent.com - #20
Merged
Merged
Conversation
…rcontent.com The tunnel list of the google destination had the entry .googleusercontent.com. This entry admits all names under that domain. The reverse DNS name of each Compute Engine VM is under that domain. For example, 81.59.117.34.bc.googleusercontent.com resolves to 34.117.59.81. Thus a harness could open a tunnel to a VM that any person can rent. A tunnel has no metering, and the egress cannot see the data in it. The Antigravity eligibility check gets the picture of the account from lh3.googleusercontent.com. A run on a real login on 7 October showed this (commit a4271d3). Replace the domain entry with this one host. The other domain entries are .githubcopilot.com, .chatgpt.com and .grok.com. The vendor operates each of these zones. None is on the Public Suffix List, and none has a wildcard DNS record. Thus they stay as they are. Add a test. It makes sure that no login destination admits a tunnel to a name that any person can get (a VM reverse name, appspot.com, run.app, cloudfunctions.net, a storage bucket, github.io, githubusercontent.com). It also makes sure that the google destination admits the hosts that a login needs. The test sends the CONNECT through the egress router with tower, which is a new dev-dependency of amem. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…overs The test of the last change examined only the strings in the tunnel list for the hosts that a login needs. It passed also when the egress admitted no exact host. The test now sends CONNECT host:443 for each of these hosts through the egress router and expects 200. A oneshot request has no upgrade, so the egress connects to nothing. The refusal check expected only 403. A refusal for a different reason, such as the ceiling, also passed it. The check now also expects the text "no destination allows a tunnel to" in the body. The egress examines only the CONNECT authority. It does not examine the TLS server name in the tunnel. Thus a tunnel is pinned to a front end, not to the tenant behind it. The test name, its comments and the comment on the google entry now state only what the change does: the google destination does not admit a name under googleusercontent.com, because a Compute Engine VM has a reverse name there that resolves to the VM. PLAN.md now gives lh3.googleusercontent.com in place of the domain. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sstriker
requested review from
andrewmusselman
and
a balanced review from Copilot
October 24, 2026 15:43
There was a problem hiding this comment.
🔵 Needs a closer look
It changes a security-sensitive egress tunnel allowlist whose correctness depends on external Google behaviour, and the author documents a merge-order hazard with PR #13 that will break the test without a network stand-in, so human oversight is warranted.
No open issues 🎉
What changed in this PR
This PR tightens the google CONNECT tunnel allowlist used by the Antigravity (agy) Google-login route. It replaces the wildcard entry .googleusercontent.com with the single exact host lh3.googleusercontent.com. The wildcard was a security hole: every Compute Engine VM has a reverse-DNS name under googleusercontent.com that resolves to the VM itself, so any harness on the google destination could open an unmetered TLS tunnel to any rented VM. The fix pins the one picture host that the eligibility check actually uses, observed on a real login.
Changes:
crates/memory/src/main.rs: narrow thegoogledestination tunnel entry from.googleusercontent.comtolh3.googleusercontent.com, with an explanatory comment, plus a new#[tokio::test]that drives the egress router to assert the login hosts are tunnelled and VM/shared-front-end names are refused.- Docs (
PLAN.md,local/README.md): update the named host to match the code. - Build (
crates/memory/Cargo.toml,Cargo.lock): addtower(dev-dependency,utilfeature) to drive the router viaoneshotin the test.
| File | Description |
|---|---|
crates/memory/src/main.rs |
Replaces the wildcard tunnel entry with the exact picture host and adds a router-level test for the allow/deny behaviour. |
PLAN.md |
Updates the M5 row to name lh3.googleusercontent.com instead of the wildcard domain. |
local/README.md |
Updates the Antigravity row to name the exact picture host. |
crates/memory/Cargo.toml |
Adds tower as a dev-dependency for the new test's oneshot call. |
Cargo.lock |
Records the new tower dependency edge for the amem crate. |
Review effort: Balanced
The test asked for a 200 from each host that a Google login needs. After #13, the egress resolves the name and checks the addresses before the 200. That step needs a network. The test now asserts only that the allowlist lets each host through. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The test asserted only that the allowlist did not refuse these hosts. A refusal for another reason passed, and so did a host that another destination admits. The test now raises the session label above the ceiling of google. Each host must then get the refusal that names google. That refusal comes before the egress resolves the name, so the test needs no network. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The CONNECT allowlist of the
googledestination had the entry.googleusercontent.com. Every Compute Engine VM has a reverse-DNS name under that domain, which resolves to the VM itself. For example,81.59.117.34.bc.googleusercontent.comresolves to34.117.59.81. So a harness under that destination could open a TLS tunnel, with no metering, to any VM that someone rents.The change
crates/memory/src/main.rs: the entry is nowlh3.googleusercontent.com, the one host that Antigravity (agy) needs there. Commit a4271d3 records this from a real Google login: "agy asks www.googleapis.com for the account and lh3.googleusercontent.com for its picture before anything else, and answered once both were tunnelled."local/README.mdandPLAN.md:102name the exact host.Other wildcard entries
The other dot entries are
.githubcopilot.com,.chatgpt.comand.grok.com. They stay, for these reasons:Failing first
Against
main:a tunnel to 81.59.117.34.bc.googleusercontent.com left: 200 right: 403.The test sends
CONNECT host:443through the egress router, which never dials:With exact hosts refused in the router (a mutation):
a tunnel to oauth2.googleapis.com ... left: 403 right: 200.make check: 621 tests, 620 passed. The one failure was the knowncompanion_browserrace, which a separate PR fixes (branchfix/companion-blank-race). It passed when run alone.Adversarial pass
A fresh agent attacked
7ae42d7. It confirmed these points:.googleusercontent.comput back, the test fails.It found these gaps, all fixed in
783cb04:It also found a larger problem, already on
mainand not in this PR. The tunnel checks only the CONNECT authority. Inside the tunnel, the harness chooses the TLS server name (SNI). The agent showed, through the real egress:CONNECT lh3.googleusercontent.comthen TLS to a GCS bucket or an App Engine app worked;chatgpt.comreached other Cloudflare customers;api.githubcopilot.comreachedraw.githubusercontent.com.The fix is to read the ClientHello and require its SNI to match the CONNECT host. A separate PR adds this check.
Note for review
Merge with #13. #13 makes an admitted tunnel resolve its name before it answers. That step needs a network. So the test now raises the session label above the ceiling of
googlefirst (46c5ceb). Each host that the login needs must then get the refusal that namesgoogle: "this session has read restricted material and google accepts at most internal". That refusal comes before the egress resolves the name, so the test needs no network. Underunshare -rnit passes in 0.45 s.Mutations of the entry list:
lh3.googleusercontent.comremoved:a tunnel to lh3.googleusercontent.com: {"error":{"message":"no destination allows a tunnel to lh3.googleusercontent.com:443",...}}.lh3.googleusercontent.commoved togithub:... "this session has read restricted material and github accepts at most internal" ....Second adversarial pass. A fresh agent attacked
df1b376. It found that the intermediate test (e169c6d) passed when every admitted tunnel was refused for another reason, and when another destination admitted the host. It proposed the ceiling route, which46c5cebuses. It also noted, as reasoning only, that a profile picture onlh4tolh6would be refused. The real login in a4271d3 usedlh3only. If anotherlhNhost is needed, the fix is to add that host, not the domain.make checkafter the merge ofmain: 642 tests, 642 passed, 3 skipped.Posted by Claude on behalf of @sstriker