Skip to content

Wire authorization into workspace model - #2445

Merged
derekwaynecarr merged 14 commits into
NVIDIA:mainfrom
derekwaynecarr:decarr/workspace-authz
Jul 30, 2026
Merged

Wire authorization into workspace model#2445
derekwaynecarr merged 14 commits into
NVIDIA:mainfrom
derekwaynecarr:decarr/workspace-authz

Conversation

@derekwaynecarr

@derekwaynecarr derekwaynecarr commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

Implement RFC 0011 Phase 2 workspace-scoped authorization enforcement. Every gRPC method now carries a proto-level AuthorizationRule annotation that declares its auth mode, required scope, and minimum role. A two-layer authorization architecture enforces these rules: middleware validates authentication mode, scope claims, and global roles; handlers call authorize_workspace() to enforce workspace membership and workspace-level roles. Comprehensive e2e tests cover non-member rejection across ~40 RPCs, cross-workspace isolation, workspace admin privilege boundaries, and the OIDC PKCE flow.

Related Issue

Implements Phase 2 of RFC 0011 — Multi-Player Design

Changes

Proto-driven authorization metadata

  • Add AuthorizationRule message in proto/options.proto (extension 50000 on MethodOptions) with fields: auth_mode, workspace_role, global_role, scope
  • Annotate every RPC in openshell.proto and inference.proto with its authorization rule
  • Auth modes: unauthenticated, bearer, sandbox, dual (bearer + sandbox)
  • Workspace roles: user, admin; global role: platform_admin

Descriptor pool authorization loader (descriptor_authz.rs)

  • Replace the #[rpc_authz] proc macro with runtime resolution via prost_reflect::DescriptorPool
  • Build HashMap<String, DescriptorAuthEntry> keyed by gRPC path at startup
  • Fail-closed: error if any method in openshell.v1 or openshell.inference.v1 is missing its annotation

Middleware authorization (authz.rs, method_authz.rs)

  • Rewrite AuthzPolicy::check() to look up proto annotations from the descriptor pool
  • Methods with global_role require the admin OIDC claim; all others require the user claim
  • Admin role implicitly satisfies user role requirements
  • Scope enforcement gated by scopes_enabled flag

Workspace-scoped authorization (workspace_authz.rs)

  • authorize_workspace() — single authorization entry point for all workspace-scoped handlers; checks membership and minimum workspace role, with platform admin bypass
  • authorize_sandbox_workspace() — data-plane variant that reads workspace from the sandbox record
  • require_platform_admin() — cross-workspace operations (CreateWorkspace, DeleteWorkspace, etc.)
  • is_platform_admin_principal() — predicate for conditional logic (e.g. all_workspaces flag)

Handler call sites

  • Sandbox: CreateSandbox, GetSandbox, ListSandboxes, DeleteSandbox, ExecSandbox, ForwardTcp, WatchSandbox — membership + User role; all_workspaces requires platform admin
  • Provider: CreateProvider, UpdateProvider, DeleteProvider, ImportProviderProfiles, UpdateProviderProfiles, DeleteProviderProfile, ConfigureProviderRefresh, RotateProviderCredential, DeleteProviderRefresh — membership + Admin role; ListProviders, GetProvider, ListProviderProfiles — membership + User role
  • Policy: GetSandboxConfig (dual), UpdateConfig (dual — Admin for users, User for sandbox callers with restricted writes), GetDraftPolicy (dual), draft chunk management — membership + Admin role
  • Inference: SetInferenceRoute, DeleteInferenceRoute — membership + Admin role; GetInferenceRoute — membership + User role
  • Workspace: GetWorkspace, ListWorkspaceMembers — membership + User role; AddWorkspaceMember, RemoveWorkspaceMember — membership + Admin role; privilege escalation guard prevents non-platform-admins from assigning admin role
  • ListWorkspaces: Any user with openshell-user OIDC claim can call; handler filters results by membership (platform admins see all, regular users see only workspaces they belong to)

CLI whoami command

  • Add openshell whoami to report the gateway-validated identity (subject, roles, provider)

TUI workspace-scoped provider profiles

  • Fix providers_v2_enabled to always enable workspace-scoped provider profile fetching (was incorrectly reading a non-existent proto field)

OIDC test infrastructure

  • Add e2e/with-keycloak.sh — helper script for running commands against a local Keycloak fixture
  • Add OIDC mode to e2e/with-podman-gateway.sh — conditional mTLS vs OIDC gateway configuration
  • Add audience mappers to both Keycloak clients in scripts/keycloak-realm.json
  • Add e2e-oidc-pkce feature and oidc_pkce test target in e2e/rust/
  • Add e2e/python/oidc/helpers.py — shared OIDC token acquisition and workspace management helpers

E2E test coverage

  • Python workspace_authz_test.py (~1200 lines): parametrized non-member rejection for ~40 RPCs, dual-mode RPC access, ListWorkspaces membership filtering, platform admin bypass, user member read/write boundary, workspace admin operations, privilege escalation guard, all_workspaces rejection for non-admins
  • Rust oidc_pkce.rs (~1500 lines): three-identity scenarios (admin, user, user-b); cross-workspace isolation tests for reads, member management, provider management, sandbox creation, and sandbox listing; workspace admin lifecycle scenarios
  • E2E teardown fix: delete_workspace retries on "still contains resources" to handle async container teardown lag across all runtimes

Testing

  • mise run pre-commit passes
  • Unit tests added/updated — descriptor_authz.rs, workspace_authz.rs, method_authz.rs cover all principal types, role combinations, and edge cases (19 tests)
  • E2E tests added/updated:
    • mise run e2e:oidc-pkce (Podman) — 42 passed
    • mise run e2e:oidc-pkce:docker (Docker) — 42 passed
    • mise run e2e:oidc-python:docker (Docker) — 84 passed

Checklist

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

@derekwaynecarr
derekwaynecarr requested review from a team, maxamillion and mrunalp as code owners July 23, 2026 23:15
@copy-pr-bot

copy-pr-bot Bot commented Jul 23, 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.

@derekwaynecarr
derekwaynecarr marked this pull request as draft July 23, 2026 23:15
@derekwaynecarr
derekwaynecarr force-pushed the decarr/workspace-authz branch from 7b1a5ae to 956cfe9 Compare July 24, 2026 18:20
@derekwaynecarr
derekwaynecarr marked this pull request as ready for review July 24, 2026 18:24
@lbelyaev

Copy link
Copy Markdown

This is the piece we've been waiting on, and the two-layer split — proto-annotated middleware for mode/scope/global-role, authorize_workspace() for membership and workspace role. Three notes from running a workspace-scoped authorization model in production, two concrete and one structural.

  1. Membership + role gates who can act, but there's still no workspace-scoped deny. Every call site here resolves to an allow decision: a member with the required role passes, everyone else is rejected. What a Workspace Admin still can't express is a scoped deny — "this workspace may not reach endpoint X," composed under the gateway default rather than replacing it. In our experience that gap is the one enterprises hit first, because their baseline is a deny list they need to hold regardless of what a member is otherwise allowed to do. Phase 2 is the natural place to decide whether that's in scope or explicitly deferred, since the annotation model would have to carry it. Right now it's neither expressible nor called out.

  2. Membership records are keyed by name, so name reuse can rebind authority. authorize_workspace() resolves membership via get_message_by_name(&workspace, &subject), and the id UUID on ObjectMeta isn't part of the authorization key. That means deleting a workspace and later creating a new one with the same name inherits any membership records that were keyed to the old name — a member of the old workspace is silently a member of the new one. This is the follow-up flagged in feat(workspace): add workspace resource model with scoping, membershi… #2243 ("worth revisiting before Phase 2 makes membership records authorization data") — Phase 2 is that moment. Tying the membership record to the workspace's UUID rather than its name solves it.

  3. On dual-mode writes: for UpdateConfig, a sandbox caller authorizes at User and passes through validate_sandbox_caller_update, while a human authorizes at Admin. That split is right. One structural note: validate_sandbox_caller_update is a denylist — it rejects global, delete_setting, empty name, and non-policy-sync shapes, then falls through to Ok. That means a field added to UpdateConfigRequest later is sandbox-writable by default, unless whoever adds it remembers to extend this guard. We've found the inverse — an allowlist of the fields a sandbox caller may touch, defaulting new fields to admin-only — holds up better as the request surface grows, since the failure mode is a rejected write rather than a silently-permitted one.

@drew drew left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

Validation: This maintainer-authored PR is project-valid for OpenShell because it implements RFC 0011 Phase 2 workspace-scoped authorization.
Head SHA: a7878d43325a2f6a323c844f3d8b48f688e870eb

Review findings:

  • Three warning-level findings require author follow-up before gator can move this to pipeline watch.

Docs: Fern docs were updated for the direct workspace/auth UX changes, and folder navigation should pick up the new workspace page.

Next state: gator:in-review

Comment thread e2e/with-docker-gateway.sh
Comment thread crates/openshell-server/src/grpc/workspace.rs Outdated
Comment thread crates/openshell-server/src/auth/descriptor_authz.rs
@drew

drew commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

/ok to test a7878d4

@drew drew added the gator:in-review Gator is reviewing or awaiting PR review feedback label Jul 28, 2026
@derekwaynecarr
derekwaynecarr force-pushed the decarr/workspace-authz branch from a7878d4 to 31c749a Compare July 28, 2026 17:40
@derekwaynecarr

Copy link
Copy Markdown
Collaborator Author

/ok to test 31c749a

@drew drew added the test:e2e Requires end-to-end coverage label Jul 28, 2026
@github-actions

Copy link
Copy Markdown

Label test:e2e applied for 31c749a. 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 and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

@drew

drew commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

gator-agent

PR Review Status

Validation: This maintainer-authored PR is project-valid for OpenShell because it implements RFC 0011 Phase 2 workspace-scoped authorization and updates the related CLI/docs/test surfaces.
Head SHA: 31c749a4903455bb0fb2bde0ca3e0a4f2ee53333

Thanks @derekwaynecarr. I re-checked the fixes you noted on July 28 for descriptor authorization validation and ListWorkspaces pagination; both are resolved on this head. The prior Docker OIDC readiness issue is also resolved by the /healthz readiness path.

Review findings:

  • No blocking code-review findings remain from the independent principal-engineer review.

Docs: Fern docs were updated for the direct workspace/auth UX changes, including the workspace access page and whoami references. The sandboxes docs folder is navigation-driven, so the new page is picked up by frontmatter ordering.

Checks: OpenShell / Branch Checks is currently failing because cargo fmt --all -- --check reports formatting diffs in crates/openshell-server/src/auth/descriptor_authz.rs in both Rust jobs.

E2E: I applied test:e2e because this PR changes authorization, policy, and sandbox-visible gateway behavior. The E2E label help workflow is still pending.

Next action: @derekwaynecarr, please format crates/openshell-server/src/auth/descriptor_authz.rs and push an update so gator can re-check the new head.

Next state: gator:in-review

@derekwaynecarr
derekwaynecarr force-pushed the decarr/workspace-authz branch from 31c749a to 9a7deb9 Compare July 28, 2026 18:45
@derekwaynecarr

Copy link
Copy Markdown
Collaborator Author

/ok to test 9a7deb9

@drew drew left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

Validation: This maintainer-authored PR is project-valid for OpenShell because it implements RFC 0011 Phase 2 workspace-scoped authorization and updates the related CLI/docs/test surfaces.
Head SHA: 9a7deb96b6c11e438cbe669508e9ad313de272bb

Review findings:

  • One critical no-auth gateway compatibility regression and one warning-level resource-consumption issue require author follow-up before gator can move this to pipeline watch.

Docs: Fern docs were updated for the direct workspace/auth UX changes, including the new workspace access page under the existing sandboxes navigation folder.

Checks: Required checks are still pending on this head, and test:e2e is applied. Code review findings are currently blocking pipeline handoff.

Next action: @derekwaynecarr, please address the inline gator findings and push an update so gator can re-review the new head.

Next state: gator:in-review

Comment thread crates/openshell-server/src/grpc/mod.rs
Comment thread crates/openshell-server/src/grpc/workspace.rs Outdated
@derekwaynecarr

Copy link
Copy Markdown
Collaborator Author

/ok to test 6bf2335

@drew drew left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

Validation: This maintainer-authored PR is project-valid for OpenShell because it implements RFC 0011 Phase 2 workspace-scoped authorization and updates the related CLI, docs, and E2E surfaces.
Head SHA: 6bf2335cddead5a802aeb87034fa0a1eef9142ae

Thanks @derekwaynecarr. I re-reviewed the latest head after your updates. The descriptor validation, ListWorkspaces pagination correctness, Docker OIDC readiness, and formatting issues from earlier gator passes are no longer the gating items, but the no-auth gateway compatibility issue still remains and the current diff has two additional authorization/resource concerns.

Review findings:

  • One critical finding and two warning-level findings require author follow-up before gator can move this to pipeline watch.

Docs: Fern docs were updated for the direct workspace/auth UX changes, including workspace access and whoami coverage.

Checks: OpenShell / Branch Checks and OpenShell / Helm Lint are passing on this head. OpenShell / E2E is still pending with test:e2e applied.

Next action: @derekwaynecarr, please address the inline gator findings and push an update so gator can re-review the new head.

Next state: gator:in-review

Comment thread crates/openshell-server/src/grpc/mod.rs
Comment thread crates/openshell-server/src/grpc/workspace.rs Outdated
Comment thread crates/openshell-server/src/grpc/sandbox.rs Outdated

@drew drew left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

Validation: This maintainer-authored PR remains project-valid for OpenShell because it implements RFC 0011 Phase 2 workspace-scoped authorization and updates the related CLI, docs, and E2E surfaces.
Head SHA: 372d046ddd04b806d4bf0f4d3b1369b4cff7d745

Thanks @derekwaynecarr. I re-reviewed the latest head after your July 28 update. The no-auth gateway compatibility regression, sandbox workspace-enumeration ordering, and unbounded ListWorkspaces membership query from the prior gator pass are improved or resolved on this head. Two warning-level issues still need follow-up before gator can move this to pipeline watch.

Review findings:

  • Two warning-level findings require author follow-up before gator can move this to pipeline watch.

Docs: Fern docs and the openshell-cli skill were updated for the direct workspace/auth and whoami UX changes.

Checks: OpenShell / Branch Checks, OpenShell / Helm Lint, and OpenShell / E2E are currently waiting for the copy-pr /ok to test mirror on this head. test:e2e remains applied because this PR changes authorization, policy, and sandbox-visible gateway behavior.

Next action: @derekwaynecarr, please address the inline gator findings and push an update so gator can re-review the new head.

Next state: gator:in-review

Comment thread crates/openshell-server/src/grpc/provider.rs Outdated
Comment thread crates/openshell-server/src/grpc/workspace.rs Outdated
@drew

drew commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

/ok to test 372d046

@derekwaynecarr

Copy link
Copy Markdown
Collaborator Author

/ok to test cf2601f

@drew drew left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

Validation: This maintainer-authored PR remains project-valid for OpenShell because it implements RFC 0011 Phase 2 workspace-scoped authorization and updates the related CLI, docs, and E2E surfaces.
Head SHA: cf2601fe3fa1ea7416edd686726154690b3507f6

Thanks @derekwaynecarr. I re-reviewed the latest head after your July 28 updates. The prior no-auth gateway compatibility issue, sandbox workspace-enumeration ordering, and several earlier authorization/pagination findings look improved or resolved on this head. Three warning-level issues still need follow-up before gator can move this to pipeline watch.

Review findings:

  • Three warning-level findings require author follow-up before gator can move this to pipeline watch.

Docs: Fern docs and the openshell-cli skill were updated for the direct workspace/auth and whoami UX changes.

Checks: OpenShell / Branch Checks and OpenShell / E2E are currently pending on this head, while OpenShell / Helm Lint is passing. test:e2e remains applied because this PR changes authorization, policy, and sandbox-visible gateway behavior.

Next action: @derekwaynecarr, please address the inline gator findings and push an update so gator can re-review the new head.

Next state: gator:in-review

Comment thread crates/openshell-server/src/grpc/provider.rs Outdated
Comment thread crates/openshell-server/src/inference.rs Outdated
Comment thread crates/openshell-server/src/persistence/sqlite.rs Outdated
@drew drew added gator:blocked Gator is blocked by process or repository gates and removed gator:in-review Gator is reviewing or awaiting PR review feedback gator:blocked Gator is blocked by process or repository gates labels Jul 29, 2026
- Fix TUI unconditionally setting providers_v2_enabled after provider
  refresh; read the actual gateway setting via GetGatewayConfig at
  startup instead
- Fix SQLite json_extract with dotted label keys (e.g. example.com/env)
  by quoting the key in the JSON path
- Add authed_request wrappers to upstream OCI identity tests that were
  missing a principal after rebase
- Add test proving GetGatewayConfig is accessible without Platform Admin
- Add test for dotted/prefixed Kubernetes-style label key filtering

Signed-off-by: Derek Carr <decarr@redhat.com>
@derekwaynecarr

Copy link
Copy Markdown
Collaborator Author

/ok to test 8083f58

@drew drew left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

Validation: This maintainer-authored PR remains project-valid for OpenShell because it implements RFC 0011 Phase 2 workspace-scoped authorization and updates the related CLI, TUI, docs, and E2E surfaces.
Head SHA: 8083f58d6f7283e866ff1c9be7fd05469796450a

Thanks @derekwaynecarr. I checked your note that handle_get_gateway_config does not itself require Platform Admin and reviewed the new handler-level test. The handler is unguarded, but the production descriptor marks GetGatewayConfig with global_role: platform_admin, and AuthzPolicy enforces that role before the request reaches the handler. The test therefore bypasses the gate it is intended to cover.

The prior workspace-oracle, membership-pagination, dotted-label-key, and unconditional TUI provider-mode findings are resolved or improved on this head. The independent re-review found two critical issues and one warning-level issue that still need author follow-up; see the inline comments.

General finding:

  • Warning: .agents/skills/openshell-cli/SKILL.md now mentions whoami but does not document workspace membership and role workflows, while .agents/skills/tui-development/SKILL.md still describes the older gateway/sandbox/log-only data flow. Please update both skills for the new workspace authorization, provider-capability refresh, and expected permission-denial behavior.

Docs: Fern documentation covers workspace roles and membership, whoami, authorization failures, and platform-only operations. The new workspace page participates in the existing folder-based navigation.

Checks: OpenShell / Branch Checks and OpenShell / Helm Lint are passing. OpenShell / E2E is pending with test:e2e applied.

Next action: @derekwaynecarr, please address the inline findings and skill drift, then push an update so gator can review the new head.

Next state: gator:in-review

Comment thread crates/openshell-cli/src/run.rs
Comment thread crates/openshell-server/src/persistence/sqlite.rs Outdated
Comment thread crates/openshell-tui/src/lib.rs
- Loosen GetGatewayConfig from platform_admin to scope-only so workspace
  users can discover providers_v2_enabled during sandbox creation with
  inferred-provider commands; update proto descriptor, descriptor
  validation, and RFC 0011 access table
- Add validate_label_selector to handle_list_workspaces and escape
  single quotes in SQLite json_extract interpolation (CWE-89
  defense-in-depth)
- Re-fetch providers_v2_enabled after TUI gateway switch so the new
  gateway's capability is reflected
- Add e2e test for workspace user with inferred-provider command
- Add persistence test for adversarial label keys with SQL injection
  attempts
- Add handler test for invalid label selector rejection in
  ListWorkspaces

Signed-off-by: Derek Carr <decarr@redhat.com>
@derekwaynecarr

Copy link
Copy Markdown
Collaborator Author

/ok to test 3b5a656

@drew drew left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

Validation: This maintainer-authored PR remains project-valid because it implements RFC 0011 Phase 2 workspace-scoped authorization and updates the related CLI, TUI, docs, and E2E surfaces.
Head SHA: 3b5a65639fb468d584ed6fa6064d86b6890ee02e

Thanks @derekwaynecarr. I reviewed the new head after your latest authorization, SQLite selector, TUI capability-refresh, and E2E updates. The prior Platform Admin gate on GetGatewayConfig, selector quote handling, and gateway-switch refresh concerns are improved, but the independent review found five line-specific warnings and two general warnings that still need follow-up.

General findings:

  • Warning (CWE-400): crates/openshell-server/src/grpc/validation.rs:635 and crates/openshell-server/src/persistence/sqlite.rs:537 bound individual label keys and values but not total selector bytes or pair count. SQLite emits one predicate and bind variable per pair. Cap selector size and pair count before persistence and test the boundary.
  • Warning: .agents/skills/openshell-cli/cli-reference.md:29 still omits whoami from its Complete Command Tree, while .agents/skills/tui-development/SKILL.md:343 and its gateway-switch lifecycle still describe the older sandbox/log-only architecture. Update those companion references for workspace/provider/settings refresh and the new capability fetch.

Docs: Fern docs cover workspace roles, membership, whoami, authorization failures, and platform-only operations. The new page participates in folder-based navigation. The companion agent references above remain stale.

Checks: OpenShell / Branch Checks and OpenShell / E2E are pending. OpenShell / Helm Lint and the non-applicable GPU gate are green. Code-review findings currently block pipeline handoff.

Next action: @derekwaynecarr, please address the inline and general findings and push an update so gator can review the new head.

Next state: gator:in-review

Comment thread crates/openshell-server/src/auth/descriptor_authz.rs Outdated
Comment thread proto/openshell.proto
Comment thread crates/openshell-tui/src/lib.rs Outdated
Comment thread crates/openshell-tui/src/lib.rs
Comment thread crates/openshell-server/src/grpc/sandbox.rs Outdated
- Cap label selector pairs at 64 (CWE-400) to bound SQLite dynamic SQL
- Add SCOPE_ONLY_METHODS allowlist for scope-without-role RPCs (CWE-863)
- Normalize ID-based data-plane handlers to return NOT_FOUND for
  unauthorized sandboxes, closing the cross-workspace oracle (CWE-203)
- Fix TUI provider profile cache lookup key mismatch for legacy
  providers with empty profile_workspace
- Add whoami to CLI skill reference command tree
- Update TUI skill doc with workspace, provider, and settings coverage
- Document scope/workspace orthogonality on GetGatewayConfig proto

Signed-off-by: Derek Carr <decarr@redhat.com>
@derekwaynecarr

Copy link
Copy Markdown
Collaborator Author

/ok to test d8f54e0

@drew drew left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

Validation: This maintainer-authored PR remains project-valid because it implements RFC 0011 Phase 2 workspace-scoped authorization and updates the related CLI, TUI, Fern docs, and E2E surfaces.
Head SHA: d8f54e0ff8acc0dfdbfec6b3329f2c8fc20875e0

Thanks @derekwaynecarr. I checked the three prior threads you marked resolved on this head. The broader Platform Admin gate, descriptor validation, SQLite selector, provider-cache, gateway-refresh, docs, and agent-reference issues are fixed, but three narrower behaviors remain unresolved.

Review findings:

  • Three warning-level findings require follow-up; see the inline comments.

Docs: Fern documentation covers whoami, workspace membership and roles, authorization failures, platform-only operations, and workspace-scoped workflows. The new workspace page participates in the existing folder-driven navigation.

Checks: OpenShell / Branch Checks and OpenShell / E2E are pending on this head. OpenShell / Helm Lint is green, and test:e2e remains applied.

Next action: @derekwaynecarr, please address the inline findings and push an update so gator can re-review the new head.

Next state: gator:in-review

Comment thread proto/openshell.proto
Comment thread crates/openshell-server/src/grpc/policy.rs Outdated
Comment thread crates/openshell-tui/src/lib.rs
GetSandboxConfig and GetSandboxLogs in policy.rs had the same
fetch-before-authorize pattern that leaked cross-workspace sandbox
existence. Promote fetch_and_authorize_sandbox to pub(super) and
use it from both sandbox.rs and policy.rs handlers.

Signed-off-by: Derek Carr <decarr@redhat.com>
@derekwaynecarr

Copy link
Copy Markdown
Collaborator Author

/ok to test 542b259

Cross-workspace sandbox access via ID-based handlers now returns
NOT_FOUND instead of PERMISSION_DENIED to prevent existence inference.
Update the unit test and OIDC e2e assertion to match.

Signed-off-by: Derek Carr <decarr@redhat.com>
@derekwaynecarr

Copy link
Copy Markdown
Collaborator Author

/ok to test baf9ced

@drew drew left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

Validation: This maintainer-authored PR remains project-valid because it implements RFC 0011 Phase 2 workspace-scoped authorization and updates the related CLI, TUI, Fern docs, agent references, and E2E surfaces.
Head SHA: 542b25917ed58eba10fbade4b5f24ad1f3291e43

Thanks @derekwaynecarr. I honored your current-head maintainer dispositions: the GetGatewayConfig scope behavior remains accepted as pre-existing design, and the separate global-settings read-only rendering concern remains waived as follow-up UX polish. I checked the latest diff and complete durable-feedback ledger; the findings below are distinct new regressions or newly delegated trust-boundary risks.

Review findings:

  • Two critical CWE-918 findings and two warning-level correctness findings require author follow-up; see the inline comments.

General findings:

  • Warning: crates/openshell-tui/src/app.rs:1151, crates/openshell-tui/src/lib.rs:2101, and crates/openshell-tui/src/lib.rs:2481 still cycle every user into an all workspaces view, but this PR restricts those list calls to Platform Admins. On denial the refreshes retain the prior workspace rows while the title says all. Suppress that option for non-admins, or revert and clear state on denial, and cover an OIDC Workspace User cycle.
  • Warning: docs/sandboxes/inference-routing.mdx:20,28, .agents/skills/openshell-cli/SKILL.md:509, and .agents/skills/debug-inference/SKILL.md:45 still describe gateway-wide inference and omit workspace selection plus the User-read/Admin-write boundary. Update the Fern page and both maintenance-map companion skills with workspace-scoped commands and denial diagnostics.

Docs: The new workspace Fern page is discoverable through existing folder navigation, but the inference documentation and companion skills above remain stale.

Checks: OpenShell / Branch Checks and OpenShell / E2E are still pending on this head; DCO and OpenShell / Helm Lint are green. Code-review findings currently block pipeline handoff.

Next action: @derekwaynecarr, please address the inline and general findings and push an update so gator can review the new head.

Next state: gator:in-review

Comment thread crates/openshell-server/src/inference.rs
Comment thread crates/openshell-server/src/grpc/provider.rs
Comment thread crates/openshell-server/src/grpc/sandbox.rs Outdated
Comment thread e2e/python/oidc/workspace_authz_test.py Outdated
Comment thread .agents/skills/openshell-cli/cli-reference.md Outdated
…mats

Only remap PERMISSION_DENIED to NOT_FOUND in fetch_and_authorize_sandbox
and RevokeSshSession, letting INTERNAL and UNAUTHENTICATED propagate
as-is. Fix whoami --output format values in cli-reference.md to match
the actual CLI (table/json/yaml, not text/json).

Signed-off-by: Derek Carr <decarr@redhat.com>
@derekwaynecarr

Copy link
Copy Markdown
Collaborator Author

/ok to test 595195c

@drew drew left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

Validation: This maintainer-authored PR remains project-valid because it implements RFC 0011 Phase 2 workspace-scoped authorization and updates the related CLI, TUI, Fern docs, agent references, and E2E surfaces.
Head SHA: 595195c70bc848f4bace816bfaad2679b009d051

Thanks @derekwaynecarr. I checked your latest head after the sandbox error-mapping, whoami reference, and cross-workspace log-test updates. Those prior findings are resolved, and the independent review found no critical issues. One line-specific authentication-test warning and two general warnings still need follow-up.

General findings:

  • Warning: crates/openshell-tui/src/app.rs:1151, crates/openshell-tui/src/lib.rs:2101, and crates/openshell-tui/src/lib.rs:2481 still let every user cycle into all workspaces, although those list requests now require Platform Admin. On denial, the title changes to all while prior workspace rows remain displayed. Suppress this option for non-admins, or revert the selection and clear stale rows on denial, and add Workspace User coverage.
  • Warning: docs/sandboxes/inference-routing.mdx:20, .agents/skills/debug-inference/SKILL.md:18, and .agents/skills/openshell-cli/SKILL.md:509 still describe inference routing as gateway-wide. Update them with workspace selection, the Workspace User read/Workspace Admin write boundary, and permission-denial diagnostics.

Docs: Workspace and whoami Fern documentation is present and folder navigation needs no docs/index.yml change, but the inference-routing documentation and companion skills above remain stale.

Checks: OpenShell / Branch Checks, OpenShell / Helm Lint, and OpenShell / E2E are pending on this head. test:e2e remains applied.

Next action: @derekwaynecarr, please address the inline and general findings and push an update so gator can re-review the new head.

Next state: gator:in-review

Comment thread e2e/python/oidc/oidc_auth_test.py
@derekwaynecarr

Copy link
Copy Markdown
Collaborator Author

/ok to test e492518

@drew drew left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

Validation: This maintainer-authored PR remains project-valid because it implements RFC 0011 Phase 2 workspace-scoped authorization and updates the related CLI, TUI, Fern docs, agent references, and E2E surfaces.
Head SHA: e492518c89c3f32f7032d4627a4be439b9bcd721

Thanks @derekwaynecarr. I carried forward your July 29 maintainer dispositions on the existing endpoint-fetch hardening concerns. The only change since the last gator-reviewed head is the Keycloak CI network-namespace fix, which does not materially change those code paths, so those waived findings remain suppressed.

Review findings:

  • Two warning-level findings require follow-up; see the inline comments.

Docs: Fern documentation covers whoami, workspace membership and roles, authorization failures, platform-only operations, and workspace-scoped workflows. The new workspace page participates in the existing folder-driven navigation.

Checks: OpenShell / Branch Checks and OpenShell / E2E are pending on this head. OpenShell / Helm Lint is green, and test:e2e remains applied.

Next action: @derekwaynecarr, please address the inline findings and push an update so gator can re-review the new head.

Next state: gator:in-review

Comment thread e2e/python/oidc/oidc_auth_test.py
Comment thread crates/openshell-server/src/grpc/workspace.rs
In GitHub Actions job containers, Docker port publishing lands on the
host, not inside the job container. Detect this environment and attach
Keycloak to the job container's network namespace instead, with
hardened defaults (cap-drop ALL, no-new-privileges, loopback-only
listener).

Signed-off-by: Derek Carr <decarr@redhat.com>
@derekwaynecarr
derekwaynecarr force-pushed the decarr/workspace-authz branch from e492518 to 0e43f1c Compare July 29, 2026 22:38
@derekwaynecarr

Copy link
Copy Markdown
Collaborator Author

/ok to test 0e43f1c

@drew drew left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

gator-agent

PR Review Status

Validation: This maintainer-authored PR remains project-valid because it implements RFC 0011 Phase 2 workspace-scoped authorization and updates the related CLI, TUI, Fern docs, agent references, and E2E surfaces.
Head SHA: 0e43f1ce9cd9c9e3ad019f3da138c0c4c654a116

Thanks @derekwaynecarr. I reviewed the latest Keycloak network-namespace CI fix and carried forward the complete durable-feedback ledger. That focused CI change does not resolve the two outstanding authentication/input-boundary warnings from the prior head. The independent review also found one distinct SSH-session existence discrepancy; see the inline comments.

Review findings:

  • Three warning-level findings require follow-up; see the inline comments.

Docs: Fern documentation covers whoami, workspace membership and roles, authorization failures, platform-only operations, and workspace-scoped workflows. The companion CLI/TUI agent references are updated, and the existing folder-driven navigation picks up the new workspace page.

Checks: OpenShell / Branch Checks, OpenShell / Helm Lint, and OpenShell / GPU E2E are green. OpenShell / E2E is pending with test:e2e applied. Code-review findings currently block pipeline handoff.

Next action: @derekwaynecarr, please address the inline findings and push an update so gator can review the new head.

Next state: gator:in-review

Comment thread e2e/python/oidc/oidc_auth_test.py
Comment thread crates/openshell-server/src/grpc/workspace.rs
Comment thread crates/openshell-server/src/grpc/sandbox.rs
@derekwaynecarr
derekwaynecarr enabled auto-merge July 30, 2026 00:26
@derekwaynecarr
derekwaynecarr added this pull request to the merge queue Jul 30, 2026
Merged via the queue into NVIDIA:main with commit 9c019a9 Jul 30, 2026
122 of 127 checks passed
@derekwaynecarr
derekwaynecarr deleted the decarr/workspace-authz branch July 30, 2026 00:42
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 test:e2e Requires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants