fix(mcp): judge run_command on the command and flags cobra resolves

.claude/skills/where-to-make-changes/SKILL.md

@@ -146,5 +146,5 @@ description: Use when you know what to change but not where — a lookup table m
 | Change the dashboard's exit code or banner | `finishDashboard` (`internal/commands/dashboard.go`) — the seam both are tested through |
 | Change what each dashboard tier costs, or where a collector lives | `collectProDataFast` / `collectProDataFull` (`internal/commands/dashboard_pro.go`), and `fixedCostAuditChecks` / `fleetScaledAuditChecks` (`internal/commands/pro_audit.go`). State the cost through `dashboardCostNote` (`dashboard.go`) — it is the single source every surface quotes |
 | Change which objects a category's item count covers | `classicCategorySources` / `proCategorySources` (`internal/commands/dashboard_pro_categories.go`) |
-| Change what an MCP child may not do | `blockedChildFlagPrefixes` / `blockedChildCommandPaths` / `refuseReportThroughRunCommand` (`internal/commands/mcp.go`) — prefix-matched, and the blocked set is swept from the assembled tree by a test |
+| Change what an MCP child may not do | `mcpRefusedCommands` / `mcpLocalPathFlags` / `mcpDirFlags` / `blockedChildFlagPrefixes` / `refuseReportThroughRunCommand` (`internal/commands/mcp.go`) — judged on the command and flags cobra resolves, server-side in `buildChildArgs` and again in the child by `refuseInMCPChild`; tree-walk tests fail on an unclassified path-shaped flag or a stale entry |
 | Change what the MCP report tool returns | `runReportChild` (`internal/commands/mcp.go`) — exit 7 keeps the file, anything else removes it |

CHANGELOG.md

@@ -11,6 +11,96 @@ commit types the repo already uses (`feat!`/`build!` for a breaking change).
 
 ## Unreleased
 
+### Breaking — MCP `run_command` refuses local paths and credential output unless the operator allows them
+
+`run_command` used to compare the model's raw argument list against a short
+deny-list. A command alias (`cfg`), a leading flag (`--no-color multi`) or a
+flag inside the path (`pro -q backup`) got past it. Flags that name a local
+file were not on the list, so the model could read, write or delete files on
+the machine running `mcp serve`, and `pro diff --target <profile>` used
+another profile's credentials.
+
+The server now judges the command and the flags that cobra resolves, and the
+child process checks again before it runs. These now fail over MCP:
+
+- A flag whose value is a local file to read (`--from-file`, `--file`,
+  `--script-file`, `--input` and the like), unless the path is inside the
+  directory the operator passes to `mcp serve --input-dir <dir>`.
+  `--password-file` is always refused.
+- A flag whose value is a local path to write (`--save-to`, a command's own
+  `--output`, `--report-dir`, `--dir` on `sync`). The `-o/--output` format
+  flag is not affected.
+- `pro diff` with a side that is neither the server's profile nor a directory
+  inside `--input-dir`.
+- Body logging: `-vvv`, or any `--verbose` level of 3 or more however it is
+  spelled. It logs each response body to stderr before any redaction, and
+  `run_command` returns stderr. Use `-vv` or less. In every mode, not only over MCP,
+  the `-vv` header log now shows `Cookie` and `Set-Cookie` values as
+  `[redacted]`, as it already did for `Authorization`.
+- `multi`, `mcp`, `completion`, the `config` write subcommands,
+  `config validate`, `doctor`, every `setup`, both `backup` commands and
+  `jcds sync`.
+- The commands that print an access token: `auth token` under `platform`,
+  `pro` and `protect`, and `pro api-authentication token`, `oauth-token` and
+  `keep-alive`.
+- `pro sso-oauth-session-tokens`, which prints the session's access and ID
+  tokens.
+- The commands that mint a credential and print it:
+  `pro api-integrations client-credentials` (a new client secret) and
+  `protect api-clients apply` (a new API client's password).
+- `pro cloud-distribution-point create` and `patch`, whose response
+  carries the CloudFront private key that signs download URLs.
+- The commands that set a Jamf Pro login password to a value the model
+  chose: `pro jamf-pro-user-account-settings change-password` and
+  `pro accounts create`, `update` and `apply`.
+- `protect downloads csr` and `websocket-auth`, which write the tenant's
+  `.p12` key material into the server's working directory.
+- `protect action-configs export`, whose document carries each report
+  client's header values, such as a SIEM or webhook bearer token. A redacted
+  copy would overwrite the real credential when applied.
+
+`config show` still runs over MCP, with each token, client ID and client
+secret shown as `<redacted>`. `config list --status` checks only the server's
+profile. These Protect commands also run over MCP with the credential shown as
+`<redacted>`: `action-configs get` and `apply` (each report client's header
+values, and the userinfo and query of each report-client URL),
+`data-forwarding get` and `update` (the Sentinel shared key) and
+`api-clients get` (the password). `pro cloud-distribution-point list` runs
+over MCP with the CloudFront private key and the CDN password shown as
+`<redacted>` in every output format. Every Classic `get` and `list` prints each
+field that Classic `--set` refuses as a credential as `<redacted>`, in every
+output format, so `-o raw` is not the wire bytes over MCP, and `pro diff`
+shows those fields' old and new values as `<redacted>` while still reporting
+the change. `get` and `list` on `classic-macos-config-profiles` and
+`classic-mobile-config-profiles`, `pro diff` on `profiles` and `pro blueprints
+components configuration-profile --id/--name` also redact profile payloads by
+key name: the value of a key named `Challenge`, or ending in any case in
+`password`, `secret`, `token`, `authkey`, `apikey`, `accesskey`, `privatekey`,
+`secretkey`, `passcode` or `credential`, prints as `<redacted>`, and so does a
+PKCS#12 certificate. Every other payload value is shown, a custom payload's
+included. The payload is re-encoded, so the record is still a profile
+document. A payload that does not decode is redacted whole, and the blueprint
+converter refuses it. Outside MCP their output is unchanged. Secrets of the pinned tenant's devices (the LAPS
+password, the recovery lock password, the FileVault personal recovery key),
+the JCDS upload credentials of `pro jamf-cloud-distribution-service
+renew-credentials` and `pro jamf-cloud-distribution-service-files create`,
+and blueprint configuration, a secret a component carries included, are
+still shown.
+
+`mcp serve --input-dir ""` (for example `--input-dir "$DIR"` with `DIR`
+unset) is now an error instead of starting with no input directory.
+
+**Migration:** if an agent sends file bodies through `run_command` (for
+example `pro scripts create --script-file …`), start the server with
+`mcp serve --input-dir <dir>` and keep those files in that directory.
+
+### Behaviour — `protect plans config-profile` refuses a plan name that is not a file name
+
+Without `-O`, the command saves `<plan name>.mobileconfig` in the working
+directory. It now refuses a plan name that contains `/` or `\`, or is empty,
+`.` or `..`, because that name would put the file somewhere else. Pass
+`-O <path>` for such a plan. Every other name is saved as before.
+
 ### Breaking — `--set` refuses credential fields in Pro, Platform and Security Cloud
 
 A `--set` value is on the command line, so it lands in shell history, in `ps`

generator/classic/generator.go

@@ -617,6 +617,14 @@ func new{{ .GoName }}ListCmd(ctx *registry.CLIContext) *cobra.Command {
 			if err != nil {
 				return err
 			}
+			if body, err = redactClassicReadInMCPChild(body, {{ bodySpecVar . }}); err != nil {
+				return err
+			}
+{{- if .IsConfigProfile }}
+			if body, err = redactClassicProfilePayloadsInMCPChild(body); err != nil {
+				return err
+			}
+{{- end }}
 {{- if .ListSubset }}
 			// /JSSResource/{{ .Path }} returns users + groups combined; narrow to
 			// the "{{ .ListSubset }}" subset so this command behaves like a
@@ -638,7 +646,7 @@ func new{{ .GoName }}ListCmd(ctx *registry.CLIContext) *cobra.Command {
 			return ctx.Output.PrintRaw(subsetXML)
 {{- else }}
 			// Default to pretty-printed XML; use -o json/yaml/table/csv for structured output.
-			// -o xml = pretty-printed XML, -o raw = exact wire bytes.
+			// -o xml = pretty-printed XML, -o raw = the wire bytes outside an MCP child.
 			if (!cmd.Flags().Changed("output") && !cmd.Flags().Changed("field") && ctx.Output.Format() == "json") || ctx.Output.Format() == "xml" || ctx.Output.Format() == "raw" {
 				return ctx.Output.PrintBytes(body)
 			}
@@ -725,8 +733,16 @@ func new{{ .GoName }}GetCmd(ctx *registry.CLIContext) *cobra.Command {
 			if err != nil {
 				return err
 			}
+			if body, err = redactClassicReadInMCPChild(body, {{ bodySpecVar . }}); err != nil {
+				return err
+			}
+{{- if .IsConfigProfile }}
+			if body, err = redactClassicProfilePayloadsInMCPChild(body); err != nil {
+				return err
+			}
+{{- end }}
 			// Default to pretty-printed XML; use -o json/yaml/table/csv for structured output.
-			// -o xml = pretty-printed XML, -o raw = exact wire bytes.
+			// -o xml = pretty-printed XML, -o raw = the wire bytes outside an MCP child.
 			if (!cmd.Flags().Changed("output") && !cmd.Flags().Changed("field") && ctx.Output.Format() == "json") || ctx.Output.Format() == "xml" || ctx.Output.Format() == "raw" {
 				return ctx.Output.PrintBytes(body)
 			}
@@ -1596,22 +1612,20 @@ const classicRegistryTemplate = `// Copyright 2026, Jamf Software LLC
 package generated
 
 import (
+	"bytes"
+	"encoding/xml"
 	"io"
 	"os"
 {{- if or (anyNeedsClassicNameResolve .) (anyClassicFileFields .) (anyHasGroupPath .) }}
 	"context"
 	"encoding/json"
-	"encoding/xml"
 	"path/filepath"
 {{- end }}
 	"slices"
 	"sort"
 	"strings"
 	"strconv"
 	"fmt"
-{{- if or (anyIsConfigProfile .) (anyClassicFileFields .) (anyListSubset .) (anyClassicExtraLookups .) (anyHasGroupPath .) }}
-	"bytes"
-{{- end }}
 {{- if or (anyIsConfigProfile .) (anyClassicFileFields .) (anyClassicExtraLookups .) }}
 	"net/url"
 {{- end }}
@@ -1625,9 +1639,7 @@ import (
 {{- if anyClassicExtraLookups . }}
 	"github.com/Jamf-Concepts/jamf-cli/internal/exitcode"
 {{- end }}
-{{- if or (anyNeedsClassicNameResolve .) (anyClassicFileFields .) (anyListSubset .) }}
 	"github.com/Jamf-Concepts/jamf-cli/internal/xmlconv"
-{{- end }}
 {{- if or (anyIsConfigProfile .) (anyClassicFileFields .) }}
 	"github.com/Jamf-Concepts/jamf-cli/internal/profileconvert"
 {{- end }}
@@ -1643,6 +1655,33 @@ func RegisterClassicCommands(root *cobra.Command, ctx *registry.CLIContext) {
 {{- end }}
 }
 
+// classicBodySpecsByCommand maps each Classic command group to its body spec.
+var classicBodySpecsByCommand = map[string]classicBodySpec{
+{{- range . }}
+	"{{ .CLIName }}": {{ bodySpecVar . }},
+{{- end }}
+}
+
+// ClassicCredentialLeaves returns the element names that carry a credential in
+// the Classic resource whose command group is cliName, or nil for none.
+func ClassicCredentialLeaves(cliName string) map[string]bool {
+	return classicCredentialLeaves(classicBodySpecsByCommand[cliName])
+}
+
+// classicCredentialLeaves is the last segment of each credential path in spec.
+// Within one resource no such name is also worn by a field that is not a
+// credential, so an element is matched by name alone at any depth.
+func classicCredentialLeaves(spec classicBodySpec) map[string]bool {
+	if len(spec.Credentials) == 0 {
+		return nil
+	}
+	leaves := make(map[string]bool, len(spec.Credentials))
+	for path := range spec.Credentials {
+		leaves[strings.TrimSuffix(path[strings.LastIndex(path, ".")+1:], "[]")] = true
+	}
+	return leaves
+}
+
 // readClassicBody reads an XML request body from --from-file, or from stdin when
 // the flag is absent. Unlike readApplyInput it tolerates an absent body and
 // returns nil, leaving the caller to decide whether that is an error — classic
@@ -1668,6 +1707,102 @@ func readClassicBody(fromFile string) ([]byte, error) {
 	return nil, nil
 }
 
+// classicRedactedText is "<redacted>" escaped as XML element text, so every
+// output format decodes it back to the marker.
+const classicRedactedText = "&lt;redacted&gt;"
+
+// redactClassicReadInMCPChild returns body with the text of every element
+// named after one of spec's credential fields replaced by the redaction
+// marker, when this process is a child of ` + "`mcp serve`" + `. Every get and list
+// prints through it, before choosing a format, so -o raw is not the wire
+// bytes there. A body it cannot parse as XML is refused rather than printed.
+func redactClassicReadInMCPChild(body []byte, spec classicBodySpec) ([]byte, error) {
+	leaves := classicCredentialLeaves(spec)
+	if len(leaves) == 0 || !registry.InMCPChild() || len(bytes.TrimSpace(body)) == 0 {
+		return body, nil
+	}
+	if !xmlconv.IsXML(body) {
+		return nil, fmt.Errorf("the Classic API answered with a body that is not XML, so its credential fields cannot be redacted and it is not printed over MCP")
+	}
+	type span struct{ start, end int64 }
+	var spans []span
+	var names []string
+	var starts []int64
+	dec := xml.NewDecoder(bytes.NewReader(body))
+	for {
+		before := dec.InputOffset()
+		tok, err := dec.RawToken()
+		if err == io.EOF {
+			break
+		}
+		if err != nil {
+			return nil, fmt.Errorf("parsing the Classic API response to redact its credential fields, so it is not printed over MCP: %w", err)
+		}
+		switch t := tok.(type) {
+		case xml.StartElement:
+			names = append(names, t.Name.Local)
+			starts = append(starts, dec.InputOffset())
+		case xml.EndElement:
+			n := len(names)
+			if n == 0 {
+				continue
+			}
+			if leaves[names[n-1]] && before > starts[n-1] {
+				spans = append(spans, span{starts[n-1], before})
+			}
+			names, starts = names[:n-1], starts[:n-1]
+		}
+	}
+	if len(spans) == 0 {
+		return body, nil
+	}
+	sort.Slice(spans, func(i, j int) bool { return spans[i].start < spans[j].start })
+	var out bytes.Buffer
+	var cursor int64
+	for _, sp := range spans {
+		if sp.start < cursor {
+			continue
+		}
+		out.Write(body[cursor:sp.start])
+		out.WriteString(classicRedactedText)
+		cursor = sp.end
+	}
+	out.Write(body[cursor:])
+	return out.Bytes(), nil
+}
+
+// classicProfilePayloadCommands are the Classic command groups whose records
+// carry a configuration profile's plist in <payloads>.
+var classicProfilePayloadCommands = map[string]bool{
+{{- range . }}{{ if .IsConfigProfile }}
+	"{{ .CLIName }}": true,
+{{- end }}{{ end }}
+}
+
+// ClassicCarriesProfilePayloads reports whether the Classic resource whose
+// command group is cliName carries a configuration profile in <payloads>.
+func ClassicCarriesProfilePayloads(cliName string) bool {
+	return classicProfilePayloadCommands[cliName]
+}
+{{ if anyIsConfigProfile . }}
+// redactClassicProfilePayloadsInMCPChild returns body with each secret inside
+// a configuration profile's <payloads> plist replaced by the redaction marker,
+// when this process is a child of mcp serve. A payload that does not decode is
+// replaced whole, and a body that is not XML is refused rather than printed.
+func redactClassicProfilePayloadsInMCPChild(body []byte) ([]byte, error) {
+	if !registry.InMCPChild() || len(bytes.TrimSpace(body)) == 0 {
+		return body, nil
+	}
+	if !xmlconv.IsXML(body) {
+		return nil, fmt.Errorf("the Classic API answered with a body that is not XML, so its profile payloads cannot be redacted and it is not printed over MCP")
+	}
+	out, err := profileconvert.RedactClassicProfilePayloads(body)
+	if err != nil {
+		return nil, fmt.Errorf("parsing the Classic API response to redact its profile payloads, so it is not printed over MCP: %w", err)
+	}
+	return out, nil
+}
+{{ end }}
 // ── Schema-derived request bodies (--scaffold and --set) ──────────────────
 //
 // The Classic API takes XML and its manifest (specs/classic/resources.yaml)

internal/client/client.go

@@ -541,8 +541,10 @@ func RedactBodyForLog(data []byte) []byte {
 	return redactBodyForLog(data)
 }
 
-// logHeaders prints HTTP headers to w in sorted order. When redactAuth is true,
-// the Authorization header value is replaced with "[redacted]".
+// logHeaders prints HTTP headers to w in sorted order. A Cookie or Set-Cookie
+// value is always replaced with "[redacted]", since a session cookie
+// authenticates the same as the token, and so is Authorization when redactAuth
+// is true.
 func logHeaders(w io.Writer, h http.Header, redactAuth bool) {
 	keys := make([]string, 0, len(h))
 	for k := range h {
@@ -551,7 +553,7 @@ func logHeaders(w io.Writer, h http.Header, redactAuth bool) {
 	sort.Strings(keys)
 	for _, k := range keys {
 		v := strings.Join(h[k], ", ")
-		if redactAuth && strings.EqualFold(k, "Authorization") {
+		if redactAuth && strings.EqualFold(k, "Authorization") || strings.EqualFold(k, "Cookie") || strings.EqualFold(k, "Set-Cookie") {
 			v = "[redacted]"
 		}
 		_, _ = fmt.Fprintf(w, "    %s: %s\n", k, v)

internal/client/log_headers_cookie_test.go

@@ -0,0 +1,30 @@
+// Copyright 2026, Jamf Software LLC
+
+package client
+
+import (
+	"bytes"
+	"net/http"
+	"strings"
+	"testing"
+)
+
+func TestLogHeaders_RedactsCookiesInEveryMode(t *testing.T) {
+	h := http.Header{}
+	h.Add("Set-Cookie", "APBALANCEID=aws.S3CRET-affinity; Path=/; Secure")
+	h.Add("Cookie", "JSESSIONID=S3CRET-session")
+	h.Set("Content-Type", "application/xml")
+	for _, redactAuth := range []bool{true, false} {
+		var buf bytes.Buffer
+		logHeaders(&buf, h, redactAuth)
+		got := buf.String()
+		if strings.Contains(got, "S3CRET-") {
+			t.Errorf("logHeaders(redactAuth=%v) prints a cookie value:\n%s", redactAuth, got)
+		}
+		for _, want := range []string{"Set-Cookie: [redacted]", "Cookie: [redacted]", "Content-Type: application/xml"} {
+			if !strings.Contains(got, want) {
+				t.Errorf("logHeaders(redactAuth=%v) should print %q:\n%s", redactAuth, want, got)
+			}
+		}
+	}
+}