Relay health reporting, Mosh test coverage, and private-HOME hardening - #59
Merged
Merged
Conversation
An account of these fixes (and a fourth, latent one) lives at
docs/specs/meowshell-home-directory-upgrade.md in Sniperlyf3/MeowSSH, not
in this repo despite what it's usually cited as -- I could not find any
copy of it here, on any branch, in this repo's history, or via GitHub code
search, so I'm working from that sibling copy plus the task description.
Problem: EnsureSecure only ever throws when a pre-existing HOME/WorkDirectory
grants group/other access. A directory an older version of this library left
at 0755 (Directory.CreateDirectory applies the process umask, typically 022)
is then permanently unusable: EnsureSecure runs before every process this
library launches, so every connect/serve/forward/SOCKS call fails forever
with nothing that repairs it. On Android there's no shell to chmod it from,
so the only user-facing remedy is clearing app data. Separately,
EnsureSecure's IOException wasn't in any entry point's documented <exception>
list, so a caller following ConnectAsync's own contract (catch
TailcatException) would never catch it. MeowshellHomeDirectory was also
internal, so a consumer managing its own storage layout had no supported way
to prepare a directory this same way. And MeowshellBinaries.EnsureExecutable
checked only the owner-execute bit before repairing it, which is the wrong
bit to assume on Android: the app isn't the file's owner there (the binary
lives in system-owned, read-only NativeLibraryDir) and runs it through the
other-execute bit instead, so a binary that ever shipped without the owner
bit would hit an undocumented UnauthorizedAccessException trying to repair
a bit nobody needed.
Fix:
- EnsureSecure now narrows an over-permissive Unix directory in place
(chmod, then re-read) instead of only rejecting it. A directory this
process doesn't own still fails the chmod and is still refused -- the
chmod is attempted before any trust decision is made, and the symlink
check still runs first so a link is never chmod'd through.
- Every process-launching entry point (MeowshellAgentConnection.ConnectAsync,
MeowshellServer.StartAsync, TailcatClient's internal Prepare, TailcatListener
via it, MeowshellSocksProxy/PortForward.StartAsync, MeowshellMoshConnection.
ConnectAsync) now calls a new EnsureSecureForEntryPoint instead of
EnsureSecure directly, which wraps the raw IOException as a TailcatException
carrying the new MeowshellErrorCode.HomeDirectoryUnsafe.
- Added public MeowshellHome.Prepare(path), a thin wrapper over
MeowshellHomeDirectory.EnsureSecure, so an external consumer no longer has
to reimplement the symlink/ACL/narrowing logic itself. It keeps the plain
IOException contract (not TailcatException), matching the spec's proposed
signature.
- MeowshellBinaries.EnsureExecutable now treats either UserExecute or
OtherExecute as "already executable by this process" and skips the chmod
entirely in that case; when it does attempt one, UnauthorizedAccessException
is caught and turned into a plain IOException naming the binary.
MeowSSH's own MeowshellSshEngine.ConnectAsync catches TailcatException first
and falls back to IOException; TailcatException's Code switch already treats
an unrecognized code as SshFailure.Unknown with the exception's own message
as the description, so this change is absorbed by the existing fallback
without modifying MeowSSH.
Spec ambiguity: the spec's "Finding 2" (maxConnections default changed from
unlimited to 256) is documentation-only per the spec itself ("No code change
needed") and isn't one of the three numbered fixes in the task, so it's left
untouched here.
Verified: mkdir -p nupkg; dotnet build dotnet/Meowshell.sln (0 errors,
0 warnings); dotnet test dotnet/Meowshell.sln -- 203 passed, 0 failed,
0 skipped (.NET 10.0.401 SDK + 8.0.31 runtime installed via dotnet-install.sh
to /tmp/dotnet-sdk). Confirmed each new test actually exercises its fix by
reverting the corresponding production code and re-running the targeted
tests: EnsureSecureNarrows*/PrepareNarrows* fail with the old reject-only
IOException, EnsureSecureForEntryPointWraps*/GenerateKeyRejectsAnUnsafe*
fail with a raw IOException instead of TailcatException, and both new
MeowshellBinaries tests fail (one with a raw UnauthorizedAccessException)
against the old unconditional-UserExecute-only check -- then restored.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013WM5HyhwRLKBDyHR6qitkk
Mosh support (cmd/meowshell/mosh_agent.go) shipped with no test file or
test body referencing it anywhere in the repo, despite being a
user-facing, commercially-marketed feature -- an audit found the gap.
Added:
- mosh_agent_test.go: unit tests for moshConnectPattern (including an
anchoring regression -- the regex must not match "MOSH CONNECT" as a
mid-line substring), resolveMoshDestination (IPv4 literals, hostname
resolution, the deliberate IPv6 rejection mosh-go's udp4-only Dial
requires, unresolvable hosts), moshAgentCmd's argument/tailcat-address
validation, and bootstrapMosh against a real in-process fake SSH
server (success, a missing MOSH CONNECT line, a non-zero remote exit,
an out-of-range port). Also covers moshAgentRuntime.close()'s
sync.Once under concurrent callers, and serveMoshFrames' handling of
data/resize/close_channel before the Mosh client exists yet.
- mosh_agent_e2e_test.go: a real end-to-end run of the compiled
mosh-agent binary against a real system mosh-server (started over a
fake in-process SSH bootstrap), through actual Mosh/UDP to a real
shell -- host-key TOFU, resize, a real command's output round-tripped
back, and a clean close_channel shutdown -- plus the missing-
mosh-server-on-PATH failure path and a real-process check that a
tailcat address is rejected before touching stdin/stdout.
Found and fixed one real bug while writing these: serveMoshFrames
handled close_channel in the same switch as resize, gated behind "the
Mosh client already exists". A close_channel arriving during the SSH
bootstrap or the mosh-server dial -- exactly the window a caller
cancelling mid-connect lands in -- was silently dropped instead of
ending the session, relying on the client's stdin eventually closing
some other way to notice at all. Moved close_channel out from behind
that gate, alongside prompt_response.
Verified: go vet ./... and go test ./... (needs .tailcat-src per
CLAUDE.md) pass except the pre-existing, sandbox-only
TestListenUnixAllowsRootOwnedStickyTmpStyleParent failure (root
container, confirmed unrelated by two previous agents). All 16 new
tests run and pass with dist/{meowshell,tailcat}_linux_amd64 built (0
skipped) -- mosh-server installed via apt for the E2E test, which
depends on parsing its real output. Mutation-checked every assertion by
reverting the close_channel fix and separately breaking the regex
anchor, the IPv4-only check, the port range check, the sync.Once guard,
and the "no MOSH CONNECT line" check -- each one made its corresponding
test fail, then was reverted. Also mutation-checked the E2E marker
technique itself: an earlier draft used a marker equal to the typed
command, which a pty's local echo satisfies without the remote shell
running anything -- switched to a marker computed only in the shell's
real output, confirmed a wrong marker now times out instead of passing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013WM5HyhwRLKBDyHR6qitkk
…il now) Continuing from 4ccf051 (Go-side Mosh coverage): MeowshellMoshConnection.cs had its own ConnectAsync, ReadLoopAsync, prompt dispatch and DisposeAsync, sharing only the wire format with MeowshellAgentConnection, and none of it was exercised by any test. Extended e2e/fakeagent with two Mosh-only magic destinations it needed to stand in for a real mosh-server session: "mosh-echo" (echoes a data frame straight back, since a real Mosh UDP session can't be scripted to echo chosen bytes deterministically -- what comes back depends on a real shell and pty) and "mosh-error-before-connect" (an "error" control message before "connected", for the mid-bootstrap-failure shape a real unreachable Mosh endpoint or a remote with no mosh-server can't be reproduced on demand). Added 10 test methods (15 cases): successful connect/IsConnected, handshake prompts answered through configureConnection vs. left unanswered (must come back "cancelled", never a default accept -- CLAUDE.md is explicit about this), an "error" arriving before "connected" failing ConnectAsync with that error instead of hanging to the timeout, resize argument validation (both the rejects and the ordinary-dimensions accept path), a real WriteAsync -> agent -> ReadLoopAsync -> OutputReceived byte-exact round trip, DisposeAsync's close_channel/stdin-close path exiting without a kill, and cancellation/timeout during ConnectAsync both surfacing correctly and killing the child process. Found and fixed a real gap in the process-kill coverage while finishing this (started by a previous agent, interrupted by a rate limit mid mutation test with the kill step already stripped out of MeowshellMoshConnection.cs on disk): the two tests written for it used "exec sleep 30" as the stand-in agent process. DisposeAsync's "await _readLoop" blocks on that process's still-open stdout regardless of whether the kill step runs, so with the kill removed, the awaited ConnectAsync call (which runs DisposeAsync from its catch block) simply blocked for the full 30 seconds until sleep exited on its own -- at which point the pid-liveness check correctly found it gone, and the test passed either way, just slower. Switched the stand-in to "exec tail -f /dev/null" (never exits by itself) and bounded the awaits with WaitAsync(15s), so a missing kill now fails those two tests in ~15s instead of silently passing in ~30s. MeowshellMoshConnection.cs itself needed no change -- the kill step (TryKill + a second 3s grace) was already correct; only the tests trying to prove it were blind to its absence. Verified: mkdir -p nupkg; dotnet build dotnet/Meowshell.sln; dotnet test dotnet/Meowshell.sln --filter MeowshellMoshConnectionTests -- all 15 pass, 0 skipped (go present on PATH so e2e/fakeagent builds). Mutation-checked every one of the 10 test methods individually: removed/weakened the production check or behavior each test claims to cover, watched that specific test (and only that one) fail, reverted, confirmed `git diff` on MeowshellMoshConnection.cs was empty again before moving to the next. Covered: both resize range checks, the ushort upper bound, IsConnected's definition, configureConnection actually being invoked, the unanswered- prompt default (Cancelled vs. a wrong Accept), the "error" control message's Code plumbing, close_channel actually being written on Dispose, the stdout/stderr stream-marker filter gating OutputReceived, and (as described above) both process-kill paths together with the test fix that made them meaningful. Also confirmed the Go side (4ccf051) independently: go vet ./... and go test ./... pass except the pre-existing sandbox-only TestListenUnixAllowsRootOwnedStickyTmpStyleParent (root container, per CLAUDE.md); all 16 Mosh Go tests, including the 3 real E2E ones against a real mosh-server, ran and passed with 0 skipped once dist/*_linux_amd64 existed and MEOWSHELL/TAILCAT were left unset so findE2EBinary's ../../dist fallback resolved them relative to the package dir. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013WM5HyhwRLKBDyHR6qitkk
…readable
MeowSSHAPI's admission-quota check (deploy/single-node/derper/patches/
admission-reason.patch in meowsshapi) refuses over-quota connections by
sending a derp.FrameHealth; magicsock already records it via
SetDERPRegionHealth. Investigation found that text was reachable nowhere:
tailscale.com/health exposes DERP region health only through its
Warnable-templated Strings() (ipnstate.Status.Health), and this project's own
build-tags.txt sets "ts_omit_health", which compiles buildfeatures.HasHealth
to false -- making SetDERPRegionHealth an unconditional no-op regardless of
what Client API exists on top of it. The signal was being dropped before it
ever reached a Go value, let alone the wire.
Fix, in patches/tailcat/derp-health-status.patch, following live-path-status.
patch's precedent of a small reviewed patch on the pinned source:
- drop ts_omit_health from build-tags.txt so health.Tracker actually runs.
- add Client.RelayHealth(), parsing the one Warnable template
("...is reporting an issue: <problem>") since health.Tracker has no raw
getter for derp.HealthMessage.Problem. A pinned test
(TestDerpRelayHealthProblemExtractsRawText) fails loudly if that upstream
wording ever changes, instead of silently losing the reason text.
Verified: go vet ./... clean; the new patch applies cleanly in build.sh's
exact patch order against a fresh tailcat checkout (reproduced manually);
`go test .` in the patched tailcat tree passes all 4 new tests plus the
existing PathStatus ones. Mutation-checked
TestDerpRelayHealthProblemExtractsRawText by corrupting the separator
literal -- it failed as expected, then passed again once reverted.
Go-side wiring (control protocol message, agent reporter, .NET exposure)
follows in subsequent commits.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013WM5HyhwRLKBDyHR6qitkk
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
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
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
…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
Problem: every existing test for reportTailcatRelayHealth (agent_health_test.go, agent_control_reader_test.go) and every dotnet relay_health test drove it through e2e/fakeagent's scripted output or a hand-scripted agentHealthStatusSource -- none of them exercised the real, in-process tailcat.Client the previous commit actually wired the reporter to. And tailcatHealthSource itself (the wiring's source-selection half) had no test of its own the way tailcatPathSource's would, so a regression that made it return nil unconditionally would have been invisible: a real, healthy connection legitimately emits zero relay_health frames either way (unlike PathStatus, RelayHealth's ok=true only when there IS a problem -- "ok true with an empty problem never happens" per tailcat.Client.RelayHealth's own doc comment), so passive E2E observation alone can't tell "reporter absent" from "reporter present but healthy". Fix: two new tests. TestTailcatHealthSourceMirrorsTailcatPathSourceForALiveTailcatClient is a white-box unit test pinning tailcatHealthSource's contract directly (nil before a.tcClient exists, wraps the exact live client once it does) -- verified by mutating it to always return nil and watching the test fail. TestAgentRelayHealthReporterAttachesToARealTailcatConnection is a real agent E2E test (findE2EBinary-gated, like the existing ones) that drives several real exec channels against a real tailcat destination while recording any interleaved relay_health frame, asserting a genuinely healthy loopback-DERP connection never produces a false-positive report. Verified by mutating the real tailcat.Client.RelayHealth (in .tailcat-src, not committed) to panic and rebuilding dist/meowshell_linux_amd64 against it: the test failed with that goroutine's stack trace in the agent's stderr, proving the reporter is actually polling the real, live client and not a no-op -- restored and confirmed byte-identical (md5sum) before this commit. Known gap, noted honestly rather than glossed over: this repo's E2E harness has no way to make a real DERP server emit a genuine derp.FrameHealth (that requires deploy/single-node/derper/patches/admission-reason.patch, which lives in the meowsshapi repo against a real derper deployment neither this repo nor this task may touch), so no test here exercises RelayHealth() actually returning ok=true end-to-end. That emission logic itself stays covered by agent_health_test.go's scripted source and by the reply-correlation regression tests. Real numbers: go test ./cmd/meowshell/... -v with $MEOWSHELL/$TAILCAT set to the freshly built dist binaries shows PASS for both new tests and 0 SKIP package-wide (186 PASS, 1 pre-existing unrelated FAIL, 0 SKIP). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013WM5HyhwRLKBDyHR6qitkk
tailcatdial.go now calls tailcat.Client.RelayHealth directly, the same way it already calls PathStatus -- a deliberate choice so relay-health reporting is type-checked at compile time instead of hidden behind reflection. The "Fetch tailcat source" steps in the build and windows-e2e jobs applied only live-path-status.patch, so vet against that checkout failed with "c.cl.RelayHealth undefined (type *tailcat.Client has no field or method RelayHealth)". Apply derp-health-status.patch alongside it in exactly those two steps. The "Build a host tailcat to act as the client" steps (android-e2e, probe) stay pristine on purpose: they prove meowshell still interoperates with an unmodified upstream tailcat, and patching them would delete that coverage. Verified by reproducing CI's step locally -- a fresh checkout of tailcat.ref with only those two patches applied, then `go vet ./...` against it, which failed before and passes after. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013WM5HyhwRLKBDyHR6qitkk
windows-e2e died in "Fetch tailcat source" with
error: patch failed: build-tags.txt:1
error: build-tags.txt: patch does not apply
while the build job applied the same two patches at the same pinned ref
without complaint. The asymmetry is not the patch: tailcat's own
.gitattributes pins "build-tags.txt text eol=lf", deliberately, so that a
CRLF checkout cannot put a stray carriage return in the last build tag for
anyone doing $(cat build-tags.txt). GitHub's Windows runners set
core.autocrlf=true globally, so the checkout ends up CRLF for every .go file
and LF for that one, and git apply -- which converts an LF patch's content to
the worktree's convention before matching -- then matches the .go hunks but
not build-tags.txt. derp-health-status.patch is the only patch that edits it,
which is why nothing caught this until RelayHealth needed it.
Clone .tailcat-src with core.autocrlf=false in that job, so the whole
checkout is LF like the patches. `git clone -c` writes the setting into the
new repository's config, so the `checkout --detach FETCH_HEAD` two lines
later inherits it rather than re-converting. The job only vets and tests
cmd/meowshell against these sources and never builds tailcat itself, so
nothing there wants CRLF.
Verified by reconstructing the runner's exact state locally -- a checkout at
tailcat.ref with .go files converted to CRLF and build-tags.txt left LF, and
the patches converted to CRLF to stand in for git apply's worktree
conversion: live-path-status.patch applies (exit 0) and
derp-health-status.patch fails with the identical "patch failed:
build-tags.txt:1", reproducing CI. Cloning the same ref with
core.autocrlf=false and applying both unconverted patches: both exit 0.
Not touched: patches/tailcat/derp-health-status.patch itself, which is a
reviewed artifact that ./build.sh and the build job apply correctly on Linux,
and the pristine "Build a host tailcat to act as the client" steps.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013WM5HyhwRLKBDyHR6qitkk
Second half of the windows-e2e fix. Cloning .tailcat-src with core.autocrlf=false made the *target* tree LF, and the failure moved from "patch failed: build-tags.txt:1" on the second patch to "patch failed: tailcat.go:1998" on the first -- because the patch files are themselves part of this repository's checkout, and GitHub's windows-latest runners set core.autocrlf=true globally, so actions/checkout had been rewriting them to CRLF all along. An LF tree and a CRLF patch do not match, in either direction. Mark *.patch as -text so git never converts them on any platform. This is the mirror image of tailcat's own .gitattributes pinning build-tags.txt to eol=lf, and it is what keeps the two halves consistent: LF patches applied to an LF checkout. Verified locally against the pinned ref by reproducing all four combinations. CRLF tree + CRLF patch: both apply. CRLF tree + LF patch: live-path-status fails at tailcat.go:1998. LF tree + CRLF patch: identical failure -- which is what CI just reported. LF tree + LF patch, the state this commit and its parent together produce: both apply, exit 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013WM5HyhwRLKBDyHR6qitkk
windows-e2e now gets past the tailcat patch step and runs go vet for the first time on this branch, which fails: mosh_agent_e2e_test.go:149:12: undefined: syscall.Kill. The file carries no build constraint, so it is compiled on every GOOS, but its t.Cleanup reaping the real mosh-server TestMoshAgentEndToEnd starts calls syscall.Kill, which only exists on Unix. Tagging the whole file unix-only (matching keystage_unix_test.go / sockperm_unix_test.go) was the lazy option but not the true one: only TestMoshAgentEndToEnd needs a real mosh-server (gated by requireMoshServerBinary) and the syscall.Kill it takes to reap it. TestMoshAgentSurfacesAMissingMoshServer and TestMoshAgentRejectsTailcatAddressAsARealProcess drive the mosh-agent subprocess through nothing but the in-process fake SSH server and the compiled meowshell binary -- both cross-platform -- so blanket-tagging the file would silently drop two tests that vet's own failure gives no reason to take off Windows. Fix: move just the signal send into a killMoshServerProcess helper, split unix/windows like the rest of this package already does (exec_unix.go / nativeexec_windows.go, sockperm_unix.go / sockperm_windows.go, ...). The Windows half is unreachable in practice -- requireMoshServerBinary skips TestMoshAgentEndToEnd before it's ever called, since there's no real mosh-server there -- but must still type-check, so it uses the portable os.Process.Kill instead of a signal Windows doesn't have. Verified: - GOOS=windows go vet ./... : was "undefined: syscall.Kill", now clean. - GOOS=linux go vet ./... : clean, unchanged. - go test ./cmd/meowshell/ -run Mosh -v on this host (mosh-server 1.4.0 on PATH, dist/ built): all three e2e tests actually ran (1.11s for the full end-to-end one, not an instant skip) and passed; pgrep afterward shows no leaked mosh-server process, confirming killMoshServerProcess still reaps it. - No other file in cmd/meowshell has this class of bug: grep for "syscall." turns up 10 files total, and the other 9 were already _unix.go/_unix_test.go. 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.
Ten commits covering the Meowshell half of managed-relay health reporting, test coverage for a shipped-but-untested feature, and the private-HOME upgrade path.
Managed-relay health reporting
When a user exceeds their monthly managed-DERP allowance, MeowSSHAPI refuses admission and the relay sends the reason as a
derp.FrameHealthmessage. Previously that text never left the stack:build-tags.txtsetts_omit_health, which compilesbuildfeatures.HasHealthtofalseand makesSetDERPRegionHealthan unconditional no-op — so the text never reached even a Go value.patches/tailcat/derp-health-status.patchdrops that tag and addsClient.RelayHealth(). A pinned test fails loudly if upstream's health wording ever changes.agent_health.goaddsreportTailcatRelayHealth, wired intoagentCmdfrom the same in-processtailcat.Clientthe path reporter uses, emittingrelay_healthcontrol messages.MeowshellAgentConnectionexposesCurrentRelayHealth/RelayHealthChanged.nullmeans healthy — "never had a problem" and "just cleared" collapse deliberately, matching the Go side'somitempty. A non-null value is literal pre-formatted user-facing text, not a code to map.Reply-correlation hazard, found and fixed. Adding a second asynchronous control message reintroduced the class of bug that made PR #52's
forward_socksflake 22% of runs:expectRequestReply(used by everysftp_opround trip) only skipped"path", so an interleavedrelay_healthframe failed in-flight replies with an emptyRequestID. Fixed, with a regression test pinning it.The reporter's liveness is proven by mutation rather than by observation — a healthy connection legitimately emits zero health frames, so "working" and "absent" look identical passively. Making the real
RelayHealthpanic and rebuilding makes the E2E test fail with that goroutine's stack trace.Mosh test coverage
Mosh shipped end to end with zero tests anywhere. Added 16 Go tests (including three real E2E against a real
mosh-server) and 15 .NET cases covering protocol shapes, prompts, error paths, cancellation and the process-kill path.Two kill-path tests were found to be structurally incapable of failing: they used
exec sleep 30as the stand-in process, so with the kill removedDisposeAsyncsimply blocked on still-open stdout untilsleepexited on its own, and the liveness assertion then passed coincidentally. Switched totail -f /dev/nullwith bounded waits.Private HOME directory
Implements
docs/specs/meowshell-home-directory-upgrade.md:EnsureSecurenow narrows a pre-existing over-permissive directory instead of rejecting it forever (a user upgrading with a0755HOME was previously stuck); adds the publicMeowshellHome.Prepare()entry point; wraps the rawIOExceptionasTailcatException/MeowshellErrorCode.HomeDirectoryUnsafeat all six call sites; and stopsEnsureExecutablethrowingUnauthorizedAccessExceptionon a read-only AndroidNativeLibraryDir.Verification
go vet ./...clean;go test ./...— 186 pass, 0 skip with real binaries present (only the known root-containerTestListenUnixAllowsRootOwnedStickyTmpStyleParentfails, pre-existing).dotnet test dotnet/Meowshell.sln— 224 pass, 0 skip.🤖 Generated with Claude Code
https://claude.ai/code/session_013WM5HyhwRLKBDyHR6qitkk
Generated by Claude Code