registrystack / registrystack/registry-stack

Move blocking secret reads and assertion signing out of the OAuth token-cache critical section

Open
#681 0 comments 0 reactions 0 assignees View on GitHub
area:platform enhancement rust triage:roadmap
Dominant language
Rust
Stars
2
Forks
0
Avg merge
2h 55m
Merged PRs (30d)
130

Description

## What

`OauthPlan::access_token` (`crates/registry-evidence/src/source.rs`) holds the
per-source token-cache mutex across the token round trip, and builds the token
request while holding it. Building that request does two things that block a
Tokio worker thread:

- a synchronous filesystem read of the client secret or the client assertion
private JWK, through `SecretResolver`;
- for the `private_key_jwt` form, synchronous ECDSA P-384 or RSA-2048 signing of
the client assertion.

Neither runs under `spawn_blocking`.

## Why it is not simply a bug

Holding the lock across the refresh is the intended single-flight behaviour: it
is what stops N concurrent evidence requests for one source from each firing
their own token request at the authorization server. That part should stay.

The cost is that the blocking work sits *inside* that critical section, so it is
serialized behind the same lock and lands on a runtime worker rather than the
blocking pool. Every concurrent caller for that source waits through it, bounded
only by the configured source timeout via `admission_timeout`. Adding assertion
signing made the section measurably more expensive than when it only read a
secret file.

## Impact

Availability and tail latency under concurrent refresh for a single source. Not
a correctness, disclosure, or authentication problem: the token is still
correct, the credential still never leaves the process, and a failure is still
fail-closed. Filing publicly for that reason, rather than through `SECURITY.md`,
whose scope is authentication bypass, credential disclosure, audit redaction or
integrity failure, signing-key handling, connector data leakage, and privacy
regressions.

## Sketch of a fix

Move the secret read and the signature onto the blocking pool, and prepare the
request before taking the lock so the critical section covers only the round trip
and the cache write. Either change alone helps; the second is the one that
restructures the cache, which is why this is its own change rather than part of
the assertion work.

## Notes

Pre-existing in shape. The secret read was already there; assertion signing was
added alongside the `private_key_jwt` client authentication form and made the
section heavier. Deliberately left out of scope of that change.

Contributor guide

Open the contributing guide

Research direction

Start in crates/registry-evidence/src/source.rs at OauthPlan::access_token and trace the token-cache mutex, SecretResolver reads, request construction, and assertion signing. Verify the existing single-flight cache behavior remains intact while blocking work no longer runs on a Tokio worker or occupies the critical section; completion should preserve fail-closed behavior and the configured admission_timeout.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
api, authentication, backend, performance
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.