Merge pull request #1 from mowdep/copilot/enforce-chmod-600-on-ssh-key

fix: enforce 0o600 on SFTP key files and retry mount on permission failure

apps/agent/src/volume-host/backends/sftp.ts

@@ -4,7 +4,7 @@ import * as os from "node:os";
 import * as path from "node:path";
 import { spawn } from "node:child_process";
 import type { BackendConfig } from "@zerobyte/contracts/volumes";
-import { FILE_MODES, logger, writeFileWithMode } from "@zerobyte/core/node";
+import { FILE_MODES, ensureFileMode, logger, writeFileWithMode } from "@zerobyte/core/node";
 import { toMessage } from "@zerobyte/core/utils";
 import { OPERATION_TIMEOUT, SSH_KEYS_DIR } from "../constants";
 import { getMountForPath } from "../fs";
@@ -142,7 +142,30 @@ const mount = async (config: BackendConfig, mountPath: string) => {
 		const args = [`${config.username}@${config.host}:${config.path || ""}`, mountPath, "-o", options.join(",")];
 		if (config.password) args.push("-o", "password_stdin");
 		logger.info(`Executing sshfs: sshfs ${args.join(" ")}`);
-		await runSshfs(args, config.password);
+
+		try {
+			await runSshfs(args, config.password);
+		} catch (sshfsError) {
+			// On some Docker bind-mount filesystems (e.g. Synology NAS) the chmod call
+			// inside writeFileWithMode may not have taken effect due to ACL inheritance.
+			// SSH refuses to use key files whose permissions are too open, so check and
+			// re-apply 0o600 on all sensitive files written above, then retry once.
+			const [keyFixed, knownHostsFixed] = await Promise.all([
+				config.privateKey
+					? ensureFileMode(getPrivateKeyPath(mountPath), FILE_MODES.ownerReadWrite)
+					: Promise.resolve(false),
+				config.knownHosts
+					? ensureFileMode(getKnownHostsPath(mountPath), FILE_MODES.ownerReadWrite)
+					: Promise.resolve(false),
+			]);
+
+			if (keyFixed || knownHostsFixed) {
+				logger.warn("SFTP mount failed and key file permissions were incorrect; permissions have been fixed. Retrying mount...");
+				await runSshfs(args, config.password);
+			} else {
+				throw sshfsError;
+			}
+		}
 
 		logger.info(`SFTP volume at ${mountPath} mounted successfully.`);
 		return { status: "mounted" as const };

packages/core/src/node/__tests__/fs.test.ts

@@ -2,7 +2,7 @@ import fs from "node:fs/promises";
 import os from "node:os";
 import path from "node:path";
 import { afterEach, describe, expect, test } from "vitest";
-import { FILE_MODES, writeFileWithMode } from "../fs";
+import { FILE_MODES, ensureFileMode, writeFileWithMode } from "../fs";
 
 const tempDirectories = new Set<string>();
 
@@ -15,7 +15,57 @@ afterEach(async () => {
 	tempDirectories.clear();
 });
 
+describe("ensureFileMode", () => {
+	test("returns false and leaves the file unchanged when mode is already correct", async () => {
+		const tempDirectory = await fs.mkdtemp(path.join(os.tmpdir(), "zerobyte-ensure-file-mode-"));
+		tempDirectories.add(tempDirectory);
+
+		const filePath = path.join(tempDirectory, "identity");
+		await fs.writeFile(filePath, "content");
+		await fs.chmod(filePath, 0o600);
+
+		const fixed = await ensureFileMode(filePath, FILE_MODES.ownerReadWrite);
+
+		expect(fixed).toBe(false);
+		expect((await fs.stat(filePath)).mode & 0o777).toBe(0o600);
+	});
+
+	test("returns true and applies the correct mode when permissions are wrong", async () => {
+		const tempDirectory = await fs.mkdtemp(path.join(os.tmpdir(), "zerobyte-ensure-file-mode-"));
+		tempDirectories.add(tempDirectory);
+
+		const filePath = path.join(tempDirectory, "identity");
+		await fs.writeFile(filePath, "content");
+		await fs.chmod(filePath, 0o755);
+
+		const fixed = await ensureFileMode(filePath, FILE_MODES.ownerReadWrite);
+
+		expect(fixed).toBe(true);
+		expect((await fs.stat(filePath)).mode & 0o777).toBe(0o600);
+	});
+
+	test("returns false when the file does not exist", async () => {
+		const tempDirectory = await fs.mkdtemp(path.join(os.tmpdir(), "zerobyte-ensure-file-mode-"));
+		tempDirectories.add(tempDirectory);
+
+		const fixed = await ensureFileMode(path.join(tempDirectory, "nonexistent.key"), FILE_MODES.ownerReadWrite);
+		expect(fixed).toBe(false);
+	});
+});
+
 describe("writeFileWithMode", () => {
+	test("applies the requested mode when creating a new file", async () => {
+		const tempDirectory = await fs.mkdtemp(path.join(os.tmpdir(), "zerobyte-write-file-with-mode-"));
+		tempDirectories.add(tempDirectory);
+
+		const filePath = path.join(tempDirectory, "identity");
+
+		await writeFileWithMode(filePath, "content", FILE_MODES.ownerReadWrite);
+
+		expect(await fs.readFile(filePath, "utf8")).toBe("content");
+		expect((await fs.stat(filePath)).mode & 0o777).toBe(0o600);
+	});
+
 	test("applies the requested mode even when rewriting an existing file", async () => {
 		const tempDirectory = await fs.mkdtemp(path.join(os.tmpdir(), "zerobyte-write-file-with-mode-"));
 		tempDirectories.add(tempDirectory);

packages/core/src/node/fs.ts

@@ -6,7 +6,32 @@ export const FILE_MODES = {
 
 type FileMode = (typeof FILE_MODES)[keyof typeof FILE_MODES];
 
+/**
+ * Checks whether an existing file has the expected mode and, if not, applies chmod.
+ * Returns true when the mode was corrected, false when the file already had the
+ * correct mode or does not exist.
+ */
+export const ensureFileMode = async (filePath: string, mode: FileMode): Promise<boolean> => {
+	try {
+		const stat = await fs.stat(filePath);
+		if ((stat.mode & 0o777) !== mode) {
+			await fs.chmod(filePath, mode);
+			return true;
+		}
+	} catch {
+		// File does not exist or cannot be stat'd — nothing to fix.
+	}
+	return false;
+};
+
 export const writeFileWithMode = async (filePath: string, data: string, mode: FileMode) => {
+	// Remove any existing file first so the mode option on writeFile is always applied
+	// on a fresh file creation (mode is ignored for existing files). This also avoids
+	// inheriting incorrect permissions from a previously-created file on filesystems
+	// where chmod may not behave as expected (e.g. Docker bind-mounts on some NAS systems).
+	await fs.unlink(filePath).catch((error: NodeJS.ErrnoException) => {
+		if (error.code !== "ENOENT") throw error;
+	});
 	await fs.writeFile(filePath, data, { mode });
 	await fs.chmod(filePath, mode);
 };

packages/core/src/node/index.ts

@@ -2,4 +2,4 @@ export { safeSpawn, safeExec } from "./spawn.js";
 export type { SafeSpawnParams, SafeSpawnParamsLines, SafeSpawnParamsRaw, SpawnResult } from "./spawn.js";
 export { logger } from "./logger.js";
 export { sanitizeSensitiveData } from "../utils/sanitize.js";
-export { FILE_MODES, writeFileWithMode } from "./fs.js";
+export { FILE_MODES, writeFileWithMode, ensureFileMode } from "./fs.js";