mirror of
https://github.com/Dokploy/dokploy.git
synced 2026-10-20 07:31:37 +00:00
fix(traefik): abort middleware update on load error to protect shared middlewares.yml
This commit is contained in:
@@ -0,0 +1,237 @@
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import type { ApplicationNested, Domain } from "@dokploy/server";
|
||||
import { ExecError } from "@dokploy/server/utils/process/ExecError";
|
||||
import { createForwardAuthMiddleware } from "@dokploy/server/utils/traefik/forward-auth";
|
||||
import { afterEach, beforeEach, describe, expect, test, vi } from "vitest";
|
||||
import { parse, stringify } from "yaml";
|
||||
|
||||
const execMocks = vi.hoisted(() => ({
|
||||
execAsyncRemote: vi.fn(),
|
||||
writeFileRemote: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("@dokploy/server/utils/process/execAsync", async (importOriginal) => {
|
||||
const actual =
|
||||
await importOriginal<
|
||||
typeof import("@dokploy/server/utils/process/execAsync")
|
||||
>();
|
||||
return {
|
||||
...actual,
|
||||
execAsyncRemote: execMocks.execAsyncRemote,
|
||||
writeFileRemote: execMocks.writeFileRemote,
|
||||
};
|
||||
});
|
||||
|
||||
const dbMocks = vi.hoisted(() => ({
|
||||
findFirst: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("@dokploy/server/db", () => ({
|
||||
db: {
|
||||
query: {
|
||||
forwardAuthSettings: { findFirst: dbMocks.findFirst },
|
||||
},
|
||||
},
|
||||
}));
|
||||
|
||||
const forwardAuthDomain: Domain = {
|
||||
applicationId: "app-1",
|
||||
certificateType: "none",
|
||||
createdAt: "",
|
||||
domainId: "domain-1",
|
||||
host: "app-b.example.com",
|
||||
https: false,
|
||||
path: null,
|
||||
port: 3000,
|
||||
customEntrypoint: null,
|
||||
serviceName: "",
|
||||
composeId: "",
|
||||
customCertResolver: null,
|
||||
domainType: "application",
|
||||
uniqueConfigKey: 7,
|
||||
previewDeploymentId: "",
|
||||
internalPath: "/",
|
||||
stripPath: false,
|
||||
middlewares: null,
|
||||
forwardAuthEnabled: true,
|
||||
enabled: true,
|
||||
};
|
||||
|
||||
const makeApp = (appName: string, serverId: string | null) =>
|
||||
({
|
||||
appName,
|
||||
serverId,
|
||||
redirects: [],
|
||||
security: [],
|
||||
}) as unknown as ApplicationNested;
|
||||
|
||||
const existingMiddlewaresYaml = stringify({
|
||||
http: {
|
||||
middlewares: {
|
||||
"auth-app-a-1": { basicAuth: { users: ["u:hash"] } },
|
||||
"redirect-to-https": {
|
||||
redirectScheme: { scheme: "https", permanent: true },
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const sshError = () =>
|
||||
new ExecError("SSH connection error: Connection timed out", {
|
||||
command: "cat /etc/dokploy/traefik/dynamic/middlewares.yml",
|
||||
serverId: "server-1",
|
||||
originalError: new Error("Connection timed out"),
|
||||
});
|
||||
|
||||
const parseWritten = () => {
|
||||
expect(execMocks.writeFileRemote).toHaveBeenCalledOnce();
|
||||
const [, , yaml] = execMocks.writeFileRemote.mock.calls[0] ?? [];
|
||||
return parse(yaml as string) as {
|
||||
http?: { middlewares?: Record<string, unknown> };
|
||||
};
|
||||
};
|
||||
|
||||
describe("createForwardAuthMiddleware — remote load error handling", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
execMocks.writeFileRemote.mockResolvedValue(undefined);
|
||||
dbMocks.findFirst.mockResolvedValue({
|
||||
authDomain: "auth.example.com",
|
||||
https: true,
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
test("aborts and does not overwrite middlewares.yml when the SSH load fails", async () => {
|
||||
execMocks.execAsyncRemote.mockRejectedValue(sshError());
|
||||
|
||||
await expect(
|
||||
createForwardAuthMiddleware(
|
||||
makeApp("app-b", "server-1"),
|
||||
forwardAuthDomain,
|
||||
),
|
||||
).rejects.toThrow(/SSH connection error/);
|
||||
|
||||
// The shared middlewares.yml must NOT be overwritten with an empty +
|
||||
// app-b-only config; existing entries on the server are preserved.
|
||||
expect(execMocks.writeFileRemote).not.toHaveBeenCalled();
|
||||
expect(execMocks.execAsyncRemote).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
test("propagates the original ExecError instead of a masked 'File not found'", async () => {
|
||||
execMocks.execAsyncRemote.mockRejectedValue(sshError());
|
||||
|
||||
await expect(
|
||||
createForwardAuthMiddleware(
|
||||
makeApp("app-b", "server-1"),
|
||||
forwardAuthDomain,
|
||||
),
|
||||
).rejects.toBeInstanceOf(ExecError);
|
||||
});
|
||||
|
||||
test("preserves other apps' middlewares and redirect-to-https on successful load", async () => {
|
||||
execMocks.execAsyncRemote.mockResolvedValue({
|
||||
stdout: existingMiddlewaresYaml,
|
||||
stderr: "",
|
||||
});
|
||||
|
||||
await createForwardAuthMiddleware(
|
||||
makeApp("app-b", "server-1"),
|
||||
forwardAuthDomain,
|
||||
);
|
||||
|
||||
const written = parseWritten();
|
||||
const keys = Object.keys(written.http?.middlewares ?? {});
|
||||
|
||||
// app-b's forward-auth middlewares are added
|
||||
expect(keys).toContain("forward-auth-app-b-7");
|
||||
expect(keys).toContain("forward-auth-app-b-7-errors");
|
||||
// app-a's existing auth middleware is preserved (regression guard)
|
||||
expect(keys).toContain("auth-app-a-1");
|
||||
// the built-in server-wide middleware is preserved
|
||||
expect(keys).toContain("redirect-to-https");
|
||||
});
|
||||
|
||||
test("no-ops when forwardAuthEnabled is false", async () => {
|
||||
await createForwardAuthMiddleware(makeApp("app-b", "server-1"), {
|
||||
...forwardAuthDomain,
|
||||
forwardAuthEnabled: false,
|
||||
});
|
||||
|
||||
expect(dbMocks.findFirst).not.toHaveBeenCalled();
|
||||
expect(execMocks.execAsyncRemote).not.toHaveBeenCalled();
|
||||
expect(execMocks.writeFileRemote).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test("no-ops when no auth gate is configured for the server", async () => {
|
||||
dbMocks.findFirst.mockResolvedValue(undefined);
|
||||
|
||||
await createForwardAuthMiddleware(
|
||||
makeApp("app-b", "server-1"),
|
||||
forwardAuthDomain,
|
||||
);
|
||||
|
||||
expect(dbMocks.findFirst).toHaveBeenCalledOnce();
|
||||
expect(execMocks.execAsyncRemote).not.toHaveBeenCalled();
|
||||
expect(execMocks.writeFileRemote).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("createForwardAuthMiddleware — local load error handling", () => {
|
||||
let cwd: string;
|
||||
let dynamicPath: string;
|
||||
|
||||
beforeEach(() => {
|
||||
cwd = fs.mkdtempSync(path.join(os.tmpdir(), "dokploy-fa-"));
|
||||
dynamicPath = path.join(cwd, ".docker", "traefik", "dynamic");
|
||||
fs.mkdirSync(dynamicPath, { recursive: true });
|
||||
vi.spyOn(process, "cwd").mockReturnValue(cwd);
|
||||
vi.clearAllMocks();
|
||||
dbMocks.findFirst.mockResolvedValue({
|
||||
authDomain: "auth.example.com",
|
||||
https: true,
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
fs.rmSync(cwd, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test("aborts and leaves the filesystem untouched when the local file is missing", async () => {
|
||||
// No middlewares.yml present → loadMiddlewares throws "File not found".
|
||||
await expect(
|
||||
createForwardAuthMiddleware(makeApp("app-b", null), forwardAuthDomain),
|
||||
).rejects.toThrow(/File not found/);
|
||||
|
||||
// No destructive empty write happened: the file is still absent.
|
||||
expect(fs.existsSync(path.join(dynamicPath, "middlewares.yml"))).toBe(
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
test("preserves existing middlewares on successful local load", async () => {
|
||||
const configPath = path.join(dynamicPath, "middlewares.yml");
|
||||
fs.writeFileSync(configPath, existingMiddlewaresYaml, "utf8");
|
||||
|
||||
await createForwardAuthMiddleware(
|
||||
makeApp("app-b", null),
|
||||
forwardAuthDomain,
|
||||
);
|
||||
|
||||
const written = parse(fs.readFileSync(configPath, "utf8")) as {
|
||||
http?: { middlewares?: Record<string, unknown> };
|
||||
};
|
||||
const keys = Object.keys(written.http?.middlewares ?? {});
|
||||
|
||||
expect(keys).toContain("forward-auth-app-b-7");
|
||||
expect(keys).toContain("forward-auth-app-b-7-errors");
|
||||
expect(keys).toContain("auth-app-a-1");
|
||||
expect(keys).toContain("redirect-to-https");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,211 @@
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import type { ApplicationNested, Domain } from "@dokploy/server";
|
||||
import { ExecError } from "@dokploy/server/utils/process/ExecError";
|
||||
import { createPathMiddlewares } from "@dokploy/server/utils/traefik/middleware";
|
||||
import { afterEach, beforeEach, describe, expect, test, vi } from "vitest";
|
||||
import { parse, stringify } from "yaml";
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
execAsyncRemote: vi.fn(),
|
||||
writeFileRemote: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("@dokploy/server/utils/process/execAsync", async (importOriginal) => {
|
||||
const actual =
|
||||
await importOriginal<
|
||||
typeof import("@dokploy/server/utils/process/execAsync")
|
||||
>();
|
||||
return {
|
||||
...actual,
|
||||
execAsyncRemote: mocks.execAsyncRemote,
|
||||
writeFileRemote: mocks.writeFileRemote,
|
||||
};
|
||||
});
|
||||
|
||||
// A domain with path-rewriting configured so createPathMiddlewares actually
|
||||
// enters its load → mutate → overwrite body (it early-returns otherwise).
|
||||
const pathRewritingDomain: Domain = {
|
||||
applicationId: "app-1",
|
||||
certificateType: "none",
|
||||
createdAt: "",
|
||||
domainId: "domain-1",
|
||||
host: "app-b.example.com",
|
||||
https: false,
|
||||
path: "/public",
|
||||
port: 3000,
|
||||
customEntrypoint: null,
|
||||
serviceName: "",
|
||||
composeId: "",
|
||||
customCertResolver: null,
|
||||
domainType: "application",
|
||||
uniqueConfigKey: 9,
|
||||
previewDeploymentId: "",
|
||||
internalPath: "/app/v2",
|
||||
stripPath: true,
|
||||
middlewares: null,
|
||||
forwardAuthEnabled: false,
|
||||
enabled: true,
|
||||
};
|
||||
|
||||
const makeApp = (appName: string, serverId: string | null) =>
|
||||
({
|
||||
appName,
|
||||
serverId,
|
||||
redirects: [],
|
||||
security: [],
|
||||
}) as unknown as ApplicationNested;
|
||||
|
||||
// Pre-existing shared middlewares.yml contents from an already-deployed app A
|
||||
// plus the built-in server-wide redirect-to-https middleware.
|
||||
const existingMiddlewaresYaml = stringify({
|
||||
http: {
|
||||
middlewares: {
|
||||
"auth-app-a-1": { basicAuth: { users: ["u:hash"] } },
|
||||
"redirect-app-a-1": {
|
||||
redirectRegex: {
|
||||
regex: "^/$",
|
||||
replacement: "/new",
|
||||
permanent: false,
|
||||
},
|
||||
},
|
||||
"redirect-to-https": {
|
||||
redirectScheme: { scheme: "https", permanent: true },
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const sshError = () =>
|
||||
new ExecError("SSH connection error: Connection timed out", {
|
||||
command: "cat /etc/dokploy/traefik/dynamic/middlewares.yml",
|
||||
serverId: "server-1",
|
||||
originalError: new Error("Connection timed out"),
|
||||
});
|
||||
|
||||
const parseWritten = () => {
|
||||
expect(mocks.writeFileRemote).toHaveBeenCalledOnce();
|
||||
const [, , yaml] = mocks.writeFileRemote.mock.calls[0] ?? [];
|
||||
return parse(yaml as string) as {
|
||||
http?: { middlewares?: Record<string, unknown> };
|
||||
};
|
||||
};
|
||||
|
||||
describe("createPathMiddlewares — remote load error handling", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mocks.writeFileRemote.mockResolvedValue(undefined);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
test("aborts and does not overwrite middlewares.yml when the SSH load fails", async () => {
|
||||
mocks.execAsyncRemote.mockRejectedValue(sshError());
|
||||
|
||||
await expect(
|
||||
createPathMiddlewares(makeApp("app-b", "server-1"), pathRewritingDomain),
|
||||
).rejects.toThrow(/SSH connection error/);
|
||||
|
||||
// The shared middlewares.yml must NOT be overwritten with an empty +
|
||||
// app-b-only config; the existing entries on the server are preserved.
|
||||
expect(mocks.writeFileRemote).not.toHaveBeenCalled();
|
||||
expect(mocks.execAsyncRemote).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
test("does not mask transport errors as a benign 'File not found'", async () => {
|
||||
// The original ExecError must propagate so callers see the real cause
|
||||
// (SSH/parse failure) rather than a misleading "File not found".
|
||||
mocks.execAsyncRemote.mockRejectedValue(sshError());
|
||||
|
||||
await expect(
|
||||
createPathMiddlewares(makeApp("app-b", "server-1"), pathRewritingDomain),
|
||||
).rejects.toBeInstanceOf(ExecError);
|
||||
});
|
||||
|
||||
test("preserves other apps' middlewares and redirect-to-https on successful load", async () => {
|
||||
mocks.execAsyncRemote.mockResolvedValue({
|
||||
stdout: existingMiddlewaresYaml,
|
||||
stderr: "",
|
||||
});
|
||||
|
||||
await createPathMiddlewares(
|
||||
makeApp("app-b", "server-1"),
|
||||
pathRewritingDomain,
|
||||
);
|
||||
|
||||
const written = parseWritten();
|
||||
const keys = Object.keys(written.http?.middlewares ?? {});
|
||||
|
||||
// app-b's new path middlewares are added
|
||||
expect(keys).toContain("addprefix-app-b-9");
|
||||
expect(keys).toContain("stripprefix-app-b-9");
|
||||
// app-a's existing middlewares are preserved (regression guard)
|
||||
expect(keys).toContain("auth-app-a-1");
|
||||
expect(keys).toContain("redirect-app-a-1");
|
||||
// the built-in server-wide middleware is preserved
|
||||
expect(keys).toContain("redirect-to-https");
|
||||
});
|
||||
|
||||
test("early-returns without loading when the domain has no path rewriting", async () => {
|
||||
await createPathMiddlewares(makeApp("app-b", "server-1"), {
|
||||
...pathRewritingDomain,
|
||||
internalPath: "/",
|
||||
stripPath: false,
|
||||
path: null,
|
||||
});
|
||||
|
||||
expect(mocks.execAsyncRemote).not.toHaveBeenCalled();
|
||||
expect(mocks.writeFileRemote).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("createPathMiddlewares — local load error handling", () => {
|
||||
let cwd: string;
|
||||
let dynamicPath: string;
|
||||
|
||||
beforeEach(() => {
|
||||
cwd = fs.mkdtempSync(path.join(os.tmpdir(), "dokploy-mw-"));
|
||||
dynamicPath = path.join(cwd, ".docker", "traefik", "dynamic");
|
||||
fs.mkdirSync(dynamicPath, { recursive: true });
|
||||
vi.spyOn(process, "cwd").mockReturnValue(cwd);
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
fs.rmSync(cwd, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test("aborts and leaves the filesystem untouched when the local file is missing", async () => {
|
||||
// No middlewares.yml present → loadMiddlewares throws "File not found".
|
||||
await expect(
|
||||
createPathMiddlewares(makeApp("app-b", null), pathRewritingDomain),
|
||||
).rejects.toThrow(/File not found/);
|
||||
|
||||
// No destructive empty write happened: the file is still absent.
|
||||
expect(fs.existsSync(path.join(dynamicPath, "middlewares.yml"))).toBe(
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
test("preserves existing middlewares on successful local load", async () => {
|
||||
const configPath = path.join(dynamicPath, "middlewares.yml");
|
||||
fs.writeFileSync(configPath, existingMiddlewaresYaml, "utf8");
|
||||
|
||||
await createPathMiddlewares(makeApp("app-b", null), pathRewritingDomain);
|
||||
|
||||
const written = parse(fs.readFileSync(configPath, "utf8")) as {
|
||||
http?: { middlewares?: Record<string, unknown> };
|
||||
};
|
||||
const keys = Object.keys(written.http?.middlewares ?? {});
|
||||
|
||||
expect(keys).toContain("addprefix-app-b-9");
|
||||
expect(keys).toContain("stripprefix-app-b-9");
|
||||
expect(keys).toContain("auth-app-a-1");
|
||||
expect(keys).toContain("redirect-app-a-1");
|
||||
expect(keys).toContain("redirect-to-https");
|
||||
});
|
||||
});
|
||||
@@ -37,18 +37,6 @@ export const forwardAuthMiddlewareName = (
|
||||
const proxyUrl = () =>
|
||||
`http://${FORWARD_AUTH_SERVICE_NAME}:${FORWARD_AUTH_PORT}`;
|
||||
|
||||
const loadOrEmptyMiddlewares = async (
|
||||
serverId: string | null,
|
||||
): Promise<FileConfig> => {
|
||||
try {
|
||||
return serverId
|
||||
? await loadRemoteMiddlewares(serverId)
|
||||
: loadMiddlewares<FileConfig>();
|
||||
} catch {
|
||||
return { http: { middlewares: {} } };
|
||||
}
|
||||
};
|
||||
|
||||
const persistMiddlewares = async (
|
||||
config: FileConfig,
|
||||
serverId: string | null,
|
||||
@@ -85,7 +73,12 @@ export const createForwardAuthMiddleware = async (
|
||||
const authDomainHttps = authGate.https;
|
||||
|
||||
const { appName, serverId } = app;
|
||||
const config = await loadOrEmptyMiddlewares(serverId);
|
||||
let config: FileConfig;
|
||||
if (serverId) {
|
||||
config = await loadRemoteMiddlewares(serverId);
|
||||
} else {
|
||||
config = loadMiddlewares<FileConfig>();
|
||||
}
|
||||
|
||||
config.http = config.http || {};
|
||||
config.http.middlewares = config.http.middlewares || {};
|
||||
|
||||
@@ -89,22 +89,8 @@ export const loadMiddlewares = <T>() => {
|
||||
export const loadRemoteMiddlewares = async (serverId: string) => {
|
||||
const { DYNAMIC_TRAEFIK_PATH } = paths(true);
|
||||
const configPath = join(DYNAMIC_TRAEFIK_PATH, "middlewares.yml");
|
||||
|
||||
try {
|
||||
const { stdout, stderr } = await execAsyncRemote(
|
||||
serverId,
|
||||
`cat ${configPath}`,
|
||||
);
|
||||
|
||||
if (stderr) {
|
||||
console.error(`Error: ${stderr}`);
|
||||
throw new Error(`File not found: ${configPath}`);
|
||||
}
|
||||
const config = parse(stdout) as FileConfig;
|
||||
return config;
|
||||
} catch (_) {
|
||||
throw new Error(`File not found: ${configPath}`);
|
||||
}
|
||||
const { stdout } = await execAsyncRemote(serverId, `cat ${configPath}`);
|
||||
return parse(stdout) as FileConfig;
|
||||
};
|
||||
export const writeMiddleware = (config: FileConfig) => {
|
||||
const { DYNAMIC_TRAEFIK_PATH } = paths();
|
||||
@@ -132,17 +118,9 @@ export const createPathMiddlewares = async (
|
||||
let config: FileConfig;
|
||||
|
||||
if (app.serverId) {
|
||||
try {
|
||||
config = await loadRemoteMiddlewares(app.serverId);
|
||||
} catch {
|
||||
config = { http: { middlewares: {} } };
|
||||
}
|
||||
config = await loadRemoteMiddlewares(app.serverId);
|
||||
} else {
|
||||
try {
|
||||
config = loadMiddlewares<FileConfig>();
|
||||
} catch {
|
||||
config = { http: { middlewares: {} } };
|
||||
}
|
||||
config = loadMiddlewares<FileConfig>();
|
||||
}
|
||||
|
||||
if (!config) {
|
||||
|
||||
Reference in New Issue
Block a user