mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 09:41:04 +01:00
Hide registry credentials from SSH argv and pin publishing actions
Use a mode-0600 RemoteCommand file with percent escaping for sensitive management commands. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
225dd396c5
commit
fb28034fb4
4 files changed
+43
-7
No files matched your search
@@ -29,7 +29,7 @@ jobs:
|
||||
- run: pnpm install --frozen-lockfile
|
||||
- name: Build and publish immutable experimental image
|
||||
run: node cli/node_modules/tsx/dist/cli.mjs scripts/build-exe-dev-image.ts --push --metadata-file /tmp/exe-image.json
|
||||
- uses: actions/upload-artifact@v4
|
||||
- uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4
|
||||
with:
|
||||
name: exe-dev-image-digest
|
||||
path: /tmp/exe-image.json
|
||||
@@ -64,7 +64,7 @@ let context: PluginContext | undefined;
|
||||
|
||||
async function management(config: Config, command: string): Promise<Record<string, unknown>> {
|
||||
let output: string;
|
||||
try { output = await ssh(config, "exe.dev", command); }
|
||||
try { output = await ssh(config, "exe.dev", command, undefined, Boolean(config.registryAuth)); }
|
||||
catch (error) { throw new Error(config.registryAuth ? String((error as Error).message).split(config.registryAuth).join("[REDACTED]") : (error as Error).message); }
|
||||
const result = JSON.parse(output);
|
||||
if (result.error) throw new Error(`exe.dev: ${config.registryAuth ? String(result.error).split(config.registryAuth).join("[REDACTED]") : String(result.error)}`);
|
||||
|
||||
@@ -0,0 +1,27 @@
|
||||
import { EventEmitter } from "node:events";
|
||||
import { PassThrough } from "node:stream";
|
||||
import { readFile, stat } from "node:fs/promises";
|
||||
import { afterEach, expect, it, vi } from "vitest";
|
||||
const mocks = vi.hoisted(() => ({ spawn: vi.fn() }));
|
||||
vi.mock("node:child_process", () => ({ spawn: mocks.spawn }));
|
||||
import { openSsh } from "./transport.js";
|
||||
const cleanups: (() => Promise<void>)[] = [];
|
||||
afterEach(async () => { await Promise.all(cleanups.splice(0).map((cleanup) => cleanup())); mocks.spawn.mockReset(); });
|
||||
it("keeps sensitive commands in a private file, escapes percent expansion, and removes it", async () => {
|
||||
const child = Object.assign(new EventEmitter(), { stdin: new PassThrough(), stdout: new PassThrough(), stderr: new PassThrough() });
|
||||
mocks.spawn.mockReturnValue(child);
|
||||
const command = "new --registry-auth='test:tok%en'";
|
||||
const opened = await openSsh({ sshPrivateKey: "test-key", strictHostKeyChecking: "accept-new", timeoutMs: 1000 }, "exe.dev", command, true);
|
||||
cleanups.push(opened.cleanup);
|
||||
const args = mocks.spawn.mock.calls[0][1] as string[];
|
||||
expect(args.join(" ")).not.toContain("tok%en");
|
||||
const file = args[args.indexOf("-F") + 1];
|
||||
expect((await stat(file)).mode & 0o777).toBe(0o600);
|
||||
expect(await readFile(file, "utf8")).toBe("RemoteCommand new --registry-auth='test:tok%%en'\n");
|
||||
await opened.cleanup();
|
||||
await expect(stat(file)).rejects.toMatchObject({ code: "ENOENT" });
|
||||
});
|
||||
it("rejects newline injection into a sensitive SSH config before spawning", async () => {
|
||||
await expect(openSsh({ strictHostKeyChecking: "accept-new", timeoutMs: 1000 }, "exe.dev", "new\nLocalCommand unsafe", true)).rejects.toThrow("single-line");
|
||||
expect(mocks.spawn).not.toHaveBeenCalled();
|
||||
});
|
||||
@@ -13,7 +13,7 @@ export interface SshConfig {
|
||||
|
||||
export const quote = (value: string): string => `'${value.replace(/'/g, `'"'"'`)}'`;
|
||||
|
||||
export async function openSsh(config: SshConfig, host: string, command: string): Promise<{
|
||||
export async function openSsh(config: SshConfig, host: string, command: string, sensitiveCommand = false): Promise<{
|
||||
child: ChildProcessWithoutNullStreams;
|
||||
cleanup: () => Promise<void>;
|
||||
}> {
|
||||
@@ -32,12 +32,21 @@ export async function openSsh(config: SshConfig, host: string, command: string):
|
||||
knownHosts = path.join(temporary, "known_hosts");
|
||||
await writeFile(knownHosts, config.knownHosts + "\n", { mode: 0o600 });
|
||||
}
|
||||
const args = ["-F", "/dev/null", "-T", "-o", "BatchMode=yes", "-o", "ForwardAgent=no",
|
||||
let configFile = "/dev/null";
|
||||
if (sensitiveCommand) {
|
||||
if (/[\r\n\0]/.test(command)) throw new Error("Sensitive SSH commands must be single-line");
|
||||
configFile = path.join(temporary, "config");
|
||||
// RemoteCommand keeps registry credentials out of the process argv. OpenSSH
|
||||
// expands percent tokens in this directive; escape every literal percent.
|
||||
await writeFile(configFile, `RemoteCommand ${command.replaceAll("%", "%%")}\n`, { mode: 0o600 });
|
||||
}
|
||||
const args = ["-F", configFile, "-T", "-o", "BatchMode=yes", "-o", "ForwardAgent=no",
|
||||
"-o", "ClearAllForwardings=yes", "-o", "ConnectTimeout=15", "-o", "ServerAliveInterval=15",
|
||||
"-o", "ServerAliveCountMax=3", "-o", `StrictHostKeyChecking=${config.knownHosts ? "yes" : config.strictHostKeyChecking}`,
|
||||
"-o", `UserKnownHostsFile=${knownHosts}`];
|
||||
if (identity) args.push("-i", identity, "-o", "IdentitiesOnly=yes");
|
||||
args.push(host, command);
|
||||
args.push(host);
|
||||
if (!sensitiveCommand) args.push(command);
|
||||
const child = spawn("ssh", args, { stdio: "pipe" });
|
||||
child.stdin.on("error", () => {});
|
||||
const cleanup = async () => { await rm(temporary, { recursive: true, force: true }); };
|
||||
@@ -49,8 +58,8 @@ export async function openSsh(config: SshConfig, host: string, command: string):
|
||||
}
|
||||
}
|
||||
|
||||
export async function ssh(config: SshConfig, host: string, command: string, stdin?: string): Promise<string> {
|
||||
const { child } = await openSsh(config, host, command);
|
||||
export async function ssh(config: SshConfig, host: string, command: string, stdin?: string, sensitiveCommand = false): Promise<string> {
|
||||
const { child } = await openSsh(config, host, command, sensitiveCommand);
|
||||
return await new Promise((resolve, reject) => {
|
||||
let stdout = ""; let stderr = ""; let expired = false;
|
||||
const timer = setTimeout(() => { expired = true; child.kill("SIGKILL"); }, config.timeoutMs);
|
||||
|
||||
Reference in new issue
Block a user