Conversation
|
🌿 Preview your docs: https://nvidia-preview-pr-3492.docs.buildwithfern.com/openshell |
bc5e9b7 to
830e3ff
Compare
drew
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
This maintainer-authored follow-up to #3439 is project-valid, and its docs cover the new service-authentication behavior. The initial code-only review found one blocking credential-boundary defect.
Action required: keep a gateway bearer token from ever being relayed to the sandbox service after session-cookie authentication, and add regression coverage for the two-request flow.
Blocking findings:
GATOR-830e3ff2-01: a standard gatewayAuthorizationtoken can be forwarded upstream after the new session cookie takes precedence on a later request.
Carried findings:
- None
Non-blocking suggestions:
- None
Gator metadata
- Validation: maintainer-authored gateway security work stacked on #3439 and related to #3402
- Docs: Fern docs and architecture guidance updated
- Checks: current-head branch checks are still running
- E2E: required for gateway service routing; dispatch follows resolution of blocking feedback
- Head SHA:
830e3ff2882300a81d2b375470ddacb9dadb23a6 - Base SHA:
8733df2c8db75379b512062df0129f96937721b3 - Merge base SHA:
8733df2c8db75379b512062df0129f96937721b3 - Patch ID:
d8db2c327f16082f60cce94c9a84b66fea72977a - Gator payload:
9 - Review mode:
initial - Previous reviewed SHA: none
- Review budget exhausted: no
- Maintainer decision required: no
- Next state:
gator:in-review
830e3ff to
93a4160
Compare
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
93a4160 to
c7f47eb
Compare
drew
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Thanks @drew. I checked the retained-cookie fix you described on the new head: the original gateway-token forwarding defect is resolved, and I closed that Gator thread. The follow-up review found one new blocking credential-precedence regression in the remediation.
Action required: preserve application Authorization headers when trusted loopback or mTLS already authenticates the gateway request, while keeping remote bearer authentication ahead of a stale service cookie.
Blocking findings:
GATOR-c7f47ebe-01: trusted loopback and mTLS requests now interpret application bearer credentials as gateway credentials before their established identity is considered.
Carried findings:
- None;
GATOR-830e3ff2-01is resolved.
Gator metadata
- Validation: maintainer-authored gateway security work stacked on merged PR #3439 and related to #3402
- Docs: Fern docs and architecture guidance updated
- Checks: current-head Branch Checks are still running
- E2E: required for gateway service routing; dispatch follows resolution of blocking feedback
- Head SHA:
c7f47ebed1e976fbaf6a71902da78ca44f2fd418 - Base SHA:
1905069948f96921daf88bffb160dceb9ac2a307 - Merge base SHA:
1905069948f96921daf88bffb160dceb9ac2a307 - Patch ID:
6747782f0665829b25497fb7d5325d0e290028ff - Gator payload:
9 - Review mode:
follow_up - Previous reviewed SHA:
830e3ff2882300a81d2b375470ddacb9dadb23a6 - Review budget exhausted: no
- Maintainer decision required: no
- Next state:
gator:in-review
| context: ServiceRequestAuthContext, | ||
| workspace: &str, | ||
| ) -> Result<ServiceRequestAuthorization, ServiceRouteError> { | ||
| let credential = service_request_credential(&headers)?; |
There was a problem hiding this comment.
gator-agent
Warning — GATOR-c7f47ebe-01 · Trusted requests consume application bearer auth
Summary: A local client or mTLS-authenticated user can call a sandbox service that expects its own bearer token. This line parses that application token as a gateway credential before the already-trusted loopback or mTLS identity is considered, so the request is rejected or the application token is stripped. Supported bearer-authenticated services then stop working on those paths.
Fix: Preserve trusted-loopback and mTLS precedence for standard Authorization, while retaining dedicated-header precedence and remote Authorization-before-cookie behavior. Add coverage for both trusted contexts.
Verify: Send Authorization: Bearer application-token with trusted_local=true, then with an enabled mTLS peer. Both requests must authenticate through their trusted context and relay the header unchanged; a remote bearer request with a stale cookie must still consume the gateway token.
Agent context
- Ownership: The remediation moved standard-header parsing ahead of both trusted identity branches.
- Location:
crates/openshell-server/src/service_routing.rs:225
Summary
Require gateway authentication and workspace access for remotely exposed sandbox services while preserving local loopback development. Add a dedicated OpenShell service credential channel so applications can continue using the standard
Authorizationheader.This PR is stacked on #3439.
Related Issue
Related to #3402
Changes
OpenShell-Service-Authorization, standard bearer authorization, mTLS identity, and an exact-host secure session cookieTesting
mise run pre-commitpassesAdditional verification:
cargo test -p openshell-server(1,685 passed, 8 ignored; all integration tests passed)cargo clippy -p openshell-server --all-targets -- -D warningsChecklist