Skip to content

2707 configurable uid gid range/ebusto - #2737

Open
ebusto wants to merge 9 commits into
NVIDIA:mainfrom
ebusto:2707-configurable-uid-gid-range/ebusto
Open

2707 configurable uid gid range/ebusto#2737
ebusto wants to merge 9 commits into
NVIDIA:mainfrom
ebusto:2707-configurable-uid-gid-range/ebusto

Conversation

@ebusto

@ebusto ebusto commented Aug 13, 2026

Copy link
Copy Markdown

Summary

Operators can lower the sandbox UID/GID minima in gateway.toml so enterprise
system identities below 1000 (for example GID 30 or UID 500) are accepted.
Defaults stay 1000, and root (0) is still rejected.

Related Issue

Fixes #2707

Changes

  • Add [openshell.gateway] min_sandbox_uid and min_sandbox_gid (TOML only;
    default 1000, minimum 1).
  • Thread those limits through policy validation on create, update, get, merge
    retry, draft-chunk approval, and agent proposals.
  • Inject OPENSHELL_MIN_SANDBOX_UID / OPENSHELL_MIN_SANDBOX_GID into Docker,
    Podman, and Kubernetes sandbox environments so the supervisor and OPA engine
    use the same range.
  • Document the keys in docs/reference/gateway-config.mdx.

Testing

  • mise run pre-commit passes
  • Unit tests added/updated
  • E2E tests added/updated (if applicable)
    Unit coverage includes default-min rejection, min=1 accepting GID 30 / UID
    500, supervisor env parsing, OPA from_proto with env limits, and merge /
    agent-proposal paths that previously re-validated against the default 1000
    floor. Farm VM check: gateway with min_sandbox_uid = 1 / min_sandbox_gid = 1
    and a UID/GID 500 user.

Checklist

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

@copy-pr-bot

copy-pr-bot Bot commented Aug 13, 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.

@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown

All contributors have signed the DCO ✍️ ✅
Posted by the DCO Assistant Lite bot.

@ebusto

ebusto commented Aug 13, 2026

Copy link
Copy Markdown
Author

I have read the DCO document and I hereby sign the DCO.

@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: Project-valid implementation of validated issue #2707; the operator-configurable lower identity floor preserves the default of 1000 and rejects root.
Head SHA: b1b4bb32705432815f1cd0f9a1313698dbd942e9
Base SHA: c5498239e66581d5842b50773eff07ace65726f1
Merge base SHA: c5498239e66581d5842b50773eff07ace65726f1
Patch ID: a4928771b3cd59dd91c7902df8e2f7eda9552bd7
Gator payload: 4
Review mode: initial
Previous reviewed SHA: none
Review budget exhausted: no
Maintainer decision required: no

Blocking findings:

  • GATOR-b1b4bb32-01: Kubernetes still resolves and validates its authoritative workload identity against the fixed 1000 floor, so the advertised configured lower minima cannot survive that supported path.
  • GATOR-b1b4bb32-02: VM gateways accept policies using the lowered global minima but do not propagate the minima to the guest supervisor or reject that configuration.

Carried findings:

  • None

Non-blocking suggestions:

  • None

Docs: docs/reference/gateway-config.mdx is updated, but its stated backend coverage exposes the unresolved VM contract described above.

Next state: gator:in-review

Comment thread crates/openshell-driver-kubernetes/src/config.rs
Comment thread crates/openshell-server/src/grpc/sandbox.rs
@drew drew added the gator:in-review Gator is reviewing or awaiting PR review feedback label Aug 13, 2026
@ebusto
ebusto marked this pull request as draft August 14, 2026 17:19
@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 labels Aug 14, 2026
@drew

drew commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

gator-agent

Blocked

Head SHA: af883c40f9e017b3ea4258bbcce7188379848fd5
Gator payload: 4

Gator is blocked because this PR is currently a draft.

Thanks @ebusto — I saw your update that you are taking a pass at VM and Kubernetes support, and the latest head includes changes in both driver paths. The independent follow-up review of those changes will resume once the PR is ready for review.

Next action: @ebusto, mark the PR ready for review when the implementation is ready for another pass.

@ebusto
ebusto marked this pull request as ready for review August 14, 2026 19:30
@ebusto

ebusto commented Aug 14, 2026

Copy link
Copy Markdown
Author

Added support for both the VM and Kubernetes drivers, and tested locally.

@drew drew added test:e2e Requires end-to-end coverage test:e2e-kubernetes Requires Kubernetes end-to-end coverage labels Aug 14, 2026
@github-actions

Copy link
Copy Markdown

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

@github-actions

Copy link
Copy Markdown

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

@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: Project-valid implementation of validated issue #2707; the operator-configurable lower identity floor preserves the default of 1000 and rejects root.
Head SHA: 2d85bfdac97ec5911e219a5a88a1cbcb4fdeb406
Base SHA: c5498239e66581d5842b50773eff07ace65726f1
Merge base SHA: c5498239e66581d5842b50773eff07ace65726f1
Patch ID: 50892fef3f32f3547d6cba711be65c9c375a358d
Gator payload: 4
Review mode: follow_up
Previous reviewed SHA: b1b4bb32705432815f1cd0f9a1313698dbd942e9
Review budget exhausted: no
Maintainer decision required: no

Thanks @ebusto. I checked your VM and Kubernetes update against the two prior identity-propagation findings. The current delta now passes the configured minima into the VM guest, uses the configured minima for Kubernetes identity resolution, and adds render-level coverage proving UID 500/GID 30 survives the Kubernetes path.

Blocking findings:

  • No blocking findings remain.

Carried findings:

  • GATOR-b1b4bb32-01: Resolved by the Kubernetes minima propagation and render regression coverage.
  • GATOR-b1b4bb32-02: Resolved by VM driver argument and guest-environment propagation.

Docs: docs/reference/gateway-config.mdx documents the configured minima and supported backend propagation.

Required tests: test:e2e and test:e2e-kubernetes applied; test-dispatch helper runs are queued, but the actual required workflows are not yet confirmed queued.

Next state: gator:in-review pending required test dispatch.

@drew drew 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 Aug 14, 2026
@drew

drew commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

/ok to test 2d85bfd

@drew drew added gator:watch-pipeline Gator is monitoring PR CI/CD status gator:blocked Gator is blocked by process or repository gates and removed gator:in-review Gator is reviewing or awaiting PR review feedback gator:watch-pipeline Gator is monitoring PR CI/CD status labels Aug 14, 2026
@ebusto

ebusto commented Aug 14, 2026

Copy link
Copy Markdown
Author

Regarding the failing checks, I don't think this has anything to do with my changes, as nothing credentials related was touched. Analysis from my buddies Cursor and Grok:

The failing check is not the 500/30 identity work. provider_credentials_are_stored_in_configured_backend died because the gateway never wrote a Kubernetes Secret.

What failed

mise run e2e:kubernetes:credential-drivers ran one test. It finished in 0.13s:

kubectl -n openshell get secret openshell-cred-3095bae637f808f79fa79ce7624c4bedb6a66ecb
Error from server (NotFound): secrets "..." not found

Gateway RPCs all returned 200:

  1. DeleteProvider (cleanup)
  2. CreateProvider (14ms)
  3. GetProvider
  4. DeleteProvider (teardown)

The gateway pod was 1/1 Running. Startup logs show the Kubernetes compute driver, not kubernetes-secrets. There is no log of a Secret create.

What that means

That test expects CreateProvider to persist OPENAI_API_KEY through the Kubernetes Secrets credential driver. Helm defaults leave that driver off, so credentials go to the encrypted DB store (db-credstore). CreateProvider still succeeds; no openshell-cred-* Secret is created; the assertion fails immediately.

The harness is supposed to layer deploy/helm/openshell/ci/values-credential-driver-kubernetes-secrets.yaml (credentialDrivers.kubernetesSecrets.enabled: true) when OPENSHELL_E2E_CREDENTIAL_DRIVERS=1. The mise task env was applied (the cargo command used --test credential_drivers and the credential-drivers feature), so the overlay should have been passed to Helm. The running gateway still behaved like the default store.

Relation to this PR

The committed UID/GID changes do not touch credential-driver code or the Helm credential templates. This is a credential-storage wiring failure, not an identity-minima failure. The suite never reached the Vault pass; it died on the first (kubernetes-secrets) run.

Most likely cause

The installed gateway.toml did not have an active credential_drivers = ["kubernetes-secrets"] (overlay not applied, or the gateway did not load that list), so it fell back to DB storage. A useful confirm on the next run is the ConfigMap:

kubectl -n openshell get cm openshell-config -o jsonpath='{.data.gateway\.toml}'

You want credential_drivers = ["kubernetes-secrets"] and [openshell.credential_drivers.kubernetes-secrets]. If those are missing, Helm did not take the overlay. If they are present, the gateway ignored them and still used db-credstore.

ebusto added 2 commits August 14, 2026 15:05
Operators can lower min_sandbox_uid and min_sandbox_gid in gateway.toml
so enterprise system IDs such as GID 30 are accepted. Root remains
rejected.

Signed-off-by: Eric Busto <ebusto@nvidia.com>
Signed-off-by: Eric Busto <ebusto@nvidia.com>
ebusto added 2 commits August 14, 2026 15:05
Use configured min_sandbox_uid/gid for driver identity, OpenShift SCC
start IDs, sidecar proxy_uid, and network-init so a lowered range is
enforced in combined and sidecar pods, matching Docker, Podman, and VM.

Testing:
- cargo test -p openshell-driver-kubernetes --lib -- identity OpenShift
  proxy_uid combined_and_sidecar_inject parse_openshift
  validate_sandbox_identity validate_proxy_uid
  resolve_sandbox_uid_uses_openshift
- cargo check -p openshell-driver-kubernetes --bins
- mise run helm:skaffold:dev with min_sandbox_uid/gid = 1 and namespace
  annotations openshift.io/sa.scc.uid-range=500/50000 and
  supplemental-groups=500/50000; sandbox id reported uid=500 gid=500
- mise run helm:skaffold:dev:sidecar with the same mins and annotations;
  agent ran as UID 500, network-init --proxy-gid 500 succeeded

Signed-off-by: Eric Busto <ebusto@nvidia.com>
Add validation, SCC, and combined/sidecar render regressions so a
configured pair below the old 1000 floor is accepted and emitted
instead of falling back to 1000/1000.

Testing:
- cargo test -p openshell-driver-kubernetes --lib --
  validate_sandbox_identity_config_accepts_uid_500
  parse_openshift_supplemental
  combined_and_sidecar_render_resolved_identity_500_30

Signed-off-by: Eric Busto <ebusto@nvidia.com>
@ebusto
ebusto force-pushed the 2707-configurable-uid-gid-range/ebusto branch from 2d85bfd to 61eecb5 Compare August 14, 2026 22:08
@ebusto
ebusto requested a review from sjenning as a code owner August 14, 2026 22:08

@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: Project-valid implementation of validated issue #2707; the operator-configurable lower identity floor preserves the default of 1000, continues to reject root, and is documented for supported backends.
Head SHA: 61eecb5e5d3a221c9cfd272c583785a051e7220f
Base SHA: 44bf0df485f15c2992b04c1d597e6967b4b37bcd
Merge base SHA: 59479f492a63b68d899a6fc284cd53de2b7fdfb6
Patch ID: 77bef87d51f95ed6f4d4c72ff456a7d65f48098f
Gator payload: 4
Review mode: follow_up
Previous reviewed SHA: 2d85bfdac97ec5911e219a5a88a1cbcb4fdeb406
Review budget exhausted: no
Maintainer decision required: no

Thanks @ebusto. I checked the current rebase and author-only range-diff, including the Kubernetes render coverage for UID 500/GID 30. The two prior identity-propagation findings remain resolved, and the independent follow-up review found no new blocking defects.

I also checked your note about the prior credential-driver failure. That run failed because the expected Kubernetes Secret was absent; the reviewed identity-minima delta does not touch credential-driver storage, so I am not treating that failure as a code-review blocker for this PR. The required suites still need to rerun against this head.

Blocking findings:

  • No blocking findings remain.

Carried findings:

  • GATOR-b1b4bb32-01: Resolved; configured minima and resolved UID 500/GID 30 survive the Kubernetes render path.
  • GATOR-b1b4bb32-02: Resolved; VM minima remain propagated into the guest environment.

Docs: docs/reference/gateway-config.mdx documents the new gateway keys, defaults, root prohibition, and backend propagation.

Required tests: test:e2e and test:e2e-kubernetes remain applied, but the copy-PR mirror is still at the previous head and no required workflow is queued for this SHA yet.

Next state: gator:in-review pending current-head test dispatch.

@drew

drew commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

/ok to test 61eecb5

@drew drew added gator:in-review Gator is reviewing or awaiting PR review feedback gator:watch-pipeline Gator is monitoring PR CI/CD status gator:approval-needed Gator completed review; maintainer approval needed gator:blocked Gator is blocked by process or repository gates and removed gator:blocked Gator is blocked by process or repository gates gator:in-review Gator is reviewing or awaiting PR review feedback gator:watch-pipeline Gator is monitoring PR CI/CD status gator:approval-needed Gator completed review; maintainer approval needed labels Aug 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gator:blocked Gator is blocked by process or repository gates test:e2e Requires end-to-end coverage test:e2e-kubernetes Requires Kubernetes end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support low numeric UID and GID values.

2 participants