sicd: v2 wire preserves argv boundaries (content = argv netstrings + terminator + payload)

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>

cmd/sicd/main.go

@@ -91,6 +91,29 @@ func readNetstringStream(r *bufio.Reader) ([]byte, bool) {
 	return content, true
 }
 
+// parseV2Content splits a v2 netstring's content into argv (a run of netstrings terminated by
+// the empty netstring 0:,) and the trailing payload. Length-framed argv preserves argument
+// boundaries and lets any byte (space, NUL, 0xFF) appear in an argument; readNetstringStream
+// bounds each element and its length token, so a hostile content cannot OOM here.
+func parseV2Content(content []byte) (argv [][]byte, payload []byte, ok bool) {
+	r := bufio.NewReader(bytes.NewReader(content))
+	for {
+		ns, good := readNetstringStream(r)
+		if !good {
+			return nil, nil, false
+		}
+		if len(ns) == 0 {
+			break // the empty netstring 0:, terminates argv
+		}
+		argv = append(argv, ns) // readNetstringStream returns a fresh slice; no aliasing
+	}
+	rest, err := io.ReadAll(r)
+	if err != nil {
+		return nil, nil, false
+	}
+	return argv, rest, true
+}
+
 func waitForChild(cmd *exec.Cmd) int {
 	if err := cmd.Wait(); err != nil {
 		if exitErr, ok := err.(*exec.ExitError); ok {
@@ -151,23 +174,23 @@ func runV2() {
 		os.Exit(1)
 	}
 
-	nulIdx := bytes.IndexByte(content, 0x00)
+	// v2 content = argv netstrings + the empty-netstring terminator 0:, + payload. Length-framed
-	if nulIdx < 0 {
+	// argv PRESERVES argument boundaries (sic's founding guarantee: `touch 'a b'` is ONE file)
-		fmt.Fprintf(os.Stderr, "sicd: content missing NUL separator\n")
+	// and lets ANY byte appear in an argument — unlike the old space-split. Everything after the
+	// terminator is the payload (a nested frame for the next hop, or empty).
+	argv, payload, ok := parseV2Content(content)
+	if !ok {
+		fmt.Fprintf(os.Stderr, "sicd: invalid v2 content (argv framing)\n")
+		os.Exit(1)
+	}
+	if len(argv) == 0 {
+		fmt.Fprintf(os.Stderr, "sicd: empty command\n")
 		os.Exit(1)
 	}
-	command := content[:nulIdx]
-	payload := content[nulIdx+1:]
-
-	argv := bytes.Split(command, []byte(" "))
 	argvStr := make([]string, len(argv))
 	for i, a := range argv {
 		argvStr[i] = string(a)
 	}
-	if len(argvStr) == 0 {
-		fmt.Fprintf(os.Stderr, "sicd: empty command\n")
-		os.Exit(1)
-	}
 
 	cmd := exec.Command(argvStr[0], argvStr[1:]...)
 	stdinPipe, err := cmd.StdinPipe()

cmd/sicd/sicd_test.go

@@ -75,7 +75,15 @@ func be32(n uint32) []byte {
 
 // frame builds one wire frame: magic, 4-byte BE length, netstring(command NUL payload).
 func frame(command, payload []byte) []byte {
-	ns := netstring(concat(command, []byte{0x00}, payload))
+	// v2 content = argv netstrings + the empty-netstring terminator 0:, + payload. The command is
+	// split on spaces HERE (test-side convenience); the wire itself no longer space-splits — argv
+	// arrives length-framed so `touch 'a b'` keeps its boundary.
+	var content []byte
+	for _, a := range bytes.Split(command, []byte(" ")) {
+		content = concat(content, netstring(a))
+	}
+	content = concat(content, []byte("0:,"), payload)
+	ns := netstring(content)
 	return concat([]byte{0x00}, be32(uint32(len(ns))), ns)
 }
 
@@ -590,3 +598,29 @@ func TestPumpStdinCleanEofReturnsChildStatus(t *testing.T) {
 		t.Fatalf("a clean EOF is a legitimate end; must return the child's status 7, got %d", code)
 	}
 }
+
+// v2FrameArgv builds a v2 frame from EXPLICIT argv (no space-split) — the whole point of the
+// argv-netstring wire: each element is length-framed and rides untouched.
+func v2FrameArgv(argv [][]byte, payload []byte) []byte {
+	var content []byte
+	for _, a := range argv {
+		content = concat(content, netstring(a))
+	}
+	content = concat(content, []byte("0:,"), payload)
+	ns := netstring(content)
+	return concat([]byte{0x00}, be32(uint32(len(ns))), ns)
+}
+
+func TestV2ArgvBoundaryPreserved(t *testing.T) {
+	// sic's founding guarantee, now on the v2 wire: an argument containing a space is ONE argv
+	// element, never re-split. Before the argv-netstring wire this failed — the space-split gave
+	// the script two args and printed "2|a". This is the regression that pins the fix.
+	script := shScript(t, `printf '%s|%s\n' "$#" "$1"`)
+	r := runSicd(t, v2FrameArgv([][]byte{[]byte(script), []byte("a b")}, nil))
+	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("v2 argv boundary LOST: script saw %q, want %q", got, want)
+	}
+}