Skip to content

feat(sandbox): honor OCI image working directories - #2530

Open
matthewgrossman wants to merge 6 commits into
mainfrom
feat/2526-oci-working-dir/matthewgrossman
Open

feat(sandbox): honor OCI image working directories#2530
matthewgrossman wants to merge 6 commits into
mainfrom
feat/2526-oci-working-dir/matthewgrossman

Conversation

@matthewgrossman

@matthewgrossman matthewgrossman commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Docker now honors a usable OCI Config.WorkingDir as the sandbox workspace, with an OpenShell-managed /sandbox fallback for empty or root declarations. The resolved workspace is used consistently by the Docker driver, supervisor, filesystem policy, SSH transfers, and editor launch. Podman support was extracted into the follow-up PR linked below; Podman, Kubernetes/OpenShift, and VM retain /sandbox in this PR.

This work was originally developed as a stack on #2509. That PR has merged, and this PR now targets main directly. The original scope also included Podman support; that work was extracted into #2563, which is stacked on this PR.

Related Issue

Part of #2526

Changes

  • inspect WorkingDir and User from the same immutable Docker image and pass the resolved root through the existing supervisor workdir argument
  • enforce a path-independent no-authority-expansion invariant: the final OCI UID, GID, and supplementary groups must already be able to traverse every parent and write and enter the workdir
  • use no-follow file-descriptor traversal and kernel effective-access checks so symlinks, ACLs, and LSM decisions are honored
  • keep /sandbox compatibility behavior unchanged for images with an empty or root workdir
  • reject Docker image VOLUME declarations and OpenShell control mounts that would mask or collide with the resolved workdir
  • use filesystem.include_workdir instead of automatically granting literal /sandbox
  • discover the canonical remote workspace over SSH for upload defaults, download containment, and editor launch
  • preserve fixed /sandbox behavior for Podman, Kubernetes/OpenShift, and VM
  • update architecture, Docker driver, policy, CLI, and agent guidance

Why kernel-effective access checks

Linux access is not determined by ownership and chmod mode bits alone. A POSIX ACL can grant a particular image user access that mode-bit inspection would reject, while an enforcing Linux Security Module such as SELinux or AppArmor can deny access that mode bits appear to allow.

OpenShell does not add ACL or LSM policy management in this PR. Instead, a one-shot validator assumes the final sandbox UID, GID, and supplementary groups and asks the Linux kernel whether that identity can traverse the path and write and enter the workdir. This delegates the decision to the same authority the workload will encounter, avoids maintaining an incomplete permission model in OpenShell, and preserves the central rule that selecting an image workdir must not expand the image user's authority.

Testing

  • mise run pre-commit passes
  • Focused Docker, core, policy/prover, sandbox, supervisor-process, and Podman compatibility unit suites pass
  • Unit tests cover final-identity access, supplementary groups, POSIX ACL grants, enforcing LSM denials, symlink/restrictive-parent rejection, mount collisions, and compatibility fallback
  • Docker E2E coverage includes OCI WorkingDir behavior, direct/SSH access, transfers, and nested volumes

Checklist

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

@github-actions

Copy link
Copy Markdown

@drew drew added the gator:blocked Gator is blocked by process or repository gates label Jul 28, 2026
@drew

drew commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

gator-agent

Blocked

Gator is blocked because PR #2530 currently has merge conflicts (mergeStateStatus: DIRTY) against its base branch.

Next action: @matthewgrossman, please rebase or merge the base branch and resolve the conflicts, then push an updated head SHA so gator can run the independent review.

@matthewgrossman
matthewgrossman force-pushed the feat/2526-oci-working-dir/matthewgrossman branch from 6838984 to 493abe0 Compare July 28, 2026 22:39

@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 #2530 implements linked issue #2526, which is already marked state:agent-ready / state:pr-opened, and the current head is open, ready for review, and no longer conflict-blocked.
Head SHA: 493abe06869d41a5b161a1a97abc21c0c3ce6436

Review findings:

  • Warning: Docker/Podman pass the resolved workspace to the supervisor, but the create specs still appear to inherit the image process cwd. For hand-crafted OCI images where Config.WorkingDir is missing on disk or is not a directory, the container runtime may fail before the supervisor can run the intended workspace validation/preparation path. Please consider starting the supervisor from a known-safe cwd such as / while passing --workdir <resolved-root> separately, and add coverage for missing/non-directory WorkingDir.
  • See the inline warning about accepting whitespace/control-character OCI WorkingDir values before SSH discovery and shell-text flows consume the path.

Docs: Fern docs and relevant agent skill guidance were updated; no docs navigation change appears required because no new page was added.

Next state: gator:in-review

Comment thread crates/openshell-core/src/driver_mounts.rs 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
@matthewgrossman
matthewgrossman force-pushed the feat/2526-oci-working-dir/matthewgrossman branch from c02986a to d14d6c9 Compare July 28, 2026 23:33
@matthewgrossman
matthewgrossman force-pushed the feat/2331-policy-first-oci-user/matthewgrossman branch from 6d53441 to 254364a Compare July 28, 2026 23:38
@matthewgrossman
matthewgrossman force-pushed the feat/2526-oci-working-dir/matthewgrossman branch from d14d6c9 to 41b25a7 Compare July 28, 2026 23:39
@drew drew added test:e2e Requires end-to-end coverage gator:watch-pipeline Gator is monitoring PR CI/CD status 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 41b25a7. 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 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

gator-agent

PR Review Status

Validation: PR #2530 implements linked issue #2526, which is already marked state:agent-ready / state:pr-opened. The PR is open, ready for review, and no longer conflict-blocked.
Head SHA: 41b25a7415a4b502412b5e92926ac9730deac14f

Review findings:

  • No blocking findings remain from the independent review.
  • The previous gator ledger item about whitespace/control-character OCI WorkingDir handling appears addressed in this head.

Docs: Fern docs and relevant agent skill guidance were updated; no docs navigation change appears required because no new page was added.

E2E: Applied test:e2e because this change affects sandbox lifecycle, policy behavior, Docker/Podman driver behavior, and SSH transfer/editor workspace behavior.

Checks: Required checks are not green yet. OpenShell / Branch Checks is still in progress; the Rust jobs are currently failing, but the run is still active and failed logs are not available until it completes, so gator will classify the failure on the next reconciliation cycle.

Next state: gator:watch-pipeline

@drew drew added gator:in-review Gator is reviewing or awaiting PR review feedback and removed gator:watch-pipeline Gator is monitoring PR CI/CD status labels Jul 29, 2026

@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 #2530 implements maintainer-authored, agent-ready issue #2526 and remains open, non-draft, mergeable, and project-valid.
Head SHA: 6bafbb649530f60871f7b4a545a6c56bfde5ead0

Review findings:

  • One warning remains about guaranteeing that an existing OCI workspace is writable after ownership preparation; see the inline comment.
  • The prior whitespace/control-character OCI WorkingDir finding remains addressed and was not re-raised.

Docs: Fern docs and relevant agent skill guidance are updated; no navigation change is needed because no page was added.

E2E: test:e2e remains appropriate for the sandbox lifecycle, policy, Docker/Podman, and SSH workspace changes.

Next state: gator:in-review

Comment thread crates/openshell-supervisor-process/src/process.rs Outdated

@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 #2530 implements maintainer-authored, agent-ready issue #2526 and remains open, non-draft, mergeable, and project-valid.
Head SHA: ad58a35413db4717d8d487b782d2f45465d9bd18

Review findings:

  • One warning remains about guaranteeing that an existing OCI workspace is writable after ownership preparation; see the inline comment.
  • The new head addresses the SSH connection-limit path, but it does not change the workspace ownership/preparation code behind the unresolved warning.
  • The prior whitespace/control-character OCI WorkingDir finding remains addressed and was not re-raised.

Docs: Fern docs and relevant agent skill guidance are updated; no navigation change is needed because no page was added.

E2E: test:e2e remains appropriate for the sandbox lifecycle, policy, Docker/Podman, and SSH workspace changes.

Next state: gator:in-review

Comment thread crates/openshell-supervisor-process/src/process.rs Outdated

@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 #2530 implements agent-ready issue #2526 and remains open, non-draft, mergeable, and project-valid.
Head SHA: ccba5bb168afa4d7b554bb1b42b121871c745fd0

Thanks @matthewgrossman. I checked your latest replies against the current diff: the whitespace/control-character validation and existing 0555 workspace permission findings are resolved and were not re-raised.

Review findings:

  • Three distinct warnings remain; see the inline comments for the Docker pre-inspection mount check, inaccessible workspace ancestors, and incomplete protection of system paths selected through OCI WorkingDir.

Docs: Fern docs and the relevant generate-sandbox-policy / openshell-cli skill guidance are updated; no navigation change is needed because no page was added or moved.

E2E: test:e2e remains appropriate for the sandbox lifecycle, policy, Docker/Podman, and SSH workspace changes.

Next state: gator:in-review

Comment thread crates/openshell-driver-docker/src/lib.rs
Comment thread crates/openshell-supervisor-process/src/process.rs Outdated
Comment thread crates/openshell-core/src/driver_mounts.rs Outdated

@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 #2530 implements agent-ready issue #2526 and remains open, non-draft, mergeable, and project-valid.
Head SHA: babd6ba679067f40eaacbf5bfad4a65283e4acaa

Thanks @matthewgrossman. I checked your current-head replies against the diff: the Docker placeholder-metadata mount collision, inaccessible 0700 ancestor case, and protected-system-tree coverage are addressed and were not re-raised.

Review findings:

  • One distinct warning remains for workspace ancestors traversable only through a named user's supplementary groups; see the inline comment.
  • One docs wording correction is suggested; see the inline comment.

Docs: Relevant Fern docs and agent skill guidance are updated, and no navigation change is needed because no page was added or moved. The baseline-filesystem wording should be qualified as noted inline.

E2E: test:e2e remains appropriate for the sandbox lifecycle, policy, Docker/Podman, SSH, transfer, and editor-workspace changes.

Next state: gator:in-review

Comment thread crates/openshell-supervisor-process/src/process.rs Outdated
Comment thread docs/sandboxes/policies.mdx Outdated
Base automatically changed from feat/2331-policy-first-oci-user/matthewgrossman to main July 29, 2026 05:37
@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 29, 2026
@drew drew added the gator:in-review Gator is reviewing or awaiting PR review feedback label Jul 29, 2026

@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 #2530 implements agent-ready issue #2526 and remains open, non-draft, mergeable, and project-valid.
Head SHA: 0af4badfd437830521dade6f36439b12da9bb514

Thanks @elezar. I checked the questions you raised about existing upload targets, relative workspace paths, containment-helper reuse, dynamic mount collisions, stale comments, the supervisor workdir contract, and naming. The current commit addresses those points: it documents merge/overwrite behavior, accepts workspace-relative downloads with containment checks, reuses the core path helper, clarifies the dynamic collision and Kubernetes workdir contracts, and updates the naming/comments.

Review findings:

  • Warning: crates/openshell-prover/src/policy.rs:260-273 still materializes literal /sandbox when filesystem.include_workdir is true, while this PR changes that setting to mean the driver-resolved workspace. model::encode_filesystem() therefore receives stale filesystem facts for non-default OCI workspaces, even though the current prover queries do not yet consume them. This cross-file finding is summarized here because the relevant lines are outside the current diff. Please represent the workdir symbolically or pass the resolved workspace into the prover, and cover a non-default root such as /workspace/project.
  • Suggestion: The new documented behavior for uploading into an existing named directory (overwrite matching entries while preserving unrelated entries) lacks an E2E case. Consider adding one conflicting remote file and one unrelated remote file, then verify overwrite and preservation.
  • See the inline suggestion about Podman coverage for nested-volume ownership.
  • The eight durable earlier gator findings remain addressed and were not re-raised.

Docs: Fern docs and related agent skills cover the workspace-aware transfer, policy, and compute-driver behavior. No navigation change is needed because no page was added or moved.

Checks: Branch Checks, DCO, Helm Lint, and Docker E2E are green. The required E2E aggregate is still pending, with one Podman job currently failed and other jobs still running; CI classification should wait until the run completes.

Next state: gator:in-review

Comment thread e2e/rust/tests/driver_config_volume.rs

@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 #2530 implements agent-ready issue #2526 and remains open, non-draft, mergeable, and project-valid.
Head SHA: d6ea9d52ab9a57f07a6f513a5c7dd95ce91adaa9

Thanks @matthewgrossman. I checked the two new commits after 0af4badfd437830521dade6f36439b12da9bb514 against the prior feedback. The nested-volume ownership E2E now runs for both Docker and Podman, so that suggestion is addressed. The durable earlier findings remain resolved and were not re-raised.

Review findings:

  • Warning: crates/openshell-prover/src/policy.rs:260-273 still expands filesystem.include_workdir to literal /sandbox, so the prover models stale filesystem facts for a non-default OCI workspace even though the runtime now uses the driver-resolved root. This remains outside the current diff. Please represent the workdir symbolically or pass the resolved workspace into the prover, cover /workspace/project, and preserve explicit /sandbox paths literally.
  • See the inline suggestion for the remaining upload merge/overwrite E2E gap.

Docs: Fern docs and relevant policy/CLI skill guidance cover the workspace-aware behavior; no navigation update is needed because no page was added or moved.

E2E: test:e2e remains appropriate. Podman nested-volume coverage is now present, but the upload merge contract still lacks the focused case noted inline.

Next state: gator:in-review

Comment thread e2e/rust/tests/custom_image.rs

@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 #2530 implements agent-ready issue #2526 and remains open, non-draft, mergeable, and project-valid.
Head SHA: 12b0bd6d25b6246cbec861fc95ed36ac13a5082e

Thanks @matthewgrossman. I checked the new current-head commit after d6ea9d52ab9a57f07a6f513a5c7dd95ce91adaa9. Building the OCI fixture with the active container engine is correct and introduces no apparent regression, but it does not address the remaining general findings.

Review findings:

  • Warning: crates/openshell-prover/src/policy.rs:270 still expands filesystem.include_workdir to literal /sandbox, so the prover models stale filesystem facts for non-default OCI workspaces. This is outside the current diff. Please pass the resolved workspace into the prover or represent it symbolically, preserve explicit /sandbox paths literally, and cover /workspace/project.
  • Warning: .agents/skills/debug-openshell-cluster/SKILL.md:151 covers OCI USER diagnostics but not the new WorkingDir readiness failures routed there by the agent-infrastructure maintenance map. This is outside the current diff. Please add Docker/Podman guidance for invalid or protected paths, symlink/non-directory roots, and inaccessible ancestors.
  • See the inline suggestion for the still-missing upload merge/overwrite E2E case.
  • Durable earlier findings remain addressed and were not re-raised.

Docs: Fern docs cover the direct workspace, transfer, and policy UX, and no navigation change is needed because no page was added or moved. The companion debug-openshell-cluster guidance remains incomplete as noted above.

E2E: test:e2e remains appropriate for the sandbox lifecycle, policy, Docker/Podman, SSH, transfer, and editor-workspace changes.

Next state: gator:in-review

Comment thread e2e/rust/tests/custom_image.rs
@drew

drew commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

gator-agent

PR Review Status

Validation: PR #2530 implements agent-ready issue #2526 and remains open, non-draft, mergeable, and project-valid.
Head SHA: 6a1a875912e6ae62670b91a49e35255bff9872c5

Thanks @matthewgrossman. I checked the new current-head E2E after 12b0bd6d25b6246cbec861fc95ed36ac13a5082e. It now seeds an existing remote directory, verifies a conflicting file is overwritten, confirms a new file is added, and confirms an unrelated remote file is preserved, so the upload merge/overwrite coverage request is resolved.

Review findings:

  • Warning: crates/openshell-prover/src/policy.rs:270 still expands filesystem.include_workdir to literal /sandbox, so the prover models stale filesystem facts for non-default OCI workspaces. This is outside the current diff. Please pass the resolved workspace into the prover or retain the workdir symbolically, cover /workspace/project, and preserve explicit /sandbox paths literally.
  • Warning: .agents/skills/debug-openshell-cluster/SKILL.md:151 still covers OCI USER readiness failures but not the new WorkingDir failure paths. This is outside the current diff. Please add Docker/Podman guidance for relative or protected paths, symlink/non-directory components, inaccessible ancestors, and ownership/permission preparation failures.
  • Durable earlier gator findings remain addressed and were not re-raised.

Docs: Relevant Fern docs and the policy/CLI skill guidance are updated, and no navigation change is needed because no page was added or moved. The companion cluster-debug guidance remains incomplete as noted above.

E2E: test:e2e remains appropriate. The required E2E check is currently pending.

Next state: gator:in-review

@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 #2530 implements agent-ready issue #2526 and remains open, non-draft, mergeable, and project-valid.
Head SHA: a672d3c7c883341a183f0b97022771b6cf0a904d

Thanks @matthewgrossman. I checked the current-head changes against the previous disposition. The prover now models an unresolved OCI workdir symbolically instead of inventing literal /sandbox, the Docker/Podman cluster-debug guidance covers the previously missing WorkingDir failure modes, and the upload merge/overwrite E2E is present. Those earlier findings are resolved, and the durable resolved findings were not re-raised.

Review findings:

  • Two distinct Podman warnings remain, plus one companion-skill filter correction; see the inline comments.
  • The capability-free UID-0 Podman probe rejects a valid workdir beneath an owner-only parent even when the final sandbox identity can traverse it.
  • Semantic probe failures are reduced to an exit code and removed before an actionable diagnostic can reach operators.

Docs: Fern documentation covers OCI workspace selection, filesystem policy behavior, transfers, custom-image requirements, and driver differences. No navigation update is needed because no page was added or moved. The cluster-debug probe-name filter needs the inline correction.

E2E: test:e2e remains appropriate. Add focused Podman coverage for a valid workdir beneath a 0700 parent owned by the eventual sandbox user.

Next state: gator:in-review

Comment thread crates/openshell-driver-podman/src/container.rs Outdated
Comment thread crates/openshell-driver-podman/src/driver.rs Outdated
Comment thread .agents/skills/debug-openshell-cluster/SKILL.md Outdated
@matthewgrossman

Copy link
Copy Markdown
Contributor Author

Addressing the two warnings from the prior Gator summary:

  • Prover workdir modeling: addressed on the current head. When the resolved image workdir is unavailable, the prover now represents it symbolically as <OCI_WORKDIR> rather than inventing /sandbox. Tests cover materializing /workspace/project, preserving an explicitly configured /sandbox path, and avoiding duplication when the resolved workdir is already present.
  • Cluster-debug guidance: addressed on the current head. The skill now covers malformed and non-directory workdirs, symlink components, inaccessible ancestors, insufficient final-identity permissions, special filesystems, OpenShell control-path collisions, and the Docker/Podman-specific validation flows. It also includes Podman probe timeout and cleanup diagnostics.

@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 #2530 implements agent-ready issue #2526 and remains open, non-draft, and project-valid.
Head SHA: 1d2f22e7c5413339d458b2ea5c353c1a185165f4

Thanks @matthewgrossman. I checked the current-head Podman changes against the previous disposition. The probe now validates as the final sandbox identity, captures bounded diagnostics before cleanup, covers the owner-only parent case, and uses the corrected probe-name filter. Those findings are resolved, and durable earlier findings were not re-raised.

Review findings:

  • One warning remains about the scope of the global Podman lifecycle lock; see the inline comment.

Docs: Fern docs, architecture guidance, and related agent skills are sufficient. No navigation update is needed because no page was added or moved.

E2E: test:e2e remains appropriate. Add a focused concurrency regression for the lifecycle-lock scope.

Next state: gator:in-review

Comment thread crates/openshell-driver-podman/src/driver.rs Outdated

@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 #2530 implements agent-ready issue #2526 and remains open, non-draft, mergeable, and project-valid.
Head SHA: ed0619ac55022bc17ac1be741c455112aec43337

Thanks @matthewgrossman. I checked the current-head change against the previous Podman lifecycle-lock finding. Create/start and removal remain serialized, while polling and bounded log retrieval now run without holding the global lock; the added concurrency regression covers that scope, so the prior finding is resolved.

Review findings:

  • Critical (CWE-269): Docker can inherit forged workspace-attestation variables from the image because the driver removes those keys instead of explicitly overriding them; see the inline comment.
  • Warning: The credential-dropped workspace probe still decides access from Unix mode bits rather than the kernel's ACL/LSM access decision; see the inline comment.
  • Durable earlier gator findings remain addressed and were not re-raised.

Docs: Existing Fern pages, architecture guidance, and related agent skills cover the workspace behavior; no navigation change is needed because no page was added or moved.

E2E: test:e2e remains appropriate for this sandbox lifecycle, policy, Docker/Podman, SSH, transfer, and editor-workspace change.

Next state: gator:in-review

Comment thread crates/openshell-driver-docker/src/lib.rs Outdated
Comment thread crates/openshell-supervisor-process/src/process.rs
@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 30, 2026
@drew

drew commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

gator-agent

Review Convergence Checkpoint

Head SHA: 59f179e527872ac13d430bab814586a00b6559f5
Base SHA: df698042d0c7760f9dc9233b09d45d5f1c219c2b
Merge base SHA: df698042d0c7760f9dc9233b09d45d5f1c219c2b
Patch ID: f48b7f6a854439a4fa64241e6d4079a4e81e1ecd
Gator payload: 2

Thirteen finding-bearing review rounds have completed, so Gator has reached the mandatory maintainer convergence checkpoint.

Thanks @matthewgrossman. I checked the latest author-only delta after ed0619ac55022bc17ac1be741c455112aec43337, excluding the merged upstream changes. The bounded checkpoint review found no new Critical defect introduced by that delta.

Root-cause findings:

  • gator-inline-3678664540: forged image workspace-attestation metadata. The latest delta explicitly overrides driver-owned attestation variables for Docker and other non-probed specs and adds forged-image coverage; this appears addressed.
  • gator-inline-3678664544: workspace validation disagreed with kernel ACL/LSM-effective access. The latest delta uses no-follow descriptor traversal plus effective-credential kernel access checks and adds focused coverage; this appears addressed.
  • Earlier workspace-resolution, identity, mount-collision, Podman-probe, prover, docs, and E2E findings are recorded as addressed in the durable review history. No technical blocker remains from the bounded Critical-only checkpoint review.

Scope growth:

  • Remediation expanded from OCI workdir selection into supervisor identity handling, kernel-effective access validation, Docker/Kubernetes attestation sanitization, Podman probe lifecycle and diagnostics, prover behavior, companion skills, and E2E coverage.
  • No new public configuration or policy surface was introduced by the latest delta.

Reviewer-quality signals:

  • Ledger telemetry: 14 review rounds, 13 finding-bearing rounds, 17 unique findings, 0 duplicate finding-ID occurrences, and 0 findings repeated across review heads.
  • The history contains legacy semantically overlapping upload-E2E suggestions and repeated read-only-workspace warnings; these are review-convergence signals, not contributor defects.
  • No waived finding was proposed again, and the latest reviewer proposed no unchanged-code finding.

Maintainer action: please choose one of the checkpoint outcomes—accept the current scope, split follow-up work, waive an obligation, or explicitly authorize another autonomous review round.

Next state: gator:blocked

@drew

drew commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

gator-agent

Review Convergence Checkpoint

Head SHA: 6855d4e728b6f10b34861fab4630d774b5d97283
Base SHA: df698042d0c7760f9dc9233b09d45d5f1c219c2b
Merge base SHA: df698042d0c7760f9dc9233b09d45d5f1c219c2b
Patch ID: ad5c2505c8d108971ca61bccf206b050c4389b8f
Gator payload: 2

Thirteen finding-bearing review rounds have completed, so Gator remains at the mandatory maintainer convergence checkpoint.

Thanks @matthewgrossman. I checked both author-only commits after checkpoint head 59f179e527872ac13d430bab814586a00b6559f5, including the newest borrow-only cleanup after 199e9ee81b36aa79e942cd9dde2e2fa8491975b5. The bounded Critical-only checkpoint reviews found no new Critical security, data-loss, or correctness defect introduced by either delta.

Root-cause findings:

  • The durable workspace-resolution, identity, mount-collision, Podman-probe, prover, docs, E2E, forged-attestation, and kernel-effective-access findings retain their prior dispositions.
  • The latest delta does not materially invalidate the prior finding resolutions or introduce a new technical blocker.

Scope growth:

  • Remediation remains broader than the original OCI workdir selection, spanning supervisor identity handling, effective-access validation, driver attestation, Podman probe lifecycle, prover behavior, companion skills, and E2E coverage.
  • The latest delta does not add another subsystem or new public configuration or policy surface.

Reviewer-quality signals:

  • Ledger telemetry remains 14 review rounds, 13 finding-bearing rounds, 17 unique findings, 0 duplicate finding-ID occurrences, and 0 findings repeated across review heads.
  • The latest checkpoint reviewer proposed 0 findings, including 0 unchanged-code proposals and 0 duplicate invariants.

Maintainer action: please choose one of the checkpoint outcomes—accept the current scope, split follow-up work, waive an obligation, or explicitly authorize another autonomous review round.

Next state: gator:blocked

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
@matthewgrossman
matthewgrossman force-pushed the feat/2526-oci-working-dir/matthewgrossman branch from 6855d4e to 6b706bf Compare July 30, 2026 20:22
@drew

drew commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

gator-agent

Review Convergence Checkpoint

Head SHA: 6b706bf5ecd6932d6ecfdcef07b7ebf3ad1b8cea
Base SHA: 28f3bee0ce7e89c845016fc0554f8990a31543bb
Merge base SHA: 28f3bee0ce7e89c845016fc0554f8990a31543bb
Patch ID: 0ea5dd473e2aecbfc5dff89e11d10ddf6c9a0c87
Gator payload: 2

Thirteen finding-bearing review rounds have completed, so Gator remains at the mandatory maintainer convergence checkpoint.

Thanks @matthewgrossman. I checked the force-pushed, squashed author patch against checkpoint head 6855d4e728b6f10b34861fab4630d774b5d97283, separating the new base from the effective author changes. The bounded Critical-only checkpoint review found no new Critical security, data-loss, or correctness defect.

Root-cause findings:

  • gator-inline-3678664540: the forged workspace-attestation route from the earlier patch no longer exists in the current implementation; Docker now supplies the workspace through its driver-owned supervisor command path.
  • gator-inline-3678664544: the earlier kernel-effective ACL/LSM access-check resolution no longer holds in this squash. The current head again validates workspace access from ownership and POSIX mode metadata. Because the convergence checkpoint prohibits another autonomous Warning round, this remains a maintainer scope decision rather than a newly posted Warning.
  • Earlier workspace-resolution, identity, mount-collision, prover, docs, and E2E history remains recorded in the durable ledger. The Podman-specific implementation has been removed from this PR's effective patch even though linked issue feat: honor OCI WorkingDir for Docker and Podman workspaces #2526 currently covers both Docker and Podman.

Scope growth:

  • The current patch is narrower than the previous checkpoint patch but still spans Docker workspace selection, supervisor identity and filesystem handling, CLI transfer/editor behavior, policy/prover behavior, Kubernetes compatibility, documentation, skills, and E2E coverage.
  • The Docker/Podman scope split and the removed kernel-effective validation need explicit maintainer disposition; Gator will not autonomously expand or re-review the scope again.

Reviewer-quality signals:

  • Ledger telemetry: 14 review rounds, 13 finding-bearing rounds, 17 unique findings, 0 duplicate finding-ID occurrences, and 0 findings repeated across review heads.
  • Current checkpoint reviewer telemetry: 0 proposed findings, 0 unchanged-code proposals, 0 duplicate invariants, and 0 missing-reproducer downgrades.

Maintainer action: please accept the current Docker-only scope, confirm a split follow-up for Podman, waive the remaining access-validation obligation, or explicitly authorize another autonomous review round.

Next state: gator:blocked

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

drew commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

gator-agent

Review Convergence Checkpoint

Head SHA: 79706d62106b791779850923111c8153accfcd21
Base SHA: 596d729e3fbe4f828424cadbe0bf1b3c5b307ada
Merge base SHA: 596d729e3fbe4f828424cadbe0bf1b3c5b307ada
Patch ID: 0ea5dd473e2aecbfc5dff89e11d10ddf6c9a0c87
Gator payload: 2

Thirteen finding-bearing review rounds have completed, so Gator remains at the mandatory maintainer convergence checkpoint.

Thanks @matthewgrossman. I checked the merge-from-main commit after checkpoint head 6b706bf5ecd6932d6ecfdcef07b7ebf3ad1b8cea. The feedback ledger reports the same effective patch ID as that reviewed checkpoint head, so there is no new author-only delta and no additional autonomous reviewer run is warranted.

Root-cause findings:

  • gator-inline-3678664540: the earlier forged workspace-attestation route remains absent from the current Docker implementation.
  • gator-inline-3678664544: the current implementation still uses ownership and POSIX mode metadata for workspace access validation. Because the convergence checkpoint prohibits another autonomous Warning round, this remains a maintainer scope decision.
  • Earlier workspace-resolution, identity, mount-collision, prover, docs, and E2E history retains its durable disposition. Podman implementation remains split into follow-up PR feat(podman): honor OCI image working directories #2563.

Scope growth:

  • The effective patch is unchanged by this merge-from-main commit and adds no subsystem, public configuration, or policy surface.
  • The Docker/Podman scope split and remaining access-validation obligation still need explicit maintainer disposition.

Reviewer-quality signals:

  • Ledger telemetry remains 14 review rounds, 13 finding-bearing rounds, 17 unique findings, 0 duplicate finding-ID occurrences, and 0 findings repeated across review heads.
  • No reviewer was run for this upstream-only head update because its effective patch matches the already-reviewed checkpoint patch.

Maintainer action: please accept the current Docker-only scope, confirm the Podman split, waive the remaining access-validation obligation, or explicitly authorize another autonomous review round.

Next state: gator:blocked

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

drew commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

gator-agent

Re-check After Maintainer Update

Thanks @matthewgrossman. I verified your repository-admin authority and re-evaluated current head 7306dd7b24b862a4b5c1330e35158f1d9f85ab69 after your update confirming the Podman split and the effective-access remediation.

Head SHA: 7306dd7b24b862a4b5c1330e35158f1d9f85ab69
Base SHA: 596d729e3fbe4f828424cadbe0bf1b3c5b307ada
Merge base SHA: 596d729e3fbe4f828424cadbe0bf1b3c5b307ada
Patch ID: ec6d61c7623c6322f502beaa75aae0aed2efe3df
Gator payload: 2
Review mode: human_checkpoint

What I checked:

  • The complete durable feedback ledger and the author-only delta after checkpoint head 79706d62106b791779850923111c8153accfcd21.
  • The one-shot validator added in fe98793f and its ACL/LSM-focused coverage in 7240e3fc.
  • A bounded independent Critical-only checkpoint review, which found no new Critical security, data-loss, or correctness defect.
  • Your explicit split of the Podman work into follow-up PR feat(podman): honor OCI image working directories #2563.

Checkpoint disposition: resolved. The Podman split supplies the requested maintainer scope decision, and the current delta addresses the remaining kernel-effective access obligation. Earlier durable findings retain their recorded dispositions.

Pipeline disposition: author action is still required. OpenShell / Branch Checks fails in both Rust lint jobs because the new tests do not compile:

  • crates/openshell-supervisor-process/src/process.rs:3150 and :3156 call crate::sandbox::prepare_current_user and crate::sandbox::enforce, but those functions are exported under crate::sandbox::linux.
  • crates/openshell-supervisor-process/src/process.rs:3090 leaves the ACL permission literal type ambiguous for to_ne_bytes.
  • The same test code also triggers unused_qualifications for std::ffi::CString and the sandbox calls.

Please fix the compile/clippy errors and push a new head. OpenShell / E2E is still pending; Helm Lint and GPU E2E are green.

Reviewer-quality signals: 0 proposed findings, 0 unchanged-code proposals, 0 duplicate invariants, and 0 missing-reproducer downgrades. Ledger telemetry remains 14 review rounds, 13 finding-bearing rounds, 17 unique findings, and no duplicate finding IDs.

Next state: gator:in-review

@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 30, 2026
/// Keep this list tied to concrete `OpenShell` control resources. Workspace
/// safety itself is enforced from the image's ownership and mode metadata,
/// not by guessing which distro paths are sensitive.
const OPENSHELL_CONTROL_PATHS: &[&str] = &[

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

will this list need to be updated frequently? what I'm worried about is that some new path that the supervisor relies on /etc/something starts to happen, but we don't add it here, and then some sandbox image is allowed to set its WORKDIR /etc/something, and then can write whatever to it, influencing how the supervisor runs


Image `VOLUME` declarations must not cover the workdir or one of its parents
because Docker would mount the volume before the supervisor could validate the
immutable image path.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

how would this work in normal docker images; can a VOLUME shadow a WORKDIR?

and how do we guarantee that in this PR? doing manual path arithmetic?

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
resolve_oci_supplementary_gids_at(declaration, primary_gid, Path::new(GROUP_PATH))
}

fn resolve_oci_supplementary_gids_at(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

why did this have to get added in this PR? I would've expected the previous USER PR to handle this

@drew

drew commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

gator-agent

Re-check After Author and Maintainer Update

Thanks @matthewgrossman. I re-evaluated current head a554b062ced98a045a2e2181e77c3bd0412a7173 after your compile/clippy repair and your current-head review questions.

Head SHA: a554b062ced98a045a2e2181e77c3bd0412a7173
Base SHA: 596d729e3fbe4f828424cadbe0bf1b3c5b307ada
Merge base SHA: 596d729e3fbe4f828424cadbe0bf1b3c5b307ada
Patch ID: 459ade88e4b124009a14d5a49b056d0ef1d61573
Gator payload: 2
Review mode: human_checkpoint
Previous reviewed SHA: 7306dd7b24b862a4b5c1330e35158f1d9f85ab69

What I checked:

  • The complete durable feedback ledger and the author-only delta after 7306dd7b24b862a4b5c1330e35158f1d9f85ab69.
  • The real effective-identity write probe added in this head and the compile/clippy corrections requested in the previous disposition.
  • A bounded independent Critical-only checkpoint review. It found no new Critical security, data-loss, or correctness defect introduced by this delta.
  • Your current questions about whether the protected supervisor-path set can drift, how OCI VOLUME can shadow a WORKDIR and how the PR detects that collision, and why the identity-related change belongs in this PR.

Disposition: the previous compile/clippy blocker is resolved. OpenShell / Branch Checks, DCO, Helm Lint, and the other completed required gates are green. OpenShell / E2E is still pending.

Remaining items:

  • The three trusted maintainer review questions above still need a reply or explicit resolution before gator can hand the PR to pipeline-only monitoring.

Durable earlier findings retain their recorded dispositions. Reviewer telemetry for this round: 0 proposed findings, 0 blockers, 0 unchanged-code proposals, 0 duplicate invariants, and 0 missing-reproducer downgrades. Ledger telemetry is 15 review rounds, 13 finding-bearing rounds, 17 unique findings, 0 duplicate finding-ID occurrences, and 0 findings repeated across review heads.

Next state: gator:in-review

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.

3 participants