Clear the leftover StepFun Oasis token on revoke (SBS-920)

[tsouth89]

Summary

• StepFun's refresh path writes the new Oasis token to the OS keyring (codexbar-stepfun / api_key). resolve_token reads that copy after Preferences is empty.
• revoke_managed_credentials only cleared api_keys.json, manual_cookies.json, and token-accounts.json. After Revoke stored credentials, the next fetch still authenticated from the leftover keyring token.
• Revoke now calls a provider-owned clear_persisted_credentials hook. StepFun deletes that keyring entry. Missing is success. Any other keyring error, or a delete that reports success while the token is still there, fails the revoke.

Closes nothing in Linear: SBS-920 stays In Progress until review.

A user who hits this now sees

Someone who pastes a StepFun Oasis token, lets it refresh, then clicks Revoke stored credentials gets CredentialRevoked only if the keyring copy is gone (or was never there). The next Auto refresh cannot pick the leftover token back up. If the keyring is locked or the delete cannot be confirmed, revoke fails and the UI shows the error instead of success while the token remains. Preferences / api_keys.json was already cleared by revoke; that path is unchanged.

Environment variables (STEPFUN_OASIS_TOKEN / STEPFUN_TOKEN) are not managed credentials and still resolve after revoke, same as before.

Fail without the fix

Reverted only clear_token_secret to a no-op Ok(()) (pre-SBS-920 revoke: do not delete the keyring copy), then ran the new test:

thread 'providers::stepfun::tests::revoke_clears_refreshed_keyring_token_so_resolve_cannot_revive_session' panicked at rust/src/providers/stepfun/mod.rs:643:14:
leftover keyring token must not authenticate after revoke: "access...refresh"
test providers::stepfun::tests::revoke_clears_refreshed_keyring_token_so_resolve_cannot_revive_session ... FAILED

failures:
    providers::stepfun::tests::revoke_clears_refreshed_keyring_token_so_resolve_cannot_revive_session

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 1011 filtered out

Restored the delete + post-delete confirmation. The same test then passed.

Tests use an in-memory store, not the OS keychain. A suite that talks to a real keychain is what went red on a headless runner before.

Pattern sweep

rg -n --glob '!target/**' "persist_refreshed_token|set_password|keyring::Entry" rust/

Live set_password of a refreshed token: only StepFun. That is the write this PR clears.

Other hits, left alone on purpose:

Site	What it does	Why not cleared here
rust/src/core/credentials.rs / credential_migration.rs	set_password / delete_credential	Unused (dead_code). Not on the revoke path.
Gemini persist_refreshed_credentials	Writes the Gemini CLI credentials file	Not keyring. File is owned by Gemini CLI, not Ceiling's api_keys.json.
Claude persist_refreshed_credentials	Writes .credentials.json	Same: Claude Code's file. If the token was loaded from keyring/env and there is no file, persist is a no-op and does not write keyring.
kilo / codebuff / crof / venice / groq / deepgram / doubao / openaiapi / llmproxy / zai / elevenlabs / warp / deepseek / openrouter / nanogpt / providers::resolve_api_key / Claude oauth load_from_keyring	Read keyring only	Ceiling did not write those entries. They may be shared with another app. Deleting them on Ceiling revoke would sign the user out of something Ceiling does not own.

If another provider later writes a live token to the keyring, it needs a branch in providers::clear_persisted_credentials. Settings does not name StepFun in code.

What this change makes more likely

Revoke for StepFun now talks to the OS keyring. A locked, missing, or lying backend fails the whole revoke even after the three shared files are already cleared. The user sees an error and can retry (both the file removals and the hook are idempotent). That is the intended fail-closed tradeoff: reporting CredentialRevoked while the leftover token still authenticates is the bug.

Fetch still falls through to env on a keyring read error, matching the previous resolver. Only revoke fails closed.

Quality gate

Required CI (.github/workflows/ci.yml):

Check	What I ran	Result
cargo fmt --all --check	yes, repo root	pass
cargo test --manifest-path rust/Cargo.toml	yes	1006 passed, 6 failed — the 6 are pre-existing Linux path/project-name assertions (codex_sessions WSL slash vs backslash, grok_costs / cost_scanner attributing C:\projects\... instead of the leaf name). None are in this diff. CI rust-shared is windows-latest. New StepFun tests: 8 passed.
cargo clippy --manifest-path rust/Cargo.toml --all-targets -- -D warnings	yes	fails on this Linux box on two pre-existing unused items I did not touch: secure_file.rs:489 unused error, updater.rs:442 verify_installer_signature_or_delete (Windows-only in practice). CI clippy is windows-latest.
.\scripts\ci\test-store-submission-preparation.ps1	no	Windows-only; this box is Linux.
cargo test / clippy desktop crate	attempted	cannot run here: Tauri pull in glib-2.0, which is not installed. CI rust-desktop is windows-latest. This PR does not change the desktop crate.
Frontend job	no	no JS/TS change.

target/ dirs deleted after the runs.

Gaps

• Did not run the Windows CI jobs on Windows.
• Did not exercise a real OS keyring (Secret Service / Credential Manager) end-to-end. The hook that production revoke calls (OsTokenSecretStore) is the same keyring API persist already used; tests cover the store contract, not gnome-keyring.
• Did not add a settings-level test that calls revoke_managed_credentials against real api_keys.json paths — that function uses the machine config dir. Wiring is a one-line call after the existing file writes.
• Did not change Claude / Gemini CLI credential files, or delete other providers' read-only keyring lookups.
• Did not mark SBS-920 Done. Did not merge.

Note

Clear the StepFun Oasis keyring token on credential revoke

• revoke_managed_credentials now calls a provider-specific clear_persisted_credentials hook before writing cleared stores to disk; for StepFun, this deletes the OS keyring token and confirms its absence.
• If the keyring deletion fails or the token is still present after deletion, the revoke aborts and leaves the credential intact in preferences so the user can retry.
• persist_refreshed_token in StepFun now checks under the state lock whether the credential is still configured before writing to the keyring, preventing a background refresh from restoring a token after revoke.
• A TokenSecretStore trait is introduced to abstract keyring operations, enabling the new delete-then-confirm semantics and making the logic testable.
• Behavioral Change: a StepFun sign-out that cannot confirm keyring token deletion now fails visibly instead of silently leaving the token in place.

^{Macroscope summarized 0337a78.}

[coderabbitai]

Warning

Review limit reached

@tsouth89, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 12 minutes

Limit details: You’ve used all 1 included review currently available under your plan.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d6909486-3782-4e48-a666-1360b360a679

📥 Commits

Reviewing files that changed from the base of the PR and between b596ebb and 0337a78.

📒 Files selected for processing (6)

• CHANGELOG.md
• rust/src/providers/mod.rs
• rust/src/providers/stepfun/mod.rs
• rust/src/settings.rs
• rust/src/settings/api_keys.rs
• rust/src/settings/tests.rs

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[cloudflare-workers-and-pages]

Deploying with    Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	ceiling	0337a78	Commit Preview URL Branch Preview URL	Aug 18 2026, 02:32 AM

[github-actions]

Automated review

New in this pass: 4 issues.

1. Unreadable API keys store is treated as a revoke

   rust/src/providers/stepfun/mod.rs:439 · disposition: block · confidence: high · severity: high · quick win

   stepfun_credential_configured calls ApiKeys::load(), which is try_load().unwrap_or_default(). A DPAPI, decode, or read failure therefore looks like no Preferences key, so persist_refreshed_token_in skips store.set. ProviderRefreshInputs::load() uses the same fail-open load, so ctx.api_key is empty and resolve_token authenticates from the last keyring copy. The refresh replaces that Oasis token (new access plus refresh_token) and then drops it. The next fetch still has the old keyring refresh; if StepFun rotated it, the session dies even though the user never signed out. The UI still shows Revoke, because an unreadable store sets hasProviderCredentials to null rather than false.

   Prompt for AI agents

   In rust/src/providers/stepfun/mod.rs around line 439: Treat only a successful load with no StepFun key (and no env) as revoked. On try_load() Err, keep still_configured true so persist still writes. Add a test that a failing keys read still calls store.set. Verify against the current code first; if no longer valid, skip with a brief reason. Keep the change minimal.

2. persist_refreshed_token_in lock-and-disk check has no failing test

   rust/src/providers/stepfun/mod.rs:418 · disposition: fix-if-quick · confidence: high · severity: medium · quick win

   Production persist goes through persist_refreshed_token_in, which takes the state lock and calls stepfun_credential_configured(). The new tests only call persist_refreshed_token_when with an injected bool. Removing the lock, always passing true, or making stepfun_credential_configured always return true would leave every test in this diff green, so the rewrite-after-revoke race this revision is meant to close can return unnoticed.

   Prompt for AI agents

   In rust/src/providers/stepfun/mod.rs around line 418: Inject the configured check (or a temp keys file) into persist_refreshed_token_in. Add a test that empties Preferences the way revoke does, then asserts the store is not written. Verify against the current code first; if no longer valid, skip with a brief reason. Keep the change minimal.

3. Pre-existing: leftover keyring sessions still have no Revoke control

   rust/src/providers/stepfun/mod.rs:544 · disposition: follow-up · confidence: high · severity: low

   resolve_token_in still returns a keyring token when Preferences is empty. Users who already signed out under the old revoke (files cleared, keyring left) stay signed in. shouldShowCredentialRevoke is false when every file store reports no credential, so the new clear_token_secret hook never runs for them. This is the same leftover the PR is fixing for a later Sign out, not a new write.

   Prompt for AI agents

   In rust/src/providers/stepfun/mod.rs around line 544: Do not auto-delete the keyring on launch: a user-placed codexbar-stepfun/api_key is indistinguishable from the leftover. Show Revoke when that StepFun keyring copy is present, or tell those users to paste a dummy key and revoke. Verify against the current code first; if no longer valid, skip with a brief reason. Keep the change minimal.

4. Public revoke docs still say the hook runs after the files

   rust/src/settings.rs:63 · disposition: advisory · confidence: high · severity: low · quick win

   The rustdoc on revoke_managed_credentials says the provider hook runs after the shared files are written. revoke_managed_credentials_in now calls clear_persisted before keys.save_to, cookies.save_to, and save_unlocked. A later edit that restores the documented order would hide Revoke on hook failure and leave a live keyring token, the two bugs this revision just closed.

   Prompt for AI agents

   In rust/src/settings.rs around line 63: Rewrite that rustdoc to match the hook-first order already described at the call site. Verify against the current code first; if no longer valid, skip with a brief reason. Keep the change minimal.

Resolved since the previous pass: 3.

_{For coding agents: fix BLOCK and FIX IF QUICK findings now; everything else is tracked or informational; never exceed one CodeRev fix round per PR.}

_{Advisory. Findings generated by grok-subscription and muse-spark-1.2-contributor, each filtered through a 3-vote refutation panel with the changed code in evidence.}