Prove direct Tailcat payload bypasses DERP - #52
Merged
Merged
Conversation
This branch's Android probe E2E job (dotnet-android-e2e) fails with
PROBE_SSH_FAIL, meowshell agent error connecting: connecting to tailcat
destination: netmon.New: route ip+net: netlinkrib: permission denied.
This PR switched the primary Tailcat SSH transport in agent.go from
shelling out to the separate tailcat binary to an in-process
tailcat.Client (tailcatdial.go's tailcatClientDialer, dialed from
agent.connect's direct-address path and from forwardClient). That
client's initLocked calls netmon.New() unconditionally, and
netmon.New() opens a netlink socket and calls bind() on it, which
Android's SELinux policy denies to every app in the untrusted_app
domain. patches/tailcat/android-netmon-interface-getter.patch already
works around this for the tailcat subprocess by registering an
anet-backed netmon.RegisterInterfaceGetter in cmd/tailcat, but that
patch only touches the tailcat module -- it never applied to the
meowshell binary, which had no such registration and, until this
branch, never called netmon.New() itself either. Now that the SSH
transport runs netmon.New() inside the meowshell process too, meowshell
needs its own copy of the same workaround.
Add cmd/meowshell/netmon_android.go (//go:build android), mirroring the
tailcat patch's RegisterInterfaceGetter + AltAddrs handling, and add
github.com/wlynxg/anet v0.0.5 (the same version CLAUDE.md pins for
.tailcat-src) to the root go.mod/go.sum so it resolves outside the
tailcat replace directory. build.sh's ldflags already pass
-checklinkname=0 for every android build target regardless of binary,
so no build.sh change is needed for anet's go:linkname use to link
into meowshell too.
Verified: go vet ./... and go test ./... pass against a freshly
bootstrapped .tailcat-src (patches applied per CLAUDE.md); the only
failure is the pre-existing, environment-specific
TestListenUnixAllowsRootOwnedStickyTmpStyleParent (reproduces
identically on this branch before this commit -- this sandbox's /tmp
does not honor a sticky-bit chmod). Ran the agent E2E suite against
binaries just built by ./build.sh (156 passed, 0 skipped, same one
pre-existing failure), confirming TestAgentForwardsThroughTailcatDestination
(the in-process tailcat.Client path this fix targets) still passes on
linux/amd64. ./build.sh itself built tailcat+meowshell for every
linux/windows target with exit 0; android targets skipped for lack of
an NDK in this sandbox (goenv's designed behavior, matching what CI's
"Locate the pinned NDK" step exists to avoid). Could not do a full cgo
cross-compile for android here (no NDK, no network budget to fetch
one): got as far as confirming cmd/meowshell/netmon_android.go's
imports resolve and gofmt is clean, and that its RegisterInterfaceGetter
call and Interface{Interface, AltAddrs} literal match the exact
tailscale.com/net/netmon and github.com/wlynxg/anet signatures already
vetted by the reviewed tailcat patch this mirrors; the android build
itself needs the pinned NDK CI installs, which is the one part of this
fix that could not be verified end-to-end in this sandbox.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013WM5HyhwRLKBDyHR6qitkk
Owner
Author
|
The The job fails while creating the Copilot code-scanning review session, before it reads any code: It failed identically on the previous head ( Flagging rather than re-running: a re-run exercises the same unlicensed API call and fails the same way. Generated by Claude Code |
TestAgentForwardsThroughTailcatDestination/forward_socks read the
channel_opened reply with a raw mustReadFrame+decodeControl instead of
expectChannelOpenedMessage. agentCmd's reportTailcatPath goroutine
(added by this PR) writes asynchronous "path" control frames on the
same stream while the session runs, so that raw read could consume a
path notification instead of the reply it was waiting for -- exactly
what CI saw: opened.Msg == "path" with a populated Direct field and an
empty BoundAddr. forward_local, right above it in the same test, was
already fixed to use expectChannelOpenedMessage and never flaked.
Fix: forward_socks now goes through expectChannelOpenedMessage too,
which loops past "path" frames until channel_opened (or "error")
arrives, with the existing readFrameWithDeadline timeout as backstop.
Audited every other raw mustReadFrame/decodeControl use in this
package for the same gap. All of them are safe already, for reasons
worth recording rather than re-discovering later:
- agent_auth_e2e_test.go, agent_security_e2e_test.go,
agent_forward_e2e_test.go, agent_tcp_e2e_test.go: connect through
startAgent/startTestSSHServer, a plain TCP "testuser@host:port"
destination. tailcatPathSource() (agent.go) only returns non-nil
when session.tcClient is set, which a TCP destination never does,
so reportTailcatPath is never started for these -- no path frames
can appear on their streams at all.
- agent_sftp_e2e_test.go does connect through a tailcat address, but
its raw reads are either filtered by ChannelID first (a path frame
rides channel 0, so a loop waiting on a non-zero sftp channel skips
it as a mismatched ChannelID before ever decoding it) or go through
expectChannelOpenedMessage/expectRequestReply, both of which already
skip Msg=="path".
Verified against the real binaries (go build per CLAUDE.md; go1.27.1):
- Reproduced first: with the bug still in place,
`go test ./cmd/meowshell/ -run TestAgentForwardsThroughTailcatDestination -count=50`
failed forward_socks 11/50 times (22%), each with the same
Msg:path / empty BoundAddr signature as the CI log.
- After the fix: the same command at -count=200 passed 200/200.
- Full suite: `go vet ./...` clean; `go test ./...` passes except
TestListenUnixAllowsRootOwnedStickyTmpStyleParent, which also fails
on unmodified HEAD (6677f7f) in this sandbox because it runs as
root -- a pre-existing, unrelated environment artifact, not
something this change touches.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013WM5HyhwRLKBDyHR6qitkk
Sniperlyf3
pushed a commit
that referenced
this pull request
Sep 19, 2026
tailcat.Client.RelayHealth() (previous commit) has nowhere to go once the main SSH connection is up. Add a "relay_health" control message to the wire format (protocol.go) carrying RelayProblem -- the managed relay's raw DERP health text, e.g. an over-quota admission refusal -- and reportTailcatRelayHealth/agentHealthStatusSource in agent_health.go, matching reportTailcatPath/agentPathStatusSource's shape exactly (same poll-and-emit-on-change loop, same "skip until the source is ok" contract) so it slots into the same asynchronous-notification model this project already uses for live path telemetry. Per CLAUDE.md's warning about PR #52 (a "path" reporter racing with open_channel replies because a test helper assumed the next control frame was always the one it asked for): audited every control-frame reader in agent_e2e_test.go. expectChannelOpened, readUntilExit, readExitOnly, and readUntil all loop on an unmatched msg.Msg already, so they tolerate an unknown message type by construction -- but that tolerance had never actually been exercised against anything but "path". Added agent_control_reader_test.go, which needs no real binaries, to pin it directly against "relay_health" for expectChannelOpened and readExitOnly (the two shapes real callers use: waiting for a reply, and waiting on a specific channel). This reporter is not yet wired into agentCmd's live connection -- neither is reportTailcatPath, despite existing as a tested primitive since "Add Tailcat path reporter primitive" / "Decouple path reporter from patched Tailcat type". Following that same incremental precedent here; wiring (there is no in-process tailcat.Client at all for the main connection today, only for exit-node forwarding's lazily-created a.tcClient -- see the handback report for the constraint this leaves for follow-up) is left for a subsequent PR. Verified: go vet ./... clean; go test ./... clean except TestListenUnixAllowsRootOwnedStickyTmpStyleParent (pre-existing, root container only, per CLAUDE.md). Mutation-checked every new test in this commit: TestReportTailcatRelayHealthIgnoresUnknownAndEmitsOnlyChanges (broke the change-detection comparison), TestHealthControlMessageFromStatus* (broke field passthrough), TestExpectChannelOpenedSkipsAnInterleavedRelayHealthMessage (rewrote expectChannelOpened's loop into a single read, reproducing PR #52's exact bug) -- each failed as expected, then passed again with the source reverted to byte-identical. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013WM5HyhwRLKBDyHR6qitkk
Sniperlyf3
pushed a commit
that referenced
this pull request
Sep 19, 2026
The Go side (54a3bf2, 610756a) parses the managed relay's DERP health text and emits it as a "relay_health" control message; nothing on the .NET side read it yet. Add AgentMessage.RelayProblem (protocol.cs, snake_case-mapped to relay_problem, matching the Go wire field exactly) and MeowshellAgentConnection.CurrentRelayHealth/RelayHealthChanged, following CurrentPath/PathChanged's existing shape: null means healthy, whether because nothing was ever wrong or a problem just cleared, exactly like the Go side's own omitempty contract. Inherited draft had a real bug: HandleControlAsync's "relay_health" case computed the new problem value, compared it to CurrentRelayHealth to decide whether to raise the event, but never actually stored it -- the assignment was missing, left behind under a "// mutation: dropped assignment" comment from an interrupted mutation-testing pass. CurrentRelayHealth would have stayed null forever and, worse, a health-then-clear sequence would silently stop notifying after the first change (second read compares the incoming value against a CurrentRelayHealth that was never updated, so a "problem cleared" report short-circuits against the wrong baseline instead of firing). Fixed by adding the assignment; mutation-checked by reverting it and watching RelayHealthUpdatesAreExposedOnTheSameConnection time out waiting for the clear notification, then restoring byte-identical. Per CLAUDE.md's PR #52 warning (a "path" reporter racing with open_channel replies because a helper assumed the next control frame was always its reply): MeowshellAgentConnection dispatches every control frame through one central read loop that resolves pending open/request completions by RequestId in a dictionary, never by frame order, so it cannot have that bug by construction -- unlike the Go E2E helpers in agent_e2e_test.go, there is no "read one frame and assume it's the reply" here to audit. Added RelayHealthArrivingMidOpenChannelDoesNotBreakReplyCorrelation to pin that guarantee anyway, against e2e/fakeagent's new relayHealthRaceDestination, which writes an interleaved relay_health frame deterministically before every channel_opened reply. Mutation- checked by inserting a `_pendingOpens.Clear()` into the relay_health handler (simulating the PR #52 failure mode: an unsolicited message disturbing a pending reply) and watching the test time out, then reverting to byte-identical. Also mutation-checked the wire-shape test (MeowshellAgentRelayHealthProtocolTests.Relay_problem_round_trips) by renaming the JSON property via [JsonPropertyName]; it and the empty-vs-missing test both failed as expected, reverted clean. Verified: dotnet build dotnet/Meowshell.sln clean; dotnet test dotnet/Meowshell.sln -- 223/223 passing (218 baseline + 5 new: 3 protocol round-trip tests, RelayHealthUpdatesAreExposedOnTheSameConnection, RelayHealthArrivingMidOpenChannelDoesNotBreakReplyCorrelation), with MEOWSHELL/TAILCAT pointed at real dist/ binaries so none of it silently skipped. go vet ./... and go test ./... unaffected (untouched by this commit) -- clean except the pre-existing root-container TestListenUnixAllowsRootOwnedStickyTmpStyleParent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013WM5HyhwRLKBDyHR6qitkk
Sniperlyf3
pushed a commit
that referenced
this pull request
Sep 19, 2026
Problem: reportTailcatRelayHealth (610756a) was only ever driven by e2e/fakeagent's scripted "relay_health" frames -- agentCmd never started it against the in-process tailcat.Client that PR #52 (just merged) gave the primary SSH transport, so relay_health control messages were never emitted by a real agent, only by the .NET test double. Fix: mirror tailcatPathSource/reportTailcatPath's wiring exactly. tailcatForwardClient.RelayHealth() delegates to the wrapped tailcat.Client's own RelayHealth (added by 54a3bf2's derp-health-status.patch), and agentSession.tailcatHealthSource() returns nil for the same cases tailcatPathSource does (no client, TCP destination) so the reporter only ever runs against a real Tailcat connection. agentCmd starts the goroutine right after the path reporter, with its own context/cancel pair so either can be torn down independently. Verified: a mutation that makes the real tailcat.Client.RelayHealth panic crashes the new agent E2E test (see the next commit) with that exact goroutine's stack trace, confirming reportTailcatRelayHealth is actually polling the live client and not a no-op. go vet ./... and go test ./... pass (186 PASS, 0 SKIP, only the pre-existing root-owned-sticky-tmp failure this container always produces). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013WM5HyhwRLKBDyHR6qitkk
Sniperlyf3
pushed a commit
that referenced
this pull request
Sep 19, 2026
…s live Problem: with reportTailcatRelayHealth actually wired into agentCmd (previous commit), a "relay_health" control message can now land on the shared control stream at any point after connect -- the same async-notification hazard "path" already has. expectChannelOpenedMessage tolerated it only by accident (an unmatched msg.Msg falls out of its switch with no default case, so the loop just reads on) rather than by declared intent, and expectRequestReply -- used by every sftp_op round trip -- only special-cased "path" and would have failed a real sftp_op with "reply RequestID = "", want ..." the moment a relay_health frame won the race against its reply. That's the exact shape of bug behind PR #52's own forward_socks flake (22% failure rate): an unrelated async control message mistaken for, or breaking correlation with, the reply a helper was waiting on. Fix: make expectChannelOpenedMessage's "path"/"relay_health" tolerance explicit instead of incidental, and add the same relay_health skip expectRequestReply already has for "path". Verified: TestExpectRequestReplySkipsAnInterleavedRelayHealthMessage (new, mirroring the existing TestExpectChannelOpenedSkipsAnInterleaved.../ TestReadExitOnlySkipsAnInterleaved... regression tests) fails with exactly that "reply RequestID" mismatch when the relay_health skip is reverted, and passes with it in place. File confirmed byte-identical to its pre-mutation state (md5sum) before this commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013WM5HyhwRLKBDyHR6qitkk
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
2 participants
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.
Closes the remaining direct-path half of the managed-DERP E2E matrix and fixes the production path that the first E2E attempt exposed.
Root cause found by the real-binary test:
meowshell agentstill opened its primary Tailcat SSH transport through a separatetailcatsubprocess.tailcat.Clientwere therefore not attached to the exact WireGuard engine carrying the SSH session.Fix:
tailcat.Clientfor the primary Tailcat SSH transportPathStatusfrom that exact live client into the agent control protocolDiscoPingto actively nudge endpoint discovery instead of leaving an otherwise-idle SSH connection on bootstrap DERP longer than necessaryE2E proof:
e2e/testderpDirectProduction accounting remains separate and authoritative: MeowSSHAPI #42 proves bytes that actually traverse the patched managed DERP are metered exactly. Live path state remains diagnostic UX, never billing input.