Skip to content

fix(providers): keep refresh credential handles stable - #2780

Open
mrunalp wants to merge 1 commit into
NVIDIA:mainfrom
mrunalp:fix/2777-stable-credential-handles/mrunalp
Open

fix(providers): keep refresh credential handles stable#2780
mrunalp wants to merge 1 commit into
NVIDIA:mainfrom
mrunalp:fix/2777-stable-credential-handles/mrunalp

Conversation

@mrunalp

@mrunalp mrunalp commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

🏗️ build-from-issue-agent

Summary

Keep gateway-managed refresh credentials usable by long-running sandbox processes when their short-lived access tokens rotate. Workloads now receive an opaque, identity-stable handle whose resolver value advances to the current token, while explicit refresh reconfiguration and authorization-boundary changes revoke the old handle.

Related Issue

Closes #2777

Changes

  • Add a durable authorization epoch to gateway refresh configuration, with a stable migration fallback for existing records.
  • Derive per-sandbox workload credential handles from the provider identity, credential key, authorization epoch, and canonical endpoint boundary.
  • Resolve stable handles only against the current credential value; do not retain their old token values in the revision-generation queue.
  • Rotate the handle on explicit refresh reconfiguration, provider replacement, refresh deletion, or endpoint-boundary changes so stale processes fail closed.
  • Include refresh authorization metadata in provider-environment revisions without hashing refresh secrets.
  • Add focused unit/integration coverage plus a Podman E2E that exercises 12 rotations, supervisor-state reconstruction, and reconfiguration revocation.
  • Document the lifecycle, rollout restart requirement, reserved placeholder namespace, and long-running workload behavior.

Deviations from Plan

None — implemented as planned.

Testing

  • mise run pre-commit passes
  • mise run test passes
  • mise run ci passes
  • cargo test -p openshell-core passes
  • cargo test -p openshell-server passes
  • cargo test -p openshell-supervisor-network passes
  • Podman E2E provider_refresh_handles passes through 12 token rotations and verifies explicit reconfiguration revocation
  • Real OpenClaw + gog validation: one already-running OpenClaw gateway accessed Gmail before and after an OpenShell provider refresh without restarting; no mailbox content was captured
  • Unit tests added/updated
  • E2E tests added/updated

Tests added:

  • Unit: stable placeholder parsing, current-only resolution, expiry, 12 rotations, reconstruction, endpoint/auth-epoch revocation, and refresh-epoch persistence.
  • Integration: gateway binding derivation, refresh rotation stability, reconstruction, and explicit reconfiguration revocation.
  • E2E: e2e/rust/tests/provider_refresh_handles.rs covers a long-running Podman process through 12 rotations and a reauthorization boundary.

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • User-facing and architecture documentation updated
  • Related OpenShell CLI skill updated

Documentation updated:

  • architecture/sandbox.md: stable handle data flow and revocation invariants.
  • docs/sandboxes/providers-v2.mdx: workload behavior, authorization epochs, and rollout guidance.
  • .agents/skills/openshell-cli/SKILL.md: refresh lifecycle guidance for CLI workflows.

Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
@mrunalp
mrunalp requested review from a team, derekwaynecarr and sjenning as code owners August 17, 2026 18:04
@copy-pr-bot

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

@mrunalp

mrunalp commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator Author

🏗️ build-from-issue-agent

E2E Test Attestation

Local E2E tests passed. CI does not currently run this Podman scenario, so this comment serves as the verification record.

Field Value
Commit a505e8d9eb92235e60bdb5dc421ade54836cd6ed
Command cargo test --manifest-path e2e/rust/Cargo.toml --features e2e-podman --test provider_refresh_handles -- --nocapture
Gateway mode Podman
Result ✅ All passed

Test Summary

1 passed; 0 failed; finished in 77.58s

Tests Executed

  • provider_refresh_handles::long_running_process_survives_rotations_and_reconfigure_revokes — PASSED

The test uses a fake issuer that invalidates each old access token. A single long-running workload succeeds initially, survives 12 token rotations with its original environment, and then fails closed after explicit refresh reconfiguration; a newly executed process succeeds with the replacement handle.

@mrunalp

mrunalp commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator Author

/ok to test a505e8d

@mrunalp mrunalp added the test:e2e Requires end-to-end coverage label Aug 17, 2026
@github-actions

Copy link
Copy Markdown

Label test:e2e applied for a505e8d. 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

This comment was marked as duplicate.

record,
key,
endpoints,
refresh_epochs.get(key).map(String::as_str),

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.

A running process can keep using its old credential handle after an administrator manually replaces a refresh-managed credential. Because the handle does not change here, that process immediately gains access to the replacement credential, which could belong to a different account or authorization.

Please prevent normal provider updates from changing or deleting credentials owned by active refresh configuration, including related output credentials. Those values should be changed through refresh rotate/reconfigure/delete instead. Alternatively, changing them must also create a new handle. Add a test confirming that an old handle cannot access a manually replaced credential.

@drew
drew dismissed their stale review August 18, 2026 06:38

Replaced with a focused inline review comment.

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

Labels

test:e2e Requires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: long-running processes lose provider access after credential refresh

2 participants