mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-09 14:38:59 +02:00
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 <cursoragent@cursor.com>
This commit is contained in:
@@ -8,10 +8,10 @@
|
||||
# --check <id> [--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 [<id>] → 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
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user