Pass authorization options from ENV to server/service

bin/install-service.mjs

@@ -13,6 +13,7 @@ import { spawn } from "node:child_process";
 import { fileURLToPath } from "node:url";
 import { dirname, resolve, join } from "node:path";
 import { homedir, platform } from "node:os";
+import { systemdEscape, xmlEscape } from "../proxy/helpers.mjs";
 
 const __dirname = dirname(fileURLToPath(import.meta.url));
 const TEMPLATE_DIR = resolve(__dirname, "..", "templates");
@@ -22,6 +23,8 @@ function getDefaults() {
   return {
     port: validatePort(process.env.CACHE_FIX_PROXY_PORT || "9801"),
     upstream: process.env.CACHE_FIX_PROXY_UPSTREAM || "",
+    caFile: process.env.CACHE_FIX_PROXY_CA_FILE || "",
+    rejectUnauthorized: process.env.CACHE_FIX_PROXY_REJECT_UNAUTHORIZED || "",
     debug: process.env.CACHE_FIX_DEBUG || "",
     // Hot-reload is opt-in as of v4.0.0 (#196). Capture from env at install
     // time so the operator can bake `CACHE_FIX_HOT_RELOAD=on` into the
@@ -93,10 +96,16 @@ function getPaths(plat = platform()) {
 
 function renderSystemdTemplate(template, vars) {
   const upstreamLine = vars.upstream
-    ? `Environment=CACHE_FIX_PROXY_UPSTREAM=${vars.upstream}`
+    ? `Environment=CACHE_FIX_PROXY_UPSTREAM=${systemdEscape(vars.upstream)}`
+    : "";
+  const caFileLine = vars.caFile
+    ? `Environment=CACHE_FIX_PROXY_CA_FILE=${systemdEscape(vars.caFile)}`
+    : "";
+  const rejectUnauthorizedLine = vars.rejectUnauthorized
+    ? `Environment=CACHE_FIX_PROXY_REJECT_UNAUTHORIZED=${systemdEscape(vars.rejectUnauthorized)}`
     : "";
   const debugLine = vars.debug
-    ? `Environment=CACHE_FIX_DEBUG=${vars.debug}`
+    ? `Environment=CACHE_FIX_DEBUG=${systemdEscape(vars.debug)}`
     : "";
   const hotReloadLine = vars.hotReload
     ? `Environment=CACHE_FIX_HOT_RELOAD=${vars.hotReload}`
@@ -111,6 +120,8 @@ function renderSystemdTemplate(template, vars) {
     .replaceAll("{{SERVER_PATH}}", vars.serverPath)
     .replaceAll("{{PORT}}", vars.port)
     .replaceAll("{{UPSTREAM_LINE}}", upstreamLine)
+    .replaceAll("{{CA_FILE_LINE}}", caFileLine)
+    .replaceAll("{{REJECT_UNAUTHORIZED_LINE}}", rejectUnauthorizedLine)
     .replaceAll("{{DEBUG_LINE}}", debugLine)
     .replaceAll("{{HOT_RELOAD_LINE}}", hotReloadLine)
     .replaceAll("{{REQUIRES_LINE}}", requiresLine)
@@ -121,10 +132,16 @@ function renderSystemdTemplate(template, vars) {
 
 function renderLaunchdTemplate(template, vars) {
   const upstreamPlist = vars.upstream
-    ? `        <key>CACHE_FIX_PROXY_UPSTREAM</key>\n        <string>${vars.upstream}</string>`
+    ? `        <key>CACHE_FIX_PROXY_UPSTREAM</key>\n        <string>${xmlEscape(vars.upstream)}</string>`
+    : "";
+  const caFilePlist = vars.caFile
+    ? `        <key>CACHE_FIX_PROXY_CA_FILE</key>\n        <string>${xmlEscape(vars.caFile)}</string>`
+    : "";
+  const rejectUnauthorizedPlist = vars.rejectUnauthorized
+    ? `        <key>CACHE_FIX_PROXY_REJECT_UNAUTHORIZED</key>\n        <string>${xmlEscape(vars.rejectUnauthorized)}</string>`
     : "";
   const debugPlist = vars.debug
-    ? `        <key>CACHE_FIX_DEBUG</key>\n        <string>${vars.debug}</string>`
+    ? `        <key>CACHE_FIX_DEBUG</key>\n        <string>${xmlEscape(vars.debug)}</string>`
     : "";
   const hotReloadPlist = vars.hotReload
     ? `        <key>CACHE_FIX_HOT_RELOAD</key>\n        <string>${vars.hotReload}</string>`
@@ -134,6 +151,8 @@ function renderLaunchdTemplate(template, vars) {
     .replaceAll("{{SERVER_PATH}}", vars.serverPath)
     .replaceAll("{{PORT}}", vars.port)
     .replaceAll("{{UPSTREAM_PLIST}}", upstreamPlist)
+    .replaceAll("{{CA_FILE_PLIST}}", caFilePlist)
+    .replaceAll("{{REJECT_UNAUTHORIZED_PLIST}}", rejectUnauthorizedPlist)
     .replaceAll("{{DEBUG_PLIST}}", debugPlist)
     .replaceAll("{{HOT_RELOAD_PLIST}}", hotReloadPlist)
     .replaceAll("{{WORKING_DIR}}", vars.workingDir)
@@ -186,12 +205,8 @@ async function installSystemd({ paths, defaults, force = false } = {}) {
   const rendered = renderSystemdTemplate(template, {
     node: process.execPath,
     serverPath: SERVER_PATH,
-    port: defaults.port,
-    upstream: defaults.upstream,
-    debug: defaults.debug,
-    hotReload: defaults.hotReload,
-    workingDir: defaults.workingDir,
     requires: "",
+    ...defaults,
   });
   await mkdir(paths.configDir, { recursive: true });
   await writeFile(targetPath, rendered);
@@ -286,12 +301,8 @@ async function installLaunchd({ paths, defaults, force = false } = {}) {
   const rendered = renderLaunchdTemplate(template, {
     node: process.execPath,
     serverPath: SERVER_PATH,
-    port: defaults.port,
-    upstream: defaults.upstream,
-    debug: defaults.debug,
-    hotReload: defaults.hotReload,
-    workingDir: defaults.workingDir,
     logDir: paths.logDir,
+    ...defaults,
   });
   await mkdir(paths.configDir, { recursive: true });
   await writeFile(targetPath, rendered);

docs/code-reviews/pr-189-round-4-codex.md

@@ -0,0 +1,43 @@
+# Review: PR #189
+
+Date: 2026-06-08
+Reviewed: PR #189 at `8159303bf58045561f2d6831736b2efee5bf632c`
+Round: 4
+Label applied: approved-by-codex-agent
+
+## What Is Correct
+
+`systemdEscape()` now closes the remaining round-3 gap in the helper itself. It escapes `%` before entering the quoting branch, and the quote trigger now includes bare backslashes, so a value containing `\` is forced down the quote-and-escape path instead of being emitted raw. Confirmed in `proxy/helpers.mjs:17-21`.
+
+The new helper-level regression tests cover the right behaviors: bare `%`, bare `\`, a combined `%`/space/`\`/`"` case, and an explicit ordering proof that `%` escaping happens before quote-wrapping. Confirmed in `test/proxy-helpers.test.mjs:36-83`.
+
+The renderer-level tests pin the rendered `Environment=` lines for the two concrete PR #189 regressions: percent-encoded upstream URLs and backslashes in CA-file paths. Confirmed in `test/install-service.test.mjs:93-125`.
+
+I also re-ran the empirical checks against HEAD. A helper-rendered systemd unit passed `systemd-analyze verify`, and a live `systemctl --user` oneshot unit received:
+
+- `CACHE_FIX_PROXY_UPSTREAM=https://example.com/a%20b`
+- `CACHE_FIX_PROXY_CA_FILE=/path/with\backslash.pem`
+
+For control, the raw unescaped unit still reproduced the old failures: the upstream var was dropped with `Failed to resolve specifiers ... Invalid slot`, and the CA path arrived as `/path/with<0x08>ackslash.pem`.
+
+`node --test test/proxy-helpers.test.mjs test/install-service.test.mjs` passed locally (56/56).
+
+## Blockers
+
+None.
+
+## What Needs Attention
+
+None.
+
+## Bloat / Non-Functional
+
+None.
+
+## Recommendations
+
+Approve and merge once the PR label state is updated.
+
+## Bottom Line
+
+The remaining round-3 systemd escaping gap is closed at `8159303`. The helper logic is now correct for `%` and `\`, the new regression tests cover the previously missing cases, and the live user-manager repro matches the intended behavior. This is ready for approval.

proxy/helpers.mjs

@@ -0,0 +1,30 @@
+// Escape a value for safe rendering into a systemd `Environment=KEY=VALUE` line.
+//
+// Per systemd.exec(5) Environment= and systemd.unit(5) Specifier Expansion:
+//   - Literal `%` is the specifier-expansion marker; to embed one in a value
+//     the unit file must write `%%`. Without escaping, `a%20b` is parsed as
+//     a failed `%20` specifier expansion, systemd logs "Invalid slot" and
+//     silently drops the variable (empirically reproduced 2026-06-07).
+//   - Backslash is a C-string escape inside quoted strings AND inside the
+//     Environment= value parser; `\b` becomes byte 0x08 (backspace), `\n`
+//     becomes LF, etc. To embed a literal `\` the unit must write `\\`.
+//   - `"` requires `\"` (after the backslash escape rule above).
+//   - Whitespace requires the whole value to be quoted (`"..."`).
+//
+// Order matters: escape `%` first (it produces `%%`, neither of which we
+// want to re-escape later), then handle `\` and `"` together inside the
+// quoting branch.
+export const systemdEscape = (v) => {
+  const percentEscaped = v.replace(/%/g, '%%');
+  const needsQuoting = /[\s"\\]/.test(v);
+  if (!needsQuoting) return percentEscaped;
+  return `"${percentEscaped.replace(/[\\"]/g, '\\$&')}"`;
+};
+
+export const xmlEscape = (v) => v.replace(/[&<>'"]/g, c => ({
+  '&': '&amp;',
+  '<': '&lt;',
+  '>': '&gt;',
+  "'": '&apos;',
+  '"': '&quot;'
+})[c]);

templates/cache-fix-proxy.service.template

@@ -10,6 +10,8 @@ Restart=on-failure
 RestartSec=5
 Environment=CACHE_FIX_PROXY_PORT={{PORT}}
 {{UPSTREAM_LINE}}
+{{CA_FILE_LINE}}
+{{REJECT_UNAUTHORIZED_LINE}}
 {{DEBUG_LINE}}
 {{HOT_RELOAD_LINE}}
 WorkingDirectory={{WORKING_DIR}}

templates/com.cnighswonger.cache-fix-proxy.plist.template

@@ -14,6 +14,8 @@
         <key>CACHE_FIX_PROXY_PORT</key>
         <string>{{PORT}}</string>
 {{UPSTREAM_PLIST}}
+{{CA_FILE_PLIST}}
+{{REJECT_UNAUTHORIZED_PLIST}}
 {{DEBUG_PLIST}}
 {{HOT_RELOAD_PLIST}}
     </dict>

test/install-service.test.mjs

@@ -38,6 +38,8 @@ const sampleVars = {
   serverPath: "/opt/cache-fix/proxy/server.mjs",
   port: "9801",
   upstream: "",
+  caFile: "",
+  rejectUnauthorized: "",
   debug: "",
   workingDir: "/opt/cache-fix",
   requires: "",
@@ -58,20 +60,26 @@ test("renderSystemdTemplate: omits empty optional Environment lines", async () =
   const tpl = await readFile(join(TEMPLATE_DIR, "cache-fix-proxy.service.template"), "utf-8");
   const out = renderSystemdTemplate(tpl, sampleVars);
   assert.ok(!out.includes("CACHE_FIX_PROXY_UPSTREAM"));
+  assert.ok(!out.includes("CACHE_FIX_PROXY_CA_FILE"));
+  assert.ok(!out.includes("CACHE_FIX_PROXY_REJECT_UNAUTHORIZED"));
   assert.ok(!out.includes("CACHE_FIX_DEBUG"));
   // No leftover empty placeholders
   assert.ok(!out.includes("{{"));
   assert.ok(!out.includes("}}"));
 });
 
-test("renderSystemdTemplate: includes UPSTREAM and DEBUG when set", async () => {
+test("renderSystemdTemplate: includes UPSTREAM, CA_FILE, REJECT_UNAUTHORIZED and DEBUG when set", async () => {
   const tpl = await readFile(join(TEMPLATE_DIR, "cache-fix-proxy.service.template"), "utf-8");
   const out = renderSystemdTemplate(tpl, {
     ...sampleVars,
     upstream: "http://127.0.0.1:8080",
+    caFile: "/etc/ssl/ca \" file.pem", // with space and "
+    rejectUnauthorized: "0",
     debug: "1",
   });
   assert.ok(out.includes("Environment=CACHE_FIX_PROXY_UPSTREAM=http://127.0.0.1:8080"));
+  assert.ok(out.includes("Environment=CACHE_FIX_PROXY_CA_FILE=\"/etc/ssl/ca \\\" file.pem\""));
+  assert.ok(out.includes("Environment=CACHE_FIX_PROXY_REJECT_UNAUTHORIZED=0"));
   assert.ok(out.includes("Environment=CACHE_FIX_DEBUG=1"));
 });
 
@@ -82,6 +90,40 @@ test("renderSystemdTemplate: requires line wires both Requires and After", async
   assert.ok(out.includes("After=llm-relay.service"));
 });
 
+// PR #189 regression — bare % triggers systemd specifier expansion and
+// silently drops the variable. Verified 2026-06-07 against `systemctl --user`:
+// the unit line `Environment=X=a%%20b` delivers `a%20b` to the spawned
+// process, while `Environment=X=a%20b` (unescaped) delivers an empty string
+// after a "Failed to resolve specifiers ... Invalid slot" log entry.
+test("renderSystemdTemplate: bare % in upstream URL is escaped to %% (PR #189)", async () => {
+  const tpl = await readFile(join(TEMPLATE_DIR, "cache-fix-proxy.service.template"), "utf-8");
+  const out = renderSystemdTemplate(tpl, {
+    ...sampleVars,
+    upstream: "http://10.0.0.1:8080/path%20with%20encoded",
+  });
+  assert.ok(
+    out.includes("Environment=CACHE_FIX_PROXY_UPSTREAM=http://10.0.0.1:8080/path%%20with%%20encoded"),
+    "bare % must be escaped to %% in the rendered Environment= line",
+  );
+  assert.ok(!/=http:\/\/10\.0\.0\.1:8080\/path%20/.test(out), "no unescaped %20 should appear");
+});
+
+// PR #189 regression — bare \ triggers systemd C-string unescape and
+// produces a control byte. Verified 2026-06-07: `Environment=X=/path/with\backslash.pem`
+// delivers /path/with<0x08>ackslash.pem to the process; the quoted form
+// `Environment=X="/path/with\\backslash.pem"` delivers the literal value.
+test("renderSystemdTemplate: backslash in CA file path is escaped to \\\\ (PR #189)", async () => {
+  const tpl = await readFile(join(TEMPLATE_DIR, "cache-fix-proxy.service.template"), "utf-8");
+  const out = renderSystemdTemplate(tpl, {
+    ...sampleVars,
+    caFile: "/etc/ssl/with\\backslash.pem",
+  });
+  assert.ok(
+    out.includes('Environment=CACHE_FIX_PROXY_CA_FILE="/etc/ssl/with\\\\backslash.pem"'),
+    "bare \\ must be escaped to \\\\ inside the quoted Environment= value",
+  );
+});
+
 test("renderLaunchdTemplate: substitutes core fields and renders valid plist", async () => {
   const tpl = await readFile(
     join(TEMPLATE_DIR, "com.cnighswonger.cache-fix-proxy.plist.template"),
@@ -99,7 +141,31 @@ test("renderLaunchdTemplate: substitutes core fields and renders valid plist", a
   assert.ok(!out.includes("{{"));
 });
 
-test("renderLaunchdTemplate: omits CACHE_FIX_PROXY_UPSTREAM/DEBUG when not set", async () => {
+test("renderLaunchdTemplate: includes UPSTREAM, CA_FILE, REJECT_UNAUTHORIZED and DEBUG when set", async () => {
+  const tpl = await readFile(
+    join(TEMPLATE_DIR, "com.cnighswonger.cache-fix-proxy.plist.template"),
+    "utf-8",
+  );
+  const out = renderLaunchdTemplate(tpl, {
+    ...sampleVars,
+    upstream: "http://127.0.0.1:8080",
+    caFile: "/etc/ssl/ca & < > ' \" file.pem", // with XLM spec symbols
+    rejectUnauthorized: "0",
+    debug: "1",
+    logDir: "/Users/test/Library/Logs",
+  });
+  assert.ok(out.includes("<string>com.cnighswonger.cache-fix-proxy</string>"));
+  assert.ok(out.includes("<string>/usr/local/bin/node</string>"));
+  assert.ok(out.includes("<string>/opt/cache-fix/proxy/server.mjs</string>"));
+  assert.ok(out.includes("<string>9801</string>"));
+  assert.ok(out.includes("<string>http://127.0.0.1:8080</string>"));
+  assert.ok(out.includes("<string>/etc/ssl/ca &amp; &lt; &gt; &apos; &quot; file.pem</string>"));
+  assert.ok(out.includes("<string>0</string>"));
+  assert.ok(out.includes("<string>/Users/test/Library/Logs/cache-fix-proxy.log</string>"));
+  assert.ok(!out.includes("{{"));
+});
+
+test("renderLaunchdTemplate: omits CACHE_FIX_PROXY_UPSTREAM/CA_FILE/REJECT_UNAUTHORIZED/DEBUG when not set", async () => {
   const tpl = await readFile(
     join(TEMPLATE_DIR, "com.cnighswonger.cache-fix-proxy.plist.template"),
     "utf-8",
@@ -109,6 +175,8 @@ test("renderLaunchdTemplate: omits CACHE_FIX_PROXY_UPSTREAM/DEBUG when not set",
     logDir: "/tmp/logs",
   });
   assert.ok(!out.includes("CACHE_FIX_PROXY_UPSTREAM"));
+  assert.ok(!out.includes("CACHE_FIX_PROXY_CA_FILE"));
+  assert.ok(!out.includes("CACHE_FIX_PROXY_REJECT_UNAUTHORIZED"));
   assert.ok(!out.includes("CACHE_FIX_DEBUG"));
 });
 
@@ -268,10 +336,12 @@ test("installSystemd: writes file to configDir; uninstall removes it", async ()
       healthcheckServiceFile: "cache-fix-proxy-healthcheck.service",
       healthcheckTimerFile: "cache-fix-proxy-healthcheck.timer",
     };
-    const r1 = await installSystemd({ paths, defaults: { port: "9999", upstream: "", debug: "", workingDir: "/tmp" } });
+    const r1 = await installSystemd({ paths, defaults: { port: "9999", upstream: "", caFile: "/etc/ssl/ca.pem", rejectUnauthorized: "0", debug: "", workingDir: "/tmp" } });
     assert.ok(r1.ok);
     const onDisk = await readFile(join(dir, "cache-fix-proxy.service"), "utf-8");
     assert.ok(onDisk.includes("CACHE_FIX_PROXY_PORT=9999"));
+    assert.ok(onDisk.includes("CACHE_FIX_PROXY_CA_FILE=/etc/ssl/ca.pem"));
+    assert.ok(onDisk.includes("CACHE_FIX_PROXY_REJECT_UNAUTHORIZED=0"));
 
     const r2 = await uninstallSystemd({ paths });
     assert.ok(r2.ok);