diff --git a/docs/examples.md b/docs/examples.md index ff2fbef..c162993 100644 --- a/docs/examples.md +++ b/docs/examples.md @@ -64,6 +64,8 @@ pnpm mcode provider list --json `--context-limit` and `--output-limit` each accept a positive safe integer (at most `9007199254740991`). Either flag can be used independently. The same limits apply to every repeated `--model`; only the first model is tested and selected by `--use`. The JSON list shows the configured values as `contextLimit` and `maxOutputTokens`. Without these flags, the existing defaults remain unchanged (unknown custom models currently fall back to 200,000 context tokens and 16,384 output tokens). Model discovery does not infer your local server's context size. +`--api-key-env` reads the current environment variable value and stores that value in the active profile's `config.yaml`; it does not save an environment-variable reference. The file still contains plaintext credentials. On POSIX systems, config writes and temporary copies use `0600`. When loading existing files, MCode removes group/other access while preserving the owner's permissions; already-private files such as `0400` or `0600` do not require a permission change. Loading fails if an unsafe main config cannot be restricted. Older migration backups are also checked, but inspection or repair failures produce a warning identifying the directory or backup that needs manual attention rather than preventing the main config from loading. Windows file modes do not provide equivalent ACL protection; restrict access to the profile directory using Windows permissions. + [Live acceptance](verification.md) separately verified MiniMax Token Plan and one configured BYOK provider. This is not a guarantee for every compatible service. ## 3. Search and image input diff --git a/packages/config/src/byok-config.ts b/packages/config/src/byok-config.ts index 595ffeb..99b9c1b 100644 --- a/packages/config/src/byok-config.ts +++ b/packages/config/src/byok-config.ts @@ -4,10 +4,11 @@ // (they are part of the Config surface); the back edge here is type-only, so // there is no runtime import cycle. -import { createHash } from 'node:crypto'; +import { createHash, randomUUID } from 'node:crypto'; import fs from 'node:fs'; import path from 'node:path'; import yaml from 'js-yaml'; +import { restrictConfigFileSync, writePrivateConfigFileSync } from './private-config-file.js'; import type { CustomProvidersConfig, @@ -243,10 +244,9 @@ export function migrateLegacyByokProvidersOnDisk( writeLegacyMigrationMarker(raw, marker); try { const backupPath = backupConfigForMigration(configPath); - fs.writeFileSync( + writePrivateConfigFileSync( configPath, yaml.dump(raw, { indent: 2, lineWidth: -1, noRefs: true }), - 'utf-8', ); return { migrated: true, migratedProviders, backupPath }; } catch (err) { @@ -474,12 +474,43 @@ function rewriteNexusModelProvider( return true; } +/** Repair old backups without making archival maintenance a config-load dependency. */ +export function restrictLegacyByokBackups(configPath: string): void { + if (process.platform === 'win32') return; + const directory = path.dirname(configPath); + let entries: fs.Dirent[]; + try { + entries = fs.readdirSync(directory, { withFileTypes: true }); + } catch { + console.warn( + `[config] Could not inspect legacy BYOK backups in ${JSON.stringify(directory)}. ` + + 'Backup permissions were not verified; check directory access and restrict backup permissions manually.', + ); + return; + } + for (const entry of entries) { + if (entry.isFile() && entry.name.startsWith(LEGACY_BYOK_BACKUP_PREFIX)) { + const backupPath = path.join(directory, entry.name); + try { + restrictConfigFileSync(backupPath); + } catch { + // Do not expose arbitrary error messages that could contain config content. + console.warn( + `[config] Could not restrict legacy BYOK backup ${JSON.stringify(backupPath)}. ` + + 'It may still be readable by other users; restrict its permissions manually.', + ); + } + } + } +} + function backupConfigForMigration(configPath: string): string { const backupPath = path.join( path.dirname(configPath), - `${LEGACY_BYOK_BACKUP_PREFIX}${Date.now()}`, + `${LEGACY_BYOK_BACKUP_PREFIX}${Date.now()}.${randomUUID()}`, ); - fs.copyFileSync(configPath, backupPath); + restrictConfigFileSync(configPath); + writePrivateConfigFileSync(backupPath, fs.readFileSync(configPath), true); return backupPath; } diff --git a/packages/config/src/config.ts b/packages/config/src/config.ts index 74254ed..db9f710 100644 --- a/packages/config/src/config.ts +++ b/packages/config/src/config.ts @@ -3,6 +3,7 @@ import { type RunawayGuardSettings, } from "./runaway-guard-config.js"; import fs from "node:fs"; +import { restrictConfigFileSync, writePrivateConfigFileSync } from "./private-config-file.js"; import path from "node:path"; import os from "node:os"; import { spawnSync } from "node:child_process"; @@ -13,6 +14,7 @@ import { applyManagedMinimaxContextLimits, applyRequiredProviderOverrides, migrateLegacyByokProvidersOnDisk, + restrictLegacyByokBackups, normalizeLegacyThinkingEfforts, parseCustomProvidersConfig, parseMinimaxApiConfig, @@ -1648,10 +1650,9 @@ function syncManagedPresetBaseUrl(configPath: string): void { return; (options as Record).baseURL = presetBaseURL; - fs.writeFileSync( + writePrivateConfigFileSync( configPath, yaml.dump(raw, { indent: 2, lineWidth: -1, noRefs: true }), - "utf-8", ); } @@ -1830,7 +1831,8 @@ function ensureConfigFile(): void { const defaultDataDir = resolveDataDir({ homeDir: os.homedir() }); const defaultConfigPath = path.join(defaultDataDir, "config.yaml"); if (configPath !== defaultConfigPath && fs.existsSync(defaultConfigPath)) { - fs.copyFileSync(defaultConfigPath, configPath); + restrictConfigFileSync(defaultConfigPath); + writePrivateConfigFileSync(configPath, fs.readFileSync(defaultConfigPath), true); } return; } @@ -1841,7 +1843,7 @@ function ensureConfigFile(): void { provider: managedPresetBaseUrlSyncEnabled ? preset.provider : undefined, defaultModel: preset.defaultModel, }); - fs.writeFileSync(configPath, content, "utf-8"); + writePrivateConfigFileSync(configPath, content, true); } function readConfigFile(configPath = getConfigPath()): Record { @@ -1897,6 +1899,8 @@ export function setManagedPresetBaseUrlSyncEnabled(enabled: boolean): void { } export function prepareConfigFileForRead(configPath: string): void { + restrictConfigFileSync(configPath); + restrictLegacyByokBackups(configPath); if (legacyByokProviderMigrationEnabled) { migrateLegacyByokProvidersOnDisk( configPath, diff --git a/packages/config/src/cu-backend-io.ts b/packages/config/src/cu-backend-io.ts index dc55238..49d8668 100644 --- a/packages/config/src/cu-backend-io.ts +++ b/packages/config/src/cu-backend-io.ts @@ -1,6 +1,7 @@ import fs from 'node:fs'; import path from 'node:path'; import yaml from 'js-yaml'; +import { writePrivateConfigFileSync } from './private-config-file.js'; import { getConfig, getConfigPath, resetConfig } from './config.js'; import { type CuBackend, @@ -51,7 +52,7 @@ export function setCuBackend(value: CuBackend): void { raw.cuBackend = value; fs.mkdirSync(path.dirname(configPath), { recursive: true }); - fs.writeFileSync(configPath, yaml.dump(raw), 'utf-8'); + writePrivateConfigFileSync(configPath, yaml.dump(raw)); resetConfig(); } diff --git a/packages/config/src/local-model-provider-write.ts b/packages/config/src/local-model-provider-write.ts index fa52152..8a19a42 100644 --- a/packages/config/src/local-model-provider-write.ts +++ b/packages/config/src/local-model-provider-write.ts @@ -229,6 +229,7 @@ async function withLockedConfig( try { await fs.promises.mkdir(dirname(configPath), { recursive: true }); await fs.promises.writeFile(configPath, '', { flag: 'a', mode: LOCAL_CONFIG_FILE_MODE }); + await fs.promises.chmod(configPath, LOCAL_CONFIG_FILE_MODE); release = await lockfile.lock(configPath, { stale: 10_000, retries: { retries: 20, factor: 1, minTimeout: 5, maxTimeout: 25 }, @@ -257,26 +258,24 @@ async function withLockedConfig( async function atomicWriteFile(filePath: string, content: string): Promise { const tmpPath = join(dirname(filePath), `.config-tmp-${randomBytes(6).toString('hex')}`); + let created = false; try { - const mode = await readFilePermissionMode(filePath); - await fs.promises.writeFile(tmpPath, content, { encoding: 'utf-8', mode }); - await fs.promises.chmod(tmpPath, mode); + const mode = LOCAL_CONFIG_FILE_MODE; + const temporary = await fs.promises.open(tmpPath, 'wx', mode); + created = true; + try { + await temporary.writeFile(content, 'utf-8'); + await temporary.chmod(mode); + } finally { + await temporary.close(); + } await fs.promises.rename(tmpPath, filePath); } catch { - await fs.promises.unlink(tmpPath).catch(() => undefined); + if (created) await fs.promises.unlink(tmpPath).catch(() => undefined); throw new LocalModelProviderConfigWriteError(); } } -async function readFilePermissionMode(filePath: string): Promise { - try { - return (await fs.promises.stat(filePath)).mode & 0o777; - } catch (error) { - if (isNodeError(error) && error.code === 'ENOENT') return LOCAL_CONFIG_FILE_MODE; - throw error; - } -} - function readLocalRawConfig(configPath: string): Record { if (!existsSync(configPath)) return {}; try { @@ -337,10 +336,6 @@ function assertSafeConfigRecord(record: Record): void { } } -function isNodeError(error: unknown): error is NodeJS.ErrnoException { - return error instanceof Error && 'code' in error; -} - function isPlainRecord(value: unknown): value is Record { return Boolean(value) && typeof value === 'object' && !Array.isArray(value); } diff --git a/packages/config/src/private-config-file.ts b/packages/config/src/private-config-file.ts new file mode 100644 index 0000000..82e4f09 --- /dev/null +++ b/packages/config/src/private-config-file.ts @@ -0,0 +1,33 @@ +import fs from "node:fs"; + +/** Config documents and their copies can contain plaintext credentials. */ +export const PRIVATE_CONFIG_FILE_MODE = 0o600; + +/** Remove non-owner access without changing an owner's read-only policy. */ +export function restrictConfigFileSync(filePath: string): void { + if (process.platform === "win32") return; + const mode = fs.statSync(filePath).mode; + // Already-private files may live on read-only mounts or be immutable. + if ((mode & 0o077) === 0) return; + fs.chmodSync(filePath, mode & 0o700); +} + +/** Restrict access before truncating or writing any secret-bearing content. */ +export function writePrivateConfigFileSync( + filePath: string, + content: string | Buffer, + exclusive = false, +): void { + const fd = fs.openSync( + filePath, + exclusive ? "wx" : "a", + PRIVATE_CONFIG_FILE_MODE, + ); + try { + fs.fchmodSync(fd, PRIVATE_CONFIG_FILE_MODE); + fs.ftruncateSync(fd, 0); + fs.writeFileSync(fd, content); + } finally { + fs.closeSync(fd); + } +} diff --git a/packages/config/src/tui-status-line-write.ts b/packages/config/src/tui-status-line-write.ts index d799366..e24f54b 100644 --- a/packages/config/src/tui-status-line-write.ts +++ b/packages/config/src/tui-status-line-write.ts @@ -11,10 +11,12 @@ export async function writeTuiStatusLineSetting( ): Promise { const configPath = join(dataDir, 'config.yaml'); let temporaryPath: string | undefined; + let temporaryCreated = false; let release: (() => Promise) | undefined; try { await fs.mkdir(dataDir, { recursive: true }); await fs.writeFile(configPath, '', { flag: 'a', mode: 0o600 }); + await fs.chmod(configPath, 0o600); // Replace the real file, preserving any symlink used to manage profile settings. // Keep the temporary file on the target filesystem so rename remains atomic. const targetPath = await fs.realpath(configPath); @@ -33,12 +35,15 @@ export async function writeTuiStatusLineSetting( if (items === undefined) delete nextTui.statusLine; else nextTui.statusLine = [...items]; const next = { ...document, tui: nextTui }; - const mode = (await fs.stat(targetPath)).mode & 0o777; - await fs.writeFile(temporaryPath, yaml.dump(next, { lineWidth: -1, noRefs: true }), { - encoding: 'utf8', - mode, - }); - await fs.chmod(temporaryPath, mode); + const mode = 0o600; + const temporary = await fs.open(temporaryPath, 'wx', mode); + temporaryCreated = true; + try { + await temporary.writeFile(yaml.dump(next, { lineWidth: -1, noRefs: true }), 'utf8'); + await temporary.chmod(mode); + } finally { + await temporary.close(); + } await fs.rename(temporaryPath, targetPath); } catch { // Parser errors can contain unrelated credentials from the config source. @@ -46,7 +51,7 @@ export async function writeTuiStatusLineSetting( 'Unable to save status line settings. Check config.yaml syntax and permissions.', ); } finally { - if (temporaryPath) await fs.unlink(temporaryPath).catch(() => undefined); + if (temporaryCreated && temporaryPath) await fs.unlink(temporaryPath).catch(() => undefined); await release?.().catch(() => undefined); } } diff --git a/packages/config/test/config-file-permissions.test.ts b/packages/config/test/config-file-permissions.test.ts new file mode 100644 index 0000000..bc22d65 --- /dev/null +++ b/packages/config/test/config-file-permissions.test.ts @@ -0,0 +1,294 @@ +import fs from "node:fs"; +import os from "node:os"; +import { join } from "node:path"; +import { spawnSync } from "node:child_process"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { + getConfig, + getConfigPath, + resetConfig, + setLegacyByokProviderMigrationEnabled, + setManagedPresetBaseUrlSyncEnabled, +} from "../src/config.js"; +import { updateLocalByokConfig } from "../src/local-model-provider-write.js"; +import { writeTuiStatusLineSetting } from "../src/tui-status-line-write.js"; +import { setCuBackend } from "../src/cu-backend-io.js"; +import { writePrivateConfigFileSync } from "../src/private-config-file.js"; +import { migrateLegacyByokProvidersOnDisk } from "../src/byok-config.js"; +import { + updateLocalConfigFile, + updateLocalByokConfig as updateLegacyByok, +} from "../../local-runtime/src/config/update.js"; + +const secret = "synthetic-config-permissions-key"; +const document = `custom_provider:\n example:\n options:\n apiKey: ${secret}\n models: {}\n`; +let root: string; +let dataDir: string; +let configPath: string; + +// Windows chmod does not express an owner-only ACL; these assertions are POSIX only. +describe.skipIf(process.platform === "win32")( + "credential-bearing config permissions", + () => { + beforeEach(() => { + root = fs.mkdtempSync(join(os.tmpdir(), "mcode-config-permissions-")); + dataDir = join(root, "profile"); + fs.mkdirSync(dataDir); + vi.spyOn(os, "homedir").mockReturnValue(root); + vi.stubEnv("MINIMAX_DATA_DIR", dataDir); + vi.stubEnv("__MAVIS_RUNTIME_MANAGED", "0"); + resetConfig(); + setLegacyByokProviderMigrationEnabled(false); + setManagedPresetBaseUrlSyncEnabled(false); + configPath = getConfigPath(); + expect(configPath).toBe(join(dataDir, "config.yaml")); + }); + + afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllEnvs(); + resetConfig(); + setLegacyByokProviderMigrationEnabled(true); + setManagedPresetBaseUrlSyncEnabled(true); + fs.rmSync(root, { recursive: true, force: true }); + }); + + const writers = [ + [ + "BYOK", + () => + updateLocalByokConfig((draft) => { + draft.minimax_api = { apiKey: secret }; + }), + ], + [ + "legacy BYOK", + () => + updateLegacyByok((draft) => { + draft.minimax_api = { apiKey: secret }; + }), + ], + [ + "ordinary setting", + () => updateLocalConfigFile({ permissionMode: "default" }), + ], + ["status line", () => writeTuiStatusLineSetting(dataDir, ["model"])], + ["CU backend", () => setCuBackend("native")], + ] as const; + + it.each(writers)("%s creates a private config", async (_name, write) => { + await write(); + expect(fs.statSync(configPath).mode & 0o777).toBe(0o600); + }); + + it.each(writers)( + "%s repairs an existing public config and preserves credentials", + async (_name, write) => { + fs.writeFileSync(configPath, document); + fs.chmodSync(configPath, 0o644); + await write(); + expect(fs.statSync(configPath).mode & 0o777).toBe(0o600); + expect(fs.readFileSync(configPath, "utf8")).toContain(secret); + }, + ); + + it.each(writers.slice(0, 4))( + "%s keeps the old file and temporary credentials private on rename failure", + async (_name, write) => { + fs.writeFileSync(configPath, document); + fs.chmodSync(configPath, 0o644); + let temporaryMode: number | undefined; + vi.spyOn(fs.promises, "rename").mockImplementation(async (source) => { + temporaryMode = fs.statSync(source).mode & 0o777; + throw new Error("synthetic rename failure"); + }); + await expect(write()).rejects.toThrow(); + expect(temporaryMode).toBe(0o600); + expect(fs.statSync(configPath).mode & 0o777).toBe(0o600); + expect(fs.readFileSync(configPath, "utf8")).toBe(document); + expect(fs.readdirSync(dataDir)).toEqual(["config.yaml"]); + }, + ); + + it.each(writers.slice(0, 4))( + "%s aborts before mutation if permissions cannot be restricted", + async (_name, write) => { + fs.writeFileSync(configPath, document); + vi.spyOn(fs.promises, "chmod").mockRejectedValue( + new Error("synthetic chmod failure"), + ); + await expect(write()).rejects.toThrow(); + expect(fs.readFileSync(configPath, "utf8")).toBe(document); + expect(fs.readdirSync(dataDir)).toEqual(["config.yaml"]); + }, + ); + + it("repairs an existing config when loaded without an edit", () => { + fs.writeFileSync(configPath, document); + fs.chmodSync(configPath, 0o644); + getConfig(); + expect(fs.statSync(configPath).mode & 0o777).toBe(0o600); + expect(fs.readFileSync(configPath, "utf8")).toBe(document); + }); + + it.each([0o400, 0o600])("loads an already-private %i config without chmod", (mode) => { + fs.writeFileSync(configPath, document, { mode }); + const chmod = vi.spyOn(fs, "chmodSync").mockImplementation(() => { + throw new Error("synthetic read-only filesystem"); + }); + expect(() => getConfig()).not.toThrow(); + expect(chmod).not.toHaveBeenCalled(); + expect(fs.statSync(configPath).mode & 0o777).toBe(mode); + }); + + it("removes non-owner access without granting owner write permission", () => { + fs.writeFileSync(configPath, document); + fs.chmodSync(configPath, 0o440); + getConfig(); + expect(fs.statSync(configPath).mode & 0o777).toBe(0o400); + }); + + it("still rejects loading an unsafe main config when chmod fails", () => { + fs.writeFileSync(configPath, document); + fs.chmodSync(configPath, 0o644); + vi.spyOn(fs, "chmodSync").mockImplementation(() => { + throw new Error("synthetic chmod failure"); + }); + expect(() => getConfig()).toThrow("synthetic chmod failure"); + expect(fs.readFileSync(configPath, "utf8")).toBe(document); + }); + + it.skipIf(process.platform !== "darwin")("loads immutable private config and backup files", () => { + const backup = join(dataDir, "config.yaml.bak.byok-legacy-provider.immutable"); + const files = [configPath, backup]; + try { + for (const file of files) { + fs.writeFileSync(file, document, { mode: 0o400 }); + const result = spawnSync("chflags", ["uchg", file], { encoding: "utf8" }); + expect(result.status, result.stderr).toBe(0); + } + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + expect(() => getConfig()).not.toThrow(); + expect(warn).not.toHaveBeenCalled(); + for (const file of files) expect(fs.statSync(file).mode & 0o777).toBe(0o400); + } finally { + for (const file of files) spawnSync("chflags", ["nouchg", file]); + } + }); + + it("creates managed defaults privately", () => { + vi.stubEnv("__MAVIS_RUNTIME_MANAGED", "1"); + getConfig(); + expect(fs.statSync(configPath).mode & 0o777).toBe(0o600); + }); + + it("copies default-profile credentials into a private config", () => { + const defaults = join(root, ".minimax"); + fs.mkdirSync(defaults); + fs.writeFileSync(join(defaults, "config.yaml"), document, { + mode: 0o644, + }); + getConfig(); + expect(fs.statSync(configPath).mode & 0o777).toBe(0o600); + expect(fs.readFileSync(configPath, "utf8")).toBe(document); + }); + + it("repairs old migration backups even when migration is disabled", () => { + fs.writeFileSync(configPath, document); + const backup = join(dataDir, "config.yaml.bak.byok-legacy-provider.123"); + fs.writeFileSync(backup, document); + fs.chmodSync(backup, 0o644); + getConfig(); + expect(fs.statSync(backup).mode & 0o777).toBe(0o600); + expect(fs.readFileSync(backup, "utf8")).toBe(document); + }); + + it("warns on backup repair failure, loads the config and repairs remaining backups", () => { + fs.writeFileSync(configPath, document, { mode: 0o600 }); + const failed = join(dataDir, "config.yaml.bak.byok-legacy-provider.1"); + const repaired = join(dataDir, "config.yaml.bak.byok-legacy-provider.2"); + for (const backup of [failed, repaired]) { + fs.writeFileSync(backup, document); + fs.chmodSync(backup, 0o644); + } + const chmod = fs.chmodSync; + vi.spyOn(fs, "chmodSync").mockImplementation((file, mode) => { + if (file === failed) throw new Error(secret); + chmod(file, mode); + }); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + expect(() => getConfig()).not.toThrow(); + expect(fs.statSync(failed).mode & 0o777).toBe(0o644); + expect(fs.statSync(repaired).mode & 0o777).toBe(0o600); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn).toHaveBeenCalledWith(expect.stringContaining(JSON.stringify(failed))); + expect(warn).toHaveBeenCalledWith(expect.stringContaining("restrict its permissions manually")); + expect(JSON.stringify(warn.mock.calls)).not.toContain(secret); + }); + + it("warns when backup inspection fails without blocking the main config", () => { + fs.writeFileSync(configPath, document, { mode: 0o600 }); + vi.spyOn(fs, "readdirSync").mockImplementation(() => { + throw new Error(secret); + }); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + expect(() => getConfig()).not.toThrow(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining(JSON.stringify(dataDir))); + expect(warn).toHaveBeenCalledWith(expect.stringContaining("Backup permissions were not verified")); + expect(JSON.stringify(warn.mock.calls)).not.toContain(secret); + }); + + it("keeps a symlinked status-line config and its credentials private", async () => { + const target = join(root, "linked.yaml"); + fs.writeFileSync(target, document); + fs.chmodSync(target, 0o644); + fs.symlinkSync(target, configPath); + await writeTuiStatusLineSetting(dataDir, ["model"]); + expect(fs.lstatSync(configPath).isSymbolicLink()).toBe(true); + expect(fs.statSync(target).mode & 0o777).toBe(0o600); + expect(fs.readFileSync(target, "utf8")).toContain(secret); + }); + + it("restricts synchronous copies before writing their first credential byte", () => { + const write = fs.writeFileSync; + const modes: number[] = []; + vi.spyOn(fs, "writeFileSync").mockImplementation((file, ...args) => { + if (typeof file === "number") + modes.push(fs.fstatSync(file).mode & 0o777); + return write(file, ...args); + }); + writePrivateConfigFileSync(configPath, document); + expect(modes).toEqual([0o600]); + }); + + it("does not truncate or write credentials when restriction fails", () => { + fs.writeFileSync(configPath, "original"); + vi.spyOn(fs, "fchmodSync").mockImplementation(() => { + throw new Error("synthetic permission failure"); + }); + expect(() => writePrivateConfigFileSync(configPath, document)).toThrow( + "synthetic permission failure", + ); + expect(fs.readFileSync(configPath, "utf8")).toBe("original"); + }); + + it("creates a private migration backup and repairs the migrated config", () => { + fs.writeFileSync( + configPath, + `provider:\n legacy:\n options:\n apiKey: ${secret}\n baseURL: https://example.invalid/v1\n models: {}\n`, + ); + fs.chmodSync(configPath, 0o644); + const result = migrateLegacyByokProvidersOnDisk(configPath, { + isManagedRuntime: () => true, + shouldEnforceManagedProviderProtection: () => false, + getManagedPreset: () => ({ provider: {}, defaultModel: "" }), + isManagedPresetBaseUrl: () => false, + }); + expect(result.migrated).toBe(true); + for (const file of [configPath, result.backupPath!]) { + expect(fs.statSync(file).mode & 0o777).toBe(0o600); + expect(fs.readFileSync(file, "utf8")).toContain(secret); + } + }); + }, +); diff --git a/packages/local-runtime/src/config/update.ts b/packages/local-runtime/src/config/update.ts index 3973cdb..21882e8 100644 --- a/packages/local-runtime/src/config/update.ts +++ b/packages/local-runtime/src/config/update.ts @@ -81,6 +81,7 @@ export async function updateLocalConfigFile( try { await fs.promises.mkdir(dirname(configPath), { recursive: true }); await fs.promises.writeFile(configPath, '', { flag: 'a', mode: LOCAL_CONFIG_FILE_MODE }); + await fs.promises.chmod(configPath, LOCAL_CONFIG_FILE_MODE); release = await lockfile.lock(configPath, { stale: 10_000, retries: { retries: 20, factor: 1, minTimeout: 5, maxTimeout: 25 }, @@ -138,6 +139,7 @@ export async function compareAndSetLocalModelContext( try { await fs.promises.mkdir(dirname(configPath), { recursive: true }); await fs.promises.writeFile(configPath, '', { flag: 'a', mode: LOCAL_CONFIG_FILE_MODE }); + await fs.promises.chmod(configPath, LOCAL_CONFIG_FILE_MODE); release = await lockfile.lock(configPath, { stale: 10_000, retries: { retries: 20, factor: 1, minTimeout: 5, maxTimeout: 25 }, @@ -229,6 +231,7 @@ export async function updateLocalByokConfig( try { await fs.promises.mkdir(dirname(configPath), { recursive: true }); await fs.promises.writeFile(configPath, '', { flag: 'a', mode: LOCAL_CONFIG_FILE_MODE }); + await fs.promises.chmod(configPath, LOCAL_CONFIG_FILE_MODE); release = await lockfile.lock(configPath, { stale: 10_000, retries: { retries: 20, factor: 1, minTimeout: 5, maxTimeout: 25 }, @@ -288,26 +291,24 @@ export async function updateLocalByokConfig( export async function atomicWriteFile(filePath: string, content: string): Promise { const tmpPath = join(dirname(filePath), `.config-tmp-${randomBytes(6).toString('hex')}`); + let created = false; try { - const mode = await readFilePermissionMode(filePath); - await fs.promises.writeFile(tmpPath, content, { encoding: 'utf-8', mode }); - await fs.promises.chmod(tmpPath, mode); + const mode = LOCAL_CONFIG_FILE_MODE; + const temporary = await fs.promises.open(tmpPath, 'wx', mode); + created = true; + try { + await temporary.writeFile(content, 'utf-8'); + await temporary.chmod(mode); + } finally { + await temporary.close(); + } await fs.promises.rename(tmpPath, filePath); } catch { - await fs.promises.unlink(tmpPath).catch(() => undefined); + if (created) await fs.promises.unlink(tmpPath).catch(() => undefined); throw new LocalConfigWriteError(); } } -async function readFilePermissionMode(filePath: string): Promise { - try { - return (await fs.promises.stat(filePath)).mode & 0o777; - } catch (err) { - if (isNodeError(err) && err.code === 'ENOENT') return LOCAL_CONFIG_FILE_MODE; - throw err; - } -} - function readLocalRawConfig(configPath: string): Record { if (!existsSync(configPath)) return {}; try { @@ -476,10 +477,6 @@ function toLocalConfigError(err: unknown): Error { return new LocalConfigWriteError(); } -function isNodeError(err: unknown): err is NodeJS.ErrnoException { - return err instanceof Error && 'code' in err; -} - function isPlainRecord(value: unknown): value is Record { return Boolean(value) && typeof value === 'object' && !Array.isArray(value); } diff --git a/packages/tui/src/runtime/lifecycle.ts b/packages/tui/src/runtime/lifecycle.ts index b8720af..e754374 100644 --- a/packages/tui/src/runtime/lifecycle.ts +++ b/packages/tui/src/runtime/lifecycle.ts @@ -154,6 +154,8 @@ export async function createTuiRuntime( }, }; }; + // Reject unreadable or unsafe config before auth watchers can keep a failed CLI alive. + const requestedMcodeTools = getConfig().beta?.mcodeTools === true; const readAuthContext = dependencies.readAuthContext ?? readCliAuthContext; const importSharedAuthContext = dependencies.importSharedAuthContext ?? importSharedCliAuthContext; @@ -318,7 +320,6 @@ export async function createTuiRuntime( } } } - const requestedMcodeTools = getConfig().beta?.mcodeTools === true; let mcodeToolsReadiness: McodeToolsReadiness = fallbackMcodeToolsReadiness( requestedMcodeTools, authScope.buildEnv, diff --git a/release/public-source.json b/release/public-source.json index 6b6dc3b..2e32da9 100644 --- a/release/public-source.json +++ b/release/public-source.json @@ -448,6 +448,7 @@ "packages/config/src/opencode-config.ts", "packages/config/src/permission-config.ts", "packages/config/src/pid-file.ts", + "packages/config/src/private-config-file.ts", "packages/config/src/provider-auth-mode.ts", "packages/config/src/review-config.ts", "packages/config/src/runaway-guard-config.ts", @@ -461,6 +462,7 @@ "packages/config/src/tool-result-compaction-config.ts", "packages/config/src/tui-config.ts", "packages/config/src/tui-status-line-write.ts", + "packages/config/test/config-file-permissions.test.ts", "packages/local-runtime-v2/assets/agents/_default/prompt-base-all.md", "packages/local-runtime-v2/assets/agents/_default/prompt-base-all.md.hbs", "packages/local-runtime-v2/assets/agents/_default/prompt-base-windows.md", diff --git a/test/smoke.test.mjs b/test/smoke.test.mjs index 81ae2d1..52e9856 100644 --- a/test/smoke.test.mjs +++ b/test/smoke.test.mjs @@ -1,7 +1,7 @@ import test from "node:test"; import assert from "node:assert/strict"; import { spawn, spawnSync } from "node:child_process"; -import { mkdtempSync, rmSync, readFileSync, existsSync } from "node:fs"; +import { mkdtempSync, rmSync, readFileSync, existsSync, writeFileSync, chmodSync, statSync } from "node:fs"; import { tmpdir } from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; @@ -77,6 +77,61 @@ test("provider configuration loads from an isolated data directory", (t) => { assert.doesNotMatch(result.stdout, /custom_provider:/); }); +test("config permission failures preserve private reads and terminate unsafe startup", { + skip: process.platform !== "darwin", +}, (t) => { + const options = fixture(t); + const config = path.join(options.env.MINIMAX_DATA_DIR, "config.yaml"); + const backup = `${config}.bak.byok-legacy-provider.smoke`; + const flag = (file, value) => { + const result = spawnSync("chflags", [value, file], { encoding: "utf8" }); + assert.equal(result.status, 0, result.stderr); + }; + const list = () => { + const result = spawnSync(process.execPath, [cli, "provider", "list"], { + ...options, + encoding: "utf8", + timeout: 15000, + // A leaked startup watcher can handle SIGTERM without releasing the process. + killSignal: "SIGKILL", + }); + assert.equal(result.error, undefined, result.stderr); + return result; + }; + try { + for (const file of [config, backup]) { + writeFileSync(file, "logLevel: info\n", { mode: 0o400 }); + flag(file, "uchg"); + } + const privateRead = list(); + assert.equal(privateRead.status, 0, privateRead.stderr); + assert.doesNotMatch(privateRead.stderr, /Could not restrict legacy BYOK backup/); + assert.equal(statSync(config).mode & 0o777, 0o400); + assert.equal(statSync(backup).mode & 0o777, 0o400); + + flag(backup, "nouchg"); + chmodSync(backup, 0o644); + flag(backup, "uchg"); + const backupWarning = list(); + assert.equal(backupWarning.status, 0, backupWarning.stderr); + assert.match(backupWarning.stderr, /Could not restrict legacy BYOK backup/); + assert.ok(backupWarning.stderr.includes(JSON.stringify(backup))); + assert.equal(statSync(backup).mode & 0o777, 0o644); + + flag(config, "nouchg"); + chmodSync(config, 0o644); + flag(config, "uchg"); + const unsafeRead = list(); + assert.equal(unsafeRead.status, 1, unsafeRead.stderr); + assert.match(unsafeRead.stderr, /EPERM/); + assert.equal(statSync(config).mode & 0o777, 0o644); + } finally { + for (const file of [config, backup]) { + if (existsSync(file)) flag(file, "nouchg"); + } + } +}); + test("offline smoke children ignore ambient proxy variables", async (t) => { const proxyNames = [ "HTTP_PROXY", diff --git a/test/vitest-suites.json b/test/vitest-suites.json index 251399d..f0d0fac 100644 --- a/test/vitest-suites.json +++ b/test/vitest-suites.json @@ -137,6 +137,7 @@ "packages/tui/test/unit/tui/controller/product/feature-flow.test.ts", "packages/tui/test/unit/tui/features/settings/hotkeys-picker.test.ts", "packages/tui/test/unit/incident-reporter-privacy.test.ts", + "packages/config/test/config-file-permissions.test.ts", "packages/agent-modules/context-manager/test/compaction-usage.test.ts" ], "status-contract": [