FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat(compute): delegate sandbox authentication to drivers by drew · Pull Request #2968 · NVIDIA/OpenShell · GitHub

/ OpenShell Public

feat(compute): delegate sandbox authentication to drivers - #2968

Open
drew wants to merge 4 commits into
mainfrom
2417-delegate-sandbox-auth/drew
ConversationCommits (4)ChecksFiles changed
Open

feat(compute): delegate sandbox authentication to drivers#2968
drew wants to merge 4 commits into
mainfrom
2417-delegate-sandbox-auth/drew

Conversation

drew commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Summary

Delegate sandbox bootstrap credential authentication to the selected compute driver so in-process and external drivers share the same trust contract. Keep gateway authorization and JWT minting authoritative while moving Kubernetes-specific TokenReview policy behind the Kubernetes driver.

Related Issue

Related to #2417

This is the foundational PR for #2823; #2823 will be rebased on this branch.

Changes

  • add the capability-gated AuthenticateSandbox compute-driver RPC
  • add a path-scoped gateway authenticator that accepts only the driver-returned sandbox ID
  • move Kubernetes TokenReview, namespace, pod UID, and Sandbox owner validation into the Kubernetes driver
  • retain Kubernetes bootstrap settings under [openshell.drivers.kubernetes]
  • document the driver socket as a sandbox-identity trust boundary and update cluster debugging guidance

Testing

  • mise run pre-commit passes (format and Markdown passed; aggregate run was blocked by sandboxed sccache IPC, dependency downloads, and bundled buf execution)
  • Unit tests added/updated
  • E2E tests added/updated (not run; requires a Kubernetes cluster)

Additional verification:

  • cargo check -p openshell-server -p openshell-driver-kubernetes
  • cargo check -p openshell-server --tests
  • cargo test -p openshell-driver-kubernetes token_review --lib
  • cargo clippy -p openshell-server -p openshell-driver-kubernetes --all-targets -- -D warnings
  • cargo fmt --all -- --check
  • git diff --check

Checklist

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

drew requested review from a team, derekwaynecarr, mrunalp and sjenning as code owners August 26, 2026 22:08

copy-pr-bot Bot commented Aug 26, 2026

Copy link
Copy Markdown

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.

drew force-pushed the 2417-delegate-sandbox-auth/drew branch 2 times, most recently from 72845bc to 1aa14c9 Compare August 26, 2026 22:12

github-actions Bot commented Aug 26, 2026

Copy link
Copy Markdown

🌿 Preview your docs: https://nvidia-preview-pr-2968.docs.buildwithfern.com/openshell

drew force-pushed the 2417-delegate-sandbox-auth/drew branch from 1aa14c9 to 99956e3 Compare August 27, 2026 00:06

sjenning commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

review-github-pr with Sol. Mostly loss of test coverage and some docs updates that were missed.

Potential Concerns

  • Security-critical test coverage regresses substantially. The removed authenticator tested audience rejection, every namespace mode, owner-reference validation, path scoping, and error propagation. The replacement adds only two TokenReview helper tests and one annotation test (driver.rs:4963); the new gateway authenticator has no direct tests.
  • Agent/docs drift remains: debug-openshell-cluster still tells operators to inspect openshell.io/sandbox-id (SKILL.md:395), while the code now uses openshell.ai/sandbox-id. Gateway-auth and compute-driver docs also still attribute TokenReview validation to the gateway (docs/reference/gateway-auth.mdx:194, docs/reference/sandbox-compute-drivers.mdx:376).
  • gateway-config.mdx:253 says external drivers read backend settings from [openshell.drivers.], but remote gateway configuration reads only socket_path (driver_config.rs:52); the external Kubernetes binary receives those settings through CLI flags/environment variables.

sjenning added the test:e2e Requires end-to-end coverage label Aug 27, 2026

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown

Label test:e2e applied, but pull-request/2968 does not exist yet. A maintainer needs to comment /ok to test 99956e39237f1f350d0d69fd5603840b315a2008 to mirror this PR. Once the mirror exists, re-apply the label or re-run Branch E2E Checks from the Actions tab.

sjenning commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

/ok to test 99956e3

drew commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

Resolved in 94eaa7c. I restored direct gateway compute-driver authenticator coverage for success, path scoping, missing credentials, empty identities, and driver error propagation; restored Kubernetes TokenReview audience, pod binding, all namespace modes, pod UID/annotation, and controlling Sandbox owner UID/ID validation coverage; updated the debug skill and auth/compute-driver docs to assign TokenReview to the Kubernetes driver and use openshell.ai/sandbox-id; and clarified that an external driver gateway table supplies socket_path while the external process receives its own backend settings through CLI/environment. The focused build also found and fixed the Docker tracing wrapper missing AuthenticateSandbox delegation. The stack PR #2823 has been rebased onto this new base tip.

drew commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

/ok to test 94eaa7c

drew commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

/ok to test bf3b484

pierzchalski mentioned this pull request Aug 27, 2026
Open
drew force-pushed the 2417-delegate-sandbox-auth/drew branch from bf3b484 to 3c1450d Compare August 27, 2026 14:55
drew added 4 commits August 27, 2026 17:38
feat(compute): delegate sandbox authentication to drivers
54ae8d6 
Signed-off-by: Drew Newberry <385+drew@users.noreply.github.com>
fix(kubernetes): align sandbox identity annotation
2ccaa3c 
Signed-off-by: Drew Newberry <385+drew@users.noreply.github.com>
test(auth): restore sandbox bootstrap coverage
1dec2fc 
Signed-off-by: Drew Newberry <385+drew@users.noreply.github.com>
fix(kubernetes): satisfy ownership test lint
b120f83 
Signed-off-by: Drew Newberry <385+drew@users.noreply.github.com>
drew force-pushed the 2417-delegate-sandbox-auth/drew branch from 3c1450d to b120f83 Compare August 28, 2026 00:43

drew commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator Author

/ok to test b120f83

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

mrunalp Awaiting requested review from mrunalp mrunalp is a code owner
sjenning Awaiting requested review from sjenning sjenning is a code owner
derekwaynecarr Awaiting requested review from derekwaynecarr derekwaynecarr is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

test:e2e Requires end-to-end coverage

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

Back | FazBrowse Home | New Git URL