Skip to content

feat(sandbox): use policy-first OCI image identity - #2509

Open
matthewgrossman wants to merge 11 commits into
mainfrom
feat/2331-policy-first-oci-user/matthewgrossman
Open

feat(sandbox): use policy-first OCI image identity#2509
matthewgrossman wants to merge 11 commits into
mainfrom
feat/2331-policy-first-oci-user/matthewgrossman

Conversation

@matthewgrossman

@matthewgrossman matthewgrossman commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

🏗️ build-from-issue-agent

Summary

Removes Docker and Podman's dependency on a baked-in sandbox account.
Explicit policy identity fields win independently; omitted fields fall back to
the final image's OCI Config.User, which is validated inside the image before
any agent child starts.

This PR is intentionally limited to process identity. It does not create,
chown, or otherwise prepare /sandbox for OCI-derived identities. OCI
WorkingDir and dynamic workspace-root support remain in follow-up issue
#2526.

Related Issue

Closes #2331

Changes

  • Preserve per-field run_as_user / run_as_group omission for Docker and
    Podman while retaining legacy sandbox defaults for Kubernetes, OpenShift,
    VM, and unknown/remote drivers.
  • Have Docker and Podman inspect the selected image, pass protected OCI
    identity metadata, and create the container from the inspected immutable
    image ID.
  • Normalize driver identity through a shared enum while preserving explicit
    policy values and OCI-declared name or numeric components.
  • Resolve names before readiness, derive only a missing primary GID, and avoid
    passwd/group access for accountless numeric UID:GID.
  • Keep the supervisor root while direct and SSH agent children use the same
    existing privilege-drop path.
  • Limit supplementary-group clearing to OCI-derived identities so
    Kubernetes/OpenShift retain inherited platform groups.
  • Preserve the requested Podman image reference in observability metadata while
    launching the inspected immutable image.
  • Leave Kubernetes/OpenShift and VM workspace behavior, mounts, account files,
    readiness, and gateway protocols unchanged.
  • Update architecture, driver, compute-driver, and BYOC documentation to match
    the narrowed scope.

Scope change

An earlier revision attempted to create and change ownership of /sandbox for
OCI-derived identities. That behavior and its recursive/nested-mount tests were
removed: it expanded an identity PR into a partial workspace implementation.
The complete workspace solution will use OCI WorkingDir and is tracked in
#2526.

Deviations from Plan

  • OCI-declared names remain names after fail-fast validation instead of being
    rewritten numerically. This preserves the image's declared account semantics
    while still rejecting unknown, ambiguous, malformed, and root identities.
  • Docker/Podman /sandbox preparation was removed after review and deferred to
    the coherent workspace-root follow-up.

Testing

  • mise run pre-commit passes
  • mise run test passes
  • Unit tests added/updated
  • Focused Docker E2E passes

Tests added or updated:

  • Unit: Driver-scoped policy omission persistence; Docker/Podman
    immutable-image pinning and protected metadata; explicit and OCI-declared
    name preservation; lazy passwd lookup; named, numeric, mixed, missing,
    malformed, ambiguous, and root OCI identity handling; direct/SSH identity
    material containment; Kubernetes/OpenShift compatibility.
  • Integration: Existing server and driver integration suites pass as part
    of mise run test.
  • E2E: e2e/rust/tests/custom_image.rs covers a named OCI user/group and a
    passwd-less numeric UID:GID; both focused Docker-backed tests pass locally.

Podman unit and driver suites pass. Podman E2E was not run because Podman is not
installed on this host.

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)

Documentation updated:

  • architecture/compute-runtimes.md
  • Docker and Podman driver READMEs
  • Compute-driver and BYOC documentation

Closes #2331

Preserve per-field policy omission, derive Docker and Podman fallbacks from the inspected immutable image, and resolve the final numeric identity before starting agent children.

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
@github-actions

Copy link
Copy Markdown

@matthewgrossman

Copy link
Copy Markdown
Contributor Author

🏗️ build-from-issue-agent

E2E Test Attestation

Local E2E tests passed. CI does not currently run E2E tests, so this comment serves as the verification record.

Field Value
Commit 3bf0b9f6f5456092938e711dc35a896d937110cd
Command e2e/with-docker-gateway.sh cargo test --manifest-path e2e/rust/Cargo.toml --features e2e-docker --test custom_image
Gateway mode Docker
Result ✅ All passed

Test Summary

2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

Tests Executed

  • custom_image::sandbox_from_passwd_less_numeric_oci_user — PASSED
  • custom_image::sandbox_from_custom_dockerfile — PASSED

Podman E2E was not run because Podman is not installed on this host. Podman unit and driver suites passed under mise run ci.

@drew drew left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

Validation: PR #2509 is project-valid because it implements linked, agent-ready sandbox issue #2331, and the author has maintainer authority.
Head SHA: 3bf0b9f6f5456092938e711dc35a896d937110cd

Review findings:

  • Blocking sandbox identity regression found inline.
  • Numeric OCI identity edge case found inline.
  • Fern docs were partially updated, but public tutorials/examples still describe process.run_as_user: sandbox / run_as_group: sandbox as preserving default process settings in docs/get-started/tutorials/first-network-policy.mdx, docs/get-started/tutorials/github-sandbox.mdx, and examples/sandbox-policy-quickstart/*. With omission now carrying driver fallback semantics, update those snippets to omit process or clearly scope explicit sandbox to the base image.

Docs: Updates are present, but the tutorial/example gap above remains blocking for this direct user-facing policy behavior change.

Next state: gator:in-review

Comment thread crates/openshell-supervisor-process/src/identity.rs Outdated
Comment thread crates/openshell-supervisor-process/src/identity.rs Outdated
@drew drew added the gator:in-review Gator is reviewing or awaiting PR review feedback label Jul 28, 2026
@matthewgrossman
matthewgrossman marked this pull request as draft July 28, 2026 04:46
@copy-pr-bot

copy-pr-bot Bot commented Jul 28, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

Keep explicit policy values and OCI-declared names intact, defer passwd lookup until a primary GID is required, and refresh stale policy examples.

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
@matthewgrossman

Copy link
Copy Markdown
Contributor Author

🏗️ build-from-issue-agent

Updated E2E Test Attestation

Local E2E tests passed after addressing review feedback.

Field Value
Commit f1631303
Command e2e/with-docker-gateway.sh cargo test --manifest-path e2e/rust/Cargo.toml --features e2e-docker --test custom_image
Gateway mode Docker
Result ✅ All passed
2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
  • custom_image::sandbox_from_passwd_less_numeric_oci_user — PASSED
  • custom_image::sandbox_from_custom_dockerfile — PASSED

Podman E2E remains unavailable because Podman is not installed on this host. Podman unit and driver coverage passed under mise run ci.

@matthewgrossman

Copy link
Copy Markdown
Contributor Author

Addressed the remaining documentation finding in f1631303: removed the generic explicit sandbox:sandbox process blocks from docs/get-started/tutorials/first-network-policy.mdx, docs/get-started/tutorials/github-sandbox.mdx, examples/sandbox-policy-quickstart/policy.yaml, and examples/sandbox-policy-quickstart/README.md. The surrounding quickstart text now describes the retained fields as filesystem settings rather than default process settings. Markdown lint and full mise run ci pass.

@drew

drew commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

gator-agent

Blocked

Head SHA: f163130351d232d7c47417c76317c262c6473409

Gator is blocked because PR #2509 is still marked as a draft. I saw @matthewgrossman's update that f1631303 addressed the remaining documentation finding; I did not re-run the current-head code review while the PR is still draft.

Next action: @matthewgrossman should mark the PR ready for review when it is ready for gator to re-check the latest head and continue CI/test monitoring.

@drew drew added gator:blocked Gator is blocked by process or repository gates and removed gator:in-review Gator is reviewing or awaiting PR review feedback labels Jul 28, 2026
@matthewgrossman
matthewgrossman marked this pull request as ready for review July 28, 2026 05:02
@matthewgrossman

Copy link
Copy Markdown
Contributor Author

gator I summon you!

@drew drew left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

Validation: PR #2509 is project-valid because it implements linked, agent-ready sandbox issue #2331 and the author has maintainer authority.
Head SHA: f163130351d232d7c47417c76317c262c6473409

Thanks @matthewgrossman, I checked your f1631303 update against the prior gator findings. The explicit policy-field and numeric UID:GID issues appear addressed, and the PR is no longer draft.

Review findings:

  • Warning found inline: OCI name validation and the later privilege-drop lookup can still diverge.
  • Docs note found inline: one quickstart comment and matching tutorial prose still mention preserving process settings after the examples removed process:.

Docs: Fern docs were updated, but the stale wording above should be corrected with the review feedback.

Next state: gator:in-review

Comment thread crates/openshell-supervisor-process/src/identity.rs
Comment thread examples/sandbox-policy-quickstart/policy.yaml Outdated
@drew drew added gator:in-review Gator is reviewing or awaiting PR review feedback and removed gator:blocked Gator is blocked by process or repository gates labels Jul 28, 2026
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

@drew drew left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

Validation: PR #2509 is project-valid because it implements linked, agent-ready sandbox issue #2331 and the author has maintainer authority.
Head SHA: a229ecba987c3dc1700979391495d7d8f5b5e7cf

Thanks @matthewgrossman, I re-checked the latest head after your update and the prior gator findings.

Review findings:

  • No blocking findings remain. The independent code review found no actionable issues on the current head.

Docs: Fern docs and quickstart examples were updated for the direct policy/runtime UX change; no navigation update appears needed.

E2E: test:e2e is required because this changes sandbox lifecycle, Docker/Podman driver behavior, and process identity enforcement.

Next state: gator:watch-pipeline

@drew drew added gator:watch-pipeline Gator is monitoring PR CI/CD status test:e2e Requires end-to-end coverage and removed gator:in-review Gator is reviewing or awaiting PR review feedback labels Jul 28, 2026
@github-actions

Copy link
Copy Markdown

Label test:e2e applied for a229ecb. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

@drew drew removed the gator:watch-pipeline Gator is monitoring PR CI/CD status label Jul 28, 2026
@drew drew added gator:in-review Gator is reviewing or awaiting PR review feedback gator:blocked Gator is blocked by process or repository gates and removed gator:in-review Gator is reviewing or awaiting PR review feedback labels Jul 28, 2026
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

@drew drew left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

Validation: PR #2509 is project-valid because it implements linked, agent-ready sandbox issue #2331 and the author has maintainer authority.
Head SHA: 89ab89592e187efa092f2608a8cdeced70f5ee71

Thanks @matthewgrossman, I re-checked the latest head after the new supervisor pre-exec fix and main merge commits. The prior gator findings remain addressed, and the current-head independent review found one remaining runtime-boundary warning inline.

Review findings:

  • Warning found inline: Kubernetes/OpenShift resolved identity injection does not neutralize the new OCI identity env var, so image/user env can create a conflicting driver identity before readiness.

Docs: Fern docs and quickstart examples were updated for the direct policy/runtime UX change; no navigation update appears needed.

E2E: test:e2e remains required because this changes sandbox lifecycle, Docker/Podman driver behavior, and process identity enforcement.

Next state: gator:in-review

Comment thread crates/openshell-supervisor-process/src/identity.rs
@drew drew added gator:in-review Gator is reviewing or awaiting PR review feedback and removed gator:blocked Gator is blocked by process or repository gates labels Jul 28, 2026
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

@drew drew left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

Validation: PR #2509 is project-valid because it implements linked, agent-ready sandbox issue #2331 and the author has maintainer authority.
Head SHA: 41502ad0a42b237ca5512c9f9afd47b4206b8280

Thanks @matthewgrossman, I re-checked the latest head after the Kubernetes/OpenShift identity protection update. The prior runtime-boundary warning appears addressed, and the current-head independent review found one remaining example correctness warning inline.

Review findings:

  • Warning found inline: the BYOC example now declares USER app, but /sandbox remains root-owned.

Docs: Fern docs and related agent skills were updated for the direct policy/runtime UX change; no navigation update appears needed.

E2E: test:e2e remains required because this changes sandbox lifecycle, Docker/Podman driver behavior, and process identity enforcement.

Next state: gator:in-review

Comment thread examples/bring-your-own-container/Dockerfile Outdated
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

@drew drew left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

Validation: PR #2509 is project-valid because it implements linked, agent-ready sandbox issue #2331 and the author has maintainer authority.
Head SHA: 5ca344e6ecc9e233933075e3fb2707e0083571b4

Thanks @matthewgrossman, I re-checked the latest head after the workspace-root refactor. The prior workspace hard-link concern appears addressed by owning only the workspace root, but the current-head independent review found two remaining privilege-drop issues inline.

Review findings:

  • Critical found inline: supplementary groups can survive the UID/GID drop for explicit numeric or partial OCI identity paths.
  • Critical found inline: no-driver identity resolution can leave a group-only policy running as UID 0.

Docs: Fern docs and related agent skills were updated for the direct policy/runtime UX change; no navigation update appears needed.

E2E: test:e2e remains required because this changes sandbox lifecycle, Docker/Podman driver behavior, and process identity enforcement.

Next state: gator:in-review

Comment thread crates/openshell-supervisor-process/src/process.rs
Comment thread crates/openshell-supervisor-process/src/identity.rs Outdated
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

@drew drew left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

Validation: PR #2509 remains project-valid because it implements linked, agent-ready sandbox issue #2331 and the author has maintainer authority.
Head SHA: 7efabcab54fa3d233a3ce2e7a4a38a9f7ec583ec

Thanks @matthewgrossman, I re-checked the latest head after the partial identity-drop hardening. The prior supplementary-group and no-driver UID 0 findings appear addressed, but the current-head independent review found remaining E2E and example-policy issues.

Review findings:

  • Warnings found inline: Podman-selected E2E tests still use the CLI --from <Dockerfile> path, which builds through Docker defaults instead of the Podman harness socket.
  • Warning: several shipped example policies still explicitly set run_as_user: sandbox / run_as_group: sandbox (examples/agent-driven-policy-management/policy.template.yaml, examples/local-inference/sandbox-policy.yaml, examples/multi-agent-notepad/policy.template.yaml, examples/governance-interceptor/policy.yaml, and examples/governance-interceptor/smoke.sh). That disables the new OCI fallback and will fail for custom images without a sandbox account unless those examples intentionally require the legacy identity.

Docs: Fern docs and the related policy/CLI/debug skills were updated for the main behavior change, but the example-policy drift above remains.

E2E: test:e2e remains required because this changes sandbox lifecycle, Docker/Podman driver behavior, and process identity enforcement.

Next state: gator:in-review

Comment thread e2e/rust/Cargo.toml Outdated
Comment thread e2e/rust/tests/driver_config_volume.rs Outdated
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
@matthewgrossman

Copy link
Copy Markdown
Contributor Author

Addressed the latest Gator findings in c60fae3:

  • Restored custom_image to the Docker-only E2E lane.
  • Kept generic driver_config volume/bind coverage in the shared Docker/Podman lane, while gating only the Dockerfile-based OCI workspace test to Docker.
  • Removed explicit sandbox:sandbox process identity from the five generic example policies called out in the review, including the governance smoke fixture, so they inherit the active driver's identity selection.

Validation: e2e-podman and e2e-docker feature builds pass; focused Docker custom_image and driver_config_volume E2E pass (5/5); mise run pre-commit and mise run test pass.

@drew drew left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

Validation: PR #2509 remains project-valid because it implements linked, agent-ready sandbox issue #2331 and the author has maintainer authority.
Head SHA: c60fae323b5d45339f5e05f37fdfacf3e1c8159e

Thanks @matthewgrossman, I re-checked the latest head after your c60fae32 update. The prior Podman E2E selection and generic example-policy findings appear addressed, but the current-head independent review found remaining runtime-validation and docs warnings inline.

Review findings:

  • Warnings found inline: the explicit/default process identity validation path now accepts arbitrary nonzero numeric UID/GID values instead of preserving the prior sandbox ID range for non-OCI inputs.
  • Warning found inline: the policy schema docs now understate explicit identity validation by saying only root/0 are rejected.

Docs: Fern docs and related agent skills were updated for the main policy-first OCI behavior, but the explicit-policy validation wording still needs the inline correction.

E2E: test:e2e remains required because this changes sandbox lifecycle, Docker/Podman driver behavior, and process identity enforcement. Required Branch Checks and E2E are currently pending for this head.

Next state: gator:in-review

Comment thread crates/openshell-supervisor-process/src/process.rs
Comment thread crates/openshell-supervisor-process/src/process.rs
Comment thread docs/reference/policy-schema.mdx Outdated
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>

@drew drew left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

Validation: PR #2509 remains project-valid because it implements linked, agent-ready sandbox issue #2331 and the author has maintainer authority.
Head SHA: 30cd1eca9cb0d7d1f96671990fab6e1aa9ba36f4

Thanks @matthewgrossman, I re-checked the latest head after your 30cd1eca update. The prior explicit/default UID/GID validation and policy-schema documentation findings appear addressed, but the current-head independent review found two remaining documentation precision warnings inline and one test coverage warning.

Review findings:

  • Warnings found inline: two docs pages still say explicit process identity only rejects root/0, which understates the restored sandbox-range constraint.
  • Warning: Podman OCI identity has unit coverage, but no end-to-end coverage exercises a real Podman inspect/create/immutable-image launch path. A Podman E2E or driver integration test would reduce risk around API schema and image-ID behavior.

Docs: Fern docs were updated for the main policy-first OCI behavior, but the two inline docs corrections remain.

E2E: test:e2e remains required because this changes sandbox lifecycle, Docker/Podman driver behavior, and process identity enforcement. Required Branch Checks and E2E are currently pending for this head.

Next state: gator:in-review

| `filesystem_policy` | Static | Controls which directories the agent can access on disk. Paths are split into `read_only` and `read_write` lists. Any path not listed in either list is inaccessible. Set `include_workdir: true` to automatically add the agent's working directory to `read_write`. [Landlock LSM](https://docs.kernel.org/security/landlock.html) enforces these restrictions at the kernel level. |
| `landlock` | Static | Configures Landlock LSM enforcement behavior. Set `compatibility` to `best_effort` (skip individual inaccessible paths while applying remaining rules) or `hard_requirement` (fail if any path is inaccessible or the required kernel ABI is unavailable). Refer to the [Policy Schema Reference](/reference/policy-schema#landlock) for the full behavior table. |
| `process` | Static | Sets the OS-level identity for the agent process. `run_as_user` and `run_as_group` default to `sandbox`. Root (`root` or `0`) is rejected. The agent also runs with seccomp filters that block dangerous system calls. |
| `process` | Static | Optionally overrides the OS-level identity for the agent process. Docker and Podman fall back per field to the image's OCI `USER`; Kubernetes uses its platform-selected numeric identity. Root (`root` or `0`) is rejected. The agent also runs with seccomp filters that block dangerous system calls. |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gator-agent

Warning: This still says only root / 0 is rejected, but explicit policy values are stricter: they must be sandbox or a numeric UID/GID in the allowed sandbox range. Please mirror the policy-schema wording so users do not try explicit named users or system IDs that only work through Docker/Podman OCI USER fallback.

| Default | `run_as_user: sandbox`, `run_as_group: sandbox`. The supervisor calls `setuid()`/`setgid()` with post-condition verification, disables core dumps with `RLIMIT_CORE=0`, and on Linux sets `PR_SET_DUMPABLE=0`. |
| What you can change | Set `run_as_user` and `run_as_group` in the `process` section. Validation rejects root (`root` or `0`). |
| Default | The compute driver selects a non-root identity. Docker and Podman use the image's OCI `USER` as a per-field fallback. The supervisor calls `setuid()`/`setgid()` with post-condition verification, disables core dumps with `RLIMIT_CORE=0`, and on Linux sets `PR_SET_DUMPABLE=0`. |
| What you can change | Set either or both `run_as_user` and `run_as_group` fields in the `process` section. Each explicit field takes precedence. Validation rejects root (`root` or `0`). |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gator-agent

Warning: Same validation mismatch: explicit process fields reject more than root. Suggested wording: explicit policy values must be sandbox or numeric UID/GID values in the allowed sandbox range; Docker/Podman may use named or non-root system IDs only from OCI USER fallback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gator:in-review Gator is reviewing or awaiting PR review feedback test:e2e Requires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: preserve image-declared identity in Docker and Podman

2 participants