Repository navigation
Phase 6c: authentication — tests and doubles - #76
Merged
Merged
Conversation
Wahbeh-Mohammad
added
type:feature
Phase 6c, cut from main at f1fe848 with nothing of phase 6a present, so the steps take their own logger: keyword and AUTH-31 calls phase 3b's Body#replayable? directly. Twenty-five new lib files: auth.rb, the flat AuthResolutionError under error/, and twenty-three under auth/ -- the closed five-member Scheme set with ALL and .of, Requirement, Descriptor and the pure three-tier Resolver (AUTH-1..7); BearerToken, KeyCredential, NamedKeyCredential and PasswordCredential, each redacting in #to_s, #inspect and, for the two Data types, #pretty_print, because pp walks a Data's members and never calls an #inspect override (AUTH-8..10); the never-raising RFC 7235 list parser Challenges.parse over Challenge, the one fold point (AUTH-12, AUTH-13); BasicHandler over pack("m0") and never Base64 (AUTH-14); the challenge-only DigestHandler with MD5, MD5-sess, SHA-256 and SHA-256-sess, qop=auth or legacy, a per-nonce counter in its own BoundedMap and a typed failure for a credential ISO-8859-1 cannot carry (AUTH-15..22, AUTH-24); ChallengeHandlerChain with its proxy-aware header name and the hook adapter (AUTH-23, AUTH-25); the stateless KeyStamper (AUTH-26); BearerStamper with a lock-free hot path and XCUT-12's sanctioned lock-across-fetch (AUTH-34..36); AsyncBearerStamper with AUTH-37's three zones and one single-flight slot; BearerProvider.fetch_async as AUTH-11's never-raising default; and the AUTH pillar Step and AsyncStep at Stages::AUTH, built through .build(stamper:, challenge_hook:, logger:), forking for every drive, reading the cross-origin marker off the cursor and never a header, guarding HTTPS before any fetch or write, replaying a 401 at most once behind the replayability gate and closing the superseded response (AUTH-27..33, AUTH-38). Three earlier files widen in place: BoundedMap gains #update(key), the read-yield-write the nonce counter needs; Instrumentation::Events gains AUTH_REFRESH, the ninth event; lib/dexpace.rb gains the twenty-five-line Phase 6c block. Every file has a sig/ mirror; the strict Steep target is green with no relaxation and no Digest, SecureRandom or Random in any signature. Three existing tests change as pins the code invalidated: the smoke suite's layer table and its preloaded stdlib features, the seam surface's require pin (digest joins the four), and 5b's keys_test.rb (eight events become nine). The surface manifest is regenerated once, 956 to 1059 lines, all 103 rows read against the object model.
Review round 0's R0-3 and R0-4, both in DigestHandler. UnencodableCredentialError always named ISO-8859-1, and its message blamed the challenge for not advertising charset=UTF-8, even when the UTF-8 branch was the one that raised -- a BINARY-tagged credential against a charset=UTF-8 challenge fails "\xE4 from ASCII-8BIT to UTF-8" and was reported as a Latin-1 failure. #materialize now takes the branch's target Encoding and the error names it; the message's reason is keyed by the target (a private REASONS table) so each branch says why its encoding applied. The same branch also refuses a UTF-8-tagged credential carrying an invalid sequence: `encode` to the same encoding passes bytes through unvalidated, so such a value would have been hashed as it was, which is the silently wrong response R10 rejects. #compute took the nonce count before hashing, so a refused attempt consumed an nc and the next response on that nonce went out one higher than the server had seen. The credential is now materialised first -- the one step that can raise -- and the count taken after it, which is the order the design's own authorization_for fence has. The RBS mirrors follow: REASONS declared, the three private signatures that carry the materialised parts and the target updated.
Review round 0's R0-2. Regexp.new, unlike a Regexp literal, returns an unfrozen object, so the parser's eight timeout-compiled patterns were the one set of core patterns not frozen at load: phase 5a's HTTPDate::GRAMMAR freezes and its suite pins the property. The eight now freeze the same way, so the tests branch can pin "a private, frozen Regexp with a per-pattern timeout" on each of them and a removed `timeout:` runs red instead of surviving. Private constants; no surface change.
Review round 1's R1-1 and R1-3.
AsyncStep validated a challenge hook's future-settled value outside the
frame that closes the 401: a hook future fulfilling with a non-request
failed the step's future with the 401 body left open, contradicting the
class comment and AUTH-32. Step#consult's rescue becomes one
closing_on_error(response) frame both runtimes use; AsyncStep overrides
consult to pass a future through and checks the settled value inside the
same frame, so a String, or a future of a future, closes the 401 before
the frame fails the future. replacement! is strict everywhere.
UnencodableCredentialError carried the rescued conversion error as its
cause, whose message names the offending character (U+65E5) or byte
("\xE4") of the secret, and full_message renders a cause on every
supported Ruby (AUTH-8). Both raises in DigestHandler#materialize now
spell cause: nil, and the error carries the value's own encoding as
#source_encoding and in its message instead, so the diagnostic loses
only the character. BasicHandler raised the bare Encoding error for a
BINARY-tagged or invalid UTF-8-tagged field; each field is now
transcoded under its own name and refused as an InvalidArgumentError
naming the field and the two encodings, cause nil.
RBS mirrors follow; the surface manifest gains the one new reader.
Review round 2's R2-1 and R2-4, both in AsyncBearerStamper. The expired-or-missing zone derived the caller's future from the single-flight slot through Future#then, and #then wires the derived future's cancellation back to its source. The source here is the ONE slot every coalesced caller shares, and every arrival until the provider settles, so cancelling one request's future -- which AsyncStep#observe forwards from the step's future -- cancelled every other waiter, and every new request coalesced onto an already-cancelled future until the provider settled (AUTH-37, SEAM-18). R12 prescribes a second #on_settle and a second Completer; the stamper now builds each waiter's future that way, settled FROM the slot's settlement and never wired back to it, so cancelling a waiter detaches that waiter alone and the fetch, the cache and the other waiters are untouched. One private #settle classifies both completers by Future#then's three rules: a cancellation stays a cancellation (a provider that cancels its own fetch cancels the slot and every waiter with its reason), a failure is the same object, a success settles with the block's value; a stamp the outbound header grammar refuses fails that waiter only. The class comment's line beginning `@lock` read to YARD as an unknown tag; reworded. RBS mirror follows; no public surface changes.
Review round 3's R3-1, in both bearer stampers. AUTH-35's validation checked a fetched token for nil, class and expiry and nothing else, so a token whose `Bearer <token>` wire form the outbound header grammar refuses (HTTP-18: a trailing newline read off a file, a CR) was written into the cache. It can never be sent, so no 401 can ever arrive to evict it (AUTH-36), and it stays until it expires -- never, for a token with no expiry: the sync stamper raised HTTP-18's InvalidArgumentError on every later call with the provider never asked again, and the async stamper's fresh zone raised it synchronously out of a method that returns a Future, failing every later request through an AsyncStep with the transport never reached. AUTH-35 wants a misbehaving provider result uncached so a later request retries. The grammar check is now the fourth rejection in BearerStamper#validate and AsyncBearerStamper#invalid, a ProviderError whose message never names the token: nothing is cached, the sync call raises from inside the lock with @token untouched, the async waiters fail with it, the slot clears, and the next call fetches again. Checked where the token arrives, as KeyStamper checks its key where IT arrives (construction), not in BearerToken.build, whose contract is AUTH-9's non-blank rule. A cached token therefore always stamps, so #stamp's fresh and expiring zones cannot raise; #deliver's rescue stays for a request whose own derivation refuses. Comments on both stampers and ProviderError follow; no public surface changes, the RBS mirrors are unchanged.
The reconciled Layers class carried both phase 6a's retry pin and phase 6c's authentication pin beside the phase-3b through phase-5 cases and reached 104 lines, over Metrics/ClassLength's 100, which the honest RuboCop run sees and the nested-worktree rake gate does not. The two phase-6 cases move to a sibling PhaseSixLayers class, the shape phase 4a's reconciliation used for the same collision.
Twenty-nine suites, one per public auth file plus bounded_map_test.rb, the first true mirror of a private constant, plus five that carry no lib mirror and say so in their headers: step_bearer_challenge_test.rb (a second suite over step.rb), pillar_integration_test.rb (one example set over both runtimes), cross_origin_convergence_test.rb (Task 15, guarded on defined?(Dexpace::Redirect::Step) and skipping on this base with a reason naming 6b), matrix_facts_test.rb (Task 1's facts as a standing test, deriving the four Digest expectations rather than transcribing them) and error/auth_resolution_error_test.rb. Eight top-level test-support doubles, one class per file: ChallengeFixtures, FixedCnonce, SequencedTransport (named so as not to collide with the ScriptedTransport phase 6a is writing at the same time), SequencedAsyncTransport, ScriptedBearerProvider, ScriptedAsyncBearerProvider, SpyCursor and AuthFixtures. The AUTH-24 proof is deterministic: bounded_map_test.rb forces the interleaving between the read and the write through the block, and bearer_stamper_test.rb parks sixteen threads on a barrier so the single-flight fetch is counted, never timed. Every "never raises" claim is asserted on the value, identity claims use assert_same, the test helper is named dispatch because run is Minitest::Test#run, and every thread a test starts is joined.
Review round 0's findings, each with the line that now runs red on its revert. R0-1: AsyncStep's post-eviction routing was indistinguishable under the suite, because the real AsyncBearerStamper fetches through #stamp and #stamp_fresh alike once its cache is empty (mutation M45 survived). A new top-level double, SpyBearerStamper, answers "Bearer cached" from #stamp and "Bearer fresh" from #stamp_fresh and records every call, and two BearerTest cases drive a 401-then-200 through it: a successful eviction retries through #stamp_fresh and a failed one through #stamp. M45 now fails on ["Bearer cached", "Bearer fresh"] versus twice cached. R0-2: the parser suite pins each of the eight scanner patterns as a private, frozen Regexp with a non-nil #timeout, as http_date_test.rb pins CFG-31's grammar; a removed `timeout:` or `.freeze` is a red test. The suite crossed Metrics/ClassLength and is split into GrammarTest and LeniencyTest over one shared helper. R0-3: a BINARY-tagged credential under charset=UTF-8 raises naming UTF-8 with the conversion error as cause; a UTF-8-tagged credential with an invalid sequence is refused on its own bytes, cause nil; a Latin-1-tagged one is transcoded and accepted. The error suite asserts the UTF-8 message says the challenge advertised the charset and never that it did not. R0-4: a refused Digest attempt leaves the nonce's counter unset, and the next response on that nonce sends nc=00000001.
Review round 1's R1-1, R1-2, R1-3 and R1-5. R1-2: the AUTH-30 order was asserted by count alone, so a close-after- drive mutation survived on both interpreters. The replay's scripted reply is now a callable that reads the 401's close count AS the second drive reaches the transport, in step_test.rb, step_bearer_challenge_ test.rb and both async_step_test.rb branches; the sync and async close-after-drive mutations run red. R1-1: async_step_test.rb gains the future-fulfilled non-request shape, a String and a future of a future, each closing the 401 and failing the future with the InvalidArgumentError naming the class. R1-3: unencodable_credential_error_test.rb takes the source_encoding keyword and proves the error is raised with no cause inside an in-flight rescue; digest_handler_test.rb asserts nil cause, the source encoding, and that message, detailed_message, inspect, full_message and every each_cause message carry neither U+65E5, the character nor the password; basic_handler_test.rb asserts the typed, causeless refusal of a BINARY-tagged and an invalid UTF-8-tagged field naming no byte. R1-5: the async AUTH-35 test asserts the rejected results cache nothing through #evict_if_matches and the cache slot, so round 1's M51 runs red.
Review round 2's R2-1, R2-2 and R2-3. async_bearer_stamper_test.rb gains a CancellationTest: cancelling one of three coalesced waiters (a #stamp_fresh one among them) leaves the others pending, the provider's fetch unsettled and a new arrival coalescing onto the live slot with one fetch in all, and once the provider settles the survivors stamp the token, the cache holds it and the cancelled one carries its own reason; a provider cancelling its own fetch cancels every waiter AS a cancellation with the provider's reason, caches nothing and frees the slot for a retry; a token the outbound header grammar refuses fails that waiter alone and never raises into the settling thread. async_step_test.rb gains the same isolation through a real async pipeline: one cancelled request, the other two drive with the fresh token. Round 2's tree, the Future#then derivation, a completer wired back to the slot, a cancellation forwarded as a failure, the rescue dropped and a slot left set all run red on 4.0.6 and 3.2.11. digest_handler_test.rb pins the handler's increment to BoundedMap#update deterministically: the store's #[], #set and #put are narrowed in place to raise (the handler is frozen, so the store is not replaced), #update records its key, and two counts on one nonce read 00000001 and 00000002 through two recorded updates. The reviewer's read-then-set counter, which the sixteen-thread race never observed under the GVL, now raises on both interpreters. async_bearer_stamper_test.rb's AUTH-36 case gains the sync suite's exactness pins -- a doubled space, a superstring and the bare token do not evict -- so a substring comparison on the async half runs red as it already did on the sync one.
Review round 3's R3-1. A provider token the outbound header grammar refuses -- a trailing newline, a CR, a non-ASCII byte -- is now AUTH-35's fourth rejection, and the suites pin it where the round found it unobserved: bearer_stamper_test.rb (split into CacheTest, RejectionTest and EvictionTest under Metrics/ClassLength) asserts three such tokens each raise ProviderError with @token nil and no name of the token in the message, then the next call fetches the clean one; async_bearer_stamper_test.rb FailureTest asserts two waiters on one fetch both fail with it, nothing is cached, the slot is free, and the next #stamp returns a future that stamps the clean token -- never a synchronous raise -- and that an unusable BACKGROUND refresh is logged as http.auth.refresh without naming the token and caches nothing, the still-valid token stamped meanwhile. step_bearer_challenge_test.rb and async_step_test.rb carry the same through a real pipeline: one dispatch fails, the transport is never reached, the next two are 200 with one refetch. Round 3's CancellationTest case fed exactly such a token to prove #deliver's rescue; the fourth rejection now pre-empts it before the stamp, so the case becomes a request whose own derivation refuses, which is the raise left for the rescue to keep off the settling thread. Eight mutations run red on 4.0.6 and 3.2.11: the grammar check dropped from either stamper, the sync token cached before validation, the rescue dropped, either message naming the token, the async write made unconditional, and the refusal raised out of the settle block.
Wahbeh-Mohammad
force-pushed
the
24-phase-6c-authentication
branch
from
September 19, 2026 09:56
430c527 to
5a74e17
Compare
Wahbeh-Mohammad
force-pushed
the
24-phase-6c-authentication-tests
branch
from
September 19, 2026 09:56
13fe732 to
a870f7e
Compare
Wahbeh-Mohammad
changed the base branch from
24-phase-6c-authentication
to
main
September 19, 2026 10:02
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #24. Second PR of phase 6c's stack — the tests for the code PR below it. Targets that PR's branch; the phase record is the next PR up.
What lands
38 files, +4,489: twenty-six suites under
gems/dexpace-core/test/dexpace/auth/mirroringlib/one file per file (plusstep_bearer_challenge_test.rb,pillar_integration_test.rb,matrix_facts_test.rband the guardedcross_origin_convergence_test.rb),auth_test.rb,error/auth_resolution_error_test.rb,bounded_map_test.rb(the first true test mirror of aprivate_constant: deterministic barrier tests for#updateunder sixteen threads), and nine top-level doubles:SequencedTransport/SequencedAsyncTransport— a scripted sequence of responses, exceptions or callables (a callable can read a close flag as the call arrives — how the close-before-replay ORDER is asserted); named to avoid colliding with 6a's concurrentScriptedTransport— the pair is reconciled after both lanes land.ScriptedBearerProvider/ScriptedAsyncBearerProvider,SpyBearerStamper(tells#stamp_freshfrom#stamp),SpyCursor(counts#fork/#callon a driver-made cursor),FixedCnonce,ChallengeFixtures,AuthFixtures(https_request,closable_response,unauthorized,auth_pipeline,closes_of). 4c'sForkingProbe/StateProbe, 3b'sFakeBody, phase 2'sFakeTransport/FakeAsyncTransport, 5a'sFakeClock, 5b'sRecordingSink, 4b'sRecoveryFixturesconsumed unchanged.Every file's header names the IDs it exercises; every suite over 100 lines is split into nested classes; every thread is joined; nothing uses
assert_nothing_raised; no test reads real ENV; theAUTH-24handler test pins the increment toBoundedMap#updatedeterministically (R2-2), not by a race.What the suites prove rather than restate: the four Digest expectations derived on each interpreter and only produced values committed; the parser's twelve assertions plus the timeout pins on its eight patterns; every credential's
#to_s/#inspect/#pretty_printand every error's#message/#detailed_message/#inspect/#full_messagefree of the secret (R1-3); the step forking once per drive and never calling its cursor; the 401 closed as the replay reaches the transport (R1-2); the marker read from cursor state with aForkingProbeforking{cross_origin: true},{cross_origin: false}and no REDIRECT step at all; the bearer hot path lock-free, sixteen threads coalescing on one fetch, exact-match eviction on both stampers (R2-3), one waiter's cancel detaching that waiter alone (R2-1); the fourthAUTH-35rejection uncached on both stampers with a refetch (round 4); the async step's every failure a failed future.Verification
bundle exec rakeon Ruby 4.0.6 at this tip: exit 0, all seventeen gates green —test:gems2,353 runs / 60,235 assertions / 1 skip (the guarded end-to-end test — the one skip in the tree), line coverage 99.98%; honest RuboCop 464 files clean.Known follow-ups from the final review (not blocking)
bearer_stamper_test.rb:95: add a warm-cache (refresh-path) rejection case for the syncBearerStamper— an expired-at-fetch token and a grammar-refused token arriving on a refresh, each rejected and uncached — sovalidateskipped on refresh runs red.cross_origin_convergence_test.rbskips on this base by design (skip … unless defined?(Dexpace::Redirect::Step)); 6b un-guards it.