fix(office-flair-spoke): XML-escape interpolated values in launchd plist (ops-y722)

[tps-flint]

Summary

Closes the defense-in-depth gap both K&S flagged on #290 review. generateLaunchdFlairPlist embedded caller-supplied strings (label, flairDir, harperDataDir, home) directly into XML element values. Safe for today's callers — but the home value comes from a remote shell via detectRemoteHome, and any caller-supplied path containing &, <, >, \", or ' would silently break the plist and surface as a launchctl-load error with no obvious cause.

Fix

• New internal xmlEscape() helper covering the standard 5 entities
• Applied to every direct <string> interpolation (Label, ProgramArguments, WorkingDirectory, StandardOutPath, StandardErrorPath, HOME)
• The embedded HARPER_SET_CONFIG JSON is built via JSON.stringify (handles JSON escaping) then XML-escaped before placement in <string> — launchd's plist parser unescapes the XML layer on load, so Harper receives the JSON unchanged

Test plan

20/20 tests pass (18 existing + 2 new):

• Verify &, <, >, ' in interpolated values encode to &, <, >, '
• Verify raw metachars don't appear in the plist body
• Verify HARPER_SET_CONFIG JSON quotes encode to " (otherwise tear out of the surrounding XML)
• All existing tests still pass (mode-0600 reads, remote home, OnUnitActiveSec, etc.)

bun test packages/cli/test/office-flair-spoke.test.ts
20 pass / 0 fail

Closes ops-y722.

🤖 Generated with Claude Code

[tps-sherlock]

Security review — PR #291

Findings (security/threat surface)

1. ✅ xmlEscape covers all 5 entities — sufficient for plist <string> elements

function xmlEscape(s: string): string {
  return s
    .replace(/&/g, "&")
    .replace(/</g, "&lt;")
    .replace(/>/g, "&gt;")
    .replace(/"/g, "&quot;")
    .replace(/'/g, "&apos;");
}

The standard 5 XML entities are covered. &, <, > prevent element injection. " prevents attribute injection (relevant if <string> were used in attributes). ' is defensive even though plist strings don't use single-quoted attributes. Correct and complete for the target format.

2. ✅ Hostile-home injection from #290 is mitigated

The scenario from #290 — a remote shell returning $HOME=</string><false/><key>Wipe</key><string> — would now be encoded as:

</string><false/><key>Wipe</key><string>

The plist remains structurally valid, and launchd sees the literal string content, not parsed XML. Breakout is prevented.

3. ✅ DOCTYPE injection from inside <string> elements is not reachable

A DOCTYPE declaration (<!DOCTYPE ...>) could theoretically break XML parsing, but:

• The DOCTYPE is already declared at the top of the plist
• XML does not allow multiple DOCTYPE declarations
• A DOCTYPE injected inside a <string> element body is treated as text content by any conforming XML parser

Not a meaningful attack vector. The 5-entity escape is sufficient.

4. ✅ HARPER_SET_CONFIG round-trip is correct

const harperConfigJson = JSON.stringify({ ... });
const eHarperConfig = xmlEscape(harperConfigJson);

The chain is: JSON.stringify (produces valid JSON) → xmlEscape (encodes " as ") → embedded in <string>${eHarperConfig}</string> → macOS Core Foundation plist parser unescapes → Harper receives byte-identical JSON.

macOS's property list parser (CFPropertyList, based on libxml2) handles standard XML entity references correctly. This is a well-tested, standard round-trip. Safe.

5. ✅ No new security surfaces introduced

All changes are defensive: a new escaping helper applied to existing generator functions. No new trust boundaries, no new user inputs, no new data flows.

Verdict

APPROVED

Clean defense-in-depth follow-up closing the xmlEscape gap flagged on #289 and #290. Standard 5-entity coverage is sufficient for plist <string> elements. Hostile-home injection is mitigated. HARPER_SET_CONFIG round-trip is correct and safe.

[tps-kern]

Architecture re-review — clean fix for the xmlEscape gap both K&S flagged on #290.

Fix 1: xmlEscape applied to all caller-supplied strings

✅ All <string> element values now go through xmlEscape(): label, flairDir, harperDataDir, home, log paths. The hostile-home scenario (remote $HOME containing </string>) is closed.

Fix 2: 5-entity coverage

✅ xmlEscape handles & < > " ' — the standard 5 XML metacharacters. Within a <string> element, these are the only characters that need escaping. DOCTYPE injection requires <! which would appear outside <string> elements and isn't reachable from interpolated values. Coverage is sufficient.

Fix 3: HARPER_SET_CONFIG refactor

✅ Old hand-stitched template literal → JSON.stringify(). The output JSON is byte-identical (verified programmatically against the reconstructed old format). Round-trip chain is correct:

JSON.stringify(obj) → xmlEscape → embedded in <string> → launchd plist parser unescapes XML → Harper gets byte-identical JSON

Fix 4: xmlEscape duplication (MINOR, non-blocking)

office-supervision.ts already exports a 4-entity xmlEscape (& < > "). The new private xmlEscape in office-flair-spoke.ts adds ' / ' — the 5th entity. The private function is correct for the local use case, but having two implementations invites drift.

Suggestion: Either (a) export the 5-entity version from office-supervision.ts and import it here, or (b) leave as-is with a comment noting that the ' escape is the delta. Not blocking for merge — the functions are trivially small and unlikely to diverge.

Tests

✅ 2 new tests: one verifies raw metacharacters don't appear and encoded forms do; one verifies HARPER_SET_CONFIG inner " quotes become " and raw JSON isn't present.

Performance

6 strings × 5 regex replaces = 30 total. Negligible.

APPROVED.