docker testing rewrite: bake the image, drop the allowlist, kill the quoting (#750)
* docker testing rewrite: bake the image, drop the allowlist, kill the quoting - new `pnpm gha <script>` wrapper. one entry point for running any node script in the GHA-like container; replaces the runtime apt-get + useradd + chown ceremony in `action/utils/docker.ts`. - `action/Dockerfile` bakes ubuntu:24.04 + node 24 + gh + jq + sudo + testuser at uid 1000. `action/docker-entrypoint.sh` remaps to the host uid/gid and `exec`s the requested command — no `bash -c` nesting, no `escapeForDoubleQuotes`. - env passthrough: full `process.env` (+ `.env` via dotenv) flows through `--env-file`, multi-line values via `-e` fallback. drops `EnvFilterMode` / `testEnvAllowList`. - image rebuild is content-hash gated on Dockerfile + entrypoint; volume is versioned by hash so a stale `node_modules` cache from an old image can't poison a new one. - `action/play.ts` slimmed to a CLI; `run()` extracted to `action/utils/runFixture.ts`. drops the `--local` / `PLAY_LOCAL` dual mode in favor of explicit `play:local` / `runtest:local` scripts. - `action/test/run.ts` no longer self-relaunches into docker — that's `gha`'s job now. - `action/test/coverage.ts` `ALWAYS_RUN_ALL` updated to track the new files. - `wiki/docker.md` rewritten (243 → 105 lines). `wiki/action-tests.md`, `wiki/billing.md`, `wiki/adversarial.md`, `README.md`, `AGENTS.md` all updated to drop `--local` / `PLAY_LOCAL` references. verified end-to-end: `pnpm play` runs the default fixture against pullfrog/scratch, exit 0; `sudo unshare --pid` still works inside the container; `pnpm runtest` boots through the wrapper. * gha: address review feedback + 3 related issues found locally review-flagged: - bare `pnpm gha --build` now builds the image and exits 0 (was printing help and exiting 1 — docs claimed it was a valid standalone) - `initVolumeOwnership` skipped when the named volume already exists; saves the ~240ms `docker run … chown` on every warm invocation - `GIT_SSH_COMMAND` gate widened to any `id_*` private key (was hard- coded to `id_rsa`, leaving ed25519-only linux contributors with the default ssh config). dropped `-i` so ssh picks whichever key exists - new `action/.dockerignore` — partial mitigation noted: BuildKit (default since docker 23) only sends files referenced by the Dockerfile (~42B in practice), so the perf concern is mostly hypothetical. file is still worth keeping for `DOCKER_BUILDKIT=0` fallback and as documented intent for future `COPY . .` additions related issues found while validating locally: - `parseArgs` now stops flag-parsing at the first positional (or literal `--`); `pnpm gha test/run.ts --build` previously intercepted `--build` as a gha flag instead of forwarding to `test/run.ts` - new `pnpm gha --clean` command prunes orphan `pullfrog-gha:*` images and `pullfrog-gha-node-modules-*` volumes whose hash doesn't match the current Dockerfile (each Dockerfile/entrypoint edit creates a fresh hash and orphans the prior pair, ~600MB + ~200MB each — without a cleaner they accumulate silently) - `--shell` without a TTY now fails fast with an actionable message before docker is invoked, instead of producing the confusing `the input device is not a TTY` from docker run wiki updated: documents `--clean`, the parseArgs passthrough rule, and a new "Reclaiming disk" section. * gha: fidelity, flexibility, and signal-safety improvements investigated local fidelity vs the real GHA ubuntu-24.04 runner and addressed the gaps that have actually bitten contributors or could. fidelity (image now matches GHA closer): - bake build-essential, wget, xz-utils, file alongside the existing toolset. gh, jq, git, python3, sudo, ssh, build-essential, wget, xz, file, unzip, curl all present. native module builds (node-gyp, any package missing arm64 prebuilts) now work; common agent shell calls don't hit ENOENT - `host.docker.internal:host-gateway` flag wires the host into the container's DNS on linux (macOS Docker Desktop bakes it in). lets scripts that hit a local dev server use `API_URL=http://host.docker. internal:3100` and work identically on both platforms - `--init` makes tini PID 1, fixing signal forwarding during the pre-exec warmup window (Ctrl-C was previously taking up to 10s to tear down because bash-as-PID-1 swallowed the signal) - pnpm version is correctly pinned via the workspace's `packageManager` field — corepack resolves it at install time; verified via the new `--doctor` command flexibility (new affordances): - `pnpm gha --doctor` runs an inside-the-container fidelity audit: os + arch + node/pnpm/python versions, version snapshots of every baked tool, env vars (CI, HOME, TMPDIR), uid/gid, and the host.docker.internal resolution. useful for "works in CI fails locally" or vice versa - `pnpm gha --build --no-cache` busts the docker layer cache when an apt mirror, base image, or external download has changed upstream - entrypoint's `pnpm install` warmup is now wrapped in a `flock` on a file in the shared node_modules volume — concurrent `pnpm gha` invocations (e.g. play in one terminal, runtest in another) serialize their install instead of racing docs: - new "Gaps (known)" section in wiki/docker.md explicitly calling out the things this system can't do yet, including the missing `uses: ./action` semantics gap that `.github/workflows/action-gha-e2e-adhoc.yml` currently fills via GHA only (designing a local `pnpm gha-action <fixture>` is on the roadmap), service containers, parallel-run sharing, and arch differences (arm64 vs amd64) * docs: audit + corrections after testing fronts self-audit pass for stale references and incomplete pointers: - wiki/browser.md: `Docker (node:24)` → `pnpm gha container (ubuntu:24.04)`. the substance was right (chrome not preinstalled) but the base image reference was stale. - wiki/docker.md: the "Permission errors" troubleshooting line claimed the node_modules volume is chowned on every run; now correctly says "owned by the host uid on first creation; warm runs skip the chown" to match the actual behavior after the initVolumeOwnership fix. - wiki/action-tests.md: `API_URL` env-var doc now mentions BOTH paths (`localhost:` from play:local, `host.docker.internal:` from inside the container). Proxy/router recipe now shows both invocations side-by-side instead of saying "must use play:local". - wiki/billing.md: same dual-recipe update for the loop-including-the- action proxy walkthrough. - gha.ts header: expanded the usage block to include --clean / --doctor / --no-cache / --shell-TTY, added the host.docker.internal note, and pointed at wiki/docker.md for design rationale. self-document check: a future agent landing on this code can answer "how do I run a fixture / debug in shell / add a tool / diagnose fidelity / reach a local dev server" purely from gha.ts header + wiki/docker.md without spelunking through the entrypoint or git history.
This commit is contained in:
Colin McDonnell
committed by
pullfrog[bot]
pullfrog[bot]
parent
8e1acfba99
commit
76879b27ec
-286
@@ -1,286 +0,0 @@
|
||||
/**
|
||||
* shared docker utilities for running commands in containers.
|
||||
* used by both play.ts (dev) and test/run.ts (CI).
|
||||
*/
|
||||
|
||||
import { type SpawnSyncReturns, spawnSync } from "node:child_process";
|
||||
import { existsSync } from "node:fs";
|
||||
import { platform } from "node:os";
|
||||
import { join } from "node:path";
|
||||
|
||||
export type DockerRunContext = {
|
||||
actionDir: string;
|
||||
args: string[];
|
||||
platformName: NodeJS.Platform;
|
||||
home: string | undefined;
|
||||
env: NodeJS.ProcessEnv;
|
||||
uid: number;
|
||||
gid: number;
|
||||
};
|
||||
|
||||
export type SshSetup = {
|
||||
sshFlags: string[];
|
||||
sshSetupCmd: string;
|
||||
};
|
||||
|
||||
export type DockerRunArgsContext = {
|
||||
ctx: DockerRunContext;
|
||||
envFlags: string[];
|
||||
nodeCmd: string;
|
||||
sshSetup: SshSetup;
|
||||
volumeName: string;
|
||||
};
|
||||
|
||||
export type VolumeInitContext = {
|
||||
actionDir: string;
|
||||
volumeName: string;
|
||||
uid: number;
|
||||
gid: number;
|
||||
};
|
||||
|
||||
export function buildDockerRunContext(ctx: {
|
||||
actionDir: string;
|
||||
args: string[];
|
||||
}): DockerRunContext {
|
||||
return {
|
||||
actionDir: ctx.actionDir,
|
||||
args: ctx.args,
|
||||
platformName: platform(),
|
||||
home: process.env.HOME,
|
||||
env: process.env,
|
||||
uid: process.getuid?.() ?? 1000,
|
||||
gid: process.getgid?.() ?? 1000,
|
||||
};
|
||||
}
|
||||
|
||||
export function assertDockerSupported(ctx: DockerRunContext): void {
|
||||
if (ctx.platformName === "win32") {
|
||||
throw new Error("docker mode is not supported on native windows. use wsl2.");
|
||||
}
|
||||
}
|
||||
|
||||
function buildDarwinSshSetup(ctx: DockerRunContext): SshSetup {
|
||||
const sshFlags: string[] = [];
|
||||
const sshSetupCmd = "";
|
||||
if (ctx.home) {
|
||||
const knownHostsPath = join(ctx.home, ".ssh", "known_hosts");
|
||||
if (existsSync(knownHostsPath)) {
|
||||
sshFlags.push("-v", `${knownHostsPath}:/root/.ssh/known_hosts:ro`);
|
||||
}
|
||||
}
|
||||
sshFlags.push(
|
||||
"-v",
|
||||
"/run/host-services/ssh-auth.sock:/run/host-services/ssh-auth.sock",
|
||||
"-e",
|
||||
"SSH_AUTH_SOCK=/run/host-services/ssh-auth.sock"
|
||||
);
|
||||
return { sshFlags, sshSetupCmd };
|
||||
}
|
||||
|
||||
function buildLinuxSshSetup(ctx: DockerRunContext): SshSetup {
|
||||
const sshFlags: string[] = [];
|
||||
let sshSetupCmd = "";
|
||||
if (ctx.home) {
|
||||
const sshDir = join(ctx.home, ".ssh");
|
||||
if (existsSync(sshDir)) {
|
||||
sshFlags.push("-v", `${sshDir}:/tmp/.ssh-host:ro`);
|
||||
sshSetupCmd =
|
||||
"mkdir -p /tmp/home/.ssh && cp /tmp/.ssh-host/id_* /tmp/home/.ssh/ 2>/dev/null; chmod 600 /tmp/home/.ssh/id_* 2>/dev/null; " +
|
||||
"ssh-keyscan -t ed25519,rsa github.com >> /tmp/home/.ssh/known_hosts 2>/dev/null; chmod 644 /tmp/home/.ssh/known_hosts; " +
|
||||
"export GIT_SSH_COMMAND='ssh -i /tmp/home/.ssh/id_rsa -o UserKnownHostsFile=/tmp/home/.ssh/known_hosts -o StrictHostKeyChecking=no'; ";
|
||||
}
|
||||
}
|
||||
return { sshFlags, sshSetupCmd };
|
||||
}
|
||||
|
||||
export function buildSshSetup(ctx: DockerRunContext): SshSetup {
|
||||
if (ctx.platformName === "darwin") {
|
||||
return buildDarwinSshSetup(ctx);
|
||||
}
|
||||
return buildLinuxSshSetup(ctx);
|
||||
}
|
||||
|
||||
// allowlist of env vars to pass through to the container for `pnpm runtest`.
|
||||
// NOTE: `pnpm play` uses "passthrough" mode and passes ALL env vars.
|
||||
// if your env var isn't working with `pnpm runtest`, add it here!
|
||||
// see wiki/adversarial.md for documentation.
|
||||
const testEnvAllowList = new Set([
|
||||
"CI",
|
||||
"GITHUB_ACTIONS",
|
||||
"PULLFROG_DISABLE_SECURITY_INSTRUCTIONS", // disables security messaging for pentest
|
||||
"GITHUB_TOKEN",
|
||||
"GH_TOKEN",
|
||||
"GITHUB_REPOSITORY",
|
||||
"GITHUB_APP_ID",
|
||||
"GITHUB_PRIVATE_KEY",
|
||||
"OPENAI_API_KEY",
|
||||
"ANTHROPIC_API_KEY",
|
||||
"CLAUDE_CODE_OAUTH_TOKEN",
|
||||
"GEMINI_API_KEY",
|
||||
"GOOGLE_GENERATIVE_AI_API_KEY",
|
||||
"XAI_API_KEY",
|
||||
"DEEPSEEK_API_KEY",
|
||||
"OPENROUTER_API_KEY",
|
||||
"MOONSHOT_API_KEY",
|
||||
"OPENCODE_API_KEY",
|
||||
"PULLFROG_MODEL",
|
||||
"LOG_LEVEL",
|
||||
"DEBUG",
|
||||
"NODE_ENV",
|
||||
"PLAY_LOCAL",
|
||||
"HOME",
|
||||
"USER",
|
||||
"SSH_AUTH_SOCK",
|
||||
"ACTIONS_ID_TOKEN_REQUEST_URL",
|
||||
"ACTIONS_ID_TOKEN_REQUEST_TOKEN",
|
||||
"GITHUB_API_URL",
|
||||
"GITHUB_SERVER_URL",
|
||||
"GITHUB_GRAPHQL_URL",
|
||||
"GITHUB_OUTPUT",
|
||||
]);
|
||||
|
||||
export type EnvFilterMode = "allowlist" | "passthrough";
|
||||
|
||||
export function buildEnvFlags(ctx: DockerRunContext, mode: EnvFilterMode): string[] {
|
||||
const envFlags: string[] = [];
|
||||
const entries = Object.entries(ctx.env);
|
||||
|
||||
for (const entry of entries) {
|
||||
const key = entry[0];
|
||||
const value = entry[1];
|
||||
|
||||
if (value === undefined) continue;
|
||||
|
||||
if (mode === "passthrough" || testEnvAllowList.has(key)) {
|
||||
envFlags.push("-e", `${key}=${value}`);
|
||||
}
|
||||
}
|
||||
return envFlags;
|
||||
}
|
||||
|
||||
export function initializeNodeModulesVolume(ctx: VolumeInitContext): void {
|
||||
spawnSync(
|
||||
"docker",
|
||||
[
|
||||
"run",
|
||||
"--rm",
|
||||
"-v",
|
||||
`${ctx.volumeName}:/app/action/node_modules`,
|
||||
"node:24",
|
||||
"chown",
|
||||
"-R",
|
||||
`${ctx.uid}:${ctx.gid}`,
|
||||
"/app/action/node_modules",
|
||||
],
|
||||
{ stdio: "ignore", cwd: ctx.actionDir }
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* escape a string for embedding in a double-quoted shell context.
|
||||
* handles: backslash, double quote, dollar sign, backtick.
|
||||
*/
|
||||
function escapeForDoubleQuotes(str: string): string {
|
||||
return str.replace(/\\/g, "\\\\").replace(/"/g, '\\"').replace(/\$/g, "\\$").replace(/`/g, "\\`");
|
||||
}
|
||||
|
||||
export function buildDockerRunArgs(config: DockerRunArgsContext): string[] {
|
||||
const args: string[] = [
|
||||
"run",
|
||||
"--rm",
|
||||
"-t",
|
||||
"--privileged", // needed for PID namespace isolation (unshare --pid)
|
||||
"-v",
|
||||
`${config.ctx.actionDir}:/app/action:cached`,
|
||||
"-v",
|
||||
`${config.volumeName}:/app/action/node_modules`,
|
||||
"-w",
|
||||
"/app/action",
|
||||
];
|
||||
args.push(...config.envFlags);
|
||||
args.push(...config.sshSetup.sshFlags);
|
||||
|
||||
// escape nodeCmd for embedding in su -c "..." context
|
||||
const escapedNodeCmd = escapeForDoubleQuotes(config.nodeCmd);
|
||||
|
||||
// run as root initially, setup sudo for a test user, then run tests as that user
|
||||
// this simulates GHA environment where sudo is available
|
||||
const setupCmd = [
|
||||
// install sudo (node:24 is Debian-based) - check if already installed first
|
||||
`which sudo > /dev/null 2>&1 || (apt-get update -qq && apt-get install -qq -y sudo > /dev/null 2>&1)`,
|
||||
// remove any existing user/group with the same uid/gid (e.g. node:24 has "node" at 1000:1000)
|
||||
`existing_user=$(getent passwd ${config.ctx.uid} | cut -d: -f1) && [ -n "$existing_user" ] && [ "$existing_user" != "testuser" ] && userdel "$existing_user" 2>/dev/null || true`,
|
||||
`existing_group=$(getent group ${config.ctx.gid} | cut -d: -f1) && [ -n "$existing_group" ] && [ "$existing_group" != "testuser" ] && groupdel "$existing_group" 2>/dev/null || true`,
|
||||
// create user matching host uid/gid for file permissions
|
||||
`id testuser > /dev/null 2>&1 || (groupadd -g ${config.ctx.gid} testuser 2>/dev/null || true; useradd -u ${config.ctx.uid} -g ${config.ctx.gid} -m -s /bin/bash testuser 2>/dev/null || true)`,
|
||||
// configure passwordless sudo (like GHA runners) - check if already configured
|
||||
`grep -q "testuser ALL" /etc/sudoers 2>/dev/null || echo "testuser ALL=(ALL) NOPASSWD: ALL" >> /etc/sudoers`,
|
||||
// setup directories
|
||||
`mkdir -p /tmp/home/.config /tmp/home/.cache`,
|
||||
`chown -R ${config.ctx.uid}:${config.ctx.gid} /tmp/home /app/action/node_modules`,
|
||||
// install deps as user
|
||||
`su testuser -c "corepack pnpm install --frozen-lockfile --ignore-scripts"`,
|
||||
// run test as user - nodeCmd is escaped for double-quote context
|
||||
`su testuser -c "${escapedNodeCmd}"`,
|
||||
].join(" && ");
|
||||
args.push(
|
||||
"-e",
|
||||
"COREPACK_ENABLE_DOWNLOAD_PROMPT=0",
|
||||
"-e",
|
||||
"HOME=/tmp/home",
|
||||
"-e",
|
||||
"TMPDIR=/tmp",
|
||||
// always set CI=true in docker to enable sandbox - this is critical for security tests
|
||||
// without this, PID namespace isolation is skipped and tests may pass vacuously
|
||||
"-e",
|
||||
"CI=true",
|
||||
"node:24",
|
||||
"bash",
|
||||
"-c",
|
||||
`${config.sshSetup.sshSetupCmd}${setupCmd}`
|
||||
);
|
||||
return args;
|
||||
}
|
||||
|
||||
export type RunInDockerOptions = {
|
||||
actionDir: string;
|
||||
args: string[];
|
||||
nodeCmd: string;
|
||||
volumeName: string;
|
||||
envFilterMode: EnvFilterMode;
|
||||
onStart?: () => void;
|
||||
};
|
||||
|
||||
export function runInDocker(options: RunInDockerOptions): SpawnSyncReturns<Buffer> {
|
||||
const ctx = buildDockerRunContext({
|
||||
actionDir: options.actionDir,
|
||||
args: options.args,
|
||||
});
|
||||
assertDockerSupported(ctx);
|
||||
|
||||
const sshSetup = buildSshSetup(ctx);
|
||||
const envFlags = buildEnvFlags(ctx, options.envFilterMode);
|
||||
|
||||
initializeNodeModulesVolume({
|
||||
actionDir: ctx.actionDir,
|
||||
volumeName: options.volumeName,
|
||||
uid: ctx.uid,
|
||||
gid: ctx.gid,
|
||||
});
|
||||
|
||||
if (options.onStart) {
|
||||
options.onStart();
|
||||
}
|
||||
|
||||
return spawnSync(
|
||||
"docker",
|
||||
buildDockerRunArgs({
|
||||
ctx,
|
||||
envFlags,
|
||||
nodeCmd: options.nodeCmd,
|
||||
sshSetup,
|
||||
volumeName: options.volumeName,
|
||||
}),
|
||||
{ stdio: "inherit", cwd: ctx.actionDir }
|
||||
);
|
||||
}
|
||||
@@ -0,0 +1,76 @@
|
||||
// in-process fixture runner used by `play.ts` (and any future host-side
|
||||
// runner). does NOT know about Docker — that's `gha.ts`'s job. when run
|
||||
// inside the GHA container, this is what executes after the entrypoint.
|
||||
import { execSync } from "node:child_process";
|
||||
import { mkdtemp } from "node:fs/promises";
|
||||
import { devNull, tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import type { AgentResult } from "../agents/shared.ts";
|
||||
import { type Inputs, main } from "../main.ts";
|
||||
import { log } from "./cli.ts";
|
||||
import { ensureGitHubToken } from "./github.ts";
|
||||
import { setupTestRepo } from "./setup.ts";
|
||||
|
||||
export async function run(inputsOrPrompt: Inputs | string): Promise<AgentResult> {
|
||||
await ensureGitHubToken();
|
||||
|
||||
// play.ts is a CI-emulator — isolate it from the developer's user- and
|
||||
// system-scope gitconfig so checks like `validatePushDestination` see the
|
||||
// raw stored remote URL instead of values mutated by `url.*.insteadOf`
|
||||
// rewrites (a common SSH-auth convenience on dev boxes). CI runners have
|
||||
// empty gitconfigs so this is a no-op there; locally it makes `pnpm play`
|
||||
// and real runs produce identical git state. `os.devNull` canonicalizes
|
||||
// the null device across Unix (`/dev/null`) and Windows (`\\.\nul`).
|
||||
process.env.GIT_CONFIG_GLOBAL = devNull;
|
||||
process.env.GIT_CONFIG_SYSTEM = devNull;
|
||||
|
||||
const tempParent = await mkdtemp(join(tmpdir(), "pullfrog-play-"));
|
||||
const tempDir = join(tempParent, "repo");
|
||||
const originalCwd = process.cwd();
|
||||
|
||||
try {
|
||||
setupTestRepo({ tempDir });
|
||||
process.chdir(tempDir);
|
||||
|
||||
// optional pre-agent setup (e.g. seed symlinks for adversarial fixtures).
|
||||
if (process.env.PULLFROG_TEST_REPO_SETUP) {
|
||||
log.info("» running repo setup commands...");
|
||||
execSync(process.env.PULLFROG_TEST_REPO_SETUP, { cwd: tempDir, stdio: "pipe" });
|
||||
}
|
||||
|
||||
// tell main() to use the cloned tempDir instead of the GHA workspace path.
|
||||
process.env.GITHUB_WORKSPACE = tempDir;
|
||||
|
||||
const inputs: Inputs =
|
||||
typeof inputsOrPrompt === "string" ? { prompt: inputsOrPrompt } : inputsOrPrompt;
|
||||
|
||||
for (const [key, value] of Object.entries(inputs)) {
|
||||
if (value !== undefined && value !== null) {
|
||||
process.env[`INPUT_${key.toUpperCase()}`] = String(value);
|
||||
}
|
||||
}
|
||||
|
||||
const result: AgentResult = await main();
|
||||
process.chdir(originalCwd);
|
||||
|
||||
if (result.success) {
|
||||
log.success("Action completed successfully");
|
||||
return { success: true, output: result.output || undefined, error: undefined };
|
||||
}
|
||||
log.error(`Action failed: ${result.error || "Unknown error"}`);
|
||||
return { success: false, error: result.error || undefined, output: undefined };
|
||||
} catch (err) {
|
||||
const errorMessage = (err as Error).message;
|
||||
log.error(`Error: ${errorMessage}`);
|
||||
return { success: false, error: errorMessage, output: undefined };
|
||||
} finally {
|
||||
process.chdir(originalCwd);
|
||||
// sandbox isolation may create files with non-host ownership; rmSync
|
||||
// can't always delete those, so escalate.
|
||||
try {
|
||||
execSync(`sudo rm -rf "${tempParent}"`, { stdio: "ignore" });
|
||||
} catch {
|
||||
// best-effort cleanup.
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user