sicd: implement v1/v2 dual-read (migration) — first-byte dispatch + v1-legacy parser

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>
This commit is contained in:
Claude (noether)
2026-07-23 10:42:32 +02:00
parent 3ce50a8ba3
commit 5d2ee5cdc9
3 changed files with 294 additions and 6 deletions
+180
View File
@@ -0,0 +1,180 @@
// dualread_test.go — executable spec for daemon dual-read (wire protocol v1 + v2).
//
// The DEPLOYED cmd/sic client speaks v1: raw netstrings with NO preamble —
//
// v1 frame := netstring(mode) argnets "0:,"
// mode := "exec" | "sh"
// argnets := netstring(arg)... (exec: one netstring PER argv element)
// | netstring(joined command) (sh: single space-joined command line)
//
// Bytes after the "0:," terminator are the child's stdin, 8-bit clean.
// Semantics: exec runs argv directly (NO shell, NO space-splitting — each
// netstring is one argv element); sh runs the command line through a shell;
// the daemon inherits the child's exit status.
//
// The current daemon speaks only v2 (0x00 magic + be32 length + netstring)
// and exits 1 on anything else, which bricks every deployed v1 client.
// Dual-read contract: dispatch on the FIRST byte — ASCII digit ⇒ v1 frame,
// 0x00 ⇒ v2 frame, anything else ⇒ exit 1 with a diagnostic, exec nothing.
//
// Reuses the sicd_test.go harness (same package): TestMain/sicdBin, runSicd,
// frame, netstring, concat, shScript, pattern.
//
// NOTE for the implementer: sicd_test.go's malformed-input case
// "old-v1-frame-without-preamble" asserts the OPPOSITE of this contract and
// must be deleted when dual-read lands — both cannot be green.
package main
import (
"bytes"
"strings"
"testing"
)
// v1Frame replicates cmd/sic frameArgv byte-for-byte: mode netstring, then
// args as individual netstrings ("exec") or one space-joined netstring
// ("sh"), then the empty-netstring terminator.
func v1Frame(mode string, args ...string) []byte {
var b bytes.Buffer
b.Write(netstring([]byte(mode)))
switch mode {
case "exec":
for _, a := range args {
b.Write(netstring([]byte(a)))
}
case "sh":
b.Write(netstring([]byte(strings.Join(args, " "))))
}
b.WriteString("0:,")
return b.Bytes()
}
// --- v1 acceptance (RED on the current daemon: dies at the magic-byte check) --
func TestV1ExecFrameRunsCommand(t *testing.T) {
r := runSicd(t, v1Frame("exec", "echo", "hello"))
if r.code != 0 {
t.Fatalf("v1 exec frame must run; expected exit 0, got %d (killedBy=%v), stderr=%q",
r.code, r.killedBy, r.stderr)
}
if got := string(r.stdout); got != "hello\n" {
t.Fatalf("stdout = %q, want %q", got, "hello\n")
}
}
func TestV1ExecArgvBoundariesPreserved(t *testing.T) {
// v1 exec carries each argument in its OWN netstring: an arg containing
// a space is one argv element. A daemon that funnels v1 through the v2
// space-split path hands the script two args and prints "2|a".
script := shScript(t, `printf '%s|%s\n' "$#" "$1"`)
r := runSicd(t, v1Frame("exec", string(script), "a b"))
if r.code != 0 {
t.Fatalf("expected exit 0, got %d, stderr=%q", r.code, r.stderr)
}
if got, want := string(r.stdout), "1|a b\n"; got != want {
t.Fatalf("argv boundaries lost: script saw %q, want %q — v1 args must NOT be space-split", got, want)
}
}
func TestV1ExecTrailingStdinForwardedToChild(t *testing.T) {
// Bytes after the "0:," terminator are the child's stdin, verbatim:
// 1 KiB covering every byte value (NULs, 0xFF, netstring-ish colons and
// commas) must never be re-parsed as framing or mangled.
body := pattern(4)
r := runSicd(t, concat(v1Frame("exec", "cat"), body))
if r.code != 0 {
t.Fatalf("expected exit 0, got %d (killedBy=%v), stderr=%q", r.code, r.killedBy, r.stderr)
}
if !bytes.Equal(r.stdout, body) {
t.Fatalf("trailing stdin truncated or corrupted: got %d of %d bytes", len(r.stdout), len(body))
}
}
func TestV1ShModeRunsThroughShell(t *testing.T) {
// sh mode is the one v1 path that DOES go through a shell: a pipeline
// must actually pipe. `echo hi | wc -c` ⇒ "3".
r := runSicd(t, v1Frame("sh", "echo hi | wc -c"))
if r.code != 0 {
t.Fatalf("expected exit 0, got %d, stderr=%q", r.code, r.stderr)
}
if got := strings.TrimSpace(string(r.stdout)); got != "3" {
t.Fatalf("pipeline did not run through a shell: stdout = %q, want \"3\"", got)
}
}
func TestV1ChildExitStatusInherited(t *testing.T) {
r := runSicd(t, v1Frame("sh", "exit 7"))
if r.code != 7 {
t.Fatalf("v1 must inherit the child's exit status 7, got %d, stderr=%q", r.code, r.stderr)
}
}
// --- v2 must keep working alongside v1 (green today; regression guard) -------
func TestV2FrameStillAcceptedAlongsideV1(t *testing.T) {
payload := []byte("v2 payload\n")
trailing := []byte("and trailing stdin")
r := runSicd(t, concat(frame([]byte("cat"), payload), trailing))
if r.code != 0 {
t.Fatalf("expected exit 0, got %d (killedBy=%v), stderr=%q", r.code, r.killedBy, r.stderr)
}
if want := concat(payload, trailing); !bytes.Equal(r.stdout, want) {
t.Fatalf("stdout = %q, want %q", r.stdout, want)
}
}
// --- dispatch must stay strict (green today; pins the future dispatcher) -----
func TestDispatchRejectsUnknownFirstByte(t *testing.T) {
// Dual-read must not degenerate into "anything non-0x00 is v1": a first
// byte that is neither an ASCII digit nor 0x00 exits 1, execs nothing.
r := runSicd(t, concat([]byte{0xff}, v1Frame("exec", "cat")))
if r.code != 1 {
t.Fatalf("expected exit 1, got %d (killedBy=%v)", r.code, r.killedBy)
}
if len(r.stdout) != 0 {
t.Fatalf("unknown first byte must not exec anything, stdout=%q", r.stdout)
}
if len(bytes.TrimSpace(r.stderr)) == 0 {
t.Fatal("unknown first byte must produce a diagnostic on stderr")
}
}
func TestV1TruncatedFrameMissingTerminatorExits1(t *testing.T) {
// EOF before the "0:," terminator: malformed — diagnose and exit 1,
// never hang waiting for more netstrings and never exec a partial argv.
r := runSicd(t, concat(netstring([]byte("exec")), netstring([]byte("cat"))))
if r.code != 1 {
t.Fatalf("expected exit 1, got %d (killedBy=%v), stdout=%q", r.code, r.killedBy, r.stdout)
}
if len(r.stdout) != 0 {
t.Fatalf("truncated v1 frame must not exec anything, stdout=%q", r.stdout)
}
if len(bytes.TrimSpace(r.stderr)) == 0 {
t.Fatal("truncated v1 frame must produce a diagnostic on stderr")
}
}
func TestV1UnknownModeRejected(t *testing.T) {
// Mode is a closed set {exec, sh}; anything else is malformed, not a
// command to run.
wire := concat(netstring([]byte("rm")), netstring([]byte("-rf")), []byte("0:,"))
r := runSicd(t, wire)
if r.code != 1 {
t.Fatalf("expected exit 1, got %d (killedBy=%v), stdout=%q", r.code, r.killedBy, r.stdout)
}
if len(bytes.TrimSpace(r.stderr)) == 0 {
t.Fatal("unknown v1 mode must produce a diagnostic on stderr")
}
}
func TestV1EmptyExecArgvRejected(t *testing.T) {
// "4:exec,0:," — terminator immediately after the mode: no command.
r := runSicd(t, v1Frame("exec"))
if r.code != 1 {
t.Fatalf("expected exit 1, got %d (killedBy=%v), stdout=%q", r.code, r.killedBy, r.stdout)
}
if len(bytes.TrimSpace(r.stderr)) == 0 {
t.Fatal("empty v1 exec argv must produce a diagnostic on stderr")
}
}
+114 -5
View File
@@ -1,6 +1,7 @@
package main package main
import ( import (
"bufio"
"bytes" "bytes"
"encoding/binary" "encoding/binary"
"errors" "errors"
@@ -10,6 +11,7 @@ import (
"os/exec" "os/exec"
"os/signal" "os/signal"
"strconv" "strconv"
"strings"
"syscall" "syscall"
) )
@@ -44,6 +46,39 @@ func readNetstring(buf []byte) ([]byte, bool) {
return buf[start:end], true return buf[start:end], true
} }
// readNetstringStream reads ONE djb netstring (<len>:<bytes>,) from a stream (unlike
// readNetstring, which parses a single fully-buffered frame). ok=false on EOF or malformed
// framing (non-digit length, short body, missing comma, length > 1<<24). Used by the v1 path,
// which is a SEQUENCE of netstrings terminated by the empty netstring 0:,.
func readNetstringStream(r *bufio.Reader) ([]byte, bool) {
lenStr, err := r.ReadString(':')
if err != nil {
return nil, false // EOF before ':' — truncated
}
lenStr = lenStr[:len(lenStr)-1] // drop the ':'
if len(lenStr) == 0 {
return nil, false
}
for i := 0; i < len(lenStr); i++ {
if lenStr[i] < '0' || lenStr[i] > '9' {
return nil, false
}
}
n, err := strconv.Atoi(lenStr)
if err != nil || n > 1<<24 {
return nil, false
}
content := make([]byte, n)
if _, err := io.ReadFull(r, content); err != nil {
return nil, false
}
comma, err := r.ReadByte()
if err != nil || comma != ',' {
return nil, false
}
return content, true
}
func waitForChild(cmd *exec.Cmd) int { func waitForChild(cmd *exec.Cmd) int {
if err := cmd.Wait(); err != nil { if err := cmd.Wait(); err != nil {
if exitErr, ok := err.(*exec.ExitError); ok { if exitErr, ok := err.(*exec.ExitError); ok {
@@ -61,16 +96,30 @@ func waitForChild(cmd *exec.Cmd) int {
func main() { func main() {
signal.Ignore(syscall.SIGPIPE) signal.Ignore(syscall.SIGPIPE)
magic := make([]byte, 1) // Dual-read dispatch on the FIRST byte, so a v2 daemon can be deployed without bricking the
if _, err := io.ReadFull(os.Stdin, magic); err != nil { // v1 clients still in the field: 0x00 = the v2 preamble; an ASCII digit = the first byte of
fmt.Fprintf(os.Stderr, "sicd: read magic: %v\n", err) // a v1 netstring length (legacy, no preamble); anything else is malformed.
first := make([]byte, 1)
if _, err := io.ReadFull(os.Stdin, first); err != nil {
fmt.Fprintf(os.Stderr, "sicd: read first byte: %v\n", err)
os.Exit(1) os.Exit(1)
} }
if magic[0] != 0x00 { switch {
fmt.Fprintf(os.Stderr, "sicd: invalid magic byte: 0x%02x\n", magic[0]) case first[0] == 0x00:
runV2() // magic consumed; the rest (len32 + netstring + trailing) is on stdin
case first[0] >= '0' && first[0] <= '9':
// The byte we consumed for dispatch is the first digit of the first netstring's
// length — push it back before parsing.
runV1(bufio.NewReader(io.MultiReader(bytes.NewReader(first), os.Stdin)))
default:
fmt.Fprintf(os.Stderr, "sicd: invalid first byte: 0x%02x\n", first[0])
os.Exit(1) os.Exit(1)
} }
}
// runV2 is the v2 wire path (magic already consumed by main): 4-byte big-endian length, one
// netstring whose content is <command> 0x00 <payload>, then trailing stdin forwarded to the child.
func runV2() {
lenBytes := make([]byte, 4) lenBytes := make([]byte, 4)
if _, err := io.ReadFull(os.Stdin, lenBytes); err != nil { if _, err := io.ReadFull(os.Stdin, lenBytes); err != nil {
fmt.Fprintf(os.Stderr, "sicd: read length: %v\n", err) fmt.Fprintf(os.Stderr, "sicd: read length: %v\n", err)
@@ -135,6 +184,66 @@ func main() {
os.Exit(pumpStdin(stdinPipe, os.Stdin, func() int { return waitForChild(cmd) }, os.Stderr)) os.Exit(pumpStdin(stdinPipe, os.Stdin, func() int { return waitForChild(cmd) }, os.Stderr))
} }
// runV1 handles the legacy v1 wire the deployed cmd/sic still sends (raw netstrings, no
// preamble): first netstring = mode ("exec" | "sh"); then the argv/command netstrings;
// terminated by the empty netstring 0:,. Everything after the terminator is the child's stdin.
// Kept only for the migration window so a v2 daemon does not reject v1 callers.
func runV1(r *bufio.Reader) {
modeB, ok := readNetstringStream(r)
if !ok {
fmt.Fprintf(os.Stderr, "sicd: v1 malformed frame (mode)\n")
os.Exit(1)
}
mode := string(modeB)
if mode != "exec" && mode != "sh" {
fmt.Fprintf(os.Stderr, "sicd: v1 unknown mode %q\n", mode)
os.Exit(1)
}
// Read argv/command netstrings until the empty-netstring terminator. EOF first = truncated.
var argv []string
for {
ns, ok := readNetstringStream(r)
if !ok {
fmt.Fprintf(os.Stderr, "sicd: v1 truncated frame (no 0:, terminator)\n")
os.Exit(1)
}
if len(ns) == 0 {
break // the empty netstring 0:, terminates the frame
}
argv = append(argv, string(ns))
}
var cmd *exec.Cmd
switch mode {
case "exec":
if len(argv) == 0 {
fmt.Fprintf(os.Stderr, "sicd: v1 empty exec argv\n")
os.Exit(1)
}
cmd = exec.Command(argv[0], argv[1:]...) // each netstring is ONE argv element, never space-split
case "sh":
cmd = exec.Command("sh", "-c", strings.Join(argv, " "))
}
stdinPipe, err := cmd.StdinPipe()
if err != nil {
fmt.Fprintf(os.Stderr, "sicd: create stdin pipe: %v\n", err)
os.Exit(1)
}
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
if err := cmd.Start(); err != nil {
fmt.Fprintf(os.Stderr, "sicd: exec: %v\n", err)
os.Exit(1)
}
// v1 has no framed payload: everything after 0:, (still buffered in r + the rest of stdin)
// is the child's stdin. Reuse the v2 pump so EPIPE/reap/exit-status semantics match exactly.
os.Exit(pumpStdin(stdinPipe, r, func() int { return waitForChild(cmd) }, os.Stderr))
}
// pumpStdin copies src (sicd's own stdin) into the child's stdin (dst), closes dst so the // pumpStdin copies src (sicd's own stdin) into the child's stdin (dst), closes dst so the
// child sees EOF, then reaps. EPIPE from the WRITE side means the child stopped reading // child sees EOF, then reaps. EPIPE from the WRITE side means the child stopped reading
// (| head semantics) — not an error, so the child's own status is returned. Any OTHER error // (| head semantics) — not an error, so the child's own status is returned. Any OTHER error
-1
View File
@@ -299,7 +299,6 @@ func TestMalformedInputExits1WithDiagnostic(t *testing.T) {
}{ }{
{"empty-input", nil}, {"empty-input", nil},
{"missing-magic", []byte("garbage with no magic byte")}, {"missing-magic", []byte("garbage with no magic byte")},
{"old-v1-frame-without-preamble", []byte("4:exec,3:cat,0:,")},
{"body-not-a-netstring", concat([]byte{0x00}, be32(14), []byte("not&a netstrng"))}, {"body-not-a-netstring", concat([]byte{0x00}, be32(14), []byte("not&a netstrng"))},
{"truncated-declared-length", concat([]byte{0x00}, be32(100), netstring([]byte("cat\x00hi")))}, {"truncated-declared-length", concat([]byte{0x00}, be32(100), netstring([]byte("cat\x00hi")))},
{"content-missing-nul-truncated", concat([]byte{0x00}, be32(6), netstring([]byte("ca")))}, {"content-missing-nul-truncated", concat([]byte{0x00}, be32(6), netstring([]byte("ca")))},