Skip to content

feat(compute)!: check allowed service ports before creating sandboxes or forwards - #5116

Open
shiju-nv wants to merge 3 commits into
NVIDIA:mainfrom
shiju-nv:feat/5110-service-port-admission/shiju-nv
Open

shiju-nv wants to merge 3 commits into
NVIDIA:mainfrom
shiju-nv:feat/5110-service-port-admission/shiju-nv

Conversation

@shiju-nv

@shiju-nv shiju-nv commented Nov 2, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Check requested service ports against the driver's approved ports before creating a sandbox or opening a local forwarding listener. Keep enforcing the saved ports on gateway routes, and show whether the target uses isolated loopback or shared host loopback that the operator explicitly allowed.

Related Issue

Closes #5110. Part of #5114. Coordinates with service-exposure issue #4310; merged RFC #4267 provides design context.

Rebased onto main after #5204 merged.

Changes

  • Persist the admitted scope and ports, then enforce them for create, exposure, TCP relays and pooled HTTP connections.
  • Preserve authentication, workspace authorization, connection limits and the separate SSH transport path.
  • Carry requested ports and admission through CLI, TUI, templates and supported SDKs, including preflight and replayed responses.
  • Show admission in CLI output and explain when an older sandbox must be recreated.

Release Notes

Source-breaking API addition for the next minor release only. Complete raw Rust messages need defaults for the new port and admission fields. Upgrade gateway writers together. A legacy driver can omit admission; missing evidence from a negotiated modern driver rejects forwarding until the sandbox is recreated with admitted ports. Maintainers must seed and qualify the minor train before landing; do not merge or backport this into the active patch train.

Testing

  • Independent review of the rebase adaptations
  • Rust formatting and diff checks
  • Unit tests added/updated (if applicable)
  • Go forwarding, converter, and fake tests with the race detector; Go bindings regenerated
  • Public/durable schema inventory: merged fingerprint and counts remain unverified pending CI measurement
  • Rust regressions and E2E on the rebased head: pending hosted CI

Local Rust validation was interrupted by disk exhaustion and removal of the shared build cache.

Checklist

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

@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown

Compatibility review (advisory)

New or updated breaking changes; see the full report for all findings.

1. rust-sdk — No explicit risk acceptance found

Previously complete Rust creation-spec and raw-message literals now require new forward_ports or ingress_admission fields.

2. state — No explicit risk acceptance found

Existing sandboxes without status.ingress_admission previously supported forwarding and service routes, but ingress-aware drivers now reject those operations after upgrade.

  • Impact: Upgraded deployments lose existing CLI/SDK TCP forwards and HTTP service endpoints until sandbox recreation, returning admission errors or HTTP 404 instead of connecting.
  • Action: Preserve supported forwarding for retained sandboxes through an upgrade migration in v0.1.7, or defer the recreation-required behavior to a qualified minor train.
  • PR: feat(compute)!: check allowed service ports before creating sandboxes or forwards #5116
Review details
  • Execution: complete
  • Candidate: PR #5116 (0cee2c2049769e7cc76138b13f7167890a80105a)
  • Baseline: c92c2c7983b1307715f6a922366dd3759e41b35b (c92c2c7983b1307715f6a922366dd3759e41b35b)
  • Reviewed: rust-sdk, python-sdk, go-sdk, typescript-sdk, api-behavior, cli, configuration, policy, helm, state

Advisory review; publication gates are unchanged.

Branch check run

@shiju-nv shiju-nv added the test:e2e Requires end-to-end coverage label Nov 2, 2026
@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown

Label test:e2e applied for 441d22c. 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.

@shiju-nv
shiju-nv force-pushed the feat/5110-service-port-admission/shiju-nv branch 2 times, most recently from 0267aa7 to 717c9c6 Compare November 2, 2026 16:59
@shiju-nv
shiju-nv changed the base branch from main to fix/stopped-driver-watch-fence-c92/shiju-nv November 2, 2026 16:59
@shiju-nv
shiju-nv force-pushed the fix/stopped-driver-watch-fence-c92/shiju-nv branch from d104104 to 760396e Compare November 3, 2026 11:45
@shiju-nv
shiju-nv force-pushed the feat/5110-service-port-admission/shiju-nv branch from 717c9c6 to a8d2d7f Compare November 3, 2026 11:47
@shiju-nv shiju-nv changed the title feat(compute)!: admit service ports before provisioning feat(compute)!: check allowed service ports before creating sandboxes or forwards Nov 4, 2026
@pimlock
pimlock deleted the branch NVIDIA:fix/stopped-driver-watch-fence-c92/shiju-nv November 10, 2026 20:25
@pimlock pimlock closed this Nov 10, 2026
@pimlock

pimlock commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator

@shiju-nv FYI this got closed automatically.

It looks like it's because the #5204 was merged, which deletes the branch.
Usually what happens in this case is these stacked PRs would be updated to main, but because this PR is opened from the external fork (shiju-nv/OpenShell) that didn't happen and instead this got closed. I cannot reopen it, I think you'll need to update the base branch and open it? Please let me know if you need help.

A requested service port should be rejected before provisioning a sandbox or opening a local forwarding listener when the driver cannot admit it. This checks create, exposure and forwarding requests against a retained driver admission record, and shows whether the target uses isolated or operator-approved shared-host loopback.

Signed-off-by: Shiju <shiju@nvidia.com>
Signed-off-by: Shiju <shiju@nvidia.com>
@shiju-nv shiju-nv reopened this Nov 10, 2026
@shiju-nv
shiju-nv changed the base branch from fix/stopped-driver-watch-fence-c92/shiju-nv to main November 10, 2026 21:29
@shiju-nv
shiju-nv force-pushed the feat/5110-service-port-admission/shiju-nv branch from 2f477fa to 8410431 Compare November 10, 2026 21:29
Signed-off-by: Shiju <shiju@nvidia.com>
@github-actions

Copy link
Copy Markdown

Compatibility review (advisory)

New or updated breaking changes; see the full report for all findings.

1. typescript-sdk — No explicit risk acceptance found

Generated raw CreateSandboxRequest now requires a forwardPorts array that previously complete message literals did not contain.

  • Impact: Consumers constructing complete objects typed as CreateSandboxRequest fail type-checking, while curated callers and schema-based initialization remain compatible.
  • Action: Preserve raw request source compatibility, or defer the addition to a permitted breaking train with guidance to add forwardPorts: [] or use create(CreateSandboxRequestSchema, init).
  • PR: feat(compute)!: check allowed service ports before creating sandboxes or forwards #5116
Review details
  • Execution: complete
  • Candidate: PR #5116 (a2513619bd307c917c5ba904c670c0b2f2a7e70d)
  • Baseline: bfa68b208b3e8bf816f983326a4f19466dba0572 (bfa68b208b3e8bf816f983326a4f19466dba0572)
  • Reviewed: rust-sdk, python-sdk, go-sdk, typescript-sdk, api-behavior, cli, configuration, policy, helm, state

Advisory review; publication gates are unchanged.

Branch check run

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Check allowed service ports before creating a sandbox or opening a forward

2 participants