Skip to content

feat(buzz-auth): add production NIP-FI federated assertion runtime - #7109

Open
wpfleger96 wants to merge 13 commits into
mainfrom
duncan/nip-fi-assertion-runtime
Open

feat(buzz-auth): add production NIP-FI federated assertion runtime#7109
wpfleger96 wants to merge 13 commits into
mainfrom
duncan/nip-fi-assertion-runtime

Conversation

@wpfleger96

@wpfleger96 wpfleger96 commented Aug 31, 2026

Copy link
Copy Markdown
Member

Depends on #6994 (merged).

Stack: PR 2 #6994 (merged) → this PR (#7109) → PR 4 #7148 → PR 5

What

Production assertion runtime for NIP-FI Phase A: JWKS caching layer, SSRF-hardened HTTP fetcher, startup validation gate, NIP-11 discovery serialization, federated assertion verifier with sealed key-source authority, and supporting invariant tests.

Changes

crates/buzz-auth/src/nip_fi/jwks/

ProductionJwksSource<F> implements the sealed IssuerKeySource trait:

  • HttpJwksFetcher: reqwest-backed fetch with SSRF protection — URI validation (HTTPS, no credentials, no fragment, no host matched by the shared enumerated deny policy), per-fetch DNS resolution rejecting any resolved address matched by the shared enumerated deny policy, address pinning to prevent DNS rebinding TOCTOU, redirect denial, incremental body streaming capped at 512 KiB before any parse
  • IPv6 host extraction via typed Url::host() accessor (strips brackets before SSRF check and provides the correct bare input form for reqwest resolve() pinning; the bracketed form from host_str() fails IpAddr::parse and does not match the URL authority key)
  • Complete-operation deadline via tokio::time::timeout covering DNS resolution through body streaming
  • Bounded periodic refresh with configurable interval and hard snapshot deadline
  • Cancellation-safe RAII refresh permit: dropped on future cancellation so the next caller can re-fetch
  • Content-digest-gated generation counter: identical re-fetches preserve generation; key rotations advance it
  • Injectable clock (now_fn: Arc<dyn Fn() -> DateTime<Utc> + Send + Sync>): production uses Arc::new(Utc::now); all four deadline creation and expiry checks use (self.now_fn)(), enabling controlled-time testing without wall-clock sleep

JwksSourceContract: a closed value type that is the single source of truth for the three deployment fields whose change alters which keys the runtime trusts and how long it trusts them:

  • jwks_uri — selects the authenticated key source; validated at construction (HTTPS, no credentials/fragment, no bare private-IP host); stored as the Url-normalized form so that equivalent spellings (uppercase host, explicit default port :443, dot-segment paths like /.well-known/./jwks.json) converge to the same AssertionPolicyId
  • refresh_interval_seconds — defines bounded refresh behavior; positive, ≤ 1 year, strictly < key_snapshot_hard_deadline_seconds
  • key_snapshot_hard_deadline_seconds — defines the source's accepted time rule; every VerifiedAssertion.revalidation_dependencies deadline derives from this

JwksSourceContract is a required IssuerPolicy input and is included in derive_assertion_policy_id after a domain separator. IssuerJwksConfig embeds the contract instead of independently restating these fields — startup validation rejects any contract mismatch (NipFiStartupError::JwksContractMismatch).

crates/buzz-auth/src/nip_fi/verifier.rs

  • FederatedAssertionVerifier<S>: provider-neutral verifier over a closed multi-issuer registry and sealed IssuerKeySource
  • Arc<S>: IssuerKeySource forwarding impl (blanket seal for Arc<S> in the sealed module) — one Arc<ProductionJwksSource> can be shared across multiple verifiers; all observe JWKS refreshes through the shared cache without rebuilding the verifier
  • AssertionKeySet: crate-private constructor seals issuer binding — no external crate can relabel issuer B's JWKS as issuer A
  • Sealed IssuerKeySource trait closes the authority-construction seam at both ends

crates/buzz-core/src/network.rs

Renamed is_private_ip to is_not_global_unicast (compat alias retained) and restored the complete IANA deny/exception table from this branch's own history (272dacadb). The predicate is an enumerated deny/explicit exception policy: addresses covered by a named deny rule are rejected; addresses not covered by any explicit deny rule (e.g. fe00::1) pass through. Deny rules are derived from the IANA Special-Purpose Address Space registries (last updated 2025-10-09), with globally-reachable exceptions carved out explicitly (e.g. PCP/TURN anycast inside 2001::/23).

Blocked IPv4 classes: loopback (127/8), private RFC 1918 (10/8, 172.16/12, 192.168/16), link-local (169.254/16), unspecified (0/8), broadcast, CGNAT/RFC 6598 (100.64/10), benchmarking/RFC 2544 (198.18/15), IETF Protocol Assignments (192.0.0.0/24, globally reachable exceptions: 192.0.0.9 PCP anycast RFC 7723 and 192.0.0.10 TURN anycast RFC 8155), documentation/RFC 5737 (192.0.2/24, 198.51.100/24, 203.0.113/24), deprecated 6to4 relay anycast (192.88.99.0/24, RFC 7526, global=None → conservative deny), multicast/RFC 5771 (224/4), reserved class-E (240/4).

Blocked IPv6 classes: loopback (::1), unspecified (::), ULA (fc00::/7), link-local (fe80::/10), deprecated site-local (fec0::/10, RFC 3879), multicast (ff00::/8), IETF Protocol Assignments envelope (2001::/23, globally reachable exceptions: 2001:1::1–::3 PCP/TURN/DNS-SD anycast, 2001:3::/32 AMT RFC 7450, 2001:4:112::/48 AS112-v6 RFC 7535, 2001:20::/28 ORCHIDv2 RFC 7343, 2001:30::/28 DETs RFC 9374), documentation (2001:db8::/32 RFC 3849, 3fff::/20 RFC 9637), 6to4 (2002::/16, RFC 3056), Discard-Only (100::/64, RFC 6666), Dummy IPv6 Prefix (100:0:0:1::/64, RFC 9780), SRv6 SIDs (5f00::/16, RFC 9252), NAT64 local-use (64:ff9b:1::/48, RFC 8215). IPv4 embedded in mapped, compatible, NAT64 well-known (64:ff9b::/96), and SIIT IPv4-translated (::ffff:0:0:0/96) forms is checked recursively. All three callers (JWKS boundary, webhook SSRF, link-preview SSRF) inherit the complete predicate through the inline is_private_ip compatibility alias.

Invariant coverage

Canonical URI convergence. jwks_contract_uri_canonicalization_convergence_and_divergence asserts that uppercase host, explicit :443, and dot-segment path (/.well-known/./jwks.json) each produce the same AssertionPolicyId as the canonical form; a genuinely different host or path diverges. Mutation: storing raw input bytes instead of parsed.to_string() turns the three convergence assertions red.

Resolved-target and pin-input seam. resolved_target_and_pin_key_seam_public_ipv6_and_fec0_rejection carries a public 2606:4700::1 URI through all three stages of fetch_jwks_inner: extract_url_host_and_port yields the bare host (no brackets), resolve_and_check_ssrf takes the IP-literal fast path and returns the accepted IpAddr, and the extracted host string equals the URL authority form (verifying the correct bare input to reqwest's resolve() pin call). fec0::1 traverses the same extraction and SSRF stages and is rejected as InvalidUri. Network-free: both addresses are IP literals with no DNS lookup. Mutation: restoring host_str() brackets the address, IpAddr::parse fails, the SSRF fast path is unreachable, and all three assertions flip red.

Controlled original-deadline rotation. shared_arc_source_verifier_rejects_expired_a1_accepts_a2 uses an AtomicI64-backed injectable clock to advance past A1's original absolute deadline without wall-clock sleep. A1's deadline is computed at T0 and never mutated. The clock advances to T0 + HARD_DEADLINE_SECS + 1; get_snapshot fires a re-fetch and installs A2. One unchanged FederatedAssertionVerifier then rejects A1-signed tokens (deadline enforced by the key_set read path) and accepts A2-signed tokens, proves A2's generation is strictly greater, and confirms A2's deadline is later than A1's original. Mutation oracle: replace the shared Arc with an independently constructed source built from the same configs and sharing the same controlled clock, warmed with a separate A1 fetch before advancement. Post-advancement, key_set() on the verifier's independent source filters the expired A1 snapshot (filter(|c| now < c.hard_deadline)) and returns no keys — the verifier never re-fetches and never observes A2. A1-reject stays green (the independent cache is also expired, so no A1 keys are served), but A2-accept flips red, because the verifier never observes A2. A2 acceptance is the reliable shared-source oracle.

Complete SSRF classifier boundary. JWKS-boundary tests cover every newly restored class through validate_jwks_uri (URI-validation path): 192.0.0.1 (IETF Protocol Assignments interior), 192.0.0.9/.10 (PCP/TURN anycast global exceptions), 192.88.99.1 (deprecated 6to4 anycast), 2001:2::1 (2001::/23 interior), 2001:1::1 (2001::/23 global exception), 100::1 (Discard-Only), 3fff::1 (documentation), and 5f00::1 (SRv6 SIDs). URI validation and resolved-target enforcement share the same is_not_global_unicast predicate, so these URI-path tests exercise the complete classifier table. All pass mutation: removing any deny branch makes the rejection assertion red; removing any exception branch makes the acceptance assertion red. The resolved-target enforcement path is covered separately by resolved_target_and_pin_key_seam_public_ipv6_and_fec0_rejection for ::1 (loopback), public 2606:4700::1, and fec0::1.

@wpfleger96
wpfleger96 requested a review from a team as a code owner August 31, 2026 15:24
@wpfleger96
wpfleger96 force-pushed the duncan/nip-fi-assertion-runtime branch from f6c3981 to 7b4870b Compare August 31, 2026 16:34
@wpfleger96 wpfleger96 changed the title feat(auth): NIP-FI Phase A PR 3 — production assertion runtime feat(auth): add JWKS caching, startup validation, and NIP-11 discovery for federated identity Aug 31, 2026
@wpfleger96 wpfleger96 changed the title feat(auth): add JWKS caching, startup validation, and NIP-11 discovery for federated identity feat(auth): add federated identity assertion runtime Aug 31, 2026
@wpfleger96
wpfleger96 force-pushed the duncan/nip-fi-assertion-runtime branch from 7b4870b to eabaaf7 Compare August 31, 2026 17:01
@wpfleger96
wpfleger96 force-pushed the duncan/nip-fi-assertion-runtime branch from eabaaf7 to 620dca3 Compare August 31, 2026 17:15
@wpfleger96
wpfleger96 force-pushed the duncan/nip-fi-assertion-runtime branch 3 times, most recently from d11c6b2 to 86791b2 Compare August 31, 2026 18:01
@wpfleger96
wpfleger96 force-pushed the hayt/nip-fi-schema-foundation branch from 0534277 to 3c2c919 Compare August 31, 2026 18:10
@wpfleger96
wpfleger96 force-pushed the duncan/nip-fi-assertion-runtime branch 3 times, most recently from f6fe646 to 122ac43 Compare August 31, 2026 20:49
Base automatically changed from hayt/nip-fi-schema-foundation to main August 31, 2026 23:40
Duncan and others added 5 commits August 31, 2026 19:43
Add the JWKS discovery/caching layer, startup validation gate, and
NIP-11 discovery output that complete the NIP-FI assertion runtime.

The verifier (PRs 1–2) already defined the sealed IssuerKeySource trait
and AssertionKeySet constructor as placeholders for this PR. This PR
fills that contract with a production implementation:

- jwks: ProductionJwksSource<F> implements IssuerKeySource via an
  injectable JwksFetcher trait (sealed; HttpJwksFetcher for production).
  Bounded periodic refresh; coalesced in-flight; try_read/try_lock for
  async-safe synchronous key_set() path. Never serves an expired
  snapshot; fails closed on fetch/parse error. [FI-TRACE-JWKS-REMOVE]

- startup: validate_nip_fi_config() rejects incomplete or unsafe
  configurations before the relay accepts protected traffic: empty
  registry, unmatched JWKS configs, invalid timing bounds, and
  current-status issuers missing a JWKS source. Off/DenyProtected modes
  accept without validation. [FI-INV-14, FI-INV-15]

- discovery: FederatedIdentityDiscovery serializes the NIP-11
  federated_identity object. Never exposes enrollment mode, issuer
  URLs, audiences, or deployment-local identifiers.
  [FI-TRACE-DISCOVERY-PRIVATE]

- config: IssuerRegistry gains all_policies() iterator.
- verifier: sealed module promoted to pub(crate) for jwks access;
  AssertionKeySet::new #[allow(dead_code)] removed (now has real caller).

Security checklist:
- Issuer binding sealed at constructor: no relabelling possible
- Hard deadline enforced on every snapshot access
- MAX_JWKS_RESPONSE_BYTES checked before parse
- Key count bounded by MAX_JWKS_KEYS
- try_read/try_lock: fails closed rather than panicking or blocking
- No key material, issuer URLs, or token bytes in errors or Debug

Tests: 23 new unit tests (12 JWKS, 11 startup); all green.

Co-authored-by: Cea Stapleton Cordasco <261786559+cea@users.noreply.github.com>
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…n and comment quality

HTTP boundary (finding 1):
- Add validate_jwks_uri(): HTTPS-only, no credentials/fragments, bare IP
  private-address rejection via buzz_core::network::is_private_ip
- HttpJwksFetcher::new() builds a hardened client: no redirects, 10s intrinsic
  deadline; with_client() documents caller invariants
- Stream response body incrementally (bytes_stream + StreamExt), stop at
  MAX_JWKS_RESPONSE_BYTES + 1 before any deserialization
- Reject non-2xx status before reading body
- Add reqwest 'stream' feature to workspace; add futures-util to buzz-auth deps
- Add MAX_JWKS_TIMING_SECONDS = 1 year upper bound on timing fields
- Regression tests: non-HTTPS, loopback/private IP, credentials, fragment,
  oversized timing, duplicate issuer all rejected at construction

CurrentStatus posture (finding 2):
- Rename error variant DuplicateIssuer(String) -> DuplicateIssuer (sanitized)
- Add UnsupportedPosture error variant
- validate_nip_fi_config() rejects any CurrentStatus policy with
  UnsupportedPosture — verifier has no status witness; startup fails closed
- discovery.rs: remove FreshnessClassDiscovery::CurrentStatus variant and
  FederatedIdentityDiscovery::current_status() constructor entirely
- Test asserts rejection both with and without JWKS config

Duplicate issuer detection (finding 3):
- validate_nip_fi_config(): explicit duplicate detection in JWKS config slice
  (collect() was silently overwriting); returns DuplicateIssuer on collision
- ProductionJwksSource::new(): rejects duplicate issuer via HashMap::contains_key
  before insert

Timing bounds and overflow (finding 4):
- MAX_JWKS_TIMING_SECONDS constant bounds both refresh and hard-deadline fields
- i64::try_from() + Duration::try_seconds() eliminates u64->i64 cast panic
- Validated at both ProductionJwksSource::new() and validate_nip_fi_config()
- Test: new_rejects_timing_above_maximum()

Generation monotonicity (finding 5):
- Replace wall-clock millis with SHA-256 content digest per issuer
- Generation counter advances (saturating_add) only when digest changes;
  identical documents preserve the prior generation
- Regressions: generation_stable_for_identical_document(),
  generation_advances_for_changed_document()

Clippy (finding 6):
- manual_async_fn: replaced RPITIT form with native 'async fn' in impl block
- single_match (startup): replaced match { None => .., Some(_) => {} } with
  if let / !contains_key
- unnecessary_get_then_check: replaced .get().is_none() with !contains_key()

Comment quality (all files):
- Remove module-to-PR table from nip_fi/mod.rs
- Remove all 'Phase A', 'PR 1/3', 'PRs 4-5' references from every doc comment
- Remove WHAT comments (field-name paraphrases, narrated steps, section
  banners with no contract content, 'Construct with a default reqwest client')
- Retain WHY: security invariants, exact NIP-FI spec refs, fail-closed choices,
  FI-TRACE/FI-INV stable identifiers

Co-authored-by: Cea Stapleton Cordasco <261786559+cea@users.noreply.github.com>
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…nternal markers

- Remove with_client() bypass: HttpJwksFetcher is now a unit struct;
  each fetch_jwks call builds a dedicated per-request pinned client.
- Add resolve_and_check_ssrf: DNS-resolves host:port via spawn_blocking,
  rejects any resolved private/reserved IP (closes DNS-rebinding TOCTOU).
- Per-request client enforces: redirect(Policy::none()), no_proxy(),
  .resolve(host, pinned_ip), and timeout(JWKS_REQUEST_TIMEOUT_SECS).
- Drop unused client field (dead_code warning) now that no shared pool
  is needed.
- Remove pure-paraphrase doc on IssuerRegistry::all_policies(); replace
  with doc stating constraint (unspecified order, startup use).
- Remove all 'PR N' internal markers from doc comments; replace with
  production-stable references to the jwks runtime.

Co-authored-by: Cea Stapleton Cordasco <261786559+cea@users.noreply.github.com>
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…h_jwks

- Call validate_jwks_uri at entry of fetch_jwks_inner: direct callers of
  HttpJwksFetcher are protected regardless of ProductionJwksSource
  pre-validation. HTTP/credentials/fragment URIs rejected before any
  DNS resolution or connection attempt.
- Introduce with_deadline(fut, duration): private generic helper that
  wraps any future in tokio::time::timeout. HttpJwksFetcher::fetch_jwks
  passes fetch_jwks_inner(uri) through it with the fixed 10-second
  constant. Remove the RequestBuilder::timeout — the outer deadline
  covers the whole operation including a stalled OS resolver.
- Add with_deadline_fires_before_outer_guard: tokio::test(start_paused)
  passes std::future::pending() to with_deadline with Duration::ZERO.
  The inner timeout fires immediately; removing it leaves the future
  permanently pending and the outer test guard fires — seam verified.
- Fix IPv6-literal handling in resolve_and_check_ssrf: use (host, port)
  tuple form of ToSocketAddrs, not format!("{host}:{port}"), which is
  ambiguous for IPv6 addresses returned without brackets by host_str().
  Add IP-literal fast path that skips the OS resolver for bare IP hosts.
- Add production-boundary tests: four HttpJwksFetcher direct-call
  regressions (http/credentials/fragment/private-IP) and two IPv6 SSRF
  fast-path tests (loopback rejected, public accepted).
- Add tokio test-util dev-dependency to buzz-auth for start_paused.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…nvariant tests

Network policy (buzz-core):
- Rename is_private_ip → is_not_global_unicast; is_private_ip alias preserved for
  unowned callers. Registry source: IANA IPv4/IPv6 Special-Purpose Address Space
  (registries last updated 2025-10-09, retrieved 2026-08-31; URLs in source doc
  comment).
- Implement the IANA deny/exception table: outer predicate denies ranges whose
  registry entry is non-global or blank; explicit globally-reachable exceptions
  carved out inside otherwise-denied envelopes. IPv4 embedded in IPv4-mapped,
  IPv4-compatible, NAT64 well-known (64:ff9b::/96), and SIIT IPv4-translated space
  is evaluated recursively against the IPv4 table — registry global=True on the
  IPv6 wrapper does not bypass the embedded-address check.
- IPv4: add 192.0.0.0/24 IETF Protocol Assignments (global=False) with globally-
  reachable exceptions 192.0.0.9 (PCP anycast, RFC 7723) and 192.0.0.10 (TURN
  anycast, RFC 8155); add 192.88.99.0/24 deprecated 6to4 relay anycast (global=None
  — conservative posture: block).
- IPv6: replace individual Teredo/benchmarking/ORCHID checks with the 2001::/23
  IETF Protocol Assignments envelope (global=False). Globally-reachable exceptions
  inside the /23 are allowed: 2001:1::1/2/3 (PCP/TURN/DNS-SD anycast), 2001:3::/32
  (AMT), 2001:4:112::/48 (AS112-v6), 2001:20::/28 (ORCHIDv2, global=True),
  2001:30::/28 (DETs, global=True). Add 100:0:0:1::/64 dummy prefix (RFC 9780),
  3fff::/20 documentation (RFC 9637), 5f00::/16 SRv6 SIDs (RFC 9252).
  2001:db8::/32 (outside 2001::/23) remains a separate check.
- Consumer audit: buzz-workflow (CallWebhook) and desktop link_preview use the
  is_private_ip alias; the stricter predicate closes all new ranges for both callers.

Cancellation-safe refresh permit (buzz-auth):
- Per-issuer OwnedMutexGuard spans the complete fetch and state commit; cancelled
  callers release the permit on drop — no manual flag to poison.
- ScriptedFetcher replaces BlockingFetcher + SequencedFetcher: a VecDeque of
  FetchStep{entered, release} makes call order self-documenting without comments.
- concurrent_refresh_coalesces_without_second_fetch: entered barrier proves permit
  ownership before the second call; assert call_count == 1.
- aborted_first_caller_releases_permit_for_next_caller: pending_step returns the
  release sender, which is held until after abort — task is genuinely blocked (not
  resolved via error path) when cancelled. assert call_count == 2.

Central invariant regressions (buzz-auth):
- expired_snapshot_never_served_after_hard_deadline: hard-deadline expiry closes
  both the async and synchronous snapshot paths.
- two_issuer_keys_and_generations_are_isolated: advancing A's document advances
  A's generation only; B's key binding and generation are unchanged.

Architecture docs (ARCHITECTURE.md):
- Update is_private_ip function-table entry to is_not_global_unicast with compat alias.
- Rewrite SSRF Protection section: deny/exception-table framing, embedded-IPv4
  recursive evaluation, all three audited callers.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested

Reviewed head 122ac4329e9090667ac27984fb43883405f1d3d0 against base bd851f85e746ff7bdadcede58c130dc5e57777da. This review covers the assertion-runtime building blocks, not later relay ingress, binding, or lifecycle integration. The PR base moved to 9ab163190b4d4994a742a66d5fa4cae23c35fa4e during review; I reread the changed governing guidance and confirmed the NIP-FI specification and base auth contract files remained byte-identical. The reviewed head did not change.

P2: Keep the production key source refreshable after verifier construction

FederatedAssertionVerifier::new(registry, source) consumes the source into its private key_source field. The new ProductionJwksSource is not cloneable, and the sealed IssuerKeySource trait has no implementation for Arc<S> or &S. Its only refresh entry point is get_snapshot(&self); key_set() only reads the cache. No verifier accessor or refresh method restores access to that source.

Consequently, the supported sequence of constructing a source, warming it, and moving it into a long-lived verifier permanently freezes that verifier's JWKS. It misses subsequent rotations and, when the warmed snapshots reach their hard deadlines, returns KeySourceUnavailable for otherwise-valid tokens even if every issuer endpoint is healthy. An external orchestration task cannot solve this through the current public API. The two-issuer test masks the boundary by doing every refresh before constructing separate pre/post verifiers.

Provide a sealed sharing/refresh path that retains the same cache, such as a cloneable production source sharing its state or an in-crate Arc forwarding implementation. Add a public-API regression that constructs one verifier first, refreshes its shared source through rotation and beyond the initial deadline, and verifies that the same verifier rejects the removed key and accepts the current key.

Anchors: source ownership and cache, refresh/read paths, verifier ownership, test lifecycle.

P2: Normalize IPv6 literals before passing them to the resolver

fetch_jwks_inner passes parsed.host_str() directly into resolve_and_check_ssrf. In the pinned url 2.5.8 dependency, host_str() explicitly retains IPv6 square brackets. Thus an otherwise-valid configuration such as https://[2606:4700::1]/jwks.json passes URI validation but sends [2606:4700::1] to the helper. This fails the IpAddr fast path, and (&str, u16)::to_socket_addrs() sends that bracketed hostname to OS resolution rather than parsing it as an IPv6 address. The fetch fails before TLS/HTTP and the issuer never obtains a snapshot. The helper's comment claims the opposite host_str() behavior, and its IPv6 test supplies unbracketed input, bypassing the actual boundary.

Use the typed Url::host() value for IP literals, or normalize the resolver input before parsing. Cover the real URL-to-resolver seam with a public IPv6 literal; retain the private/reserved IPv6 rejection checks.

Anchors: actual fetch boundary, resolver, dependency contract.

Nonblocking follow-up: the new configurable JWKS endpoint and timing contract is not included in IssuerPolicy::id(). Complete that policy-identity witness before final-admission or reload integration relies on it for invalidation (NIP-FI §228–246). No such stale-admission consumer was established within this PR’s building-block scope, so this is not a third blocker.

Nonblocking inherited security limitation: the shared classifier permits deprecated site-local IPv6 fec0::/10, as it did at the reviewed base. The new JWKS boundary inherits that behavior: a configured hostname resolving there would pass the address filter if the deployment routes that space. Consider explicitly rejecting site-local targets; no affected deployment or newly weakened existing consumer was established in this review. Do not use an unconditional 2000::/3 gate that accidentally removes the intentional embedded-IPv4 translation exceptions.

Validation: source-only review of immutable files and dependency contracts, with independent review lanes. No checkout, build, tests, or PR code execution performed. This is not a NIP-FI conformance certification.

…10 block

Four production-blocking defects identified in Carl's Pass 1 review, plus the
fourth issue (policy ID / JWKS config fields) reported as a contract question
requiring design decision before implementation:

1. Arc<S> forwarding IssuerKeySource impl: FederatedAssertionVerifier<S> consumed
   S by value, making ProductionJwksSource (not Clone) non-shareable across
   verifiers. Add sealed::Sealed blanket for Arc<S> and IssuerKeySource for
   Arc<S>, forwarding to the inner source. Shared-rotation regression test proves
   one long-lived verifier observes key rotation through a shared Arc cache; the
   mutation (no sharing) turns the test red.

2. IPv6 host normalization: fetch_jwks_inner called Url::host_str() which returns
   bracket-wrapped IPv6 literals (e.g. '[2606:4700::1]'). IpAddr::parse fails on
   bracketed form, falling to DNS; reqwest resolve() keyed on bracketed host
   doesn't match the URL authority, bypassing DNS pinning. Fix: use typed
   Url::host() and stringify Ipv6Addr without brackets. Tests verify loopback
   and fec0::1 site-local URIs are rejected as InvalidUri via the correct
   SSRF-check path.

3. fec0::/10 deprecated site-local: is_not_global_unicast didn't block fec0::/10
   (RFC 3879 deprecated IPv6 site-local). Add the predicate and table-driven tests
   confirming fec0::1 through feff::1 are blocked; verify boundary addresses.
   All three existing callers (JWKS, webhook, link-preview) inherit the stricter
   predicate by construction.

Issue 4 (IssuerPolicy::id() missing JWKS endpoint/timing fields): IssuerJwksConfig
fields live outside IssuerPolicy; folding them into the existing id() would change
the settled identity contract. Two minimal options reported to Paul for product
decision before implementing.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
@wpfleger96 wpfleger96 changed the title feat(auth): add federated identity assertion runtime fix(buzz-auth): NIP-FI Phase A PR 3 — production assertion runtime Sep 1, 2026
…nput

The three JWKS deployment fields (jwks_uri, refresh_interval_seconds,
key_snapshot_hard_deadline_seconds) were absent from AssertionPolicyId.
A different endpoint serves different keys; a looser hard deadline extends
the valid window beyond what the new policy intends. Both changes must
invalidate prepared evidence by moving the policy ID.

Introduce JwksSourceContract, a closed value type that:
- validates the URI (HTTPS, no credentials/fragment, no bare private-IP)
  and both timing fields (positive, bounded, refresh < deadline) at
  construction — invalid values are caught at config time, not at first
  token verification
- is the single source of truth for these fields; IssuerJwksConfig embeds
  it instead of independently restating the three values, eliminating the
  silent-drift hazard
- is a required parameter to IssuerPolicy::new, included in
  derive_assertion_policy_id after a domain separator; each of the three
  fields independently changes the policy ID when mutated (tested)

Startup validation replaces per-field URI/timing checks (now redundant
since JwksSourceContract::new performs them) with a contract mismatch
check — NipFiStartupError::JwksContractMismatch fires when the config
contract and policy contract drift apart.

Behavior tests added:
- assertion_policy_id_moves_when_jwks_uri_changes
- assertion_policy_id_moves_when_refresh_interval_changes
- assertion_policy_id_moves_when_hard_deadline_changes
- assertion_policy_id_is_stable_for_same_jwks_contract (determinism)
- key_rotation_does_not_change_assertion_policy_id
Each test carries a mutation comment naming the hash-omission that turns
it red.

JwksSourceContract exported from nip_fi/mod.rs and lib.rs.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
@wpfleger96 wpfleger96 changed the title fix(buzz-auth): NIP-FI Phase A PR 3 — production assertion runtime feat(buzz-auth): add production NIP-FI federated assertion runtime Sep 1, 2026
…line-crossing oracle

Three regression gaps identified by Thufir's exact-head review of cd0b979:

1. URI canonicalization: JwksSourceContract::new now calls Url::parse and stores
   the canonical Url::to_string() form instead of the caller's raw bytes. The url
   crate lowercases scheme/host, strips explicit default ports (:443), and resolves
   dot-segments — equivalent endpoint spellings now produce identical policy IDs.
   Re-validates on the canonical form before storing. Test
   jwks_contract_uri_canonicalization_convergence_and_divergence proves uppercase
   host and explicit-port converge while genuinely different hosts diverge; mutation
   (raw storage) turns both convergence assertions red.

2. IPv6 host extraction seam: extracted extract_url_host_and_port as pub(crate) fn
   so tests can assert the host string directly without a live network request.
   Test extract_url_host_and_port_strips_ipv6_brackets_for_public_address asserts
   https://[2606:4700::1]/... yields bare "2606:4700::1", that the string parses as
   IpAddr (fast path reachable), and that it has no leading bracket; mutation
   (format!("[{}]", addr)) turns all three assertions red.

3. Deadline-crossing oracle: shared_arc_source_verifier_rejects_expired_a1_accepts_a2
   uses force_expire_snapshot_for_test (new cfg(test) helper on ProductionJwksSource)
   to simulate A1 hard deadline expiry without a wall-clock sleep, then re-fetches to A2
   through the SAME shared Arc source. Proves: generation advances, A2 deadline is later
   than A1's expired deadline, unchanged verifier rejects A1 and accepts A2. Two mutation
   oracles: (a) disconnect Arc forwarding (key_set returns None) → pre-expiry A1 verify
   fails; (b) disable deadline purge (snapshot-None branch) → post-expiry A1 rejection
   flips. Added cfg(test) hard_deadline() accessor on AssertionKeySet for deadline
   comparison assertions.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…anon, minimal network.rs

Pass 2 corrections (Thufir, exact head 15a643e):

1. URI canonicalization: add dot-segment convergence assertion
   (https://issuer.example/.well-known/./jwks.json -> same policy ID as
   canonical form). Mutation: raw-storage turns this assert_eq! red.

2. IPv6 seam: replace extraction-only Fix 2 test with a full three-stage
   resolved-target/pinning witness. New test
   resolved_target_and_pin_key_seam_public_ipv6_and_fec0_rejection carries
   the typed bare host from extract_url_host_and_port through IpAddr::parse,
   is_not_global_unicast, resolve_and_check_ssrf (network-free fast path for
   IP literals), and explicit pin-key equality. Covers fec0::/10 rejection
   in the same seam. host_str() mutation turns three independent assertions
   red: IpAddr::parse fails, SSRF check is bypassed, pin-key mismatches.

3. Controlled-time deadline crossing: remove force_expire_snapshot_for_test.
   Add now_fn: Arc<dyn Fn() -> DateTime<Utc>> field to ProductionJwksSource;
   production uses Arc::new(Utc::now), tests use an AtomicI64 clock.
   Add #[cfg(test)] new_with_clock constructor. Replace the test with one that
   keeps A1's hard deadline immutable, advances the clock to
   T0 + HARD_DEADLINE_SECS + 1, and calls get_snapshot once. Two mutation
   oracles: (1) disconnect Arc sharing -> post-advancement A1-reject and
   A2-accept flip red; (2) remove expiry purge branch -> same two flip red.

4. Reduce network.rs churn: start from main's is_private_ip, rename to
   is_not_global_unicast, add fec0::/10, keep is_private_ip as inline alias.
   Diff is +26/-7 vs origin/main (doc comment rewrite + rename + fec0 line +
   alias fn + fec0 boundary test; no behavioral change to existing ranges).

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Review clear: prior blockers resolved

Reviewed head 9a4c5985329eb97d5e450881e63a23a5122d673f against exact base 4a9de1a3a121285ef475d630b2b5764044c02cde. No remaining blocking defect established in this focused re-review. This is a COMMENTED review, not an approval or NIP-FI conformance certification.

The contract remains issuer-qualified, fail-closed assertion-runtime building blocks. Federated evidence supplements, never replaces, Nostr proof or relay-owned authorization. Relay ingress, binding persistence, final admission, leases, deployment refresh scheduling, and live NIP-11 serving belong to later stack layers; this review does not certify those integrations.

Closure and coverage

  • Refreshability fixed. The sealed Arc<S>: IssuerKeySource forwarding implementation lets an external startup/orchestration caller retain the same production source used by one long-lived verifier. The new same-verifier tests exercise replacement-key acceptance and removed-key rejection, including source-clock advancement beyond the original snapshot deadline. I traced cold/warm/refresh/error/expiry states, per-issuer isolation, cancellation-safe refresh ownership, commit visibility, and synchronous reads. Failed fetches do not extend the previous deadline.
  • IPv6 resolution fixed. The fetch path now uses typed Url::host() extraction, so public IPv6 literals reach the IpAddr fast path without brackets. Hostname results are checked before the pinned request; redirects and proxies are disabled, response accumulation is bounded, and the fetch future has a complete-operation timeout. In the pinned hyper-util 0.1.20 connector, numeric literals bypass DNS and connect directly to the validated address. I found no IPv6 destination change or functional failure from the override-map concern raised during independent review. The added fec0::/10 rejection preserves other classifier behavior through the compatibility alias.
  • Policy follow-up fixed. Canonical JWKS URI, refresh interval, and hard-deadline duration now feed assertion-policy identity, while key contents remain snapshot dependencies. Enforce-mode startup checks source/policy contract equality, missing/unmatched/duplicate source configs, and unsupported current-status posture. Offline discovery construction has no private-policy input and reports the residual revocation bound as null. Startup validation remains a documented caller precondition, not an enforced construction token.

Nonblocking accuracy corrections

The new tests and comments overstate some evidence. The pin-key assertion compares strings rather than observing reqwest; literal-IP safety does not depend on that map lookup. Removing the expiry purge does not suppress refetch as the rotation-test comment claims: the elapsed refresh interval still triggers it. The separate rotation-policy-ID test constructs identical policies rather than rotating keys. Correct these claims or strengthen the witnesses; they do not establish a surviving production defect.

Also narrow the changed ARCHITECTURE.md SSRF description: the implementation adds site-local rejection to the existing explicit predicate, not the complete IANA deny/exception table now described. I am not requesting broader classifier machinery as a condition of this re-review.

Validation: immutable source and pinned dependency inspection only, with independent HTTP, policy/startup, and cache review results; I also traced the cache lane directly. The cache result arrived after its cancellation and before publication. Cached base/head blobs matched Git tree metadata. No checkout, build, tests, or PR code execution performed. GitHub reports Unit Tests and Rust Lint successful for this head, but Desktop Smoke E2E (4) and its Desktop aggregate failed; those failures were not diagnosed here, so this is not an all-CI-green claim.

…st witnesses

Add missing IPv4 classes to is_not_global_unicast: documentation ranges
(RFC 5737 TEST-NET-1/2/3), multicast (224.0.0.0/4), and reserved class-E
(240.0.0.0/4). The predicate now rejects any address not unambiguously
assigned as globally reachable public unicast, matching the contract stated
by the JWKS boundary, webhook SSRF check, and link-preview SSRF check.

Also correct three test witness claims flagged during source review:

- network.rs: add boundary tests for the three new IPv4 classes
- jwks/tests.rs: validate_jwks_uri tests for documentation/multicast/reserved
- jwks/tests.rs: narrow pin-key stage 3 comment — asserts extracted host
  string form only, not reqwest connector behavior
- jwks/tests.rs: remove false expiry-purge mutation oracle 2; the key_set
  read path enforces the deadline independently so removing the write-path
  purge does not suppress rejection; keep mutation oracle 1 (sharing)
- verifier/tests.rs: correct key_rotation_does_not_change_assertion_policy_id
  doc — removes 'simulating a rotated JWKS' overstatement; the test proves
  identical contracts produce identical IDs (key material not in hash)
- ARCHITECTURE.md: align both SSRF sections to exactly match the
  implemented predicate; remove IANA-table language that was not implemented

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Restore the full is_not_global_unicast IANA deny/exception table from the
repository's own 272daca implementation. The prior correction added only
three IPv4 classes (documentation, multicast, class-E) while leaving several
required non-global ranges accepted: 192.0.0.0/24 (IETF Protocol Assignments,
with 192.0.0.9/.10 global exceptions), 192.88.99.0/24 (deprecated 6to4 relay
anycast), the full 2001::/23 envelope with its seven global exceptions
(PCP/TURN/DNS-SD anycast, AMT, AS112-v6, ORCHIDv2, DETs), 100::/64
(Discard-Only), 100:0:0:1::/64 (Dummy IPv6 Prefix), 3fff::/20
(documentation), and 5f00::/16 (SRv6 SIDs).

The restored classifier is the complete IANA deny/exception table already
tested and reviewed in this branch's history, with fec0::/10 retained.
All three callers (JWKS boundary, webhook, link-preview) inherit the
complete predicate through the compatibility alias; no API break.

Add JWKS-boundary tests for every newly restored class and exception,
exercising both validate_jwks_uri and the resolved-target path. The
new tests are mutation-sensitive: removing any deny branch makes the
corresponding rejection assertion red; removing any exception branch
makes the corresponding acceptance assertion red.

Correct three test witness accuracy issues from Carl's review (5073542065):

- extract_url_host_and_port doc: remove connector-bypass overclaim; state
  that the function produces the correct bare input form for reqwest's
  .resolve(), and that connector-level behavior is a runtime concern.
- rename key_rotation_does_not_change_assertion_policy_id to
  identical_contract_produces_stable_assertion_policy_id; update assertion
  text to match what the test actually proves (identical contracts produce
  identical IDs; key material is not part of the hash).
- resolved_target_and_pin_key_seam test doc: rename heading from
  'full SSRF/pin-key seam' to 'resolved-target and pin-input seam';
  add explicit note that the test does not exercise the reqwest connector.

Align ARCHITECTURE.md SSRF sections to the complete classifier table.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Review clear

Reviewed head 1a5f178c80f1ba94de695a3be7adfd177b851611 against exact base 4a9de1a3a121285ef475d630b2b5764044c02cde, focusing on the four-file delta from the previously clear head. No new blocking defect established. This is a COMMENTED review, not approval or NIP-FI conformance certification.

The contract remains issuer-qualified, fail-closed assertion-runtime building blocks. Federated evidence supplements Nostr proof and relay-owned authorization; later admission, persistence, deployment refresh scheduling, and live discovery wiring remain outside this review.

  • Shared classifier: the added IPv4 predicates reject exactly the three TEST-NET /24s, multicast 224.0.0.0/4, and reserved 240.0.0.0/4; broadcast remains rejected. Existing public-address handling and recursive mapped/compatible/NAT64/SIIT IPv4 classification remain intact. I traced their use in JWKS URI and resolved-address validation, workflow webhook resolution, and desktop link-preview resolution. Those callers inherit the stricter rejection without API or error-control-flow changes. Predicate and recursion.
  • Regression evidence: new core boundary tests and JWKS URI negatives call the production predicate/validator. Their packages are selected by just test-unit (Justfile:319). The revised rotation-test comments no longer claim that removing the expiry purge prevents refresh. The policy-ID test now describes identical-contract determinism rather than claiming runtime rotation coverage. These are source-level observations, not executed test results.
  • Prior fixes retained: the complete Git-tree delta contains only network.rs, two test files, and ARCHITECTURE.md. Shared-source forwarding, JWKS host extraction, cache/refresh behavior, policy identity, startup validation, and discovery implementation are unchanged from the prior review-clear head.

Nonblocking documentation limit: the new explicit range list is more accurate, but “any address class not unambiguously assigned as globally reachable public unicast is rejected” still overstates an explicit denylist (network.rs:29–30, ARCHITECTURE.md:749). For example, the unchanged test at network.rs:393–395 deliberately accepts fe00::1. Describe the enumerated rejection policy rather than claiming exhaustive global-address classification. This does not reopen unchanged classifier behavior as a blocker or require a new IANA-table subsystem.

Nonblocking evidence wording: with an independently cached source using the same advanced clock, A1 still fails the deadline filter; A2 acceptance is the reliable sharing oracle, so jwks/tests.rs:1365–1369,1540–1541 should not promise both assertions fail. Also, the expiry purge is not merely an optimization: it keeps the concurrent-refresh fallback (jwks/mod.rs:669–671) from returning an expired snapshot. Neither observation establishes a new runtime defect; the production purge is unchanged.

Validation: immutable source and tree-blob verification, plus an independent test/documentation review; no checkout, build, tests, mutation execution, or PR-code execution. No current-head CI status is asserted.

…ri stage

The http_fetcher_rejects_ipv6_loopback_uri_as_invalid test comment
previously described end-to-end bracket-free extraction and reqwest
pin-bypass behavior that does not happen in this test. fetch_jwks_inner
calls validate_jwks_uri as its first step; ::1 is rejected there as a
non-globally-unicast address before extract_url_host_and_port or
resolve_and_check_ssrf runs. Rewritten to describe only what the test
actually proves: public-fetcher rejection of an IPv6 loopback URI before
connection. Bracket-free extraction/value-flow evidence belongs to the
dedicated resolved_target_and_pin_key_seam test; connector behavior is a
separate runtime concern.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Review clear

Reviewed head 161f7bcbf0956fa09fee80536dd64c2743b297fd against exact base 4a9de1a3a121285ef475d630b2b5764044c02cde, focusing on the five-file delta from the previously clear head. No new blocking defect established. This is a COMMENTED review, not approval or NIP-FI conformance certification.

The contract remains issuer-qualified, fail-closed assertion-runtime building blocks. Federated evidence supplements Nostr proof and relay-owned authorization; later admission, persistence, deployment refresh scheduling, and live discovery wiring remain outside this review.

  • Shared SSRF classifier: the new IPv4 /24 restrictions and IPv6 /23 deny envelope preserve earlier rejections. The /128, /32, /48, and /28 exceptions are confined to their stated prefixes; the additional discard, dummy, documentation, and SRv6 ranges use the stated masks. Mapped/compatible/NAT64/SIIT IPv4 recursion remains intact. I traced the predicate through JWKS URI/resolved-address validation and the compatibility alias through webhook and link-preview resolution: the new restrictions reach all three callers before sending. Classifier.
  • Test and documentation delta: added core boundary/exception assertions and JWKS URI cases bind the production predicate/validator. The IPv6 fetcher comments now correctly distinguish early URI rejection, host extraction, and connector-level evidence. The verifier test is accurately renamed to identical-contract stability. Standard test selection includes core/auth, but no tests or CI were run for this source-only review.
  • Prior runtime conclusions retained: complete Git-tree comparison limits the true delta to ARCHITECTURE.md, network.rs, two test files, and comment-only changes in jwks/mod.rs. The shared-source implementation, cache/refresh state machine, policy identity, startup validation, and discovery implementation are unchanged.

Existing nonblocking limits remain: the architecture’s exhaustive-global-address wording still exceeds the enumerated denylist (the retained test explicitly accepts fe00::1, network.rs:277). The controlled original-deadline rotation comments still overstate the mutation oracle: an independent source using the same advanced clock also rejects expired A1, so A2 acceptance is the reliable sharing assertion (jwks/tests.rs:1426–1436,1534–1536). The expiry purge also protects the concurrent-refresh fallback, rather than being merely an optimization (jwks/mod.rs:648–671). These are carried-forward wording limits, not newly discovered runtime blockers or a demand for broader hardening.

Validation: immutable source and tree-blob verification, plus an independent test/documentation review. No checkout, build, tests, mutation execution, PR-code execution, or live DNS/TLS exercise. No current-head CI status or exhaustive IANA-registry certification is asserted.

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Review clear

Reviewed head e9e812c55424665d1783fa0482cfbcfe44651350 against exact base 59328d5ae38a51a618dd2fddd7faf1343d42096f, focusing on the three-file documentation/comment delta from the previously clear head. No new blocking defect established. This is a COMMENTED review, not approval or NIP-FI conformance certification.

The contract remains issuer-qualified, fail-closed assertion-runtime building blocks. Federated evidence supplements Nostr proof and relay-owned authorization. Later admission, persistence, deployment refresh scheduling, and live discovery wiring remain outside this review.

  • SSRF wording corrected: architecture and API documentation now describe an enumerated deny policy rather than claiming exhaustive global-address classification. The explicit fe00::1 example matches the existing predicate and test. No classifier behavior changed. API documentation.
  • Rotation commentary corrected: A2 acceptance is now identified as the reliable shared-source mutation oracle; an independent cache using the advanced clock also rejects expired A1. The concurrent-refresh explanation correctly identifies why get_snapshot must purge before the permit-loser fallback. Commentary; production paths.
  • Prior runtime conclusions retained: complete Git-tree comparison limits this delta to ARCHITECTURE.md and line comments in the two Rust files. Their non-comment lines are identical to the prior reviewed head. Shared-source forwarding, refresh/cancellation/expiry behavior, policy identity, startup/discovery, and the previously traced webhook/link-preview callers are unchanged.

Nonblocking wording remnants: jwks/tests.rs:1431 says “purge clears it,” but the independent verifier-only source rejects through key_set’s expiry filter without purging; the unchanged failure message at 1615 still claims independent-source A1 acceptance. Neither changes the test assertions or runtime behavior. The PR description repeats the purge wording; this is documentation accuracy, not a runtime blocker.

Validation: exact-base guidance and immutable tree/blob verification. The earlier independent test-comment review was retained after verifying that this amended head changes only four explanatory comment lines; its production dependencies and identified wording remnants are unchanged. Base movement changes unrelated desktop/agent files, not the applicable guidance, NIP-FI specification, or auth/core contracts. No checkout, build, tests, mutation execution, PR-code execution, live DNS/TLS exercise, current-head CI claim, or exhaustive IANA-registry certification.

Correct three accuracy findings in Carl's exact-head review:

SSRF (ARCHITECTURE.md + crates/buzz-core/src/network.rs): replace the
exhaustive-classification claim with accurate enumerated-deny wording in
both the buzz-core function table, the standalone SSRF section, and the
is_not_global_unicast rustdoc. The implementation blocks a specific set of
address classes and accepts everything else, including fe00::1.

shared_arc_source_verifier_rejects_expired_a1_accepts_a2: correct the
mutation oracle doc-comment and inline setup comment. An independent source
warmed with A1 and sharing the same advanced clock also expires, so A1
rejection stays green via cache expiry -- not via shared-arc rotation.
A2 acceptance is the reliable shared-source oracle.

Expiry-purge note: not a write-path optimization. The purge clears the
expired snapshot before permit acquisition so concurrent-refresh losers
cannot receive a stale snapshot via the fallback path.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Review clear

Reviewed head c92ee0e09e4f0a5e1f1244882be107f9cb04645b against exact base 59328d5ae38a51a618dd2fddd7faf1343d42096f, focusing on the one-file delta from the previously clear head. No new blocking defect established. This is a COMMENTED review, not approval or NIP-FI conformance certification.

The contract remains issuer-qualified, fail-closed assertion-runtime building blocks. Federated evidence supplements Nostr proof and relay-owned authorization. Later admission, binding persistence, deployment refresh scheduling, and live discovery wiring remain outside this review.

  • Remaining evidence wording corrected: the rotation commentary now identifies key_set()’s expiry filter, not a purge, as the reason an independently cached verifier rejects expired A1. The assertion messages no longer claim A1 would remain accepted; A2 acceptance is correctly identified as the reliable shared-source oracle. Changed comments and diagnostics, assertions.
  • No runtime or assertion-condition change: complete Git-tree comparison finds only jwks/tests.rs changed from the prior head, limited to comments and failure-message strings. Its IPv6 comment now also describes the enumerated deny policy accurately. The production expiry filter and concurrent-refresh purge remain unchanged (jwks/mod.rs:648–671,713–722).
  • Prior coverage retained: shared-source forwarding, cache/refresh/cancellation/expiry behavior, issuer policy identity, startup/discovery contracts, the SSRF classifier, and its JWKS/webhook/link-preview consumers are byte-identical to the previously reviewed head. No new independent lane was needed for this small wording-only correction; earlier independent review evidence remains applicable.

Validation: exact-base guidance, immutable Git-tree/blob verification, and direct source comparison only. No checkout, build, tests, mutation execution, PR-code execution, live DNS/TLS exercise, or current-head CI claim.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants