feat: keep access tokens valid across a pepper change

[netomi]

Stacked on #2159 — based on refactor/token-hash-pepper, not main, so review that one first and this diff stays to the keyring itself. Retarget to main once #2159 merges.

Two commits: the keyring, then the startup warning that the keyring makes actionable.

The problem

Changing ovsx.access-token.token-hash-pepper invalidates every token in the registry. The raw value is never stored — createAccessToken hashes it on the way in (AccessTokenService.java:179) and the row keeps only the digest — so nothing can rehash a row under a new pepper except the holder presenting the token again. The v0→v1 upgrade is no template: upgradeToken works only because v0 rows still literally carried their raw value.

That matters most for the case nearly every instance is actually in. The pepper defaults to the empty string and no shipped configuration sets one, so adopting a pepper is itself a rotation — and today it logs out every user's tokens at once.

The approach

Rotation can't happen on the stored side, so it happens on the verification side. useAccessToken retries a token that misses under the current pepper against each pepper the instance used before, and rewrites the row it finds:

var token = repositories.findPersonalAccessToken(hashTokenValue(tokenValue));
if (token == null) {
    // the pepper may have changed since this token was issued; the row is rewritten if so
    token = findTokenHashedWithPreviousPepper(tokenValue);
}
if (token == null) {
    // assume DB contains token v0; ...

Each token migrates to the current pepper the next time it is used. The retry sits ahead of the existing v0 fallback, and the rewrite happens before the active/expired/scope checks — matching what the v0 upgrade already does, since which pepper hashed a row says nothing about whether that token is usable.

Configuration

Property	Purpose
ovsx.access-token.token-hash-previous-peppers	comma separated list of retired peppers
ovsx.access-token.token-hash-accept-unpeppered	accept hashes carrying no pepper — the switch for adopting a pepper where there was none

The flag exists because the unpeppered case is a previous pepper of "", which a comma separated list cannot express unambiguously. Both feed one deduplicated keyring that drops the current pepper, since that is always tried first.

The startup warning

An instance with no pepper now says so once at boot, naming what it costs and what to do:

No ovsx.access-token.token-hash-pepper is configured, so personal access tokens are stored as unkeyed hashes. Whoever obtains a copy of the database can then confirm a token found elsewhere without touching this registry, and match hashes across dumps and instances. Generate one with 'openssl rand -base64 32'; to keep the tokens that already exist working, set ovsx.access-token.token-hash-accept-unpeppered=true at the same time and drop it once they have expired.

WARN, not a startup failure: the empty default has to keep working. It belongs in this PR rather than on its own, because before the keyring the only honest advice was "set a pepper and invalidate every token" — the warning is worth making only once acting on it is safe. Checks hasText rather than isEmpty, since a pepper of blanks is no more of a secret than none.

To be clear about what the empty default does and does not cost: token values are 256 CSPRNG bits, so a leaked hash column still yields no usable tokens either way. What an unkeyed hash gives away is an offline confirmation oracle for a token found elsewhere — no rate limit, no access-log entry, no accessed_timestamp bump — and the ability to join hashes across dumps and instances, since unkeyed SHA-256 is globally deterministic. Worth a warning, not worth a refusal to start.

What this deliberately does not do

It cannot finish the migration early. No background job can rehash a token nobody presents, so a retired pepper must stay configured until every row that might still use it is gone — bounded by ovsx.access-token.expiration plus one expiry sweep, and unbounded where expiry is disabled (expiration: 0), because an unused token then lives forever and so must its pepper. That's documented on the property itself rather than left for an operator to find out.

Cost

One extra query per configured previous pepper, and only for a token that hasn't been used since the rotation. An instance not mid-rotation has an empty keyring and does no extra work — asserted by doesNotRetryPreviousPeppersWhenTheCurrentOneMatches, where the keyring stub going unread is half the point.

Also

A current pepper containing a comma is now rejected at startup, rather than at the rotation where it would silently split into two wrong peppers — by which point the tokens hashed with it are unreachable and the mistake looks like data loss.

Verification

• ./gradlew test --tests '*AccessToken*' — 46 tests pass (--rerun-tasks), including 11 new AccessTokenConfigTest cases (binding, ordering, dedup, current-pepper removal, comma rejection, and both warning paths) and 5 new rotation cases in AccessTokenServiceTest
• spotlessCheck clean on every touched file

AccessTokenConfigTest needs .withInitializer(... setConversionService(ApplicationConversionService.getSharedInstance())) — a bare ApplicationContextRunner has no conversion service, so neither the Duration properties nor the comma separated list bind without it. Worth knowing for the next config test.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

token-hash-previous-peppers can currently admit blank/empty list entries, which may unintentionally enable unpeppered-token fallback without the explicit token-hash-accept-unpeppered flag.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR adds support for lazy personal access token hash “pepper” rotation by retrying verification against a configured keyring of previous peppers and rehashing the token row on successful match, so existing tokens remain valid across a pepper change without requiring the raw token to be stored.

Changes:

• Add pepper-rotation fallback lookup in AccessTokenService.useAccessToken() that tries prior peppers and rewrites the stored hash on match.
• Introduce new config properties and derived “pepper keyring” in AccessTokenConfig, plus a startup warning when no pepper is configured.
• Add focused unit tests covering keyring derivation/binding and rotation behavior.

File summaries

File	Description
server/src/main/java/org/eclipse/openvsx/accesstoken/AccessTokenService.java	Retries token lookup using previous peppers and rehashes rows to the current pepper on successful match.
server/src/main/java/org/eclipse/openvsx/accesstoken/AccessTokenConfig.java	Adds config for previous peppers/unpeppered acceptance, derives an immutable keyring, and logs a startup warning when pepper is blank.
server/src/test/java/org/eclipse/openvsx/accesstoken/AccessTokenServiceTest.java	Adds rotation/adoption tests ensuring fallback is ordered, stops on first match, and avoids extra lookups on current-pepper hits.
server/src/test/java/org/eclipse/openvsx/accesstoken/AccessTokenConfigTest.java	New tests validating property binding/order/dedup/current-pepper removal/comma rejection and warning behavior.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.