diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index e8d163ec2..f7ceb283d 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -12,6 +12,7 @@ ### Fixed +- Fixed writes to `auth.json` and `models-store.json` overriding administrator-managed file permissions and ACLs ([#7779](https://github.com/earendil-works/pi/issues/7779)). - Fixed UTF-8 BOM markers preventing frontmatter and user configuration files from loading ([#8337](https://github.com/earendil-works/pi/issues/8337)). - Fixed invalid settings files being easy to miss during interactive startup by rendering warnings with the file path inside the TUI ([#7829](https://github.com/earendil-works/pi/issues/7829)). - Fixed the subagent example repeatedly prompting before running project-local agents in trusted repositories ([#8261](https://github.com/earendil-works/pi/issues/8261)). diff --git a/packages/coding-agent/src/core/auth-storage.ts b/packages/coding-agent/src/core/auth-storage.ts index 71be9d022..9d6495919 100644 --- a/packages/coding-agent/src/core/auth-storage.ts +++ b/packages/coding-agent/src/core/auth-storage.ts @@ -4,7 +4,7 @@ */ import type { AuthOperationOptions, Credential, CredentialInfo, CredentialStore } from "@earendil-works/pi-ai"; -import { chmodSync, existsSync, mkdirSync, readFileSync, writeFileSync } from "fs"; +import { existsSync, mkdirSync, readFileSync, writeFileSync } from "fs"; import { dirname, join } from "path"; import lockfile from "proper-lockfile"; import { setTimeout as sleep } from "timers/promises"; @@ -21,6 +21,7 @@ type LockResult = { next?: string; }; +// The mode applies only on creation so administrator-managed modes and ACLs remain intact. const AUTH_FILE_WRITE_OPTIONS = { encoding: "utf-8", mode: 0o600 } as const; type AuthFileReload = { @@ -62,7 +63,6 @@ export class FileAuthStorageBackend implements AuthStorageBackend { private ensureFileExists(): void { if (!existsSync(this.authPath)) { writeFileSync(this.authPath, "{}", AUTH_FILE_WRITE_OPTIONS); - chmodSync(this.authPath, 0o600); } } @@ -104,7 +104,6 @@ export class FileAuthStorageBackend implements AuthStorageBackend { const { result, next } = fn(current); if (next !== undefined) { writeFileSync(this.authPath, next, AUTH_FILE_WRITE_OPTIONS); - chmodSync(this.authPath, 0o600); } return result; } finally { @@ -186,7 +185,6 @@ export class FileAuthStorageBackend implements AuthStorageBackend { options?.signal?.throwIfAborted(); if (next !== undefined) { writeFileSync(this.authPath, next, AUTH_FILE_WRITE_OPTIONS); - chmodSync(this.authPath, 0o600); } throwIfCompromised(); return result; diff --git a/packages/coding-agent/test/auth-storage.test.ts b/packages/coding-agent/test/auth-storage.test.ts index 801697ee2..61cdbefba 100644 --- a/packages/coding-agent/test/auth-storage.test.ts +++ b/packages/coding-agent/test/auth-storage.test.ts @@ -1,4 +1,4 @@ -import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { chmodSync, existsSync, mkdirSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { type CredentialStore, createModels, type Provider } from "@earendil-works/pi-ai"; @@ -139,6 +139,22 @@ describe("AuthStorage", () => { expect(release).toHaveBeenCalledTimes(1); }); + test.skipIf(process.platform === "win32")("creates new auth files with owner-only permissions", () => { + AuthStorage.create(authJsonPath); + + expect(statSync(authJsonPath).mode & 0o777).toBe(0o600); + }); + + test.skipIf(process.platform === "win32")("preserves the mode of an existing auth file", async () => { + writeAuthJson({ anthropic: { type: "api_key", key: "old" } }); + chmodSync(authJsonPath, 0o660); + const storage = AuthStorage.create(authJsonPath); + + await storage.modify("anthropic", async () => ({ type: "api_key", key: "new" })); + + expect(statSync(authJsonPath).mode & 0o777).toBe(0o660); + }); + test("modify persists a credential while preserving unrelated external edits", async () => { writeAuthJson({ anthropic: { type: "api_key", key: "old" } }); const storage = AuthStorage.create(authJsonPath); diff --git a/packages/coding-agent/test/models-store.test.ts b/packages/coding-agent/test/models-store.test.ts index 30e592f7c..bf645042c 100644 --- a/packages/coding-agent/test/models-store.test.ts +++ b/packages/coding-agent/test/models-store.test.ts @@ -1,4 +1,4 @@ -import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { chmodSync, existsSync, mkdirSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import type { Model } from "@earendil-works/pi-ai"; @@ -53,6 +53,17 @@ describe("FileModelsStore", () => { expect((await reloaded.read("two"))?.models.map((entry) => entry.id)).toEqual(["m2"]); }); + it.skipIf(process.platform === "win32")("preserves the mode of an existing models file", async () => { + const managedModelsPath = join(sharedTempDir, "managed-mode.json"); + writeFileSync(managedModelsPath, "{}"); + chmodSync(managedModelsPath, 0o660); + const store = new FileModelsStore(managedModelsPath); + + await store.write("one", { models: [model("one", "m1")], checkedAt: 100 }); + + expect(statSync(managedModelsPath).mode & 0o777).toBe(0o660); + }); + it("coalesces file reloads across concurrent readers and interleaved storage instances", async () => { writeFileSync( sharedModelsPath,