From 500d8195b02679ea50a060688ab11984dde3a79a Mon Sep 17 00:00:00 2001 From: Jeremi Joslin Date: Wed, 16 Sep 2026 09:40:18 +0700 Subject: [PATCH 1/2] test(oid4vci): expire recovery state on a clock the test moves Every deadline the offer store holds is a whole Unix second, so a recovery test that expired state by configuring a one-second window and sleeping past it was asking the wall clock to land where the test needed it: state created just before a second boundary held almost none of the window it was given, and a loaded machine spent the rest, so a request the test expected to be served came back refused. Give the service a clock to read instead. `DeliveryService` holds the function it reads the current second from, `with_clock` replaces it, and the recovery tests that need a deadline to have passed move their own reading forward by whole seconds rather than sleeping. Those tests now run on the lifetimes a deployment may configure, and a wallet proof they send is dated by the same reading the service judges it against. The same rewrite covers the two other parts of that test, for the access token and the nonce windows, and the sibling saturation test that also expired an offer ledger by sleeping. Security review note: this touches expiry enforcement, so the seam is built to be unreachable from a deployment. `DeliveryService::load` reads the system clock, `with_clock` is hidden from the documented surface and reachable only from a caller that already constructs a service from halves, and no configuration key selects a clock. The store is unchanged: it still takes the reading as an argument and still expires an entry when `expires_at <= now`. Signed-off-by: Jeremi Joslin --- .../registry-evidence-oid4vci/src/service.rs | 46 +++++- .../tests/state_machine_recovery.rs | 155 ++++++++++++++---- 2 files changed, 160 insertions(+), 41 deletions(-) diff --git a/crates/registry-evidence-oid4vci/src/service.rs b/crates/registry-evidence-oid4vci/src/service.rs index 6c5504935d..5b6ea18069 100644 --- a/crates/registry-evidence-oid4vci/src/service.rs +++ b/crates/registry-evidence-oid4vci/src/service.rs @@ -160,8 +160,13 @@ pub struct DeliveryService { authorizer: Arc, issuer: Arc, metrics: Arc, + /// Where every deadline in this process is read against. + clock: Clock, } +/// How a handler reads the current second. +type Clock = Arc i64 + Send + Sync>; + impl std::fmt::Debug for DeliveryService { fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { formatter @@ -204,9 +209,30 @@ impl DeliveryService { authorizer, issuer, metrics, + clock: Arc::new(now), } } + /// Read every deadline against a clock the caller supplies. + /// + /// Every deadline this service holds is a whole Unix second, so a test that + /// needs one to have passed has to be able to say when it passed. Sleeping + /// past a configured window leaves that to where the boundary fell, and an + /// entry created just before one holds almost none of the window it was + /// given. A serving process never comes through here: [`Self::load`] reads + /// the system clock, and no configuration key reaches this. + #[doc(hidden)] + #[must_use] + pub fn with_clock(mut self, clock: impl Fn() -> i64 + Send + Sync + 'static) -> Self { + self.clock = Arc::new(clock); + self + } + + /// The current second, as every deadline in this process is judged against. + fn now(&self) -> i64 { + (self.clock)() + } + /// Validate a configuration without taking what a serving process holds. /// /// Everything [`DeliveryService::load`] does, and no socket. The loaded key @@ -404,7 +430,7 @@ async fn cleanup_expired(service: Arc, mut stopped: watch::Rece loop { tokio::select! { _ = interval.tick() => { - let expired = service.store.sweep(now()); + let expired = service.store.sweep(service.now()); service.metrics.record_cleanup(expired); service .metrics @@ -555,7 +581,7 @@ async fn create_offer( &code, transaction_code.as_ref().map(|code| code.as_str()), prepared, - now(), + service.now(), ) { return store_response(&service, &error, Outcome::StoreSaturated); } @@ -625,7 +651,7 @@ async fn token( &code, transaction_code.as_ref().map(|code| code.as_str()), &access_token, - now(), + service.now(), ) { return store_response(&service, &error, Outcome::CodeClaimRefused); } @@ -675,7 +701,7 @@ async fn nonce(State(service): State>, body: Bytes) -> Resp service.metrics.record_outcome(Outcome::NonceMinted); value_response( StatusCode::OK, - &json!({"c_nonce": service.nonces.mint(now())}), + &json!({"c_nonce": service.nonces.mint(service.now())}), ) } @@ -708,7 +734,10 @@ async fn credential( return unauthorized("a bearer access token is required"); }; let access_token = Zeroizing::new(access_token.to_owned()); - let prepared = match service.store.claim_access_token(&access_token, now()) { + let prepared = match service + .store + .claim_access_token(&access_token, service.now()) + { Ok(prepared) => prepared, Err(StoreError::Unknown) => { service.metrics.record_outcome(Outcome::TokenClaimRefused); @@ -718,7 +747,7 @@ async fn credential( }; stage.mark_claimed(); service.metrics.record_outcome(Outcome::TokenClaimed); - let expired = service.store.sweep(now()); + let expired = service.store.sweep(service.now()); service.metrics.record_cleanup(expired); service .metrics @@ -874,7 +903,7 @@ fn holder_key_from_proof( "the proof does not carry a nonce", )); }; - match service.nonces.verify(&nonce, now()) { + match service.nonces.verify(&nonce, service.now()) { Ok(()) => {} Err(NonceError::Expired) => { return Err(ProofRefusal::nonce( @@ -901,7 +930,7 @@ fn holder_key_from_proof( max_age: PROOF_MAX_AGE, max_future_skew: PROOF_MAX_FUTURE_SKEW, }; - let claims = validate_oid4vci_proof_jwt(proof, &policy, now()) + let claims = validate_oid4vci_proof_jwt(proof, &policy, service.now()) .map_err(|_| ProofRefusal::new("invalid_proof", "the proof was not accepted"))?; holder_public_key(&claims.holder_jwk).ok_or(ProofRefusal::new( "invalid_proof", @@ -972,6 +1001,7 @@ fn bearer_credential(headers: &HeaderMap) -> Option<&str> { (!credential.is_empty()).then_some(credential) } +/// The system clock, which is the reading a serving process takes. fn now() -> i64 { Utc::now().timestamp() } diff --git a/crates/registry-evidence-oid4vci/tests/state_machine_recovery.rs b/crates/registry-evidence-oid4vci/tests/state_machine_recovery.rs index ac4fd00633..b203d3f61b 100644 --- a/crates/registry-evidence-oid4vci/tests/state_machine_recovery.rs +++ b/crates/registry-evidence-oid4vci/tests/state_machine_recovery.rs @@ -8,7 +8,10 @@ use std::{ fs, - sync::{Arc, Mutex}, + sync::{ + atomic::{AtomicI64, Ordering}, + Arc, Mutex, + }, time::Duration, }; @@ -309,6 +312,36 @@ struct Harness { issuer: Arc, } +/// The clock a test runs the service on: the wall-clock second a deployment +/// reads, plus an offset the test moves forward. +/// +/// Every deadline the store holds is a whole Unix second, so a test that +/// expires state by sleeping is asking the wall clock to land where the test +/// needs it. An entry created just before a boundary holds almost none of its +/// configured window, and a loaded machine spends the rest. Moving the reading +/// forward instead puts the deadline that must have passed behind the service +/// by construction, and leaves everything created afterwards its whole window. +#[derive(Clone, Default)] +struct TestClock(Arc); + +impl TestClock { + /// The reading the service takes right now. + fn now(&self) -> i64 { + chrono::Utc::now().timestamp() + self.0.load(Ordering::SeqCst) + } + + /// Move every later reading forward by whole seconds. + fn advance(&self, seconds: i64) { + self.0.fetch_add(seconds, Ordering::SeqCst); + } + + /// The reading function a service built on this clock holds. + fn reading(&self) -> impl Fn() -> i64 + Send + Sync + 'static { + let clock = self.clone(); + move || clock.now() + } +} + fn loaded_config() -> DeliveryConfig { let directory = tempfile::tempdir().expect("a temporary deployment directory"); let path = directory.path().join("oid4vci.yaml"); @@ -316,32 +349,57 @@ fn loaded_config() -> DeliveryConfig { DeliveryConfig::load(&path).expect("the reference deployment loads before test mutation") } -fn server_with_issuer( +fn service_with_issuer( mut config: DeliveryConfig, issuer: Arc, -) -> Arc { +) -> DeliveryService { // The listener belongs to a real deployment. axum-test binds its own // random loopback port, while every published identifier remains the // deployment's exact configured value. config.listener.port = 8090; - let service = Arc::new(DeliveryService::with_halves( - config, - Arc::new(StubAuthorizer), - issuer, - )); + DeliveryService::with_halves(config, Arc::new(StubAuthorizer), issuer) +} + +fn served(service: DeliveryService) -> Arc { Arc::new( TestServer::builder() .http_transport() - .build(build_app(service)), + .build(build_app(Arc::new(service))), ) } +fn server_with_issuer( + config: DeliveryConfig, + issuer: Arc, +) -> Arc { + served(service_with_issuer(config, issuer)) +} + +fn server_with_issuer_on_clock( + config: DeliveryConfig, + issuer: Arc, + clock: &TestClock, +) -> Arc { + served(service_with_issuer(config, issuer).with_clock(clock.reading())) +} + fn harness(config: DeliveryConfig) -> Harness { let issuer = Arc::new(RecordingIssuer::new()); let server = server_with_issuer(config, Arc::clone(&issuer) as Arc); Harness { server, issuer } } +/// A harness whose service reads the test's clock rather than the wall clock. +fn harness_on_clock(config: DeliveryConfig, clock: &TestClock) -> Harness { + let issuer = Arc::new(RecordingIssuer::new()); + let server = server_with_issuer_on_clock( + config, + Arc::clone(&issuer) as Arc, + clock, + ); + Harness { server, issuer } +} + fn offer_body(transaction_code: bool) -> Value { json!({ "credentialConfigurationId": CONFIGURATION_ID, @@ -419,6 +477,13 @@ fn private_jwk() -> String { } fn proof_jwt(private_key: &str, nonce: &str) -> String { + proof_jwt_at(private_key, nonce, chrono::Utc::now().timestamp()) +} + +/// A proof dated by a reading the caller chose, for the tests that run the +/// service on their own clock: a wallet dates its proof by the clock the +/// service reads it against, so a test that moves that clock moves both. +fn proof_jwt_at(private_key: &str, nonce: &str, issued_at: i64) -> String { let private = PrivateJwk::parse(private_key).expect("the holder key parses"); let header = json!({ "alg": "ES256", @@ -427,7 +492,7 @@ fn proof_jwt(private_key: &str, nonce: &str) -> String { }); let claims = json!({ "aud": "https://wallet.example.org", - "iat": chrono::Utc::now().timestamp(), + "iat": issued_at, "nonce": nonce, }); let signing_input = format!( @@ -1100,15 +1165,17 @@ async fn offer_saturation_fails_closed_without_evicting_a_live_exchange() { async fn token_saturation_does_not_spend_the_offer_that_could_not_be_exchanged() { let mut config = loaded_config(); config.store.maximum_offers = 1; - // Store deadlines use whole seconds. This leaves the recovered offer at - // least four seconds for the credential exchange even near a boundary. - config.store.offer_lifetime_seconds = 5; - config.store.access_token_lifetime_seconds = 60; - config.store.nonce_lifetime_seconds = 30; - let harness = harness(config); + // The held token has to outlive the reading that expires the first offer's + // ledger, so its window is set above the offer window rather than near it. + config.store.access_token_lifetime_seconds = 900; + let offer_lifetime = config.store.offer_lifetime_seconds as i64; + let clock = TestClock::default(); + let harness = harness_on_clock(config, &clock); let held_token = access_token(&harness.server).await; - tokio::time::sleep(Duration::from_millis(5_100)).await; + // Past the offer window, so the redeemed offer's ledger is expired however + // the seconds fell and the single ledger slot is free for the offer below. + clock.advance(offer_lifetime + 1); let preserved_code = offered_code(&create_offer(&harness.server, false).await); let saturated = harness @@ -1133,7 +1200,11 @@ async fn token_saturation_does_not_spend_the_offer_that_could_not_be_exchanged() .server .post(CREDENTIAL_PATH) .add_header("authorization", format!("Bearer {held_token}")) - .json(&credential_body(vec![proof_jwt(&private_jwk(), &nonce)])) + .json(&credential_body(vec![proof_jwt_at( + &private_jwk(), + &nonce, + clock.now(), + )])) .await; assert_eq!(released.status_code(), StatusCode::OK); @@ -1262,11 +1333,14 @@ async fn unknown_redeemed_and_locked_codes_share_one_value_free_error() { async fn expiry_cleanup_preserves_live_state_and_releases_expired_state() { let mut cleanup_config = loaded_config(); cleanup_config.store.maximum_offers = 2; - cleanup_config.store.offer_lifetime_seconds = 1; - let cleanup = harness(cleanup_config); + let offer_lifetime = cleanup_config.store.offer_lifetime_seconds as i64; + let cleanup_clock = TestClock::default(); + let cleanup = harness_on_clock(cleanup_config, &cleanup_clock); let expired_code = offered_code(&create_offer(&cleanup.server, false).await); - tokio::time::sleep(Duration::from_millis(1_100)).await; + // Past the configured offer window, so this offer is expired however the + // seconds fell, and the two created below hold a whole window each. + cleanup_clock.advance(offer_lifetime + 1); let expired = cleanup .server .post(TOKEN_PATH) @@ -1292,13 +1366,19 @@ async fn expiry_cleanup_preserves_live_state_and_releases_expired_state() { .await; assert_eq!(live.status_code(), StatusCode::OK); - let mut token_config = loaded_config(); - token_config.store.access_token_lifetime_seconds = 1; - let token_expiry = harness(token_config); + let token_config = loaded_config(); + let access_token_lifetime = token_config.store.access_token_lifetime_seconds as i64; + let token_clock = TestClock::default(); + let token_expiry = harness_on_clock(token_config, &token_clock); let expired_token = access_token(&token_expiry.server).await; - tokio::time::sleep(Duration::from_millis(1_100)).await; + // Past the configured access token window, on the same reading. + token_clock.advance(access_token_lifetime + 1); let nonce = minted_nonce(&token_expiry.server).await; - let request = credential_body(vec![proof_jwt(&private_jwk(), &nonce)]); + let request = credential_body(vec![proof_jwt_at( + &private_jwk(), + &nonce, + token_clock.now(), + )]); let expired_token_response = token_expiry .server .post(CREDENTIAL_PATH) @@ -1321,18 +1401,24 @@ async fn expiry_cleanup_preserves_live_state_and_releases_expired_state() { ); assert_eq!(token_expiry.issuer.call_count(), 0); - let mut nonce_config = loaded_config(); - nonce_config.store.access_token_lifetime_seconds = 5; - nonce_config.store.nonce_lifetime_seconds = 1; - let nonce_expiry = harness(nonce_config); + let nonce_config = loaded_config(); + let nonce_lifetime = nonce_config.store.nonce_lifetime_seconds as i64; + let nonce_clock = TestClock::default(); + let nonce_expiry = harness_on_clock(nonce_config, &nonce_clock); let token = access_token(&nonce_expiry.server).await; let nonce = minted_nonce(&nonce_expiry.server).await; - tokio::time::sleep(Duration::from_millis(1_100)).await; + // Past the nonce window and well inside the longer access token window, so + // the refusal below is the nonce and the token is still there to claim. + nonce_clock.advance(nonce_lifetime + 1); let expired_nonce_response = nonce_expiry .server .post(CREDENTIAL_PATH) .add_header("authorization", format!("Bearer {token}")) - .json(&credential_body(vec![proof_jwt(&private_jwk(), &nonce)])) + .json(&credential_body(vec![proof_jwt_at( + &private_jwk(), + &nonce, + nonce_clock.now(), + )])) .await; assert_eq!( expired_nonce_response.status_code(), @@ -1345,11 +1431,14 @@ async fn expiry_cleanup_preserves_live_state_and_releases_expired_state() { .server .post(CREDENTIAL_PATH) .add_header("authorization", format!("Bearer {token}")) - .json(&credential_body(vec![proof_jwt( + .json(&credential_body(vec![proof_jwt_at( &private_jwk(), &fresh_nonce, + nonce_clock.now(), )])) .await; + // A live token that was already claimed for the refused request, so + // correcting the nonce does not restore it. assert_eq!(retry.status_code(), StatusCode::UNAUTHORIZED); assert_eq!(nonce_expiry.issuer.call_count(), 0); } From 32ef1e83cd2da9169ce500785f6dde7ccf4e3c42 Mon Sep 17 00:00:00 2001 From: Jeremi Joslin Date: Wed, 16 Sep 2026 09:26:32 +0700 Subject: [PATCH 2/2] fix(docs): fail the evidence-anchor checker on an unlisted top-level directory A citation into a top-level directory REPOSITORY_ROOTS omits parses as no citation at all, so its anchor is checked against nothing. That gap used to surface only through a docs-suite test gated behind the path-filtered Docs checks job, so a PR that added a tracked directory without touching docs paths could merge clean and leave the gap for the next PR to hit. Add checkRepositoryRoots, which compares git's tracked top-level directories against REPOSITORY_ROOTS and runs from the CLI entry point on every invocation, including the path-filter-free Evidence anchors job that already gates every pull request. Rewrite the existing "names every top-level directory" test to call the new function against the real checkout instead of duplicating its git-diffing logic, and add tests proving it fails closed and names the missing directory. Signed-off-by: Jeremi Joslin --- docs/site/scripts/check-evidence-anchors.mjs | 36 +++++++++++++- .../scripts/check-evidence-anchors.test.mjs | 49 ++++++++++++------- 2 files changed, 64 insertions(+), 21 deletions(-) diff --git a/docs/site/scripts/check-evidence-anchors.mjs b/docs/site/scripts/check-evidence-anchors.mjs index 68d0896f11..768aec14a0 100644 --- a/docs/site/scripts/check-evidence-anchors.mjs +++ b/docs/site/scripts/check-evidence-anchors.mjs @@ -5,6 +5,7 @@ // line references they carry are inside those files, and the symbols they name are // present in at least one path the same anchor cites. +import { execFileSync } from 'node:child_process'; import { readFileSync, readdirSync, realpathSync, statSync } from 'node:fs'; import { dirname, relative, resolve, sep } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -602,6 +603,35 @@ function mdxPages(directory) { return pages.sort(); } +// Every top-level directory the repository tracks, checked against REPOSITORY_ROOTS. A +// citation into a directory the list omits parses as no citation at all, so its anchor is +// checked against nothing and the gap stays silent until a page happens to cite it. Git, +// not a directory listing, is what says which directories the repository keeps: a listing +// also carries build output and local tooling, and which of those are present differs +// between a clean CI checkout and a working machine, so a listing would fail for reasons +// that are not drift. `git ls-tree` reads the tree object HEAD already carries, so it also +// works in the shallow, single-commit checkout CI runs this script from. +export function checkRepositoryRoots({ repoRoot = resolve(scriptDir, '../../..') } = {}) { + const tracked = execFileSync('git', ['ls-tree', '-d', '--name-only', 'HEAD'], { + cwd: repoRoot, + encoding: 'utf8', + }) + .split('\n') + .filter((name) => name !== ''); + // The roots are regular expression source, so a dot-directory carries its escape. + const named = new Set(REPOSITORY_ROOTS.map((root) => root.replaceAll('\\', ''))); + const missing = tracked.filter((name) => !named.has(name)); + if (missing.length === 0) { + return []; + } + const directories = missing.length === 1 ? 'directory' : 'directories'; + return [ + `the repository tracks the top-level ${directories} ${missing.join(', ')}, which ` + + 'REPOSITORY_ROOTS in check-evidence-anchors.mjs does not name; a citation into one parses ' + + 'as no citation at all, so add it to REPOSITORY_ROOTS', + ]; +} + export function checkEvidenceAnchors({ repoRoot = resolve(scriptDir, '../../..'), docsRoot, @@ -856,12 +886,14 @@ export function parseArguments(args) { if (process.argv[1] && resolve(process.argv[1]) === scriptPath) { try { const options = parseArguments(process.argv.slice(2)); + const rootErrors = checkRepositoryRoots(); const result = checkEvidenceAnchors(options); + const errors = [...rootErrors, ...result.errors]; const counts = `${result.anchors} anchors, ${result.paths} cited paths, and ${result.symbols} cited symbols checked; ` + `${result.lineRefs} line-range citations found`; - if (result.errors.length > 0) { - console.error(result.errors.join('\n')); + if (errors.length > 0) { + console.error(errors.join('\n')); console.error(`Evidence anchor check failed: ${counts}.`); process.exitCode = 1; } else { diff --git a/docs/site/scripts/check-evidence-anchors.test.mjs b/docs/site/scripts/check-evidence-anchors.test.mjs index aabcd4ba25..1da4ad0ce6 100644 --- a/docs/site/scripts/check-evidence-anchors.test.mjs +++ b/docs/site/scripts/check-evidence-anchors.test.mjs @@ -9,8 +9,8 @@ import { fileURLToPath } from 'node:url'; import YAML from 'yaml'; import { - REPOSITORY_ROOTS, checkEvidenceAnchors, + checkRepositoryRoots, extractAnchors, extractSymbols, parseAnchor, @@ -886,24 +886,35 @@ test('root CI runs the anchor check on every pull request and gates the branch o }); test('names every top-level directory the repository tracks as a citation root', () => { - // A citation root the list does not name parses as no citation at all, so the anchor - // carrying it is checked against nothing. Git is what says which directories the - // repository keeps: a listing of the checkout also carries build output and local - // tooling, and which of those are present differs between a clean CI checkout and a - // working machine, so a listing would fail for reasons that are not drift. - const tracked = execFileSync('git', ['ls-tree', '-d', '--name-only', 'HEAD'], { - cwd: repositoryRoot, - encoding: 'utf8', - }) - .split('\n') - .filter((name) => name !== ''); - assert.ok(tracked.length > 0); - // The roots are regular expression source, so a dot-directory carries its escape. - const named = new Set(REPOSITORY_ROOTS.map((root) => root.replaceAll('\\', ''))); - assert.deepEqual( - tracked.filter((name) => !named.has(name)), - [], - ); + // checkRepositoryRoots carries the git-versus-REPOSITORY_ROOTS comparison itself; this + // test runs it against the real checkout so a directory the repository has grown that + // REPOSITORY_ROOTS does not yet name fails here the same way it fails in CI. + assert.deepEqual(checkRepositoryRoots(), []); +}); + +function gitCheckout(t, { directory }) { + const root = mkdtempSync(resolve(tmpdir(), 'registry-evidence-roots-')); + t.after(() => rmSync(root, { recursive: true, force: true })); + write(root, `${directory}/file.txt`, 'tracked\n'); + execFileSync('git', ['init', '--quiet'], { cwd: root }); + execFileSync('git', ['config', 'user.email', 'tests@example.invalid'], { cwd: root }); + execFileSync('git', ['config', 'user.name', 'Evidence Anchor Tests'], { cwd: root }); + execFileSync('git', ['add', '.'], { cwd: root }); + execFileSync('git', ['commit', '--quiet', '-m', 'tracked directory'], { cwd: root }); + return root; +} + +test('passes a checkout whose tracked top-level directories are all named in REPOSITORY_ROOTS', (t) => { + const root = gitCheckout(t, { directory: 'crates' }); + assert.deepEqual(checkRepositoryRoots({ repoRoot: root }), []); +}); + +test('fails closed when a tracked top-level directory is missing from REPOSITORY_ROOTS', (t) => { + const root = gitCheckout(t, { directory: 'unlisted-root' }); + const errors = checkRepositoryRoots({ repoRoot: root }); + assert.equal(errors.length, 1); + assert.match(errors[0], /unlisted-root/); + assert.match(errors[0], /REPOSITORY_ROOTS/); }); test('reads a citation into a top-level directory beside the crates and products', (t) => {