fix(config): make legacy migration durable

packages/coding-agent/CHANGELOG.md

@@ -3,6 +3,8 @@
 ## [Unreleased]
 
 ### Fixed
+
+- Hardened legacy config migration against partial files, permission widening, ambiguous publication cleanup, and unbounded failure diagnostics.
 - ACP session configuration now emits the spec-defined `category` field on the Mode, Model, and Thinking select options (`mode`, `model`, `thought_level`), so standards-compliant ACP clients such as Paseo discover models, modes, and thinking levels instead of an empty model picker (#3922).
 - The ACP session model catalog is now filtered to active providers via `providers.list/active`, falling back to the full catalog on older session hosts, so ACP clients no longer list models for providers without usable credentials (#3922).
 

packages/coding-agent/src/config/config-file.ts

@@ -1,5 +1,8 @@
+import { randomUUID } from "node:crypto";
 import * as fs from "node:fs";
 import * as path from "node:path";
+import type { NativeNoReplaceResult } from "@gajae-code/natives";
+import * as native from "@gajae-code/natives";
 import { getAgentDir, isEnoent, logger } from "@gajae-code/utils";
 import { JSONC, YAML } from "bun";
 import type { ZodType } from "zod/v4";
@@ -10,20 +13,257 @@ interface ConfigSchemaError {
 	message: string | undefined;
 }
 
-function migrateJsonToYml(jsonPath: string, ymlPath: string) {
+interface FileIdentity {
+	dev: number | bigint;
+	ino: number | bigint;
+}
+
+function sameIdentity(left: FileIdentity, right: FileIdentity): boolean {
+	return left.dev === right.dev && left.ino === right.ino;
+}
+
+const EVIDENCE_TOKEN = /^[A-Za-z0-9][A-Za-z0-9_.:-]{0,63}$/;
+
+function sanitizeEvidence(value: unknown, fallback: string): string {
+	return typeof value === "string" && EVIDENCE_TOKEN.test(value) ? value : fallback;
+}
+
+function readEvidence(value: unknown, property: string, fallback: string): string {
 	try {
-		if (fs.existsSync(ymlPath)) return;
-		if (!fs.existsSync(jsonPath)) return;
+		if ((typeof value !== "object" && typeof value !== "function") || value === null) return fallback;
+		return sanitizeEvidence(Reflect.get(value, property), fallback);
+	} catch {
+		return fallback;
+	}
+}
+
+function warningDetails(
+	outcomeCode: string,
+	stage: string,
+	error?: unknown,
+	errorCode?: unknown,
+): Record<string, string> {
+	return {
+		outcomeCode: sanitizeEvidence(outcomeCode, "unknown"),
+		stage: sanitizeEvidence(stage, "unknown"),
+		errorCode: sanitizeEvidence(errorCode, readEvidence(error, "code", "unknown")),
+		errorMessage: "Legacy config migration was not proven durable.",
+	};
+}
+
+function warnMigration(outcomeCode: string, stage: string, error?: unknown, errorCode?: unknown): void {
+	logger.warn("migrateJsonToYml: migration not completed", warningDetails(outcomeCode, stage, error, errorCode));
+}
+
+function isStructuredPublishOutcome(value: NativeNoReplaceResult): boolean {
+	return (
+		typeof value.ok === "boolean" &&
+		typeof value.mutationState === "string" &&
+		typeof value.durabilityState === "string" &&
+		typeof value.reason === "string" &&
+		typeof value.primitive === "string" &&
+		value.primitive.length > 0 &&
+		typeof value.phase === "string" &&
+		typeof value.diagnostic === "object" &&
+		value.diagnostic !== null
+	);
+}
+
+function isCommittedPublishOutcome(value: NativeNoReplaceResult): boolean {
+	return (
+		isStructuredPublishOutcome(value) &&
+		value.mutationState === "committed" &&
+		value.durabilityState === "not_attempted" &&
+		value.reason === "none" &&
+		value.phase === "complete"
+	);
+}
+
+const CERTIFIED_NON_COMMIT_REASONS = new Set([
+	"destination_exists",
+	"atomic_unavailable",
+	"invalid_request",
+	"cross_device",
+	"permission_denied",
+	"io_failure",
+	"interrupted",
+	"identity_violation",
+]);
+
+function isCertifiedNonCommit(value: NativeNoReplaceResult): boolean {
+	return (
+		isStructuredPublishOutcome(value) &&
+		value.ok === false &&
+		value.mutationState === "not_committed" &&
+		value.durabilityState === "not_attempted" &&
+		CERTIFIED_NON_COMMIT_REASONS.has(value.reason)
+	);
+}
+
+function writeFully(fd: number, bytes: Uint8Array): void {
+	let offset = 0;
+	while (offset < bytes.byteLength) {
+		const written = fs.writeSync(fd, bytes, offset, bytes.byteLength - offset);
+		if (written <= 0) throw new Error("Config migration temp write made no progress");
+		offset += written;
+	}
+}
+
+function syncParentDirectory(ymlPath: string): void {
+	const noFollow = fs.constants.O_NOFOLLOW;
+	const directory = fs.constants.O_DIRECTORY;
+	if (typeof noFollow !== "number" || noFollow === 0 || typeof directory !== "number" || directory === 0) {
+		throw new Error("Secure directory sync is unsupported");
+	}
+	const fd = fs.openSync(path.dirname(ymlPath), fs.constants.O_RDONLY | directory | noFollow);
+	try {
+		fs.fsyncSync(fd);
+	} finally {
+		fs.closeSync(fd);
+	}
+}
+
+function removeCertifiedTemp(tempPath: string, identity: FileIdentity | undefined): void {
+	if (!identity) return;
+	try {
+		const current = fs.lstatSync(tempPath);
+		if (!current.isFile() || current.isSymbolicLink() || !sameIdentity(current, identity)) return;
+		fs.unlinkSync(tempPath);
+	} catch {
+		// Cleanup is best effort and never grants authority to touch another path.
+	}
+}
+
+export function migrateJsonToYml(jsonPath: string, ymlPath: string): void {
+	let tempPath: string | undefined;
+	let tempIdentity: FileIdentity | undefined;
+	let tempFd: number | undefined;
+	let publicationCommitted = false;
+	let cleanupTemp = true;
+	let stage = "destination_identity";
+	try {
+		try {
+			fs.lstatSync(ymlPath);
+			return;
+		} catch (error) {
+			if (!isEnoent(error)) {
+				warnMigration("destination_identity_failed", stage, error);
+				return;
+			}
+		}
+
+		let source: fs.Stats;
+		try {
+			source = fs.lstatSync(jsonPath);
+		} catch (error) {
+			if (!isEnoent(error)) warnMigration("source_identity_failed", "source_identity", error);
+			return;
+		}
+		if (!source.isFile() || source.isSymbolicLink()) {
+			warnMigration("source_not_regular", "source_identity");
+			return;
+		}
+
+		const noFollow = fs.constants.O_NOFOLLOW;
+		if (typeof noFollow !== "number" || noFollow === 0) {
+			warnMigration("no_follow_unsupported", "source_open");
+			return;
+		}
+		stage = "source_open";
+
+		const sourceFd = fs.openSync(jsonPath, fs.constants.O_RDONLY | noFollow);
+		let content: string | undefined;
+		let sourceMode: number | undefined;
+		try {
+			const openedSource = fs.fstatSync(sourceFd);
+			if (openedSource.isFile() && sameIdentity(source, openedSource)) {
+				sourceMode = openedSource.mode & 0o777;
+				content = fs.readFileSync(sourceFd, "utf8");
+			}
+		} finally {
+			fs.closeSync(sourceFd);
+		}
+		if (content === undefined || sourceMode === undefined) {
+			warnMigration("source_identity_changed", "source_read");
+			return;
+		}
 
-		const content = fs.readFileSync(jsonPath, "utf-8");
 		const parsed = JSON.parse(content);
 		if (!parsed) {
-			logger.warn("migrateJsonToYml: invalid json structure", { path: jsonPath });
+			warnMigration("invalid_json_structure", "source_parse");
 			return;
 		}
-		fs.writeFileSync(ymlPath, YAML.stringify(parsed, null, 2));
+		const bytes = Buffer.from(YAML.stringify(parsed, null, 2), "utf8");
+
+		stage = "temp_create";
+		tempPath = path.join(path.dirname(ymlPath), `.${path.basename(ymlPath)}.${randomUUID()}.tmp`);
+		tempFd = fs.openSync(
+			tempPath,
+			fs.constants.O_WRONLY | fs.constants.O_CREAT | fs.constants.O_EXCL | noFollow,
+			sourceMode,
+		);
+		const openedTemp = fs.fstatSync(tempFd);
+		if (!openedTemp.isFile()) throw new Error("Config migration temp is not a regular file");
+		tempIdentity = openedTemp;
+		stage = "temp_write";
+		writeFully(tempFd, bytes);
+		fs.fchmodSync(tempFd, sourceMode);
+		stage = "temp_sync";
+		fs.fsyncSync(tempFd);
+		fs.closeSync(tempFd);
+		tempFd = undefined;
+
+		stage = "publication";
+		const publication = native.renameNoReplacePath(tempPath, ymlPath);
+		if (isCommittedPublishOutcome(publication)) {
+			publicationCommitted = true;
+			try {
+				stage = "parent_sync";
+				syncParentDirectory(ymlPath);
+			} catch (error) {
+				warnMigration("published_parent_sync_failed", stage, error);
+				return;
+			}
+			if (!publication.ok) {
+				warnMigration(
+					"published_outcome_not_proven",
+					readEvidence(publication, "phase", "publication"),
+					undefined,
+					readEvidence(publication, "code", "unknown"),
+				);
+			}
+			return;
+		}
+		if (isCertifiedNonCommit(publication)) {
+			if (publication.reason !== "destination_exists") {
+				warnMigration(
+					`not_committed_${publication.reason}`,
+					readEvidence(publication, "phase", "publication"),
+					undefined,
+					readEvidence(publication, "code", "unknown"),
+				);
+			}
+			return;
+		}
+		cleanupTemp = false;
+		warnMigration(
+			"publication_outcome_indeterminate",
+			readEvidence(publication, "phase", "publication"),
+			undefined,
+			readEvidence(publication, "code", "unknown"),
+		);
 	} catch (error) {
-		logger.warn("migrateJsonToYml: migration failed", { error: String(error) });
+		if (stage === "publication") cleanupTemp = false;
+		warnMigration("migration_failed", stage, error);
+	} finally {
+		if (tempFd !== undefined) {
+			try {
+				fs.closeSync(tempFd);
+			} catch {
+				// The identity check below remains authoritative for cleanup.
+			}
+		}
+		if (!publicationCommitted && cleanupTemp && tempPath) removeCertifiedTemp(tempPath, tempIdentity);
 	}
 }