WP8, the last code before deploy. Rewrites the client entrypoint on the v2 wire:
* target parsing: host[/hop1/hop2...]; hops resolved to incus/docker/pct verbs via
/etc/sic/hosts.toml (loaded only when hops are present), folded into the frame onion
by buildChain — verbs[0] is the outermost layer the host sicd enters first.
* argv is carried as [][]byte end to end and NEVER space-joined; --sh is the sole
exception (user asked for a shell line -> [sh -c <joined>]).
* stdin is forwarded AFTER the frame (the v1 client set Stdin to the frame alone, so
`sic host cat >f <local` wrote a zero-byte file). The copy runs in an abandoned
goroutine so a stdin-ignoring command on a tty never blocks on a read that never EOFs.
* ssh -T: the frame is binary; a pty would mangle it.
* exit code inherited from the far end.
Dead v1 framing (netstring/frameArgv/indexOf) removed — frame.go owns the wire now.
Also corrected the stale runV2 doc comment in sicd (still described the removed 0:,
terminator; it is argc-prefixed since e1f93de).
E2e on boltzmann (ssh localhost + real sicd), all green: touch "a b" -> ONE file;
echo A "" B -> "A B"; pipe+redirect stdin both forward; exit 7 propagates; a
stdin-ignoring echo does not hang; --sh; and netstring-looking args (3:x, a,b:c)
survive as single arguments.
Ship-blocker #1: the empty-netstring terminator collided with a legit empty "" argument —
parseV2Content broke on the first len(ns)==0, so echo A "" B ran as echo A and B bled into
child stdin (wrong command executed, not rejected). Replaced the terminator with an explicit
argc prefix: content = netstring(argc) + argv netstrings + payload; the daemon reads EXACTLY
argc elements, so "" is a normal arg. Fixes the collision AND the element-count bound in one.
Daemon parseV2Content, client v2Frame, both test helpers, and the reference builder all updated.
Proof: TestV2EmptyArgPreserved (echo A "" B -> "A B") + TestV2FrameEmptyArgRepresentable.
All v2/v1 tests green, vet clean. Daemon now clears reviewer 964 #1/#2/#3 — deployable.
#3 [DoS]: runV2 allocated make([]byte, nsLen) from a raw uint32 (up to 4 GiB) BEFORE
readNetstring capped it — 5 bytes committed ~4 GB. Now reject nsLen>1<<24 immediately.
#2 [DoS]: parseV2Content had no element-count/byte cap (16 MiB content = 4.2M argv, ~388 MB).
Now caps maxArgs=4096 / maxFrameBytes=1<<20, mirroring runV1. Regression tests for both.
#1 (empty-arg terminator collision) is a SEPARATE ship-blocker, deferred (needs a wire-design
call: explicit argc vs client-side reject) — do NOT deploy the daemon until #1 lands.
v2Frame now takes explicit argv [][]byte (not a space-joined command string) and builds
content = argv-netstrings + 0:, + payload, so the client preserves the users argument boundaries
end-to-end. wrapHop space-splits the hop VERB (space-safe) + appends sicd; buildChain folds
verbs[0] outermost. Consolidated the three WP tests into frame_test.go with an independent
reference builder. Addressing layer (parseTarget/verbFor/resolveVerbs/parseHostsConfig) unchanged.
The v2 command field was a single string the daemon SPACE-SPLIT into argv — which silently
broke sic's founding guarantee (`ssh host touch 'a b'` makes TWO files; `sic host touch 'a b'`
must make ONE). The composition of the v2 client surfaced this: every argument with a space
would be re-split.
Fix (markus chose it): v2 content is now a run of djb netstrings (the argv), terminated by the
empty netstring 0:,, then the payload — mirroring the boundary-safe v1 exec encoding. Each
argument is length-framed, so spaces, NUL, 0xFF all ride untouched; readNetstringStream already
bounds each element + its length token, so a hostile content cannot OOM here. runV2 uses the new
parseV2Content; the test frame() helper space-splits test-side only (the wire never does).
Proof: TestV2ArgvBoundaryPreserved — argv {script, "a b"} makes the script see 1 arg "a b", not
2. All existing v2 + v1 dual-read tests stay green; go vet clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
buildChain(verbs, cmd, payload): innermost v2Frame(cmd,payload), wrap outward so verbs[0] is the
outermost layer; zero verbs = a single v2 frame (bare host). @godev/deepseek.
wrapHop(verb, inner) = v2Frame(verb+" sicd", inner): the hop command is the runtime verb with
sicd appended (the inner-hop peeler), the inner frame is the payload. @godev/deepseek, one grind.
First function-level package of Piece 2. v2Frame(command, payload) builds one v2 frame:
0x00 + be32(len(netstring)) + netstring(command 0x00 payload). Byte-exact tests (cat/hi, empty
payload nil==[]byte{}, binary payload riding untouched). @testdesigner proved satisfiability in
scratch + caught a be32 arithmetic error in the spec; @godev (deepseek default) implemented it —
the small-package decomposition carrying on the room grinder where the monolithic parser could not.
#1 [HIGH, was a real DoS]: readNetstringStream read the length token via bufio.ReadString(':'),
which buffers the whole pre-':' run UNBOUNDED — the 1<<24 cap only bounded the body, applied
AFTER the token was parsed. A protocol-confined caller sending an endless digit run with no ':'
drove sicd to ~1 GB RSS (measured by @reviewer). Now the token is read byte-by-byte with an
8-digit hard cap (a valid <=1<<24 length is 8 digits), so it rejects after <=9 bytes regardless
of input size — memory is bounded by construction, before the value is trusted.
#2 [MED]: nothing bounded the NUMBER of netstrings per frame (2M tiny ones ~140 MB before exec).
runV1 now caps element count (4096) and total argv bytes (1<<20).
#3 [MED, documented]: an empty netstring 0:, is always the terminator, so a legit empty ""
argument collides with it (silent argv truncation + stdin bleed). Inherent to the v1 wire, not
introduced here — noted in a comment; a fix would need a wire change.
Regression tests: overlong length token (100k digits) -> exit 1; 9-digit length -> exit 1;
frame over the element cap -> exit 1. go test ./... ok, go vet clean. Found by @reviewer's
probe (reproduced 1 GB RSS), not a re-read — exactly why the outside look isn't skippable.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The v2-only daemon rejected v1 frames, so deploying it would brick every v1 client in the
field. main() now dispatches on the FIRST byte: 0x00 = v2 (path unchanged); an ASCII digit =
a new STREAMING v1-legacy parser (that digit is the first byte of the first netstring length,
pushed back via io.MultiReader before parsing); anything else = exit 1 with a diagnostic.
v1 wire (what the deployed cmd/sic sends, no preamble): netstring(mode "exec"|"sh") +
argv/command netstrings + the empty-netstring terminator 0:, ; everything after the terminator
is the child's stdin. exec runs argv directly (each netstring is ONE argv element, never
space-split); sh runs via sh -c. Reuses pumpStdin so EPIPE / reap / exit-status semantics
match the v2 path exactly.
Removed the sicd_test.go "old-v1-frame-without-preamble expects exit 1" subtest: it pinned the
v2-only REJECTION of v1, which dual-read deliberately reverses. v1 acceptance is now covered by
dualread_test.go (9 cases: exec/sh, argv boundaries, trailing stdin, exit status, and the four
reject paths).
Implemented by noether (rich agent) after the room hit a genuine wall — DeepSeek couldn't build
the parser in 10 attempts, Kimi-k2.6 soliloquy-stalled past the 300s per-call timeout. All green:
go test ./... ok, 27/27 Python reference.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Acting on @reviewer's fourth-pass verdict (all three points accepted):
1. Refactored the pump into pumpStdin(dst, src, reap, errOut) int and covered it with unit
tests using a mock erroring io.Reader — which IS the Python reference's mechanism
(monkeypatching os.read to raise OSError), translated. Three cases: a genuine read error
reaps + exits 1 + diagnoses; EPIPE is head-semantics (return child status, no diagnostic);
clean EOF returns child status. The waitForChild added in the prior commit now has coverage.
2. Corrected the pty test's skip string. My earlier rationale was wrong twice, as @reviewer's
raw syscall.Read probe showed: it is NOT a Go-vs-CPython poller difference — the KERNEL
returns a clean EOF on the slave side in BOTH languages (EIO-after-hangup is master-side);
and the Python reference never used a pty (it mocks os.read). The skip now states the
corrected facts and points at TestPumpStdin*.
3. Dropped readNetstring's vestigial third return (rest was provably always empty).
Go ok (incl. 3 new unit tests), 27/27 Python, go vet clean.
FINDING 1 (security) — the Go daemon accepted netstrings the Python reference REJECTS and
then EXECUTED them, so a malformed frame that fails closed on a Python hop ran on a Go hop.
parse_netstring now matches gateway/sicd exactly: reject non-digit lengths (incl. a leading
'+'/'-'), reject lengths > 1<<24, reject trailing bytes after the closing comma. The three
cases now exit 1 with a diagnostic instead of exec'ing. Both suites pin them (the Python
suite didn't either, which is how this slipped through green tests).
FINDING 2 (read-error path) — TestStdinReadErrorDiagnosedNotSwallowed is SKIPPED, with the
rationale in the skip string, because its premise is false in Go. It injected a fault by
closing a pty master mid-pump, assuming the slave read returns EIO — true for CPython's
os.read(), but Go's runtime poller delivers a clean EOF instead (probed directly: slave.Read
-> io.EOF, io.Copy -> nil; a socketpair peer close is EOF too — SO_LINGER/RST is TCP-only).
No fd mechanism available to the test produces a distinguishable READ error on sicd's stdin.
That is not a gap in main.go: the trailing stdin is UNFRAMED by design, so a truncated source
and a clean end are BOTH EOF and cannot be told apart — the pump treats EOF as a legitimate
end and already reaps-then-exits-nonzero on a genuine non-EPIPE error (kept in main.go), which
just doesn't arise via pty/socket close in Go.
Two local models burned 20+ grind attempts trying to satisfy this test before the premise was
probed — a textbook "a failure that looks like incapability is instrumentation." Proper
coverage for the reap-on-real-error branch = refactor the pump into a unit-testable func fed by
an io.Reader that returns a non-EOF error; left as a follow-up.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Go daemon is what actually runs on 30+ fleet hosts; until now it had none of the
wire-protocol work that went into the Python reference (gateway/sicd) — which is why sic
still ate a bare -- and silently wrote zero-byte files in production. This ports it: preamble
+ nested netstring peeling (one layer per hop), payload -> child stdin then remaining stdin
pumped through (the zero-byte-file fix), bare -- survives as opaque payload, malformed frame
-> exit 1, no deadlock above the 64 KiB pipe buffer, short writes looped, EPIPE handled,
signal-killed child -> exit 128+WTERMSIG. Built by the bullpen room (@testdesigner -> @godev
-> @reviewer), the first Go work the room could grade.
CHECKPOINT commit, green but NOT final: 28/28 Go, 24/24 Python, go vet clean. @reviewer's
third pass found four divergences from the reference that a fix round is about to address —
recorded here so the checkpoint is honest:
1. (must-fix, security) the Go netstring parser accepts frames the reference REJECTS and
executes them: trailing data after the comma, a +-prefixed length, and no 1<<24 cap. A
malformed frame that fails closed on a Python hop currently runs on a Go hop. The Python
suite does not pin these either — both suites need the malformed cases added.
2. (must-fix) the read-error path os.Exit(1)s without reaping the child, leaving it detached
and still writing to inherited stdout after sicd reports its status.
3. (test) an EPIPE test's premise is wrong in Go (LookPath fails at Start, so the payload
write never happens); the path is really covered by the signal test.
4. (test) the pty EIO test is synchronized only by a 300 ms sleep.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The quotesu setup notes exposed internal host details and aren't useful to
readers, so remove the directory. The quoting-hell demo now requires the
target host as an argument (any host running sicd) instead of defaulting to
an internal name, and its labels say sic rather than the old rs. Example
hostnames in the README/SKILL/main.go are now the neutral placeholder host1.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EWpfhDgYNA21tETDP9ueBE
Client (sic) frames argv as netstrings and pipes it over ssh to a daemon
(sicd) that execvp's it, so no shell re-parses the arguments. exec mode by
default; opt-in sh mode for pipes/redirects/globs. Go client + daemon, Python
reference daemon, design doc, quoting-hell demo, and foreground/background
agent skills.
Renamed from the climcp prototype.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EWpfhDgYNA21tETDP9ueBE
Claude (noether)