fix(supervisor): fail fast with diagnostic error when CAP_SETPCAP is unavailable

[waynesun09]

Summary

When the supervisor lacks CAP_SETPCAP in its effective set (for example, a container runtime that does not grant it), the child's prctl(PR_CAPBSET_DROP) returns EPERM during privilege drop. The clear runs inside pre_exec, which cannot emit structured logs, so the failure surfaced only as an opaque child exit with code 1.

Following the review discussion, this PR keeps the bounding-set clear fail-closed — no tolerance path, no degraded mode — and instead makes the failure diagnosable:

• A parent-side preflight probes PR_CAPBSET_DROP availability before fork(). When the clear would fail with a non-empty bounding set, it emits an OCSF DetectionFinding alert and refuses the spawn with an actionable error, instead of letting the child die inside pre_exec where structured logging is unavailable.
• The child-side EPERM error in validate_capability_bounding_set_clear() now carries the same CAP_SETPCAP diagnosis and remediation hint, as a backstop for environments the parent probe cannot predict.

The version-skew trigger that originally surfaced this (older gateway pulling a newer :latest supervisor image) was fixed separately by #2070, which closed #2068. This PR addresses the remaining gap from #2069: a supervisor that cannot clear the bounding set should fail fast with a clear diagnosis rather than an undiagnosable crash.

Related Issue

Related to #2069

Changes

• crates/openshell-supervisor-process/src/process.rs:
  • Add check_capability_bounding_set_readiness(), called before fork() under uses_privileged_process_setup(). It probes with a non-destructive bounding::drop() on a capability already absent from the bounding set (the same prctl(PR_CAPBSET_DROP) syscall bounding::clear() uses), and on EPERM with a non-empty set emits a High-severity OCSF DetectionFinding and fails the spawn with a diagnostic error.
  • Extend the EPERM + non-empty arm of validate_capability_bounding_set_clear() to return a diagnostic error naming CAP_SETPCAP and the remediation, keeping the existing fail-closed behavior.
  • Add capability_bounding_set_readiness_matches_clear_availability test; strengthen capability_bounding_set_clear_rejects_nonempty_eperm to assert the CAP_SETPCAP diagnosis.
• architecture/sandbox.md: Keep the fail-closed invariant text and document the parent-side preflight (diagnostic error + OCSF finding when CAP_SETPCAP is unavailable).

Compared to the previous revision of this PR: the EPERM-tolerance branch, the degraded-mode documentation, and the rootless-caps CI job are all dropped per review feedback.

Testing

• cargo test -p openshell-supervisor-process --lib -- capability_bounding drop_privileges — 15 passed (rust:1.95 Linux container)
• Same suite in a container with --cap-drop SETPCAP (root, non-empty bounding set, denied PR_CAPBSET_DROP) — 15 passed; drop_privileges_succeeds_for_current_group exercises the fail-closed error path and the new readiness test hits its error branch with the CAP_SETPCAP diagnosis
• cargo clippy -p openshell-supervisor-process --all-targets -- -D warnings clean
• cargo fmt --check clean

Checklist

• Follows Conventional Commits
• Commits are signed off (DCO)
• Architecture docs updated

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[github-actions]

All contributors have signed the DCO ✍️ ✅
_{Posted by the DCO Assistant Lite bot.}

[waynesun09]

I have read the DCO document and I hereby sign the DCO.

[waynesun09]

recheck

[maxamillion]

Few points of note:

• Security boundary behavior changed but architecture docs were not updated.
  architecture/sandbox.md still says child capability bounding-set clearing is fail-closed and that EPERM is tolerated only when the set is already empty. This PR intentionally changes that invariant. The architecture doc should be updated in the same PR to describe the degraded rootless mode and its reliance on seccomp/Landlock.

• The degraded path relies on Landlock, but Landlock may be best-effort.
  The warning says the child relies on “seccomp and Landlock,” but elsewhere Landlock can run in best-effort mode and continue unavailable or failed. If the bounding set remains non-empty and Landlock is unavailable/best-effort, the actual confinement story is weaker than the warning implies. Consider tightening the message or adding an explicit check/comment explaining acceptable residual risk.

• Consider emitting this as an OCSF security/config event, not only tracing::warn!.
  Per project logging guidance, degraded sandbox controls and unavailable confinement primitives are operator-visible security posture events. This warning represents a confinement degradation and may warrant a structured OCSF finding or config-state event so it appears in sandbox security telemetry.

• Commit metadata includes Assisted-by: Claude.
  Project instructions say commits must not mention Claude or AI agents. The commit bodies in this PR include Assisted-by: Claude; those should be removed before merge.

[TaylorMutch]

/ok to test 1dc253d

[elezar]

@alangou could this have been introduced in #2001? Do you mind having a look?

[alangou]

/ok to test 3f95d51

[alangou]

The new OCSF degraded-mode alert may not fire.

The parent-side probe returns early if CAP_SETPCAP is present in the effective set, but the commit message says Podman can grant CAP_SETPCAP while AppArmor still makes bounding::clear() fail with EPERM. Then we skip the parent DetectionFinding, and only hit the warn! inside pre_exec, which is exactly the context this patch says cannot reliably emit structured logs.

Could we make the readiness probe test the actual bounding-set clear behavior ?

[alangou]

/ok to test b3a0e2a

[alangou]

@waynesun09 look to me. Just fix the format issue and we're good to merge

[waynesun09]

@alangou cool, I'm on it now.

[waynesun09]

@alangou it's updated, please check, thanks

[alangou]

/ok to test 9ab2fcb

[waynesun09]

@alangou the new ci clippy failure on backticked AppArmor is fixed, sorry I missed that in the local test

[elezar]

This test does not align with how we would ideally be testing this feature. We should add an e2e test suite that uses rootless podman, and not just run the unit tests as a regular user.

[waynesun09]

Ok, I'm on checking the current e2e test which is with nested container, with the out layer started with --privilege and it might have the AppArmor disabled and could not cover the bug, I'm still testing on it, will let you know the findings.

If we could add the e2e test, do you want me to keep current drop privilege test or just keep it?

[elezar]

I started #2119 to move the rootless tests out of the container, but I have not yet been able to reproduce the failure you're seeing.

Feel free to comment on the PR if you something obvious sticks out.

@alangou don't take my comments as blocking. If this fixes the regression, we can merge and then add better testing ... Although being able to reproduce the behaviour would have been a win.

[waynesun09]

This is triggered by the supervisor image version pinning bug (#2068). The Podman driver defaults supervisor_image to :latest, so an older gateway (pre-#2001, SETPCAP in cap_drop) pulls a newer supervisor (v0.0.73 with bounding::clear()). The gateway's container spec never grants CAP_SETPCAP, but the supervisor now needs it — prctl(PR_CAPBST_DROP) returns EPERM and the supervisor crashes.

I verified locally via podman inspect on a gateway-created container: SETPCAP was in CapDrop, confirming the pre-#2001 container spec.

With a matched gateway+supervisor both post-#2001 (SETPCAP in cap_add), the prctl succeeds because drop_capability_bounding_set() runs before setuid() while CapEff is still full. I need to verify that locally first — will update here once confirmed.

To reproduce: use a pre-#2001 gateway binary with the latest supervisor image (the :latest default makes this happen naturally until #2068 is fixed).

Either way the fix is still valid as defensive code: the supervisor shouldn't crash when CAP_SETPCAP is absent regardless of the reason (version skew, custom container specs, non-podman runtimes).

[waynesun09]

Confirmed locally: matched v0.0.73 gateway + supervisor does not crash.

$ openshell status
  Version: 0.0.73

$ openshell sandbox create --no-keep -- echo "ok"
Created sandbox: lawful-soldierfish
ok
✓ Deleted sandbox lawful-soldierfish

$ openshell sandbox create --name verify-caps -- bash -c "cat /proc/self/status | grep -i cap; id"
CapBnd: 0000000000000000
uid=998(sandbox) gid=998(sandbox)

Bounding set cleared successfully — drop_capability_bounding_set() runs before setuid() while CapEff still has CAP_SETPCAP, so prctl(PR_CAPBSET_DROP) succeeds.

The crash only reproduces under version skew: pre-#2001 gateway (SETPCAP in cap_drop) + post-#2001 supervisor. The :latest pinning bug (#2068) creates this naturally when the registry publishes a new supervisor ahead of a gateway upgrade.

The fix remains valid as defensive hardening — the supervisor shouldn't crash when CAP_SETPCAP is absent regardless of the cause.

[waynesun09]

Reproduced locally with v0.0.72 gateway + v0.0.73 supervisor:

$ /tmp/openshell-072/openshell status
  Version: 0.0.72

$ /tmp/openshell-072/openshell sandbox create --no-keep -- echo "ok"
Error: sandbox is not ready
Container exited with code 1

$ podman inspect openshell-sandbox-repro-crash --format '{{json .HostConfig.CapDrop}}'
["CAP_DAC_OVERRIDE","CAP_FSETID","CAP_KILL","CAP_NET_BIND_SERVICE","CAP_SETFCAP","CAP_SETPCAP","CAP_SYS_CHROOT"]

v0.0.72 gateway has CAP_SETPCAP in cap_drop. v0.0.73 gateway removed it (via #2001). The version skew is the trigger.

[waynesun09]

Rebased on main (includes #2119). Dropped the old rootless-caps CI job initially thinking #2119's e2e covered it, then re-added after analysis.

Why e2e-podman-rootless (#2119) doesn't cover this fix:

The e2e builds the gateway from the same checkout (e2e_build_gateway_binaries → cargo build -p openshell-server) and uses a supervisor image tagged with the same commit SHA (inputs.image-tag). Gateway and supervisor are always matched — the version skew that triggers the EPERM path never occurs.

Pinning a v0.0.72 gateway in e2e isn't good either, coupling a regression test to a specific old release is fragile.

Why the unit test job works:

The rootless-caps job runs cargo test -p openshell-supervisor-process --lib -- capability_bounding drop_privileges as an unprivileged testuser on bare ubuntu-24.04. That user has no CAP_SETPCAP in CapEff, so prctl(PR_CAPBSET_DROP) returns EPERM with a non-empty bounding set — directly exercising validate_capability_bounding_set_clear() through drop_privileges(). No version pinning needed, stable across future versions.

[maxamillion]

I am struggling with this change because it degrades the security boundary and I prefer the fail-closed behavior that exists today. I'd be interested in what @drew, @TaylorMutch, and/or @cgwalters have to say about it.

[maxamillion]

Maybe there should be a config option that allows the degraded behavior 🤔

[elezar]

Since this is caused by a version mismatch between the gateway and the supervisor, I don't think we should change the handling of the bounding set at this stage. What we should do is:

• Fix Podman and Kubernetes drivers default supervisor image to :latest instead of pinning to gateway version #2068 to improve consistency.
• Decide what our version-skew policy is w.r.t to the gateway and supervisor and include smoke tests that cover these cases.

[drew]

I am struggling with this change because it degrades the security boundary and I prefer the fail-closed behavior that exists today. I'd be interested in what @drew, @TaylorMutch, and/or @cgwalters have to say about it.

I think we should avoid degrading any boundary and always prefer to fail closed. If there is a capability that is unavailable, then we should find a way to explicitly configure a topology (#2048) to support that configuration.

[waynesun09]

@maxamillion @elezar @drew Thanks for the feedback — I've reworked the PR to keep the bounding-set clear fail-closed. The EPERM-tolerance branch, degraded-mode docs, and the rootless-caps CI job are all dropped.

What remains is diagnosability: the clear runs inside pre_exec, which can't emit structured logs, so a blocked clear previously surfaced only as an opaque exit code 1. The parent now probes PR_CAPBSET_DROP availability before fork() and refuses the spawn with an actionable error plus a High-severity OCSF DetectionFinding when CAP_SETPCAP is unavailable. The child-side EPERM error carries the same diagnosis as a backstop.

Supporting genuinely capability-limited runtimes as an explicit opt-in seems like a better fit for the topology work in #2048.

Verified in rust:1.95 containers, including a --cap-drop SETPCAP run that exercises the refusal path end-to-end; details in the updated description.