From a5536caa387221e3461020dce2b92db37570e011 Mon Sep 17 00:00:00 2001 From: hetaoBackend Date: Sat, 19 Sep 2026 07:21:53 +0800 Subject: [PATCH 1/5] fix: restrict credential-bearing config files to owner access Repair existing POSIX config and migration backup permissions, and keep all config writers and temporary files private. Fixes #159. --- docs/examples.md | 2 + packages/config/src/byok-config.ts | 22 +- packages/config/src/config.ts | 12 +- packages/config/src/cu-backend-io.ts | 3 +- .../config/src/local-model-provider-write.ts | 29 +-- packages/config/src/private-config-file.ts | 30 +++ packages/config/src/tui-status-line-write.ts | 19 +- .../test/config-file-permissions.test.ts | 213 ++++++++++++++++++ packages/local-runtime/src/config/update.ts | 31 ++- release/public-source.json | 2 + test/vitest-suites.json | 3 +- 11 files changed, 314 insertions(+), 52 deletions(-) create mode 100644 packages/config/src/private-config-file.ts create mode 100644 packages/config/test/config-file-permissions.test.ts diff --git a/docs/examples.md b/docs/examples.md index 40e2e6897..4ca97e003 100644 --- a/docs/examples.md +++ b/docs/examples.md @@ -52,6 +52,8 @@ pnpm mcode exec "Explain this project's test entry points" --model Replace the example URL, model name, and IDs with your configuration and the IDs returned by the list command. `--use` sets the default model; `exec --model` overrides only the current run. Backslash line continuations are for POSIX shells; use a single line in PowerShell. +`--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, MCode restricts the config and its migration backups to mode `0600`, including existing files when loaded. Config writes and temporary copies also use `0600`. 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 595ffeb05..5d659a226 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,24 @@ function rewriteNexusModelProvider( return true; } +/** Repair credential-bearing backups produced by older versions on profile load. */ +export function restrictLegacyByokBackups(configPath: string): void { + if (process.platform === 'win32') return; + const directory = path.dirname(configPath); + for (const entry of fs.readdirSync(directory, { withFileTypes: true })) { + if (entry.isFile() && entry.name.startsWith(LEGACY_BYOK_BACKUP_PREFIX)) { + restrictConfigFileSync(path.join(directory, entry.name)); + } + } +} + 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 74254edeb..db9f71090 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 dc5523841..49d8668cb 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 fa521527f..8a19a428d 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 000000000..2e289be1c --- /dev/null +++ b/packages/config/src/private-config-file.ts @@ -0,0 +1,30 @@ +import fs from "node:fs"; + +/** Config documents and their copies can contain plaintext credentials. */ +export const PRIVATE_CONFIG_FILE_MODE = 0o600; + +/** Tighten existing POSIX files as well as newly created ones. */ +export function restrictConfigFileSync(filePath: string): void { + if (process.platform !== "win32") + fs.chmodSync(filePath, PRIVATE_CONFIG_FILE_MODE); +} + +/** 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 d799366e8..e24f54b06 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 000000000..8fcbaadb7 --- /dev/null +++ b/packages/config/test/config-file-permissions.test.ts @@ -0,0 +1,213 @@ +import fs from "node:fs"; +import os from "node:os"; +import { join } from "node:path"; +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("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("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 3973cdb6d..21882e86f 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/release/public-source.json b/release/public-source.json index b9c1ff4f2..75ac06958 100644 --- a/release/public-source.json +++ b/release/public-source.json @@ -439,6 +439,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", @@ -452,6 +453,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/vitest-suites.json b/test/vitest-suites.json index 3568d3578..938cbb3a0 100644 --- a/test/vitest-suites.json +++ b/test/vitest-suites.json @@ -133,7 +133,8 @@ "packages/tui/test/unit/tui/controller/product/command-flow.test.ts", "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/tui/test/unit/incident-reporter-privacy.test.ts", + "packages/config/test/config-file-permissions.test.ts" ], "status-contract": [ "packages/tui/test/unit/tui-build-mode-contract.test.ts" From 79a69b8f05a2bc2a92f168658f642e156ddcc641 Mon Sep 17 00:00:00 2001 From: hetaoBackend Date: Sat, 19 Sep 2026 11:55:51 +0800 Subject: [PATCH 2/5] test: bound Windows filesystem concurrency and runtime startup --- .agents/skills/testing-workflow/SKILL.md | 6 ++++++ scripts/run-vitest-suite.mjs | 4 ++++ test/smoke.test.mjs | 21 ++++++++++++++------ test/source-sync.test.mjs | 25 ++++++++++++++---------- 4 files changed, 40 insertions(+), 16 deletions(-) diff --git a/.agents/skills/testing-workflow/SKILL.md b/.agents/skills/testing-workflow/SKILL.md index cf44573ab..512bd7c73 100644 --- a/.agents/skills/testing-workflow/SKILL.md +++ b/.agents/skills/testing-workflow/SKILL.md @@ -39,6 +39,12 @@ suites are outside this distribution's verification. Artifact-dependent tests require a current `pnpm build`. Keep test state and reports outside the repository, using synthetic data and temporary directories. +The declared Vitest gate runner serializes test files on Windows to limit +filesystem contention while retaining individual test deadlines. Run the owning +gate for platform acceptance; a direct Vitest invocation uses its default worker +count. Full-runtime smoke children have a 30-second Windows startup budget, +matching ACP; lightweight help and validation checks retain 15 seconds. + ## Delivery verification Run `git diff --check` and the relevant individual gates while editing. Review diff --git a/scripts/run-vitest-suite.mjs b/scripts/run-vitest-suite.mjs index 19f23f500..ffc0c8909 100644 --- a/scripts/run-vitest-suite.mjs +++ b/scripts/run-vitest-suite.mjs @@ -26,6 +26,10 @@ const result = spawnSync( "run", "--config", "vitest.oss.config.mjs", + // These suites create many SQLite databases and watched directories. On + // Windows, concurrent files can exhaust the 5s test budget through I/O + // contention. Serialize files without relaxing individual test deadlines. + ...(process.platform === "win32" ? ["--maxWorkers", "1"] : []), ...files, ], { stdio: "inherit", cwd: repositoryRoot, env: environment }, diff --git a/test/smoke.test.mjs b/test/smoke.test.mjs index 81ae2d193..8dc2b409a 100644 --- a/test/smoke.test.mjs +++ b/test/smoke.test.mjs @@ -9,6 +9,14 @@ import Database from "better-sqlite3"; const root = fileURLToPath(new URL("../", import.meta.url)); const cli = path.join(root, "dist/cli.js"); +// Full runtime startup can exceed 15s on Windows CI (ACP took 22s). +// Match the ACP startup budget; lightweight help/validation stays at 15s. +const runtimeTimeoutMs = process.platform === "win32" ? 30000 : 15000; +function assertSuccessfulChild(result) { + assert.equal(result.error, undefined, + `CLI spawn failed: ${result.error?.message}; signal=${result.signal}; stderr=${result.stderr}`); + assert.equal(result.status, 0, result.stderr); +} const version = JSON.parse( readFileSync(path.join(root, "packages/tui/package.json"), "utf8"), ).version; @@ -46,6 +54,7 @@ function fixture(t, environment = process.env) { MAVIS_DATA_DIR: dataDir, MCODE_TEST_NETWORK_AUDIT: audit, MCODE_TEST_MANAGED_OFFLINE: "1", + MCODE_TEST_PROCESS_PROBE: "1", NODE_OPTIONS: `--import=${new URL("./network-deny.mjs", import.meta.url).href}`, }, }; @@ -70,9 +79,9 @@ test("provider configuration loads from an isolated data directory", (t) => { const result = spawnSync(process.execPath, [cli, "provider", "list"], { ...fixture(t), encoding: "utf8", - timeout: 15000, + timeout: runtimeTimeoutMs, }); - assert.equal(result.status, 0, result.stderr); + assertSuccessfulChild(result); assert.match(result.stdout, /minimax/); assert.doesNotMatch(result.stdout, /custom_provider:/); @@ -97,9 +106,9 @@ test("offline smoke children ignore ambient proxy variables", async (t) => { const result = spawnSync(process.execPath, [cli, "provider", "list"], { ...options, encoding: "utf8", - timeout: 15000, + timeout: runtimeTimeoutMs, }); - assert.equal(result.status, 0, result.stderr); + assertSuccessfulChild(result); assert.match(result.stdout, /minimax/); assert.match( readFileSync(options.env.MCODE_TEST_NETWORK_AUDIT + ".managed", "utf8"), @@ -225,9 +234,9 @@ test("local plugin browsing remains available with managed services offline", (t const result = spawnSync(process.execPath, [cli, ...args], { ...options, encoding: "utf8", - timeout: 15000, + timeout: runtimeTimeoutMs, }); - assert.equal(result.status, 0, result.stderr); + assertSuccessfulChild(result); const output = JSON.parse(result.stdout); assert.ok(output !== null); assert.doesNotMatch(result.stdout, /"official"/); diff --git a/test/source-sync.test.mjs b/test/source-sync.test.mjs index b0caead77..c1085e1aa 100644 --- a/test/source-sync.test.mjs +++ b/test/source-sync.test.mjs @@ -404,16 +404,21 @@ test('suite runner preserves gate arguments and canonicalizes Windows temporary writeFileSync(path.join(root, 'node_modules/vitest/cli.cjs'), 'console.log(JSON.stringify({args:process.argv.slice(2),temp:process.env.TEMP,tmp:process.env.TMP,cwd:process.cwd()})); process.exit(17);'); const alias = path.join(root, 'temporary-alias'); symlinkSync(path.join(root, 'temporary'), alias, process.platform === 'win32' ? 'junction' : 'dir'); - const result = spawnSync(process.execPath, [path.join(root, 'scripts/run-vitest-suite.mjs'), 'fixture'], { - encoding: 'utf8', env: { ...process.env, TEMP: alias, TMP: alias }, - }); - assert.equal(result.status, 17, result.stderr); - const child = JSON.parse(result.stdout); - assert.deepEqual(child.args, ['run', '--config', 'vitest.oss.config.mjs', 'test/example.test.ts']); - assert.equal(realpathSync(child.cwd), realpathSync(root)); - const expected = process.platform === 'win32' ? realpathSync.native(alias) : alias; - assert.equal(child.temp, expected); - assert.equal(child.tmp, expected); + for (const platform of ['linux', 'darwin', 'win32']) { + const preload = path.join(root, 'platform.cjs'); + writeFileSync(preload, `Object.defineProperty(process, 'platform', { value: ${JSON.stringify(platform)} });`); + const result = spawnSync(process.execPath, ['--require', preload, path.join(root, 'scripts/run-vitest-suite.mjs'), 'fixture'], { + encoding: 'utf8', env: { ...process.env, TEMP: alias, TMP: alias, TMPDIR: alias }, + }); + assert.equal(result.status, 17, result.stderr); + const child = JSON.parse(result.stdout); + assert.deepEqual(child.args, ['run', '--config', 'vitest.oss.config.mjs', + ...(platform === 'win32' ? ['--maxWorkers', '1'] : []), 'test/example.test.ts']); + assert.equal(realpathSync(child.cwd), realpathSync(root)); + const expected = platform === 'win32' ? realpathSync.native(alias) : alias; + assert.equal(child.temp, expected); + assert.equal(child.tmp, expected); + } }); test('public support forms preserve destination URLs and separate Desktop from CLI reports', () => { From e7115f84dc6cdfe385af6584ac081ca735505d4e Mon Sep 17 00:00:00 2001 From: hetaoBackend Date: Sat, 19 Sep 2026 12:58:06 +0800 Subject: [PATCH 3/5] Revert "test: bound Windows filesystem concurrency and runtime startup" This reverts commit 79a69b8f05a2bc2a92f168658f642e156ddcc641. --- .agents/skills/testing-workflow/SKILL.md | 6 ------ scripts/run-vitest-suite.mjs | 4 ---- test/smoke.test.mjs | 21 ++++++-------------- test/source-sync.test.mjs | 25 ++++++++++-------------- 4 files changed, 16 insertions(+), 40 deletions(-) diff --git a/.agents/skills/testing-workflow/SKILL.md b/.agents/skills/testing-workflow/SKILL.md index 512bd7c73..cf44573ab 100644 --- a/.agents/skills/testing-workflow/SKILL.md +++ b/.agents/skills/testing-workflow/SKILL.md @@ -39,12 +39,6 @@ suites are outside this distribution's verification. Artifact-dependent tests require a current `pnpm build`. Keep test state and reports outside the repository, using synthetic data and temporary directories. -The declared Vitest gate runner serializes test files on Windows to limit -filesystem contention while retaining individual test deadlines. Run the owning -gate for platform acceptance; a direct Vitest invocation uses its default worker -count. Full-runtime smoke children have a 30-second Windows startup budget, -matching ACP; lightweight help and validation checks retain 15 seconds. - ## Delivery verification Run `git diff --check` and the relevant individual gates while editing. Review diff --git a/scripts/run-vitest-suite.mjs b/scripts/run-vitest-suite.mjs index ffc0c8909..19f23f500 100644 --- a/scripts/run-vitest-suite.mjs +++ b/scripts/run-vitest-suite.mjs @@ -26,10 +26,6 @@ const result = spawnSync( "run", "--config", "vitest.oss.config.mjs", - // These suites create many SQLite databases and watched directories. On - // Windows, concurrent files can exhaust the 5s test budget through I/O - // contention. Serialize files without relaxing individual test deadlines. - ...(process.platform === "win32" ? ["--maxWorkers", "1"] : []), ...files, ], { stdio: "inherit", cwd: repositoryRoot, env: environment }, diff --git a/test/smoke.test.mjs b/test/smoke.test.mjs index 8dc2b409a..81ae2d193 100644 --- a/test/smoke.test.mjs +++ b/test/smoke.test.mjs @@ -9,14 +9,6 @@ import Database from "better-sqlite3"; const root = fileURLToPath(new URL("../", import.meta.url)); const cli = path.join(root, "dist/cli.js"); -// Full runtime startup can exceed 15s on Windows CI (ACP took 22s). -// Match the ACP startup budget; lightweight help/validation stays at 15s. -const runtimeTimeoutMs = process.platform === "win32" ? 30000 : 15000; -function assertSuccessfulChild(result) { - assert.equal(result.error, undefined, - `CLI spawn failed: ${result.error?.message}; signal=${result.signal}; stderr=${result.stderr}`); - assert.equal(result.status, 0, result.stderr); -} const version = JSON.parse( readFileSync(path.join(root, "packages/tui/package.json"), "utf8"), ).version; @@ -54,7 +46,6 @@ function fixture(t, environment = process.env) { MAVIS_DATA_DIR: dataDir, MCODE_TEST_NETWORK_AUDIT: audit, MCODE_TEST_MANAGED_OFFLINE: "1", - MCODE_TEST_PROCESS_PROBE: "1", NODE_OPTIONS: `--import=${new URL("./network-deny.mjs", import.meta.url).href}`, }, }; @@ -79,9 +70,9 @@ test("provider configuration loads from an isolated data directory", (t) => { const result = spawnSync(process.execPath, [cli, "provider", "list"], { ...fixture(t), encoding: "utf8", - timeout: runtimeTimeoutMs, + timeout: 15000, }); - assertSuccessfulChild(result); + assert.equal(result.status, 0, result.stderr); assert.match(result.stdout, /minimax/); assert.doesNotMatch(result.stdout, /custom_provider:/); @@ -106,9 +97,9 @@ test("offline smoke children ignore ambient proxy variables", async (t) => { const result = spawnSync(process.execPath, [cli, "provider", "list"], { ...options, encoding: "utf8", - timeout: runtimeTimeoutMs, + timeout: 15000, }); - assertSuccessfulChild(result); + assert.equal(result.status, 0, result.stderr); assert.match(result.stdout, /minimax/); assert.match( readFileSync(options.env.MCODE_TEST_NETWORK_AUDIT + ".managed", "utf8"), @@ -234,9 +225,9 @@ test("local plugin browsing remains available with managed services offline", (t const result = spawnSync(process.execPath, [cli, ...args], { ...options, encoding: "utf8", - timeout: runtimeTimeoutMs, + timeout: 15000, }); - assertSuccessfulChild(result); + assert.equal(result.status, 0, result.stderr); const output = JSON.parse(result.stdout); assert.ok(output !== null); assert.doesNotMatch(result.stdout, /"official"/); diff --git a/test/source-sync.test.mjs b/test/source-sync.test.mjs index c1085e1aa..b0caead77 100644 --- a/test/source-sync.test.mjs +++ b/test/source-sync.test.mjs @@ -404,21 +404,16 @@ test('suite runner preserves gate arguments and canonicalizes Windows temporary writeFileSync(path.join(root, 'node_modules/vitest/cli.cjs'), 'console.log(JSON.stringify({args:process.argv.slice(2),temp:process.env.TEMP,tmp:process.env.TMP,cwd:process.cwd()})); process.exit(17);'); const alias = path.join(root, 'temporary-alias'); symlinkSync(path.join(root, 'temporary'), alias, process.platform === 'win32' ? 'junction' : 'dir'); - for (const platform of ['linux', 'darwin', 'win32']) { - const preload = path.join(root, 'platform.cjs'); - writeFileSync(preload, `Object.defineProperty(process, 'platform', { value: ${JSON.stringify(platform)} });`); - const result = spawnSync(process.execPath, ['--require', preload, path.join(root, 'scripts/run-vitest-suite.mjs'), 'fixture'], { - encoding: 'utf8', env: { ...process.env, TEMP: alias, TMP: alias, TMPDIR: alias }, - }); - assert.equal(result.status, 17, result.stderr); - const child = JSON.parse(result.stdout); - assert.deepEqual(child.args, ['run', '--config', 'vitest.oss.config.mjs', - ...(platform === 'win32' ? ['--maxWorkers', '1'] : []), 'test/example.test.ts']); - assert.equal(realpathSync(child.cwd), realpathSync(root)); - const expected = platform === 'win32' ? realpathSync.native(alias) : alias; - assert.equal(child.temp, expected); - assert.equal(child.tmp, expected); - } + const result = spawnSync(process.execPath, [path.join(root, 'scripts/run-vitest-suite.mjs'), 'fixture'], { + encoding: 'utf8', env: { ...process.env, TEMP: alias, TMP: alias }, + }); + assert.equal(result.status, 17, result.stderr); + const child = JSON.parse(result.stdout); + assert.deepEqual(child.args, ['run', '--config', 'vitest.oss.config.mjs', 'test/example.test.ts']); + assert.equal(realpathSync(child.cwd), realpathSync(root)); + const expected = process.platform === 'win32' ? realpathSync.native(alias) : alias; + assert.equal(child.temp, expected); + assert.equal(child.tmp, expected); }); test('public support forms preserve destination URLs and separate Desktop from CLI reports', () => { From 400a025182029173d6163ad5c9f9277b31534cf7 Mon Sep 17 00:00:00 2001 From: hetaoBackend Date: Sat, 19 Sep 2026 13:01:22 +0800 Subject: [PATCH 4/5] fix: preserve private read-only config during permission repair --- docs/examples.md | 2 +- packages/config/src/byok-config.ts | 25 +++++- packages/config/src/private-config-file.ts | 9 ++- .../test/config-file-permissions.test.ts | 81 +++++++++++++++++++ 4 files changed, 110 insertions(+), 7 deletions(-) diff --git a/docs/examples.md b/docs/examples.md index 9086bc203..c162993dd 100644 --- a/docs/examples.md +++ b/docs/examples.md @@ -64,7 +64,7 @@ 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, MCode restricts the config and its migration backups to mode `0600`, including existing files when loaded. Config writes and temporary copies also use `0600`. Windows file modes do not provide equivalent ACL protection; restrict access to the profile directory using Windows permissions. +`--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. diff --git a/packages/config/src/byok-config.ts b/packages/config/src/byok-config.ts index 5d659a226..99b9c1bcd 100644 --- a/packages/config/src/byok-config.ts +++ b/packages/config/src/byok-config.ts @@ -474,13 +474,32 @@ function rewriteNexusModelProvider( return true; } -/** Repair credential-bearing backups produced by older versions on profile load. */ +/** 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); - for (const entry of fs.readdirSync(directory, { withFileTypes: true })) { + 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)) { - restrictConfigFileSync(path.join(directory, entry.name)); + 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.', + ); + } } } } diff --git a/packages/config/src/private-config-file.ts b/packages/config/src/private-config-file.ts index 2e289be1c..82e4f0907 100644 --- a/packages/config/src/private-config-file.ts +++ b/packages/config/src/private-config-file.ts @@ -3,10 +3,13 @@ import fs from "node:fs"; /** Config documents and their copies can contain plaintext credentials. */ export const PRIVATE_CONFIG_FILE_MODE = 0o600; -/** Tighten existing POSIX files as well as newly created ones. */ +/** Remove non-owner access without changing an owner's read-only policy. */ export function restrictConfigFileSync(filePath: string): void { - if (process.platform !== "win32") - fs.chmodSync(filePath, PRIVATE_CONFIG_FILE_MODE); + 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. */ diff --git a/packages/config/test/config-file-permissions.test.ts b/packages/config/test/config-file-permissions.test.ts index 8fcbaadb7..bc22d6549 100644 --- a/packages/config/test/config-file-permissions.test.ts +++ b/packages/config/test/config-file-permissions.test.ts @@ -1,6 +1,7 @@ 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, @@ -130,6 +131,51 @@ describe.skipIf(process.platform === "win32")( 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(); @@ -157,6 +203,41 @@ describe.skipIf(process.platform === "win32")( 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); From 9ce8e562a0a0d731e8449a59a12562f895ae846a Mon Sep 17 00:00:00 2001 From: hetaoBackend Date: Sat, 19 Sep 2026 13:07:42 +0800 Subject: [PATCH 5/5] fix: validate config before starting auth watchers --- packages/tui/src/runtime/lifecycle.ts | 3 +- test/smoke.test.mjs | 57 ++++++++++++++++++++++++++- 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/packages/tui/src/runtime/lifecycle.ts b/packages/tui/src/runtime/lifecycle.ts index b8720affa..e75437408 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/test/smoke.test.mjs b/test/smoke.test.mjs index 81ae2d193..52e98561f 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",