msd-core/tests/settings-jsonc.test.cjs

/**
 * MSD Tools Tests - settings.json JSONC (JSON with comments) support
 *
 * Validates that the installer's readSettings() correctly handles
 * settings.json files containing comments (line and block) without
 * silently overwriting them with empty objects.
 *
 * Closes: #1461
 */

const { test, describe } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('fs');
const os = require('os');
const path = require('path');

const { cleanup, mockPartialWriteThenThrow } = require('./helpers.cjs');

// ─── load real install.js exports once ───────────────────────────────────────
//
// install.js prints a banner at module-load time (outside its MSD_TEST_MODE
// guard) — silence stdout for the duration of the require() so test output
// stays clean.  The main-logic block IS gated on MSD_TEST_MODE, so no
// installer side-effects run.
//
// Guard line (bin/install.js:12287):
//   if (require.main === module && !process.env.MSD_TEST_MODE) {
//
let installExports;
{
  process.env.MSD_TEST_MODE = '1';
  const _origWrite = process.stdout.write.bind(process.stdout);
  process.stdout.write = () => true; // suppress banner
  try {
    installExports = require('../bin/install.js');
  } finally {
    process.stdout.write = _origWrite;
  }
}
const { readSettings, writeSettings, stripJsonComments } = installExports;

// ─── tests ───────────────────────────────────────────────────────────────────

describe('stripJsonComments (#1461)', () => {

  test('strips line comments', () => {
    const input = `{
  // This is a comment
  "key": "value"
}`;
    const result = JSON.parse(stripJsonComments(input));
    assert.deepStrictEqual(result, { key: 'value' });
  });

  test('strips block comments', () => {
    const input = `{
  /* Block comment */
  "key": "value"
}`;
    const result = JSON.parse(stripJsonComments(input));
    assert.deepStrictEqual(result, { key: 'value' });
  });

  test('strips multi-line block comments', () => {
    const input = `{
  /*
   * Multi-line
   * block comment
   */
  "key": "value"
}`;
    const result = JSON.parse(stripJsonComments(input));
    assert.deepStrictEqual(result, { key: 'value' });
  });

  test('preserves comments inside string values', () => {
    const input = `{
  "url": "https://example.com/path",
  "description": "Use // for line comments"
}`;
    const result = JSON.parse(stripJsonComments(input));
    assert.strictEqual(result.url, 'https://example.com/path');
    assert.strictEqual(result.description, 'Use // for line comments');
  });

  test('handles trailing commas', () => {
    const input = `{
  "a": 1,
  "b": 2,
}`;
    const result = JSON.parse(stripJsonComments(input));
    assert.deepStrictEqual(result, { a: 1, b: 2 });
  });

  test('handles inline comments after values', () => {
    const input = `{
  "timeout": 5000, // milliseconds
  "retries": 3 // max attempts
}`;
    const result = JSON.parse(stripJsonComments(input));
    assert.strictEqual(result.timeout, 5000);
    assert.strictEqual(result.retries, 3);
  });

  test('handles standard JSON (no comments) unchanged', () => {
    const input = '{"key": "value", "num": 42}';
    const result = JSON.parse(stripJsonComments(input));
    assert.deepStrictEqual(result, { key: 'value', num: 42 });
  });

  test('handles empty object', () => {
    const result = JSON.parse(stripJsonComments('{}'));
    assert.deepStrictEqual(result, {});
  });

  test('handles real-world settings.json with comments', () => {
    const input = `{
  // My configuration
  "hooks": {
    "SessionStart": [
      {
        "matcher": "", /* match all */
        "hooks": [
          {
            "type": "command",
            "command": "node ~/.claude/hooks/msd-statusline.js"
          }
        ]
      }
    ]
  },
  "statusLine": {
    "command": "node ~/.claude/hooks/msd-statusline.js",
    "refreshInterval": 10
  }
}`;
    const result = JSON.parse(stripJsonComments(input));
    assert.ok(result.hooks, 'should have hooks');
    assert.ok(result.statusLine, 'should have statusLine');
    assert.strictEqual(result.statusLine.refreshInterval, 10);
  });
});

describe('readSettings null return on malformed files (#1461)', () => {
  test('readSettings strips JSONC comments when reading a real file', (t) => {
    const tmpFile = path.join(os.tmpdir(), `msd-settings-test-jsonc-${process.pid}.json`);
    fs.writeFileSync(tmpFile, `{\n  // a comment\n  "key": "value"\n}`);
    t.after(() => fs.unlinkSync(tmpFile));
    const result = readSettings(tmpFile);
    assert.deepStrictEqual(
      result,
      { key: 'value' },
      'readSettings should use stripJsonComments so a commented file parses, not warns-as-malformed'
    );
  });

  test('readSettings returns null on truly malformed files (not empty object)', (t) => {
    const tmpFile = path.join(os.tmpdir(), `msd-settings-test-malformed-return-${process.pid}.json`);
    fs.writeFileSync(tmpFile, '{ this is not valid json');
    t.after(() => fs.unlinkSync(tmpFile));
    const result = readSettings(tmpFile);
    assert.strictEqual(result, null, 'readSettings should return null on parse failure, not empty object');
  });

  test('callers guard against null readSettings return', () => {
    const installPath = path.join(__dirname, '..', 'bin', 'install.js');
    // allow-test-rule: structural-implementation-guard (#1461) (#3545) — structural
    // assertion on internal wiring inside install()'s (~2500-line)
    // settings-configuration call sites — the
    // null-guard only manifests behaviorally deep inside a full install()
    // run, so the source-text check is the minimum-cost regression guard
    // that a caller was not added without also checking readSettings'
    // documented null return
    const content = fs.readFileSync(installPath, 'utf8');
    // Should have null guards at the settings configuration call sites
    assert.ok(
      content.includes('=== null') || content.includes('rawSettings === null'),
      'callers should check for null return from readSettings'
    );
  });
});

// ─── seam-4 (#1191): real readSettings via exported function ─────────────────
//
// These tests exercise the REAL readSettings from bin/install.js (not a
// replica), using real temp files.  The structural grep below is a secondary
// belt-and-suspenders anchoring the source text; the primary assertions are
// the behavioural ones beneath it.

describe('readSettings: JSON null coalesced to empty, malformed warns (#1191)', () => {
  test('valid JSON null coalesces to {} via the real function (early behavioral anchor)', (t) => {
    // Behavioral anchor: if someone removes the coalescing, this test catches
    // it before the more detailed behavioural test below even runs.
    const tmpFile = path.join(os.tmpdir(), `msd-settings-test-null-anchor-${process.pid}.json`);
    fs.writeFileSync(tmpFile, 'null');
    t.after(() => fs.unlinkSync(tmpFile));
    const result = readSettings(tmpFile);
    assert.deepStrictEqual(
      result,
      {},
      'install.js readSettings must coalesce valid JSON null to {} (not malformed warning)'
    );
  });

  test('valid JSON null content returns empty object with no malformed warning (real function)', () => {
    // A settings file containing literally `null` is valid JSON.
    // readSettings must treat it as empty settings ({}) — no warning emitted.
    const tmpFile = path.join(os.tmpdir(), `msd-settings-test-null-${process.pid}.json`);
    fs.writeFileSync(tmpFile, 'null');
    const warnCalls = [];
    const origWarn = console.warn;
    console.warn = (...args) => warnCalls.push(args.join(' '));
    let result;
    try {
      result = readSettings(tmpFile);
    } finally {
      console.warn = origWarn;
      fs.unlinkSync(tmpFile);
    }
    assert.deepStrictEqual(result, {}, 'JSON null must coalesce to {}');
    const malformedWarns = warnCalls.filter(w => w.includes('malformed') || w.includes('Could not parse'));
    assert.strictEqual(malformedWarns.length, 0, 'no malformed warning expected for valid JSON null');
  });

  test('malformed content returns null and emits malformed warning (real function)', () => {
    // A file containing `{ broken` is not valid JSON (even after comment-stripping).
    // readSettings must emit a malformed warning and return null.
    const tmpFile = path.join(os.tmpdir(), `msd-settings-test-broken-${process.pid}.json`);
    fs.writeFileSync(tmpFile, '{ broken');
    const warnCalls = [];
    const origWarn = console.warn;
    console.warn = (...args) => warnCalls.push(args.join(' '));
    let result;
    try {
      result = readSettings(tmpFile);
    } finally {
      console.warn = origWarn;
      fs.unlinkSync(tmpFile);
    }
    assert.strictEqual(result, null, 'malformed JSON must return null');
    const malformedWarns = warnCalls.filter(w => w.includes('malformed') || w.includes('Could not parse'));
    assert.strictEqual(malformedWarns.length, 1, 'exactly one malformed warning expected');
  });

  test('valid object content returns parsed object with no warning (real function)', () => {
    const tmpFile = path.join(os.tmpdir(), `msd-settings-test-valid-${process.pid}.json`);
    fs.writeFileSync(tmpFile, '{"hooks":{}}');
    const warnCalls = [];
    const origWarn = console.warn;
    console.warn = (...args) => warnCalls.push(args.join(' '));
    let result;
    try {
      result = readSettings(tmpFile);
    } finally {
      console.warn = origWarn;
      fs.unlinkSync(tmpFile);
    }
    assert.deepStrictEqual(result, { hooks: {} }, 'valid object must be returned as-is');
    const malformedWarns = warnCalls.filter(w => w.includes('malformed') || w.includes('Could not parse'));
    assert.strictEqual(malformedWarns.length, 0, 'no warning expected for valid JSON object');
  });

  test('absent file returns empty object with no warning (real function)', () => {
    const tmpFile = path.join(os.tmpdir(), `msd-settings-test-absent-${process.pid}.json`);
    // ensure file does NOT exist
    try { fs.unlinkSync(tmpFile); } catch { /* already absent */ }
    const warnCalls = [];
    const origWarn = console.warn;
    console.warn = (...args) => warnCalls.push(args.join(' '));
    let result;
    try {
      result = readSettings(tmpFile);
    } finally {
      console.warn = origWarn;
    }
    assert.deepStrictEqual(result, {}, 'absent file must return {}');
    assert.strictEqual(warnCalls.length, 0, 'no warning expected for absent file');
  });
});

// ─── writeSettings durability (#1874 F5) ─────────────────────────────────────
//
// Claude Code discards the ENTIRE settings file on any parse failure, so a
// truncated write costs the user every hook, permission, and statusline they
// have — not just MSD's entries. writeSettings is the sole writer of this
// surface for six runtimes, so it must never leave a partial file behind.

describe('writeSettings durability (#1874 F5)', () => {

  // The user's existing settings: entries MSD does not own and must not lose.
  const PRIOR = JSON.stringify({
    permissions: { allow: ['Bash(npm test)'] },
    statusLine: { command: '/usr/local/bin/my-statusline' },
    env: { MY_TOKEN: 'keep-me' },
  }, null, 2) + '\n';

  function withTmpDir(fn) {
    const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'msd-write-settings-'));
    try { return fn(dir); } finally { cleanup(dir); }
  }

  test('a failure mid-write leaves the previous settings file intact', (t) => {
    withTmpDir((dir) => {
      const settingsPath = path.join(dir, 'settings.json');
      fs.writeFileSync(settingsPath, PRIOR);

      // Simulate the crash window faithfully: the bytes that were written
      // before the failure DO land on disk, then the call fails. A mock that
      // merely throws would pass against a non-atomic writer, proving nothing.
      t.after(mockPartialWriteThenThrow(fs, undefined, 12, {
        code: 'ENOSPC',
        message: 'ENOSPC: no space left on device',
      }));

      let threw = false;
      try {
        writeSettings(settingsPath, { hooks: { SessionStart: [] } });
      } catch {
        threw = true;
      }

      assert.ok(threw, 'the write failure must propagate, not be swallowed');
      assert.strictEqual(
        fs.readFileSync(settingsPath, 'utf8'),
        PRIOR,
        'settings.json must be byte-identical to its pre-write contents'
      );
      // The real cost of the bug: a truncated file is discarded wholesale by
      // the host, so assert the user's non-MSD entries actually survive.
      const recovered = JSON.parse(fs.readFileSync(settingsPath, 'utf8'));
      assert.deepStrictEqual(recovered.permissions.allow, ['Bash(npm test)']);
      assert.strictEqual(recovered.env.MY_TOKEN, 'keep-me');
    });
  });

  test('a failure mid-write leaves no temp file behind', (t) => {
    withTmpDir((dir) => {
      const settingsPath = path.join(dir, 'settings.json');
      fs.writeFileSync(settingsPath, PRIOR);

      t.after(mockPartialWriteThenThrow(fs, undefined, 12, {
        code: 'EIO',
        message: 'simulated mid-write failure',
      }));
      try {
        writeSettings(settingsPath, { hooks: {} });
      } catch { /* expected */ }

      assert.deepStrictEqual(
        fs.readdirSync(dir).sort(),
        ['settings.json'],
        'no .tmp-* residue may survive a failed write'
      );
    });
  });

  test('a successful write produces the same bytes as before (format contract)', () => {
    withTmpDir((dir) => {
      const settingsPath = path.join(dir, 'settings.json');
      const settings = { hooks: { SessionStart: [{ hooks: [{ command: 'x' }] }] }, env: { A: '1' } };

      writeSettings(settingsPath, settings);

      // Two-space indent + trailing newline is the on-disk contract other
      // tooling (and users' diffs) depend on; atomicity must not disturb it.
      assert.strictEqual(
        fs.readFileSync(settingsPath, 'utf8'),
        JSON.stringify(settings, null, 2) + '\n'
      );
      assert.deepStrictEqual(readSettings(settingsPath), settings, 'must round-trip through readSettings');
      assert.deepStrictEqual(fs.readdirSync(dir), ['settings.json'], 'no temp residue on success');
    });
  });

  test('hardened permissions survive the rewrite', () => {
    withTmpDir((dir) => {
      const settingsPath = path.join(dir, 'settings.json');
      fs.writeFileSync(settingsPath, PRIOR);
      // 0o600 is the hardened-secrets posture: settings.json can carry env
      // tokens, and rename() would otherwise swap in a umask-default inode.
      fs.chmodSync(settingsPath, 0o600);

      writeSettings(settingsPath, { hooks: {}, env: { MY_TOKEN: 'keep-me' } });

      assert.deepStrictEqual(
        readSettings(settingsPath),
        { hooks: {}, env: { MY_TOKEN: 'keep-me' } },
        'the write must land through a chmod-hardened target on every OS'
      );
      if (process.platform !== 'win32') {
        assert.strictEqual(
          fs.statSync(settingsPath).mode & 0o7777,
          0o600,
          'a pre-existing non-default mode must survive the temp+rename write'
        );
      }
    });
  });

  test('the temp file is created exclusively — a pre-planted symlink is not followed', { skip: process.platform === 'win32' }, (t) => {
    withTmpDir((dir) => {
      const settingsPath = path.join(dir, 'settings.json');
      fs.writeFileSync(settingsPath, PRIOR);
      const victimPath = path.join(dir, 'victim');
      fs.writeFileSync(victimPath, 'victim-bytes');

      // Squat every plausible near-future temp path with a symlink to the
      // victim; an O_EXCL writer must skip them all instead of writing
      // through one.
      const planted = [];
      const origWriteFileSync = fs.writeFileSync;
      fs.writeFileSync = (target, data, options) => {
        const resolved = String(target);
        if (/\.tmp-\d+-\d+$/.test(resolved) && !fs.existsSync(resolved)) {
          try {
            fs.symlinkSync(victimPath, resolved);
            planted.push(resolved);
          } catch { /* already there */ }
        }
        return origWriteFileSync(target, data, options);
      };
      t.after(() => { fs.writeFileSync = origWriteFileSync; });
      assert.throws(
        () => writeSettings(settingsPath, { hooks: {} }),
        (e) => e.code === 'EEXIST',
        'an exclusive create must refuse every squatted temp path'
      );

      assert.strictEqual(fs.readFileSync(victimPath, 'utf8'), 'victim-bytes',
        'the symlink target must never receive the settings payload');
      assert.strictEqual(fs.readFileSync(settingsPath, 'utf8'), PRIOR,
        'the settings file must be untouched when every temp path is squatted');
      for (const link of planted) fs.unlinkSync(link);
    });
  });
});