feat(credentials): add provider credential storage drivers by sjenning · Pull Request #2437 · NVIDIA/OpenShell

sjenning · 2026-07-23T03:33:42Z

Superseding #1986 while @TaylorMutch is away

Summary

Adds gateway-owned credential storage for Providers v2. New provider credential
writes now persist opaque handles instead of inline secret values, with default
encrypted database storage when no external credential backend is configured and
opt-in Kubernetes Secrets or Vault backends for deployments that want dedicated
secret storage.

Related Issue

Related to #1931

Changes

Adds a credential driver protobuf contract for store, resolve, delete, list,
and capability calls.
Adds gateway credential runtime support for a single active external
credential driver, in-tree drivers, and UDS-connected external drivers.
Adds the default encrypted database credential store as
openshell-driver-db-credstore.
Adds standalone UDS-capable openshell-driver-kubernetes-secrets and
openshell-driver-vault binaries, plus in-tree gateway loading.
Updates Providers v2 create/update/delete paths to store new credential values
through credential storage and persist internal credential_handles.
Preserves upgrade compatibility for existing inline/plaintext provider records:
legacy inline credentials remain readable, config-only updates leave them
inline, and only newly submitted/rotated credential values move into credential
storage.
Rejects user-supplied provider.credential_handles on provider create/update.
Resolves credential handles at runtime for sandbox provider environments,
provider refresh, and managed inference routes.
Updates Helm defaults so no external driver means default encrypted DB
credential storage, backed by a retained key-encryption-key Secret injected
into the gateway.
Adds Helm/Skaffold values and targeted e2e coverage for Kubernetes Secrets and
Vault credential storage backends. The local Vault e2e path deploys OpenBao as
a Vault-compatible fixture.
Updates gateway/provider docs, architecture notes, CI labels, and local test
tasks for credential driver validation.

Default Behavior

Credential drivers are opt-in for external backends. If
[openshell.gateway].credential_drivers is omitted, the gateway uses the default
encrypted database credential store. The submitted secret is encrypted into a
driver-owned credential object outside the provider record, and the provider
record stores only an opaque handle.

The explicit empty list form, credential_drivers = [], is invalid. Operators
should omit the field for default encrypted DB storage or select exactly one
external backend such as kubernetes-secrets or vault.

Existing provider records that already contain inline plaintext credentials are
still resolved for upgrade compatibility, but new provider creates and credential
updates use credential storage handles.

Testing

cargo check -p openshell-driver-vault -p openshell-server
mise run pre-commit
mise run helm:test
mise run e2e:kubernetes:credential-drivers
mise run e2e:rust
mise run e2e:podman - provider/default credential coverage passed before
the suite failed in sandbox_create_with_no_keep_cleans_up_after_tty_command;
isolated rerun of OPENSHELL_E2E_PODMAN_TEST=sandbox_lifecycle mise run e2e:podman reproduced the same Podman cleanup failure.

Checklist

Follows Conventional Commits
Commits are signed off (DCO)
Architecture docs updated
Gateway configuration docs updated

copy-pr-bot · 2026-07-23T03:33:45Z

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.

sjenning · 2026-07-23T03:35:20Z

Obvious issue left over from the rebase in ‎.github/workflows/branch-e2e.yml. Will fix tomorrow.

sjenning · 2026-07-23T14:29:14Z

Obvious issue left over from the rebase in ‎.github/workflows/branch-e2e.yml

Fixed

sjenning · 2026-07-23T16:31:41Z

Hold on this. It is all messed up.

mrunalp · 2026-07-24T17:43:49Z

/ok to test d1e10d1

mrunalp · 2026-07-28T18:27:29Z

/ok to test 052540f

mrunalp · 2026-07-28T21:42:49Z

/ok to test 1806c9b

johntmyers

gator-agent

PR Review Status

Validation: This is project-valid work for roadmap issue #1931 and implements its gateway-owned credential-storage scope.
Head SHA: 1806c9be07e0307137ac7c0b8cced209018e81fb
Base SHA: b1c7ff684e9b16f88d050c29b0093d07389fbbbb
Merge base SHA: b1c7ff684e9b16f88d050c29b0093d07389fbbbb
Patch ID: 8c7fdb202e6f6004ee9069a12d24c5bb02ffb8c7
Gator payload: 2
Review mode: initial
Previous reviewed SHA: none

Blocking findings:

GATOR-1806c9be-01 (Critical): backend object identity is not workspace/provider-unique, allowing a cross-workspace Vault credential collision and disclosure.
GATOR-1806c9be-02 (Warning): handle-backed credentials bypass persisted provider expiration.
GATOR-1806c9be-03 (Warning): refresh overwrites the committed backend object before validation/CAS and can delete the still-referenced handle on failure.
GATOR-1806c9be-05 (Warning): the configured UDS startup timeout does not bound capability negotiation or runtime RPCs.

Carried findings:

None.

Non-blocking suggestions:

Add openshell-driver-db-credstore to the AGENTS.md architecture inventory and document the new credential-driver Skaffold profiles in the Helm development skill.

Docs: Relevant published provider and gateway-configuration docs are updated.

Next state: gator:in-review

sjenning · 2026-07-31T19:40:18Z

@johntmyers thanks for the review! All review feedback should be addressed by the new commits. I also rebased to resolve a conflict.

Apply configured timeouts to both startup capability negotiation and runtime RPCs to prevent indefinite hangs (GATOR-1806c9be-05). - Add DEFAULT_CREDENTIAL_DRIVER_RPC_TIMEOUT_SECS constant (30s) - Apply timeout to GetCapabilities during startup connection - Apply timeout to all runtime RPCs (store, delete, resolve) - Use tokio::time::timeout to bound the entire GetCapabilities operation during startup, not just the socket connection - Return contextual deadline errors on timeout This prevents a faulty or overloaded driver from hanging gateway operations indefinitely.

The Kubernetes auth Vault resolve test was constructing a managed path with test-workspace/test-provider-id but sending default/prov-123 in the request, causing validation to reject the request (GATOR-18e32351-01). - Update test to use test-workspace and test-provider-id in the request to match the logical_path construction - This ensures the test exercises the intended code path and validates Kubernetes auth resolution properly The test now passes and correctly validates identity enforcement.

Stage refresh replacements under genuinely distinct immutable handles using a unique staging ID to prevent overwriting committed values (GATOR-1806c9be-03). - Generate a unique staging ID using UUID for each refresh operation - Use this staging ID when storing credentials instead of the real provider ID - Pass the same staging ID during cleanup on failure to delete only staged objects - This ensures deterministic paths (Vault) and object names (K8s) don't collide with the committed provider's credentials The fix prevents failed refreshes from silently replacing active credentials or breaking providers by deleting still-referenced backend objects.

Add local tokio::time::timeout wrappers around credential driver RPCs to bound non-compliant or stalled UDS peers (GATOR-1806c9be-05). - Wrap StoreCredential, DeleteCredential, and ResolveCredentials in local timeouts - Return contextual deadline_exceeded errors when timeouts occur - Keep existing gRPC timeout metadata for compliant implementations - GetCapabilities during startup was already wrapped in previous commit This ensures a faulty local driver cannot hang gateway operations indefinitely, even if it accepts the connection but never responds to the RPC.

Signed-off-by: Seth Jennings <sjenning@redhat.com>

sjenning · 2026-08-05T14:55:55Z

/ok to test ee5d4d6

mrunalp · 2026-08-05T15:01:36Z

/ok to test ee5d4d6

Signed-off-by: Seth Jennings <sjenning@redhat.com>

sjenning · 2026-08-05T15:25:34Z

/ok to test e3c5f70

mrunalp · 2026-08-05T15:29:02Z

/ok to test e3c5f70

johntmyers · 2026-08-05T15:45:58Z

gator-agent

Review Convergence Checkpoint

Head SHA: e3c5f7004107893a62607066358a78c7ec93c902
Base SHA: 8c7dd148a9e6360c9d5b2830e339a0dc4b3f3032
Merge base SHA: 8c7dd148a9e6360c9d5b2830e339a0dc4b3f3032
Patch ID: a8cfad1a02cfeeab78c4dab5629bb52f0a95e2cf
Gator payload: 3

Three finding-bearing review rounds have completed.

Root-cause findings:

All prior root-cause findings remain resolved. The current author-only delta grants actions: read to the credential-driver E2E reusable-workflow call; the bounded Critical-only checkpoint review found no new defect.

Scope growth:

None in the current delta.

Reviewer-quality signals:

The current reviewer proposed no findings, duplicate invariants, waived or resolved re-raises, unchanged-code findings, or missing-reproducer downgrades. The durable ledger records 17 repeated finding-ID occurrences across historical dispositions; no new occurrence was proposed in this round.

Maintainer action: accept the current scope, split follow-up work, waive a finding, or explicitly authorize another autonomous review round.

Next state: gator:blocked

johntmyers · 2026-08-05T16:51:35Z

gator-agent

Monitoring Complete

Monitoring is complete because this PR has merged.

Head SHA: e3c5f7004107893a62607066358a78c7ec93c902
Gator payload: 3

Final status: Gator review completed with no blocking findings remaining, the required test:e2e label was applied, required checks completed, and maintainer approval was present before merge.

I removed the active gator:* label because there is nothing left for gator to monitor on this PR.

sjenning requested review from a team, derekwaynecarr, maxamillion and mrunalp as code owners July 23, 2026 03:33

This was referenced Jul 23, 2026

feat: add credential drivers for provider secret storage #1931

Open

feat(credentials): add provider credential storage drivers #1986

Open

sjenning force-pushed the 1931-credential-drivers-implementation branch 2 times, most recently from 6a10141 to 0d42e4e Compare July 23, 2026 14:28

sjenning force-pushed the 1931-credential-drivers-implementation branch from 0d42e4e to d1e10d1 Compare July 23, 2026 23:16

varshaprasad96 mentioned this pull request Jul 24, 2026

feat(server): add credential storage drivers for Provider v2 #2454

Closed

10 tasks

varshaprasad96 mentioned this pull request Jul 25, 2026

fix(credentials): harden credential storage drivers sjenning/OpenShell#3

Open

4 tasks

sjenning force-pushed the 1931-credential-drivers-implementation branch from d1e10d1 to 052540f Compare July 27, 2026 18:58

sjenning force-pushed the 1931-credential-drivers-implementation branch from 052540f to 1806c9b Compare July 28, 2026 19:40

mrunalp added the gator:in-review Gator is reviewing or awaiting PR review feedback label Jul 30, 2026

johntmyers reviewed Jul 30, 2026

View reviewed changes

Comment thread crates/openshell-driver-vault/src/lib.rs Outdated

Comment thread crates/openshell-server/src/grpc/provider.rs

Comment thread crates/openshell-server/src/provider_refresh.rs Outdated

Comment thread crates/openshell-server/src/credentials.rs Outdated

sjenning force-pushed the 1931-credential-drivers-implementation branch from 1806c9b to 18e3235 Compare July 31, 2026 19:39

sjenning added 7 commits August 5, 2026 09:49

fix(credentials): preserve ownership for staged refreshes

0f18876

Signed-off-by: Seth Jennings <sjenning@redhat.com>

fix(credentials): bound startup capability probe

c88561c

Signed-off-by: Seth Jennings <sjenning@redhat.com>

test(provider): authenticate credential handler requests

ee5d4d6

Signed-off-by: Seth Jennings <sjenning@redhat.com>

fix(ci): grant actions read to credential driver e2e

e3c5f70

Signed-off-by: Seth Jennings <sjenning@redhat.com>

mrunalp approved these changes Aug 5, 2026

View reviewed changes

Uh oh!

Conversation

sjenning commented Jul 23, 2026

Summary

Related Issue

Changes

Default Behavior

Testing

Checklist

Uh oh!

copy-pr-bot Bot commented Jul 23, 2026

Uh oh!

sjenning commented Jul 23, 2026

Uh oh!

sjenning commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sjenning commented Jul 23, 2026

Uh oh!

mrunalp commented Jul 24, 2026

Uh oh!

mrunalp commented Jul 28, 2026

Uh oh!

mrunalp commented Jul 28, 2026

Uh oh!

johntmyers left a comment

Choose a reason for hiding this comment

PR Review Status

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sjenning commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sjenning commented Aug 5, 2026

Uh oh!

mrunalp commented Aug 5, 2026

Uh oh!

sjenning commented Aug 5, 2026

Uh oh!

mrunalp commented Aug 5, 2026

Uh oh!

johntmyers commented Aug 5, 2026

Review Convergence Checkpoint

Uh oh!

johntmyers commented Aug 5, 2026

Monitoring Complete

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sjenning commented Jul 23, 2026 •

edited

Loading

sjenning commented Jul 31, 2026 •

edited

Loading