fix: stop guest state probes leaking cat errors into command output

Every code-vm invocation printed

  cat: /run/sandbox/squid-allow.d/10-host-config.conf: No such file or directory

before the agent's own output. Session setup probes for the allowlist
fragment on each run, AdminOutput streams stderr to the caller, and that
fragment is legitimately absent whenever extraDomains is empty — which is
the default, so the message appeared every time.

Fixing only the reported line would have left four more probe reads with
the same defect, and this is already the second instance of it: the Squid
reconfigure fix silenced the write path and left the reads alone. So
reads go through Client.ReadFile, which discards stderr inside the guest
and passes the path as an argument rather than interpolating it into the
command string. No raw cat probes remain. Absence now returns empty
content, which is what every caller already treated a read failure as.

The suite asserts an agent command emits nothing of its own and that
hello yields exactly hello, so noise from a future probe fails a
test rather than reaching the terminal.

fakeRunner matches stubs on a substring of the argv instead of its exact
shape: keying on the shape had made these very tests pass empty content
silently when the read changed form.

Author: stempler

internal/cli/allow.go

@@ -119,10 +119,7 @@ func mergeDomains(a, b []string) []string {
 // and open mode the generated config has no such ACL lines, and the result is
 // empty — the only cost is that a redundant entry may be offered.
 func guestAllowedDomains(ctx context.Context, cl lima.Client) []string {
-	out, err := cl.AdminOutput(ctx, []string{"cat", "/etc/squid/squid.conf"})
-	if err != nil {
-		return nil
-	}
+	out := cl.ReadFile(ctx, "/etc/squid/squid.conf")
 	var domains []string
 	for _, line := range splitLines(string(out)) {
 		if d := strings.TrimPrefix(line, squidACLPrefix); d != line {
@@ -285,11 +282,7 @@ func collectCandidates(ctx context.Context, cl lima.Client, args []string, runni
 		return nil, fmt.Errorf("reading denied domains needs a running VM; start it with `code-vm start`, " +
 			"or pass domains explicitly: code-vm allow example.com")
 	}
-	log, err := cl.AdminOutput(ctx, []string{"cat", "/var/log/squid/access.log"})
-	if err != nil {
-		return nil, fmt.Errorf("read the proxy log: %w", err)
-	}
-	candidates := parseDeniedDomains(string(log))
+	candidates := parseDeniedDomains(string(cl.ReadFile(ctx, "/var/log/squid/access.log")))
 	if len(candidates) == 0 {
 		fmt.Fprintln(out, "No denied requests in the proxy log.")
 	}

internal/cli/firewall.go

@@ -35,10 +35,7 @@ func setFirewallModeArgs(mode string) ([]string, error) {
 // file. Shared with `code-vm allow`, which reports whether a new domain took
 // effect live or only once allowlist mode returns.
 func currentFirewallMode(ctx context.Context, cl lima.Client) (string, error) {
-	verify, err := cl.AdminOutput(ctx, []string{"cat", "/run/firewall-verify"})
-	if err != nil {
-		return "", fmt.Errorf("read firewall state: %w", err)
-	}
+	verify := cl.ReadFile(ctx, "/run/firewall-verify")
 	for _, line := range splitLines(string(verify)) {
 		if strings.HasPrefix(line, "FIREWALL_MODE=") {
 			return strings.TrimPrefix(line, "FIREWALL_MODE="), nil

internal/cli/status.go

@@ -35,8 +35,8 @@ func newStatusCmd() *cobra.Command {
 				return nil
 			}
 			fmt.Fprintln(out, "firewall:")
-			verify, err := cl.AdminOutput(cmd.Context(), []string{"cat", "/run/firewall-verify"})
-			if err != nil {
+			verify := cl.ReadFile(cmd.Context(), "/run/firewall-verify")
+			if len(verify) == 0 {
 				fmt.Fprintln(out, "  unavailable")
 				return nil
 			}

internal/lima/limactl.go

@@ -108,6 +108,25 @@ func (c Client) AdminOutput(ctx context.Context, cmd []string) ([]byte, error) {
 	return c.R.Output(ctx, c.AdminArgs(cmd)...)
 }
 
+// ReadFile returns the contents of a guest file, and empty content when it does
+// not exist or cannot be read.
+//
+// Callers use this to probe guest state — is a fragment installed, what mode did
+// the firewall record — where absence is an ordinary answer. `cat` writes "No
+// such file or directory" to stderr, and AdminOutput streams stderr to the
+// caller's terminal, so probing with it directly printed that line into the
+// user's own command output on every invocation.
+//
+// The path travels as an argument rather than inside the command string, so it
+// cannot be interpreted as shell syntax.
+func (c Client) ReadFile(ctx context.Context, path string) []byte {
+	out, err := c.AdminOutput(ctx, []string{"sh", "-c", `cat -- "$0" 2>/dev/null`, path})
+	if err != nil {
+		return nil
+	}
+	return out
+}
+
 // Status returns the instance status, or "" when the instance does not exist.
 func (c Client) Status(ctx context.Context) (string, error) {
 	out, err := c.R.Output(ctx, "list", c.instance(), "--format", "{{.Status}}")

internal/lima/limactl_test.go

@@ -140,3 +140,27 @@ func TestClientDefaultsToTheStandardInstance(t *testing.T) {
 		t.Errorf("a zero Client must target %q, got %v", InstanceName, got)
 	}
 }
+
+// Probing guest state must not print cat's "No such file or directory" into the
+// user's terminal: AdminOutput streams stderr, so the redirect has to happen in
+// the guest. The path must travel as an argument, not inside the command string.
+func TestReadFileSuppressesStderrAndPassesPathSafely(t *testing.T) {
+	f := &fakeRunner{}
+	Client{R: f}.ReadFile(context.Background(), "/run/sandbox/squid-allow.d/10-host-config.conf")
+	if len(f.calls) != 1 {
+		t.Fatalf("expected one call, got %v", f.calls)
+	}
+	argv := f.calls[0]
+	joined := strings.Join(argv, " ")
+	if !strings.Contains(joined, "2>/dev/null") {
+		t.Errorf("read must discard stderr in the guest: %v", argv)
+	}
+	if argv[len(argv)-1] != "/run/sandbox/squid-allow.d/10-host-config.conf" {
+		t.Errorf("path must be the last argument, not interpolated: %v", argv)
+	}
+	for _, a := range argv[:len(argv)-1] {
+		if strings.Contains(a, "10-host-config.conf") {
+			t.Errorf("path must not appear inside the command string: %v", argv)
+		}
+	}
+}

internal/session/allowlist.go

@@ -62,8 +62,10 @@ func FragmentContent(domains []string) string {
 // workspace.
 func ApplyAllowlist(ctx context.Context, d Deps) error {
 	dst := fragmentDir + "/" + HostFragmentName
-	// A read failure means the fragment is absent, which counts as a change.
-	current, _ := d.Client.AdminOutput(ctx, []string{"cat", dst})
+	// Absent counts as a change. ReadFile keeps cat's "No such file" off the
+	// caller's terminal: this runs on every invocation, and the fragment is
+	// legitimately missing whenever no extra domains are configured.
+	current := d.Client.ReadFile(ctx, dst)
 
 	if len(d.Config.ExtraDomains) == 0 {
 		if len(current) == 0 {

internal/session/allowlist_test.go

@@ -23,7 +23,13 @@ func (f *fakeRunner) Run(_ context.Context, args ...string) error {
 
 func (f *fakeRunner) Output(_ context.Context, args ...string) ([]byte, error) {
 	f.calls = append(f.calls, args)
-	return f.out[strings.Join(args, " ")], nil
+	joined := strings.Join(args, " ")
+	for match, content := range f.out {
+		if strings.Contains(joined, match) {
+			return content, nil
+		}
+	}
+	return nil, nil
 }
 
 func (f *fakeRunner) ranAny(substr string) bool {
@@ -85,9 +91,7 @@ func TestApplyAllowlistInstallsFragmentAndReloadsSquid(t *testing.T) {
 
 func TestApplyAllowlistSkipsReloadWhenUnchanged(t *testing.T) {
 	existing := FragmentContent([]string{"registry.example.com"})
-	r := &fakeRunner{out: map[string][]byte{
-		strings.Join(lima.Client{}.AdminArgs([]string{"cat", fragmentPath()}), " "): []byte(existing),
-	}}
+	r := &fakeRunner{out: map[string][]byte{fragmentPath(): []byte(existing)}}
 	d := testDeps(t, r)
 	d.Config.ExtraDomains = []string{"registry.example.com"}
 	if err := ApplyAllowlist(context.Background(), d); err != nil {
@@ -113,8 +117,7 @@ func TestApplyAllowlistNoDomainsAndNoFragmentIsNoOp(t *testing.T) {
 // domain would stay allowed for the rest of the VM's lifetime.
 func TestApplyAllowlistRemovesFragmentWhenDomainsCleared(t *testing.T) {
 	r := &fakeRunner{out: map[string][]byte{
-		strings.Join(lima.Client{}.AdminArgs([]string{"cat", fragmentPath()}), " "): []byte(
-			FragmentContent([]string{"registry.example.com"})),
+		fragmentPath(): []byte(FragmentContent([]string{"registry.example.com"})),
 	}}
 	d := testDeps(t, r)
 	d.Config.ExtraDomains = nil

test-vm-sandbox.sh

@@ -181,6 +181,23 @@ else
 fi
 rmdir "$WORK_SUBDIR"
 
+# A command's output must be exactly its own. Session setup probes guest state
+# on every invocation, and those reads used to leak cat's "No such file or
+# directory" for the allowlist fragment — which is absent whenever no extra
+# domains are configured, i.e. by default.
+QUIET_OUT=$(agent true 2>&1)
+if [ -z "$QUIET_OUT" ]; then
+    pass "an agent command produces no output of its own"
+else
+    fail "an agent command leaked session-setup noise: [$QUIET_OUT]"
+fi
+
+if [ "$(agent echo hello 2>&1)" = "hello" ]; then
+    pass "an agent command's output is exactly its own"
+else
+    fail "an agent command's output is exactly its own (got: $(agent echo hello 2>&1))"
+fi
+
 if agent env | grep -q '^DOCKER_HOST=unix:///run/user/'; then
     pass "DOCKER_HOST is exported to the agent"
 else