feat(agent): enforce per-sandbox cgroup v2 memory limits - #3142
feat(agent): enforce per-sandbox cgroup v2 memory limits#3142daryllimyt wants to merge 15 commits into
Conversation
Ten concurrent agent sandboxes on one executor pod crossed the pod memory limit and the kernel group-killed the container, failing all in-flight sessions at once. rlimit_as bounds neither the sandbox process tree nor tmpfs, so aggregate memory was unbounded. - emit use_cgroupv2/cgroup_mem_max nsjail config (fields verified against the pinned nsjail commit) so each sandbox tree gets its own kernel-enforced memory.max and a runaway session fails alone - prepare the cgroup root at worker startup (move PIDs to a leaf, enable the memory controller, verify with a probe cgroup); any failure logs one warning and falls back to no cgroup limits - clamp max_concurrent_activities to the container memory budget (limit - reserve) / per-sandbox cap instead of overcommitting - write a best-effort readiness sentinel for a future exec probe
daryllimyt
added
enhancement
|
✅ No security or compliance issues detected. Reviewed everything up to 594c9c5. Security Overview
Detected Code Changes
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94fa42c242
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A SIGKILLed worker cannot run its cleanup, and the sentinel may live on a filesystem that survives container restarts, so a file-existence readiness probe would report the restarted worker ready during startup. Remove any existing sentinel before startup work begins and remove it unconditionally on exit.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 842e26f357
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Address review blockers on the per-sandbox cgroup limits: - Derive the cgroup v2 root from /proc/self/cgroup instead of assuming /sys/fs/cgroup is the delegated root, so privileged host-namespace containers (Kubernetes/containerd) never prepare children in the node hierarchy. Preparation returns a typed PreparedCgroup; the root is retained on preparation failure so budget validation still reads the container's memory.max. - Fail fast at worker startup when reserve + one sandbox exceeds the container memory limit instead of forcing concurrency to one. - Apply the prepared cgroup to stdio MCP probe sandboxes and cap probe memory at the per-slot sandbox limit. - Validate sandbox memory (>=1), executor reserve (>=0), and worker concurrency (>=1) at the configuration boundary. - Expose the sandbox memory env vars in compose files and add an agent-executor healthcheck on the readiness sentinel.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72679fb114
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Interpolate TRACECAT__AGENT_EXECUTOR_READY_FILE into both the agent-executor environment and the healthcheck test so an operator override keeps the worker and healthcheck pointed at the same sentinel path.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a037f5ce0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Move the stale-sentinel cleanup ahead of concurrency validation and the memory-budget check so a fail-fast crashloop cannot leave a SIGKILLed predecessor's sentinel visible to healthchecks.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed5ce8b2b4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Start the agent-executor entrypoint as root in the sandbox Compose overlay, remount /sys/fs/cgroup read-write, chown the cgroup v2 delegation files to apiuser, and drop privileges via setpriv before the worker starts. Without delegation the root-owned cgroupfs made preparation fail with EACCES as UID 1001, silently disabling per-sandbox memory limits in the only compose deployment that enables nsjail.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ffbe671d1b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
setpriv changes only IDs, so the delegation path left HOME=/root for the UID 1001 worker, breaking HOME-dependent tooling (npx MCP probes, SSH helpers). Set HOME/USER/LOGNAME explicitly instead of --reset-env, which would clear the service configuration environment.
Main removed the shared entrypoint.sh migration wrapper (#3147), which carried this branch's cgroup delegation logic. Reconcile by moving delegation into docker/scripts/agent-executor-entrypoint.sh, baked into both image stages and referenced only by the sandbox overlay's entrypoint override. The script now derives the container's own cgroup v2 directory from /proc/self/cgroup, so it delegates the correct subtree under both private (compose) and host (privileged Kubernetes) cgroup namespaces and never touches anything above it; environments without the needed privileges degrade to rlimit-only limits unchanged.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 080c889b35
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Require a unified cgroup v2 entry and an existing cgroup.controllers file before the entrypoint delegates ownership, so cgroup v1 hosts never get their cgroupfs root chowned, and drop the basename-based main guard in detect_cgroup_root: a runtime-assigned cgroup legitimately named main must be treated as the container boundary, not escaped to its parent. The cached pre-move root remains authoritative for budget validation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f544e7e227
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Skip the root-side remount/chown when TRACECAT__AGENT_SANDBOX_CGROUP_ENABLED is falsy (mirroring env_bool), matching the Python-side preparation skip. The privilege drop stays unconditional so a root-started container never reaches the worker as root regardless of the flag.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17c69a92fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Walk every visible hierarchy level from the executor's cgroup up to the cgroupfs mount and take the minimum finite memory.max, so task-scoped limits (e.g. an ECS task limit with no container-level limit) are not misread as unlimited when the leaf reports max. PreparedCgroup carries the cgroupfs boundary so the walk never leaves the mount.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab6c868ce9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7276d2fb67
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb99c561ef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 594c9c5a4f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1 participant
Why
A production agent-executor pod was kernel-OOM-killed (exit 137) when ten concurrent agent sandboxes plus the worker process crossed the pod's 16Gi memory limit. Because the container cgroup is the only memory boundary, the kernel group-killed the entire container — all ten in-flight agent sessions failed simultaneously and, with
maximum_attempts=1, every parent workflow failed with them.Today the only per-sandbox control is
rlimit_as, which is per-process virtual address space: it bounds neither the sandbox process tree in aggregate nor tmpfs, so nothing stops total usage short of the pod limit.What
use_cgroupv2/cgroup_mem_max(=TRACECAT__AGENT_SANDBOX_MEMORY_MB) /cgroup_mem_swap_max: 0, so each sandbox tree gets its own kernel-enforcedmemory.maxand a runaway session OOMs alone instead of taking down the worker and its neighbors. Field names verified against the nsjail commit pinned in the Dockerfile. Existingrlimit_*lines are kept as defense in depth.tracecat/agent/sandbox/cgroup.py): moves worker PIDs into a leaf cgroup, enables thememorycontroller on the namespace root (the cgroup v2 no-internal-process rule), and verifies with a throwaway probe cgroup. Any failure — including macOS/dev and non-privileged environments — logs one warning and falls back to running without cgroup limits; sandbox launches are never blocked by cgroup setup.max_concurrent_activitiesis clamped to(container memory.max − TRACECAT__AGENT_EXECUTOR_MEMORY_RESERVE_MB) / TRACECAT__AGENT_SANDBOX_MEMORY_MB, with a loud error log when the configured value exceeds the budget. Clamping (not crashing) is deliberate so a config mistake cannot crash-loop the fleet.New env vars:
TRACECAT__AGENT_SANDBOX_CGROUP_ENABLED(defaulttrue, graceful fallback),TRACECAT__AGENT_EXECUTOR_MEMORY_RESERVE_MB(default4096),TRACECAT__AGENT_EXECUTOR_READY_FILE.Testing
memory.max), probe failure matrix on fake cgroup trees (permission errors, vanishing PIDs), readiness sentinel lifecycle with a mocked worker.ruff check,ruff format,basedpyright --warningsclean on all changed files including tests.Summary by cubic
Adds per-sandbox cgroup v2 memory limits for agent sandboxes and a container-aware concurrency budget so a runaway session OOMs alone. Includes a Compose sandbox overlay that delegates cgroups only when nsjail is enabled, plus a readiness sentinel and healthchecks.
New Features
/proc/self/cgroup; setuse_cgroupv2,cgroupv2_mount,cgroup_mem_max(fromTRACECAT__AGENT_SANDBOX_MEMORY_MB), andcgroup_mem_swap_max: 0in nsjail when available.mainleaf, enable the memory controller, verify with a probe cgroup; on failure, log once and continue without per-sandbox limits (root kept for budget checks).cgroup.controllers; keep the pre-move root authoritative; treat ancestor limits as boundaries; gate root-side delegation onTRACECAT__AGENT_SANDBOX_CGROUP_ENABLED=trueandTRACECAT__DISABLE_NSJAIL=false.max_concurrent_activitiesto (min finitememory.maxacross the cgroup and its ancestors −TRACECAT__AGENT_EXECUTOR_MEMORY_RESERVE_MB) /TRACECAT__AGENT_SANDBOX_MEMORY_MB; if no slot fits, fail fast.min(1024, TRACECAT__AGENT_SANDBOX_MEMORY_MB). Action sandboxes are unchanged.TRACECAT__AGENT_EXECUTOR_READY_FILE./sys/fs/cgrouprw, delegates the container’s cgroup v2 subtree toapiuseronly when nsjail is enabled, then drops privileges.Migration
TRACECAT__AGENT_EXECUTOR_MEMORY_RESERVE_MB/TRACECAT__AGENT_SANDBOX_MEMORY_MB.TRACECAT__DISABLE_NSJAIL=falseand layerdocker-compose.sandbox.yml; it configures privileges and cgroup delegation.Written for commit 594c9c5. Summary will update on new commits.