fix(installer): confine writes to the project and stop guessing on markers

Da7-Tech · Da7-Tech · commit 05dac9d05ad3 · 2026-07-16T18:35:13.000+03:00
Adversarial audit of the 0.5.2 upgrade path found the installer would write
through a pre-planted symlink (with or without --force) to a file outside
the project, report false success on a begin-marker with no end, delete user
content between a stray begin and the real block, and honor an undocumented
--update alias with force semantics.

Writes now refuse symlinked or out-of-tree destinations, malformed or
duplicated marker pairs abort untouched, every destructive dedicated-file
update saves the replaced content to &lt;file&gt;.bak, and unknown flags are
rejected. Rule body unchanged; v0.5.3.
diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json
@@ -1,6 +1,6 @@
 {
   "name": "trial",
-  "version": "0.5.2",
+  "version": "0.5.3",
   "description": "Pre-delivery evidence gate: withhold unsupported completion claims and release only verified results.",
   "author": {
     "name": "Da7-Tech",
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,36 @@
 # Changelog
 
+## 0.5.3 — 2026-07-16
+
+Installer hardening after an adversarial audit of the 0.5.2 upgrade path; no
+rule change.
+
+- **The installer now refuses symlinked destinations and paths resolving
+  outside the project.** A hostile checkout could pre-plant a destination path
+  (`.cursor/rules/trial.mdc`, `AGENTS.md`, or a parent directory) as a symlink
+  to a file elsewhere, and `npx github:Da7-Tech/trial <agent>` — with or
+  without `--force` — would overwrite the link's target. Writes are now
+  confined to the real project directory and never follow a symlink.
+- **Malformed managed markers stop the update instead of mis-firing.** A
+  `trial:begin` with no matching `trial:end` previously produced a false
+  "updated" success while installing nothing; a stray `begin` above the real
+  block silently deleted the user content between them; duplicated pairs left
+  a permanently stale second block. All three cases now abort with a clear
+  message and leave the file byte-identical.
+- **Every destructive dedicated-file update keeps a `.bak`.** The signature
+  substring test cannot distinguish an older Trial rule from a personal file
+  that merely quotes the signature phrase, so the replaced content is always
+  saved next to the destination as a one-step undo (also under `--force`).
+- **`--update` is gone; unknown flags are rejected.** It was an undocumented
+  alias of `--force` whose gentle name belied force semantics. `--force` is
+  the single, explicit escape hatch, and any unrecognized `--flag` now exits
+  with usage instead of being ignored.
+- **Known, accepted limitation:** the installer performs no version
+  comparison — re-running an *older* package over a newer installed rule
+  overwrites it (the `.bak` and your VCS are the recovery path).
+- **No behavioral rule change.** The canonical rule body is byte-identical to
+  0.5.1/0.5.2; only the installer, its tests, and version metadata changed.
+
 ## 0.5.2 — 2026-07-16
 
 Installer upgrade path for dedicated-file targets; no rule change.
diff --git a/README.md b/README.md
@@ -79,7 +79,7 @@ The full rule your agent reads is [`agents/codex/AGENTS.md`](agents/codex/AGENTS
 npx github:Da7-Tech/trial cursor
 ```
 
-Re-running upgrades in place: shared files (`AGENTS.md`, …) update between their managed markers, and dedicated rule files update when they hold a Trial rule (a same-version reinstall is a no-op). A non-Trial file already at the destination path is never clobbered — the installer refuses and points you to `--force` if you really mean to replace it.
+Re-running upgrades in place: shared files (`AGENTS.md`, …) update between their managed markers — malformed or duplicated markers make the installer stop rather than guess — and dedicated rule files update when they carry the Trial signature, with the replaced content always saved to `<file>.bak` first. A file without the signature is refused (re-run with `--force` to replace it; the `.bak` is still written), and the installer never writes through a symlink or outside the project directory.
 
 Or copy the file yourself:
 
diff --git a/SKILL.md b/SKILL.md
@@ -3,7 +3,7 @@ name: trial
 description: "A pre-delivery evidence gate for coding agents. Hold the final response as a private draft, judge every user-visible claim against fresh covering receipts, and release only verified results. Use when unsupported completion claims, shallow green tests, stale evidence, or high-risk changes must not reach the user."
 license: MIT
 metadata:
-  version: 0.5.2
+  version: 0.5.3
 ---
 
 # Trial — Pre-Delivery Evidence Gate
diff --git a/bin/install.js b/bin/install.js
@@ -44,8 +44,13 @@ function usage(code) {
 }
 
 const rawArgs = process.argv.slice(2);
-const force = rawArgs.some((a) => /^--(force|update)$/i.test(a));
-const positional = rawArgs.filter((a) => !/^--(force|update)$/i.test(a));
+const force = rawArgs.some((a) => /^--force$/i.test(a));
+const unknownFlags = rawArgs.filter((a) => /^--/.test(a) && !/^--(force|help|list)$/i.test(a));
+if (unknownFlags.length) {
+  console.error(`Unknown option "${unknownFlags[0]}".`);
+  usage(1);
+}
+const positional = rawArgs.filter((a) => !/^--force$/i.test(a));
 const arg = (positional[0] || '').toLowerCase().replace(/^--/, '');
 if (!arg || arg === 'help') usage(arg ? 0 : 1);
 if (arg === 'list') { console.log(Object.keys(TARGETS).join('\n')); process.exit(0); }
@@ -56,7 +61,33 @@ const src = fs.readFileSync(path.join(pkgRoot, srcRel), 'utf8');
 const dest = path.join(process.cwd(), destRel);
 const block = `${BEGIN}\n${src.trim()}\n${END}\n`;
 
+function fail(message) {
+  console.error(message);
+  process.exit(1);
+}
+
+// The installer must never write outside the project it is run in, and never
+// through a symlink: a hostile checkout could pre-plant the destination path
+// as a link to a file elsewhere (dotfiles, shared configs) and turn a routine
+// `npx … <agent>` into an out-of-tree overwrite.
+function assertDestinationConfined() {
+  let nearest = path.dirname(dest);
+  while (!fs.existsSync(nearest)) nearest = path.dirname(nearest);
+  const root = fs.realpathSync(process.cwd());
+  const anchor = fs.realpathSync(nearest);
+  if (anchor !== root && !anchor.startsWith(root + path.sep)) {
+    fail(`${destRel} resolves outside the project directory — refusing to write.`);
+  }
+  let stats = null;
+  try { stats = fs.lstatSync(dest); } catch (_) { /* not created yet */ }
+  if (stats && stats.isSymbolicLink()) {
+    fail(`${destRel} is a symlink — refusing to write through it. ` +
+      'Run the installer where the link points, or replace the link with a regular file.');
+  }
+}
+
 try {
+  assertDestinationConfined();
   fs.mkdirSync(path.dirname(dest), { recursive: true });
 
   if (!fs.existsSync(dest)) {
@@ -66,10 +97,20 @@ try {
     fs.writeFileSync(dest, append ? block : src);
     console.log(`Trial installed: ${destRel}`);
   } else if (append) {
-    let existing = fs.readFileSync(dest, 'utf8');
-    if (existing.includes(BEGIN)) {
-      existing = existing.replace(new RegExp(`${BEGIN}[\\s\\S]*?${END}\\n?`), block);
-      fs.writeFileSync(dest, existing);
+    const existing = fs.readFileSync(dest, 'utf8');
+    const begins = existing.split(BEGIN).length - 1;
+    const ends = existing.split(END).length - 1;
+    // A malformed or duplicated marker pair means the managed region cannot be
+    // identified safely: updating anyway would either claim success while
+    // changing nothing or eat user content straying between the wrong pair.
+    // Stop and say so instead of guessing.
+    if (begins !== ends || begins > 1 ||
+        (begins === 1 && existing.indexOf(END) < existing.indexOf(BEGIN))) {
+      fail(`${destRel} has malformed Trial markers (${begins} begin / ${ends} end) — ` +
+        'repair or remove them manually, then re-run.');
+    }
+    if (begins === 1) {
+      fs.writeFileSync(dest, existing.replace(new RegExp(`${BEGIN}[\\s\\S]*?${END}\\n?`), block));
       console.log(`Trial updated inside existing ${destRel}`);
     } else {
       fs.writeFileSync(dest, existing.trimEnd() + '\n\n' + block);
@@ -84,8 +125,14 @@ try {
     if (existing === src) {
       console.log(`Trial already up to date: ${destRel}`);
     } else if (force || existing.includes(TRIAL_SIGNATURE)) {
+      // The signature test can misfire on a personal file that merely quotes
+      // it, so every destructive update keeps the replaced content beside the
+      // destination as a one-step undo.
+      const bak = `${dest}.bak`;
+      fs.rmSync(bak, { force: true });
+      fs.writeFileSync(bak, existing);
       fs.writeFileSync(dest, src);
-      console.log(`Trial updated: ${destRel}`);
+      console.log(`Trial updated: ${destRel} (previous content saved to ${destRel}.bak)`);
     } else {
       console.error(`${destRel} already exists and is not a Trial file — refusing to overwrite. ` +
         `Re-run with --force to replace it, or remove it first.`);
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "trial-skill",
-  "version": "0.5.2",
+  "version": "0.5.3",
   "description": "A pre-delivery evidence gate that withholds unsupported completion claims and releases only verified results.",
   "keywords": [
     "ai-agents",
diff --git a/tests/install.test.js b/tests/install.test.js
@@ -138,6 +138,93 @@ test('dedicated target refuses a foreign file but --force overwrites it', () =>
   fs.rmSync(dir, { recursive: true, force: true });
 });
 
+function runArgs(cwd, args) {
+  return execFileSync('node', [installer, ...args], { cwd, encoding: 'utf8' });
+}
+
+test('symlinked destinations are refused and the link target is untouched', () => {
+  if (process.platform === 'win32') return;
+  const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'trial-sym-'));
+  const dir2 = fs.mkdtempSync(path.join(os.tmpdir(), 'trial-sym2-'));
+  const outside = fs.mkdtempSync(path.join(os.tmpdir(), 'trial-victim-'));
+  const victim = path.join(outside, 'victim.md');
+  // Contains the signature on purpose: without the lstat guard the dedicated
+  // branch would treat the link target as Trial-managed and overwrite it.
+  const original = 'IMPORTANT external file\nquotes "Trial — Pre-Delivery Evidence Gate"\n';
+  fs.writeFileSync(victim, original);
+
+  fs.mkdirSync(path.join(dir, '.cursor', 'rules'), { recursive: true });
+  fs.symlinkSync(victim, path.join(dir, '.cursor', 'rules', 'trial.mdc'));
+  assert.throws(() => run(dir, 'cursor'), /Command failed/, 'symlinked dedicated dest must be refused');
+  assert.throws(() => runArgs(dir, ['cursor', '--force']), /Command failed/, '--force must not write through a symlink');
+
+  fs.symlinkSync(victim, path.join(dir2, 'AGENTS.md'));
+  assert.throws(() => run(dir2, 'codex'), /Command failed/, 'symlinked append dest must be refused');
+
+  assert.strictEqual(fs.readFileSync(victim, 'utf8'), original, 'external file untouched');
+  fs.rmSync(dir, { recursive: true, force: true });
+  fs.rmSync(dir2, { recursive: true, force: true });
+  fs.rmSync(outside, { recursive: true, force: true });
+});
+
+test('a destination directory resolving outside the project is refused', () => {
+  if (process.platform === 'win32') return;
+  const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'trial-symdir-'));
+  const outside = fs.mkdtempSync(path.join(os.tmpdir(), 'trial-outside-'));
+  fs.symlinkSync(outside, path.join(dir, '.cursor'));
+  assert.throws(() => run(dir, 'cursor'), /Command failed/, 'symlinked parent dir must be refused');
+  assert.strictEqual(fs.readdirSync(outside).length, 0, 'nothing created outside the project');
+  fs.rmSync(dir, { recursive: true, force: true });
+  fs.rmSync(outside, { recursive: true, force: true });
+});
+
+test('malformed or duplicated markers stop the installer instead of guessing', () => {
+  const cases = [
+    ['begin with no end', '<!-- trial:begin -->\nold body\n'],
+    ['end before begin', '<!-- trial:end -->\nnoise\n<!-- trial:begin -->\n'],
+    ['duplicated pairs', '<!-- trial:begin -->\na\n<!-- trial:end -->\n<!-- trial:begin -->\nb\n<!-- trial:end -->\n'],
+    ['stray begin above a real pair', 'HEAD\n<!-- trial:begin -->\nPRECIOUS\n<!-- trial:begin -->\nold\n<!-- trial:end -->\nTAIL\n'],
+  ];
+  for (const [label, content] of cases) {
+    const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'trial-marker-'));
+    fs.writeFileSync(path.join(dir, 'AGENTS.md'), content);
+    let err;
+    try { run(dir, 'codex'); } catch (e) { err = e; }
+    assert.ok(err, `${label}: exits non-zero`);
+    assert.match(String(err.stderr || ''), /malformed Trial markers/, `${label}: names the problem`);
+    assert.strictEqual(fs.readFileSync(path.join(dir, 'AGENTS.md'), 'utf8'), content, `${label}: file untouched`);
+    fs.rmSync(dir, { recursive: true, force: true });
+  }
+});
+
+test('dedicated update keeps a .bak of the replaced content', () => {
+  const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'trial-bak-'));
+  const dest = path.join(dir, '.cursor', 'rules', 'trial.mdc');
+  fs.mkdirSync(path.dirname(dest), { recursive: true });
+  // A personal file that merely QUOTES the signature is treated as
+  // Trial-managed by the substring test; the .bak is its safety net.
+  const personal = '# My notes\nabout the "Trial — Pre-Delivery Evidence Gate" idea\nirreplaceable prose\n';
+  fs.writeFileSync(dest, personal);
+  const out = run(dir, 'cursor');
+  assert.match(out, /saved to .*trial\.mdc\.bak/, 'update announces the backup');
+  assert.strictEqual(fs.readFileSync(`${dest}.bak`, 'utf8'), personal, 'backup holds the replaced content');
+  assert.match(fs.readFileSync(dest, 'utf8'), /# Trial/, 'rule installed');
+  fs.rmSync(dir, { recursive: true, force: true });
+});
+
+test('--update is rejected as unknown instead of silently forcing', () => {
+  const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'trial-upd-'));
+  const dest = path.join(dir, '.cursor', 'rules', 'trial.mdc');
+  fs.mkdirSync(path.dirname(dest), { recursive: true });
+  fs.writeFileSync(dest, '# my own file, no signature\n');
+  let err;
+  try { runArgs(dir, ['cursor', '--update']); } catch (e) { err = e; }
+  assert.ok(err, 'exits non-zero');
+  assert.match(String(err.stderr || ''), /Unknown option/, 'names the bad flag');
+  assert.strictEqual(fs.readFileSync(dest, 'utf8'), '# my own file, no signature\n', 'file preserved');
+  fs.rmSync(dir, { recursive: true, force: true });
+});
+
 test('read-only destination fails with a friendly message, not a stack trace', () => {
   if (process.platform === 'win32') return;
   const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'trial-inst-'));

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "trial",`
`3`		`- "version": "0.5.2",`
	`3`	`+ "version": "0.5.3",`
`4`	`4`	`"description": "Pre-delivery evidence gate: withhold unsupported completion claims and release only verified results.",`
`5`	`5`	`"author": {`
`6`	`6`	`"name": "Da7-Tech",`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "trial-skill",`
`3`		`- "version": "0.5.2",`
	`3`	`+ "version": "0.5.3",`
`4`	`4`	`"description": "A pre-delivery evidence gate that withholds unsupported completion claims and releases only verified results.",`
`5`	`5`	`"keywords": [`
`6`	`6`	`"ai-agents",`