From a2ab65fb656918f26e443e671d0b0e9624042335 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Tue, 21 Jul 2026 12:33:15 +0200 Subject: [PATCH 01/19] fix(http): normalize response timeout errors Map connect and body-read aborts to the CLI TimeoutError contract so streaming and buffered requests consistently exit with network status 9. --- cli/src/lib/http.test.ts | 7 +++++-- cli/src/lib/http.ts | 39 +++++++++++++++++++++++++++++++++------ 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/cli/src/lib/http.test.ts b/cli/src/lib/http.test.ts index 5416a9f..66e2df5 100644 --- a/cli/src/lib/http.test.ts +++ b/cli/src/lib/http.test.ts @@ -458,8 +458,11 @@ describe("httpSendStream live timeouts", () => { await reader.read(); expect.unreachable("stream read should have timed out"); } catch (error) { - expect(error).toBeInstanceOf(DOMException); - expect((error as DOMException).message).toBe("stream timed out"); + expect(error).toBeInstanceOf(TimeoutError); + expect((error as TimeoutError).exitCode).toBe(9); + expect((error as TimeoutError).message).toBe( + "Request timed out: POST https://example.com/query", + ); } expect(abortObserved).toBe(true); } finally { diff --git a/cli/src/lib/http.ts b/cli/src/lib/http.ts index 3926cad..b8a83e0 100644 --- a/cli/src/lib/http.ts +++ b/cli/src/lib/http.ts @@ -131,6 +131,8 @@ function connectionError(options: HttpSendOptions, cause: unknown): NetworkError function streamWithTimeoutCleanup( stream: ReadableStream, clearTimeoutAfterRead: () => void, + signal: AbortSignal, + options: HttpStreamOptions, ): ReadableStream { let reader: ReturnType | undefined; return new ReadableStream({ @@ -149,7 +151,11 @@ function streamWithTimeoutCleanup( } } catch (error) { clearTimeoutAfterRead(); - controller.error(error); + controller.error( + signal.aborted || isAbortError(error) + ? timeoutError(options, error) + : connectionError(options, error), + ); } }, cancel(reason) { @@ -408,13 +414,21 @@ async function executeLiveRequest(options: HttpSendOptions): Promise { dispatcher: getSharedDispatcher(), } as RequestInit); } catch (error) { - if (isAbortError(error)) { + if (signal.aborted || isAbortError(error)) { throw timeoutError(options, error); } throw connectionError(options, error); } - const responseBody = await response.text(); + let responseBody: string; + try { + responseBody = await response.text(); + } catch (error) { + if (signal.aborted || isAbortError(error)) { + throw timeoutError(options, error); + } + throw connectionError(options, error); + } logHttpDebug([ `< HTTP/${response.status}`, @@ -466,7 +480,7 @@ async function executeLiveStream(options: HttpStreamOptions): Promise { abortController.abort(); }, readTimeoutMs); - return streamWithTimeoutCleanup(response.body, clearActiveTimeout); + return streamWithTimeoutCleanup( + response.body, + clearActiveTimeout, + abortController.signal, + options, + ); } - const responseBody = await response.text(); + let responseBody: string; + try { + responseBody = await response.text(); + } catch (error) { + if (abortController.signal.aborted || isAbortError(error)) { + throw timeoutError(options, error); + } + throw connectionError(options, error); + } throwHttpError( response.status, responseBody, From d80171bebb241d6e72b9c8fac7d9a72b321e15ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Tue, 21 Jul 2026 12:33:29 +0200 Subject: [PATCH 02/19] fix(config): surface configuration I/O failures Ignore only genuinely missing files and wrap other read, write, update, and clear failures as ConfigurationError. Avoid reporting a successful clear when filesystem removal fails. --- cli/src/lib/config-files.ts | 150 +++++++++++++-------- cli/src/lib/config.test.ts | 13 +- cli/src/lib/profile-configure-core.test.ts | 9 +- cli/src/lib/profile-configure-core.ts | 21 ++- 4 files changed, 127 insertions(+), 66 deletions(-) diff --git a/cli/src/lib/config-files.ts b/cli/src/lib/config-files.ts index 9285bfa..75c9c0c 100644 --- a/cli/src/lib/config-files.ts +++ b/cli/src/lib/config-files.ts @@ -2,6 +2,7 @@ import { randomBytes } from "node:crypto"; import { mkdirSync, readFileSync, renameSync, rmSync, writeFileSync } from "node:fs"; import { dirname, join } from "node:path"; import { readEnv } from "@/lib/env.ts"; +import { ConfigurationError } from "@/lib/errors.ts"; function trim(value: string): string { return value.trim(); @@ -11,6 +12,30 @@ function tempSiblingPath(filePath: string): string { return join(dirname(filePath), `.altertable-kv-${randomBytes(8).toString("hex")}`); } +function isFileNotFound(error: unknown): boolean { + return ( + typeof error === "object" && + error !== null && + "code" in error && + (error as { code?: unknown }).code === "ENOENT" + ); +} + +function configFileError(action: string, filePath: string, cause: unknown): ConfigurationError { + return new ConfigurationError(`Unable to ${action} configuration file: ${filePath}`, { cause }); +} + +function readConfigFileIfPresent(filePath: string): string | undefined { + try { + return readFileSync(filePath, "utf8"); + } catch (error) { + if (isFileNotFound(error)) { + return undefined; + } + throw configFileError("read", filePath, error); + } +} + export function configDir(): string { const override = readEnv("ALTERTABLE_CONFIG_HOME"); if (override) { @@ -29,80 +54,84 @@ export function credentialsFile(): string { } export function kvGet(filePath: string, key: string): string { - try { - const content = readFileSync(filePath, "utf8"); - for (const line of content.split("\n")) { - if (line === "" || line.startsWith("#")) { - continue; - } - const eqIndex = line.indexOf("="); - if (eqIndex === -1) { - continue; - } - const lineKey = trim(line.slice(0, eqIndex)); - if (lineKey === key) { - return trim(line.slice(eqIndex + 1)); - } - } - } catch { + const content = readConfigFileIfPresent(filePath); + if (content === undefined) { return ""; } + for (const line of content.split("\n")) { + if (line === "" || line.startsWith("#")) { + continue; + } + const eqIndex = line.indexOf("="); + if (eqIndex === -1) { + continue; + } + const lineKey = trim(line.slice(0, eqIndex)); + if (lineKey === key) { + return trim(line.slice(eqIndex + 1)); + } + } return ""; } export function kvSet(filePath: string, key: string, value: string): void { - mkdirSync(dirname(filePath), { recursive: true }); - const tmpPath = tempSiblingPath(filePath); - let found = false; - let lines: string[] = []; - try { - lines = readFileSync(filePath, "utf8").split("\n"); - } catch { - lines = []; - } + mkdirSync(dirname(filePath), { recursive: true }); + const tmpPath = tempSiblingPath(filePath); + let found = false; + const lines = readConfigFileIfPresent(filePath)?.split("\n") ?? []; - const output: string[] = []; - for (const line of lines) { - if (line === "" && output.length === 0 && lines.length === 1) { - continue; + const output: string[] = []; + for (const line of lines) { + if (line === "" && output.length === 0 && lines.length === 1) { + continue; + } + const eqIndex = line.indexOf("="); + const lineKey = eqIndex === -1 ? trim(line) : trim(line.slice(0, eqIndex)); + if (lineKey === key) { + output.push(`${key}=${value}`); + found = true; + } else { + output.push(line); + } } - const eqIndex = line.indexOf("="); - const lineKey = eqIndex === -1 ? trim(line) : trim(line.slice(0, eqIndex)); - if (lineKey === key) { + + if (!found) { output.push(`${key}=${value}`); - found = true; - } else { - output.push(line); } - } - - if (!found) { - output.push(`${key}=${value}`); - } - try { - writeFileSync( - tmpPath, - output - .filter((line, index, array) => { - if (index === array.length - 1 && line === "") { - return false; - } - return true; - }) - .join("\n") + (output.length > 0 ? "\n" : ""), - { mode: 0o600 }, - ); - renameSync(tmpPath, filePath); - } finally { - rmSync(tmpPath, { force: true }); + try { + writeFileSync( + tmpPath, + output + .filter((line, index, array) => { + if (index === array.length - 1 && line === "") { + return false; + } + return true; + }) + .join("\n") + (output.length > 0 ? "\n" : ""), + { mode: 0o600 }, + ); + renameSync(tmpPath, filePath); + } finally { + rmSync(tmpPath, { force: true }); + } + } catch (error) { + if (error instanceof ConfigurationError) { + throw error; + } + throw configFileError("write", filePath, error); } } export function kvUnset(filePath: string, key: string): void { try { - const lines = readFileSync(filePath, "utf8").split("\n"); + const content = readConfigFileIfPresent(filePath); + if (content === undefined) { + return; + } + const lines = content.split("\n"); const tmpPath = tempSiblingPath(filePath); const output: string[] = []; for (const line of lines) { @@ -119,7 +148,10 @@ export function kvUnset(filePath: string, key: string): void { } finally { rmSync(tmpPath, { force: true }); } - } catch { - // file does not exist + } catch (error) { + if (error instanceof ConfigurationError) { + throw error; + } + throw configFileError("update", filePath, error); } } diff --git a/cli/src/lib/config.test.ts b/cli/src/lib/config.test.ts index e38ede9..59e5192 100644 --- a/cli/src/lib/config.test.ts +++ b/cli/src/lib/config.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { existsSync, mkdtempSync, readFileSync, rmSync, statSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, statSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { @@ -12,9 +12,11 @@ import { getQueryDefaultPager, kvGet, kvSet, + kvUnset, resolveApiBase, resolveManagementApiBase, } from "@/lib/config.ts"; +import { ConfigurationError } from "@/lib/errors.ts"; import { profileConfigFile } from "@/lib/profile-store.ts"; let testHome = ""; @@ -45,6 +47,15 @@ describe("config", () => { expect(configGet("missing", profileName)).toBe(""); }); + test("reports configuration I/O failures instead of treating them as missing files", () => { + const directoryPath = join(testHome, "not-a-file"); + mkdirSync(directoryPath); + + expect(() => kvGet(directoryPath, "user")).toThrow(ConfigurationError); + expect(() => kvSet(directoryPath, "user", "alice")).toThrow(ConfigurationError); + expect(() => kvUnset(directoryPath, "user")).toThrow(ConfigurationError); + }); + test("resolves API bases from defaults, stored config, and environment overrides", () => { expect(resolveApiBase(profileName)).toBe("https://api.altertable.ai"); expect(resolveManagementApiBase(profileName)).toBe("https://app.altertable.ai/rest/v1"); diff --git a/cli/src/lib/profile-configure-core.test.ts b/cli/src/lib/profile-configure-core.test.ts index 14f3a93..c58a6cb 100644 --- a/cli/src/lib/profile-configure-core.test.ts +++ b/cli/src/lib/profile-configure-core.test.ts @@ -1,8 +1,9 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { existsSync, mkdtempSync, rmSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { configGet } from "@/lib/config.ts"; +import { ConfigurationError } from "@/lib/errors.ts"; import { configureRunClear, configureRunSet } from "@/lib/profile-configure-core.ts"; import { profileExists } from "@/lib/profile-store.ts"; import { createCliRuntime } from "@/lib/runtime.ts"; @@ -91,4 +92,10 @@ describe("configureRunClear", () => { expect(secretGet("api-key", "prod")).toBe(""); expect(secretGet("lakehouse/password", "staging")).toBe(""); }); + + test("reports failures while removing configuration", () => { + mkdirSync(join(testHome, "config")); + + expect(() => configureRunClear()).toThrow(ConfigurationError); + }); }); diff --git a/cli/src/lib/profile-configure-core.ts b/cli/src/lib/profile-configure-core.ts index 59eaa5e..a66c21f 100644 --- a/cli/src/lib/profile-configure-core.ts +++ b/cli/src/lib/profile-configure-core.ts @@ -1,6 +1,6 @@ import { unlinkSync, rmSync, existsSync } from "node:fs"; import { configFile, configGet, configSet, credentialsFile, kvUnset } from "@/lib/config.ts"; -import { CliError } from "@/lib/errors.ts"; +import { CliError, ConfigurationError } from "@/lib/errors.ts"; import { deriveProfileName, listProfiles } from "@/lib/profile/model.ts"; import { ensureProfileExists, @@ -228,14 +228,25 @@ export function configureRunClear(sink: OutputSink = getOutputSink()): void { for (const filePath of [configFile(), credentialsFile()]) { try { unlinkSync(filePath); - } catch { - // already removed + } catch (error) { + if ( + typeof error !== "object" || + error === null || + !("code" in error) || + (error as { code?: unknown }).code !== "ENOENT" + ) { + throw new ConfigurationError(`Unable to remove configuration file: ${filePath}`, { + cause: error, + }); + } } } try { rmSync(profilesDir(), { recursive: true, force: true }); - } catch { - // already removed + } catch (error) { + throw new ConfigurationError(`Unable to remove profiles directory: ${profilesDir()}`, { + cause: error, + }); } getCliRuntime().session = undefined; sink.writeMetadata([ From 7f4269bc24965a2bf522456a8d75172a9d14893d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Tue, 21 Jul 2026 12:33:44 +0200 Subject: [PATCH 03/19] fix(api): preserve exact JSON integers Keep append payload text unchanged after validation and serialize typed API integers beyond JavaScript's safe range as exact JSON numeric tokens. Preserve the same digits in query parameters. --- cli/src/commands/api/lib/body.ts | 35 +++++++++++++++++++----- cli/src/commands/api/lib/http.test.ts | 17 ++++++++++++ cli/src/commands/api/lib/http.ts | 8 ++++-- cli/src/commands/append/lib/data.test.ts | 5 ++-- cli/src/commands/append/lib/data.ts | 3 +- 5 files changed, 56 insertions(+), 12 deletions(-) diff --git a/cli/src/commands/api/lib/body.ts b/cli/src/commands/api/lib/body.ts index a88d7fa..32502a7 100644 --- a/cli/src/commands/api/lib/body.ts +++ b/cli/src/commands/api/lib/body.ts @@ -62,7 +62,12 @@ function parseFieldEntry(entry: string): [string, string] { return [key, value]; } -export type ApiFieldValue = string | number | boolean | null; +type ExactJsonInteger = { + kind: "exact_json_integer"; + source: string; +}; + +export type ApiFieldValue = string | number | boolean | null | ExactJsonInteger; export type ParsedApiField = { key: string; @@ -80,7 +85,8 @@ function parseTypedFieldValue(value: string): ApiFieldValue { return null; } if (/^-?\d+$/.test(value)) { - return Number(value); + const parsed = Number(value); + return Number.isSafeInteger(parsed) ? parsed : { kind: "exact_json_integer", source: value }; } if (value === "@-") { return readFileSync(0, "utf8"); @@ -114,12 +120,27 @@ function parseTypedFields( }); } -function parsedFieldsToRecord(fields: ParsedApiField[]): Record { - const body: Record = {}; +function isExactJsonInteger(value: ApiFieldValue): value is ExactJsonInteger { + return typeof value === "object" && value !== null && value.kind === "exact_json_integer"; +} + +export function apiFieldValueText(value: ApiFieldValue): string { + if (isExactJsonInteger(value)) { + return value.source; + } + return value === null ? "null" : String(value); +} + +function serializeFieldValue(value: ApiFieldValue): string { + return isExactJsonInteger(value) ? value.source : JSON.stringify(value); +} + +function serializeParsedFields(fields: ParsedApiField[]): string { + const body = new Map(); for (const field of fields) { - body[field.key] = field.value; + body.set(field.key, field.value); } - return body; + return `{${[...body].map(([key, value]) => `${JSON.stringify(key)}:${serializeFieldValue(value)}`).join(",")}}`; } export type ResolveApiBodyOptions = { @@ -189,7 +210,7 @@ export function resolveApiRequestPayload( if (hasFields) { return { - body: JSON.stringify(parsedFieldsToRecord(parsedFields)), + body: serializeParsedFields(parsedFields), queryFields: [], }; } diff --git a/cli/src/commands/api/lib/http.test.ts b/cli/src/commands/api/lib/http.test.ts index c1909cc..62b302c 100644 --- a/cli/src/commands/api/lib/http.test.ts +++ b/cli/src/commands/api/lib/http.test.ts @@ -71,6 +71,23 @@ describe("api-body", () => { expect(JSON.parse(payload.body ?? "")).toEqual({ label: "CI Bot", name: "Analytics" }); }); + test("preserves typed integers outside JavaScript's safe range", () => { + const payload = resolveApiRequestPayload({ + method: "POST", + typedFields: ["id=9007199254740993", "negative=-9007199254740993"], + }); + + expect(payload.body).toBe('{"id":9007199254740993,"negative":-9007199254740993}'); + + expect( + resolveApiHttp({ + endpoint: "/records", + method: "GET", + typedFields: ["id=9007199254740993"], + }).endpoint, + ).toBe("/records?id=9007199254740993"); + }); + test("keeps an explicit JSON request body from --input", () => { const filePath = join(testHome, "payload.json"); writeFileSync(filePath, '{"label":"raw"}', "utf8"); diff --git a/cli/src/commands/api/lib/http.ts b/cli/src/commands/api/lib/http.ts index fc3a68f..563fdd2 100644 --- a/cli/src/commands/api/lib/http.ts +++ b/cli/src/commands/api/lib/http.ts @@ -1,4 +1,8 @@ -import { resolveApiRequestPayload, type ParsedApiField } from "@/commands/api/lib/body.ts"; +import { + apiFieldValueText, + resolveApiRequestPayload, + type ParsedApiField, +} from "@/commands/api/lib/body.ts"; import type { CommandOutputMode } from "@/lib/command-output.ts"; import { CliError } from "@/lib/errors.ts"; import { encodeManagementEndpoint } from "@/lib/management-endpoint.ts"; @@ -106,7 +110,7 @@ function appendQueryFields(endpoint: string, fields: ParsedApiField[]): string { const separator = endpoint.includes("?") ? "&" : "?"; const searchParams = new URLSearchParams(); for (const field of fields) { - searchParams.append(field.key, field.value === null ? "null" : String(field.value)); + searchParams.append(field.key, apiFieldValueText(field.value)); } return `${endpoint}${separator}${searchParams.toString()}`; } diff --git a/cli/src/commands/append/lib/data.test.ts b/cli/src/commands/append/lib/data.test.ts index eba5dc6..af272da 100644 --- a/cli/src/commands/append/lib/data.test.ts +++ b/cli/src/commands/append/lib/data.test.ts @@ -3,8 +3,9 @@ import { parseAppendJsonContent } from "@/commands/append/lib/data.ts"; import { CliError } from "@/lib/errors.ts"; describe("parseAppendJsonContent", () => { - test("normalizes inline JSON objects", () => { - expect(parseAppendJsonContent('{ "id": 1 }')).toBe('{"id":1}'); + test("preserves inline JSON without rounding large integers", () => { + const input = '{ "id": 9007199254740993 }'; + expect(parseAppendJsonContent(input)).toBe(input); }); test("rejects non-JSON data", () => { diff --git a/cli/src/commands/append/lib/data.ts b/cli/src/commands/append/lib/data.ts index 87ebfa6..c281e23 100644 --- a/cli/src/commands/append/lib/data.ts +++ b/cli/src/commands/append/lib/data.ts @@ -18,7 +18,8 @@ export function parseAppendJsonContent(dataArg: string): string { } try { - return JSON.stringify(JSON.parse(jsonContent)); + JSON.parse(jsonContent); + return jsonContent; } catch { throw new CliError("Data must be valid JSON."); } From dfd20648c5ed96e66dcf93301548c5c0ccd50cb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Tue, 21 Jul 2026 12:33:58 +0200 Subject: [PATCH 04/19] fix(query): redact secrets in JSON string cells Parse JSON-shaped string cells before human rendering and recursively mask password, token, secret, and API key fields while preserving unchanged JSON text. --- cli/src/lib/query-format.test.ts | 13 +++++++++++++ cli/src/lib/query-format.ts | 13 ++++++++----- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/cli/src/lib/query-format.test.ts b/cli/src/lib/query-format.test.ts index cef6d3c..d97d387 100644 --- a/cli/src/lib/query-format.test.ts +++ b/cli/src/lib/query-format.test.ts @@ -130,6 +130,19 @@ describe("formatQueryCell", () => { expect(formatQueryCell("{}", { maxWidth: 10 })).toBe("{}"); }); + test("redacts sensitive keys inside JSON string cells", () => { + const output = formatQueryCell( + '{"password":"secret","nested":{"access_token":"token-value"},"name":"safe"}', + {}, + ); + + expect(output).toBe( + '{"password":"[REDACTED]","nested":{"access_token":"[REDACTED]"},"name":"safe"}', + ); + expect(output).not.toContain("secret"); + expect(output).not.toContain("token-value"); + }); + test("appends relative time to timestamps while keeping absolute form", () => { const timestamp = "2026-06-21T06:35:24.409Z"; const nowMs = Date.parse("2026-06-27T12:00:00.000Z"); diff --git a/cli/src/lib/query-format.ts b/cli/src/lib/query-format.ts index 1936fab..e93545e 100644 --- a/cli/src/lib/query-format.ts +++ b/cli/src/lib/query-format.ts @@ -13,7 +13,7 @@ import { type QueryDataType, } from "@/lib/query-column-types.ts"; import { pluralizeLabel } from "@/lib/pluralize.ts"; -import { redactPasswordFieldInText } from "@/lib/redact.ts"; +import { redactPasswordFieldInText, redactSensitiveJsonValue } from "@/lib/redact.ts"; import { formatRelativeTimestamp, formatTimestampWithRelative } from "@/lib/relative-time.ts"; import { getTerminalWidth, @@ -98,8 +98,11 @@ function parseJsonStringValue(value: string): string | null { return null; } try { - JSON.parse(trimmed); - return trimmed; + const parsed = JSON.parse(trimmed) as unknown; + const redacted = redactSensitiveJsonValue(parsed); + const normalized = JSON.stringify(parsed); + const normalizedRedacted = JSON.stringify(redacted); + return normalized === normalizedRedacted ? trimmed : normalizedRedacted; } catch { return null; } @@ -192,14 +195,14 @@ function formatStringQueryCell( options: QueryCellOptions, colorize: boolean, ): string { - const sanitized = redactPasswordFieldInText(value); + const jsonText = parseJsonStringValue(value); + const sanitized = jsonText ?? redactPasswordFieldInText(value); if (sanitized.length === 0) { return colorize ? renderDisplayText([span('""', "subtle")]) : '""'; } const columnTypeMap = options.columnTypeMap ?? new Map(); const dataType = resolveCellDataType(sanitized, options.columnName, columnTypeMap); - const jsonText = parseJsonStringValue(sanitized); if (dataType === "timestamp" && isTimestampValue(sanitized)) { return formatTimestampQueryCell(sanitized, options, colorize); From bb6ce2493876e7a00bfa36b9e05a7c716adacf9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Tue, 21 Jul 2026 12:34:11 +0200 Subject: [PATCH 05/19] fix(profile): roll back failed profile mutations Restore profile directories, credentials, and active selection when rename or delete steps fail. Make secret moves restore both source and target state after partial deletion failures. --- cli/src/lib/profile/model.test.ts | 64 +++++++++++++++++- cli/src/lib/profile/model.ts | 104 +++++++++++++++++++++++++----- cli/src/lib/secrets.ts | 36 ++++++++--- 3 files changed, 177 insertions(+), 27 deletions(-) diff --git a/cli/src/lib/profile/model.test.ts b/cli/src/lib/profile/model.test.ts index 91a15fa..7e8eacd 100644 --- a/cli/src/lib/profile/model.test.ts +++ b/cli/src/lib/profile/model.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { mkdtempSync, rmSync } from "node:fs"; +import { mkdtempSync, renameSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { setCliContext } from "@/context.ts"; @@ -13,7 +13,12 @@ import { renameProfile, updateProfile, } from "@/lib/profile/model.ts"; -import { getActiveProfileName, profileExists, setActiveProfile } from "@/lib/profile-store.ts"; +import { + getActiveProfileName, + profileDir, + profileExists, + setActiveProfile, +} from "@/lib/profile-store.ts"; import { secretGet } from "@/lib/secrets.ts"; import { createFakeKeychain } from "@/test-utils/keychain.ts"; @@ -127,6 +132,27 @@ describe("profile model", () => { expect(keychain.store.secretGet("api-key", "staging")).toBe("atm_staging"); }); + test("restores profile config and secrets when directory deletion fails", () => { + createEmptyProfile("staging"); + const keychain = createFakeKeychain(); + keychain.store.secretSet("api-key", "atm_staging", "staging"); + + expect(() => + deleteProfile("staging", keychain.store, { + renameDirectory(source, target) { + renameSync(profileDir(source), profileDir(target)); + }, + removeDirectory() { + throw new Error("simulated remove failure"); + }, + setActive: setActiveProfile, + }), + ).toThrow("simulated remove failure"); + + expect(profileExists("staging")).toBe(true); + expect(keychain.store.secretGet("api-key", "staging")).toBe("atm_staging"); + }); + test("renames profile config, active selection, and secrets", async () => { await configureRunSet({ profile: "staging", apiKey: "atm_staging", env: "staging" }); setActiveProfile("staging"); @@ -158,4 +184,38 @@ describe("profile model", () => { expect(keychain.store.secretGet("lakehouse/password", "staging")).toBe("lakehouse-secret"); expect(keychain.store.secretGet("api-key", "acme_staging")).toBe(""); }); + + test("rolls back config and secrets when switching the renamed active profile fails", () => { + createEmptyProfile("staging"); + setActiveProfile("staging"); + const keychain = createFakeKeychain(); + keychain.store.secretSet("api-key", "atm_staging", "staging"); + + expect(() => + renameProfile("staging", "acme_staging", keychain.store, { + renameDirectory(source, target) { + renameSync(profileDir(source), profileDir(target)); + }, + removeDirectory(name) { + rmSync(profileDir(name), { recursive: true, force: true }); + }, + setActive(name) { + if (name === "acme_staging") { + throw new Error("simulated active-profile failure"); + } + setActiveProfile(name); + }, + }), + ).toThrow("simulated active-profile failure"); + + expect(profileExists("staging")).toBe(true); + expect(profileExists("acme_staging")).toBe(false); + expect(getActiveProfileName()).toBe("staging"); + expect(keychain.store.secretGet("api-key", "staging")).toBe("atm_staging"); + expect(keychain.store.secretGet("api-key", "acme_staging")).toBe(""); + }); + + test("rejects a no-op rename when the source profile does not exist", () => { + expect(() => renameProfile("missing", "missing")).toThrow("Profile not found: missing"); + }); }); diff --git a/cli/src/lib/profile/model.ts b/cli/src/lib/profile/model.ts index b1c109b..489c506 100644 --- a/cli/src/lib/profile/model.ts +++ b/cli/src/lib/profile/model.ts @@ -1,3 +1,4 @@ +import { randomBytes } from "node:crypto"; import { renameSync, rmSync } from "node:fs"; import { configDir, @@ -14,6 +15,8 @@ import { moveProfileSecrets, secretDelete, secretExists, + secretGet, + secretSet, secretStoreDisplay, type SecretStore, } from "@/lib/secrets.ts"; @@ -43,9 +46,33 @@ const PROFILE_SECRET_ACCOUNTS = [ "oauth/refresh-token", ] as const; -type ProfileSecretStore = Pick; +type ProfileSecretStore = Pick< + SecretStore, + "moveProfileSecrets" | "secretDelete" | "secretGet" | "secretSet" +>; -const PROFILE_SECRET_STORE: ProfileSecretStore = { moveProfileSecrets, secretDelete }; +const PROFILE_SECRET_STORE: ProfileSecretStore = { + moveProfileSecrets, + secretDelete, + secretGet, + secretSet, +}; + +type ProfileMutationOperations = { + renameDirectory(source: string, target: string): void; + removeDirectory(name: string): void; + setActive(name: string): void; +}; + +const PROFILE_MUTATION_OPERATIONS: ProfileMutationOperations = { + renameDirectory(source, target) { + renameSync(profileDir(source), profileDir(target)); + }, + removeDirectory(name) { + rmSync(profileDir(name), { recursive: true, force: true }); + }, + setActive: setActiveProfile, +}; type ProfileManagementAuth = "oauth" | "api_key" | "none"; type ProfileLakehouseAuth = "basic_token" | "username_password" | "none"; @@ -412,40 +439,61 @@ export function renameProfile( source: string, target: string, secrets: ProfileSecretStore = PROFILE_SECRET_STORE, + operations: ProfileMutationOperations = PROFILE_MUTATION_OPERATIONS, ): void { assertSafeProfileName(source); assertSafeProfileName(target); ensureProfilesLayout(); - if (source === target) { - return; - } if (!profileExists(source)) { throw new ConfigurationError(`Profile not found: ${source}`); } + if (source === target) { + return; + } if (profileExists(target)) { throw new ConfigurationError(`Profile already exists: ${target}`); } const active = getActiveProfileName(); - renameSync(profileDir(source), profileDir(target)); + operations.renameDirectory(source, target); + let secretsMoved = false; try { secrets.moveProfileSecrets(source, target, [...PROFILE_SECRET_ACCOUNTS]); + secretsMoved = true; + if (active === source) { + operations.setActive(target); + } } catch (error) { + if (secretsMoved) { + try { + secrets.moveProfileSecrets(target, source, [...PROFILE_SECRET_ACCOUNTS]); + } catch { + // Best-effort rollback; preserve the original failure for the caller. + } + } if (profileExists(target) && !profileExists(source)) { - renameSync(profileDir(target), profileDir(source)); + try { + operations.renameDirectory(target, source); + } catch { + // Best-effort rollback; preserve the original failure for the caller. + } + } + if (active === source) { + try { + operations.setActive(source); + } catch { + // Best-effort rollback; preserve the original failure for the caller. + } } throw error; } - - if (active === source) { - setActiveProfile(target); - } } export function deleteProfile( name: string, secrets: ProfileSecretStore = PROFILE_SECRET_STORE, + operations: ProfileMutationOperations = PROFILE_MUTATION_OPERATIONS, ): void { assertSafeProfileName(name); ensureProfilesLayout(); @@ -461,11 +509,37 @@ export function deleteProfile( ); } - for (const account of PROFILE_SECRET_ACCOUNTS) { - secrets.secretDelete(account, name); + const savedSecrets = PROFILE_SECRET_ACCOUNTS.map((account) => ({ + account, + value: secrets.secretGet(account, name), + })); + const quarantinedName = `.deleting-${randomBytes(8).toString("hex")}`; + operations.renameDirectory(name, quarantinedName); + try { + for (const account of PROFILE_SECRET_ACCOUNTS) { + secrets.secretDelete(account, name); + } + operations.removeDirectory(quarantinedName); + } catch (error) { + for (const savedSecret of savedSecrets) { + if (savedSecret.value.length === 0) { + continue; + } + try { + secrets.secretSet(savedSecret.account, savedSecret.value, name); + } catch { + // Best-effort rollback; preserve the original failure for the caller. + } + } + if (profileExists(quarantinedName) && !profileExists(name)) { + try { + operations.renameDirectory(quarantinedName, name); + } catch { + // Best-effort rollback; preserve the original failure for the caller. + } + } + throw error; } - - rmSync(profileDir(name), { recursive: true, force: true }); } export type ConfigureCredentialStatus = { diff --git a/cli/src/lib/secrets.ts b/cli/src/lib/secrets.ts index 1f1094a..4d5aca7 100644 --- a/cli/src/lib/secrets.ts +++ b/cli/src/lib/secrets.ts @@ -217,16 +217,9 @@ export function createSecretStore( targetProfile: string, keys: readonly string[], ): void { - const prepared: Array<{ key: string; targetValue: string }> = []; + const prepared: Array<{ key: string; sourceValue: string; targetValue: string }> = []; - try { - for (const key of keys) { - const sourceValue = secretGet(key, sourceProfile); - if (sourceValue.length === 0) continue; - prepared.push({ key, targetValue: secretGet(key, targetProfile) }); - secretSet(key, sourceValue, targetProfile); - } - } catch (error) { + function restoreTargetSecrets(): void { for (const entry of prepared.toReversed()) { try { if (entry.targetValue.length > 0) { @@ -238,10 +231,33 @@ export function createSecretStore( // Best-effort rollback; preserve the original failure for the caller. } } + } + + try { + for (const key of keys) { + const sourceValue = secretGet(key, sourceProfile); + if (sourceValue.length === 0) continue; + prepared.push({ key, sourceValue, targetValue: secretGet(key, targetProfile) }); + secretSet(key, sourceValue, targetProfile); + } + } catch (error) { + restoreTargetSecrets(); throw error; } - for (const { key } of prepared) secretDelete(key, sourceProfile); + try { + for (const { key } of prepared) secretDelete(key, sourceProfile); + } catch (error) { + for (const entry of prepared) { + try { + secretSet(entry.key, entry.sourceValue, sourceProfile); + } catch { + // Best-effort rollback; preserve the original failure for the caller. + } + } + restoreTargetSecrets(); + throw error; + } } function secretStoreDisplay(): string { From 388bd30a42695a4b3f79465727a5db3daeb297aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Tue, 21 Jul 2026 14:01:47 +0200 Subject: [PATCH 06/19] fix(api): canonicalize exact integer tokens Normalize signs and leading zeroes before raw numeric serialization so every typed integer produces valid JSON while preserving exact magnitude in request bodies and query parameters. --- cli/src/commands/api/lib/body.ts | 12 ++++++++++-- cli/src/commands/api/lib/http.test.ts | 7 ++++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/cli/src/commands/api/lib/body.ts b/cli/src/commands/api/lib/body.ts index 32502a7..3fc3e75 100644 --- a/cli/src/commands/api/lib/body.ts +++ b/cli/src/commands/api/lib/body.ts @@ -67,6 +67,13 @@ type ExactJsonInteger = { source: string; }; +function canonicalIntegerSource(value: string): string { + const negative = value.startsWith("-"); + const digits = negative ? value.slice(1) : value; + const canonicalDigits = digits.replace(/^0+/, "") || "0"; + return negative && canonicalDigits !== "0" ? `-${canonicalDigits}` : canonicalDigits; +} + export type ApiFieldValue = string | number | boolean | null | ExactJsonInteger; export type ParsedApiField = { @@ -85,8 +92,9 @@ function parseTypedFieldValue(value: string): ApiFieldValue { return null; } if (/^-?\d+$/.test(value)) { - const parsed = Number(value); - return Number.isSafeInteger(parsed) ? parsed : { kind: "exact_json_integer", source: value }; + const source = canonicalIntegerSource(value); + const parsed = Number(source); + return Number.isSafeInteger(parsed) ? parsed : { kind: "exact_json_integer", source }; } if (value === "@-") { return readFileSync(0, "utf8"); diff --git a/cli/src/commands/api/lib/http.test.ts b/cli/src/commands/api/lib/http.test.ts index 62b302c..fcc94ec 100644 --- a/cli/src/commands/api/lib/http.test.ts +++ b/cli/src/commands/api/lib/http.test.ts @@ -74,16 +74,17 @@ describe("api-body", () => { test("preserves typed integers outside JavaScript's safe range", () => { const payload = resolveApiRequestPayload({ method: "POST", - typedFields: ["id=9007199254740993", "negative=-9007199254740993"], + typedFields: ["id=0009007199254740993", "negative=-0009007199254740993", "zero=-000"], }); - expect(payload.body).toBe('{"id":9007199254740993,"negative":-9007199254740993}'); + expect(payload.body).toBe('{"id":9007199254740993,"negative":-9007199254740993,"zero":0}'); + expect(() => JSON.parse(payload.body ?? "")).not.toThrow(); expect( resolveApiHttp({ endpoint: "/records", method: "GET", - typedFields: ["id=9007199254740993"], + typedFields: ["id=0009007199254740993"], }).endpoint, ).toBe("/records?id=9007199254740993"); }); From f07f5b05f96b34ad5aced895250b1dfd761d75b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Tue, 21 Jul 2026 14:02:32 +0200 Subject: [PATCH 07/19] fix(http): time out streamed error bodies Apply the configured stream read deadline while consuming non-2xx response bodies and always clear the active timer after body completion or failure. --- cli/src/lib/http.test.ts | 37 +++++++++++++++++++++++++++++++++++++ cli/src/lib/http.ts | 8 ++++++++ 2 files changed, 45 insertions(+) diff --git a/cli/src/lib/http.test.ts b/cli/src/lib/http.test.ts index 66e2df5..40432be 100644 --- a/cli/src/lib/http.test.ts +++ b/cli/src/lib/http.test.ts @@ -469,6 +469,43 @@ describe("httpSendStream live timeouts", () => { globalThis.fetch = originalFetch; } }); + + test("positive readTimeoutMs also aborts stalled error response bodies", async () => { + delete process.env.ALTERTABLE_MOCK_HTTP_FILE; + const originalFetch = globalThis.fetch; + + globalThis.fetch = Object.assign( + async (_url: Parameters[0], init?: Parameters[1]) => { + const signal = init?.signal; + return new Response( + new ReadableStream({ + start(controller) { + signal?.addEventListener("abort", () => { + controller.error(new DOMException("error body timed out", "AbortError")); + }); + }, + }), + { status: 500 }, + ); + }, + { preconnect: originalFetch.preconnect }, + ); + + try { + expect( + httpSendStream({ + method: "POST", + url: "https://example.com/query", + authHeader: "Authorization: Bearer test", + body: "{}", + connectTimeoutMs: 50, + readTimeoutMs: 5, + }), + ).rejects.toBeInstanceOf(TimeoutError); + } finally { + globalThis.fetch = originalFetch; + } + }); }); describe("debug response redaction", () => { diff --git a/cli/src/lib/http.ts b/cli/src/lib/http.ts index b8a83e0..1861623 100644 --- a/cli/src/lib/http.ts +++ b/cli/src/lib/http.ts @@ -504,6 +504,12 @@ async function executeLiveStream(options: HttpStreamOptions): Promise 0) { + timeout = setTimeout(() => { + abortController.abort(); + }, readTimeoutMs); + } + let responseBody: string; try { responseBody = await response.text(); @@ -512,6 +518,8 @@ async function executeLiveStream(options: HttpStreamOptions): Promise Date: Tue, 21 Jul 2026 14:03:13 +0200 Subject: [PATCH 08/19] fix(http): distinguish peer aborts from timeouts Classify failures as timeouts only when the CLI's internal deadline signal fired. Report peer-originated AbortError failures through the network error path. --- cli/src/lib/http.test.ts | 39 +++++++++++++++++++++++++++++++++++++++ cli/src/lib/http.ts | 16 +++++----------- 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/cli/src/lib/http.test.ts b/cli/src/lib/http.test.ts index 40432be..b7ddb07 100644 --- a/cli/src/lib/http.test.ts +++ b/cli/src/lib/http.test.ts @@ -506,6 +506,45 @@ describe("httpSendStream live timeouts", () => { globalThis.fetch = originalFetch; } }); + + test("reports peer-aborted streams as network errors instead of timeouts", async () => { + delete process.env.ALTERTABLE_MOCK_HTTP_FILE; + const originalFetch = globalThis.fetch; + + globalThis.fetch = Object.assign( + async () => + new Response( + new ReadableStream({ + start(controller) { + controller.error(new DOMException("peer aborted", "AbortError")); + }, + }), + { status: 200 }, + ), + { preconnect: originalFetch.preconnect }, + ); + + try { + const stream = await httpSendStream({ + method: "POST", + url: "https://example.com/query", + authHeader: "Authorization: Bearer test", + body: "{}", + connectTimeoutMs: 50, + readTimeoutMs: 50, + }); + + try { + await stream.getReader().read(); + expect.unreachable("stream read should have failed"); + } catch (error) { + expect(error).toBeInstanceOf(NetworkError); + expect(error).not.toBeInstanceOf(TimeoutError); + } + } finally { + globalThis.fetch = originalFetch; + } + }); }); describe("debug response redaction", () => { diff --git a/cli/src/lib/http.ts b/cli/src/lib/http.ts index 1861623..5baf848 100644 --- a/cli/src/lib/http.ts +++ b/cli/src/lib/http.ts @@ -98,10 +98,6 @@ export function resolveFetchTimeoutMs(options: HttpSendOptions): number { return connectTimeoutMs; } -function isAbortError(error: unknown): boolean { - return error instanceof Error && error.name === "AbortError"; -} - /** Flatten an Error's `cause` chain into a readable detail (e.g. fetch failed -> TLS reason). */ function unwrapErrorDetail(error: unknown): string { const messages: string[] = []; @@ -152,9 +148,7 @@ function streamWithTimeoutCleanup( } catch (error) { clearTimeoutAfterRead(); controller.error( - signal.aborted || isAbortError(error) - ? timeoutError(options, error) - : connectionError(options, error), + signal.aborted ? timeoutError(options, error) : connectionError(options, error), ); } }, @@ -414,7 +408,7 @@ async function executeLiveRequest(options: HttpSendOptions): Promise { dispatcher: getSharedDispatcher(), } as RequestInit); } catch (error) { - if (signal.aborted || isAbortError(error)) { + if (signal.aborted) { throw timeoutError(options, error); } throw connectionError(options, error); @@ -424,7 +418,7 @@ async function executeLiveRequest(options: HttpSendOptions): Promise { try { responseBody = await response.text(); } catch (error) { - if (signal.aborted || isAbortError(error)) { + if (signal.aborted) { throw timeoutError(options, error); } throw connectionError(options, error); @@ -480,7 +474,7 @@ async function executeLiveStream(options: HttpStreamOptions): Promise Date: Tue, 21 Jul 2026 14:03:38 +0200 Subject: [PATCH 09/19] fix(query): preserve JSON lexemes during redaction Redact sensitive value spans with a source-aware JSON walker instead of parse-and-reserialize so unrelated large numeric identifiers and formatting remain exact. --- cli/src/lib/query-format.test.ts | 4 +- cli/src/lib/query-format.ts | 8 +-- cli/src/lib/redact.ts | 117 +++++++++++++++++++++++++++++++ 3 files changed, 121 insertions(+), 8 deletions(-) diff --git a/cli/src/lib/query-format.test.ts b/cli/src/lib/query-format.test.ts index d97d387..50c5980 100644 --- a/cli/src/lib/query-format.test.ts +++ b/cli/src/lib/query-format.test.ts @@ -132,12 +132,12 @@ describe("formatQueryCell", () => { test("redacts sensitive keys inside JSON string cells", () => { const output = formatQueryCell( - '{"password":"secret","nested":{"access_token":"token-value"},"name":"safe"}', + '{"password":"secret","nested":{"access_token":"token-value"},"id":9007199254740993,"name":"safe"}', {}, ); expect(output).toBe( - '{"password":"[REDACTED]","nested":{"access_token":"[REDACTED]"},"name":"safe"}', + '{"password":"[REDACTED]","nested":{"access_token":"[REDACTED]"},"id":9007199254740993,"name":"safe"}', ); expect(output).not.toContain("secret"); expect(output).not.toContain("token-value"); diff --git a/cli/src/lib/query-format.ts b/cli/src/lib/query-format.ts index e93545e..cd42a9f 100644 --- a/cli/src/lib/query-format.ts +++ b/cli/src/lib/query-format.ts @@ -13,7 +13,7 @@ import { type QueryDataType, } from "@/lib/query-column-types.ts"; import { pluralizeLabel } from "@/lib/pluralize.ts"; -import { redactPasswordFieldInText, redactSensitiveJsonValue } from "@/lib/redact.ts"; +import { redactPasswordFieldInText, redactSensitiveJsonText } from "@/lib/redact.ts"; import { formatRelativeTimestamp, formatTimestampWithRelative } from "@/lib/relative-time.ts"; import { getTerminalWidth, @@ -98,11 +98,7 @@ function parseJsonStringValue(value: string): string | null { return null; } try { - const parsed = JSON.parse(trimmed) as unknown; - const redacted = redactSensitiveJsonValue(parsed); - const normalized = JSON.stringify(parsed); - const normalizedRedacted = JSON.stringify(redacted); - return normalized === normalizedRedacted ? trimmed : normalizedRedacted; + return redactSensitiveJsonText(trimmed); } catch { return null; } diff --git a/cli/src/lib/redact.ts b/cli/src/lib/redact.ts index 03f4a63..4bb76c8 100644 --- a/cli/src/lib/redact.ts +++ b/cli/src/lib/redact.ts @@ -41,6 +41,123 @@ export function redactSensitiveJsonValue(value: unknown): unknown { return value; } +function jsonStringEnd(source: string, start: number): number { + let index = start + 1; + while (index < source.length) { + if (source[index] === "\\") { + index += 2; + continue; + } + if (source[index] === '"') { + return index + 1; + } + index += 1; + } + return source.length; +} + +function jsonValueEnd(source: string, start: number): number { + const first = source[start]; + if (first === '"') { + return jsonStringEnd(source, start); + } + if (first !== "{" && first !== "[") { + let index = start; + while (index < source.length && !/[\s,}\]]/.test(source[index] ?? "")) { + index += 1; + } + return index; + } + + const closingCharacters = [first === "{" ? "}" : "]"]; + let index = start + 1; + while (index < source.length && closingCharacters.length > 0) { + const character = source[index]; + if (character === '"') { + index = jsonStringEnd(source, index); + continue; + } + if (character === "{") closingCharacters.push("}"); + if (character === "[") closingCharacters.push("]"); + if (character === closingCharacters.at(-1)) closingCharacters.pop(); + index += 1; + } + return index; +} + +/** Redact sensitive object fields while preserving all non-sensitive JSON source lexemes. */ +export function redactSensitiveJsonText(source: string): string { + JSON.parse(source); + let index = 0; + + function renderWhitespace(): string { + const start = index; + while (/\s/.test(source[index] ?? "")) index += 1; + return source.slice(start, index); + } + + function renderValue(redact: boolean): string { + if (redact) { + index = jsonValueEnd(source, index); + return '"[REDACTED]"'; + } + if (source[index] === "{") return renderObject(); + if (source[index] === "[") return renderArray(); + const end = jsonValueEnd(source, index); + const rendered = source.slice(index, end); + index = end; + return rendered; + } + + function renderObject(): string { + let rendered = "{"; + index += 1; + rendered += renderWhitespace(); + while (source[index] !== "}") { + const keyStart = index; + const keyEnd = jsonStringEnd(source, keyStart); + const rawKey = source.slice(keyStart, keyEnd); + const key = JSON.parse(rawKey) as string; + rendered += rawKey; + index = keyEnd; + rendered += renderWhitespace(); + rendered += source[index] ?? ""; + index += 1; + rendered += renderWhitespace(); + rendered += renderValue(isSensitiveJsonKey(key)); + rendered += renderWhitespace(); + if (source[index] === ",") { + rendered += ","; + index += 1; + rendered += renderWhitespace(); + } + } + index += 1; + return `${rendered}}`; + } + + function renderArray(): string { + let rendered = "["; + index += 1; + rendered += renderWhitespace(); + while (source[index] !== "]") { + rendered += renderValue(false); + rendered += renderWhitespace(); + if (source[index] === ",") { + rendered += ","; + index += 1; + rendered += renderWhitespace(); + } + } + index += 1; + return `${rendered}]`; + } + + const leadingWhitespace = renderWhitespace(); + const rendered = renderValue(false); + return leadingWhitespace + rendered + renderWhitespace(); +} + export function truncateBodySnippet(body: string, maxLength = BODY_MAX_LENGTH): string { if (body.length <= maxLength) { return body; From c4835e9e058459cd21e42911c50e15511e26bfed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Tue, 21 Jul 2026 14:03:52 +0200 Subject: [PATCH 10/19] fix(profile): restore config after partial deletion Snapshot profile config bytes and permissions before quarantine removal, then recreate them during rollback even when recursive deletion removed part or all of the quarantined directory. --- cli/src/lib/profile/model.test.ts | 13 +++++++++---- cli/src/lib/profile/model.ts | 22 +++++++++++++++++++++- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/cli/src/lib/profile/model.test.ts b/cli/src/lib/profile/model.test.ts index 7e8eacd..7edad85 100644 --- a/cli/src/lib/profile/model.test.ts +++ b/cli/src/lib/profile/model.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { mkdtempSync, renameSync, rmSync } from "node:fs"; +import { mkdtempSync, readFileSync, renameSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { setCliContext } from "@/context.ts"; @@ -15,6 +15,7 @@ import { } from "@/lib/profile/model.ts"; import { getActiveProfileName, + profileConfigFile, profileDir, profileExists, setActiveProfile, @@ -134,6 +135,8 @@ describe("profile model", () => { test("restores profile config and secrets when directory deletion fails", () => { createEmptyProfile("staging"); + updateProfile("staging", { organizationSlug: "acme", environment: "staging" }); + const originalConfig = readFileSync(profileConfigFile("staging")); const keychain = createFakeKeychain(); keychain.store.secretSet("api-key", "atm_staging", "staging"); @@ -142,14 +145,16 @@ describe("profile model", () => { renameDirectory(source, target) { renameSync(profileDir(source), profileDir(target)); }, - removeDirectory() { - throw new Error("simulated remove failure"); + removeDirectory(name) { + rmSync(profileConfigFile(name)); + throw new Error("simulated partial remove failure"); }, setActive: setActiveProfile, }), - ).toThrow("simulated remove failure"); + ).toThrow("simulated partial remove failure"); expect(profileExists("staging")).toBe(true); + expect(readFileSync(profileConfigFile("staging"))).toEqual(originalConfig); expect(keychain.store.secretGet("api-key", "staging")).toBe("atm_staging"); }); diff --git a/cli/src/lib/profile/model.ts b/cli/src/lib/profile/model.ts index 489c506..d2b35b4 100644 --- a/cli/src/lib/profile/model.ts +++ b/cli/src/lib/profile/model.ts @@ -1,5 +1,13 @@ import { randomBytes } from "node:crypto"; -import { renameSync, rmSync } from "node:fs"; +import { + existsSync, + mkdirSync, + readFileSync, + renameSync, + rmSync, + statSync, + writeFileSync, +} from "node:fs"; import { configDir, configFile, @@ -513,6 +521,10 @@ export function deleteProfile( account, value: secrets.secretGet(account, name), })); + const configPath = profileConfigFile(name); + const savedConfig = existsSync(configPath) + ? { contents: readFileSync(configPath), mode: statSync(configPath).mode & 0o777 } + : undefined; const quarantinedName = `.deleting-${randomBytes(8).toString("hex")}`; operations.renameDirectory(name, quarantinedName); try { @@ -538,6 +550,14 @@ export function deleteProfile( // Best-effort rollback; preserve the original failure for the caller. } } + if (savedConfig) { + try { + mkdirSync(profileDir(name), { recursive: true }); + writeFileSync(profileConfigFile(name), savedConfig.contents, { mode: savedConfig.mode }); + } catch { + // Best-effort rollback; preserve the original failure for the caller. + } + } throw error; } } From bda8c859f6251d491cfdc5a3b88c034bc91d6f60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Tue, 21 Jul 2026 14:22:56 +0200 Subject: [PATCH 11/19] fix(profile): restore configless profiles on rollback Recreate the source profile directory unconditionally after failed deletion, then restore config bytes only when the original profile had a config file. --- cli/src/lib/profile/model.test.ts | 23 ++++++++++++++++++++++- cli/src/lib/profile/model.ts | 10 +++++----- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/cli/src/lib/profile/model.test.ts b/cli/src/lib/profile/model.test.ts index 7edad85..0db7daa 100644 --- a/cli/src/lib/profile/model.test.ts +++ b/cli/src/lib/profile/model.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { mkdtempSync, readFileSync, renameSync, rmSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, renameSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { setCliContext } from "@/context.ts"; @@ -158,6 +158,27 @@ describe("profile model", () => { expect(keychain.store.secretGet("api-key", "staging")).toBe("atm_staging"); }); + test("restores a configless profile after its quarantined directory was removed", () => { + mkdirSync(profileDir("configless"), { recursive: true }); + const keychain = createFakeKeychain(); + + expect(() => + deleteProfile("configless", keychain.store, { + renameDirectory(source, target) { + renameSync(profileDir(source), profileDir(target)); + }, + removeDirectory(name) { + rmSync(profileDir(name), { recursive: true }); + throw new Error("simulated remove-then-fail"); + }, + setActive: setActiveProfile, + }), + ).toThrow("simulated remove-then-fail"); + + expect(profileExists("configless")).toBe(true); + expect(existsSync(profileConfigFile("configless"))).toBe(false); + }); + test("renames profile config, active selection, and secrets", async () => { await configureRunSet({ profile: "staging", apiKey: "atm_staging", env: "staging" }); setActiveProfile("staging"); diff --git a/cli/src/lib/profile/model.ts b/cli/src/lib/profile/model.ts index d2b35b4..b411447 100644 --- a/cli/src/lib/profile/model.ts +++ b/cli/src/lib/profile/model.ts @@ -550,13 +550,13 @@ export function deleteProfile( // Best-effort rollback; preserve the original failure for the caller. } } - if (savedConfig) { - try { - mkdirSync(profileDir(name), { recursive: true }); + try { + mkdirSync(profileDir(name), { recursive: true }); + if (savedConfig) { writeFileSync(profileConfigFile(name), savedConfig.contents, { mode: savedConfig.mode }); - } catch { - // Best-effort rollback; preserve the original failure for the caller. } + } catch { + // Best-effort rollback; preserve the original failure for the caller. } throw error; } From 1ab5b1db31307d49dce47ed6c6a595300b8cc78d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Tue, 21 Jul 2026 14:24:51 +0200 Subject: [PATCH 12/19] fix(profile): report incomplete rollback recovery Collect failed secret, directory, configuration, and active-profile restoration steps. Return a ConfigurationError with the original cause and actionable rollback details when recovery is incomplete. --- cli/src/lib/profile/model.test.ts | 74 ++++++++++++++++++++++++ cli/src/lib/profile/model.ts | 93 ++++++++++++++++++++++++++----- 2 files changed, 153 insertions(+), 14 deletions(-) diff --git a/cli/src/lib/profile/model.test.ts b/cli/src/lib/profile/model.test.ts index 0db7daa..53ccfd4 100644 --- a/cli/src/lib/profile/model.test.ts +++ b/cli/src/lib/profile/model.test.ts @@ -4,6 +4,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { setCliContext } from "@/context.ts"; import { configureRunSet } from "@/lib/profile-configure-core.ts"; +import { ConfigurationError } from "@/lib/errors.ts"; import { createEmptyProfile, deleteProfile, @@ -179,6 +180,43 @@ describe("profile model", () => { expect(existsSync(profileConfigFile("configless"))).toBe(false); }); + test("reports incomplete recovery after profile deletion fails", () => { + createEmptyProfile("staging"); + const keychain = createFakeKeychain(); + keychain.store.secretSet("api-key", "atm_staging", "staging"); + const secrets = { + ...keychain.store, + secretSet() { + throw new Error("simulated secret restore failure"); + }, + }; + + let thrown: unknown; + try { + deleteProfile("staging", secrets, { + renameDirectory(source, target) { + renameSync(profileDir(source), profileDir(target)); + }, + removeDirectory() { + throw new Error("simulated deletion failure"); + }, + setActive: setActiveProfile, + }); + } catch (error) { + thrown = error; + } + + expect(thrown).toBeInstanceOf(ConfigurationError); + expect((thrown as ConfigurationError).message).toContain("rollback was incomplete"); + expect((thrown as ConfigurationError).cause).toBeInstanceOf(Error); + expect(((thrown as ConfigurationError).cause as Error).message).toBe( + "simulated deletion failure", + ); + expect((thrown as ConfigurationError).details).toContain( + "Rollback failure (restore secret api-key): simulated secret restore failure", + ); + }); + test("renames profile config, active selection, and secrets", async () => { await configureRunSet({ profile: "staging", apiKey: "atm_staging", env: "staging" }); setActiveProfile("staging"); @@ -241,6 +279,42 @@ describe("profile model", () => { expect(keychain.store.secretGet("api-key", "acme_staging")).toBe(""); }); + test("reports incomplete recovery after profile rename fails", () => { + createEmptyProfile("staging"); + setActiveProfile("staging"); + const keychain = createFakeKeychain(); + + let thrown: unknown; + try { + renameProfile("staging", "acme_staging", keychain.store, { + renameDirectory(source, target) { + renameSync(profileDir(source), profileDir(target)); + }, + removeDirectory(name) { + rmSync(profileDir(name), { recursive: true, force: true }); + }, + setActive(name) { + throw new Error( + name === "acme_staging" + ? "simulated rename completion failure" + : "simulated active restore failure", + ); + }, + }); + } catch (error) { + thrown = error; + } + + expect(thrown).toBeInstanceOf(ConfigurationError); + expect((thrown as ConfigurationError).message).toContain("rollback was incomplete"); + expect(((thrown as ConfigurationError).cause as Error).message).toBe( + "simulated rename completion failure", + ); + expect((thrown as ConfigurationError).details).toContain( + "Rollback failure (restore active profile): simulated active restore failure", + ); + }); + test("rejects a no-op rename when the source profile does not exist", () => { expect(() => renameProfile("missing", "missing")).toThrow("Profile not found: missing"); }); diff --git a/cli/src/lib/profile/model.ts b/cli/src/lib/profile/model.ts index b411447..911bdbc 100644 --- a/cli/src/lib/profile/model.ts +++ b/cli/src/lib/profile/model.ts @@ -82,6 +82,34 @@ const PROFILE_MUTATION_OPERATIONS: ProfileMutationOperations = { setActive: setActiveProfile, }; +type ProfileRollbackFailure = { + step: string; + error: unknown; +}; + +function profileMutationErrorDetail(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + +function rethrowAfterProfileRollback( + originalError: unknown, + rollbackFailures: ProfileRollbackFailure[], + message: string, +): never { + if (rollbackFailures.length === 0) { + throw originalError; + } + throw new ConfigurationError(`${message} Automatic rollback was incomplete.`, { + cause: originalError, + details: [ + `Original failure: ${profileMutationErrorDetail(originalError)}`, + ...rollbackFailures.map( + ({ step, error }) => `Rollback failure (${step}): ${profileMutationErrorDetail(error)}`, + ), + ].join("\n"), + }); +} + type ProfileManagementAuth = "oauth" | "api_key" | "none"; type ProfileLakehouseAuth = "basic_token" | "username_password" | "none"; export type ProfileAuth = { @@ -473,28 +501,45 @@ export function renameProfile( operations.setActive(target); } } catch (error) { + const rollbackFailures: ProfileRollbackFailure[] = []; if (secretsMoved) { try { secrets.moveProfileSecrets(target, source, [...PROFILE_SECRET_ACCOUNTS]); - } catch { - // Best-effort rollback; preserve the original failure for the caller. + } catch (rollbackError) { + rollbackFailures.push({ step: "restore secrets", error: rollbackError }); } } if (profileExists(target) && !profileExists(source)) { try { operations.renameDirectory(target, source); - } catch { - // Best-effort rollback; preserve the original failure for the caller. + } catch (rollbackError) { + rollbackFailures.push({ step: "restore profile directory", error: rollbackError }); } } if (active === source) { try { operations.setActive(source); - } catch { - // Best-effort rollback; preserve the original failure for the caller. + } catch (rollbackError) { + rollbackFailures.push({ step: "restore active profile", error: rollbackError }); } } - throw error; + if (!profileExists(source)) { + rollbackFailures.push({ + step: "verify source profile", + error: new Error(`Profile directory is missing: ${source}`), + }); + } + if (profileExists(target)) { + rollbackFailures.push({ + step: "verify target cleanup", + error: new Error(`Renamed profile directory still exists: ${target}`), + }); + } + rethrowAfterProfileRollback( + error, + rollbackFailures, + `Failed to rename profile "${source}" to "${target}". Inspect both profiles before retrying.`, + ); } } @@ -533,21 +578,25 @@ export function deleteProfile( } operations.removeDirectory(quarantinedName); } catch (error) { + const rollbackFailures: ProfileRollbackFailure[] = []; for (const savedSecret of savedSecrets) { if (savedSecret.value.length === 0) { continue; } try { secrets.secretSet(savedSecret.account, savedSecret.value, name); - } catch { - // Best-effort rollback; preserve the original failure for the caller. + } catch (rollbackError) { + rollbackFailures.push({ + step: `restore secret ${savedSecret.account}`, + error: rollbackError, + }); } } if (profileExists(quarantinedName) && !profileExists(name)) { try { operations.renameDirectory(quarantinedName, name); - } catch { - // Best-effort rollback; preserve the original failure for the caller. + } catch (rollbackError) { + rollbackFailures.push({ step: "restore profile directory", error: rollbackError }); } } try { @@ -555,10 +604,26 @@ export function deleteProfile( if (savedConfig) { writeFileSync(profileConfigFile(name), savedConfig.contents, { mode: savedConfig.mode }); } - } catch { - // Best-effort rollback; preserve the original failure for the caller. + } catch (rollbackError) { + rollbackFailures.push({ step: "restore profile configuration", error: rollbackError }); } - throw error; + if (!profileExists(name)) { + rollbackFailures.push({ + step: "verify source profile", + error: new Error(`Profile directory is missing: ${name}`), + }); + } + if (profileExists(quarantinedName)) { + rollbackFailures.push({ + step: "verify quarantine cleanup", + error: new Error(`Quarantined profile directory still exists: ${quarantinedName}`), + }); + } + rethrowAfterProfileRollback( + error, + rollbackFailures, + `Failed to delete profile "${name}". Inspect the profile before retrying.`, + ); } } From d5eb9a24ca2118d035869a2e1a8b135a0ed379cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Tue, 21 Jul 2026 14:25:16 +0200 Subject: [PATCH 13/19] test(redact): cover source-preserving JSON boundaries Exercise nested arrays, escaped sensitive keys, structured sensitive values, whitespace and large-number preservation, and malformed JSON directly against the source-aware redactor. --- cli/src/lib/redact.test.ts | 41 +++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/cli/src/lib/redact.test.ts b/cli/src/lib/redact.test.ts index e23c160..ce6814f 100644 --- a/cli/src/lib/redact.test.ts +++ b/cli/src/lib/redact.test.ts @@ -1,5 +1,9 @@ import { describe, expect, test } from "bun:test"; -import { MASKED_PASSWORD_PLACEHOLDER, redactPasswordFieldInText } from "@/lib/redact.ts"; +import { + MASKED_PASSWORD_PLACEHOLDER, + redactPasswordFieldInText, + redactSensitiveJsonText, +} from "@/lib/redact.ts"; describe("redactPasswordFieldInText", () => { test("redacts password:value without a space after the colon", () => { @@ -20,3 +24,38 @@ describe("redactPasswordFieldInText", () => { ); }); }); + +describe("redactSensitiveJsonText", () => { + const cases = [ + { + name: "nested objects inside arrays", + input: '[{"token":"secret","id":9007199254740993},{"safe":"value"}]', + expected: '[{"token":"[REDACTED]","id":9007199254740993},{"safe":"value"}]', + }, + { + name: "escaped sensitive keys", + input: '{"pass\\u0077ord":"secret","id":9007199254740993}', + expected: '{"pass\\u0077ord":"[REDACTED]","id":9007199254740993}', + }, + { + name: "structured sensitive values", + input: '{"client_secret":{"nested":[1,2]},"refresh_token":["a","b"],"safe":{"x":1}}', + expected: '{"client_secret":"[REDACTED]","refresh_token":"[REDACTED]","safe":{"x":1}}', + }, + { + name: "surrounding and structural whitespace", + input: ' \n { "password" : "secret" , "id" : 9007199254740993 } \t', + expected: ' \n { "password" : "[REDACTED]" , "id" : 9007199254740993 } \t', + }, + ] as const; + + for (const { name, input, expected } of cases) { + test(`redacts ${name} without changing other source text`, () => { + expect(redactSensitiveJsonText(input)).toBe(expected); + }); + } + + test("rejects malformed JSON", () => { + expect(() => redactSensitiveJsonText('{"password":"secret"')).toThrow(); + }); +}); From bfb5ee19df1a77b02b9c8437acfe58903afae5a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Tue, 21 Jul 2026 14:44:29 +0200 Subject: [PATCH 14/19] refactor(profile): clarify mutation rollback flow Compose rename and delete recovery from named rollback steps, snapshot profile state through semantic types, and keep incomplete-recovery diagnostics centralized. --- cli/src/lib/profile/model.ts | 276 ++++++++++++++++++++++------------- 1 file changed, 175 insertions(+), 101 deletions(-) diff --git a/cli/src/lib/profile/model.ts b/cli/src/lib/profile/model.ts index 911bdbc..e1dccbd 100644 --- a/cli/src/lib/profile/model.ts +++ b/cli/src/lib/profile/model.ts @@ -91,23 +91,39 @@ function profileMutationErrorDetail(error: unknown): string { return error instanceof Error ? error.message : String(error); } -function rethrowAfterProfileRollback( +function runProfileRollbackStep( + failures: ProfileRollbackFailure[], + step: string, + operation: () => void, +): void { + try { + operation(); + } catch (error) { + failures.push({ step, error }); + } +} + +function rethrowProfileMutationFailure( originalError: unknown, rollbackFailures: ProfileRollbackFailure[], - message: string, + operation: string, + recoveryAdvice: string, ): never { if (rollbackFailures.length === 0) { throw originalError; } - throw new ConfigurationError(`${message} Automatic rollback was incomplete.`, { - cause: originalError, - details: [ - `Original failure: ${profileMutationErrorDetail(originalError)}`, - ...rollbackFailures.map( - ({ step, error }) => `Rollback failure (${step}): ${profileMutationErrorDetail(error)}`, - ), - ].join("\n"), - }); + throw new ConfigurationError( + `${operation} failed and automatic rollback was incomplete. ${recoveryAdvice}`, + { + cause: originalError, + details: [ + `Original failure: ${profileMutationErrorDetail(originalError)}`, + ...rollbackFailures.map( + ({ step, error }) => `Rollback failure (${step}): ${profileMutationErrorDetail(error)}`, + ), + ].join("\n"), + }, + ); } type ProfileManagementAuth = "oauth" | "api_key" | "none"; @@ -471,6 +487,127 @@ export function updateProfile(name: string, update: ProfileUpdate): ProfileInspe return inspectProfile(name); } +type ProfileConfigSnapshot = { + contents: Buffer; + mode: number; +}; + +type ProfileSecretSnapshot = { + account: (typeof PROFILE_SECRET_ACCOUNTS)[number]; + value: string; +}; + +function snapshotProfileConfig(name: string): ProfileConfigSnapshot | undefined { + const filePath = profileConfigFile(name); + if (!existsSync(filePath)) { + return undefined; + } + return { + contents: readFileSync(filePath), + mode: statSync(filePath).mode & 0o777, + }; +} + +function snapshotProfileSecrets( + name: string, + secrets: ProfileSecretStore, +): ProfileSecretSnapshot[] { + return PROFILE_SECRET_ACCOUNTS.map((account) => ({ + account, + value: secrets.secretGet(account, name), + })); +} + +function rollbackProfileRename(options: { + source: string; + target: string; + wasActive: boolean; + secretsMoved: boolean; + secrets: ProfileSecretStore; + operations: ProfileMutationOperations; +}): ProfileRollbackFailure[] { + const { source, target, wasActive, secretsMoved, secrets, operations } = options; + const failures: ProfileRollbackFailure[] = []; + + if (secretsMoved) { + runProfileRollbackStep(failures, "restore secrets", () => { + secrets.moveProfileSecrets(target, source, PROFILE_SECRET_ACCOUNTS); + }); + } + if (profileExists(target) && !profileExists(source)) { + runProfileRollbackStep(failures, "restore profile directory", () => { + operations.renameDirectory(target, source); + }); + } + if (wasActive) { + runProfileRollbackStep(failures, "restore active profile", () => { + operations.setActive(source); + }); + } + + if (!profileExists(source)) { + failures.push({ + step: "verify source profile", + error: new Error(`Profile directory is missing: ${source}`), + }); + } + if (profileExists(target)) { + failures.push({ + step: "verify target cleanup", + error: new Error(`Renamed profile directory still exists: ${target}`), + }); + } + return failures; +} + +function rollbackProfileDelete(options: { + name: string; + quarantinedName: string; + configSnapshot: ProfileConfigSnapshot | undefined; + secretSnapshots: ProfileSecretSnapshot[]; + secrets: ProfileSecretStore; + operations: ProfileMutationOperations; +}): ProfileRollbackFailure[] { + const { name, quarantinedName, configSnapshot, secretSnapshots, secrets, operations } = options; + const failures: ProfileRollbackFailure[] = []; + + for (const { account, value } of secretSnapshots) { + if (value.length === 0) continue; + runProfileRollbackStep(failures, `restore secret ${account}`, () => { + secrets.secretSet(account, value, name); + }); + } + + if (profileExists(quarantinedName) && !profileExists(name)) { + runProfileRollbackStep(failures, "restore profile directory", () => { + operations.renameDirectory(quarantinedName, name); + }); + } + + runProfileRollbackStep(failures, "restore profile configuration", () => { + mkdirSync(profileDir(name), { recursive: true }); + if (configSnapshot) { + writeFileSync(profileConfigFile(name), configSnapshot.contents, { + mode: configSnapshot.mode, + }); + } + }); + + if (!profileExists(name)) { + failures.push({ + step: "verify source profile", + error: new Error(`Profile directory is missing: ${name}`), + }); + } + if (profileExists(quarantinedName)) { + failures.push({ + step: "verify quarantine cleanup", + error: new Error(`Quarantined profile directory still exists: ${quarantinedName}`), + }); + } + return failures; +} + export function renameProfile( source: string, target: string, @@ -491,54 +628,29 @@ export function renameProfile( throw new ConfigurationError(`Profile already exists: ${target}`); } - const active = getActiveProfileName(); + const wasActive = getActiveProfileName() === source; operations.renameDirectory(source, target); let secretsMoved = false; try { - secrets.moveProfileSecrets(source, target, [...PROFILE_SECRET_ACCOUNTS]); + secrets.moveProfileSecrets(source, target, PROFILE_SECRET_ACCOUNTS); secretsMoved = true; - if (active === source) { + if (wasActive) { operations.setActive(target); } } catch (error) { - const rollbackFailures: ProfileRollbackFailure[] = []; - if (secretsMoved) { - try { - secrets.moveProfileSecrets(target, source, [...PROFILE_SECRET_ACCOUNTS]); - } catch (rollbackError) { - rollbackFailures.push({ step: "restore secrets", error: rollbackError }); - } - } - if (profileExists(target) && !profileExists(source)) { - try { - operations.renameDirectory(target, source); - } catch (rollbackError) { - rollbackFailures.push({ step: "restore profile directory", error: rollbackError }); - } - } - if (active === source) { - try { - operations.setActive(source); - } catch (rollbackError) { - rollbackFailures.push({ step: "restore active profile", error: rollbackError }); - } - } - if (!profileExists(source)) { - rollbackFailures.push({ - step: "verify source profile", - error: new Error(`Profile directory is missing: ${source}`), - }); - } - if (profileExists(target)) { - rollbackFailures.push({ - step: "verify target cleanup", - error: new Error(`Renamed profile directory still exists: ${target}`), - }); - } - rethrowAfterProfileRollback( + const rollbackFailures = rollbackProfileRename({ + source, + target, + wasActive, + secretsMoved, + secrets, + operations, + }); + rethrowProfileMutationFailure( error, rollbackFailures, - `Failed to rename profile "${source}" to "${target}". Inspect both profiles before retrying.`, + `Renaming profile "${source}" to "${target}"`, + "Inspect both profiles before retrying.", ); } } @@ -562,14 +674,8 @@ export function deleteProfile( ); } - const savedSecrets = PROFILE_SECRET_ACCOUNTS.map((account) => ({ - account, - value: secrets.secretGet(account, name), - })); - const configPath = profileConfigFile(name); - const savedConfig = existsSync(configPath) - ? { contents: readFileSync(configPath), mode: statSync(configPath).mode & 0o777 } - : undefined; + const secretSnapshots = snapshotProfileSecrets(name, secrets); + const configSnapshot = snapshotProfileConfig(name); const quarantinedName = `.deleting-${randomBytes(8).toString("hex")}`; operations.renameDirectory(name, quarantinedName); try { @@ -578,51 +684,19 @@ export function deleteProfile( } operations.removeDirectory(quarantinedName); } catch (error) { - const rollbackFailures: ProfileRollbackFailure[] = []; - for (const savedSecret of savedSecrets) { - if (savedSecret.value.length === 0) { - continue; - } - try { - secrets.secretSet(savedSecret.account, savedSecret.value, name); - } catch (rollbackError) { - rollbackFailures.push({ - step: `restore secret ${savedSecret.account}`, - error: rollbackError, - }); - } - } - if (profileExists(quarantinedName) && !profileExists(name)) { - try { - operations.renameDirectory(quarantinedName, name); - } catch (rollbackError) { - rollbackFailures.push({ step: "restore profile directory", error: rollbackError }); - } - } - try { - mkdirSync(profileDir(name), { recursive: true }); - if (savedConfig) { - writeFileSync(profileConfigFile(name), savedConfig.contents, { mode: savedConfig.mode }); - } - } catch (rollbackError) { - rollbackFailures.push({ step: "restore profile configuration", error: rollbackError }); - } - if (!profileExists(name)) { - rollbackFailures.push({ - step: "verify source profile", - error: new Error(`Profile directory is missing: ${name}`), - }); - } - if (profileExists(quarantinedName)) { - rollbackFailures.push({ - step: "verify quarantine cleanup", - error: new Error(`Quarantined profile directory still exists: ${quarantinedName}`), - }); - } - rethrowAfterProfileRollback( + const rollbackFailures = rollbackProfileDelete({ + name, + quarantinedName, + configSnapshot, + secretSnapshots, + secrets, + operations, + }); + rethrowProfileMutationFailure( error, rollbackFailures, - `Failed to delete profile "${name}". Inspect the profile before retrying.`, + `Deleting profile "${name}"`, + "Inspect the profile before retrying.", ); } } From 890402109feac938288bd8e05f5045826a72b068 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Tue, 21 Jul 2026 14:44:46 +0200 Subject: [PATCH 15/19] refactor(redact): expose source parser state Move source-preserving JSON redaction into a small parser object so cursor ownership, recursive traversal, and lexeme preservation are explicit. --- cli/src/lib/redact.ts | 197 ++++++++++++++++++++++-------------------- 1 file changed, 104 insertions(+), 93 deletions(-) diff --git a/cli/src/lib/redact.ts b/cli/src/lib/redact.ts index 4bb76c8..bdc5f8b 100644 --- a/cli/src/lib/redact.ts +++ b/cli/src/lib/redact.ts @@ -41,121 +41,132 @@ export function redactSensitiveJsonValue(value: unknown): unknown { return value; } -function jsonStringEnd(source: string, start: number): number { - let index = start + 1; - while (index < source.length) { - if (source[index] === "\\") { - index += 2; - continue; - } - if (source[index] === '"') { - return index + 1; - } - index += 1; +class SourcePreservingJsonRedactor { + private position = 0; + + constructor(private readonly source: string) {} + + redact(): string { + const leadingWhitespace = this.takeWhitespace(); + const value = this.redactValue(false); + return leadingWhitespace + value + this.takeWhitespace(); } - return source.length; -} -function jsonValueEnd(source: string, start: number): number { - const first = source[start]; - if (first === '"') { - return jsonStringEnd(source, start); + private takeWhitespace(): string { + const start = this.position; + while (/\s/.test(this.source[this.position] ?? "")) this.position += 1; + return this.source.slice(start, this.position); } - if (first !== "{" && first !== "[") { - let index = start; - while (index < source.length && !/[\s,}\]]/.test(source[index] ?? "")) { - index += 1; + + private redactValue(shouldRedact: boolean): string { + if (shouldRedact) { + this.position = this.findValueEnd(this.position); + return '"[REDACTED]"'; } - return index; + if (this.source[this.position] === "{") return this.redactObject(); + if (this.source[this.position] === "[") return this.redactArray(); + + const end = this.findValueEnd(this.position); + const value = this.source.slice(this.position, end); + this.position = end; + return value; } - const closingCharacters = [first === "{" ? "}" : "]"]; - let index = start + 1; - while (index < source.length && closingCharacters.length > 0) { - const character = source[index]; - if (character === '"') { - index = jsonStringEnd(source, index); - continue; + private redactObject(): string { + let output = "{"; + this.position += 1; + output += this.takeWhitespace(); + + while (this.source[this.position] !== "}") { + const keyStart = this.position; + const keyEnd = this.findStringEnd(keyStart); + const rawKey = this.source.slice(keyStart, keyEnd); + const key = JSON.parse(rawKey) as string; + + output += rawKey; + this.position = keyEnd; + output += this.takeWhitespace(); + output += this.source[this.position] ?? ""; + this.position += 1; + output += this.takeWhitespace(); + output += this.redactValue(isSensitiveJsonKey(key)); + output += this.takeWhitespace(); + + if (this.source[this.position] === ",") { + output += ","; + this.position += 1; + output += this.takeWhitespace(); + } } - if (character === "{") closingCharacters.push("}"); - if (character === "[") closingCharacters.push("]"); - if (character === closingCharacters.at(-1)) closingCharacters.pop(); - index += 1; + + this.position += 1; + return `${output}}`; } - return index; -} -/** Redact sensitive object fields while preserving all non-sensitive JSON source lexemes. */ -export function redactSensitiveJsonText(source: string): string { - JSON.parse(source); - let index = 0; + private redactArray(): string { + let output = "["; + this.position += 1; + output += this.takeWhitespace(); + + while (this.source[this.position] !== "]") { + output += this.redactValue(false); + output += this.takeWhitespace(); + if (this.source[this.position] === ",") { + output += ","; + this.position += 1; + output += this.takeWhitespace(); + } + } - function renderWhitespace(): string { - const start = index; - while (/\s/.test(source[index] ?? "")) index += 1; - return source.slice(start, index); + this.position += 1; + return `${output}]`; } - function renderValue(redact: boolean): string { - if (redact) { - index = jsonValueEnd(source, index); - return '"[REDACTED]"'; + private findStringEnd(start: number): number { + let position = start + 1; + while (position < this.source.length) { + if (this.source[position] === "\\") { + position += 2; + continue; + } + if (this.source[position] === '"') return position + 1; + position += 1; } - if (source[index] === "{") return renderObject(); - if (source[index] === "[") return renderArray(); - const end = jsonValueEnd(source, index); - const rendered = source.slice(index, end); - index = end; - return rendered; + return this.source.length; } - function renderObject(): string { - let rendered = "{"; - index += 1; - rendered += renderWhitespace(); - while (source[index] !== "}") { - const keyStart = index; - const keyEnd = jsonStringEnd(source, keyStart); - const rawKey = source.slice(keyStart, keyEnd); - const key = JSON.parse(rawKey) as string; - rendered += rawKey; - index = keyEnd; - rendered += renderWhitespace(); - rendered += source[index] ?? ""; - index += 1; - rendered += renderWhitespace(); - rendered += renderValue(isSensitiveJsonKey(key)); - rendered += renderWhitespace(); - if (source[index] === ",") { - rendered += ","; - index += 1; - rendered += renderWhitespace(); + private findValueEnd(start: number): number { + const firstCharacter = this.source[start]; + if (firstCharacter === '"') return this.findStringEnd(start); + if (firstCharacter !== "{" && firstCharacter !== "[") { + let position = start; + while (position < this.source.length && !/[\s,}\]]/.test(this.source[position] ?? "")) { + position += 1; } + return position; } - index += 1; - return `${rendered}}`; - } - function renderArray(): string { - let rendered = "["; - index += 1; - rendered += renderWhitespace(); - while (source[index] !== "]") { - rendered += renderValue(false); - rendered += renderWhitespace(); - if (source[index] === ",") { - rendered += ","; - index += 1; - rendered += renderWhitespace(); + const closingCharacters = [firstCharacter === "{" ? "}" : "]"]; + let position = start + 1; + while (position < this.source.length && closingCharacters.length > 0) { + const character = this.source[position]; + if (character === '"') { + position = this.findStringEnd(position); + continue; } + if (character === "{") closingCharacters.push("}"); + if (character === "[") closingCharacters.push("]"); + if (character === closingCharacters.at(-1)) closingCharacters.pop(); + position += 1; } - index += 1; - return `${rendered}]`; + return position; } +} - const leadingWhitespace = renderWhitespace(); - const rendered = renderValue(false); - return leadingWhitespace + rendered + renderWhitespace(); +/** Redact sensitive object fields while preserving all non-sensitive JSON source lexemes. */ +export function redactSensitiveJsonText(source: string): string { + JSON.parse(source); + return new SourcePreservingJsonRedactor(source).redact(); } export function truncateBodySnippet(body: string, maxLength = BODY_MAX_LENGTH): string { From cd4c0e503061af55c4a147bd26d02d1bbfb8a4b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Tue, 21 Jul 2026 14:45:05 +0200 Subject: [PATCH 16/19] refactor(config): centralize atomic file updates Share key parsing and temporary-file replacement between set and unset operations, while retaining protected file modes and actionable I/O errors. --- cli/src/lib/config-files.ts | 129 +++++++++++++++--------------------- 1 file changed, 54 insertions(+), 75 deletions(-) diff --git a/cli/src/lib/config-files.ts b/cli/src/lib/config-files.ts index 75c9c0c..caabd56 100644 --- a/cli/src/lib/config-files.ts +++ b/cli/src/lib/config-files.ts @@ -4,10 +4,6 @@ import { dirname, join } from "node:path"; import { readEnv } from "@/lib/env.ts"; import { ConfigurationError } from "@/lib/errors.ts"; -function trim(value: string): string { - return value.trim(); -} - function tempSiblingPath(filePath: string): string { return join(dirname(filePath), `.altertable-kv-${randomBytes(8).toString("hex")}`); } @@ -25,6 +21,11 @@ function configFileError(action: string, filePath: string, cause: unknown): Conf return new ConfigurationError(`Unable to ${action} configuration file: ${filePath}`, { cause }); } +function configKey(line: string): string { + const separatorIndex = line.indexOf("="); + return line.slice(0, separatorIndex === -1 ? undefined : separatorIndex).trim(); +} + function readConfigFileIfPresent(filePath: string): string | undefined { try { return readFileSync(filePath, "utf8"); @@ -36,6 +37,25 @@ function readConfigFileIfPresent(filePath: string): string | undefined { } } +function writeConfigFileAtomically(filePath: string, content: string, action: string): void { + const temporaryPath = tempSiblingPath(filePath); + let failure: unknown; + try { + writeFileSync(temporaryPath, content, { mode: 0o600 }); + renameSync(temporaryPath, filePath); + } catch (error) { + failure = error; + } + try { + rmSync(temporaryPath, { force: true }); + } catch (error) { + failure ??= error; + } + if (failure !== undefined) { + throw configFileError(action, filePath, failure); + } +} + export function configDir(): string { const override = readEnv("ALTERTABLE_CONFIG_HOME"); if (override) { @@ -66,9 +86,9 @@ export function kvGet(filePath: string, key: string): string { if (eqIndex === -1) { continue; } - const lineKey = trim(line.slice(0, eqIndex)); + const lineKey = line.slice(0, eqIndex).trim(); if (lineKey === key) { - return trim(line.slice(eqIndex + 1)); + return line.slice(eqIndex + 1).trim(); } } return ""; @@ -77,81 +97,40 @@ export function kvGet(filePath: string, key: string): string { export function kvSet(filePath: string, key: string, value: string): void { try { mkdirSync(dirname(filePath), { recursive: true }); - const tmpPath = tempSiblingPath(filePath); - let found = false; - const lines = readConfigFileIfPresent(filePath)?.split("\n") ?? []; + } catch (error) { + throw configFileError("write", filePath, error); + } - const output: string[] = []; - for (const line of lines) { - if (line === "" && output.length === 0 && lines.length === 1) { - continue; + const lines = readConfigFileIfPresent(filePath)?.split("\n") ?? []; + let found = false; + const output = lines + .filter((line) => line !== "" || lines.length > 1) + .map((line) => { + if (configKey(line) !== key) { + return line; } - const eqIndex = line.indexOf("="); - const lineKey = eqIndex === -1 ? trim(line) : trim(line.slice(0, eqIndex)); - if (lineKey === key) { - output.push(`${key}=${value}`); - found = true; - } else { - output.push(line); - } - } - - if (!found) { - output.push(`${key}=${value}`); - } + found = true; + return `${key}=${value}`; + }); + if (!found) { + output.push(`${key}=${value}`); + } - try { - writeFileSync( - tmpPath, - output - .filter((line, index, array) => { - if (index === array.length - 1 && line === "") { - return false; - } - return true; - }) - .join("\n") + (output.length > 0 ? "\n" : ""), - { mode: 0o600 }, - ); - renameSync(tmpPath, filePath); - } finally { - rmSync(tmpPath, { force: true }); - } - } catch (error) { - if (error instanceof ConfigurationError) { - throw error; - } - throw configFileError("write", filePath, error); + while (output.at(-1) === "") { + output.pop(); } + writeConfigFileAtomically(filePath, `${output.join("\n")}\n`, "write"); } export function kvUnset(filePath: string, key: string): void { - try { - const content = readConfigFileIfPresent(filePath); - if (content === undefined) { - return; - } - const lines = content.split("\n"); - const tmpPath = tempSiblingPath(filePath); - const output: string[] = []; - for (const line of lines) { - const eqIndex = line.indexOf("="); - const lineKey = eqIndex === -1 ? trim(line) : trim(line.slice(0, eqIndex)); - if (lineKey === key) { - continue; - } - output.push(line); - } - try { - writeFileSync(tmpPath, output.join("\n"), { mode: 0o600 }); - renameSync(tmpPath, filePath); - } finally { - rmSync(tmpPath, { force: true }); - } - } catch (error) { - if (error instanceof ConfigurationError) { - throw error; - } - throw configFileError("update", filePath, error); + const content = readConfigFileIfPresent(filePath); + if (content === undefined) { + return; } + + const output = content + .split("\n") + .filter((line) => configKey(line) !== key) + .join("\n"); + writeConfigFileAtomically(filePath, output, "update"); } From 2bbc5520aca42f8b8874a939c9e9e4b46407a6d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Tue, 21 Jul 2026 14:45:26 +0200 Subject: [PATCH 17/19] refactor(http): unify timeout error mapping Use one transport-error classifier and one active timeout setter across buffered and streaming requests, preserving peer-abort diagnostics and read deadlines. --- cli/src/lib/http.ts | 49 +++++++++++++++++++-------------------------- 1 file changed, 21 insertions(+), 28 deletions(-) diff --git a/cli/src/lib/http.ts b/cli/src/lib/http.ts index 5baf848..9ac7577 100644 --- a/cli/src/lib/http.ts +++ b/cli/src/lib/http.ts @@ -124,6 +124,14 @@ function connectionError(options: HttpSendOptions, cause: unknown): NetworkError ); } +function transportError( + options: HttpSendOptions, + signal: AbortSignal, + cause: unknown, +): NetworkError { + return signal.aborted ? timeoutError(options, cause) : connectionError(options, cause); +} + function streamWithTimeoutCleanup( stream: ReadableStream, clearTimeoutAfterRead: () => void, @@ -147,9 +155,7 @@ function streamWithTimeoutCleanup( } } catch (error) { clearTimeoutAfterRead(); - controller.error( - signal.aborted ? timeoutError(options, error) : connectionError(options, error), - ); + controller.error(transportError(options, signal, error)); } }, cancel(reason) { @@ -408,20 +414,14 @@ async function executeLiveRequest(options: HttpSendOptions): Promise { dispatcher: getSharedDispatcher(), } as RequestInit); } catch (error) { - if (signal.aborted) { - throw timeoutError(options, error); - } - throw connectionError(options, error); + throw transportError(options, signal, error); } let responseBody: string; try { responseBody = await response.text(); } catch (error) { - if (signal.aborted) { - throw timeoutError(options, error); - } - throw connectionError(options, error); + throw transportError(options, signal, error); } logHttpDebug([ @@ -448,9 +448,7 @@ async function executeLiveStream(options: HttpStreamOptions): Promise | undefined = setTimeout(() => { - abortController.abort(); - }, connectTimeoutMs); + let timeout: ReturnType | undefined; const clearActiveTimeout = () => { if (timeout === undefined) { return; @@ -458,6 +456,11 @@ async function executeLiveStream(options: HttpStreamOptions): Promise { + clearActiveTimeout(); + timeout = setTimeout(() => abortController.abort(), durationMs); + }; + startTimeout(connectTimeoutMs); logHttpRequest(options); logDebug(`Request: ${options.method} ${options.url}`); @@ -474,10 +477,7 @@ async function executeLiveStream(options: HttpStreamOptions): Promise= 200 && response.status < 300) { @@ -487,9 +487,7 @@ async function executeLiveStream(options: HttpStreamOptions): Promise { - abortController.abort(); - }, readTimeoutMs); + startTimeout(readTimeoutMs); return streamWithTimeoutCleanup( response.body, clearActiveTimeout, @@ -499,19 +497,14 @@ async function executeLiveStream(options: HttpStreamOptions): Promise 0) { - timeout = setTimeout(() => { - abortController.abort(); - }, readTimeoutMs); + startTimeout(readTimeoutMs); } let responseBody: string; try { responseBody = await response.text(); } catch (error) { - if (abortController.signal.aborted) { - throw timeoutError(options, error); - } - throw connectionError(options, error); + throw transportError(options, abortController.signal, error); } finally { clearActiveTimeout(); } From a86e7ceba80d35c333f1cb9e0164daeaa0ae13e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Tue, 21 Jul 2026 14:45:39 +0200 Subject: [PATCH 18/19] refactor(api): clarify exact integer serialization Name canonical integer JSON and field formatting by their wire purpose, and separate property collection from final request-body serialization. --- cli/src/commands/api/lib/body.ts | 31 +++++++++++++++++-------------- cli/src/commands/api/lib/http.ts | 4 ++-- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/cli/src/commands/api/lib/body.ts b/cli/src/commands/api/lib/body.ts index 3fc3e75..b486c90 100644 --- a/cli/src/commands/api/lib/body.ts +++ b/cli/src/commands/api/lib/body.ts @@ -64,10 +64,10 @@ function parseFieldEntry(entry: string): [string, string] { type ExactJsonInteger = { kind: "exact_json_integer"; - source: string; + json: string; }; -function canonicalIntegerSource(value: string): string { +function canonicalizeJsonInteger(value: string): string { const negative = value.startsWith("-"); const digits = negative ? value.slice(1) : value; const canonicalDigits = digits.replace(/^0+/, "") || "0"; @@ -92,9 +92,9 @@ function parseTypedFieldValue(value: string): ApiFieldValue { return null; } if (/^-?\d+$/.test(value)) { - const source = canonicalIntegerSource(value); - const parsed = Number(source); - return Number.isSafeInteger(parsed) ? parsed : { kind: "exact_json_integer", source }; + const json = canonicalizeJsonInteger(value); + const parsed = Number(json); + return Number.isSafeInteger(parsed) ? parsed : { kind: "exact_json_integer", json }; } if (value === "@-") { return readFileSync(0, "utf8"); @@ -132,23 +132,26 @@ function isExactJsonInteger(value: ApiFieldValue): value is ExactJsonInteger { return typeof value === "object" && value !== null && value.kind === "exact_json_integer"; } -export function apiFieldValueText(value: ApiFieldValue): string { +export function formatApiFieldValue(value: ApiFieldValue): string { if (isExactJsonInteger(value)) { - return value.source; + return value.json; } return value === null ? "null" : String(value); } -function serializeFieldValue(value: ApiFieldValue): string { - return isExactJsonInteger(value) ? value.source : JSON.stringify(value); +function serializeApiFieldValue(value: ApiFieldValue): string { + return isExactJsonInteger(value) ? value.json : JSON.stringify(value); } -function serializeParsedFields(fields: ParsedApiField[]): string { - const body = new Map(); +function serializeApiFields(fields: ParsedApiField[]): string { + const valuesByKey = new Map(); for (const field of fields) { - body.set(field.key, field.value); + valuesByKey.set(field.key, field.value); } - return `{${[...body].map(([key, value]) => `${JSON.stringify(key)}:${serializeFieldValue(value)}`).join(",")}}`; + const properties = [...valuesByKey].map( + ([key, value]) => `${JSON.stringify(key)}:${serializeApiFieldValue(value)}`, + ); + return `{${properties.join(",")}}`; } export type ResolveApiBodyOptions = { @@ -218,7 +221,7 @@ export function resolveApiRequestPayload( if (hasFields) { return { - body: serializeParsedFields(parsedFields), + body: serializeApiFields(parsedFields), queryFields: [], }; } diff --git a/cli/src/commands/api/lib/http.ts b/cli/src/commands/api/lib/http.ts index 563fdd2..c3dbbe8 100644 --- a/cli/src/commands/api/lib/http.ts +++ b/cli/src/commands/api/lib/http.ts @@ -1,5 +1,5 @@ import { - apiFieldValueText, + formatApiFieldValue, resolveApiRequestPayload, type ParsedApiField, } from "@/commands/api/lib/body.ts"; @@ -110,7 +110,7 @@ function appendQueryFields(endpoint: string, fields: ParsedApiField[]): string { const separator = endpoint.includes("?") ? "&" : "?"; const searchParams = new URLSearchParams(); for (const field of fields) { - searchParams.append(field.key, apiFieldValueText(field.value)); + searchParams.append(field.key, formatApiFieldValue(field.value)); } return `${endpoint}${separator}${searchParams.toString()}`; } From 86bec67422cd49980b9e8299b8d15b64a49baf29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Chalifour?= Date: Tue, 21 Jul 2026 18:00:18 +0200 Subject: [PATCH 19/19] feat(profile): use stable IDs for secret storage --- cli/src/lib/profile-store.test.ts | 4 +- cli/src/lib/profile-store.ts | 25 ++++ cli/src/lib/profile/model.test.ts | 50 +++++--- cli/src/lib/profile/model.ts | 38 +++--- cli/src/lib/secrets.test.ts | 3 +- cli/src/lib/secrets.ts | 195 ++++++++++++++---------------- tests/configure.test.ts | 41 +++++-- 7 files changed, 205 insertions(+), 151 deletions(-) diff --git a/cli/src/lib/profile-store.test.ts b/cli/src/lib/profile-store.test.ts index 71fb878..5a17c43 100644 --- a/cli/src/lib/profile-store.test.ts +++ b/cli/src/lib/profile-store.test.ts @@ -8,6 +8,7 @@ import { ensureProfileExists, ensureProfilesLayout, getActiveProfileName, + getProfileId, profileConfigFile, profileExists, resolveWorkingProfile, @@ -33,7 +34,8 @@ describe("profile store", () => { test("creates the default profile layout and active selection", () => { ensureProfilesLayout(); - expect(existsSync(profileConfigFile(DEFAULT_PROFILE_NAME))).toBe(false); + expect(existsSync(profileConfigFile(DEFAULT_PROFILE_NAME))).toBe(true); + expect(getProfileId(DEFAULT_PROFILE_NAME)).toMatch(/^[0-9a-f-]{36}$/); expect(existsSync(join(testHome, "profiles", DEFAULT_PROFILE_NAME))).toBe(true); expect(kvGet(join(testHome, "config"), "active_profile")).toBe(DEFAULT_PROFILE_NAME); }); diff --git a/cli/src/lib/profile-store.ts b/cli/src/lib/profile-store.ts index 5df3573..0c3a785 100644 --- a/cli/src/lib/profile-store.ts +++ b/cli/src/lib/profile-store.ts @@ -1,3 +1,4 @@ +import { randomUUID } from "node:crypto"; import { existsSync, mkdirSync, readdirSync } from "node:fs"; import { join, resolve, sep } from "node:path"; import { configDir, configFile, kvGet, kvSet } from "@/lib/config-files.ts"; @@ -5,6 +6,8 @@ import { ConfigurationError } from "@/lib/errors.ts"; import { readEnv } from "@/lib/env.ts"; export const DEFAULT_PROFILE_NAME = "default"; +const PROFILE_ID_KEY = "profile_id"; +const PROFILE_ID_PATTERN = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/; // Reserved pseudo-profile: the identity in effect when credentials come from // environment variables rather than a stored profile. Never a real directory. @@ -107,9 +110,30 @@ export function profileExists(name: string): boolean { return existsSync(profileDir(name)); } +export function getProfileId(name: string): string | undefined { + if (isFromEnvProfile(name) || !profileExists(name)) { + return undefined; + } + const id = kvGet(profileConfigFile(name), PROFILE_ID_KEY); + return PROFILE_ID_PATTERN.test(id) ? id : undefined; +} + +export function ensureProfileId(name: string): string { + assertSafeProfileName(name); + mkdirSync(profileDir(name), { recursive: true }); + const existingId = getProfileId(name); + if (existingId) { + return existingId; + } + const id = randomUUID(); + kvSet(profileConfigFile(name), PROFILE_ID_KEY, id); + return id; +} + export function ensureProfilesLayout(): void { if (!existsSync(profilesDir())) { mkdirSync(profileDir(DEFAULT_PROFILE_NAME), { recursive: true }); + ensureProfileId(DEFAULT_PROFILE_NAME); } if (kvGet(configFile(), "active_profile").length === 0) { kvSet(configFile(), "active_profile", DEFAULT_PROFILE_NAME); @@ -202,6 +226,7 @@ export function ensureProfileExists(name: string): void { if (!profileExists(name)) { mkdirSync(profileDir(name), { recursive: true }); } + ensureProfileId(name); } export function configGet(key: string, profileName: string): string { diff --git a/cli/src/lib/profile/model.test.ts b/cli/src/lib/profile/model.test.ts index 53ccfd4..dc549ab 100644 --- a/cli/src/lib/profile/model.test.ts +++ b/cli/src/lib/profile/model.test.ts @@ -1,8 +1,9 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { existsSync, mkdirSync, mkdtempSync, readFileSync, renameSync, rmSync } from "node:fs"; +import { mkdirSync, mkdtempSync, readFileSync, renameSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { setCliContext } from "@/context.ts"; +import { credentialsFile, kvGet, kvSet } from "@/lib/config.ts"; import { configureRunSet } from "@/lib/profile-configure-core.ts"; import { ConfigurationError } from "@/lib/errors.ts"; import { @@ -16,6 +17,8 @@ import { } from "@/lib/profile/model.ts"; import { getActiveProfileName, + getProfileId, + ensureProfilesLayout, profileConfigFile, profileDir, profileExists, @@ -105,6 +108,7 @@ describe("profile model", () => { const keychain = createFakeKeychain(); keychain.store.secretSet("api-key", "atm_staging", "staging"); keychain.store.secretSet("lakehouse/password", "lakehouse-secret", "staging"); + const profileId = getProfileId("staging"); deleteProfile("staging", keychain.store); @@ -114,7 +118,7 @@ describe("profile model", () => { expect( keychain.calls.some( (args) => - args.includes("delete-generic-password") && args.includes("profile/staging/api-key"), + args.includes("delete-generic-password") && args.includes(`profile/${profileId}/api-key`), ), ).toBe(true); }); @@ -124,7 +128,7 @@ describe("profile model", () => { process.env.ALTERTABLE_SECRET_BACKEND = "keychain"; const keychain = createFakeKeychain(); keychain.store.secretSet("api-key", "atm_staging", "staging"); - keychain.failingDeletes.add("profile/staging/api-key"); + keychain.failingDeletes.add(`profile/${getProfileId("staging")}/api-key`); expect(() => deleteProfile("staging", keychain.store)).toThrow( "Failed to delete secret from macOS keychain", @@ -159,7 +163,7 @@ describe("profile model", () => { expect(keychain.store.secretGet("api-key", "staging")).toBe("atm_staging"); }); - test("restores a configless profile after its quarantined directory was removed", () => { + test("restores a legacy configless profile with a stable identity after deletion fails", () => { mkdirSync(profileDir("configless"), { recursive: true }); const keychain = createFakeKeychain(); @@ -177,7 +181,7 @@ describe("profile model", () => { ).toThrow("simulated remove-then-fail"); expect(profileExists("configless")).toBe(true); - expect(existsSync(profileConfigFile("configless"))).toBe(false); + expect(getProfileId("configless")).toMatch(/^[0-9a-f-]{36}$/); }); test("reports incomplete recovery after profile deletion fails", () => { @@ -220,33 +224,49 @@ describe("profile model", () => { test("renames profile config, active selection, and secrets", async () => { await configureRunSet({ profile: "staging", apiKey: "atm_staging", env: "staging" }); setActiveProfile("staging"); + const profileId = getProfileId("staging"); renameProfile("staging", "acme_staging"); expect(profileExists("staging")).toBe(false); expect(profileExists("acme_staging")).toBe(true); expect(getActiveProfileName()).toBe("acme_staging"); + expect(getProfileId("acme_staging")).toBe(profileId); expect(secretGet("api-key", "acme_staging")).toBe("atm_staging"); expect(secretGet("api-key", "staging")).toBe(""); }); - test("rolls back profile config and Keychain secrets after a rename failure", () => { + test("renames a profile without rewriting Keychain secrets", () => { createEmptyProfile("staging"); process.env.ALTERTABLE_SECRET_BACKEND = "keychain"; const keychain = createFakeKeychain(); keychain.store.secretSet("api-key", "atm_staging", "staging"); keychain.store.secretSet("lakehouse/password", "lakehouse-secret", "staging"); - keychain.failingWrites.add("profile/acme_staging/lakehouse/password"); + const profileId = getProfileId("staging"); + const callsBeforeRename = keychain.calls.length; - expect(() => renameProfile("staging", "acme_staging", keychain.store)).toThrow( - "Failed to store secret in macOS keychain", - ); + renameProfile("staging", "acme_staging", keychain.store); - expect(profileExists("staging")).toBe(true); - expect(profileExists("acme_staging")).toBe(false); - expect(keychain.store.secretGet("api-key", "staging")).toBe("atm_staging"); - expect(keychain.store.secretGet("lakehouse/password", "staging")).toBe("lakehouse-secret"); - expect(keychain.store.secretGet("api-key", "acme_staging")).toBe(""); + expect(getProfileId("acme_staging")).toBe(profileId); + expect(keychain.store.secretGet("api-key", "acme_staging")).toBe("atm_staging"); + expect(keychain.store.secretGet("lakehouse/password", "acme_staging")).toBe("lakehouse-secret"); + const renameCalls = keychain.calls.slice(callsBeforeRename); + expect(renameCalls.some((args) => args.includes("add-generic-password"))).toBe(false); + expect(renameCalls.some((args) => args.includes("delete-generic-password"))).toBe(false); + }); + + test("migrates legacy name-keyed secrets before renaming a profile", () => { + ensureProfilesLayout(); + mkdirSync(profileDir("legacy"), { recursive: true }); + kvSet(credentialsFile(), "profile/legacy/api-key", "atm_legacy"); + + renameProfile("legacy", "renamed"); + + const profileId = getProfileId("renamed"); + expect(profileId).toBeTruthy(); + expect(secretGet("api-key", "renamed")).toBe("atm_legacy"); + expect(kvGet(credentialsFile(), "profile/legacy/api-key")).toBe(""); + expect(kvGet(credentialsFile(), `profile/${profileId}/api-key`)).toBe("atm_legacy"); }); test("rolls back config and secrets when switching the renamed active profile fails", () => { diff --git a/cli/src/lib/profile/model.ts b/cli/src/lib/profile/model.ts index e1dccbd..84336bd 100644 --- a/cli/src/lib/profile/model.ts +++ b/cli/src/lib/profile/model.ts @@ -20,7 +20,7 @@ import { import { ConfigurationError } from "@/lib/errors.ts"; import { CONFIG_ENV_NAMES, isSecretEnv, readEnv } from "@/lib/env.ts"; import { - moveProfileSecrets, + migrateProfileSecrets, secretDelete, secretExists, secretGet, @@ -56,11 +56,11 @@ const PROFILE_SECRET_ACCOUNTS = [ type ProfileSecretStore = Pick< SecretStore, - "moveProfileSecrets" | "secretDelete" | "secretGet" | "secretSet" + "migrateProfileSecrets" | "secretDelete" | "secretGet" | "secretSet" >; const PROFILE_SECRET_STORE: ProfileSecretStore = { - moveProfileSecrets, + migrateProfileSecrets, secretDelete, secretGet, secretSet, @@ -522,18 +522,11 @@ function rollbackProfileRename(options: { source: string; target: string; wasActive: boolean; - secretsMoved: boolean; - secrets: ProfileSecretStore; operations: ProfileMutationOperations; }): ProfileRollbackFailure[] { - const { source, target, wasActive, secretsMoved, secrets, operations } = options; + const { source, target, wasActive, operations } = options; const failures: ProfileRollbackFailure[] = []; - if (secretsMoved) { - runProfileRollbackStep(failures, "restore secrets", () => { - secrets.moveProfileSecrets(target, source, PROFILE_SECRET_ACCOUNTS); - }); - } if (profileExists(target) && !profileExists(source)) { runProfileRollbackStep(failures, "restore profile directory", () => { operations.renameDirectory(target, source); @@ -571,13 +564,6 @@ function rollbackProfileDelete(options: { const { name, quarantinedName, configSnapshot, secretSnapshots, secrets, operations } = options; const failures: ProfileRollbackFailure[] = []; - for (const { account, value } of secretSnapshots) { - if (value.length === 0) continue; - runProfileRollbackStep(failures, `restore secret ${account}`, () => { - secrets.secretSet(account, value, name); - }); - } - if (profileExists(quarantinedName) && !profileExists(name)) { runProfileRollbackStep(failures, "restore profile directory", () => { operations.renameDirectory(quarantinedName, name); @@ -593,6 +579,13 @@ function rollbackProfileDelete(options: { } }); + for (const { account, value } of secretSnapshots) { + if (value.length === 0) continue; + runProfileRollbackStep(failures, `restore secret ${account}`, () => { + secrets.secretSet(account, value, name); + }); + } + if (!profileExists(name)) { failures.push({ step: "verify source profile", @@ -629,11 +622,9 @@ export function renameProfile( } const wasActive = getActiveProfileName() === source; + secrets.migrateProfileSecrets(source, PROFILE_SECRET_ACCOUNTS); operations.renameDirectory(source, target); - let secretsMoved = false; try { - secrets.moveProfileSecrets(source, target, PROFILE_SECRET_ACCOUNTS); - secretsMoved = true; if (wasActive) { operations.setActive(target); } @@ -642,8 +633,6 @@ export function renameProfile( source, target, wasActive, - secretsMoved, - secrets, operations, }); rethrowProfileMutationFailure( @@ -674,13 +663,14 @@ export function deleteProfile( ); } + secrets.migrateProfileSecrets(name, PROFILE_SECRET_ACCOUNTS); const secretSnapshots = snapshotProfileSecrets(name, secrets); const configSnapshot = snapshotProfileConfig(name); const quarantinedName = `.deleting-${randomBytes(8).toString("hex")}`; operations.renameDirectory(name, quarantinedName); try { for (const account of PROFILE_SECRET_ACCOUNTS) { - secrets.secretDelete(account, name); + secrets.secretDelete(account, quarantinedName); } operations.removeDirectory(quarantinedName); } catch (error) { diff --git a/cli/src/lib/secrets.test.ts b/cli/src/lib/secrets.test.ts index 535fb6e..65d6b7a 100644 --- a/cli/src/lib/secrets.test.ts +++ b/cli/src/lib/secrets.test.ts @@ -3,6 +3,7 @@ import { mkdtempSync, rmSync } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; import { createFakeKeychain } from "@/test-utils/keychain.ts"; +import { getProfileId } from "@/lib/profile-store.ts"; import { secretExists, secretGet, secretSet } from "@/lib/secrets.ts"; let testHome = ""; @@ -56,8 +57,8 @@ describe("secretSet", () => { describe("secretDelete", () => { test("fails closed when Keychain deletion fails", () => { const keychain = createFakeKeychain(); - const account = "profile/default/api-key"; keychain.store.secretSet("api-key", "atm_test", "default"); + const account = `profile/${getProfileId("default")}/api-key`; keychain.failingDeletes.add(account); expect(() => keychain.store.secretDelete("api-key", "default")).toThrow( diff --git a/cli/src/lib/secrets.ts b/cli/src/lib/secrets.ts index 4d5aca7..c7fd35b 100644 --- a/cli/src/lib/secrets.ts +++ b/cli/src/lib/secrets.ts @@ -5,7 +5,12 @@ import { credentialsFile, kvGet, kvSet, kvUnset } from "@/lib/config.ts"; import { CliError } from "@/lib/errors.ts"; import { logWarn } from "@/lib/log.ts"; import { readEnv } from "@/lib/env.ts"; -import { assertSafeProfileName, isFromEnvProfile } from "@/lib/profile-store.ts"; +import { + assertSafeProfileName, + ensureProfileId, + getProfileId, + isFromEnvProfile, +} from "@/lib/profile-store.ts"; type SecretBackend = "macos" | "file"; @@ -36,7 +41,7 @@ export type SecretStore = { secretGet(key: string, profileName: string): string; secretExists(key: string, profileName: string): boolean; secretDelete(key: string, profileName: string): void; - moveProfileSecrets(sourceProfile: string, targetProfile: string, keys: readonly string[]): void; + migrateProfileSecrets(profileName: string, keys: readonly string[]): void; secretStoreDisplay(): string; }; @@ -75,11 +80,15 @@ function requireSafeCredentialsFile(): void { } } -function resolveSecretKey(key: string, profileName: string): string { +function legacySecretKey(key: string, profileName: string): string { assertSafeProfileName(profileName); return `profile/${profileName}/${key}`; } +function stableSecretKey(key: string, profileId: string): string { + return `profile/${profileId}/${key}`; +} + export function createSecretStore( secretProcess: SecretProcess = SYSTEM_SECRET_PROCESS, ): SecretStore { @@ -122,6 +131,53 @@ export function createSecretStore( } } + function readStoredSecret(storageAccount: string): string { + if (secretBackend() === "macos") { + const result = secretProcess.spawnSync( + "security", + ["find-generic-password", "-s", "altertable", "-a", storageAccount, "-w"], + { encoding: "utf8" }, + ); + if (keychainLookupFailed(result)) { + throw new CliError(`Failed to read secret from macOS keychain (${storageAccount}).`); + } + const keychainValue = result.status === 0 ? String(result.stdout).trim() : ""; + if (keychainValue !== "") return keychainValue; + } + requireSafeCredentialsFile(); + return kvGet(credentialsFile(), storageAccount); + } + + function storedSecretExists(storageAccount: string): boolean { + if (secretBackend() === "macos") { + const result = secretProcess.spawnSync( + "security", + ["find-generic-password", "-s", "altertable", "-a", storageAccount], + { stdio: "ignore" }, + ); + if (result.status === 0) return true; + if (keychainLookupFailed(result)) { + throw new CliError(`Failed to inspect secret in macOS keychain (${storageAccount}).`); + } + } + requireSafeCredentialsFile(); + return kvGet(credentialsFile(), storageAccount) !== ""; + } + + function deleteStoredSecret(storageAccount: string): void { + if (secretBackend() === "macos") { + const result = secretProcess.spawnSync( + "security", + ["delete-generic-password", "-s", "altertable", "-a", storageAccount], + { stdio: "ignore" }, + ); + if (result.status !== 0 && result.status !== KEYCHAIN_ITEM_NOT_FOUND_STATUS) { + throw new CliError(`Failed to delete secret from macOS keychain (${storageAccount}).`); + } + } + kvUnset(credentialsFile(), storageAccount); + } + function secretSet( account: string, value: string, @@ -132,7 +188,7 @@ export function createSecretStore( // commands are refused in env mode; in-process caches are deliberately dropped. if (isFromEnvProfile(profileName)) return; - const storageAccount = resolveSecretKey(account, profileName); + const storageAccount = stableSecretKey(account, ensureProfileId(profileName)); const backend = secretBackend(); if (backend === "macos" && !options?.fromArgv) { secretSetMacos(storageAccount, value); @@ -148,115 +204,54 @@ export function createSecretStore( function secretGet(key: string, profileName: string): string { if (isFromEnvProfile(profileName)) return ""; - const secretKey = resolveSecretKey(key, profileName); - const backend = secretBackend(); - if (backend === "macos") { - const result = secretProcess.spawnSync( - "security", - ["find-generic-password", "-s", "altertable", "-a", secretKey, "-w"], - { encoding: "utf8" }, - ); - if (keychainLookupFailed(result)) { - throw new CliError(`Failed to read secret from macOS keychain (${secretKey}).`); - } - const keychainValue = result.status === 0 ? String(result.stdout).trim() : ""; - if (keychainValue !== "") return keychainValue; - requireSafeCredentialsFile(); - return kvGet(credentialsFile(), secretKey); + const profileId = getProfileId(profileName); + if (profileId) { + const value = readStoredSecret(stableSecretKey(key, profileId)); + if (value !== "") return value; } - - requireSafeCredentialsFile(); - return kvGet(credentialsFile(), secretKey); + return readStoredSecret(legacySecretKey(key, profileName)); } function secretExists(key: string, profileName: string): boolean { if (isFromEnvProfile(profileName)) return false; - const secretKey = resolveSecretKey(key, profileName); - const backend = secretBackend(); - if (backend === "macos") { - const result = secretProcess.spawnSync( - "security", - ["find-generic-password", "-s", "altertable", "-a", secretKey], - { stdio: "ignore" }, - ); - if (result.status === 0) return true; - if (keychainLookupFailed(result)) { - throw new CliError(`Failed to inspect secret in macOS keychain (${secretKey}).`); - } - requireSafeCredentialsFile(); - return kvGet(credentialsFile(), secretKey) !== ""; + const profileId = getProfileId(profileName); + if (profileId && storedSecretExists(stableSecretKey(key, profileId))) { + return true; } - - requireSafeCredentialsFile(); - return kvGet(credentialsFile(), secretKey) !== ""; + return storedSecretExists(legacySecretKey(key, profileName)); } function secretDelete(key: string, profileName: string): void { if (isFromEnvProfile(profileName)) return; - const secretKey = resolveSecretKey(key, profileName); - if (secretBackend() === "macos") { - const result = secretProcess.spawnSync( - "security", - ["delete-generic-password", "-s", "altertable", "-a", secretKey], - { stdio: "ignore" }, - ); - if (result.status !== 0 && result.status !== KEYCHAIN_ITEM_NOT_FOUND_STATUS) { - throw new CliError(`Failed to delete secret from macOS keychain (${secretKey}).`); - } - kvUnset(credentialsFile(), secretKey); - return; + const profileId = getProfileId(profileName); + if (profileId) { + deleteStoredSecret(stableSecretKey(key, profileId)); } - - kvUnset(credentialsFile(), secretKey); + deleteStoredSecret(legacySecretKey(key, profileName)); } - function moveProfileSecrets( - sourceProfile: string, - targetProfile: string, - keys: readonly string[], - ): void { - const prepared: Array<{ key: string; sourceValue: string; targetValue: string }> = []; - - function restoreTargetSecrets(): void { - for (const entry of prepared.toReversed()) { - try { - if (entry.targetValue.length > 0) { - secretSet(entry.key, entry.targetValue, targetProfile); - } else { - secretDelete(entry.key, targetProfile); - } - } catch { - // Best-effort rollback; preserve the original failure for the caller. + function migrateProfileSecrets(profileName: string, keys: readonly string[]): void { + const profileId = ensureProfileId(profileName); + const legacySecrets = keys.flatMap((key) => { + const value = readStoredSecret(legacySecretKey(key, profileName)); + return value === "" ? [] : [{ key, value }]; + }); + + for (const { key, value } of legacySecrets) { + const storageAccount = stableSecretKey(key, profileId); + if (!storedSecretExists(storageAccount)) { + if (secretBackend() === "macos") { + secretSetMacos(storageAccount, value); + } else { + kvSet(credentialsFile(), storageAccount, value); + chmodSync(credentialsFile(), 0o600); } } } - - try { - for (const key of keys) { - const sourceValue = secretGet(key, sourceProfile); - if (sourceValue.length === 0) continue; - prepared.push({ key, sourceValue, targetValue: secretGet(key, targetProfile) }); - secretSet(key, sourceValue, targetProfile); - } - } catch (error) { - restoreTargetSecrets(); - throw error; - } - - try { - for (const { key } of prepared) secretDelete(key, sourceProfile); - } catch (error) { - for (const entry of prepared) { - try { - secretSet(entry.key, entry.sourceValue, sourceProfile); - } catch { - // Best-effort rollback; preserve the original failure for the caller. - } - } - restoreTargetSecrets(); - throw error; + for (const { key } of legacySecrets) { + deleteStoredSecret(legacySecretKey(key, profileName)); } } @@ -269,7 +264,7 @@ export function createSecretStore( secretGet, secretExists, secretDelete, - moveProfileSecrets, + migrateProfileSecrets, secretStoreDisplay, }; } @@ -297,12 +292,8 @@ export function secretDelete(key: string, profileName: string): void { systemSecretStore.secretDelete(key, profileName); } -export function moveProfileSecrets( - sourceProfile: string, - targetProfile: string, - keys: readonly string[], -): void { - systemSecretStore.moveProfileSecrets(sourceProfile, targetProfile, keys); +export function migrateProfileSecrets(profileName: string, keys: readonly string[]): void { + systemSecretStore.migrateProfileSecrets(profileName, keys); } export function secretStoreDisplay(): string { diff --git a/tests/configure.test.ts b/tests/configure.test.ts index 251c90e..cc3af73 100644 --- a/tests/configure.test.ts +++ b/tests/configure.test.ts @@ -4,6 +4,17 @@ import { jsonMock, whoamiMock } from "./mock-http.ts"; const queryMock = [jsonMock("POST", "/query", {})]; +async function storedProfileSecret( + workspace: TestWorkspace, + account: string, + value: string, +): Promise { + const config = await workspace.readFile(workspace.defaultProfileConfig); + const profileId = /^profile_id=(.+)$/m.exec(config)?.[1]; + if (!profileId) throw new Error("Default profile ID is missing"); + return `profile/${profileId}/${account}=${value}\n`; +} + describe("altertable profile configure", () => { let workspace: TestWorkspace; @@ -20,16 +31,22 @@ describe("altertable profile configure", () => { await workspace.resetConfig(); expect((await workspace.runCommand("altertable profile configure --user u_blabla --password s_llll")).exitCode).toBe(0); expect(await workspace.readFile(workspace.defaultProfileConfig)).toContain("user=u_blabla\n"); - expect(await workspace.readFile(workspace.credentialsFile)).toContain("profile/default/lakehouse/password=s_llll\n"); + expect(await workspace.readFile(workspace.credentialsFile)).toContain( + await storedProfileSecret(workspace, "lakehouse/password", "s_llll"), + ); expect(await workspace.fileMode(workspace.credentialsFile)).toBe("600"); await workspace.resetConfig(); expect((await workspace.runCommand("altertable profile configure --basic-token dG9rZW4=")).exitCode).toBe(0); - expect(await workspace.readFile(workspace.credentialsFile)).toContain("profile/default/lakehouse/basic-token=dG9rZW4=\n"); + expect(await workspace.readFile(workspace.credentialsFile)).toContain( + await storedProfileSecret(workspace, "lakehouse/basic-token", "dG9rZW4="), + ); await workspace.resetConfig(); expect((await workspace.runCommand("altertable profile configure --api-key atm_prod --env production")).exitCode).toBe(0); - expect(await workspace.readFile(workspace.credentialsFile)).toContain("profile/default/api-key=atm_prod\n"); + expect(await workspace.readFile(workspace.credentialsFile)).toContain( + await storedProfileSecret(workspace, "api-key", "atm_prod"), + ); expect(await workspace.readFile(workspace.defaultProfileConfig)).toContain("api_key_env=production\n"); }); @@ -38,15 +55,19 @@ describe("altertable profile configure", () => { expect((await workspace.runCommand("altertable profile configure --user u1 --password p1")).exitCode).toBe(0); expect((await workspace.runCommand("altertable profile configure --api-key atm_x --env prod")).exitCode).toBe(0); expect(await workspace.readFile(workspace.defaultProfileConfig)).toContain("user=u1\n"); - expect(await workspace.readFile(workspace.credentialsFile)).toContain("profile/default/lakehouse/password=p1\n"); - expect(await workspace.readFile(workspace.credentialsFile)).toContain("profile/default/api-key=atm_x\n"); + expect(await workspace.readFile(workspace.credentialsFile)).toContain( + await storedProfileSecret(workspace, "lakehouse/password", "p1"), + ); + expect(await workspace.readFile(workspace.credentialsFile)).toContain( + await storedProfileSecret(workspace, "api-key", "atm_x"), + ); await workspace.resetConfig(); expect((await workspace.runCommand("altertable profile configure --api-key atm_x --env prod")).exitCode).toBe(0); expect((await workspace.runCommand("altertable profile configure --api-key atm_y --env staging")).exitCode).toBe(0); const credentials = await workspace.readFile(workspace.credentialsFile); expect(credentials).not.toContain("atm_x"); - expect(credentials).toContain("profile/default/api-key=atm_y\n"); + expect(credentials).toContain(await storedProfileSecret(workspace, "api-key", "atm_y")); expect(await workspace.readFile(workspace.defaultProfileConfig)).toContain("api_key_env=staging\n"); }); @@ -61,10 +82,14 @@ describe("altertable profile configure", () => { test("reads stdin secrets", async () => { await workspace.resetConfig(); expect((await workspace.runCommand("altertable profile configure --user alice --password-stdin", { stdin: "s_fromstdin" })).exitCode).toBe(0); - expect(await workspace.readFile(workspace.credentialsFile)).toContain("profile/default/lakehouse/password=s_fromstdin\n"); + expect(await workspace.readFile(workspace.credentialsFile)).toContain( + await storedProfileSecret(workspace, "lakehouse/password", "s_fromstdin"), + ); expect((await workspace.runCommand("altertable profile configure --api-key-stdin --env prod", { stdin: "atm_fromstdin" })).exitCode).toBe(0); - expect(await workspace.readFile(workspace.credentialsFile)).toContain("profile/default/api-key=atm_fromstdin\n"); + expect(await workspace.readFile(workspace.credentialsFile)).toContain( + await storedProfileSecret(workspace, "api-key", "atm_fromstdin"), + ); }); test("profile show reports auth without leaking secrets, and non-TTY configure requires flags", async () => {