Skip to content

feat: run CPU isolation and the memory instrument inside the macro-agent sandbox - #456

Merged
GuillaumeLagrange merged 6 commits into
mainfrom
macro-agent-sandbox
Jul 28, 2026
Merged

feat: run CPU isolation and the memory instrument inside the macro-agent sandbox#456
GuillaumeLagrange merged 6 commits into
mainfrom
macro-agent-sandbox

Conversation

@GuillaumeLagrange

@GuillaumeLagrange GuillaumeLagrange commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Runner-side changes to make CPU isolation and the memory instrument work
unprivileged inside the macro-agent sandbox. Two issues, combined here because
they share the sandbox's privilege model and the same branch.

CPU isolation (COD-3012). Drop the runner's built-in systemd-run /
CGROUP: isolation for a hook-based one: an Isolation type invokes
codspeed-{pre,wrap,post}-bench and stays oblivious to how cores are attributed.
If codspeed-wrap-bench exists it takes the (unprivileged) hook path; otherwise
it falls back to systemd-run --scope --slice=codspeed.slice, so non-sandbox
hosts are unchanged.

Memory instrument (COD-3047). Let memtrack run off a delegated BPF token
instead of root: attach via bpf()-native links (uprobe_multi + tp_btf),
resolve tracked PIDs in the tracker's PID namespace, and accept
LIBBPF_BPF_TOKEN_PATH as a privilege source.

Closes COD-3012
Closes COD-3047

@codspeed-hq

codspeed-hq Bot commented Jul 15, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

✅ 17 untouched benchmarks


Comparing macro-agent-sandbox (d81db8d) with main (aebdbbc)

Open in CodSpeed

@greptile-apps

greptile-apps Bot commented Jul 15, 2026

Copy link
Copy Markdown

Greptile Summary

This PR makes two sandboxed-runner features work without host privileges: CPU isolation now uses hook scripts (codspeed-{pre,wrap,post}-bench) instead of systemd-run, and memtrack now loads its eBPF programs via a delegated BPF token instead of requiring root or file capabilities.

  • CPU isolation (Isolation enum): The old HookScriptsGuard + hardcoded systemd-run path is replaced by a tri-state Isolation::resolve() that prefers machine-installed hooks (no sudo needed; benchmark stays a descendant of the profiler) and falls back to Systemd for gen-1 images.
  • eBPF token path (BpfVariant::Token): Two BPF skeletons are compiled from the same source. The token variant uses uprobe_multi links and tp_btf/sched_process_fork (all delegatable via BPF token), while the legacy variant keeps perf_event_open-based uprobes. A new variant.h provides namespace-aware PID helpers so probes resolve PIDs in the tracker's own PID namespace.
  • Privilege-free proc_fs attach: candidate_paths() falls back from map_files/ (needs CAP_CHECKPOINT_RESTORE) to the pathname column so libraries are discoverable inside the sandbox.

Confidence Score: 5/5

Safe to merge; the isolation refactor preserves the systemd fallback and the dual-skeleton approach is mechanically correct across all probe sites.

The dual-skeleton dispatch, namespace-aware PID helpers, and proc_fs fallback path are all consistently implemented and well-tested. The previously flagged UPROBE_ARGS_RET namespace bug is fixed here. The only finding is a single eprintln! in a test helper.

Files Needing Attention: No files require special attention.

Important Files Changed

Filename Overview
src/executor/wall_time/isolation.rs Tri-state Isolation enum replaces HookScriptsGuard; Hooks path runs pre/post-bench hooks via Drop and wraps the benchmark leaf with codspeed-wrap-bench, Systemd path falls back to systemd-run.
crates/memtrack/src/ebpf/c/utils/variant.h New header providing UPROBE_SEC/URETPROBE_SEC and namespace-aware PID helpers; task_ns_tgid level<4 bound is correct for BPF verifier.
crates/memtrack/src/ebpf/c/utils/process_tracking.h sched_process_fork migrated from classic tracepoint to tp_btf for token delegation; follow_fork uses task_ns_tgid correctly.
crates/memtrack/src/ebpf/c/allocator.h All probe SEC() annotations parameterised; UPROBE_ARGS_RET and all other probes now use current_task_ids().tgid instead of global PID.
crates/memtrack/src/ebpf/memtrack/mod.rs Dual-skeleton Skel enum with with_skel! dispatch; open_and_load! sets pidns rodata before load; variant selected by has_delegated_bpf_token().
crates/memtrack/src/bpf_token.rs Shared has_delegated_bpf_token() helper (is_dir check included) now used by both the eBPF loader and the memory executor.
crates/memtrack/src/ebpf/proc_fs.rs candidate_paths() falls back from map_files to pathname column for privilege-free library discovery.
crates/memtrack/tests/shared.rs for_each_variant cross-checks BPF variants; one eprintln! violates the project tracing-macro rule.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Isolation::resolve] --> B{CODSPEED_ISOLATION=false?}
    B -- yes --> C[Isolation::None]
    B -- no --> D{codspeed-wrap-bench executable?}
    D -- yes --> E[run_pre_bench hook]
    E --> F[Isolation::Hooks]
    D -- no --> G{forced=true OR passwordless sudo?}
    G -- yes --> H[Isolation::Systemd]
    G -- no --> I[Isolation::None - log warning]
    F --> J[wrap_bench: prepend wrap-bench]
    H --> K[wrap_bench: systemd-run scope]
    C --> L[wrap_bench: passthrough]
    J --> M[benchmark child of profiler]
    K --> N[benchmark reparented - profiler needs sudo]
    F -.->|Drop| P[run post-bench hook]
Loading

Reviews (6): Last reviewed commit: "chore: make tests run release builds of ..." | Re-trigger Greptile

Comment thread crates/memtrack/src/ebpf/memtrack.rs Outdated
@GuillaumeLagrange GuillaumeLagrange changed the title [COD-3012] feat: run CPU isolation and the memory instrument inside the macro-agent sandbox feat: run CPU isolation and the memory instrument inside the macro-agent sandbox Jul 15, 2026
@GuillaumeLagrange
GuillaumeLagrange requested review from not-matthias and removed request for not-matthias July 27, 2026 13:55
@GuillaumeLagrange
GuillaumeLagrange force-pushed the macro-agent-sandbox branch 3 times, most recently from cd83132 to 1142235 Compare July 27, 2026 15:29
@GuillaumeLagrange
GuillaumeLagrange changed the base branch from main to cod-2796-switch-to-samply-profiler-by-default July 27, 2026 15:30
@GuillaumeLagrange
GuillaumeLagrange force-pushed the cod-2796-switch-to-samply-profiler-by-default branch from 228b86f to 266f658 Compare July 27, 2026 16:20
@GuillaumeLagrange

Copy link
Copy Markdown
Contributor Author

@greptileai review

Base automatically changed from cod-2796-switch-to-samply-profiler-by-default to main July 27, 2026 16:55

@not-matthias not-matthias left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, already pretty good. I think we can improve the naming a bit and let's check if we can keep the tests unified.

Lmk if you have some other thoughts on some comments.

Comment thread crates/memtrack/src/ebpf/memtrack/mod.rs Outdated
Comment thread crates/memtrack/src/ebpf/memtrack/mod.rs Outdated
Comment thread crates/memtrack/src/ebpf/memtrack/mod.rs Outdated
Comment thread crates/memtrack/src/ebpf/memtrack/mod.rs Outdated
Comment thread crates/memtrack/src/ebpf/c/utils/flavor.h Outdated
Comment thread crates/memtrack/src/ebpf/c/memtrack_perf.bpf.c Outdated
Comment thread crates/memtrack/tests/flavor_equivalence_tests.rs Outdated
Comment thread crates/memtrack/src/ebpf/c/utils/process_tracking.h Outdated
Comment thread crates/memtrack/src/ebpf/c/attach.h Outdated
Comment thread crates/memtrack/src/ebpf/c/utils/flavor.h Outdated
@GuillaumeLagrange
GuillaumeLagrange force-pushed the macro-agent-sandbox branch 5 times, most recently from f83af31 to 2ee406c Compare July 28, 2026 09:37

@not-matthias not-matthias left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Comment thread crates/memtrack/src/ebpf/c/utils/event_helpers.h Outdated
Comment thread crates/memtrack/src/ebpf/memtrack/maps.rs Outdated
Comment thread crates/memtrack/src/ebpf/memtrack/maps.rs Outdated
Comment thread crates/memtrack/src/ebpf/attach_worker.rs
@GuillaumeLagrange
GuillaumeLagrange force-pushed the macro-agent-sandbox branch 3 times, most recently from 9069db4 to 81c200a Compare July 28, 2026 12:48
Replace the runner's built-in CPU-isolation mechanisms with a single,
machine-driven one. The runner previously hard-coded `systemd-run --scope` and a
`CODSPEED_ISOLATION=CGROUP:<dir>` mode with per-spawn cgroup-dir plumbing, split
across a `HookScriptsGuard` (which ran the pre/post-bench hooks) and
`isolation.rs` (which did the wrapping).

Now a single `Isolation` type owns the whole lifecycle: `resolve()` runs the
pre-bench hook, `wrap_bench()` pins the benchmark leaf, and `Drop` runs
post-bench. Cpuset logic lives on the machine behind three hooks
(`codspeed-{pre,wrap,post}-bench`); the runner only invokes them and is otherwise
oblivious to how cores are attributed. Discovery is by hook presence:

- an executable `codspeed-wrap-bench` selects the hook path — unprivileged, and
  the benchmark stays a descendant of the profiler so it records without sudo;
- its absence falls back to `systemd-run --scope --slice=codspeed.slice`, so
  hosts without the hook keep working unchanged.

The pre-bench hook is invoked with the runner's PID so the machine places the
runner (and the profiler it spawns) onto the system cores; the runner makes no
cgroup writes of its own. The profiler's `wrap_command` flag is renamed
`isolate` -> `requires_sudo`, now true only for the systemd fallback.

Refs COD-3012
A BPF token only relaxes the capability checks made by the bpf() syscall
itself, so uprobes attached with perf_event_open() can never be
delegated: that path checks CAP_SYS_ADMIN/CAP_PERFMON against the init
user namespace. uprobe_multi links go entirely through bpf(), where the
token applies, but require kernel >= 6.6.

Compile the BPF source into two skeletons instead of one, through thin
wrappers that differ only in MEMTRACK_BPF_VARIANT_TOKEN: the token
variant attaches uprobe_multi links, the legacy variant keeps
perf_event_open for older kernels. MemtrackBpf::new picks a variant from
token availability, and with_variant pins one explicitly.

The programs also resolve PIDs in the tracker's PID namespace rather than
always the init one. eBPF observes init-namespace PIDs, so a tracker
running inside a namespace registered namespace-local PIDs the probes
would never match.

The memory executor now treats a token as sufficient privilege, and so
skips the sudo capability grant when one is present.

Refs COD-3047
The two variants are built from the same BPF source and should observe
the same allocations, since they differ only in how uprobes attach. Run
each test case under both and compare what they saw, so a divergence
fails the test that exercises it rather than a separate equivalence test.

Comparison is on the count of events per kind-and-size: addresses,
timestamps, pids and event order all differ legitimately between two runs
of the same workload. A variant that cannot attach on the host is skipped
rather than failing, since the token variant needs uprobe_multi
(kernel >= 6.6); the run fails if neither attaches.

Test workloads are now passed as closures returning a Command, as each
variant needs a fresh process.

Refs COD-3047
Allocator classification opened the mapping only through
/proc/<pid>/map_files/<range>. Opening that requires CAP_CHECKPOINT_RESTORE or
CAP_SYS_ADMIN in the *init* user namespace: proc_map_files_get_link calls
checkpoint_restore_ns_capable(&init_user_ns). Outside a sandbox memtrack passes
that check because the runner setcaps CAP_SYS_ADMIN onto the binary
(MEMTRACK_REQUIRED_CAPS), or it runs as root.

A tracker in a nested user namespace cannot pass it at all. File capabilities
are not honoured there, and a capability held in the nested namespace does not
satisfy a check made against the init one. So every mapping got EPERM, even libc
classified as "not an allocator", and no uprobe was attached. The run still
exited 0 and wrote a valid empty artifact, reporting no allocations.

Fall back to the maps pathname column, which needs no privilege beyond read
access. map_files stays preferred: it references the mapped inode, so it holds
when the path has been replaced or unlinked, whereas a reused name could resolve
to different contents. A `[deleted]` or non-absolute pathname (`[heap]`,
`[vdso]`) names no openable file and is skipped.

Classification reads the whole file, so a failed open and a readable
non-allocator were indistinguishable; openability is now probed separately.
A mapping no candidate path can reach warns instead of staying silent, but does
not abort: losing coverage of one unreachable library beats killing the run,
which is what a fatal error here would do via record_fatal's SIGKILL.

Refs COD-3047
@GuillaumeLagrange
GuillaumeLagrange merged commit d81db8d into main Jul 28, 2026
23 checks passed
@GuillaumeLagrange
GuillaumeLagrange deleted the macro-agent-sandbox branch July 28, 2026 13:58
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