diff --git a/docs/guides/global-settings.md b/docs/guides/global-settings.md index 13408c458..1dd4167c7 100644 --- a/docs/guides/global-settings.md +++ b/docs/guides/global-settings.md @@ -10,6 +10,11 @@ hack config get --global controlPlane.gateway.allowWrites hack config set --global controlPlane.gateway.allowWrites true ``` +Automatic settings updates, including `hack env backend use`, create a missing +global config but refuse to replace an existing unreadable file, malformed JSON, +or a non-object JSON value. An empty file is invalid JSON. Repair the reported file +before retrying; the failed update preserves its contents. + Common settings: - `controlPlane.gateway.bind` (default `127.0.0.1`) diff --git a/src/lib/config.ts b/src/lib/config.ts index 7cfbcf361..15f2ebedb 100644 --- a/src/lib/config.ts +++ b/src/lib/config.ts @@ -117,9 +117,7 @@ async function updateConfigFileValuesAtPath({ } } - const jsonText = await readTextFile(configPath); - const config: Record = - jsonText !== null ? parseJsonSafe(jsonText) : {}; + const config = await readConfigForUpdate({ configPath }); for (const entry of values) { setPathValue({ @@ -136,6 +134,40 @@ async function updateConfigFileValuesAtPath({ return { changed: result.changed }; } +/** Only an absent file may be initialized; existing unreadable or invalid config must survive. */ +async function readConfigForUpdate({ + configPath, +}: { + readonly configPath: string; +}): Promise> { + let text: string; + try { + text = await Bun.file(configPath).text(); + } catch (error: unknown) { + if (isRecord(error) && error.code === "ENOENT") { + return {}; + } + throw new Error( + `Cannot update config at ${configPath}: unable to read the existing file. Check its permissions and retry.` + ); + } + + let parsed: unknown; + try { + parsed = JSON.parse(text); + } catch { + throw new Error( + `Cannot update config at ${configPath}: invalid JSON. Repair the file before retrying; it has not been changed.` + ); + } + if (!isRecord(parsed)) { + throw new Error( + `Cannot update config at ${configPath}: expected a JSON object. Repair the file before retrying; it has not been changed.` + ); + } + return parsed; +} + /** * Reads a value from the global config file. * diff --git a/tests/config.test.ts b/tests/config.test.ts index 938bd64c9..e995d1ff7 100644 --- a/tests/config.test.ts +++ b/tests/config.test.ts @@ -1,11 +1,12 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { mkdtemp, rm } from "node:fs/promises"; +import { chmod, mkdtemp, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { readGlobalConfig, updateGlobalConfig, + updateProjectConfig, updateProjectConfigBatch, } from "../src/lib/config.ts"; import { restoreEnv } from "./helpers/env.ts"; @@ -65,6 +66,60 @@ describe("global config utilities", () => { expect(parsed.controlPlane.daemon.launchd.runAtLoad).toBe(true); }); + test.each([ + ["malformed JSON", '{"privateFixtureValue":"do-not-echo",', "invalid JSON"], + ["empty file", "", "invalid JSON"], + ["whitespace", " \n\t", "invalid JSON"], + ["array", "[1, 2]\n", "expected a JSON object"], + ["null", "null\n", "expected a JSON object"], + ["string", '"do-not-echo"\n', "expected a JSON object"], + ["number", "42\n", "expected a JSON object"], + ["boolean", "true\n", "expected a JSON object"], + ])("all config writers preserve %s", async (_label, contents, reason) => { + await Bun.write(configPath, contents); + const expectedError = `Cannot update config at ${configPath}: ${reason}. Repair the file before retrying; it has not been changed.`; + const operations = [ + () => updateGlobalConfig({ path: "enabled", value: true }), + () => + updateProjectConfig({ + projectDir: tempDir, + path: "enabled", + value: true, + }), + () => + updateProjectConfigBatch({ + projectDir: tempDir, + values: [ + { path: "enabled", value: true }, + { path: "sessions.mux", value: "tmux" }, + ], + }), + ]; + for (const update of operations) { + await expect(update()).rejects.toThrow(expectedError); + expect(await Bun.file(configPath).text()).toBe(contents); + } + }); + + test.skipIf(process.getuid?.() === 0)( + "unreadable existing config is not treated as missing", + async () => { + const contents = '{"existing":true}\n'; + await Bun.write(configPath, contents); + await chmod(configPath, 0o200); + try { + await expect( + updateGlobalConfig({ path: "enabled", value: true }) + ).rejects.toThrow( + `Cannot update config at ${configPath}: unable to read the existing file.` + ); + } finally { + await chmod(configPath, 0o600); + } + expect(await Bun.file(configPath).text()).toBe(contents); + } + ); + test("readGlobalConfig returns undefined for missing config", async () => { const value = await readGlobalConfig({ path: "controlPlane.daemon.launchd.installed", @@ -154,6 +209,35 @@ describe("project config batch utilities", () => { await rm(tempDir, { recursive: true, force: true }); }); + test("single project update creates a missing config and preserves unrelated fields", async () => { + const values = { nested: { enabled: true }, list: ["web", "db"] }; + await updateProjectConfig({ projectDir, path: "custom", value: values }); + await updateProjectConfig({ projectDir, path: "name", value: "example" }); + expect(await Bun.file(configPath).json()).toEqual({ + custom: values, + name: "example", + }); + }); + + test("batch update preserves unrelated and sibling fields", async () => { + await Bun.write( + configPath, + JSON.stringify({ custom: ["untouched"], sessions: { existing: true } }) + ); + await updateProjectConfigBatch({ + projectDir, + values: [ + { path: "name", value: "example" }, + { path: "sessions.mux", value: "tmux" }, + ], + }); + expect(await Bun.file(configPath).json()).toEqual({ + custom: ["untouched"], + sessions: { existing: true, mux: "tmux" }, + name: "example", + }); + }); + test("updateProjectConfigBatch persists multi-key routing overrides in one write", async () => { await updateProjectConfigBatch({ projectDir, diff --git a/tests/env-backend-command.test.ts b/tests/env-backend-command.test.ts index d04a0d734..c1481a4af 100644 --- a/tests/env-backend-command.test.ts +++ b/tests/env-backend-command.test.ts @@ -83,6 +83,28 @@ test( { timeout: 20_000 } ); +test( + "env backend use refuses invalid existing config without exposing or overwriting it", + async () => { + if (!tempGlobalConfigPath) { + throw new Error("Missing temp global config state"); + } + const contents = '{"fixtureOnly":"do-not-echo",'; + await writeFile(tempGlobalConfigPath, contents); + const result = await runHack({ + args: ["env", "backend", "use", "encrypted_file", "--json"], + env: { ...process.env, HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath }, + }); + expect(result.exitCode).not.toBe(0); + expect(`${result.stdout}${result.stderr}`).toContain( + `Cannot update config at ${tempGlobalConfigPath}: invalid JSON` + ); + expect(`${result.stdout}${result.stderr}`).not.toContain("do-not-echo"); + expect(await readFile(tempGlobalConfigPath, "utf8")).toBe(contents); + }, + { timeout: 20_000 } +); + test( "env backend use encrypted_file persists selection", async () => {