Merge PR #607: fix(settings): string-aware trailing-comma removal in JSONC reader (fixes #595)

bin/lib/settings.js

@@ -57,9 +57,38 @@ function stripJsonComments(src) {
     if (c === '/' && next === '*') { inBlock = true; i += 2; continue; }
     out += c; i++;
   }
-  // Trailing-comma sweep — only outside strings, but stripping happened above
-  // so a regex over the comment-free output is safe.
-  return out.replace(/,(\s*[}\]])/g, '$1');
+  return stripTrailingCommas(out);
+}
+
+// ── stripTrailingCommas ────────────────────────────────────────────────────
+// Remove `,` when the next non-whitespace char is `}` or `]` — but only
+// OUTSIDE strings. The old global regex ran over string contents too and
+// silently corrupted values like `"echo ,}"` → `"echo }"` (issue #595);
+// comment-stripping does not sanitize string bodies, so a string-aware scan
+// is required here as well.
+function stripTrailingCommas(src) {
+  let out = '';
+  let i = 0;
+  const n = src.length;
+  let inString = false;
+  let stringChar = '';
+  while (i < n) {
+    const c = src[i];
+    if (inString) {
+      out += c;
+      if (c === '\\') { if (i + 1 < n) { out += src[i + 1]; i += 2; continue; } }
+      if (c === stringChar) inString = false;
+      i++; continue;
+    }
+    if (c === '"' || c === "'") { inString = true; stringChar = c; out += c; i++; continue; }
+    if (c === ',') {
+      let j = i + 1;
+      while (j < n && /\s/.test(src[j])) j++;
+      if (j < n && (src[j] === '}' || src[j] === ']')) { i++; continue; } // drop the comma
+    }
+    out += c; i++;
+  }
+  return out;
 }
 
 // ── readSettings ───────────────────────────────────────────────────────────

tests/installer/unit.settings.test.mjs

@@ -22,6 +22,27 @@ test('stripJsonComments strips // line comments', () => {
   assert.equal(out.trim(), '{"a":1}');
 });
 
+test('stripJsonComments preserves ,} and ,] inside string values (issue #595)', () => {
+  // Trailing-comma removal must be string-aware: a hook command like
+  // `echo ,}` or shell brace expansion `cp file{,.bak}` must survive.
+  const src = '{"cmd": "echo ,}", // comment\n"glob": "cp file{,]x", }';
+  const parsed = JSON.parse(SETTINGS.stripJsonComments(src));
+  assert.equal(parsed.cmd, 'echo ,}');
+  assert.equal(parsed.glob, 'cp file{,]x');
+});
+
+test('stripJsonComments still removes real trailing commas after strings', () => {
+  const src = '{"a": [1, 2, 3,], "b": {"c": 1,},}';
+  const parsed = JSON.parse(SETTINGS.stripJsonComments(src));
+  assert.deepEqual(parsed, { a: [1, 2, 3], b: { c: 1 } });
+});
+
+test('stripJsonComments handles escaped quotes before ,} in strings', () => {
+  const src = '{"cmd": "say \\",}\\" done", }';
+  const parsed = JSON.parse(SETTINGS.stripJsonComments(src));
+  assert.equal(parsed.cmd, 'say ",}" done');
+});
+
 test('stripJsonComments strips /* block */ comments', () => {
   const out = SETTINGS.stripJsonComments('{/* leading */"a":1/* mid */, "b":2}');
   assert.match(out, /"a":1/);