Skip to content

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

Open
drew wants to merge 4 commits into
mainfrom
2417-delegate-sandbox-auth/drew
Open

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

Conversation

@drew

@drew 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
drew requested review from a team, derekwaynecarr, mrunalp and sjenning as code owners August 26, 2026 22:08
@copy-pr-bot

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

Copy link
Copy Markdown

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

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 sjenning added the test:e2e Requires end-to-end coverage label Aug 27, 2026
@github-actions

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

Copy link
Copy Markdown
Collaborator

/ok to test 99956e3

@drew

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

drew commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

/ok to test 94eaa7c

@drew

drew commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

/ok to test bf3b484

drew added 4 commits August 27, 2026 07:55
Signed-off-by: Drew Newberry <385+drew@users.noreply.github.com>
Signed-off-by: Drew Newberry <385+drew@users.noreply.github.com>
Signed-off-by: Drew Newberry <385+drew@users.noreply.github.com>
Signed-off-by: Drew Newberry <385+drew@users.noreply.github.com>
@drew
drew force-pushed the 2417-delegate-sandbox-auth/drew branch from bf3b484 to 3c1450d Compare August 27, 2026 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:e2e Requires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants