fix(security): pass registry login passwords through stdin

Use execFile arguments for local login and the SSH stream for remote login. Discard execution errors after sanitizing their messages so echoed credentials cannot survive in nested error fields.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Gabriel Lima
2026-10-17 12:18:12 -03:00
parent e0c8d80cde
commit b5c5faf9f0
5 changed files with 285 additions and 90 deletions
@@ -0,0 +1,205 @@
import { EventEmitter } from "node:events";
import { Duplex, PassThrough, Writable } from "node:stream";
import { inspect } from "node:util";
import { dockerLogin } from "@dokploy/server/services/registry";
import { execAsyncRemote } from "@dokploy/server/utils/process/execAsync";
import { parse } from "shell-quote";
import { beforeEach, describe, expect, it, vi } from "vitest";
const mocks = vi.hoisted(() => ({
exec: vi.fn(),
execFile: vi.fn(),
remoteExec: vi.fn(),
end: vi.fn(),
}));
vi.mock("node:child_process", async () => {
const { promisify } = await import("node:util");
return {
exec: Object.assign(mocks.exec, {
[promisify.custom]: (command: string) =>
new Promise((resolve, reject) => {
mocks.exec(
command,
(error: Error | null, stdout: string, stderr: string) => {
if (error) reject(Object.assign(error, { stdout, stderr }));
else resolve({ stdout, stderr });
},
);
}),
}),
execFile: mocks.execFile,
};
});
vi.mock("@dokploy/server/services/server", () => ({
findServerById: vi.fn(async () => ({ sshKeyId: "test-key" })),
}));
vi.mock("ssh2", async () => {
const { EventEmitter } = await import("node:events");
return {
Client: class extends EventEmitter {
exec = mocks.remoteExec;
end = mocks.end;
connect() {
queueMicrotask(() => this.emit("ready"));
return this;
}
},
};
});
const password = "fixture'\";$(false)\nsecret";
const registryUrl = "registry.example.com; false";
const username = "user'$(false)";
let localInput: string;
let remoteInput: string;
let localStdin: Writable;
let remoteStream: Duplex & { stderr: PassThrough };
let exitCode: number;
let stdout: string;
let stderr: string;
beforeEach(() => {
vi.clearAllMocks();
localInput = "";
remoteInput = "";
exitCode = 0;
stdout = "Login Succeeded";
stderr = "";
mocks.exec.mockImplementation((command, ...args) => {
const callback = args.at(-1);
queueMicrotask(() => {
callback(
exitCode === 0
? null
: Object.assign(new Error(`Command failed: ${command}`), {
code: exitCode,
}),
stdout,
stderr,
);
});
});
mocks.execFile.mockImplementation(() => {
const child = Object.assign(new EventEmitter(), {
stdout: new PassThrough(),
stderr: new PassThrough(),
stdin: new Writable({
write(chunk, _encoding, callback) {
localInput += chunk.toString();
callback();
},
}),
});
localStdin = child.stdin;
queueMicrotask(() => {
child.stdout.write(stdout);
child.stderr.write(stderr);
child.emit("close", exitCode);
});
return child;
});
mocks.remoteExec.mockImplementation((_command, callback) => {
remoteStream = Object.assign(
new Duplex({
read() {},
write(chunk, _encoding, done) {
remoteInput += chunk.toString();
done();
},
}),
{ stderr: new PassThrough() },
);
callback(null, remoteStream);
queueMicrotask(() => {
remoteStream.emit("data", stdout);
remoteStream.stderr.write(stderr);
remoteStream.emit("close", exitCode);
});
});
});
describe("docker login credentials", () => {
it("passes local credentials only on stdin, without a shell", async () => {
await dockerLogin(registryUrl, username, password);
expect(mocks.exec).not.toHaveBeenCalled();
expect(mocks.execFile).toHaveBeenCalledExactlyOnceWith("docker", [
"login",
registryUrl,
"--username",
username,
"--password-stdin",
]);
expect(localInput).toBe(password);
expect(localStdin.writableEnded).toBe(true);
});
it("escapes remote arguments and sends the password only on stream stdin", async () => {
await dockerLogin(registryUrl, username, password, "test-server");
const command = mocks.remoteExec.mock.calls[0]?.[0];
expect(command).not.toContain(password);
expect(parse(command)).toEqual([
"docker",
"login",
registryUrl,
"--username",
username,
"--password-stdin",
]);
expect(remoteInput).toBe(password);
expect(remoteStream.writableEnded).toBe(true);
expect(mocks.end).toHaveBeenCalledOnce();
});
it("ends local stdin even for an empty password", async () => {
await dockerLogin("", "user", "");
expect(localInput).toBe("");
expect(localStdin.writableEnded).toBe(true);
});
it("keeps remote execution without input unchanged", async () => {
const onData = vi.fn();
stderr = "warning";
await expect(
execAsyncRemote("test-server", "echo hello", onData),
).resolves.toEqual({
stdout,
stderr,
});
expect(remoteStream.writableEnded).toBe(false);
expect(onData.mock.calls).toEqual([[stdout], [stderr]]);
});
it.each([undefined, "test-server"])(
"does not retain passwords echoed by a failed login on %s",
async (serverId) => {
exitCode = 1;
stdout = `stdout: ${password}`;
stderr = `denied: ${password}`;
const error = await dockerLogin(
registryUrl,
username,
password,
serverId,
).catch((error: unknown) => error);
expect(error).toBeInstanceOf(Error);
expect(error).toHaveProperty(
"message",
expect.not.stringContaining(password),
);
expect(inspect(error, { depth: null })).not.toContain(password);
expect(JSON.stringify(error)).not.toContain(
JSON.stringify(password).slice(1, -1),
);
expect(error).not.toHaveProperty("originalError");
expect(error).not.toHaveProperty("cause");
},
);
});
+13 -47
View File
@@ -1,11 +1,9 @@
import {
createRegistry,
execAsyncRemote,
execFileAsync,
dockerLogin,
findRegistryById,
IS_CLOUD,
removeRegistry,
safeDockerLoginCommand,
updateRegistry,
} from "@dokploy/server";
import { db } from "@dokploy/server/db";
@@ -105,14 +103,6 @@ export const registryRouter = createTRPCRouter({
.input(apiTestRegistry)
.mutation(async ({ input }) => {
try {
const args = [
"login",
input.registryUrl,
"--username",
input.username,
"--password-stdin",
];
if (IS_CLOUD && !input.serverId) {
throw new TRPCError({
code: "NOT_FOUND",
@@ -120,20 +110,12 @@ export const registryRouter = createTRPCRouter({
});
}
if (input.serverId && input.serverId !== "none") {
await execAsyncRemote(
input.serverId,
safeDockerLoginCommand(
input.registryUrl,
input.username,
input.password,
),
);
} else {
await execFileAsync("docker", args, {
input: Buffer.from(input.password).toString(),
});
}
await dockerLogin(
input.registryUrl,
input.username,
input.password,
input.serverId,
);
return true;
} catch (error) {
@@ -169,14 +151,6 @@ export const registryRouter = createTRPCRouter({
});
}
const args = [
"login",
registryData.registryUrl,
"--username",
registryData.username,
"--password-stdin",
];
if (IS_CLOUD && !input.serverId) {
throw new TRPCError({
code: "NOT_FOUND",
@@ -184,20 +158,12 @@ export const registryRouter = createTRPCRouter({
});
}
if (input.serverId && input.serverId !== "none") {
await execAsyncRemote(
input.serverId,
safeDockerLoginCommand(
registryData.registryUrl,
registryData.username,
registryData.password,
),
);
} else {
await execFileAsync("docker", args, {
input: Buffer.from(registryData.password).toString(),
});
}
await dockerLogin(
registryData.registryUrl,
registryData.username,
registryData.password,
input.serverId,
);
return true;
} catch (error) {
+47 -19
View File
@@ -3,6 +3,7 @@ import { type apiCreateRegistry, registry } from "@dokploy/server/db/schema";
import {
execAsync,
execAsyncRemote,
execFileAsync,
} from "@dokploy/server/utils/process/execAsync";
import { TRPCError } from "@trpc/server";
import { eq } from "drizzle-orm";
@@ -27,6 +28,32 @@ export function safeDockerLoginCommand(
return `printf %s ${escapedPassword} | docker login ${escapedRegistry} -u ${escapedUser} --password-stdin`;
}
export async function dockerLogin(
registryUrl: string | undefined,
username: string | undefined,
password: string | undefined,
serverId?: string | null,
): Promise<void> {
const args = [
"login",
registryUrl ?? "",
"--username",
username ?? "",
"--password-stdin",
];
try {
if (serverId && serverId !== "none") {
const command = `docker login ${shEscape(registryUrl)} --username ${shEscape(username)} --password-stdin`;
await execAsyncRemote(serverId, command, undefined, password ?? "");
} else {
await execFileAsync("docker", args, { input: password ?? "" });
}
} catch (error) {
// Do not retain an execution error that may contain echoed credentials.
throw new Error(sanitizeRegistryError(error, password));
}
}
function sanitizeRegistryError(
error: unknown,
password: string | null | undefined,
@@ -64,16 +91,17 @@ export const createRegistry = async (
message: "Select a server to add the registry",
});
}
const loginCommand = safeDockerLoginCommand(
input.registryUrl,
input.username,
input.password,
);
try {
if (input.serverId && input.serverId !== "none") {
await execAsyncRemote(input.serverId, loginCommand);
} else if (newRegistry.registryType === "cloud") {
await execAsync(loginCommand);
if (
(input.serverId && input.serverId !== "none") ||
newRegistry.registryType === "cloud"
) {
await dockerLogin(
input.registryUrl,
input.username,
input.password,
input.serverId,
);
}
} catch (error) {
const sanitized = sanitizeRegistryError(error, input.password);
@@ -127,12 +155,6 @@ export const updateRegistry = async (
.returning()
.then((res) => res[0]);
const loginCommand = safeDockerLoginCommand(
response?.registryUrl,
response?.username,
response?.password,
);
if (
IS_CLOUD &&
!registryData?.serverId &&
@@ -145,10 +167,16 @@ export const updateRegistry = async (
}
try {
if (registryData?.serverId && registryData?.serverId !== "none") {
await execAsyncRemote(registryData.serverId, loginCommand);
} else if (response?.registryType === "cloud") {
await execAsync(loginCommand);
if (
(registryData.serverId && registryData.serverId !== "none") ||
response?.registryType === "cloud"
) {
await dockerLogin(
response?.registryUrl,
response?.username,
response?.password,
registryData.serverId,
);
}
} catch (execError) {
throw new Error(sanitizeRegistryError(execError, response?.password));
+7 -19
View File
@@ -26,9 +26,9 @@ import { resolveServiceNetworks } from "./network";
import type { Port } from "./port";
import type { Project } from "./project";
import {
dockerLogin,
findRegistryByIdWithCredentials,
type Registry,
safeDockerLoginCommand,
} from "./registry";
export const createRollback = async (
@@ -182,23 +182,6 @@ export const rollback = async (rollbackId: string) => {
);
};
const dockerLoginForRegistry = async (
registry: Registry,
serverId?: string | null,
) => {
const loginCommand = safeDockerLoginCommand(
registry.registryUrl,
registry.username,
registry.password,
);
if (serverId) {
await execAsyncRemote(serverId, loginCommand);
} else {
await execAsync(loginCommand);
}
};
const rollbackApplication = async (
appName: string,
image: string,
@@ -225,7 +208,12 @@ const rollbackApplication = async (
// alone is not sufficient — Docker Swarm also relies on the daemon's
// cached credentials (~/.docker/config.json) to distribute auth to nodes.
if (rollbackRegistry) {
await dockerLoginForRegistry(rollbackRegistry, serverId);
await dockerLogin(
rollbackRegistry.registryUrl,
rollbackRegistry.username,
rollbackRegistry.password,
serverId,
);
}
const docker = await getRemoteDocker(serverId);
+13 -5
View File
@@ -121,11 +121,6 @@ export const execFileAsync = async (
): Promise<{ stdout: string; stderr: string }> => {
const child = execFile(command, args);
if (options.input && child.stdin) {
child.stdin.write(options.input);
child.stdin.end();
}
return new Promise((resolve, reject) => {
let stdout = "";
let stderr = "";
@@ -149,6 +144,11 @@ export const execFileAsync = async (
});
child.on("error", reject);
if (options.input !== undefined && child.stdin) {
child.stdin.on("error", reject);
child.stdin.end(options.input);
}
});
};
@@ -156,6 +156,7 @@ export const execAsyncRemote = async (
serverId: string | null,
command: string,
onData?: (data: string) => void,
input?: string,
): Promise<{ stdout: string; stderr: string }> => {
if (!serverId) return { stdout: "", stderr: "" };
const server = await findServerById(serverId);
@@ -209,6 +210,13 @@ export const execAsyncRemote = async (
stderr += data.toString();
onData?.(data.toString());
});
if (input !== undefined) {
stream.on("error", (error: Error) => {
conn.end();
reject(error);
});
stream.end(input);
}
});
})
.on("error", (err) => {