Skip to content

fix(gateway): gate unsafe auth deployment modes#1871

Open
alangou wants to merge 1 commit into
NVIDIA:mainfrom
alangou:alangou/os-186-tava-2026-05-fsr-01fsr-15-gate-oidc-auth-only-and-no-auth
Open

fix(gateway): gate unsafe auth deployment modes#1871
alangou wants to merge 1 commit into
NVIDIA:mainfrom
alangou:alangou/os-186-tava-2026-05-fsr-01fsr-15-gate-oidc-auth-only-and-no-auth

Conversation

@alangou

@alangou alangou commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Summary

Gate unsafe gateway authentication postures for shared deployments. OIDC authentication-only mode now requires an explicit opt-in, and gRPC user requests without an auth path fail closed instead of passing through.

Related Issue

Closes OS-186

Changes

  • Added gateway auth posture validation for shared Kubernetes/non-loopback deployments.
  • Added explicit allow_oidc_auth_only config and Helm value for OIDC authentication-only mode.
  • Replaced implicit no-auth gRPC pass-through with UNAUTHENTICATED.
  • Updated Helm validation/rendering so auth-only OIDC renders empty roles only when explicitly opted in.
  • Added Rust and Helm tests for auth-only, partial-role rejection, no-auth fail-closed behavior, and explicit dev/proxy no-auth.
  • Updated Kubernetes, reference, security, Helm, and architecture docs.

Testing

  • mise run pre-commit passes
  • Unit tests added/updated
  • E2E tests added/updated (if applicable)

Additional validation run:

  • CARGO_TARGET_DIR="/home/op/OpenShell/target" mise run test
  • CARGO_TARGET_DIR="/home/op/OpenShell/target" mise exec -- cargo test -p openshell-server
  • mise run helm:test
  • mise run helm:lint
  • mise run helm:docs:check
  • git diff --check

Note: mise run pre-commit could not be confirmed because the shell executor stopped returning statuses.

Checklist

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

@alangou
alangou requested review from a team, derekwaynecarr and mrunalp as code owners June 11, 2026 13:26
@johntmyers

Copy link
Copy Markdown
Collaborator

gator-agent

Blocked

Gator is blocked because GitHub reports this PR has merge conflicts with main (mergeable_state: dirty).

Head SHA: bf0663eb51f92b4d07461ac5fb330c0191666aca

Next action: @alangou needs to rebase or merge main into the PR branch and resolve the conflicts. Gator will re-check after a new commit.

@johntmyers johntmyers added the gator:blocked Gator is blocked by process or repository gates label Jun 23, 2026
@johntmyers

Copy link
Copy Markdown
Collaborator

@alangou bumping this for a rebase so we can re-check

@alangou
alangou force-pushed the alangou/os-186-tava-2026-05-fsr-01fsr-15-gate-oidc-auth-only-and-no-auth branch from bf0663e to 580e58b Compare June 29, 2026 14:12
@alangou
alangou requested a review from maxamillion as a code owner June 29, 2026 14:12
@johntmyers

Copy link
Copy Markdown
Collaborator

gator-agent

PR Review Status

Validation: This PR is project-valid for OpenShell because it tightens gateway authentication posture for shared deployments, updates the Helm and server configuration surfaces, adds matching tests, and updates the affected documentation.
Head SHA: 580e58b1aa9d71d135372bb75b33f653101034e0

Review findings:

  • crates/openshell-core/src/config.rs still treats gateway_jwt as satisfying the shared deployment authentication requirement. In Helm deployments, gateway_jwt is rendered by default, so a Kubernetes gateway with no OIDC, no mTLS user auth, and no explicit trusted-proxy/development opt-in can still start. User APIs then fail closed at request time, but the shared gateway is healthy and unusable, which contradicts the intended startup guardrail. Please change the shared-deployment validation to require a user authentication path (oidc, mtls_auth, or explicit auth.allow_unauthenticated_users) and add a regression test for Kubernetes plus gateway_jwt only.

Docs: Updated in the affected Kubernetes, reference, security, Helm, and architecture docs; no new Fern navigation entry appears necessary.

Next state: gator:in-review

@johntmyers johntmyers added gator:in-review Gator is reviewing or awaiting PR review feedback and removed gator:blocked Gator is blocked by process or repository gates labels Jun 29, 2026
@alangou
alangou force-pushed the alangou/os-186-tava-2026-05-fsr-01fsr-15-gate-oidc-auth-only-and-no-auth branch from 580e58b to 9656e9e Compare June 29, 2026 15:04
@johntmyers

Copy link
Copy Markdown
Collaborator

gator-agent

PR Review Status

Validation: This PR is project-valid for OpenShell because it tightens gateway authentication posture for shared deployments, updates the Helm and server configuration surfaces, adds matching tests, and updates the affected documentation.
Head SHA: 9656e9e496581f178a8cf8a2544271f116f193b6

Review findings:

  • The previous gateway_jwt-only shared-deployment finding appears addressed, but validate_gateway_auth_posture() still misses driver-added non-loopback gateway listeners. It checks config.extra_bind_addresses, while run_server() currently validates before compute initialization and appends state.compute.gateway_bind_addresses() later. A Docker bridge or other driver-provided non-loopback listener can expose the same gRPC surface without triggering the new shared-deployment checks. Please move or repeat validation after compute is built using the effective listener set, and add regression coverage for this path.
  • validate_gateway_auth_posture() treats mtls_auth.enabled as a valid shared auth path even for Kubernetes, while the CLI and architecture docs say Kubernetes mTLS is transport-only and Kubernetes user auth must be OIDC or a trusted fronting proxy. Please enforce that invariant in the shared config validation layer too, so non-CLI callers of run_server() cannot bypass it.

Docs: Updated in the affected Kubernetes, reference, security, Helm, and architecture docs; no new Fern navigation entry appears necessary.

Next state: gator:in-review

@alangou
alangou force-pushed the alangou/os-186-tava-2026-05-fsr-01fsr-15-gate-oidc-auth-only-and-no-auth branch 2 times, most recently from df74473 to 5419723 Compare July 3, 2026 14:09
@alangou
alangou force-pushed the alangou/os-186-tava-2026-05-fsr-01fsr-15-gate-oidc-auth-only-and-no-auth branch 2 times, most recently from 34b67c2 to 541b617 Compare July 22, 2026 14:31
Comment thread crates/openshell-core/src/config.rs

@elezar elezar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, the security behavior looks correct and I did not find another authentication bypass. I have two requested changes before merge:

  • Authentication-only mode still enforces configured scopes, so the repeated statement that "any valid issuer token can call user and admin APIs" is inaccurate. Please clarify that it skips RBAC role checks while scope checks continue to apply.
  • This is an intentional upgrade break: existing OIDC deployments with empty roles will fail Helm rendering or gateway startup. Please add a short migration note telling operators to configure both roles or explicitly enable authentication-only mode before upgrading.

Additional non-blocking hardening opportunities:

  • Warn at startup when allow_oidc_auth_only is active.
  • Reject dormant opt-ins such as allow_oidc_auth_only=true when OIDC is absent or RBAC roles are configured; otherwise a later accidental removal of the roles silently activates the weaker mode.
  • Consider a focused server-boundary test. The current configuration, router, and Helm tests cover the pieces well, but do not exercise their complete wiring.

The second auth-policy boolean also suggests that a small preparatory refactor ("prefactor") could make this change easier to audit. A behavior-preserving commit could introduce internal typed states while retaining the existing serialized configuration:

enum MissingUserPolicy {
    PassThrough,
    Reject,
    DevelopmentUser,
}

enum OidcAuthorization {
    Rbac { admin_role: String, user_role: String },
    AuthenticationOnly,
}

The security commit would then only change PassThrough to Reject and require explicit permission for AuthenticationOnly. A subsequent refactor could replace the external booleans with typed policy fields and resolve them—together with the configured authenticators and effective listener addresses—into a single GatewayAuthPlan consumed by the router.

I would not block this security fix on the broader GatewayAuthPlan redesign, but I recommend doing it before adding another authentication-policy flag.

@alangou
alangou force-pushed the alangou/os-186-tava-2026-05-fsr-01fsr-15-gate-oidc-auth-only-and-no-auth branch from 541b617 to 9c0723e Compare July 22, 2026 15:54
Require explicit opt-in for OIDC authentication-only mode on shared gateway deployments and fail closed when gRPC user requests have no auth path. Align Helm validation, tests, and docs so weak auth modes are intentional and visible.

Signed-off-by: Adrien Langou <alangou@nvidia.com>
@alangou
alangou force-pushed the alangou/os-186-tava-2026-05-fsr-01fsr-15-gate-oidc-auth-only-and-no-auth branch from 9c0723e to 4f0fb3e Compare July 23, 2026 12:14
@alangou

alangou commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @elezar

Addressed the two requested changes:

Clarified that auth-only skips role checks but still enforces configured scopes.
Added an upgrade note for existing OIDC deployments with empty roles.
I also added the startup warning and rejected dormant auth-only opt-ins. I left the server-boundary test and typed auth-plan refactor for follow-up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gator:in-review Gator is reviewing or awaiting PR review feedback

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants