CodeIssuesPull RequestsActionsSecurityInsights
✨ AI
More
Settings

feat(ci): execute workflow steps in a sibling container, not the app #5595

MergedXSccantynz wants to mergefeat/ci-sidecar-runnermainopened 23h ago
ccantynzcommented 23h ago

A workflow run: step has been a Bun.spawn inside the app container. Three problems follow from that one fact, and none is a bug that can be fixed in place:

  • The app's memory limit bounds every build. A 2 GiB app cap SIGKILLed tsc on 2026-08-29 and took CI down platform-wide. The fix was raising the app's ceiling to 6g — which the compose file itself records as a stopgap.
  • A step can read the platform's secrets. It shares a PID namespace with PID 1, whose environment is the whole .env. buildRunnerEnv withholds those variables from the step's own env; /proc/1/environ hands them straight back.
  • A step can read and rewrite every repository on the instance. /data/repos is mounted read-write.

What this does

A sibling runner container: same image, different entrypoint, deliberately impoverished — no env_file, no repo mount, no database import. The app clones into a shared ci-work volume (it has the repositories and the credentials) and the runner only ever sees that directory.

The guarantee is structural rather than a denylist: the runner cannot leak DATABASE_URL because its process never had it, and cannot reach a repository because no such path is mounted.

Off by default

RUNNER_URL and RUNNER_TOKEN are both empty. A URL without a token counts as no runner, so a half-filled .env falls back to the working in-process path instead of routing every build on the platform to a container that answers 401. The runner comes up alongside and can be verified before anything routes to it.

What this does NOT close

Said plainly, because an isolation story that overstates itself gets trusted:

  • Jobs are not isolated from each other. One long-lived runner means two runs share a filesystem and a process namespace. workflow-exec-policy.ts therefore stays, with its header rewritten to say exactly why and what must be true before it goes.
  • Egress/exfiltration, kernel escape, and disk quota on ci-work are untouched.

This is what makes CI safe to offer the first customer. Per-job ephemeral containers are what make it safe to offer the tenth.

The trap worth naming

requeueRunsOrphanedByRestart assumed the executing process dies when the app does. With a sibling runner it does not — a redeploy would leave a step running while the sweep marked the run runner_restarted and enqueued a second attempt: two executions racing over one checkout. Both sweeps now ask the runner what it is actually executing. An unreachable runner reports nothing live, which is today's behaviour and the correct failure direction.

Tests

They assert absences, which is unusual and deliberate: the value here is entirely in what the runner does not have, and each property dies to one well-meaning line.

Mutation-verified — adding env_file, a ports: mapping, a git-repos mount, or an import { db } each fails a specific test. The first draft passed against its own subject because it was reading the comments that explain the rules; it strips comments now.

Verification before enabling

Design and rollout steps: docs/CI-ISOLATION-NEXT-INCREMENT.md. The acceptance test is run inside a live step, not against the compose file:

tr '\0' '\n' < /proc/1/environ   # no DATABASE_URL
ls /data/repos                    # No such file or directory
ccantynzAI Reviewcommented 23h ago

AI Triage

(no summary)

Priority: medium Risk area: mixed

Suggested labels: (no label suggestions) Suggested reviewers: (no reviewer suggestions)

Suggestions only — nothing has been applied. The PR author stays in control.

gluecron[bot]🤖 botAI Reviewcommented 23h ago

AI review unavailable

The platform's AI balance is exhausted, so AI generation is temporarily unavailable. Nothing was lost. You can queue this as a repair for the internal agent from the repository's Health page, or try again once the balance is restored. The PR is otherwise unchanged.

Cross-repo impact

See what breaks downstream if this PR merges.

Analyze →
⮌ Merged

This pull request was merged into main.

c comment · e edit title · m merge · a approve · r request changes · ? shortcuts