perf(server): drop per-connection session tokens for TCP forwards

[n1hility]

Summary

openshell forward service minted an SSH session token before every forwarded TCP connection and revoked it afterwards. The gateway already authenticates the caller and authorizes it against the sandbox's workspace on every ForwardTcp stream before it reads that token, so for TCP targets the token added no authorization while costing two store commits per connection, which is the serial section that kept forward latency growing with concurrency after #3543. TCP forwards are now authorized on the caller principal alone; SSH targets are unchanged.

Related Issue

Fixes #3733. Follow-up to #3494 and #3543.

Changes

• crates/openshell-server/src/grpc/sandbox.rs: acquire_forward_connection_guard admits a ForwardTcp init without authorization_token when the target is tcp; the principal was already authorized by resolve_and_authorize_sandbox_name at MinWorkspaceRole::User, the same role CreateSshSession requires. It reserves only the per-sandbox slot (new acquire_sandbox_connection_slot, shared with the token path; cap constants hoisted) and touches no store. A token-less ssh target is rejected with Unauthenticated("authorization_token is required for SSH targets"). A supplied token is still validated and counted per token, so older CLIs behave as before. The guard's Drop now always releases the sandbox slot.
• crates/openshell-cli/src/run.rs: service_forward_tcp no longer mints or revokes a session per connection. If a gateway answers Unauthenticated with "authorization_token is required" (pre-change gateway), the CLI logs once, falls back to per-connection tokens for the rest of that forward (a per-forward flag, not process state), and revokes them as before. Bridging logic moved unchanged into bridge_local_socket_to_forward_stream.
• architecture/gateway.md (ForwardTcp authorization) and docs/security/best-practices.mdx (service forwards carry no session token; authorized per connection on gateway identity and workspace membership; per-sandbox cap applies; the per-token limit in the defaults table corrected from 10 to the actual 3).
• proto/openshell.proto unchanged: the field is already documented as optional.

Compatibility: new CLI + new gateway, zero store access per forwarded connection; old CLI + new gateway, unchanged; new CLI + old gateway, one rejected first connection then per-connection tokens for that process. Sandbox identities gain no reach: resolve_and_authorize_sandbox_name resolves a sandbox principal only to its own sandbox regardless of token.

Testing

• cargo fmt --check and cargo clippy --all-targets -- -D warnings for openshell-server (with test-support) and openshell-cli: clean on the rebased branch.
• New tests: server tcp_forward_without_token_is_admitted_on_the_principal_alone, ssh_forward_without_token_is_rejected, tcp_forward_with_unknown_token_is_still_rejected, tcp_forwards_without_token_share_the_per_sandbox_connection_cap; CLI unit forward_requires_session_token_matches_only_the_legacy_gateway_error; CLI integration tests against the mock gateway in crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs: service_forward_omits_session_tokens_when_the_gateway_authorizes_the_principal (two connections, token-less tcp inits, data echoed, zero CreateSshSession/RevokeSshSession calls) and service_forward_falls_back_to_session_tokens_once_for_a_legacy_gateway (first connection rejected once and retried with a token, second connection goes straight to token mode, one create and one revoke per connection, data flows).
• Full suites on the branch before the final rebase: cargo test -p openshell-server --features test-support and cargo test -p openshell-cli: 2448 passed, 0 failed; after the rebase and the added integration tests, cargo test -p openshell-cli (all binaries) and the server forward tests rerun green.
• License headers and markdownlint on the changed docs: clean.
• mise run pre-commit / mise run ci: not run locally (no mise on this machine); the equivalent checks above were.
• Unit tests added/updated
• E2E tests: e2e/rust/tests/port_forward.rs drives openshell forward start, which is ssh -L over the ProxyCommand (target.ssh with a session token), so it covers the unchanged SSH branch, not the token-less TCP branch; no existing e2e drives openshell forward service. The token-less path is covered by the server unit tests and the two CLI integration tests above.

Checklist

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

[copy-pr-bot]

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.

[github-actions]

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

[drew]

/ok to test 08a9d81

[drew]

gator-agent

PR Review Status

The full initial code review found no blocking correctness, security, or maintainability issues. The authorization split keeps SSH session tokens on SSH targets, admits TCP targets only after the existing principal and workspace check, preserves the sandbox connection cap, and documents the changed service-forward behavior.

Blocking findings:

• No blocking findings remain

Carried findings:

• None

Non-blocking suggestions:

• None

Gator metadata

• Validation: Project-valid as the focused continuation of the forward-latency work explicitly identified in merged PR #3543; linked issue #3733 documents the user impact, compatibility contract, and acceptance criteria.
• Docs: The authorization behavior is updated in docs/security/best-practices.mdx and architecture/gateway.md; no navigation change is needed for edits to existing pages.
• Checks: Current-head Branch Checks and Helm Lint are running; required aggregate checks are pending.
• E2E: test:e2e applied, copy-PR mirror authorized, and Branch E2E queued for the current head. GPU- and Windows-specific labels are not required by this patch.
• Head SHA: 08a9d81162bcbf48429722f59e2d6a0d2aab4b16
• Base SHA: 4ce767fc0cadad773c398e15109c0286f4b7aa30
• Merge base SHA: a67567e583454021d6665c056d092769136a3edd
• Patch ID: 71206342d8fa71be1f14a6b7a9e87b13f620837c
• Gator payload: 10
• Review mode: initial
• Previous reviewed SHA: none
• Review budget exhausted: no
• Maintainer decision required: no
• Next state: gator:watch-pipeline

[n1hility]

Amended the commit after the initial review (08a9d811 -> f4a67c64, rebased on current main), no change to the gateway authorization logic:

• The CLI's legacy-gateway fallback flag is now owned per service_forward_tcp invocation instead of a process-wide static; identical runtime behaviour, but it makes the fallback testable in-process.
• Added two CLI integration tests against the mock gateway (crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs): token-less TCP forwards issue no CreateSshSession/RevokeSshSession, and a gateway that still requires the token gets one rejected-then-retried connection followed by straight token mode.
• Corrected the per-token connection limit in docs/security/best-practices.mdx (3, not 10) next to the paragraph this PR edits.
• Corrected the Testing section: the e2e port_forward.rs case drives openshell forward start (ssh ProxyCommand, target.ssh), so the token-less TCP path is covered by the unit and integration tests, not by that e2e.

cargo fmt, clippy with -D warnings, the full openshell-cli suite and the server forward tests are green on the new head. It will need a new /ok to test f4a67c64.

[drew]

/ok to test f4a67c6

[drew]

gator-agent

PR Review Status

Thanks @n1hility. I checked the amended author delta you described: the legacy-gateway fallback state is now scoped to one service-forward invocation, the mock-gateway integration tests cover both token-less forwarding and the legacy retry path, and the documented per-token limit matches the gateway constant. The follow-up review found no blocking correctness, security, or maintainability issues.

Blocking findings:

• No blocking findings remain

Carried findings:

• None

Gator metadata

• Validation: Project-valid as the focused implementation of validated issue #3733 and continuation of the forward-latency work in #3543.
• Docs: The service-forward authorization behavior and corrected connection limit are documented; no navigation change is needed for edits to existing pages.
• Checks: Current-head Branch Checks and required aggregate checks are queued or running.
• E2E: test:e2e is applied, the fresh copy-PR mirror was authorized, and Branch E2E is running for the current head. GPU- and Windows-specific labels are not required by this patch.
• Head SHA: f4a67c6442c6ccb95d59383eb9d57bad5b9ffc2f
• Base SHA: 4ce767fc0cadad773c398e15109c0286f4b7aa30
• Merge base SHA: 4ce767fc0cadad773c398e15109c0286f4b7aa30
• Patch ID: 55101e1e8efb386b5a162a5ad88ad504683ec27e
• Gator payload: 10
• Review mode: follow_up
• Previous reviewed SHA: 08a9d81162bcbf48429722f59e2d6a0d2aab4b16
• Review budget exhausted: no
• Maintainer decision required: no
• Next state: gator:watch-pipeline