Skip to content

Commit a853920

Browse files
committed
fix: block unreadable global settings writes
1 parent 575545f commit a853920

2 files changed

Lines changed: 43 additions & 11 deletions

File tree

agenticoding.test.ts

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -664,13 +664,41 @@ test("agenticoding settings TUI handles invalid JSON policies", async () => {
664664
});
665665
});
666666

667+
test("agenticoding settings write path refuses non-ENOENT read failures without clobbering global settings", async () => {
668+
await withIsolatedSettings(async ({ home, cwd }) => {
669+
const globalPath = join(home, ".pi", "agent", "settings.json");
670+
const original = JSON.stringify({ packages: ["keep"], handoff: { other: true } });
671+
await writeSettingsFile(globalPath, original);
672+
await chmod(globalPath, 0o200);
673+
674+
const notifications: Array<{ message: string; level: string }> = [];
675+
const ctx = {
676+
cwd,
677+
hasUI: true,
678+
ui: { notify: (message: string, level: string) => notifications.push({ message, level }) },
679+
} as any;
680+
681+
try {
682+
assert.equal(await writeGlobalHandoffResumeBehavior("proceed", ctx), false);
683+
} finally {
684+
await chmod(globalPath, 0o600);
685+
}
686+
687+
assert.equal(await readFile(globalPath, "utf8"), original);
688+
assert.equal(notifications.length, 1);
689+
assert.equal(notifications[0].level, "error");
690+
assert.match(notifications[0].message, /Unable to read global settings JSON/);
691+
assert.match(notifications[0].message, /not writing handoff\.resumeBehavior/);
692+
});
693+
});
694+
667695
test("agenticoding settings write path handles save failure with error notification", async () => {
668696
await withIsolatedSettings(async ({ home }) => {
669-
// Block the settings directory by creating a file where a directory is expected.
670-
// writeGlobalHandoffResumeBehavior calls mkdir(dirname(path), { recursive: true }),
671-
// so making .pi/agent a file (instead of a directory) will cause mkdir to throw ENOTDIR.
672-
await mkdir(join(home, ".pi"), { recursive: true });
673-
await writeFile(join(home, ".pi", "agent"), "block", "utf8");
697+
// Keep the read path in the ENOENT/create-new-file branch, then make the
698+
// existing settings directory non-writable so writeFile rejects.
699+
const settingsDir = join(home, ".pi", "agent");
700+
await mkdir(settingsDir, { recursive: true });
701+
await chmod(settingsDir, 0o500);
674702

675703
const notifications: Array<{ message: string; level: string }> = [];
676704
const ctx = {
@@ -679,10 +707,14 @@ test("agenticoding settings write path handles save failure with error notificat
679707
ui: { notify: (message: string, level: string) => notifications.push({ message, level }) },
680708
} as any;
681709

682-
await assert.rejects(
683-
() => writeGlobalHandoffResumeBehavior("proceed", ctx),
684-
/EEXIST|ENOTDIR|ENOSPC/,
685-
);
710+
try {
711+
await assert.rejects(
712+
() => writeGlobalHandoffResumeBehavior("proceed", ctx),
713+
/EACCES|EPERM|ENOSPC/,
714+
);
715+
} finally {
716+
await chmod(settingsDir, 0o700);
717+
}
686718
});
687719

688720
// The async IIFE in createAgenticodingSettingsComponent's callback wraps model.save

settings.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,8 +200,8 @@ export async function writeGlobalHandoffResumeBehavior(
200200
} catch (error) {
201201
const code = typeof error === "object" && error !== null && "code" in error ? (error as { code?: unknown }).code : undefined;
202202
if (code !== "ENOENT") {
203-
// Unreadable files are treated like missing by the resolver. Let the write
204-
// path report any real filesystem failure from writeFile below.
203+
notify(ctx, `Unable to read global settings JSON at ${path}; not writing handoff.resumeBehavior to avoid clobbering it.`, "error");
204+
return false;
205205
}
206206
}
207207

0 commit comments

Comments
 (0)