Skip to content

fix(sandbox): distinguish descriptor lookup outcomes - #4494

Open
johntmyers wants to merge 2 commits into
mainfrom
fix/51-socket-handling/johntmyers
Open

johntmyers wants to merge 2 commits into
mainfrom
fix/51-socket-handling/johntmyers

Conversation

@johntmyers

Copy link
Copy Markdown
Collaborator

Summary

Distinguish verified unregistered descriptors from descriptor inspection failures in the sandbox network broker. Deny operations when the broker cannot verify the descriptor before a native fallback.

Related Issue

No issue, per direct request.

Changes

  • Preserve distinct procfs lookup outcomes for sockets, non-sockets, and inspection errors.
  • Apply fail-closed handling to connect, bind, listen, and send fallback paths.
  • Add regression coverage for an unreadable workload and native Unix socket behavior.

Testing

  • Linux sandbox compile and Clippy checks pass
  • Non-root Linux regression tests pass
  • Docker end-to-end run passes, including transparent_tcp
  • Repository commit hooks pass

Checklist

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

Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
@johntmyers johntmyers added the test:e2e Requires end-to-end coverage label Oct 13, 2026
@github-actions

Copy link
Copy Markdown

Label test:e2e applied for 9231102. 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, sandbox, and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

@johntmyers

Copy link
Copy Markdown
Collaborator Author

gator-agent

PR Review Status

The independent review found no blocking problems in the descriptor-inspection change. Lookup errors now deny native fallback, while verified unregistered descriptors retain native Unix socket behavior.

Blocking findings: None.
Carried findings: None.
Non-blocking suggestions: None.

Gator metadata
  • Validation: Focused sandbox network enforcement fix from a verified maintainer.
  • Docs: No published interface or configuration change; existing support documentation requires broker task visibility and fail-closed qualification.
  • Head SHA: 92311026f6f0261d1b716519dc6f16d720fee168
  • Base SHA: 0d4d22eab316148e500716f86280457e9e840023
  • Merge base SHA: 0d4d22eab316148e500716f86280457e9e840023
  • Patch ID: c0d68130e9f92ee6b235b9327f11f33e6fd84a19
  • Gator payload: 12
  • Review mode: initial
  • Previous reviewed SHA: none
  • Review budget exhausted: no
  • Maintainer decision required: no
  • Checks: DCO, vouch, Helm Lint, and Trivy Changes passed. Branch Checks attempt 2 is running; the earlier macOS failure occurred in Rust setup before the build.
  • E2E: test:e2e applied. The mirror matches the current head. Following E2E Label Help, run 38438104172 was rerun and confirmed queued; test results are pending.
  • Next state: gator:watch-pipeline

@johntmyers johntmyers added gator:watch-pipeline Gator is monitoring PR CI/CD status gator:approval-needed Gator completed review; maintainer approval needed and removed gator:watch-pipeline Gator is monitoring PR CI/CD status labels Oct 13, 2026
@johntmyers
johntmyers requested a review from drew October 13, 2026 20:52

@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.

A closed file descriptor should retain the syscall's EBADF behavior.

Comment thread crates/openshell-sandbox/src/network_broker.rs
@johntmyers johntmyers added gator:in-review Gator is reviewing or awaiting PR review feedback and removed gator:approval-needed Gator completed review; maintainer approval needed labels Oct 14, 2026
Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
@johntmyers

Copy link
Copy Markdown
Collaborator Author

gator-agent

PR Review Status

Thanks @drew, I checked your request to preserve EBADF for closed descriptors. The latest commit maps a confirmed missing descriptor to EBADF, while missing tasks and other inspection errors remain distinct and fail closed. The independent follow-up review found no blocking issues in the author-only delta; regression coverage exercises the lookup, both registry paths, and brokered listen.

Blocking findings: None.
Carried findings: None.

Gator metadata
  • Validation: Focused sandbox network enforcement fix from a verified maintainer.
  • Docs: No new published interface or configuration; this delta restores native closed-descriptor error behavior.
  • Checks: Current-head Branch Checks are running; DCO, PR Issue Acceptance, Helm Lint, and Trivy Changes passed.
  • E2E: test:e2e remains applied; current-head Branch E2E Checks is queued. The label-help rerun instruction concerned the previous head; the new head already has its own required run.
  • Head SHA: 3e7bcebdf9533a5142b0afb3edda8a4d26a27bc7
  • Base SHA: 0d4d22eab316148e500716f86280457e9e840023
  • Merge base SHA: 0d4d22eab316148e500716f86280457e9e840023
  • Patch ID: ba6f8717eeb7be0ebe5db6f9fa8d5a90a2d6ef4f
  • Gator payload: 12
  • Review mode: follow_up
  • Previous reviewed SHA: 92311026f6f0261d1b716519dc6f16d720fee168
  • Review budget exhausted: no
  • Maintainer decision required: no
  • Next state: gator:watch-pipeline

@johntmyers johntmyers added gator:watch-pipeline Gator is monitoring PR CI/CD status gator:blocked Gator is blocked by process or repository gates and removed gator:in-review Gator is reviewing or awaiting PR review feedback gator:watch-pipeline Gator is monitoring PR CI/CD status gator:blocked Gator is blocked by process or repository gates labels Oct 14, 2026
@johntmyers
johntmyers requested a review from drew October 14, 2026 18:59
@drew
drew enabled auto-merge October 14, 2026 19:00
@johntmyers johntmyers added gator:merge-ready and removed gator:watch-pipeline Gator is monitoring PR CI/CD status labels Oct 14, 2026
@johntmyers
johntmyers disabled auto-merge October 14, 2026 19:44
@johntmyers
johntmyers enabled auto-merge October 14, 2026 19:44

This branch has not been deployed

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

Labels

gator:merge-ready test:e2e Requires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants