diff --git a/packages/cli/src/commands/office-flair-spoke.ts b/packages/cli/src/commands/office-flair-spoke.ts index 880da3e..73ec1e1 100644 --- a/packages/cli/src/commands/office-flair-spoke.ts +++ b/packages/cli/src/commands/office-flair-spoke.ts @@ -281,8 +281,34 @@ WantedBy=timers.target // ─── launchd plist generation (macOS branches) ──────────────────────────────── +/** + * Escape XML metacharacters in a string so it can be embedded inside an XML + * element value (e.g., `...` in a launchd plist) without + * breaking the surrounding markup or, worse, silently corrupting the plist + * and causing `launchctl load` to fail without a clear error. + * + * launchd's plist parser unescapes the standard 5 entities when reading, so + * values escaped here round-trip cleanly to whatever the spawned process + * sees in argv / env. + */ +function xmlEscape(s: string): string { + return s + .replace(/&/g, "&") + .replace(//g, ">") + .replace(/"/g, """) + .replace(/'/g, "'"); +} + /** * Generate a launchd plist for Harper on a macOS branch. + * + * All caller-supplied string values (label, flairDir, harperDataDir, home) + * are XML-escaped before interpolation — flagged by K&S on #290 as a + * defense-in-depth gap. Today's callers pass safe internal values but the + * `home` path comes from the remote shell via `detectRemoteHome` and a + * hostile / mistyped home with `&`, `<`, `>`, `"`, or `'` would silently + * break the plist. */ export function generateLaunchdFlairPlist( label: string, @@ -291,24 +317,57 @@ export function generateLaunchdFlairPlist( home: string, port: number = DEFAULT_FLAIR_PORT, ): string { + const eLabel = xmlEscape(label); + const eFlairDir = xmlEscape(flairDir); + const eDataDir = xmlEscape(harperDataDir); + const eHome = xmlEscape(home); + const eStdout = xmlEscape(join(home, ".tps", "logs", `flair-${label}.log`)); + const eStderr = xmlEscape(join(home, ".tps", "logs", `flair-${label}.error.log`)); + + // Build the inner JSON config with native JSON escaping, then XML-escape + // the whole string before embedding in .... launchd's + // plist parser undoes the XML layer; Harper consumes the resulting JSON. + const harperConfigJson = JSON.stringify({ + rootPath: harperDataDir, + http: { + port, + cors: true, + corsAccessList: [`http://127.0.0.1:${port}`, `http://localhost:${port}`], + }, + operationsApi: { + network: { + port: DEFAULT_HARPER_OPS_PORT, + cors: true, + corsAccessList: [ + `http://127.0.0.1:${DEFAULT_HARPER_OPS_PORT}`, + `http://localhost:${DEFAULT_HARPER_OPS_PORT}`, + ], + domainSocket: `${harperDataDir}/operations-server`, + }, + }, + mqtt: { network: { port: null }, webSocket: false }, + localStudio: { enabled: false }, + }); + const eHarperConfig = xmlEscape(harperConfigJson); + return ` Label - ${label} + ${eLabel} ProgramArguments /opt/homebrew/bin/node - ${flairDir}/node_modules/harper/dist/bin/harper.js + ${eFlairDir}/node_modules/harper/dist/bin/harper.js dev - ${flairDir} + ${eFlairDir} WorkingDirectory - ${flairDir} + ${eFlairDir} RunAtLoad @@ -323,19 +382,19 @@ export function generateLaunchdFlairPlist( 10 StandardOutPath - ${join(home, ".tps", "logs", `flair-${label}.log`)} + ${eStdout} StandardErrorPath - ${join(home, ".tps", "logs", `flair-${label}.error.log`)} + ${eStderr} EnvironmentVariables PATH /opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin HOME - ${home} + ${eHome} HARPER_SET_CONFIG - {"rootPath":"${harperDataDir}","http":{"port":${port},"cors":true,"corsAccessList":["http://127.0.0.1:${port}","http://localhost:${port}"]},"operationsApi":{"network":{"port":${DEFAULT_HARPER_OPS_PORT},"cors":true,"corsAccessList":["http://127.0.0.1:${DEFAULT_HARPER_OPS_PORT}","http://localhost:${DEFAULT_HARPER_OPS_PORT}"],"domainSocket":"${harperDataDir}/operations-server"}},"mqtt":{"network":{"port":null},"webSocket":false},"localStudio":{"enabled":false}} + ${eHarperConfig} diff --git a/packages/cli/test/office-flair-spoke.test.ts b/packages/cli/test/office-flair-spoke.test.ts index a45552a..a37bfd0 100644 --- a/packages/cli/test/office-flair-spoke.test.ts +++ b/packages/cli/test/office-flair-spoke.test.ts @@ -250,6 +250,46 @@ describe("generateLaunchdFlairPlist", () => { expect(plist).toContain("/Users/exedev"); // HOME env var expect(plist).not.toContain(homedir()); // no leak of the local home }); + + test("XML-escapes caller-supplied string values (ops-y722)", () => { + // K&S flagged on PR #290 that the plist embedded caller strings raw — + // a `&` or `<` in any interpolated value would silently corrupt the + // XML and break launchctl load with an unhelpful parser error. + // Note: harperDataDir is only used inside the JSON config (covered by + // the separate HARPER_SET_CONFIG test); these assertions cover the + // direct embedding paths. + const plist = generateLaunchdFlairPlist( + "label&withchars", + "~/.flair'apos", + "~/.harper/flair", + "/Users/", + 9926, + ); + // Raw metachars must not appear in the value positions + expect(plist).not.toContain("label&withchars"); + expect(plist).not.toContain("/Users/"); + // Encoded forms must be present + expect(plist).toContain("label&with<special>chars"); + expect(plist).toContain("/Users/<weird&user>"); + expect(plist).toContain("~/.flair'apos"); + }); + + test("XML-escapes HARPER_SET_CONFIG JSON before embedding (ops-y722)", () => { + // HARPER_SET_CONFIG is a JSON string wrapped in .... + // The inner JSON has quotes which would tear out of the XML element + // unless XML-escaped. Verify the embedded config is encoded. + const plist = generateLaunchdFlairPlist( + "ai.tpsdev.flair-test", + "~/.flair", + "~/.harper/flair", + "/Users/exedev", + 9926, + ); + // The inner JSON quotes must be encoded; the surrounding XML must be parseable + expect(plist).toContain(""rootPath""); + expect(plist).toContain(""http""); + expect(plist).not.toMatch(/\{"rootPath":/); // raw unescaped JSON would have this + }); }); // ─── 4. Extended supervision manifest round-trip ──────────────────────────────