From 39e35a85b5cd0e40e4e5223805025569ce610dbe Mon Sep 17 00:00:00 2001 From: Neil Martin Date: Thu, 30 Jul 2026 12:50:08 +0100 Subject: [PATCH 1/2] fix(classic): normalize config-profile payloads for the Classic API's non-spec XML handling (PI-827) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Classic API does not treat content per XML 1.0: it entity-decodes CDATA content once (spec: zero) and text content twice (spec: once). Any profile whose plist contains an entity — which means any value with & or < in it — failed POST/PUT with an opaque HTTP 409 "Unable to update the database", including Jamf's own GET/backup XML piped back into apply. Fix (all in the classic generator templates): - normalizeClassicProfilePayloadsForSend funnels every input form into the one wire format the server stores correctly: recover the true plist (text-form input is entity-decoded once), guard "]]>", escape every "&" once, wrap in CDATA. Applied as the final body transform in create/update/apply for both config-profile resources — after UUID injection, which needs the true-form plist. - injectClassicFileFields now converts binary plists to XML and rewrites embedded CDATA sections as escaped character data (textually — howett.net/plist drops trailing whitespace in CDATA text, so no parse/re-serialise round trip). - replaceClassicProfilePayload entity-guards "]]>" as a backstop. - profileconvert.NormalizeXML added for the binary-plist conversion. Wire-verified against Jamf Pro 11.x through the platform gateway: all five reserved characters in every legal representation (raw, named entity, decimal and hex refs), literal entity text, embedded CDATA sections, and GET→apply round trips — byte-exact storage through create, update, and apply. The server's only remaining transforms are its own canonicalization (entities for & < >, literals for " ', key sorting, edge-whitespace trim), which no client can influence. Full analysis: docs/solutions/conventions/ classic-profile-payload-ampersand-escaping-2026-07-30.md Co-Authored-By: Claude Fable 5 --- ...e-payload-ampersand-escaping-2026-07-30.md | 116 +++++++++++++ generator/classic/generator.go | 112 ++++++++++++- .../classic_macos_config_profiles.go | 5 + .../classic_mobile_config_profiles.go | 6 + .../generated/classic_payload_escape_test.go | 158 ++++++++++++++++++ .../pro/generated/classic_registry.go | 102 ++++++++++- internal/profileconvert/classic_uuid.go | 17 ++ 7 files changed, 513 insertions(+), 3 deletions(-) create mode 100644 docs/solutions/conventions/classic-profile-payload-ampersand-escaping-2026-07-30.md create mode 100644 internal/commands/pro/generated/classic_payload_escape_test.go diff --git a/docs/solutions/conventions/classic-profile-payload-ampersand-escaping-2026-07-30.md b/docs/solutions/conventions/classic-profile-payload-ampersand-escaping-2026-07-30.md new file mode 100644 index 00000000..a39b8f5d --- /dev/null +++ b/docs/solutions/conventions/classic-profile-payload-ampersand-escaping-2026-07-30.md @@ -0,0 +1,116 @@ +--- +title: "Classic API profile payloads must have every & pre-escaped once inside the CDATA block (PI-827 workaround)" +date: 2026-07-30 +category: conventions +module: generator/classic +problem_type: server-quirk +severity: high +applies_when: + - "Sending content to POST/PUT /JSSResource/osxconfigurationprofiles or mobiledeviceconfigurationprofiles" + - "A profile upload fails with HTTP 409 'Unable to update the database'" + - "Adding a new xml-cdata file field to the classic generator" +tags: + - classic-api + - config-profiles + - cdata + - xml-escaping + - PI-827 +--- + +## Problem + +`pro classic-macos-config-profiles apply --mobileconfig-file X.mobileconfig` +failed with HTTP 409 "Unable to update the database" whenever the +mobileconfig contained any XML entity in a value — e.g. `R&D`. +Since plists must encode `&` and `<` in values, any profile with `&` or `<` +in a string value failed to upload, even though the CLI's CDATA-wrapped +request body is spec-correct XML. + +## Root cause + +Jamf Pro's Classic API entity-decodes `` content one extra time on +ingest — including CDATA content, which a compliant parser must treat as raw +text. `&` arrives at the database as a bare `&`, breaking the server's +own re-parse of the plist. Known server bugs, all unfixed as of 11.30: +[PI-827](https://jamf.atlassian.net/browse/PI-827) (the exact issue, +Triaged/To Do since 2024), [PI-777](https://jamf.atlassian.net/browse/PI-777) +(same family), [PI-690](https://jamf.atlassian.net/browse/PI-690) (closed +Not Going To Do — Jamf treats entity-encoded plists as working as intended). +Don't expect a server-side fix; double-encoding is the de-facto contract. + +## Solution + +The full server model (all empirically verified): the Classic API entity-decodes +`` content **once for CDATA** (spec says zero) and **twice for +text-form** (spec says once — the parser's decode plus the same rogue pass). +So text-form bodies — exactly what a GET/backup response contains — also fail +on any `&`-bearing profile when piped back in. + +`normalizeClassicProfilePayloadsForSend` (classic registry template in +`generator/classic/generator.go`) funnels every input form into the one wire +format the server stores correctly, applied as the **final** body +transformation before `Client.Do` in create/update/apply (gated on +`.IsConfigProfile`): + +1. Recover the true plist: CDATA content is already true form; text-form + content is entity-decoded once (`encoding/xml` chardata parse — handles + named, decimal, and hex refs). +2. Guard `]]>` → `]]>` (backstop; see wrap-time handling below). +3. Escape every `&` once and wrap in CDATA. This is exactly invertible by + the server's single decode pass regardless of which entities the payload + contains (`&` → `&`, `&lt;` → `<`, `&#65;` → `A`). + +Ordering matters: the normalize must run *after* `injectClassicProfilePayloadUUIDs`, +which plist-parses the CDATA content and needs it in true (un-double-escaped) +form. Everything internal operates on the true plist; only the wire form is +escaped. + +Wrap-time (`injectClassicFileFields`): binary plists (`bplist0` prefix) are +converted to XML via `profileconvert.NormalizeXML`; files containing `]]>` +get embedded CDATA sections rewritten as escaped character data by +`stripCDATASections` — a **textual** rewrite, deliberately not a plist +parse/re-serialise round trip, because `howett.net/plist` is not byte-faithful +for CDATA text (drops trailing whitespace). Any stray `]]>` remaining from +malformed input is entity-guarded so the emitted document stays well-formed +(XML 1.0 §2.4/§2.7). `replaceClassicProfilePayload` carries the same guard. + +Verified live (create + update paths) with an entity-torture profile +(`&`, `<`, `>`, `"`, `'`, literal `&` text, numeric refs, unicode): +stored values round-tripped byte-identical, confirmed via GET. + +Also verified: *raw* `>` and `'` in plist text (legal XML, common in +hand-written mobileconfigs, e.g. CEL expressions like +`target.signing_time >= timestamp('...')`) pass through unchanged — the +server's decode pass only affects entities, so `&` is the only character +that needs pre-escaping. The server model this implies: Jamf extracts +payloads content as raw text (CDATA markers stripped, no parser-level +entity resolution) and entity-decodes exactly once — for text-form bodies +that coincides with spec-correct XML (cf. terraform-provider-jamfpro +PR #1103, whose single-`encoding/xml`-escape fix is correct under both +interpretations), for CDATA it's the PI-827 bug. + +## Verification + +All live-verified against Jamf Pro 11.x via the platform gateway: every +reserved character (`&` `<` `>` `"` `'`) in every legal representation (raw +where the spec allows, named entity, decimal ref, hex ref), literal entity +text (`&amp;`), embedded CDATA sections, unicode, and a Santa CEL +expression (`target.signing_time >= timestamp('...') && ...`) — byte-exact +storage through create, update, and the GET→apply text-form round trip. + +## Scope notes + +- GET responses are *not* double-encoded — the server escapes correctly on + output. The quirk is ingest-only, so no decode is needed when reading. +- The server trims leading/trailing whitespace in plist string values on + ingest (verified with entity-free values — it is the server's own plist + re-serialisation, unrelated to escaping, and no client can prevent it). + Internal whitespace survives. +- `macapplications`/`mobiledeviceapplications` `--appconfig-file` also embeds + xml-cdata content (`app_configuration/preferences`) and may have the same + server quirk — untested, not covered by the send-time normalizer yet + (the wrap-time `]]>`/binary handling does apply to it). Test before + extending `normalizeClassicProfilePayloadsForSend` to it. +- Related external context: terraform-provider-jamfpro PR #1103 fixed the + provider's own double-escaping of text-form payloads; its device-verified + evidence corroborates the decode arithmetic above. diff --git a/generator/classic/generator.go b/generator/classic/generator.go index 6c1419dd..e426f7e8 100644 --- a/generator/classic/generator.go +++ b/generator/classic/generator.go @@ -622,6 +622,9 @@ func new{{ .GoName }}CreateCmd(ctx *registry.CLIContext) *cobra.Command { return err } {{- end }} +{{ if .IsConfigProfile }} + bodyBytes = normalizeClassicProfilePayloadsForSend(bodyBytes) +{{ end -}} resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/{{ .Path }}/{{ idPath . }}/0", bytes.NewReader(bodyBytes)) {{ else }} var body io.Reader @@ -807,6 +810,7 @@ func new{{ .GoName }}UpdateCmd(ctx *registry.CLIContext) *cobra.Command { {{ if .IsConfigProfile }} bodyBytes = injectClassicProfilePayloadUUIDs(bodyBytes, existingPayload) bodyBytes = injectClassicRedeployOnUpdate(bodyBytes) + bodyBytes = normalizeClassicProfilePayloadsForSend(bodyBytes) {{ end }} path := fmt.Sprintf("/JSSResource/{{ .Path }}/{{ idPath . }}/%s", url.PathEscape(resolvedID)) @@ -1239,6 +1243,9 @@ If not, a new resource is created.` + "`" + `, fmt.Fprintf(os.Stderr, "[dry-run] Would create {{ .Singular }} %q\n", name) return nil } +{{ if .IsConfigProfile }} + data = normalizeClassicProfilePayloadsForSend(data) +{{ end -}} resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/{{ .Path }}/{{ idPath . }}/0", bytes.NewReader(data)) if err != nil { return err @@ -1287,6 +1294,7 @@ If not, a new resource is created.` + "`" + `, existingPayload := fetchClassicProfilePayloadPlist(reqCtx, ctx.Client, "{{ .Path }}", id) data = injectClassicProfilePayloadUUIDs(data, existingPayload) data = injectClassicRedeployOnUpdate(data) + data = normalizeClassicProfilePayloadsForSend(data) {{ end }} updatePath := fmt.Sprintf("/JSSResource/{{ .Path }}/{{ idPath . }}/%s", url.PathEscape(id)) resp, err := ctx.Client.Do(reqCtx, "PUT", updatePath, bytes.NewReader(data)) @@ -1350,7 +1358,7 @@ import ( {{- if or (anyNeedsClassicNameResolve .) (anyClassicFileFields .) (anyListSubset .) }} "github.com/Jamf-Concepts/jamf-cli/internal/xmlconv" {{- end }} -{{- if anyIsConfigProfile . }} +{{- if or (anyIsConfigProfile .) (anyClassicFileFields .) }} "github.com/Jamf-Concepts/jamf-cli/internal/profileconvert" {{- end }} {{- if anyHasCustomPayload . }} @@ -1611,7 +1619,9 @@ func replaceClassicProfilePayload(xmlBody, newPayload []byte) []byte { var buf bytes.Buffer buf.Write(xmlBody[:si]) buf.WriteString(openTag + "" so the payload can never terminate the CDATA section early + // (XML 1.0 §2.4/§2.7). + buf.Write(bytes.ReplaceAll(newPayload, []byte("]]>"), []byte("]]>"))) buf.WriteString("]]>" + closeTag) buf.Write(xmlBody[ei+len(closeTag):]) return buf.Bytes() @@ -1669,6 +1679,89 @@ func injectClassicRedeployOnUpdate(body []byte) []byte { insertAt := gOpen + len("") return []byte(s[:insertAt] + "All" + s[insertAt:]) } + +// stripCDATASections rewrites every section in an XML +// document as equivalent escaped character data. The parsed document is +// unchanged (CDATA content is literal text by definition — XML 1.0 §2.7), +// but the literal "]]>" terminators disappear, so the result can itself be +// CDATA-wrapped. Content after an unterminated ", ">") + var b strings.Builder + for { + si := strings.Index(s, "") + if ei < 0 { + break + } + b.WriteString(s[:si]) + b.WriteString(esc.Replace(s[si+len(""):] + } + if b.Len() == 0 { + return data + } + b.WriteString(s) + return []byte(b.String()) +} + +// normalizeClassicProfilePayloadsForSend rewrites the element into +// the one wire format the Classic API stores correctly, immediately before +// the request is sent. The server does not treat payload content per the XML +// spec: it entity-decodes CDATA content once (it must decode zero times) and +// text content twice (it must decode once) — PI-827. A profile whose plist +// contains any entity ("&", "<", ...) therefore reaches the database +// as invalid XML and the write fails with HTTP 409 "Unable to update the +// database", or worse, stores mangled. +// +// Normalization: recover the true plist (CDATA content is already true form; +// text content — e.g. a GET/backup response piped back in — is entity-decoded +// once), then guard "]]>" as "]]>" and escape every "&" once. The server's +// single CDATA decode pass exactly inverts the escape, whatever mix of +// entities the payload contains. Bodies without a non-empty +// element are returned unchanged. +func normalizeClassicProfilePayloadsForSend(body []byte) []byte { + const openTag = "" + s := string(body) + si := strings.Index(s, openTag) + if si < 0 { + return body + } + ci := si + len(openTag) + var inner string + var restAt int // index into s of the first byte after + if strings.HasPrefix(s[ci:], "") + if end < 0 { + return body + } + inner = s[ci+len("") + } else { + end := strings.Index(s[ci:], "") + if end < 0 { + return body + } + var decoded struct { + Data string ` + "`" + `xml:",chardata"` + "`" + ` + } + if err := xml.Unmarshal([]byte(openTag+s[ci:ci+end]+""), &decoded); err != nil { + return body + } + inner = decoded.Data + restAt = ci + end + len("") + } + if inner == "" { + return body + } + inner = strings.ReplaceAll(inner, "]]>", "]]>") + inner = strings.ReplaceAll(inner, "&", "&") + return []byte(s[:ci] + "" + s[restAt:]) +} {{ end }} {{ if anyClassicFileFields . }} // classicFileFieldSpec describes one Classic XML field sourced from a local file or in-memory bytes. @@ -1722,6 +1815,21 @@ func injectClassicFileFields(body []byte, rootName string, specs []classicFileFi } var inner string if s.Encoding == "xml-cdata" { + if bytes.HasPrefix(data, []byte("bplist0")) { + // Binary plist — convert to XML so it can be embedded at all. + if normalized, err := profileconvert.NormalizeXML(data); err == nil { + data = normalized + } + } + if bytes.Contains(data, []byte("]]>")) { + // "]]>" would terminate the CDATA wrapper early (XML 1.0 + // §2.4/§2.7). Embedded CDATA sections are the one way a valid + // plist contains it — rewrite them as escaped character data + // (byte-faithful, no plist parsing), then entity-guard any + // stray remainder from malformed input. + data = stripCDATASections(data) + data = bytes.ReplaceAll(data, []byte("]]>"), []byte("]]>")) + } inner = "" } else { inner = string(data) diff --git a/internal/commands/pro/generated/classic_macos_config_profiles.go b/internal/commands/pro/generated/classic_macos_config_profiles.go index f2241919..295f0b3a 100644 --- a/internal/commands/pro/generated/classic_macos_config_profiles.go +++ b/internal/commands/pro/generated/classic_macos_config_profiles.go @@ -216,6 +216,7 @@ func newClassicMacosConfigProfilesCreateCmd(ctx *registry.CLIContext) *cobra.Com return err } + bodyBytes = normalizeClassicProfilePayloadsForSend(bodyBytes) resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/osxconfigurationprofiles/id/0", bytes.NewReader(bodyBytes)) if err != nil { @@ -313,6 +314,7 @@ func newClassicMacosConfigProfilesUpdateCmd(ctx *registry.CLIContext) *cobra.Com bodyBytes = injectClassicProfilePayloadUUIDs(bodyBytes, existingPayload) bodyBytes = injectClassicRedeployOnUpdate(bodyBytes) + bodyBytes = normalizeClassicProfilePayloadsForSend(bodyBytes) path := fmt.Sprintf("/JSSResource/osxconfigurationprofiles/id/%s", url.PathEscape(resolvedID)) resp, err := ctx.Client.Do(reqCtx, "PUT", path, bytes.NewReader(bodyBytes)) @@ -610,6 +612,8 @@ If not, a new resource is created.`, fmt.Fprintf(os.Stderr, "[dry-run] Would create os_x_configuration_profile %q\n", name) return nil } + + data = normalizeClassicProfilePayloadsForSend(data) resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/osxconfigurationprofiles/id/0", bytes.NewReader(data)) if err != nil { return err @@ -641,6 +645,7 @@ If not, a new resource is created.`, existingPayload := fetchClassicProfilePayloadPlist(reqCtx, ctx.Client, "osxconfigurationprofiles", id) data = injectClassicProfilePayloadUUIDs(data, existingPayload) data = injectClassicRedeployOnUpdate(data) + data = normalizeClassicProfilePayloadsForSend(data) updatePath := fmt.Sprintf("/JSSResource/osxconfigurationprofiles/id/%s", url.PathEscape(id)) resp, err := ctx.Client.Do(reqCtx, "PUT", updatePath, bytes.NewReader(data)) diff --git a/internal/commands/pro/generated/classic_mobile_config_profiles.go b/internal/commands/pro/generated/classic_mobile_config_profiles.go index 625c48c9..00ca5cfa 100644 --- a/internal/commands/pro/generated/classic_mobile_config_profiles.go +++ b/internal/commands/pro/generated/classic_mobile_config_profiles.go @@ -193,6 +193,8 @@ func newClassicMobileConfigProfilesCreateCmd(ctx *registry.CLIContext) *cobra.Co if err != nil { return err } + + bodyBytes = normalizeClassicProfilePayloadsForSend(bodyBytes) resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/mobiledeviceconfigurationprofiles/id/0", bytes.NewReader(bodyBytes)) if err != nil { @@ -269,6 +271,7 @@ func newClassicMobileConfigProfilesUpdateCmd(ctx *registry.CLIContext) *cobra.Co bodyBytes = injectClassicProfilePayloadUUIDs(bodyBytes, existingPayload) bodyBytes = injectClassicRedeployOnUpdate(bodyBytes) + bodyBytes = normalizeClassicProfilePayloadsForSend(bodyBytes) path := fmt.Sprintf("/JSSResource/mobiledeviceconfigurationprofiles/id/%s", url.PathEscape(resolvedID)) resp, err := ctx.Client.Do(reqCtx, "PUT", path, bytes.NewReader(bodyBytes)) @@ -538,6 +541,8 @@ If not, a new resource is created.`, fmt.Fprintf(os.Stderr, "[dry-run] Would create configuration_profile %q\n", name) return nil } + + data = normalizeClassicProfilePayloadsForSend(data) resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/mobiledeviceconfigurationprofiles/id/0", bytes.NewReader(data)) if err != nil { return err @@ -569,6 +574,7 @@ If not, a new resource is created.`, existingPayload := fetchClassicProfilePayloadPlist(reqCtx, ctx.Client, "mobiledeviceconfigurationprofiles", id) data = injectClassicProfilePayloadUUIDs(data, existingPayload) data = injectClassicRedeployOnUpdate(data) + data = normalizeClassicProfilePayloadsForSend(data) updatePath := fmt.Sprintf("/JSSResource/mobiledeviceconfigurationprofiles/id/%s", url.PathEscape(id)) resp, err := ctx.Client.Do(reqCtx, "PUT", updatePath, bytes.NewReader(data)) diff --git a/internal/commands/pro/generated/classic_payload_escape_test.go b/internal/commands/pro/generated/classic_payload_escape_test.go new file mode 100644 index 00000000..41cd4c87 --- /dev/null +++ b/internal/commands/pro/generated/classic_payload_escape_test.go @@ -0,0 +1,158 @@ +// Copyright 2026, Jamf Software LLC + +package generated + +import ( + "strings" + "testing" +) + +// ─── normalizeClassicProfilePayloadsForSend ─────────────────────────────────── +// +// The Classic API does not treat content per the XML spec: it +// entity-decodes CDATA content once and text content twice (PI-827). The +// normalizer rewrites any payloads form into CDATA with every "&" escaped +// once, which the server's single decode pass exactly inverts. + +// serverIngest simulates what the Classic API stores for a CDATA-wrapped +// payload: one entity-decode pass over the content. +func serverIngest(t *testing.T, wire string) string { + t.Helper() + const openMark = "" + si := strings.Index(wire, openMark) + ei := strings.Index(wire, closeMark) + if si < 0 || ei < 0 { + t.Fatalf("wire has no CDATA payloads block: %s", wire) + } + inner := wire[si+len(openMark) : ei] + if strings.Contains(inner, "]]>") { + t.Fatalf("wire CDATA content contains ]]> — malformed XML: %s", inner) + } + r := strings.NewReplacer("&", "&", "<", "<", ">", ">", """, `"`, "'", "'") + return r.Replace(inner) +} + +func TestNormalizePayloads_CDATA_RoundTripsThroughServerDecode(t *testing.T) { + plists := []string{ + `R&D`, + `a < b > c "d"`, + `x & & y`, + `raw > and ' and "`, + `literal &amp; entity text`, + "target.signing_time >= timestamp('2025-05-31T00:00:00Z')", + } + for _, p := range plists { + body := `` + wire := string(normalizeClassicProfilePayloadsForSend([]byte(body))) + if got := serverIngest(t, wire); got != p { + t.Errorf("server would store %q, want %q", got, p) + } + } +} + +func TestNormalizePayloads_TextForm_ConvertedToCDATA(t *testing.T) { + // A GET/backup response carries payloads as entity-escaped text. The + // normalizer must decode it once to the true plist, then re-wrap so the + // server stores the plist exactly. + truePlist := `kR&D > 'x'` + escapedOnce := strings.NewReplacer("&", "&", "<", "<", ">", ">").Replace(truePlist) + body := `` + escapedOnce + `` + wire := string(normalizeClassicProfilePayloadsForSend([]byte(body))) + if !strings.Contains(wire, "<string>A & B & C</string>` + wire := string(normalizeClassicProfilePayloadsForSend([]byte(body))) + want := `A & B & C` + if got := serverIngest(t, wire); got != want { + t.Errorf("server would store %q, want %q", got, want) + } +} + +func TestNormalizePayloads_CDATATerminatorGuarded(t *testing.T) { + // "]]>" reaching the wrapper raw would truncate the CDATA section. The + // guarded form must survive the server decode as the valid entity form. + body := `a]]>b]]>` + wire := string(normalizeClassicProfilePayloadsForSend([]byte(body))) + if strings.Contains(strings.TrimSuffix(wire[strings.Index(wire, ""), "]]>") { + t.Fatalf("wire CDATA content contains raw ]]>: %s", wire) + } + if got := serverIngest(t, wire); got != `a]]>b` { + t.Errorf("server would store %q", got) + } +} + +func TestNormalizePayloads_OutsideContentUntouched(t *testing.T) { + // Ampersands outside the payloads element (e.g. in the profile name, + // already escaped once by classicXMLEscape) must not be touched. + body := `R&D Profilea & b` + wire := string(normalizeClassicProfilePayloadsForSend([]byte(body))) + if !strings.Contains(wire, `R&D Profile`) || !strings.Contains(wire, `a & b`) { + t.Errorf("content outside payloads was modified: %s", wire) + } + if !strings.Contains(wire, ``) { + t.Errorf("payloads content not escaped: %s", wire) + } +} + +func TestNormalizePayloads_NoPayloads(t *testing.T) { + body := `a & b` + if got := string(normalizeClassicProfilePayloadsForSend([]byte(body))); got != body { + t.Errorf("body without payloads changed: got %s", got) + } +} + +func TestNormalizePayloads_EmptyPayloads(t *testing.T) { + body := `` + if got := string(normalizeClassicProfilePayloadsForSend([]byte(body))); got != body { + t.Errorf("empty payloads changed: got %s", got) + } +} + +func TestNormalizePayloads_UnterminatedCDATA(t *testing.T) { + body := ` " ]]>` + want := `cdata & raw < > " ` + if got := string(stripCDATASections([]byte(in))); got != want { + t.Errorf("got %s, want %s", got, want) + } +} + +func TestStripCDATASections_MultipleSections(t *testing.T) { + in := `plain & kept` + want := `x & yplain & keptz < w` + if got := string(stripCDATASections([]byte(in))); got != want { + t.Errorf("got %s, want %s", got, want) + } +} + +func TestStripCDATASections_NoCDATA(t *testing.T) { + in := `plain & text` + if got := string(stripCDATASections([]byte(in))); got != in { + t.Errorf("content without CDATA changed: got %s", got) + } +} + +func TestStripCDATASections_Unterminated(t *testing.T) { + in := `" so the payload can never terminate the CDATA section early + // (XML 1.0 §2.4/§2.7). + buf.Write(bytes.ReplaceAll(newPayload, []byte("]]>"), []byte("]]>"))) buf.WriteString("]]>" + closeTag) buf.Write(xmlBody[ei+len(closeTag):]) return buf.Bytes() @@ -385,6 +387,89 @@ func injectClassicRedeployOnUpdate(body []byte) []byte { return []byte(s[:insertAt] + "All" + s[insertAt:]) } +// stripCDATASections rewrites every section in an XML +// document as equivalent escaped character data. The parsed document is +// unchanged (CDATA content is literal text by definition — XML 1.0 §2.7), +// but the literal "]]>" terminators disappear, so the result can itself be +// CDATA-wrapped. Content after an unterminated ", ">") + var b strings.Builder + for { + si := strings.Index(s, "") + if ei < 0 { + break + } + b.WriteString(s[:si]) + b.WriteString(esc.Replace(s[si+len(""):] + } + if b.Len() == 0 { + return data + } + b.WriteString(s) + return []byte(b.String()) +} + +// normalizeClassicProfilePayloadsForSend rewrites the element into +// the one wire format the Classic API stores correctly, immediately before +// the request is sent. The server does not treat payload content per the XML +// spec: it entity-decodes CDATA content once (it must decode zero times) and +// text content twice (it must decode once) — PI-827. A profile whose plist +// contains any entity ("&", "<", ...) therefore reaches the database +// as invalid XML and the write fails with HTTP 409 "Unable to update the +// database", or worse, stores mangled. +// +// Normalization: recover the true plist (CDATA content is already true form; +// text content — e.g. a GET/backup response piped back in — is entity-decoded +// once), then guard "]]>" as "]]>" and escape every "&" once. The server's +// single CDATA decode pass exactly inverts the escape, whatever mix of +// entities the payload contains. Bodies without a non-empty +// element are returned unchanged. +func normalizeClassicProfilePayloadsForSend(body []byte) []byte { + const openTag = "" + s := string(body) + si := strings.Index(s, openTag) + if si < 0 { + return body + } + ci := si + len(openTag) + var inner string + var restAt int // index into s of the first byte after + if strings.HasPrefix(s[ci:], "") + if end < 0 { + return body + } + inner = s[ci+len("") + } else { + end := strings.Index(s[ci:], "") + if end < 0 { + return body + } + var decoded struct { + Data string `xml:",chardata"` + } + if err := xml.Unmarshal([]byte(openTag+s[ci:ci+end]+""), &decoded); err != nil { + return body + } + inner = decoded.Data + restAt = ci + end + len("") + } + if inner == "" { + return body + } + inner = strings.ReplaceAll(inner, "]]>", "]]>") + inner = strings.ReplaceAll(inner, "&", "&") + return []byte(s[:ci] + "" + s[restAt:]) +} + // classicFileFieldSpec describes one Classic XML field sourced from a local file or in-memory bytes. type classicFileFieldSpec struct { FilePath string // user-supplied path (empty = skip when FileBytes also nil) @@ -436,6 +521,21 @@ func injectClassicFileFields(body []byte, rootName string, specs []classicFileFi } var inner string if s.Encoding == "xml-cdata" { + if bytes.HasPrefix(data, []byte("bplist0")) { + // Binary plist — convert to XML so it can be embedded at all. + if normalized, err := profileconvert.NormalizeXML(data); err == nil { + data = normalized + } + } + if bytes.Contains(data, []byte("]]>")) { + // "]]>" would terminate the CDATA wrapper early (XML 1.0 + // §2.4/§2.7). Embedded CDATA sections are the one way a valid + // plist contains it — rewrite them as escaped character data + // (byte-faithful, no plist parsing), then entity-guard any + // stray remainder from malformed input. + data = stripCDATASections(data) + data = bytes.ReplaceAll(data, []byte("]]>"), []byte("]]>")) + } inner = "" } else { inner = string(data) diff --git a/internal/profileconvert/classic_uuid.go b/internal/profileconvert/classic_uuid.go index 493aed89..75d834a8 100644 --- a/internal/profileconvert/classic_uuid.go +++ b/internal/profileconvert/classic_uuid.go @@ -59,6 +59,23 @@ func InjectIdentifiers(newPlist, existingPlist []byte) ([]byte, error) { return out, nil } +// NormalizeXML parses a plist (XML or binary) and re-serialises it as XML +// with standard entity escaping. Used to eliminate constructs that cannot +// survive CDATA-wrapping for the Classic API — most notably embedded CDATA +// sections, whose literal "]]>" terminator would end the wrapper early. +// Values round-trip exactly; only the byte representation changes. +func NormalizeXML(data []byte) ([]byte, error) { + var v any + if _, err := plist.Unmarshal(data, &v); err != nil { + return nil, fmt.Errorf("parsing plist: %w", err) + } + out, err := plist.MarshalIndent(v, plist.XMLFormat, "\t") + if err != nil { + return nil, fmt.Errorf("re-serialising plist: %w", err) + } + return out, nil +} + // ExtractProfileIdentifiers extracts the top-level PayloadUUID and // PayloadIdentifier from a mobileconfig plist. Returns empty strings when // the fields are absent (not an error condition). From 6624c46c9794ab93f8737253ebb61feb2e41f80a Mon Sep 17 00:00:00 2001 From: Neil Martin Date: Thu, 30 Jul 2026 13:46:50 +0100 Subject: [PATCH 2/2] fix(classic): verify stored payloads, handle signed/binary input, correct the PI-827 model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the initial escaping fix, driven by live probes across two tenants that refined the server model: - The 409 on raw content is a validation pass (fires for any payload type when one decode yields a bare & or <), so escape-once stays the wire form. Storage is per-payload-type: MCX custom settings and notification settings fragments are decoded once (escape stores them byte-exact); every other payload type — TCC, direct loginwindow, all mobile device payloads — is stored verbatim, keeping an extra entity layer. Values with & or < in those types cannot round-trip via any client. A raw-first/retry-on-409 design was implemented and reverted in this branch once probes showed raw never succeeds for entity-bearing profiles. - verifyClassicProfileStored: after every successful profile write, GET the stored payload and semantically compare against the submitted plist (profileconvert.DiffPayloadValues — masks server-rewritten Payload* metadata, tolerates edge-whitespace trim). Server-side corruption now surfaces as a stderr warning naming the exact paths instead of passing silently. - CMS-signed mobileconfigs are detected and their inner plist extracted (smallstep/pkcs7) with a note — the signature cannot survive this API. Binary plists are converted to XML. Live-verified: MCX reserved-character matrix byte-exact with zero warnings (osx create/update/apply, backup round-trip); mobile matrix and signed Login Window profiles create with deterministic warnings naming the affected values. Co-Authored-By: Claude Fable 5 --- ...e-payload-ampersand-escaping-2026-07-30.md | 159 +++++++++--------- generator/classic/generator.go | 156 ++++++++++++++--- go.mod | 1 + go.sum | 2 + .../pro/generated/classic_account_groups.go | 2 +- .../pro/generated/classic_account_users.go | 2 +- .../classic_advanced_computer_searches.go | 3 +- ...classic_advanced_mobile_device_searches.go | 3 +- .../classic_allowed_file_extensions.go | 2 +- .../commands/pro/generated/classic_classes.go | 3 +- .../pro/generated/classic_computer_configs.go | 3 +- .../generated/classic_computer_ext_attrs.go | 3 +- .../pro/generated/classic_computer_groups.go | 3 +- .../generated/classic_computer_invitations.go | 2 +- .../generated/classic_directory_bindings.go | 3 +- .../classic_disk_encryption_configs.go | 3 +- .../generated/classic_distribution_points.go | 3 +- .../pro/generated/classic_dock_items.go | 3 +- .../commands/pro/generated/classic_ebooks.go | 3 +- .../pro/generated/classic_ibeacons.go | 3 +- .../pro/generated/classic_jwt_configs.go | 2 +- .../pro/generated/classic_ldap_servers.go | 3 +- .../generated/classic_licensed_software.go | 3 +- .../pro/generated/classic_mac_apps.go | 3 +- .../classic_macos_config_profiles.go | 20 ++- .../pro/generated/classic_mobile_apps.go | 3 +- .../classic_mobile_config_profiles.go | 20 ++- .../generated/classic_mobile_device_groups.go | 3 +- .../pro/generated/classic_mobile_devices.go | 3 +- .../generated/classic_mobile_invitations.go | 2 +- .../classic_mobile_provisioning_profiles.go | 3 +- .../pro/generated/classic_network_segments.go | 3 +- .../pro/generated/classic_packages.go | 3 +- .../classic_patch_external_sources.go | 3 +- .../pro/generated/classic_patch_policies.go | 2 +- .../pro/generated/classic_patch_titles.go | 3 +- .../generated/classic_payload_escape_test.go | 108 ++++++------ .../pro/generated/classic_policies.go | 3 +- .../pro/generated/classic_printers.go | 3 +- .../pro/generated/classic_registry.go | 101 +++++++++-- .../classic_removable_mac_addresses.go | 3 +- .../generated/classic_restricted_software.go | 3 +- .../classic_software_update_servers.go | 3 +- .../pro/generated/classic_user_ext_attrs.go | 3 +- .../pro/generated/classic_user_groups.go | 3 +- .../pro/generated/classic_vpp_accounts.go | 2 +- .../pro/generated/classic_vpp_assignments.go | 2 +- .../pro/generated/classic_vpp_invitations.go | 2 +- .../pro/generated/classic_webhooks.go | 3 +- internal/profileconvert/classic_verify.go | 145 ++++++++++++++++ .../profileconvert/classic_verify_test.go | 107 ++++++++++++ 51 files changed, 721 insertions(+), 211 deletions(-) create mode 100644 internal/profileconvert/classic_verify.go create mode 100644 internal/profileconvert/classic_verify_test.go diff --git a/docs/solutions/conventions/classic-profile-payload-ampersand-escaping-2026-07-30.md b/docs/solutions/conventions/classic-profile-payload-ampersand-escaping-2026-07-30.md index a39b8f5d..c1164386 100644 --- a/docs/solutions/conventions/classic-profile-payload-ampersand-escaping-2026-07-30.md +++ b/docs/solutions/conventions/classic-profile-payload-ampersand-escaping-2026-07-30.md @@ -1,5 +1,5 @@ --- -title: "Classic API profile payloads must have every & pre-escaped once inside the CDATA block (PI-827 workaround)" +title: "Classic API profile payloads: escape every & once inside CDATA, then verify — the server's entity handling is per-payload-type (PI-827)" date: 2026-07-30 category: conventions module: generator/classic @@ -8,6 +8,7 @@ severity: high applies_when: - "Sending content to POST/PUT /JSSResource/osxconfigurationprofiles or mobiledeviceconfigurationprofiles" - "A profile upload fails with HTTP 409 'Unable to update the database'" + - "A stored profile value shows & / > as literal text on a device" - "Adding a new xml-cdata file field to the classic generator" tags: - classic-api @@ -20,97 +21,89 @@ tags: ## Problem `pro classic-macos-config-profiles apply --mobileconfig-file X.mobileconfig` -failed with HTTP 409 "Unable to update the database" whenever the -mobileconfig contained any XML entity in a value — e.g. `R&D`. -Since plists must encode `&` and `<` in values, any profile with `&` or `<` -in a string value failed to upload, even though the CLI's CDATA-wrapped -request body is spec-correct XML. - -## Root cause - -Jamf Pro's Classic API entity-decodes `` content one extra time on -ingest — including CDATA content, which a compliant parser must treat as raw -text. `&` arrives at the database as a bare `&`, breaking the server's -own re-parse of the plist. Known server bugs, all unfixed as of 11.30: -[PI-827](https://jamf.atlassian.net/browse/PI-827) (the exact issue, -Triaged/To Do since 2024), [PI-777](https://jamf.atlassian.net/browse/PI-777) -(same family), [PI-690](https://jamf.atlassian.net/browse/PI-690) (closed -Not Going To Do — Jamf treats entity-encoded plists as working as intended). -Don't expect a server-side fix; double-encoding is the de-facto contract. +failed with an opaque HTTP 409 "Unable to update the database" whenever the +mobileconfig contained any XML entity in a value — e.g. +`R&D`. Since plists must encode `&` and `<`, any +profile with `&` or `<` in a string value could not be uploaded. Backup +round-trips (GET XML piped back into `apply`) failed identically. + +## The server model (every clause wire-verified 2026-07-30, EU + US tenants) + +1. **Validation**: the server entity-decodes the submitted payload content + once and rejects the write with 409 when the result contains a bare `&` + or `<`. Consequence: spec-correct raw CDATA is rejected for ANY plist + containing `&`/`<`, regardless of payload types. The escaped form + (every `&` escaped once more) is the only submittable form for such + profiles. +2. **Storage is per-payload-type** (proven with a mixed profile — one wire + body, two different treatments): + - Fragments of payload types the server re-renders — + `com.apple.ManagedClient.preferences` custom settings (values AND dict + keys), `com.apple.notificationsettings` — are entity-decoded once. + The escape stores them **byte-exact**. + - Fragments of every other payload type (TCC, direct + `com.apple.loginwindow`, and all payloads on + `mobiledeviceconfigurationprofiles`) are stored **verbatim**, keeping + one extra entity layer. Values with `&`/`<` in those types cannot be + stored faithfully by any client — the device sees `&` where `&` + was meant. This matches PI-827's history: Jamf's own UI-created + exports cannot be re-uploaded intact. +3. GET responses are escaped correctly (ingest-only quirk); the server also + canonicalizes representation (entities for `& < >`, literals for `" '`, + sorted keys) and trims leading/trailing whitespace inside string values. + +Do not re-derive this from theory: a raw-first/retry-on-409 design was +implemented and reverted in this branch's history because probes showed the +409 is validation (fires for all payload types), not a per-type signal — +raw never succeeds for entity-bearing profiles, so retrying costs a round +trip and buys nothing. ## Solution -The full server model (all empirically verified): the Classic API entity-decodes -`` content **once for CDATA** (spec says zero) and **twice for -text-form** (spec says once — the parser's decode plus the same rogue pass). -So text-form bodies — exactly what a GET/backup response contains — also fail -on any `&`-bearing profile when piped back in. - `normalizeClassicProfilePayloadsForSend` (classic registry template in -`generator/classic/generator.go`) funnels every input form into the one wire -format the server stores correctly, applied as the **final** body -transformation before `Client.Do` in create/update/apply (gated on -`.IsConfigProfile`): +`generator/classic/generator.go`), applied as the final body transform in +create/update/apply for both profile resources: 1. Recover the true plist: CDATA content is already true form; text-form - content is entity-decoded once (`encoding/xml` chardata parse — handles - named, decimal, and hex refs). -2. Guard `]]>` → `]]>` (backstop; see wrap-time handling below). -3. Escape every `&` once and wrap in CDATA. This is exactly invertible by - the server's single decode pass regardless of which entities the payload - contains (`&` → `&`, `&lt;` → `<`, `&#65;` → `A`). - -Ordering matters: the normalize must run *after* `injectClassicProfilePayloadUUIDs`, -which plist-parses the CDATA content and needs it in true (un-double-escaped) -form. Everything internal operates on the true plist; only the wire form is -escaped. - -Wrap-time (`injectClassicFileFields`): binary plists (`bplist0` prefix) are -converted to XML via `profileconvert.NormalizeXML`; files containing `]]>` -get embedded CDATA sections rewritten as escaped character data by -`stripCDATASections` — a **textual** rewrite, deliberately not a plist -parse/re-serialise round trip, because `howett.net/plist` is not byte-faithful -for CDATA text (drops trailing whitespace). Any stray `]]>` remaining from -malformed input is entity-guarded so the emitted document stays well-formed -(XML 1.0 §2.4/§2.7). `replaceClassicProfilePayload` carries the same guard. - -Verified live (create + update paths) with an entity-torture profile -(`&`, `<`, `>`, `"`, `'`, literal `&` text, numeric refs, unicode): -stored values round-tripped byte-identical, confirmed via GET. - -Also verified: *raw* `>` and `'` in plist text (legal XML, common in -hand-written mobileconfigs, e.g. CEL expressions like -`target.signing_time >= timestamp('...')`) pass through unchanged — the -server's decode pass only affects entities, so `&` is the only character -that needs pre-escaping. The server model this implies: Jamf extracts -payloads content as raw text (CDATA markers stripped, no parser-level -entity resolution) and entity-decodes exactly once — for text-form bodies -that coincides with spec-correct XML (cf. terraform-provider-jamfpro -PR #1103, whose single-`encoding/xml`-escape fix is correct under both -interpretations), for CDATA it's the PI-827 bug. + content (GET/backup XML piped back in) is entity-decoded once. +2. Guard `]]>` → `]]>`, then escape every `&` once and wrap in CDATA. + +`verifyClassicProfileStored` then GETs the stored payload after every +successful write and compares it against the submitted plist +(`profileconvert.DiffPayloadValues` — parse-based, masks the +`Payload*` metadata keys the server rewrites, trims string edges). When the +server stored values differently (case 2 above), the exact dotted paths are +printed as a stderr warning. Silent corruption is never acceptable; +a warning that names `PayloadContent[3].LoginwindowText` is. + +Wrap-time (`injectClassicFileFields`): CMS-signed mobileconfigs are +detected (`profileconvert.IsSignedProfile`) and their inner plist extracted +(`ExtractSignedProfile`, smallstep/pkcs7) with a stderr note — the +signature cannot survive this API anyway. Binary plists (`bplist0`) are +converted to XML (`NormalizeXML`). Embedded CDATA sections are rewritten as +escaped character data by `stripCDATASections` — a **textual** rewrite, +deliberately not a plist re-serialise, because `howett.net/plist` drops +trailing whitespace in CDATA text. ## Verification -All live-verified against Jamf Pro 11.x via the platform gateway: every -reserved character (`&` `<` `>` `"` `'`) in every legal representation (raw -where the spec allows, named entity, decimal ref, hex ref), literal entity -text (`&amp;`), embedded CDATA sections, unicode, and a Santa CEL -expression (`target.signing_time >= timestamp('...') && ...`) — byte-exact -storage through create, update, and the GET→apply text-form round trip. +Live matrix on both tenants: all five reserved characters in every legal +representation (raw/named/decimal/hex), literal entity text, embedded CDATA +sections, dict keys with entities, CEL expressions, signed profiles, binary +plists, GET→apply round-trips — byte-exact storage for MCX-family content +through create/update/apply, with deterministic warnings for the +server-unfixable combinations (mobile profiles and typed macOS payloads +carrying `&`/`<` in values). ## Scope notes -- GET responses are *not* double-encoded — the server escapes correctly on - output. The quirk is ingest-only, so no decode is needed when reading. -- The server trims leading/trailing whitespace in plist string values on - ingest (verified with entity-free values — it is the server's own plist - re-serialisation, unrelated to escaping, and no client can prevent it). - Internal whitespace survives. -- `macapplications`/`mobiledeviceapplications` `--appconfig-file` also embeds - xml-cdata content (`app_configuration/preferences`) and may have the same - server quirk — untested, not covered by the send-time normalizer yet - (the wrap-time `]]>`/binary handling does apply to it). Test before - extending `normalizeClassicProfilePayloadsForSend` to it. -- Related external context: terraform-provider-jamfpro PR #1103 fixed the - provider's own double-escaping of text-form payloads; its device-verified - evidence corroborates the decode arithmetic above. +- `macapplications`/`mobiledeviceapplications` `--appconfig-file` also + embeds xml-cdata content — the wrap-time handling applies, but the + escape/verify path does not (untested against the server; probe before + extending). +- Related: terraform-provider-jamfpro PR #1103 and jamfplatform-go-sdk's + `PayloadsXMLText` — the SDK needs the same wire form; its acceptance + matrix in `acc_proclassic_profile_payloads_test.go` encodes the same + server model. +- Jira: PI-827 (Triaged since 2024), PI-777, PI-690 (closed Not Going To + Do). The per-payload-type evidence here is a ready-made escalation kit. diff --git a/generator/classic/generator.go b/generator/classic/generator.go index e426f7e8..c20a6191 100644 --- a/generator/classic/generator.go +++ b/generator/classic/generator.go @@ -624,8 +624,26 @@ func new{{ .GoName }}CreateCmd(ctx *registry.CLIContext) *cobra.Command { {{- end }} {{ if .IsConfigProfile }} bodyBytes = normalizeClassicProfilePayloadsForSend(bodyBytes) -{{ end -}} resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/{{ .Path }}/{{ idPath . }}/0", bytes.NewReader(bodyBytes)) + if err != nil { + return err + } + defer resp.Body.Close() + respBody, readErr := io.ReadAll(resp.Body) + if readErr != nil { + return readErr + } + verifyClassicProfileStored(reqCtx, ctx.Client, "{{ .Path }}", classicCreatedResourceID(respBody), bodyBytes) + return ctx.Output.PrintRaw(respBody) +{{ else -}} + resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/{{ .Path }}/{{ idPath . }}/0", bytes.NewReader(bodyBytes)) + if err != nil { + return err + } + defer resp.Body.Close() + + return ctx.Output.PrintResponse(resp) +{{ end -}} {{ else }} var body io.Reader stat, _ := os.Stdin.Stat() @@ -636,13 +654,13 @@ func new{{ .GoName }}CreateCmd(ctx *registry.CLIContext) *cobra.Command { } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/{{ .Path }}/{{ idPath . }}/0", body) -{{ end }} if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) +{{ end }} }, } {{ if .FileFields }} @@ -811,10 +829,16 @@ func new{{ .GoName }}UpdateCmd(ctx *registry.CLIContext) *cobra.Command { bodyBytes = injectClassicProfilePayloadUUIDs(bodyBytes, existingPayload) bodyBytes = injectClassicRedeployOnUpdate(bodyBytes) bodyBytes = normalizeClassicProfilePayloadsForSend(bodyBytes) -{{ end }} path := fmt.Sprintf("/JSSResource/{{ .Path }}/{{ idPath . }}/%s", url.PathEscape(resolvedID)) resp, err := ctx.Client.Do(reqCtx, "PUT", path, bytes.NewReader(bodyBytes)) + if err == nil { + verifyClassicProfileStored(reqCtx, ctx.Client, "{{ .Path }}", resolvedID, bodyBytes) + } +{{ else }} + path := fmt.Sprintf("/JSSResource/{{ .Path }}/{{ idPath . }}/%s", url.PathEscape(resolvedID)) + resp, err := ctx.Client.Do(reqCtx, "PUT", path, bytes.NewReader(bodyBytes)) +{{ end }} {{ else }} var body io.Reader stat, _ := os.Stdin.Stat() @@ -1245,7 +1269,19 @@ If not, a new resource is created.` + "`" + `, } {{ if .IsConfigProfile }} data = normalizeClassicProfilePayloadsForSend(data) -{{ end -}} + resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/{{ .Path }}/{{ idPath . }}/0", bytes.NewReader(data)) + if err != nil { + return err + } + defer resp.Body.Close() + respBody, readErr := io.ReadAll(resp.Body) + if readErr != nil { + return readErr + } + verifyClassicProfileStored(reqCtx, ctx.Client, "{{ .Path }}", classicCreatedResourceID(respBody), data) + fmt.Fprintf(os.Stderr, "Created {{ .Singular }} %q\n", name) + return ctx.Output.PrintRaw(respBody) +{{ else -}} resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/{{ .Path }}/{{ idPath . }}/0", bytes.NewReader(data)) if err != nil { return err @@ -1253,6 +1289,7 @@ If not, a new resource is created.` + "`" + `, defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Created {{ .Singular }} %q\n", name) return ctx.Output.PrintResponse(resp) +{{ end -}} {{ end }} } @@ -1295,12 +1332,20 @@ If not, a new resource is created.` + "`" + `, data = injectClassicProfilePayloadUUIDs(data, existingPayload) data = injectClassicRedeployOnUpdate(data) data = normalizeClassicProfilePayloadsForSend(data) -{{ end }} + updatePath := fmt.Sprintf("/JSSResource/{{ .Path }}/{{ idPath . }}/%s", url.PathEscape(id)) resp, err := ctx.Client.Do(reqCtx, "PUT", updatePath, bytes.NewReader(data)) if err != nil { return err } + verifyClassicProfileStored(reqCtx, ctx.Client, "{{ .Path }}", id, data) +{{ else }} + updatePath := fmt.Sprintf("/JSSResource/{{ .Path }}/{{ idPath . }}/%s", url.PathEscape(id)) + resp, err := ctx.Client.Do(reqCtx, "PUT", updatePath, bytes.NewReader(data)) + if err != nil { + return err + } +{{ end }} defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced {{ .Singular }} %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) @@ -1709,21 +1754,25 @@ func stripCDATASections(data []byte) []byte { return []byte(b.String()) } -// normalizeClassicProfilePayloadsForSend rewrites the element into -// the one wire format the Classic API stores correctly, immediately before -// the request is sent. The server does not treat payload content per the XML -// spec: it entity-decodes CDATA content once (it must decode zero times) and -// text content twice (it must decode once) — PI-827. A profile whose plist -// contains any entity ("&", "<", ...) therefore reaches the database -// as invalid XML and the write fails with HTTP 409 "Unable to update the -// database", or worse, stores mangled. +// normalizeClassicProfilePayloadsForSend rewrites the element +// into the only wire form the Classic API accepts for entity-bearing +// plists, immediately before the request is sent: the true plist inside a +// single CDATA section, "]]>" entity-guarded, with every "&" escaped once. +// Text-form payloads (e.g. a GET/backup response piped back in) are +// entity-decoded once to recover the plist first. Bodies without a +// non-empty element are returned unchanged. // -// Normalization: recover the true plist (CDATA content is already true form; -// text content — e.g. a GET/backup response piped back in — is entity-decoded -// once), then guard "]]>" as "]]>" and escape every "&" once. The server's -// single CDATA decode pass exactly inverts the escape, whatever mix of -// entities the payload contains. Bodies without a non-empty -// element are returned unchanged. +// Why the escape (PI-827, wire-verified 2026-07-30 on two tenants): the +// server validates payload content after one entity decode and 409s +// ("Unable to update the database") when that yields a bare "&" or "<" — +// so spec-correct raw CDATA is rejected for ANY plist containing "&" +// or "<", whatever the payload types. Storage is then per-payload-type: +// fragments of types the server re-renders (com.apple.ManagedClient +// .preferences custom settings, notification settings) are entity-decoded +// once — the escape stores them byte-exact — while other payload types' +// fragments are stored verbatim, keeping the extra layer. Profiles with +// "&"/"<" in values of those types cannot be stored faithfully by any +// client (verifyClassicProfileStored warns after the write). func normalizeClassicProfilePayloadsForSend(body []byte) []byte { const openTag = "" s := string(body) @@ -1762,6 +1811,66 @@ func normalizeClassicProfilePayloadsForSend(body []byte) []byte { inner = strings.ReplaceAll(inner, "&", "&") return []byte(s[:ci] + "" + s[restAt:]) } + +// classicProfilePayloadFromBody returns the true plist inside the request +// body's CDATA block, or nil. The block carries the normalizer's +// escaped form (every "&" escaped once); undoing that single escape — our +// own, exactly invertible — recovers the submitted plist for comparison +// against what the server stored. +func classicProfilePayloadFromBody(body []byte) []byte { + s := string(body) + si := strings.Index(s, "") + if ei < 0 { + return nil + } + return []byte(strings.ReplaceAll(s[ci:ci+ei], "&", "&")) +} + +// classicCreatedResourceID extracts the numeric from a Classic create +// response body, or "" when absent. +func classicCreatedResourceID(respBody []byte) string { + s := string(respBody) + si := strings.Index(s, "") + if si < 0 { + return "" + } + ei := strings.Index(s[si:], "") + if ei < 0 { + return "" + } + return s[si+len("") : si+ei] +} + +// verifyClassicProfileStored fetches the stored payload after a successful +// write and warns on stderr when the server did not store the submitted +// values faithfully. This is reachable for profiles that mix payload types +// with entities on the source-level side (PI-827) — no wire form can store +// those correctly, so surfacing the exact paths is the only honest +// behaviour. Best-effort: silent on any verification failure. +func verifyClassicProfileStored(ctx context.Context, client registry.HTTPClient, apiPath, id string, sentBody []byte) { + intended := classicProfilePayloadFromBody(sentBody) + if len(intended) == 0 || id == "" { + return + } + stored := fetchClassicProfilePayloadPlist(ctx, client, apiPath, id) + if len(stored) == 0 { + return + } + diffs, err := profileconvert.DiffPayloadValues(intended, stored) + if err != nil || len(diffs) == 0 { + return + } + total := len(diffs) + if total > 5 { + diffs = append(diffs[:5], "…") + } + fmt.Fprintf(os.Stderr, "warning: the server stored %d payload value(s) differently than submitted (Jamf PI-827 — no wire format stores this mix of payload types faithfully): %s\n", total, strings.Join(diffs, ", ")) +} {{ end }} {{ if anyClassicFileFields . }} // classicFileFieldSpec describes one Classic XML field sourced from a local file or in-memory bytes. @@ -1815,6 +1924,15 @@ func injectClassicFileFields(body []byte, rootName string, specs []classicFileFi } var inner string if s.Encoding == "xml-cdata" { + if profileconvert.IsSignedProfile(data) { + // CMS-signed mobileconfig — extract the inner plist. The + // signature cannot survive this API anyway: the server + // re-serialises the plist on ingest. + if unsigned, err := profileconvert.ExtractSignedProfile(data); err == nil { + fmt.Fprintln(os.Stderr, "note: input is a signed mobileconfig; uploading the inner profile (the Classic API cannot preserve the signature)") + data = unsigned + } + } if bytes.HasPrefix(data, []byte("bplist0")) { // Binary plist — convert to XML so it can be embedded at all. if normalized, err := profileconvert.NormalizeXML(data); err == nil { diff --git a/go.mod b/go.mod index 7777bd89..1e9c53f0 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/hashicorp/go-retryablehttp v0.7.8 github.com/iancoleman/strcase v0.3.0 github.com/modelcontextprotocol/go-sdk v1.6.1 + github.com/smallstep/pkcs7 v0.2.3 github.com/spf13/cobra v1.10.2 github.com/spf13/pflag v1.0.10 github.com/zalando/go-keyring v0.2.8 diff --git a/go.sum b/go.sum index 89ed9e87..df2bc574 100644 --- a/go.sum +++ b/go.sum @@ -69,6 +69,8 @@ github.com/segmentio/asm v1.1.3 h1:WM03sfUOENvvKexOLp+pCqgb/WDjsi7EK8gIsICtzhc= github.com/segmentio/asm v1.1.3/go.mod h1:Ld3L4ZXGNcSLRg4JBsZ3//1+f/TjYl0Mzen/DQy1EJg= github.com/segmentio/encoding v0.5.4 h1:OW1VRern8Nw6ITAtwSZ7Idrl3MXCFwXHPgqESYfvNt0= github.com/segmentio/encoding v0.5.4/go.mod h1:HS1ZKa3kSN32ZHVZ7ZLPLXWvOVIiZtyJnO1gPH1sKt0= +github.com/smallstep/pkcs7 v0.2.3 h1:bhoQ3TeZmdoXTatcwxCbk+FMcdsyr0gYrrW2Xq2qr+s= +github.com/smallstep/pkcs7 v0.2.3/go.mod h1:7STkdKhZaZe4xNEXTtY4j1NGeST1gYM4GA40kC5iqr8= github.com/spf13/cobra v1.10.2 h1:DMTTonx5m65Ic0GOoRY2c16WCbHxOOw6xxezuLaBpcU= github.com/spf13/cobra v1.10.2/go.mod h1:7C1pvHqHw5A4vrJfjNwvOdzYu0Gml16OCs2GRiTUUS4= github.com/spf13/pflag v1.0.9/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= diff --git a/internal/commands/pro/generated/classic_account_groups.go b/internal/commands/pro/generated/classic_account_groups.go index a156bda0..acf337bd 100644 --- a/internal/commands/pro/generated/classic_account_groups.go +++ b/internal/commands/pro/generated/classic_account_groups.go @@ -163,13 +163,13 @@ func newClassicAccountGroupsCreateCmd(ctx *registry.CLIContext) *cobra.Command { } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/accounts/groupid/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd diff --git a/internal/commands/pro/generated/classic_account_users.go b/internal/commands/pro/generated/classic_account_users.go index 11572320..6a47013d 100644 --- a/internal/commands/pro/generated/classic_account_users.go +++ b/internal/commands/pro/generated/classic_account_users.go @@ -163,13 +163,13 @@ func newClassicAccountUsersCreateCmd(ctx *registry.CLIContext) *cobra.Command { } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/accounts/userid/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd diff --git a/internal/commands/pro/generated/classic_advanced_computer_searches.go b/internal/commands/pro/generated/classic_advanced_computer_searches.go index 6833b32a..19bf9924 100644 --- a/internal/commands/pro/generated/classic_advanced_computer_searches.go +++ b/internal/commands/pro/generated/classic_advanced_computer_searches.go @@ -174,13 +174,13 @@ func newClassicAdvancedComputerSearchesCreateCmd(ctx *registry.CLIContext) *cobr } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/advancedcomputersearches/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -501,6 +501,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced advanced_computer_search %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_advanced_mobile_device_searches.go b/internal/commands/pro/generated/classic_advanced_mobile_device_searches.go index 84b2f422..fcfb8fe1 100644 --- a/internal/commands/pro/generated/classic_advanced_mobile_device_searches.go +++ b/internal/commands/pro/generated/classic_advanced_mobile_device_searches.go @@ -174,13 +174,13 @@ func newClassicAdvancedMobileDeviceSearchesCreateCmd(ctx *registry.CLIContext) * } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/advancedmobiledevicesearches/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -501,6 +501,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced advanced_mobile_device_search %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_allowed_file_extensions.go b/internal/commands/pro/generated/classic_allowed_file_extensions.go index fbefff87..56419e08 100644 --- a/internal/commands/pro/generated/classic_allowed_file_extensions.go +++ b/internal/commands/pro/generated/classic_allowed_file_extensions.go @@ -166,13 +166,13 @@ func newClassicAllowedFileExtensionsCreateCmd(ctx *registry.CLIContext) *cobra.C } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/allowedfileextensions/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd diff --git a/internal/commands/pro/generated/classic_classes.go b/internal/commands/pro/generated/classic_classes.go index 34c86c47..183266ff 100644 --- a/internal/commands/pro/generated/classic_classes.go +++ b/internal/commands/pro/generated/classic_classes.go @@ -174,13 +174,13 @@ func newClassicClassesCreateCmd(ctx *registry.CLIContext) *cobra.Command { } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/classes/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -501,6 +501,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced class %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_computer_configs.go b/internal/commands/pro/generated/classic_computer_configs.go index 3fef3358..0af52119 100644 --- a/internal/commands/pro/generated/classic_computer_configs.go +++ b/internal/commands/pro/generated/classic_computer_configs.go @@ -174,13 +174,13 @@ func newClassicComputerConfigsCreateCmd(ctx *registry.CLIContext) *cobra.Command } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/computerconfigurations/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -501,6 +501,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced computer_configuration %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_computer_ext_attrs.go b/internal/commands/pro/generated/classic_computer_ext_attrs.go index 910d68d8..57fb66d2 100644 --- a/internal/commands/pro/generated/classic_computer_ext_attrs.go +++ b/internal/commands/pro/generated/classic_computer_ext_attrs.go @@ -174,13 +174,13 @@ func newClassicComputerExtAttrsCreateCmd(ctx *registry.CLIContext) *cobra.Comman } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/computerextensionattributes/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -501,6 +501,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced computer_extension_attribute %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_computer_groups.go b/internal/commands/pro/generated/classic_computer_groups.go index 0579c151..c7cad22d 100644 --- a/internal/commands/pro/generated/classic_computer_groups.go +++ b/internal/commands/pro/generated/classic_computer_groups.go @@ -174,13 +174,13 @@ func newClassicComputerGroupsCreateCmd(ctx *registry.CLIContext) *cobra.Command } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/computergroups/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -501,6 +501,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced computer_group %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_computer_invitations.go b/internal/commands/pro/generated/classic_computer_invitations.go index cecb6a47..532ac874 100644 --- a/internal/commands/pro/generated/classic_computer_invitations.go +++ b/internal/commands/pro/generated/classic_computer_invitations.go @@ -173,13 +173,13 @@ func newClassicComputerInvitationsCreateCmd(ctx *registry.CLIContext) *cobra.Com } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/computerinvitations/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd diff --git a/internal/commands/pro/generated/classic_directory_bindings.go b/internal/commands/pro/generated/classic_directory_bindings.go index 1e3029eb..980d3229 100644 --- a/internal/commands/pro/generated/classic_directory_bindings.go +++ b/internal/commands/pro/generated/classic_directory_bindings.go @@ -174,13 +174,13 @@ func newClassicDirectoryBindingsCreateCmd(ctx *registry.CLIContext) *cobra.Comma } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/directorybindings/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -501,6 +501,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced directory_binding %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_disk_encryption_configs.go b/internal/commands/pro/generated/classic_disk_encryption_configs.go index 02673e62..32a15ec4 100644 --- a/internal/commands/pro/generated/classic_disk_encryption_configs.go +++ b/internal/commands/pro/generated/classic_disk_encryption_configs.go @@ -174,13 +174,13 @@ func newClassicDiskEncryptionConfigsCreateCmd(ctx *registry.CLIContext) *cobra.C } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/diskencryptionconfigurations/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -501,6 +501,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced disk_encryption_configuration %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_distribution_points.go b/internal/commands/pro/generated/classic_distribution_points.go index 8c2c801a..c362a39b 100644 --- a/internal/commands/pro/generated/classic_distribution_points.go +++ b/internal/commands/pro/generated/classic_distribution_points.go @@ -174,13 +174,13 @@ func newClassicDistributionPointsCreateCmd(ctx *registry.CLIContext) *cobra.Comm } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/distributionpoints/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -501,6 +501,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced distribution_point %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_dock_items.go b/internal/commands/pro/generated/classic_dock_items.go index 04f82a4f..0a099790 100644 --- a/internal/commands/pro/generated/classic_dock_items.go +++ b/internal/commands/pro/generated/classic_dock_items.go @@ -174,13 +174,13 @@ func newClassicDockItemsCreateCmd(ctx *registry.CLIContext) *cobra.Command { } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/dockitems/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -501,6 +501,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced dock_item %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_ebooks.go b/internal/commands/pro/generated/classic_ebooks.go index f71cee8e..7742aa4f 100644 --- a/internal/commands/pro/generated/classic_ebooks.go +++ b/internal/commands/pro/generated/classic_ebooks.go @@ -180,13 +180,13 @@ func newClassicEbooksCreateCmd(ctx *registry.CLIContext) *cobra.Command { } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/ebooks/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -507,6 +507,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced ebook %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_ibeacons.go b/internal/commands/pro/generated/classic_ibeacons.go index ba86ecca..05c5a663 100644 --- a/internal/commands/pro/generated/classic_ibeacons.go +++ b/internal/commands/pro/generated/classic_ibeacons.go @@ -174,13 +174,13 @@ func newClassicIbeaconsCreateCmd(ctx *registry.CLIContext) *cobra.Command { } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/ibeacons/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -501,6 +501,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced ibeacon %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_jwt_configs.go b/internal/commands/pro/generated/classic_jwt_configs.go index c56d10b9..5563ee99 100644 --- a/internal/commands/pro/generated/classic_jwt_configs.go +++ b/internal/commands/pro/generated/classic_jwt_configs.go @@ -154,13 +154,13 @@ func newClassicJwtConfigsCreateCmd(ctx *registry.CLIContext) *cobra.Command { } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/jsonwebtokenconfigurations/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd diff --git a/internal/commands/pro/generated/classic_ldap_servers.go b/internal/commands/pro/generated/classic_ldap_servers.go index 53829c8f..50fb288b 100644 --- a/internal/commands/pro/generated/classic_ldap_servers.go +++ b/internal/commands/pro/generated/classic_ldap_servers.go @@ -174,13 +174,13 @@ func newClassicLdapServersCreateCmd(ctx *registry.CLIContext) *cobra.Command { } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/ldapservers/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -501,6 +501,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced ldap_server %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_licensed_software.go b/internal/commands/pro/generated/classic_licensed_software.go index 6ed42859..cfc7f66b 100644 --- a/internal/commands/pro/generated/classic_licensed_software.go +++ b/internal/commands/pro/generated/classic_licensed_software.go @@ -174,13 +174,13 @@ func newClassicLicensedSoftwareCreateCmd(ctx *registry.CLIContext) *cobra.Comman } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/licensedsoftware/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -501,6 +501,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced licensed_software %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_mac_apps.go b/internal/commands/pro/generated/classic_mac_apps.go index 733dae5a..fa536527 100644 --- a/internal/commands/pro/generated/classic_mac_apps.go +++ b/internal/commands/pro/generated/classic_mac_apps.go @@ -194,13 +194,13 @@ func newClassicMacAppsCreateCmd(ctx *registry.CLIContext) *cobra.Command { return err } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/macapplications/id/0", bytes.NewReader(bodyBytes)) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } @@ -598,6 +598,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced mac_application %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_macos_config_profiles.go b/internal/commands/pro/generated/classic_macos_config_profiles.go index 295f0b3a..195a12cd 100644 --- a/internal/commands/pro/generated/classic_macos_config_profiles.go +++ b/internal/commands/pro/generated/classic_macos_config_profiles.go @@ -218,13 +218,17 @@ func newClassicMacosConfigProfilesCreateCmd(ctx *registry.CLIContext) *cobra.Com bodyBytes = normalizeClassicProfilePayloadsForSend(bodyBytes) resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/osxconfigurationprofiles/id/0", bytes.NewReader(bodyBytes)) - if err != nil { return err } defer resp.Body.Close() + respBody, readErr := io.ReadAll(resp.Body) + if readErr != nil { + return readErr + } + verifyClassicProfileStored(reqCtx, ctx.Client, "osxconfigurationprofiles", classicCreatedResourceID(respBody), bodyBytes) + return ctx.Output.PrintRaw(respBody) - return ctx.Output.PrintResponse(resp) }, } @@ -318,6 +322,9 @@ func newClassicMacosConfigProfilesUpdateCmd(ctx *registry.CLIContext) *cobra.Com path := fmt.Sprintf("/JSSResource/osxconfigurationprofiles/id/%s", url.PathEscape(resolvedID)) resp, err := ctx.Client.Do(reqCtx, "PUT", path, bytes.NewReader(bodyBytes)) + if err == nil { + verifyClassicProfileStored(reqCtx, ctx.Client, "osxconfigurationprofiles", resolvedID, bodyBytes) + } if err != nil { return err @@ -619,8 +626,13 @@ If not, a new resource is created.`, return err } defer resp.Body.Close() + respBody, readErr := io.ReadAll(resp.Body) + if readErr != nil { + return readErr + } + verifyClassicProfileStored(reqCtx, ctx.Client, "osxconfigurationprofiles", classicCreatedResourceID(respBody), data) fmt.Fprintf(os.Stderr, "Created os_x_configuration_profile %q\n", name) - return ctx.Output.PrintResponse(resp) + return ctx.Output.PrintRaw(respBody) } @@ -652,6 +664,8 @@ If not, a new resource is created.`, if err != nil { return err } + verifyClassicProfileStored(reqCtx, ctx.Client, "osxconfigurationprofiles", id, data) + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced os_x_configuration_profile %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_mobile_apps.go b/internal/commands/pro/generated/classic_mobile_apps.go index 128e70e1..f0c5c02c 100644 --- a/internal/commands/pro/generated/classic_mobile_apps.go +++ b/internal/commands/pro/generated/classic_mobile_apps.go @@ -194,13 +194,13 @@ func newClassicMobileAppsCreateCmd(ctx *registry.CLIContext) *cobra.Command { return err } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/mobiledeviceapplications/id/0", bytes.NewReader(bodyBytes)) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } @@ -598,6 +598,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced mobile_device_application %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_mobile_config_profiles.go b/internal/commands/pro/generated/classic_mobile_config_profiles.go index 00ca5cfa..1cdf8c88 100644 --- a/internal/commands/pro/generated/classic_mobile_config_profiles.go +++ b/internal/commands/pro/generated/classic_mobile_config_profiles.go @@ -196,13 +196,17 @@ func newClassicMobileConfigProfilesCreateCmd(ctx *registry.CLIContext) *cobra.Co bodyBytes = normalizeClassicProfilePayloadsForSend(bodyBytes) resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/mobiledeviceconfigurationprofiles/id/0", bytes.NewReader(bodyBytes)) - if err != nil { return err } defer resp.Body.Close() + respBody, readErr := io.ReadAll(resp.Body) + if readErr != nil { + return readErr + } + verifyClassicProfileStored(reqCtx, ctx.Client, "mobiledeviceconfigurationprofiles", classicCreatedResourceID(respBody), bodyBytes) + return ctx.Output.PrintRaw(respBody) - return ctx.Output.PrintResponse(resp) }, } @@ -275,6 +279,9 @@ func newClassicMobileConfigProfilesUpdateCmd(ctx *registry.CLIContext) *cobra.Co path := fmt.Sprintf("/JSSResource/mobiledeviceconfigurationprofiles/id/%s", url.PathEscape(resolvedID)) resp, err := ctx.Client.Do(reqCtx, "PUT", path, bytes.NewReader(bodyBytes)) + if err == nil { + verifyClassicProfileStored(reqCtx, ctx.Client, "mobiledeviceconfigurationprofiles", resolvedID, bodyBytes) + } if err != nil { return err @@ -548,8 +555,13 @@ If not, a new resource is created.`, return err } defer resp.Body.Close() + respBody, readErr := io.ReadAll(resp.Body) + if readErr != nil { + return readErr + } + verifyClassicProfileStored(reqCtx, ctx.Client, "mobiledeviceconfigurationprofiles", classicCreatedResourceID(respBody), data) fmt.Fprintf(os.Stderr, "Created configuration_profile %q\n", name) - return ctx.Output.PrintResponse(resp) + return ctx.Output.PrintRaw(respBody) } @@ -581,6 +593,8 @@ If not, a new resource is created.`, if err != nil { return err } + verifyClassicProfileStored(reqCtx, ctx.Client, "mobiledeviceconfigurationprofiles", id, data) + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced configuration_profile %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_mobile_device_groups.go b/internal/commands/pro/generated/classic_mobile_device_groups.go index d45a5895..97e4407d 100644 --- a/internal/commands/pro/generated/classic_mobile_device_groups.go +++ b/internal/commands/pro/generated/classic_mobile_device_groups.go @@ -174,13 +174,13 @@ func newClassicMobileDeviceGroupsCreateCmd(ctx *registry.CLIContext) *cobra.Comm } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/mobiledevicegroups/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -501,6 +501,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced mobile_device_group %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_mobile_devices.go b/internal/commands/pro/generated/classic_mobile_devices.go index d341e3bf..059bde5f 100644 --- a/internal/commands/pro/generated/classic_mobile_devices.go +++ b/internal/commands/pro/generated/classic_mobile_devices.go @@ -187,13 +187,13 @@ func newClassicMobileDevicesCreateCmd(ctx *registry.CLIContext) *cobra.Command { } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/mobiledevices/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -603,6 +603,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced mobile_device %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_mobile_invitations.go b/internal/commands/pro/generated/classic_mobile_invitations.go index 2c7ad27a..c0d4f09c 100644 --- a/internal/commands/pro/generated/classic_mobile_invitations.go +++ b/internal/commands/pro/generated/classic_mobile_invitations.go @@ -166,13 +166,13 @@ func newClassicMobileInvitationsCreateCmd(ctx *registry.CLIContext) *cobra.Comma } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/mobiledeviceinvitations/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd diff --git a/internal/commands/pro/generated/classic_mobile_provisioning_profiles.go b/internal/commands/pro/generated/classic_mobile_provisioning_profiles.go index 0bb5ddeb..b6af256f 100644 --- a/internal/commands/pro/generated/classic_mobile_provisioning_profiles.go +++ b/internal/commands/pro/generated/classic_mobile_provisioning_profiles.go @@ -174,13 +174,13 @@ func newClassicMobileProvisioningProfilesCreateCmd(ctx *registry.CLIContext) *co } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/mobiledeviceprovisioningprofiles/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -501,6 +501,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced mobile_device_provisioning_profile %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_network_segments.go b/internal/commands/pro/generated/classic_network_segments.go index 62f5d2ec..84fb5f7a 100644 --- a/internal/commands/pro/generated/classic_network_segments.go +++ b/internal/commands/pro/generated/classic_network_segments.go @@ -174,13 +174,13 @@ func newClassicNetworkSegmentsCreateCmd(ctx *registry.CLIContext) *cobra.Command } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/networksegments/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -501,6 +501,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced network_segment %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_packages.go b/internal/commands/pro/generated/classic_packages.go index 50f56120..c0ff6f5e 100644 --- a/internal/commands/pro/generated/classic_packages.go +++ b/internal/commands/pro/generated/classic_packages.go @@ -174,13 +174,13 @@ func newClassicPackagesCreateCmd(ctx *registry.CLIContext) *cobra.Command { } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/packages/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -501,6 +501,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced package %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_patch_external_sources.go b/internal/commands/pro/generated/classic_patch_external_sources.go index d0a1e6af..0ddc5584 100644 --- a/internal/commands/pro/generated/classic_patch_external_sources.go +++ b/internal/commands/pro/generated/classic_patch_external_sources.go @@ -174,13 +174,13 @@ func newClassicPatchExternalSourcesCreateCmd(ctx *registry.CLIContext) *cobra.Co } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/patchexternalsources/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -501,6 +501,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced patch_external_source %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_patch_policies.go b/internal/commands/pro/generated/classic_patch_policies.go index 057a1894..89abe9f1 100644 --- a/internal/commands/pro/generated/classic_patch_policies.go +++ b/internal/commands/pro/generated/classic_patch_policies.go @@ -154,13 +154,13 @@ func newClassicPatchPoliciesCreateCmd(ctx *registry.CLIContext) *cobra.Command { } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/patchpolicies/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd diff --git a/internal/commands/pro/generated/classic_patch_titles.go b/internal/commands/pro/generated/classic_patch_titles.go index a95de20f..dc07d855 100644 --- a/internal/commands/pro/generated/classic_patch_titles.go +++ b/internal/commands/pro/generated/classic_patch_titles.go @@ -174,13 +174,13 @@ func newClassicPatchTitlesCreateCmd(ctx *registry.CLIContext) *cobra.Command { } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/patchsoftwaretitles/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -501,6 +501,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced patch_software_title %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_payload_escape_test.go b/internal/commands/pro/generated/classic_payload_escape_test.go index 41cd4c87..0f3bbba9 100644 --- a/internal/commands/pro/generated/classic_payload_escape_test.go +++ b/internal/commands/pro/generated/classic_payload_escape_test.go @@ -9,96 +9,85 @@ import ( // ─── normalizeClassicProfilePayloadsForSend ─────────────────────────────────── // -// The Classic API does not treat content per the XML spec: it -// entity-decodes CDATA content once and text content twice (PI-827). The -// normalizer rewrites any payloads form into CDATA with every "&" escaped -// once, which the server's single decode pass exactly inverts. - -// serverIngest simulates what the Classic API stores for a CDATA-wrapped -// payload: one entity-decode pass over the content. -func serverIngest(t *testing.T, wire string) string { +// The Classic API validates payload content after one entity decode and +// 409s when that yields a bare "&" or "<" — so raw CDATA is rejected for +// any plist containing "&"/"<" and the escaped form is the only +// submittable one (PI-827). Storage is then per-payload-type: MCX custom +// settings and notification settings fragments are entity-decoded once +// (escape stores them byte-exact); other payload types keep the extra +// layer, which verifyClassicProfileStored surfaces as a warning. + +func cdataContent(t *testing.T, body string) string { t.Helper() const openMark = "" - si := strings.Index(wire, openMark) - ei := strings.Index(wire, closeMark) + si := strings.Index(body, openMark) + ei := strings.LastIndex(body, closeMark) if si < 0 || ei < 0 { - t.Fatalf("wire has no CDATA payloads block: %s", wire) + t.Fatalf("body has no CDATA payloads block: %s", body) } - inner := wire[si+len(openMark) : ei] + inner := body[si+len(openMark) : ei] if strings.Contains(inner, "]]>") { - t.Fatalf("wire CDATA content contains ]]> — malformed XML: %s", inner) + t.Fatalf("CDATA content contains a raw terminator — malformed XML: %s", body) } - r := strings.NewReplacer("&", "&", "<", "<", ">", ">", """, `"`, "'", "'") - return r.Replace(inner) + return inner } -func TestNormalizePayloads_CDATA_RoundTripsThroughServerDecode(t *testing.T) { +func TestNormalizePayloads_CDATA_EscapedOnce(t *testing.T) { + // The wire form carries every "&" escaped once; the server's single + // decode of MCX-family fragments restores the plist byte-exact. plists := []string{ `R&D`, - `a < b > c "d"`, - `x & & y`, + `a < b > c`, `raw > and ' and "`, `literal &amp; entity text`, - "target.signing_time >= timestamp('2025-05-31T00:00:00Z')", } for _, p := range plists { body := `` - wire := string(normalizeClassicProfilePayloadsForSend([]byte(body))) - if got := serverIngest(t, wire); got != p { - t.Errorf("server would store %q, want %q", got, p) + escaped := cdataContent(t, string(normalizeClassicProfilePayloadsForSend([]byte(body)))) + if decoded := strings.ReplaceAll(escaped, "&", "&"); decoded != p { + t.Errorf("server-side decode would store %q, want %q", decoded, p) } } } func TestNormalizePayloads_TextForm_ConvertedToCDATA(t *testing.T) { // A GET/backup response carries payloads as entity-escaped text. The - // normalizer must decode it once to the true plist, then re-wrap so the - // server stores the plist exactly. + // normalizer decodes it once to the true plist and re-wraps as CDATA. truePlist := `kR&D > 'x'` escapedOnce := strings.NewReplacer("&", "&", "<", "<", ">", ">").Replace(truePlist) body := `` + escapedOnce + `` - wire := string(normalizeClassicProfilePayloadsForSend([]byte(body))) - if !strings.Contains(wire, "<string>A & B & C</string>` - wire := string(normalizeClassicProfilePayloadsForSend([]byte(body))) + escaped := cdataContent(t, string(normalizeClassicProfilePayloadsForSend([]byte(body)))) want := `A & B & C` - if got := serverIngest(t, wire); got != want { - t.Errorf("server would store %q, want %q", got, want) + if decoded := strings.ReplaceAll(escaped, "&", "&"); decoded != want { + t.Errorf("server would store %q, want %q", decoded, want) } } func TestNormalizePayloads_CDATATerminatorGuarded(t *testing.T) { - // "]]>" reaching the wrapper raw would truncate the CDATA section. The - // guarded form must survive the server decode as the valid entity form. body := `a]]>b]]>` - wire := string(normalizeClassicProfilePayloadsForSend([]byte(body))) - if strings.Contains(strings.TrimSuffix(wire[strings.Index(wire, ""), "]]>") { - t.Fatalf("wire CDATA content contains raw ]]>: %s", wire) - } - if got := serverIngest(t, wire); got != `a]]>b` { - t.Errorf("server would store %q", got) + escaped := cdataContent(t, string(normalizeClassicProfilePayloadsForSend([]byte(body)))) + if decoded := strings.ReplaceAll(escaped, "&", "&"); decoded != `a]]>b` { + t.Errorf("server would store %q", decoded) } } func TestNormalizePayloads_OutsideContentUntouched(t *testing.T) { - // Ampersands outside the payloads element (e.g. in the profile name, - // already escaped once by classicXMLEscape) must not be touched. body := `R&D Profilea & b` - wire := string(normalizeClassicProfilePayloadsForSend([]byte(body))) - if !strings.Contains(wire, `R&D Profile`) || !strings.Contains(wire, `a & b`) { - t.Errorf("content outside payloads was modified: %s", wire) + got := string(normalizeClassicProfilePayloadsForSend([]byte(body))) + if !strings.Contains(got, `R&D Profile`) || !strings.Contains(got, `a & b`) { + t.Errorf("content outside payloads was modified: %s", got) } - if !strings.Contains(wire, ``) { - t.Errorf("payloads content not escaped: %s", wire) + if !strings.Contains(got, ``) { + t.Errorf("payloads content not escaped: %s", got) } } @@ -123,6 +112,29 @@ func TestNormalizePayloads_UnterminatedCDATA(t *testing.T) { } } +// ─── verify/response helpers ────────────────────────────────────────────────── + +func TestClassicProfilePayloadFromBody(t *testing.T) { + // The body carries the normalizer's escaped form; extraction undoes the + // single escape to recover the submitted plist. + body := `R&amp;D]]>` + if got := string(classicProfilePayloadFromBody([]byte(body))); got != "R&D" { + t.Errorf("got %q", got) + } + if got := classicProfilePayloadFromBody([]byte(`text`)); got != nil { + t.Errorf("text-form should return nil, got %q", got) + } +} + +func TestClassicCreatedResourceID(t *testing.T) { + if got := classicCreatedResourceID([]byte(`7105`)); got != "7105" { + t.Errorf("got %q", got) + } + if got := classicCreatedResourceID([]byte(`error`)); got != "" { + t.Errorf("want empty, got %q", got) + } +} + // ─── stripCDATASections ─────────────────────────────────────────────────────── func TestStripCDATASections_PreservesContentExactly(t *testing.T) { diff --git a/internal/commands/pro/generated/classic_policies.go b/internal/commands/pro/generated/classic_policies.go index 0cf45555..1c122963 100644 --- a/internal/commands/pro/generated/classic_policies.go +++ b/internal/commands/pro/generated/classic_policies.go @@ -180,13 +180,13 @@ func newClassicPoliciesCreateCmd(ctx *registry.CLIContext) *cobra.Command { } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/policies/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -507,6 +507,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced policy %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_printers.go b/internal/commands/pro/generated/classic_printers.go index 8013f0b0..3b89cf83 100644 --- a/internal/commands/pro/generated/classic_printers.go +++ b/internal/commands/pro/generated/classic_printers.go @@ -174,13 +174,13 @@ func newClassicPrintersCreateCmd(ctx *registry.CLIContext) *cobra.Command { } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/printers/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -501,6 +501,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced printer %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_registry.go b/internal/commands/pro/generated/classic_registry.go index ae1794e1..65d84ebd 100644 --- a/internal/commands/pro/generated/classic_registry.go +++ b/internal/commands/pro/generated/classic_registry.go @@ -416,21 +416,25 @@ func stripCDATASections(data []byte) []byte { return []byte(b.String()) } -// normalizeClassicProfilePayloadsForSend rewrites the element into -// the one wire format the Classic API stores correctly, immediately before -// the request is sent. The server does not treat payload content per the XML -// spec: it entity-decodes CDATA content once (it must decode zero times) and -// text content twice (it must decode once) — PI-827. A profile whose plist -// contains any entity ("&", "<", ...) therefore reaches the database -// as invalid XML and the write fails with HTTP 409 "Unable to update the -// database", or worse, stores mangled. +// normalizeClassicProfilePayloadsForSend rewrites the element +// into the only wire form the Classic API accepts for entity-bearing +// plists, immediately before the request is sent: the true plist inside a +// single CDATA section, "]]>" entity-guarded, with every "&" escaped once. +// Text-form payloads (e.g. a GET/backup response piped back in) are +// entity-decoded once to recover the plist first. Bodies without a +// non-empty element are returned unchanged. // -// Normalization: recover the true plist (CDATA content is already true form; -// text content — e.g. a GET/backup response piped back in — is entity-decoded -// once), then guard "]]>" as "]]>" and escape every "&" once. The server's -// single CDATA decode pass exactly inverts the escape, whatever mix of -// entities the payload contains. Bodies without a non-empty -// element are returned unchanged. +// Why the escape (PI-827, wire-verified 2026-07-30 on two tenants): the +// server validates payload content after one entity decode and 409s +// ("Unable to update the database") when that yields a bare "&" or "<" — +// so spec-correct raw CDATA is rejected for ANY plist containing "&" +// or "<", whatever the payload types. Storage is then per-payload-type: +// fragments of types the server re-renders (com.apple.ManagedClient +// .preferences custom settings, notification settings) are entity-decoded +// once — the escape stores them byte-exact — while other payload types' +// fragments are stored verbatim, keeping the extra layer. Profiles with +// "&"/"<" in values of those types cannot be stored faithfully by any +// client (verifyClassicProfileStored warns after the write). func normalizeClassicProfilePayloadsForSend(body []byte) []byte { const openTag = "" s := string(body) @@ -470,6 +474,66 @@ func normalizeClassicProfilePayloadsForSend(body []byte) []byte { return []byte(s[:ci] + "" + s[restAt:]) } +// classicProfilePayloadFromBody returns the true plist inside the request +// body's CDATA block, or nil. The block carries the normalizer's +// escaped form (every "&" escaped once); undoing that single escape — our +// own, exactly invertible — recovers the submitted plist for comparison +// against what the server stored. +func classicProfilePayloadFromBody(body []byte) []byte { + s := string(body) + si := strings.Index(s, "") + if ei < 0 { + return nil + } + return []byte(strings.ReplaceAll(s[ci:ci+ei], "&", "&")) +} + +// classicCreatedResourceID extracts the numeric from a Classic create +// response body, or "" when absent. +func classicCreatedResourceID(respBody []byte) string { + s := string(respBody) + si := strings.Index(s, "") + if si < 0 { + return "" + } + ei := strings.Index(s[si:], "") + if ei < 0 { + return "" + } + return s[si+len("") : si+ei] +} + +// verifyClassicProfileStored fetches the stored payload after a successful +// write and warns on stderr when the server did not store the submitted +// values faithfully. This is reachable for profiles that mix payload types +// with entities on the source-level side (PI-827) — no wire form can store +// those correctly, so surfacing the exact paths is the only honest +// behaviour. Best-effort: silent on any verification failure. +func verifyClassicProfileStored(ctx context.Context, client registry.HTTPClient, apiPath, id string, sentBody []byte) { + intended := classicProfilePayloadFromBody(sentBody) + if len(intended) == 0 || id == "" { + return + } + stored := fetchClassicProfilePayloadPlist(ctx, client, apiPath, id) + if len(stored) == 0 { + return + } + diffs, err := profileconvert.DiffPayloadValues(intended, stored) + if err != nil || len(diffs) == 0 { + return + } + total := len(diffs) + if total > 5 { + diffs = append(diffs[:5], "…") + } + fmt.Fprintf(os.Stderr, "warning: the server stored %d payload value(s) differently than submitted (Jamf PI-827 — no wire format stores this mix of payload types faithfully): %s\n", total, strings.Join(diffs, ", ")) +} + // classicFileFieldSpec describes one Classic XML field sourced from a local file or in-memory bytes. type classicFileFieldSpec struct { FilePath string // user-supplied path (empty = skip when FileBytes also nil) @@ -521,6 +585,15 @@ func injectClassicFileFields(body []byte, rootName string, specs []classicFileFi } var inner string if s.Encoding == "xml-cdata" { + if profileconvert.IsSignedProfile(data) { + // CMS-signed mobileconfig — extract the inner plist. The + // signature cannot survive this API anyway: the server + // re-serialises the plist on ingest. + if unsigned, err := profileconvert.ExtractSignedProfile(data); err == nil { + fmt.Fprintln(os.Stderr, "note: input is a signed mobileconfig; uploading the inner profile (the Classic API cannot preserve the signature)") + data = unsigned + } + } if bytes.HasPrefix(data, []byte("bplist0")) { // Binary plist — convert to XML so it can be embedded at all. if normalized, err := profileconvert.NormalizeXML(data); err == nil { diff --git a/internal/commands/pro/generated/classic_removable_mac_addresses.go b/internal/commands/pro/generated/classic_removable_mac_addresses.go index 19fd132b..cd931944 100644 --- a/internal/commands/pro/generated/classic_removable_mac_addresses.go +++ b/internal/commands/pro/generated/classic_removable_mac_addresses.go @@ -174,13 +174,13 @@ func newClassicRemovableMacAddressesCreateCmd(ctx *registry.CLIContext) *cobra.C } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/removablemacaddresses/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -501,6 +501,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced removable_mac_address %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_restricted_software.go b/internal/commands/pro/generated/classic_restricted_software.go index 5ff0b3ca..19fca065 100644 --- a/internal/commands/pro/generated/classic_restricted_software.go +++ b/internal/commands/pro/generated/classic_restricted_software.go @@ -180,13 +180,13 @@ func newClassicRestrictedSoftwareCreateCmd(ctx *registry.CLIContext) *cobra.Comm } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/restrictedsoftware/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -507,6 +507,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced restricted_software %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_software_update_servers.go b/internal/commands/pro/generated/classic_software_update_servers.go index f7d0a413..8e10752c 100644 --- a/internal/commands/pro/generated/classic_software_update_servers.go +++ b/internal/commands/pro/generated/classic_software_update_servers.go @@ -174,13 +174,13 @@ func newClassicSoftwareUpdateServersCreateCmd(ctx *registry.CLIContext) *cobra.C } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/softwareupdateservers/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -501,6 +501,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced software_update_server %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_user_ext_attrs.go b/internal/commands/pro/generated/classic_user_ext_attrs.go index fc36bea4..6ded1c4f 100644 --- a/internal/commands/pro/generated/classic_user_ext_attrs.go +++ b/internal/commands/pro/generated/classic_user_ext_attrs.go @@ -174,13 +174,13 @@ func newClassicUserExtAttrsCreateCmd(ctx *registry.CLIContext) *cobra.Command { } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/userextensionattributes/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -501,6 +501,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced user_extension_attribute %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_user_groups.go b/internal/commands/pro/generated/classic_user_groups.go index a97fb82c..01609daa 100644 --- a/internal/commands/pro/generated/classic_user_groups.go +++ b/internal/commands/pro/generated/classic_user_groups.go @@ -174,13 +174,13 @@ func newClassicUserGroupsCreateCmd(ctx *registry.CLIContext) *cobra.Command { } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/usergroups/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -501,6 +501,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced user_group %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/commands/pro/generated/classic_vpp_accounts.go b/internal/commands/pro/generated/classic_vpp_accounts.go index 044a29b1..8a56d399 100644 --- a/internal/commands/pro/generated/classic_vpp_accounts.go +++ b/internal/commands/pro/generated/classic_vpp_accounts.go @@ -154,13 +154,13 @@ func newClassicVppAccountsCreateCmd(ctx *registry.CLIContext) *cobra.Command { } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/vppaccounts/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd diff --git a/internal/commands/pro/generated/classic_vpp_assignments.go b/internal/commands/pro/generated/classic_vpp_assignments.go index 296aa7e1..a43c7895 100644 --- a/internal/commands/pro/generated/classic_vpp_assignments.go +++ b/internal/commands/pro/generated/classic_vpp_assignments.go @@ -161,13 +161,13 @@ func newClassicVppAssignmentsCreateCmd(ctx *registry.CLIContext) *cobra.Command } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/vppassignments/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd diff --git a/internal/commands/pro/generated/classic_vpp_invitations.go b/internal/commands/pro/generated/classic_vpp_invitations.go index 14a5e921..98048d5b 100644 --- a/internal/commands/pro/generated/classic_vpp_invitations.go +++ b/internal/commands/pro/generated/classic_vpp_invitations.go @@ -159,13 +159,13 @@ func newClassicVppInvitationsCreateCmd(ctx *registry.CLIContext) *cobra.Command } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/vppinvitations/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd diff --git a/internal/commands/pro/generated/classic_webhooks.go b/internal/commands/pro/generated/classic_webhooks.go index 798b289d..e2f1de35 100644 --- a/internal/commands/pro/generated/classic_webhooks.go +++ b/internal/commands/pro/generated/classic_webhooks.go @@ -174,13 +174,13 @@ func newClassicWebhooksCreateCmd(ctx *registry.CLIContext) *cobra.Command { } resp, err := ctx.Client.Do(reqCtx, "POST", "/JSSResource/webhooks/id/0", body) - if err != nil { return err } defer resp.Body.Close() return ctx.Output.PrintResponse(resp) + }, } return cmd @@ -501,6 +501,7 @@ If not, a new resource is created.`, if err != nil { return err } + defer resp.Body.Close() fmt.Fprintf(os.Stderr, "Replaced webhook %q (id: %s)\n", name, id) return ctx.Output.PrintResponse(resp) diff --git a/internal/profileconvert/classic_verify.go b/internal/profileconvert/classic_verify.go new file mode 100644 index 00000000..004f09a6 --- /dev/null +++ b/internal/profileconvert/classic_verify.go @@ -0,0 +1,145 @@ +// Copyright 2026, Jamf Software LLC + +package profileconvert + +import ( + "bytes" + "fmt" + "strings" + + "github.com/smallstep/pkcs7" + "howett.net/plist" +) + +// IsSignedProfile reports whether data looks like a CMS/DER-signed +// mobileconfig (ASN.1 SEQUENCE header) rather than plist XML or a binary +// plist. A false positive is harmless — ExtractSignedProfile fails cleanly +// and callers fall back to the original bytes. +func IsSignedProfile(data []byte) bool { + if len(data) < 2 || data[0] != 0x30 { + return false + } + switch data[1] { + case 0x80, 0x81, 0x82, 0x83, 0x84: + return true + } + return false +} + +// ExtractSignedProfile returns the inner plist of a CMS-signed (DER/BER) +// mobileconfig. The signature cannot survive a Classic API upload anyway — +// the server re-serialises the plist on ingest — so callers extract and +// send the content, surfacing a note to the user. +func ExtractSignedProfile(data []byte) ([]byte, error) { + p7, err := pkcs7.Parse(data) + if err != nil { + return nil, fmt.Errorf("parsing CMS envelope: %w", err) + } + if len(p7.Content) == 0 { + return nil, fmt.Errorf("CMS envelope has no content") + } + return p7.Content, nil +} + +// maskedPayloadMetaKeys are profile-metadata keys the Classic API rewrites +// or injects on ingest (identifiers, org branding, enablement flags, display +// names of payloads it re-renders). Differences there are expected server +// behaviour, not content corruption, and are excluded from verification. +var maskedPayloadMetaKeys = map[string]bool{ + "PayloadUUID": true, + "PayloadIdentifier": true, + "PayloadOrganization": true, + "PayloadDisplayName": true, + "PayloadDescription": true, + "PayloadEnabled": true, + "PayloadRemovalDisallowed": true, + "PayloadScope": true, + "PayloadVersion": true, +} + +// DiffPayloadValues compares an intended mobileconfig plist against the +// server-stored form and returns the dotted paths of setting values the +// server did not store faithfully. Metadata keys the server rewrites by +// design are masked (maskedPayloadMetaKeys); string comparison ignores +// leading/trailing whitespace because the server trims value edges on +// ingest. Keys present only in the stored form (server-injected defaults) +// are ignored — only the caller's intent is checked. +func DiffPayloadValues(intended, stored []byte) ([]string, error) { + var want, got map[string]any + if _, err := plist.Unmarshal(intended, &want); err != nil { + return nil, fmt.Errorf("parsing intended plist: %w", err) + } + if _, err := plist.Unmarshal(stored, &got); err != nil { + return nil, fmt.Errorf("parsing stored plist: %w", err) + } + var diffs []string + diffPayloadValues(want, got, "", &diffs) + return diffs, nil +} + +func diffPayloadValues(intended, stored any, path string, diffs *[]string) { + switch want := intended.(type) { + case map[string]any: + got, ok := stored.(map[string]any) + if !ok { + *diffs = append(*diffs, path) + return + } + for k, v := range want { + if maskedPayloadMetaKeys[k] { + continue + } + p := k + if path != "" { + p = path + "." + k + } + s, present := got[k] + if !present { + // A missing empty container is a harmless server omission. + if isEmptyContainer(v) { + continue + } + *diffs = append(*diffs, p) + continue + } + diffPayloadValues(v, s, p, diffs) + } + case []any: + got, ok := stored.([]any) + if !ok || len(got) != len(want) { + *diffs = append(*diffs, path) + return + } + for i := range want { + diffPayloadValues(want[i], got[i], fmt.Sprintf("%s[%d]", path, i), diffs) + } + case string: + got, ok := stored.(string) + if !ok || strings.TrimSpace(got) != strings.TrimSpace(want) { + *diffs = append(*diffs, path) + } + case []byte: + got, ok := stored.([]byte) + if !ok || !bytes.Equal(got, want) { + *diffs = append(*diffs, path) + } + default: + // Numbers, bools, dates: same parser both sides, but be lenient on + // integer width differences via the printed form. + if fmt.Sprint(intended) != fmt.Sprint(stored) { + *diffs = append(*diffs, path) + } + } +} + +func isEmptyContainer(v any) bool { + switch t := v.(type) { + case map[string]any: + return len(t) == 0 + case []any: + return len(t) == 0 + case string: + return t == "" + } + return false +} diff --git a/internal/profileconvert/classic_verify_test.go b/internal/profileconvert/classic_verify_test.go new file mode 100644 index 00000000..e8c8059c --- /dev/null +++ b/internal/profileconvert/classic_verify_test.go @@ -0,0 +1,107 @@ +// Copyright 2026, Jamf Software LLC + +package profileconvert + +import ( + "os" + "strings" + "testing" +) + +func plistDoc(inner string) []byte { + return []byte(`` + inner + ``) +} + +func TestDiffPayloadValues_Identical(t *testing.T) { + a := plistDoc(`kR&D`) + diffs, err := DiffPayloadValues(a, a) + if err != nil || len(diffs) != 0 { + t.Fatalf("want no diffs, got %v (err %v)", diffs, err) + } +} + +func TestDiffPayloadValues_CorruptedValue(t *testing.T) { + intended := plistDoc(`LoginwindowTextHere is an &`) + stored := plistDoc(`LoginwindowTextHere is an &amp;`) + diffs, err := DiffPayloadValues(intended, stored) + if err != nil { + t.Fatal(err) + } + if len(diffs) != 1 || diffs[0] != "LoginwindowText" { + t.Fatalf("want [LoginwindowText], got %v", diffs) + } +} + +func TestDiffPayloadValues_MasksServerRewrittenMeta(t *testing.T) { + intended := plistDoc(`PayloadUUIDAAAPayloadDisplayNameminekv`) + stored := plistDoc(`PayloadUUIDBBBPayloadDisplayNameCustom SettingsPayloadOrganizationOrgkv`) + diffs, err := DiffPayloadValues(intended, stored) + if err != nil || len(diffs) != 0 { + t.Fatalf("meta rewrites must be masked, got %v (err %v)", diffs, err) + } +} + +func TestDiffPayloadValues_EdgeWhitespaceTrimIgnored(t *testing.T) { + intended := plistDoc(`k padded `) + stored := plistDoc(`kpadded`) + diffs, err := DiffPayloadValues(intended, stored) + if err != nil || len(diffs) != 0 { + t.Fatalf("server edge-trim must not diff, got %v (err %v)", diffs, err) + } +} + +func TestDiffPayloadValues_NestedArrayAndDict(t *testing.T) { + intended := plistDoc(`ax1`) + stored := plistDoc(`ax2`) + diffs, err := DiffPayloadValues(intended, stored) + if err != nil { + t.Fatal(err) + } + if len(diffs) != 1 || !strings.Contains(diffs[0], "a[0].x") { + t.Fatalf("want nested path diff, got %v", diffs) + } +} + +func TestDiffPayloadValues_MissingEmptyContainerIgnored(t *testing.T) { + intended := plistDoc(`AllowListkv`) + stored := plistDoc(`kv`) + diffs, err := DiffPayloadValues(intended, stored) + if err != nil || len(diffs) != 0 { + t.Fatalf("missing empty containers must not diff, got %v (err %v)", diffs, err) + } +} + +func TestIsSignedProfile(t *testing.T) { + if IsSignedProfile([]byte(``)) { + t.Fatal("XML misdetected as signed") + } + if IsSignedProfile([]byte("bplist00xyz")) { + t.Fatal("binary plist misdetected as signed") + } + if !IsSignedProfile([]byte{0x30, 0x80, 0x06, 0x09}) { + t.Fatal("BER SignedData not detected") + } + if !IsSignedProfile([]byte{0x30, 0x82, 0x0a, 0x00}) { + t.Fatal("DER SignedData not detected") + } +} + +// TestExtractSignedProfile_RealFile exercises CMS extraction against a real +// signed mobileconfig when one is available locally; skipped otherwise. +func TestExtractSignedProfile_RealFile(t *testing.T) { + path := os.Getenv("JAMF_CLI_TEST_SIGNED_PROFILE") + if path == "" { + t.Skip("set JAMF_CLI_TEST_SIGNED_PROFILE to a signed .mobileconfig to run") + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + inner, err := ExtractSignedProfile(data) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(inner), "