sic v2 client: rework frame builders to the argv-netstring wire (match daemon bf82a80)

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.

cmd/sic/buildchain_test.go

@@ -1,31 +0,0 @@
-package main
-
-import (
-	"bytes"
-	"testing"
-)
-
-// WP3: buildChain folds wrapHop over the resolved per-hop verbs (verbs[0] outermost) around the
-// innermost v2Frame(cmd, payload). Relational: each want is built from the real v2Frame/wrapHop,
-// so this pins the composition and fold ORDER, not a re-derived byte layout.
-func TestBuildChain(t *testing.T) {
-	cmd := []byte("cat")
-	payload := []byte("hi")
-	cases := []struct {
-		name  string
-		verbs []string
-		want  []byte
-	}{
-		{"zero hops nil = single v2 frame", nil, v2Frame(cmd, payload)},
-		{"zero hops empty slice", []string{}, v2Frame(cmd, payload)},
-		{"one hop", []string{"incus exec c --"},
-			wrapHop("incus exec c --", v2Frame(cmd, payload))},
-		{"two hops, verbs[0] is the OUTERMOST layer", []string{"incus exec c --", "docker exec d"},
-			wrapHop("incus exec c --", wrapHop("docker exec d", v2Frame(cmd, payload)))},
-	}
-	for _, tc := range cases {
-		if got := buildChain(tc.verbs, cmd, payload); !bytes.Equal(got, tc.want) {
-			t.Fatalf("%s:\n got  % x\n want % x", tc.name, got, tc.want)
-		}
-	}
-}

cmd/sic/frame.go

@@ -3,40 +3,57 @@ package main
 import (
 	"encoding/binary"
 	"strconv"
+	"strings"
 )
 
-// v2Frame returns one v2 wire frame:
+// nsBytes builds a djb netstring "<len>:<bytes>," over arbitrary bytes (binary-safe).
-//
+func nsBytes(b []byte) []byte {
-//	0x00 ++ big-endian uint32 length of the netstring ++ netstring
+	out := append([]byte(strconv.Itoa(len(b))), ':')
-//
+	out = append(out, b...)
-// where netstring = "<len>:<content>," and content = command ++ 0x00 ++ payload.
+	return append(out, ',')
-func v2Frame(command []byte, payload []byte) []byte {
+}
-	// Build content: command + 0x00 + payload
-	content := make([]byte, len(command)+1+len(payload))
-	copy(content, command)
-	content[len(command)] = 0x00
-	copy(content[len(command)+1:], payload)
 
-	// Build netstring: "<len>:<content>,"
+// v2Frame builds one v2 wire frame from an EXPLICIT argv (boundary-preserving) plus a payload:
-	netstr := strconv.Itoa(len(content)) + ":" + string(content) + ","
+//
+//	0x00 ++ big-endian uint32 length of the netstring ++ netstring(content)
+//	content = <argv netstrings> ++ "0:," (empty-netstring terminator) ++ payload
+//
+// Length-framed argv is why `sic host touch 'a b'` sends ONE argument, not two — no space-split
+// anywhere. The payload is the next hop's nested frame, or empty for the innermost command.
+func v2Frame(argv [][]byte, payload []byte) []byte {
+	var content []byte
+	for _, a := range argv {
+		content = append(content, nsBytes(a)...)
+	}
+	content = append(content, "0:,"...) // empty netstring terminates argv
+	content = append(content, payload...)
 
-	// Build frame: 0x00 ++ be32(len(netstr)) ++ netstr
+	ns := nsBytes(content)
-	frame := make([]byte, 1+4+len(netstr))
+	frame := make([]byte, 1+4+len(ns))
 	frame[0] = 0x00
-	binary.BigEndian.PutUint32(frame[1:5], uint32(len(netstr)))
+	binary.BigEndian.PutUint32(frame[1:5], uint32(len(ns)))
-	copy(frame[5:], netstr)
+	copy(frame[5:], ns)
-
 	return frame
 }
 
-// wrapHop wraps an already-built inner frame for one chain hop.
+// wrapHop wraps an inner frame for one chain hop: the hop's argv is the runtime verb split on
+// spaces with "sicd" appended (the inner-hop peeler), and the inner frame is the payload. Verbs
+// are space-safe by construction (no spaces in container names/ids), so splitting them is fine.
 func wrapHop(verb string, inner []byte) []byte {
-	return v2Frame([]byte(verb+" sicd"), inner)
+	fields := strings.Fields(verb)
+	argv := make([][]byte, 0, len(fields)+1)
+	for _, f := range fields {
+		argv = append(argv, []byte(f))
+	}
+	argv = append(argv, []byte("sicd"))
+	return v2Frame(argv, inner)
 }
 
-// buildChain builds a chain of wrapped frames from verbs, with the innermost being v2Frame(cmd, payload).
+// buildChain builds the whole onion: innermost v2Frame(cmdArgv, payload), then wrap outward so
-func buildChain(verbs []string, cmd []byte, payload []byte) []byte {
+// verbs[0] is the outermost layer (the one the host's sicd enters first). Zero verbs yields a
-	frame := v2Frame(cmd, payload)
+// single v2 frame (a bare host with no nesting).
+func buildChain(verbs []string, cmdArgv [][]byte, payload []byte) []byte {
+	frame := v2Frame(cmdArgv, payload)
 	for i := len(verbs) - 1; i >= 0; i-- {
 		frame = wrapHop(verbs[i], frame)
 	}

cmd/sic/frame_test.go

@@ -2,53 +2,74 @@ package main
 
 import (
 	"bytes"
+	"encoding/binary"
+	"strconv"
 	"testing"
 )
 
-// WP1 spec for v2Frame: one v2 wire frame, byte-exact to what cmd/sicd parses:
+// Independent reference builders (a second implementation, so a bug in frame.go can't cancel out
-//   0x00  +  big-endian uint32 length of the netstring  +  netstring(command 0x00 payload)
+// of both sides of the assertion).
-// Hand-written byte literals (no helper arithmetic that could cancel a framer bug out of both
+func refNS(b []byte) []byte {
-// sides). Byte counts verified against cmd/sicd/main.go's readNetstring, which rejects an
+	out := []byte(strconv.Itoa(len(b)) + ":")
-// over- or under-counted length, so these lengths must be exact.
+	out = append(out, b...)
-
+	return append(out, ',')
-func TestV2FrameCatHi(t *testing.T) {
+}
-	// content = "cat\x00hi" (6 bytes); netstring "6:cat\x00hi," (9 bytes); frame 14 bytes.
+func refFrame(content []byte) []byte {
-	want := []byte{
+	ns := refNS(content)
-		0x00,                   // magic
+	f := []byte{0x00}
-		0x00, 0x00, 0x00, 0x09, // be32(9) = length of the netstring
+	var l [4]byte
-		'6', ':', 'c', 'a', 't', 0x00, 'h', 'i', ',',
+	binary.BigEndian.PutUint32(l[:], uint32(len(ns)))
+	return append(append(f, l[:]...), ns...)
+}
+func refContent(argv [][]byte, payload []byte) []byte {
+	var c []byte
+	for _, a := range argv {
+		c = append(c, refNS(a)...)
 	}
-	if got := v2Frame([]byte("cat"), []byte("hi")); !bytes.Equal(got, want) {
+	c = append(c, "0:,"...)
-		t.Fatalf("v2Frame(cat,hi) =\n % x\nwant\n % x", got, want)
+	return append(c, payload...)
+}
+
+func TestV2Frame(t *testing.T) {
+	// An argument containing a space is ONE length-framed element — sic's founding guarantee.
+	argv := [][]byte{[]byte("cat"), []byte("a b")}
+	payload := []byte("hi")
+	want := refFrame(refContent(argv, payload))
+	if got := v2Frame(argv, payload); !bytes.Equal(got, want) {
+		t.Fatalf("v2Frame =\n % x\nwant\n % x", got, want)
 	}
 }
 
-func TestV2FrameEmptyPayload(t *testing.T) {
+func TestV2FrameBinaryAndEmpty(t *testing.T) {
-	// content = "x\x00" (2 bytes); netstring "2:x\x00," (5 bytes); frame 10 bytes.
+	// Payload bytes that look like framing ride untouched; nil and []byte{} payload are identical.
-	want := []byte{
+	argv := [][]byte{[]byte("x")}
-		0x00,
-		0x00, 0x00, 0x00, 0x05, // be32(5)
-		'2', ':', 'x', 0x00, ',',
-	}
-	if got := v2Frame([]byte("x"), nil); !bytes.Equal(got, want) {
-		t.Fatalf("v2Frame(x,nil) =\n % x\nwant\n % x", got, want)
-	}
-	if got := v2Frame([]byte("x"), []byte{}); !bytes.Equal(got, want) {
-		t.Fatalf("nil and []byte{} payload must frame identically; got\n % x\nwant\n % x", got, want)
-	}
-}
-
-func TestV2FrameBinaryPayloadRidesUntouched(t *testing.T) {
-	// Payload bytes that look like framing (NUL, 0xFF, comma, colon) must ride INSIDE the
-	// netstring untouched — it is length-delimited, never re-parsed by delimiter.
 	payload := []byte{0x00, 0xFF, ',', ':'}
-	// content = "cat" + 0x00 + payload = 8 bytes; netstring "8:...," = 11 bytes; frame 16 bytes.
+	if got := v2Frame(argv, payload); !bytes.Equal(got, refFrame(refContent(argv, payload))) {
-	want := []byte{
+		t.Fatalf("binary payload frame mismatch")
-		0x00,
-		0x00, 0x00, 0x00, 0x0B, // be32(11)
-		'8', ':', 'c', 'a', 't', 0x00, 0x00, 0xFF, ',', ':', ',',
 	}
-	if got := v2Frame([]byte("cat"), payload); !bytes.Equal(got, want) {
+	if !bytes.Equal(v2Frame(argv, nil), v2Frame(argv, []byte{})) {
-		t.Fatalf("v2Frame binary payload =\n % x\nwant\n % x", got, want)
+		t.Fatal("nil and empty payload must frame identically")
+	}
+}
+
+func TestWrapHop(t *testing.T) {
+	inner := v2Frame([][]byte{[]byte("cat")}, nil)
+	// verb split on spaces, then "sicd" appended
+	want := v2Frame([][]byte{[]byte("incus"), []byte("exec"), []byte("c"), []byte("--"), []byte("sicd")}, inner)
+	if got := wrapHop("incus exec c --", inner); !bytes.Equal(got, want) {
+		t.Fatalf("wrapHop mismatch")
+	}
+}
+
+func TestBuildChain(t *testing.T) {
+	cmd := [][]byte{[]byte("cat")}
+	payload := []byte("hi")
+	if got, want := buildChain(nil, cmd, payload), v2Frame(cmd, payload); !bytes.Equal(got, want) {
+		t.Fatal("zero hops must equal a single v2 frame")
+	}
+	verbs := []string{"incus exec c --", "docker exec d"}
+	want := wrapHop(verbs[0], wrapHop(verbs[1], v2Frame(cmd, payload)))
+	if got := buildChain(verbs, cmd, payload); !bytes.Equal(got, want) {
+		t.Fatal("buildChain must fold verbs[0] as the outermost layer")
 	}
 }

cmd/sic/wraphop_test.go

@@ -1,87 +0,0 @@
-// Pins wrapHop, the one-hop chain wrapper the client does not have yet:
-//
-//	func wrapHop(verb string, inner []byte) []byte
-//
-// It wraps an already-built inner frame for ONE chain hop: the hop's command
-// is the runtime verb with " sicd" appended (the inner-hop peeler is sicd
-// itself), and the inner frame rides as the payload. Exactly:
-//
-//	wrapHop(verb, inner) == v2Frame([]byte(verb+" sicd"), inner)
-//
-// The equivalence tests pin that identity against v2Frame (frame.go, WP1);
-// the concrete tests are hand-written byte literals, so a shared framer bug
-// cannot cancel out of both sides.
-package main
-
-import (
-	"bytes"
-	"testing"
-)
-
-func TestWrapHopEqualsV2FrameOfVerbSicd(t *testing.T) {
-	// nestedInner is a real v2 frame (v2Frame("x", nil)): starts with the
-	// 0x00 magic, contains NULs — must ride untouched as payload.
-	nestedInner := []byte{0x00, 0x00, 0x00, 0x00, 0x05, '2', ':', 'x', 0x00, ','}
-	cases := []struct {
-		name  string
-		verb  string
-		inner []byte
-	}{
-		{"simple", "cat", []byte("hi")},
-		{"incus verb, nested frame inner", "incus exec memory --", nestedInner},
-		{"empty inner", "sh -c", nil},
-	}
-	for _, tc := range cases {
-		t.Run(tc.name, func(t *testing.T) {
-			got := wrapHop(tc.verb, tc.inner)
-			want := v2Frame([]byte(tc.verb+" sicd"), tc.inner)
-			if !bytes.Equal(got, want) {
-				t.Fatalf("wrapHop(%q, %q)\n got %q\nwant %q", tc.verb, tc.inner, got, want)
-			}
-		})
-	}
-}
-
-func TestWrapHopConcreteBytesNestedInnerUntouched(t *testing.T) {
-	// inner is itself a full v2 frame beginning with the 0x00 magic — a real
-	// nested frame. It must appear verbatim as the payload: no delimiter
-	// parsing, no escaping, no truncation at its leading NUL.
-	inner := []byte{0x00, 0x00, 0x00, 0x00, 0x05, '2', ':', 'x', 0x00, ','}
-	// command = "incus exec memory -- sicd" (25 bytes)
-	// content = command + 0x00 + inner = 25 + 1 + 10 = 36 bytes
-	// netstring = "36:" + content + "," = 40 bytes -> frame = 1 + 4 + 40 = 45
-	want := []byte{
-		0x00, // magic
-		0x00, 0x00, 0x00, 0x28, // be32 netstring length (40)
-		'3', '6', ':',
-		'i', 'n', 'c', 'u', 's', ' ', 'e', 'x', 'e', 'c', ' ',
-		'm', 'e', 'm', 'o', 'r', 'y', ' ', '-', '-', ' ', 's', 'i', 'c', 'd',
-		0x00,
-		0x00, 0x00, 0x00, 0x00, 0x05, '2', ':', 'x', 0x00, ',',
-		',',
-	}
-	got := wrapHop("incus exec memory --", inner)
-	if len(got) != 45 {
-		t.Fatalf("frame length = %d, want 45 (magic 1 + len32 4 + netstring 40): %q", len(got), got)
-	}
-	if !bytes.Equal(got, want) {
-		t.Fatalf("wrapHop(incus exec memory --, nested)\n got %q\nwant %q", got, want)
-	}
-}
-
-func TestWrapHopEmptyInner(t *testing.T) {
-	// command = "cat sicd" (8) -> content = command + 0x00 = 9 bytes
-	// -> netstring "9:<content>," = 12 bytes -> frame = 17 bytes
-	want := []byte{
-		0x00,
-		0x00, 0x00, 0x00, 0x0c,
-		'9', ':', 'c', 'a', 't', ' ', 's', 'i', 'c', 'd', 0x00, ',',
-	}
-	if got := wrapHop("cat", nil); !bytes.Equal(got, want) {
-		t.Fatalf("wrapHop(cat, nil)\n got %q\nwant %q", got, want)
-	}
-	// nil and empty inner frame identically.
-	if got := wrapHop("cat", []byte{}); !bytes.Equal(got, want) {
-		t.Fatalf("wrapHop(cat, []byte{})\n got %q\nwant %q", got, want)
-	}
-}