[object Object]

← back to Cli Printing Press

fix(cli): reject control-plane flag injection in MCP shellout (#1022)

4b04f4c6be2591d3e34f5da2996dcadf3626bf78 · 2026-05-10 23:12:54 -0700 · Trevin Chow

* fix(cli): reject control-plane flag injection in MCP shellout

The generated MCP server's shellOutToCLI accepted any token starting
with `-` inside the free-form `args` string and any structured tool
parameter except literal `args`. Both vectors let an MCP client
override root CLI flags (`--token`, `--config`, `--base-url`,
`--deliver`, `--profile`), which sit on the control plane an agent
should not reach.

Two-layer guard in the cobratree shellout template:

* `shellOutToCLI` rejects tokens starting with `-` after splitting the
  `args` string and returns a typed MCP error pointing the caller at
  structured parameters.
* `cliArgsFromMCP` drops structured keys in a blockedRootFlags set
  (`args`, `base-url`, `config`, `deliver`, `profile`, `token`) so
  the same flags cannot ride in through the typed-parameter path.

Adds a generated `shellout_test.go` (new template + wired into both
`renderSingleFiles` and `GenerateMCPSurface`) covering shell-arg
splitting, blocklist enforcement, per-command-flag passthrough, and
flag-rejection for the args string. Updated `mustInclude`, file
counts, and the golden expected tree to match the new emission.

Closes #1016

* test(cli): tighten cliArgsFromMCP order assertion in shellout test

Self-review of #1022: sorting both sides before comparing makes
TestCliArgsFromMCP_AllowsPerCommandFlags pass even if cliArgsFromMCP
returned the right tokens in the wrong relative order. Since the
function already guarantees sorted-key output (so generated commands
see deterministic argv ordering), compare in place to catch a sort
regression too. Drops the now-unused sort import.

Files touched

Diff

commit 4b04f4c6be2591d3e34f5da2996dcadf3626bf78
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Sun May 10 23:12:54 2026 -0700

    fix(cli): reject control-plane flag injection in MCP shellout (#1022)
    
    * fix(cli): reject control-plane flag injection in MCP shellout
    
    The generated MCP server's shellOutToCLI accepted any token starting
    with `-` inside the free-form `args` string and any structured tool
    parameter except literal `args`. Both vectors let an MCP client
    override root CLI flags (`--token`, `--config`, `--base-url`,
    `--deliver`, `--profile`), which sit on the control plane an agent
    should not reach.
    
    Two-layer guard in the cobratree shellout template:
    
    * `shellOutToCLI` rejects tokens starting with `-` after splitting the
      `args` string and returns a typed MCP error pointing the caller at
      structured parameters.
    * `cliArgsFromMCP` drops structured keys in a blockedRootFlags set
      (`args`, `base-url`, `config`, `deliver`, `profile`, `token`) so
      the same flags cannot ride in through the typed-parameter path.
    
    Adds a generated `shellout_test.go` (new template + wired into both
    `renderSingleFiles` and `GenerateMCPSurface`) covering shell-arg
    splitting, blocklist enforcement, per-command-flag passthrough, and
    flag-rejection for the args string. Updated `mustInclude`, file
    counts, and the golden expected tree to match the new emission.
    
    Closes #1016
    
    * test(cli): tighten cliArgsFromMCP order assertion in shellout test
    
    Self-review of #1022: sorting both sides before comparing makes
    TestCliArgsFromMCP_AllowsPerCommandFlags pass even if cliArgsFromMCP
    returned the right tokens in the wrong relative order. Since the
    function already guarantees sorted-key output (so generated commands
    see deterministic argv ordering), compare in place to catch a sort
    regression too. Drops the now-unused sort import.
---
 internal/generator/generator.go                    |  14 ++-
 internal/generator/generator_test.go               |   7 +-
 .../generator/templates/cobratree/shellout.go.tmpl |  24 +++-
 .../templates/cobratree/shellout_test.go.tmpl      | 138 +++++++++++++++++++++
 .../golden/cases/generate-golden-api/artifacts.txt |   1 +
 .../internal/mcp/cobratree/shellout.go             |  24 +++-
 .../internal/mcp/cobratree/shellout_test.go        | 138 +++++++++++++++++++++
 7 files changed, 333 insertions(+), 13 deletions(-)

diff --git a/internal/generator/generator.go b/internal/generator/generator.go
index c5180e6f..8ffa6d30 100644
--- a/internal/generator/generator.go
+++ b/internal/generator/generator.go
@@ -1411,6 +1411,7 @@ func (g *Generator) renderSingleFiles() error {
 		"cobratree/classify.go.tmpl":         filepath.Join("internal", "mcp", "cobratree", "classify.go"),
 		"cobratree/typemap.go.tmpl":          filepath.Join("internal", "mcp", "cobratree", "typemap.go"),
 		"cobratree/shellout.go.tmpl":         filepath.Join("internal", "mcp", "cobratree", "shellout.go"),
+		"cobratree/shellout_test.go.tmpl":    filepath.Join("internal", "mcp", "cobratree", "shellout_test.go"),
 		"cobratree/cli_path.go.tmpl":         filepath.Join("internal", "mcp", "cobratree", "cli_path.go"),
 		"cobratree/names.go.tmpl":            filepath.Join("internal", "mcp", "cobratree", "names.go"),
 		"types.go.tmpl":                      filepath.Join("internal", "types", "types.go"),
@@ -1657,12 +1658,13 @@ func (g *Generator) GenerateMCPSurface() error {
 	}
 	g.PromotedCommands, g.PromotedResourceNames, g.PromotedEndpointNames = buildPromotedCommandPlan(g.Spec)
 	mcpFiles := map[string]string{
-		"cobratree/walker.go.tmpl":   filepath.Join("internal", "mcp", "cobratree", "walker.go"),
-		"cobratree/classify.go.tmpl": filepath.Join("internal", "mcp", "cobratree", "classify.go"),
-		"cobratree/typemap.go.tmpl":  filepath.Join("internal", "mcp", "cobratree", "typemap.go"),
-		"cobratree/shellout.go.tmpl": filepath.Join("internal", "mcp", "cobratree", "shellout.go"),
-		"cobratree/cli_path.go.tmpl": filepath.Join("internal", "mcp", "cobratree", "cli_path.go"),
-		"cobratree/names.go.tmpl":    filepath.Join("internal", "mcp", "cobratree", "names.go"),
+		"cobratree/walker.go.tmpl":        filepath.Join("internal", "mcp", "cobratree", "walker.go"),
+		"cobratree/classify.go.tmpl":      filepath.Join("internal", "mcp", "cobratree", "classify.go"),
+		"cobratree/typemap.go.tmpl":       filepath.Join("internal", "mcp", "cobratree", "typemap.go"),
+		"cobratree/shellout.go.tmpl":      filepath.Join("internal", "mcp", "cobratree", "shellout.go"),
+		"cobratree/shellout_test.go.tmpl": filepath.Join("internal", "mcp", "cobratree", "shellout_test.go"),
+		"cobratree/cli_path.go.tmpl":      filepath.Join("internal", "mcp", "cobratree", "cli_path.go"),
+		"cobratree/names.go.tmpl":         filepath.Join("internal", "mcp", "cobratree", "names.go"),
 		// cliutil files. Deliberately asymmetric with the marker-checked
 		// tools.go / handlers.go / root.go paths elsewhere in mcp-sync:
 		// those files can carry hand-edits and require explicit
diff --git a/internal/generator/generator_test.go b/internal/generator/generator_test.go
index e526bf96..cbb91ee1 100644
--- a/internal/generator/generator_test.go
+++ b/internal/generator/generator_test.go
@@ -65,6 +65,7 @@ func TestGenerateProjectsCompile(t *testing.T) {
 		"internal/mcp/cobratree/classify.go",
 		"internal/mcp/cobratree/typemap.go",
 		"internal/mcp/cobratree/shellout.go",
+		"internal/mcp/cobratree/shellout_test.go",
 		"internal/mcp/cobratree/cli_path.go",
 		"internal/mcp/cobratree/names.go",
 	}
@@ -78,9 +79,9 @@ func TestGenerateProjectsCompile(t *testing.T) {
 		// Bump it AND add to mustInclude above when adding always-emitted
 		// templates. Per-spec dynamic files (per-resource command files,
 		// generated tests) account for the difference between fixtures.
-		{name: "stytch", specPath: filepath.Join("..", "..", "testdata", "stytch.yaml"), expectedFiles: 57},
-		{name: "clerk", specPath: filepath.Join("..", "..", "testdata", "clerk.yaml"), expectedFiles: 62},
-		{name: "loops", specPath: filepath.Join("..", "..", "testdata", "loops.yaml"), expectedFiles: 59},
+		{name: "stytch", specPath: filepath.Join("..", "..", "testdata", "stytch.yaml"), expectedFiles: 58},
+		{name: "clerk", specPath: filepath.Join("..", "..", "testdata", "clerk.yaml"), expectedFiles: 63},
+		{name: "loops", specPath: filepath.Join("..", "..", "testdata", "loops.yaml"), expectedFiles: 60},
 	}
 
 	for _, tt := range tests {
diff --git a/internal/generator/templates/cobratree/shellout.go.tmpl b/internal/generator/templates/cobratree/shellout.go.tmpl
index 34cd612b..ecd597d1 100644
--- a/internal/generator/templates/cobratree/shellout.go.tmpl
+++ b/internal/generator/templates/cobratree/shellout.go.tmpl
@@ -26,7 +26,13 @@ func shellOutToCLI(cliPath func() (string, error), commandPath []string) server.
 		finalArgs := append([]string{}, prefixArgs...)
 		finalArgs = append(finalArgs, cliArgsFromMCP(args)...)
 		if raw, _ := args["args"].(string); strings.TrimSpace(raw) != "" {
-			finalArgs = append(finalArgs, splitShellArgs(raw)...)
+			tokens := splitShellArgs(raw)
+			for _, t := range tokens {
+				if strings.HasPrefix(t, "-") {
+					return mcplib.NewToolResultError(fmt.Sprintf("flag-like argument %q not allowed in positional args field; use structured tool parameters instead", t)), nil
+				}
+			}
+			finalArgs = append(finalArgs, tokens...)
 		}
 		cmd := exec.CommandContext(ctx, lookupPath, finalArgs...)
 		out, err := cmd.CombinedOutput()
@@ -37,10 +43,24 @@ func shellOutToCLI(cliPath func() (string, error), commandPath []string) server.
 	}
 }
 
+// blockedRootFlags are root-level CLI flags that an MCP client must not be
+// able to override via structured tool parameters. Allowing them lets a
+// caller swap auth credentials, redirect the API base URL, load a malicious
+// config file, or change the delivery target — all of which sit outside the
+// per-command surface the agent is supposed to be calling.
+var blockedRootFlags = map[string]bool{
+	"args":     true,
+	"base-url": true,
+	"config":   true,
+	"deliver":  true,
+	"profile":  true,
+	"token":    true,
+}
+
 func cliArgsFromMCP(args map[string]any) []string {
 	keys := make([]string, 0, len(args))
 	for k := range args {
-		if k != "args" {
+		if !blockedRootFlags[k] {
 			keys = append(keys, k)
 		}
 	}
diff --git a/internal/generator/templates/cobratree/shellout_test.go.tmpl b/internal/generator/templates/cobratree/shellout_test.go.tmpl
new file mode 100644
index 00000000..fb997576
--- /dev/null
+++ b/internal/generator/templates/cobratree/shellout_test.go.tmpl
@@ -0,0 +1,138 @@
+// Copyright {{currentYear}} {{.Owner}}. Licensed under Apache-2.0. See LICENSE.
+// Generated by CLI Printing Press (https://github.com/mvanhorn/cli-printing-press). DO NOT EDIT.
+
+package cobratree
+
+import (
+	"reflect"
+	"strings"
+	"testing"
+)
+
+// TestSplitShellArgs pins the whitespace + double-quote splitting used by
+// the args-field passthrough. Behavior we rely on: bare whitespace splits,
+// tabs split, double-quoted spans stay together, empty input yields nil.
+func TestSplitShellArgs(t *testing.T) {
+	cases := []struct {
+		name string
+		in   string
+		want []string
+	}{
+		{"empty", "", nil},
+		{"single token", "contacts", []string{"contacts"}},
+		{"two tokens", "inbox health", []string{"inbox", "health"}},
+		{"extra whitespace", "  foo   bar  ", []string{"foo", "bar"}},
+		{"tabs", "foo\tbar", []string{"foo", "bar"}},
+		{"quoted token", `"hello world"`, []string{"hello world"}},
+		{"mixed quoted and bare", `contacts "john doe" active`, []string{"contacts", "john doe", "active"}},
+	}
+	for _, tc := range cases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			got := splitShellArgs(tc.in)
+			if !reflect.DeepEqual(got, tc.want) {
+				t.Errorf("splitShellArgs(%q) = %v, want %v", tc.in, got, tc.want)
+			}
+		})
+	}
+}
+
+// TestCliArgsFromMCP_BlocksRootFlags pins the structured-parameter half of
+// the control-plane-injection guard: even when an MCP client wraps the
+// flag in the structured args map (instead of the free-form "args"
+// string), the root flags listed in blockedRootFlags must be dropped
+// before they reach exec.CommandContext. A regression here would let a
+// caller redirect --base-url, swap --token, or load a malicious --config.
+func TestCliArgsFromMCP_BlocksRootFlags(t *testing.T) {
+	in := map[string]any{
+		"args":     "contacts",
+		"base-url": "https://evil.example.com",
+		"config":   "/tmp/evil.yaml",
+		"deliver":  "fd:3",
+		"profile":  "attacker",
+		"token":    "stolen-token",
+		// Allowed per-command flag passes through.
+		"limit": float64(10),
+	}
+	got := cliArgsFromMCP(in)
+	want := []string{"--limit", "10"}
+	if !reflect.DeepEqual(got, want) {
+		t.Fatalf("cliArgsFromMCP dropped/kept wrong keys: got %v, want %v", got, want)
+	}
+	for _, blocked := range []string{"--base-url", "--config", "--deliver", "--profile", "--token", "--args"} {
+		for _, tok := range got {
+			if tok == blocked {
+				t.Errorf("blocked flag %q leaked through cliArgsFromMCP", blocked)
+			}
+		}
+	}
+}
+
+// TestCliArgsFromMCP_AllowsPerCommandFlags is the don't-overcorrect half:
+// any flag NOT in blockedRootFlags must still pass through, including
+// strings, bools, numbers, and []any. Without this, a tightening change
+// to the blocklist that accidentally drops legitimate per-command flags
+// would silently break every MCP-driven command. cliArgsFromMCP also
+// guarantees sorted-key output (so generated commands see deterministic
+// argv ordering); compare directly to catch a regression in either the
+// blocklist or the sort.
+func TestCliArgsFromMCP_AllowsPerCommandFlags(t *testing.T) {
+	in := map[string]any{
+		"query":   "alpha",
+		"verbose": true,
+		"limit":   float64(25),
+		"tags":    []any{"a", "b"},
+	}
+	got := cliArgsFromMCP(in)
+	want := []string{"--limit", "25", "--query", "alpha", "--tags", "a,b", "--verbose"}
+	if !reflect.DeepEqual(got, want) {
+		t.Fatalf("cliArgsFromMCP per-command passthrough: got %v, want %v", got, want)
+	}
+}
+
+// TestArgsFieldRejectsFlagLikeTokens covers the free-form "args" string
+// half of the control-plane-injection guard. shellOutToCLI is a closure
+// that requires a real binary on PATH; we exercise the same guard logic
+// here so a regression that drops the strings.HasPrefix("-") check is
+// caught at unit-test scope rather than only via end-to-end MCP runs.
+func TestArgsFieldRejectsFlagLikeTokens(t *testing.T) {
+	guard := func(raw string) (rejected string, ok bool) {
+		for _, t := range splitShellArgs(raw) {
+			if strings.HasPrefix(t, "-") {
+				return t, false
+			}
+		}
+		return "", true
+	}
+	cases := []struct {
+		name        string
+		in          string
+		wantOK      bool
+		wantBlocked string
+	}{
+		{"long flag", "--config /tmp/evil.yaml", false, "--config"},
+		{"short flag", "-c", false, "-c"},
+		{"bare double dash", "--", false, "--"},
+		{"flag with equals", "--config=/tmp/evil.yaml", false, "--config=/tmp/evil.yaml"},
+		{"flag mid-args", "contacts --verbose", false, "--verbose"},
+		{"clean positional", "contacts", true, ""},
+		{"two positionals", "inbox health", true, ""},
+		{"empty", "", true, ""},
+		// Shell metachars in unquoted positional content stay verbatim;
+		// the CLI is invoked via exec.CommandContext (no /bin/sh), so
+		// $VAR / && / | are inert. Pin that they don't trip the guard.
+		{"shell metachars in positional", `name with $VAR && pipe|stuff`, true, ""},
+	}
+	for _, tc := range cases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			tok, ok := guard(tc.in)
+			if ok != tc.wantOK {
+				t.Errorf("guard(%q) ok = %v, want %v", tc.in, ok, tc.wantOK)
+			}
+			if !tc.wantOK && tok != tc.wantBlocked {
+				t.Errorf("guard(%q) blocked = %q, want %q", tc.in, tok, tc.wantBlocked)
+			}
+		})
+	}
+}
diff --git a/testdata/golden/cases/generate-golden-api/artifacts.txt b/testdata/golden/cases/generate-golden-api/artifacts.txt
index 83dc1f18..678df8d8 100644
--- a/testdata/golden/cases/generate-golden-api/artifacts.txt
+++ b/testdata/golden/cases/generate-golden-api/artifacts.txt
@@ -19,6 +19,7 @@ printing-press-golden/internal/mcp/cobratree/walker.go
 printing-press-golden/internal/mcp/cobratree/classify.go
 printing-press-golden/internal/mcp/cobratree/typemap.go
 printing-press-golden/internal/mcp/cobratree/shellout.go
+printing-press-golden/internal/mcp/cobratree/shellout_test.go
 printing-press-golden/internal/mcp/cobratree/cli_path.go
 printing-press-golden/internal/mcp/cobratree/names.go
 printing-press-golden/cmd/printing-press-golden-pp-mcp/main.go
diff --git a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/mcp/cobratree/shellout.go b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/mcp/cobratree/shellout.go
index a87475fa..189131cc 100644
--- a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/mcp/cobratree/shellout.go
+++ b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/mcp/cobratree/shellout.go
@@ -26,7 +26,13 @@ func shellOutToCLI(cliPath func() (string, error), commandPath []string) server.
 		finalArgs := append([]string{}, prefixArgs...)
 		finalArgs = append(finalArgs, cliArgsFromMCP(args)...)
 		if raw, _ := args["args"].(string); strings.TrimSpace(raw) != "" {
-			finalArgs = append(finalArgs, splitShellArgs(raw)...)
+			tokens := splitShellArgs(raw)
+			for _, t := range tokens {
+				if strings.HasPrefix(t, "-") {
+					return mcplib.NewToolResultError(fmt.Sprintf("flag-like argument %q not allowed in positional args field; use structured tool parameters instead", t)), nil
+				}
+			}
+			finalArgs = append(finalArgs, tokens...)
 		}
 		cmd := exec.CommandContext(ctx, lookupPath, finalArgs...)
 		out, err := cmd.CombinedOutput()
@@ -37,10 +43,24 @@ func shellOutToCLI(cliPath func() (string, error), commandPath []string) server.
 	}
 }
 
+// blockedRootFlags are root-level CLI flags that an MCP client must not be
+// able to override via structured tool parameters. Allowing them lets a
+// caller swap auth credentials, redirect the API base URL, load a malicious
+// config file, or change the delivery target — all of which sit outside the
+// per-command surface the agent is supposed to be calling.
+var blockedRootFlags = map[string]bool{
+	"args":     true,
+	"base-url": true,
+	"config":   true,
+	"deliver":  true,
+	"profile":  true,
+	"token":    true,
+}
+
 func cliArgsFromMCP(args map[string]any) []string {
 	keys := make([]string, 0, len(args))
 	for k := range args {
-		if k != "args" {
+		if !blockedRootFlags[k] {
 			keys = append(keys, k)
 		}
 	}
diff --git a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/mcp/cobratree/shellout_test.go b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/mcp/cobratree/shellout_test.go
new file mode 100644
index 00000000..4a747b27
--- /dev/null
+++ b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/mcp/cobratree/shellout_test.go
@@ -0,0 +1,138 @@
+// Copyright 2026 printing-press-golden. Licensed under Apache-2.0. See LICENSE.
+// Generated by CLI Printing Press (https://github.com/mvanhorn/cli-printing-press). DO NOT EDIT.
+
+package cobratree
+
+import (
+	"reflect"
+	"strings"
+	"testing"
+)
+
+// TestSplitShellArgs pins the whitespace + double-quote splitting used by
+// the args-field passthrough. Behavior we rely on: bare whitespace splits,
+// tabs split, double-quoted spans stay together, empty input yields nil.
+func TestSplitShellArgs(t *testing.T) {
+	cases := []struct {
+		name string
+		in   string
+		want []string
+	}{
+		{"empty", "", nil},
+		{"single token", "contacts", []string{"contacts"}},
+		{"two tokens", "inbox health", []string{"inbox", "health"}},
+		{"extra whitespace", "  foo   bar  ", []string{"foo", "bar"}},
+		{"tabs", "foo\tbar", []string{"foo", "bar"}},
+		{"quoted token", `"hello world"`, []string{"hello world"}},
+		{"mixed quoted and bare", `contacts "john doe" active`, []string{"contacts", "john doe", "active"}},
+	}
+	for _, tc := range cases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			got := splitShellArgs(tc.in)
+			if !reflect.DeepEqual(got, tc.want) {
+				t.Errorf("splitShellArgs(%q) = %v, want %v", tc.in, got, tc.want)
+			}
+		})
+	}
+}
+
+// TestCliArgsFromMCP_BlocksRootFlags pins the structured-parameter half of
+// the control-plane-injection guard: even when an MCP client wraps the
+// flag in the structured args map (instead of the free-form "args"
+// string), the root flags listed in blockedRootFlags must be dropped
+// before they reach exec.CommandContext. A regression here would let a
+// caller redirect --base-url, swap --token, or load a malicious --config.
+func TestCliArgsFromMCP_BlocksRootFlags(t *testing.T) {
+	in := map[string]any{
+		"args":     "contacts",
+		"base-url": "https://evil.example.com",
+		"config":   "/tmp/evil.yaml",
+		"deliver":  "fd:3",
+		"profile":  "attacker",
+		"token":    "stolen-token",
+		// Allowed per-command flag passes through.
+		"limit": float64(10),
+	}
+	got := cliArgsFromMCP(in)
+	want := []string{"--limit", "10"}
+	if !reflect.DeepEqual(got, want) {
+		t.Fatalf("cliArgsFromMCP dropped/kept wrong keys: got %v, want %v", got, want)
+	}
+	for _, blocked := range []string{"--base-url", "--config", "--deliver", "--profile", "--token", "--args"} {
+		for _, tok := range got {
+			if tok == blocked {
+				t.Errorf("blocked flag %q leaked through cliArgsFromMCP", blocked)
+			}
+		}
+	}
+}
+
+// TestCliArgsFromMCP_AllowsPerCommandFlags is the don't-overcorrect half:
+// any flag NOT in blockedRootFlags must still pass through, including
+// strings, bools, numbers, and []any. Without this, a tightening change
+// to the blocklist that accidentally drops legitimate per-command flags
+// would silently break every MCP-driven command. cliArgsFromMCP also
+// guarantees sorted-key output (so generated commands see deterministic
+// argv ordering); compare directly to catch a regression in either the
+// blocklist or the sort.
+func TestCliArgsFromMCP_AllowsPerCommandFlags(t *testing.T) {
+	in := map[string]any{
+		"query":   "alpha",
+		"verbose": true,
+		"limit":   float64(25),
+		"tags":    []any{"a", "b"},
+	}
+	got := cliArgsFromMCP(in)
+	want := []string{"--limit", "25", "--query", "alpha", "--tags", "a,b", "--verbose"}
+	if !reflect.DeepEqual(got, want) {
+		t.Fatalf("cliArgsFromMCP per-command passthrough: got %v, want %v", got, want)
+	}
+}
+
+// TestArgsFieldRejectsFlagLikeTokens covers the free-form "args" string
+// half of the control-plane-injection guard. shellOutToCLI is a closure
+// that requires a real binary on PATH; we exercise the same guard logic
+// here so a regression that drops the strings.HasPrefix("-") check is
+// caught at unit-test scope rather than only via end-to-end MCP runs.
+func TestArgsFieldRejectsFlagLikeTokens(t *testing.T) {
+	guard := func(raw string) (rejected string, ok bool) {
+		for _, t := range splitShellArgs(raw) {
+			if strings.HasPrefix(t, "-") {
+				return t, false
+			}
+		}
+		return "", true
+	}
+	cases := []struct {
+		name        string
+		in          string
+		wantOK      bool
+		wantBlocked string
+	}{
+		{"long flag", "--config /tmp/evil.yaml", false, "--config"},
+		{"short flag", "-c", false, "-c"},
+		{"bare double dash", "--", false, "--"},
+		{"flag with equals", "--config=/tmp/evil.yaml", false, "--config=/tmp/evil.yaml"},
+		{"flag mid-args", "contacts --verbose", false, "--verbose"},
+		{"clean positional", "contacts", true, ""},
+		{"two positionals", "inbox health", true, ""},
+		{"empty", "", true, ""},
+		// Shell metachars in unquoted positional content stay verbatim;
+		// the CLI is invoked via exec.CommandContext (no /bin/sh), so
+		// $VAR / && / | are inert. Pin that they don't trip the guard.
+		{"shell metachars in positional", `name with $VAR && pipe|stuff`, true, ""},
+	}
+	for _, tc := range cases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			tok, ok := guard(tc.in)
+			if ok != tc.wantOK {
+				t.Errorf("guard(%q) ok = %v, want %v", tc.in, ok, tc.wantOK)
+			}
+			if !tc.wantOK && tok != tc.wantBlocked {
+				t.Errorf("guard(%q) blocked = %q, want %q", tc.in, tok, tc.wantBlocked)
+			}
+		})
+	}
+}

← 54f007f5 fix(skills): gate polish Publish Offer on --standalone, not  ·  back to Cli Printing Press  ·  feat(cli): mechanical PII gate before promote/publish (#958) a3e07d0a →