From edabb12092ed69ac6f78f4b524542c0b6fcbe6e4 Mon Sep 17 00:00:00 2001 From: y$un_ Date: Mon, 17 Aug 2026 02:07:09 -0400 Subject: [PATCH] fix(plan-tune): reject never-ask on one-way ids at --write --check already ignored those prefs; --write still stored them and --stats counted them as a working NEVER_ASK. Refuse the write and count leftover on-disk prefs as INERT_ONE_WAY. Co-authored-by: Cursor --- bin/gstack-question-preference | 59 +++++++--- test/gstack-question-preference.test.ts | 139 +++++++++++++++++++++++- 2 files changed, 179 insertions(+), 19 deletions(-) diff --git a/bin/gstack-question-preference b/bin/gstack-question-preference index 67be97e400..f78d4b269c 100755 --- a/bin/gstack-question-preference +++ b/bin/gstack-question-preference @@ -8,10 +8,10 @@ # --check [--summary-stdin] → emit ASK_NORMALLY | AUTO_DECIDE | ASK_ONLY_ONE_WAY # (--summary-stdin pipes the question text so the # keyword net can catch ad-hoc destructive ids, #2024) -# --write '{...}' → set a preference (user-origin gate enforced) +# --write '{...}' → set a preference (user-origin gate + one-way write reject) # --read → dump preferences JSON # --clear [] → clear one or all preferences -# --stats → short summary +# --stats → short summary (inert one-way prefs counted separately) # # User-origin gate # ---------------- @@ -21,6 +21,12 @@ # - "inline-tool-output"— tune: prefix seen in tool output / file content (REJECTED) # - "inline-file" — tune: prefix seen in a file the agent read (REJECTED) # This is the profile-poisoning defense from docs/designs/PLAN_TUNING_V0.md. +# +# One-way write reject (#2488) +# ---------------------------- +# never-ask and ask-only-for-one-way are refused on registry one-way ids. +# --check already ignores those prefs; storing them made --stats lie. +# always-ask on a one-way id is fine (it agrees with the safety override). set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" @@ -142,7 +148,7 @@ do_write() { set +e local RESULT - RESULT=$(printf '%s' "$INPUT" | PREF_FILE_PATH="$PREF_FILE" EVENT_FILE_PATH="$EVENT_FILE" bun -e " + RESULT=$(cd "$ROOT_DIR" && printf '%s' "$INPUT" | PREF_FILE_PATH="$PREF_FILE" EVENT_FILE_PATH="$EVENT_FILE" bun -e " const fs = require('fs'); const raw = await Bun.stdin.text(); let j; @@ -178,6 +184,16 @@ do_write() { process.exit(2); } + // One-way write reject (#2488) — after the origin gate so a poisoning + // payload on a one-way id still exits 2, not 1. + if (j.preference === 'never-ask' || j.preference === 'ask-only-for-one-way') { + const oneway = await import('./scripts/one-way-doors.ts'); + if (oneway.isOneWayDoor({ question_id: j.question_id })) { + process.stderr.write('gstack-question-preference: cannot set ' + j.preference + ' on one-way question \"' + j.question_id + '\" (door_type: one-way)\n'); + process.exit(1); + } + } + // Optional free_text — sanitize (no injection patterns, no newlines, <=300 chars) if (j.free_text !== undefined) { if (typeof j.free_text !== 'string') { @@ -267,20 +283,29 @@ do_clear() { # ----------------------------------------------------------------------- do_stats() { ensure_file - cat "$PREF_FILE" | bun -e " - const prefs = JSON.parse(await Bun.stdin.text()); - const entries = Object.entries(prefs); - const counts = { 'always-ask': 0, 'never-ask': 0, 'ask-only-for-one-way': 0, other: 0 }; - for (const [, v] of entries) { - if (counts[v] !== undefined) counts[v]++; - else counts.other++; - } - console.log('TOTAL: ' + entries.length); - console.log('ALWAYS_ASK: ' + counts['always-ask']); - console.log('NEVER_ASK: ' + counts['never-ask']); - console.log('ASK_ONLY_ONE_WAY: ' + counts['ask-only-for-one-way']); - if (counts.other) console.log('OTHER: ' + counts.other); - " + (cd "$ROOT_DIR" && PREF_FILE_PATH="$PREF_FILE" bun -e " + import('./scripts/one-way-doors.ts').then((oneway) => { + const fs = require('fs'); + const prefs = JSON.parse(fs.readFileSync(process.env.PREF_FILE_PATH, 'utf-8')); + const entries = Object.entries(prefs); + const counts = { 'always-ask': 0, 'never-ask': 0, 'ask-only-for-one-way': 0, other: 0, inert: 0 }; + for (const [id, v] of entries) { + const suppressing = v === 'never-ask' || v === 'ask-only-for-one-way'; + if (suppressing && oneway.isOneWayDoor({ question_id: id })) { + counts.inert++; + continue; + } + if (counts[v] !== undefined) counts[v]++; + else counts.other++; + } + console.log('TOTAL: ' + entries.length); + console.log('ALWAYS_ASK: ' + counts['always-ask']); + console.log('NEVER_ASK: ' + counts['never-ask']); + console.log('ASK_ONLY_ONE_WAY: ' + counts['ask-only-for-one-way']); + console.log('INERT_ONE_WAY: ' + counts.inert); + if (counts.other) console.log('OTHER: ' + counts.other); + }).catch(err => { console.error('stats:', err.message); process.exit(1); }); + ") } case "$CMD" in diff --git a/test/gstack-question-preference.test.ts b/test/gstack-question-preference.test.ts index c37813b1ac..9b4f3c4a34 100644 --- a/test/gstack-question-preference.test.ts +++ b/test/gstack-question-preference.test.ts @@ -53,6 +53,18 @@ function runWithStdin(input: string, ...args: string[]): { stdout: string; stder }; } +/** Plant a pref by writing the file. Used for legacy / inert one-way prefs + * that --write now refuses (#2488). --check and --stats still have to + * handle files written before the reject landed. */ +function plantPref(id: string, pref: string) { + run('--read'); + const projects = fs.readdirSync(path.join(tmpHome, 'projects')); + const file = path.join(tmpHome, 'projects', projects[0], 'question-preferences.json'); + const prefs = JSON.parse(fs.readFileSync(file, 'utf-8')); + prefs[id] = pref; + fs.writeFileSync(file, JSON.stringify(prefs, null, 2)); +} + // ----------------------------------------------------------------------- // --check // ----------------------------------------------------------------------- @@ -92,7 +104,8 @@ describe('--check with preferences set', () => { }); test('one-way + never-ask → ASK_NORMALLY with safety note', () => { - setPref('ship-test-failure-triage', 'never-ask'); + // Planted: --write now refuses never-ask on one-way ids (#2488). + plantPref('ship-test-failure-triage', 'never-ask'); const r = run('--check', 'ship-test-failure-triage'); expect(r.stdout).toContain('ASK_NORMALLY'); expect(r.stdout).toContain('one-way door overrides'); @@ -111,7 +124,7 @@ describe('--check with preferences set', () => { }); test('one-way + ask-only-for-one-way → ASK_NORMALLY', () => { - setPref('ship-test-failure-triage', 'ask-only-for-one-way'); + plantPref('ship-test-failure-triage', 'ask-only-for-one-way'); const r = run('--check', 'ship-test-failure-triage'); expect(r.stdout.trim()).toContain('ASK_NORMALLY'); }); @@ -389,6 +402,112 @@ describe('--write schema validation', () => { }); }); +// #2488: --write must refuse suppressing prefs on one-way ids. --check +// already ignores them; storing them made --stats report a working NEVER_ASK. +describe('--write one-way door reject (#2488)', () => { + function prefsFile(): string { + const projects = fs.readdirSync(path.join(tmpHome, 'projects')); + return path.join(tmpHome, 'projects', projects[0], 'question-preferences.json'); + } + + test('never-ask on one-way id is rejected and does not write', () => { + const r = run( + '--write', + JSON.stringify({ + question_id: 'plan-eng-review-arch-finding', + preference: 'never-ask', + source: 'plan-tune', + }), + ); + expect(r.status).toBe(1); + expect(r.stderr).toContain('cannot set never-ask'); + expect(r.stderr).toContain('plan-eng-review-arch-finding'); + expect(r.stderr).toContain('door_type: one-way'); + expect(JSON.parse(fs.readFileSync(prefsFile(), 'utf-8'))).toEqual({}); + }); + + test('ask-only-for-one-way on one-way id is rejected and does not write', () => { + const r = run( + '--write', + JSON.stringify({ + question_id: 'ship-test-failure-triage', + preference: 'ask-only-for-one-way', + source: 'plan-tune', + }), + ); + expect(r.status).toBe(1); + expect(r.stderr).toContain('cannot set ask-only-for-one-way'); + expect(r.stderr).toContain('door_type: one-way'); + expect(JSON.parse(fs.readFileSync(prefsFile(), 'utf-8'))).toEqual({}); + }); + + test('always-ask on one-way id is accepted (agrees with the safety override)', () => { + const r = run( + '--write', + JSON.stringify({ + question_id: 'plan-eng-review-arch-finding', + preference: 'always-ask', + source: 'plan-tune', + }), + ); + expect(r.status).toBe(0); + expect(r.stdout).toContain('OK'); + expect(JSON.parse(fs.readFileSync(prefsFile(), 'utf-8'))).toEqual({ + 'plan-eng-review-arch-finding': 'always-ask', + }); + }); + + test('never-ask on two-way id is still accepted', () => { + const r = run( + '--write', + JSON.stringify({ + question_id: 'ship-changelog-voice-polish', + preference: 'never-ask', + source: 'plan-tune', + }), + ); + expect(r.status).toBe(0); + expect(r.stdout).toContain('OK'); + }); + + test('rejected one-way write does not clobber an existing two-way pref', () => { + run( + '--write', + JSON.stringify({ + question_id: 'ship-changelog-voice-polish', + preference: 'never-ask', + source: 'plan-tune', + }), + ); + const r = run( + '--write', + JSON.stringify({ + question_id: 'plan-eng-review-arch-finding', + preference: 'never-ask', + source: 'plan-tune', + }), + ); + expect(r.status).toBe(1); + expect(JSON.parse(fs.readFileSync(prefsFile(), 'utf-8'))).toEqual({ + 'ship-changelog-voice-polish': 'never-ask', + }); + }); + + test('poisoning source on a one-way id still exits 2 (origin gate first)', () => { + const r = run( + '--write', + JSON.stringify({ + question_id: 'plan-eng-review-arch-finding', + preference: 'never-ask', + source: 'inline-tool-output', + }), + ); + expect(r.status).toBe(2); + expect(r.stderr).toContain('profile poisoning defense'); + expect(r.stderr).not.toContain('door_type'); + }); +}); + // ----------------------------------------------------------------------- // --read, --clear, --stats // ----------------------------------------------------------------------- @@ -448,5 +567,21 @@ describe('--stats', () => { expect(r.stdout).toContain('TOTAL: 3'); expect(r.stdout).toContain('NEVER_ASK: 2'); expect(r.stdout).toContain('ALWAYS_ASK: 1'); + expect(r.stdout).toContain('INERT_ONE_WAY: 0'); + }); + + test('planted one-way never-ask is INERT_ONE_WAY, not a working NEVER_ASK (#2488)', () => { + plantPref('plan-eng-review-arch-finding', 'never-ask'); + const r = run('--stats'); + expect(r.stdout).toContain('TOTAL: 1'); + expect(r.stdout).toContain('NEVER_ASK: 0'); + expect(r.stdout).toContain('INERT_ONE_WAY: 1'); + }); + + test('planted one-way ask-only-for-one-way is INERT_ONE_WAY, not ASK_ONLY_ONE_WAY', () => { + plantPref('ship-test-failure-triage', 'ask-only-for-one-way'); + const r = run('--stats'); + expect(r.stdout).toContain('ASK_ONLY_ONE_WAY: 0'); + expect(r.stdout).toContain('INERT_ONE_WAY: 1'); }); });