Skip to content

Harden Docker task sandbox isolation and resource limits - #109

Merged
mrubens merged 6 commits into
developfrom
codex/docker-task-sandbox-isolation
Jul 10, 2026
Merged

Harden Docker task sandbox isolation and resource limits#109
mrubens merged 6 commits into
developfrom
codex/docker-task-sandbox-isolation

Conversation

@mrubens

@mrubens mrubens commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • enforce configurable CPU, memory/swap, PID, writable-layer, and bounded log limits for Docker workers
  • create a dedicated network per task and attach only the API and preview proxy as trusted peers
  • enforce public-only or no-egress policies with private, cloud-metadata, sibling-network, and direct gateway protection
  • reap stale provisioning containers and task networks after controller restarts
  • document the new defaults and self-host configuration across Compose and Coolify deployments

Testing

  • RUN_DOCKER_SANDBOX_INTEGRATION_TESTS=true pnpm --filter @roomote/controller exec vitest run src/compute-providers/__tests__/docker-sandbox-isolation.integration.test.ts
  • pnpm exec dotenvx run -f .env.test -- pnpm --filter @roomote/controller exec vitest run
  • pnpm exec dotenvx run -f .env.test -- pnpm --filter @roomote/env exec vitest run
  • pnpm lint:fast
  • pnpm check-types:fast
  • pnpm knip

The Docker integration test verifies enforced HostConfig limits, trusted-service reachability, sibling sandbox/preview/credential denial, immutable egress routes, and crash-recovery cleanup.

@roomote-roomote

roomote-roomote Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

No new code issues found.

  • 🔴 preview-proxy is now a hard requirement, breaking optional-preview-proxy deploymentsapps/controller/src/compute-providers/docker-sandbox-security.ts:18,380-388
  • 🔴 DOCKER_WORKER_DISK_LIMIT defaults to 20g via --storage-opt size, which fails closed on common storage driverspackages/env/src/index.ts:78, apps/controller/src/compute-providers/docker-sandbox-security.ts:132
  • 🟡 Private-range blackholes can block host.docker.internal in local developmentapps/controller/src/compute-providers/docker-sandbox-security.ts:66-73,235-237
  • 🟢 Duplicated worker-run process detection logicapps/controller/src/compute-providers/docker-sandbox-security.ts:442-458

Sync review

Re-reviewed through 2806172. Earlier findings are resolved:

  1. Preview proxy optionalREQUIRED_TRUSTED_COMPOSE_SERVICES is API-only; preview-proxy attaches when present.
  2. Disk quota posture — unsupported --storage-opt size still fails closed by default, with explicit DOCKER_WORKER_ALLOW_UNBOUNDED_DISK opt-in, operator warning, and controller env wiring so the escape hatch is actually honored in production Compose.
  3. Local egress — private/gateway blackholes only when blockDockerGateway is set (networked self-host); local host-gateway path keeps RFC1918 reachable for host.docker.internal.
  4. Shared process matcherprocessListIncludesDockerWorkerRun is the single helper for startup and reaping.

Latest hardening also looks sound: best-effort reaper startup, per-network reconcile isolation, fail-open process-list reads, inspect-then-connect race handling, and idempotent egress ip route replace.

No new actionable code issues found on this pass.

@mrubens

mrubens commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Addressed all four review findings in 20ac9b03:

  • Preview proxy is optional again: only the API is required, while preview-proxy is attached when present.
  • Writable-layer limits now retry without --storage-opt size only when Docker reports that the storage driver cannot enforce the quota; other startup failures still fail closed. The fallback emits an operator warning, and the docs explain how to retain a hard host-level disk bound.
  • Local-development egress policy continues blocking metadata ranges but no longer blackholes RFC1918 routes, preserving host.docker.internal on Docker Desktop. Self-hosted networked deployments still block private ranges and the bridge gateway.
  • Worker process-list matching is consolidated into one shared helper used by both startup ownership checks and stale-resource cleanup.

Validation:

  • controller suite: 82 passed, 2 skipped
  • real-Docker isolation suite: 2 passed, including sibling/credential isolation, trusted API/preview reachability, crash cleanup, and live host.docker.internal connectivity
  • pnpm lint:fast
  • pnpm check-types:fast
  • pnpm knip
  • no leftover test containers or networks

roomote added 2 commits July 10, 2026 14:15
…sandbox-isolation

# Conflicts:
#	apps/controller/src/compute-providers/spawn-docker-worker.ts
@mrubens

mrubens commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the latest review findings in a705dfa9:

  • Docker disk quotas now fail closed by default. DOCKER_WORKER_ALLOW_UNBOUNDED_DISK=true is an explicit operator opt-in intended only when an equivalent host-level quota exists.
  • The controller now reconciles replacement API and preview-proxy containers onto every running task network during startup and periodic cleanup. The real-Docker suite replaces the API container and verifies existing workers reconnect without gaining sibling access.
  • Docker inspect/exec failures are no longer collapsed into “resource missing” during reaping, so transient daemon or socket-proxy failures cannot trigger destructive cleanup.

Validation:

  • full controller suite: 88 passed, 2 skipped
  • environment suite: 57 passed
  • real-Docker isolation suite: 2 passed
  • production Compose security contract
  • pnpm lint:fast
  • pnpm check-types:fast
  • pnpm knip
  • no leftover test containers or networks

roomote added 2 commits July 10, 2026 14:39
…sandbox-isolation

# Conflicts:
#	deploy/coolify/docker-compose.yaml
…he sandbox reaper

The new DOCKER_WORKER_* limit and egress variables were only added to the
web env anchor in the production Compose file, so the controller (their
sole consumer) always fell back to built-in defaults. That also made the
documented DOCKER_WORKER_ALLOW_UNBOUNDED_DISK escape hatch a no-op on
hosts whose storage driver cannot enforce --storage-opt size.

Also hardens sandbox reconciliation:
- controller startup no longer crashes when the initial sandbox sweep
  fails transiently (for example mid-rolling-deploy)
- one unreconcilable task network no longer aborts the rest of the sweep
- a running worker is never reaped when its process list cannot be read
  or its task-run label is unreadable; the next cycle retries
- losing the inspect-then-connect race for a trusted service is treated
  as success instead of failing the spawn
- egress blackhole routes use `ip route replace` so the helper script is
  idempotent
@mrubens
mrubens merged commit 6fe804f into develop Jul 10, 2026
1 check passed
@mrubens
mrubens deleted the codex/docker-task-sandbox-isolation branch July 10, 2026 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants