Skip to content

fix: revocation stub comments, fail-open hardcoding, dynamic batch_size/revocation_url#35

Merged
rmhrisk merged 1 commit intomainfrom
fix/revocation-stubs-batch-size
Mar 15, 2026
Merged

fix: revocation stub comments, fail-open hardcoding, dynamic batch_size/revocation_url#35
rmhrisk merged 1 commit intomainfrom
fix/revocation-stubs-batch-size

Conversation

@rmhrisk
Copy link
Copy Markdown
Contributor

@rmhrisk rmhrisk commented Mar 15, 2026

Three classes of issues found during review of old revocation artifacts, fail-open behavior, and hardcoded values. All fixed uniformly across all four language implementations.

Stale stub comments — Five verifier stubs described the old design ('revocation by index range', 'GET /revoked format not defined'). Both wrong since PR #26. Updated to reference SPEC.md §Revocation and issue #14.

Fail-open hardcoded — All five stubs passed ok=true to the revocation trace step, recording revocation as passed unconditionally. SPEC.md explicitly prohibits this. All five now pass ok=false with 'stub only, revocation not checked' — correct fail-closed-by-default posture.

batch_size hardcoded (Go, TypeScript, Rust) — Trust config carries batch_size so deployments with non-default sizes don't need code changes. Go/TS/Rust all had const BATCH_SIZE = 16 ignoring it. Java already read it. All four runtime trust structs now read from the trust config JSON.

revocation_url added to trust structs (Go, TypeScript, Rust) — Field was in the JSON since PR #26 but not in the runtime structs. Fixed.

ARCHITECTURE.md — 'revocation range keys' → 'revocation cache keys'.

…ze/revocation_url

Three classes of issues fixed uniformly across all four language implementations.

Stale stub comments
  All five verifier stubs (Go, TS SDK, TS HTTP, Rust, Java) described the old
  revocation design: 'spec defines revocation by index range' and 'GET /revoked
  format not yet defined'. Both were accurate before PR #26; both are wrong now.
  Updated to reference SPEC.md §Revocation (Bloom filter cascade, fully specified),
  point to revocationUrl in the trust config, and direct readers to issue #14.

Fail-open hardcoded as default
  All five stubs passed ok=true to the revocation trace step, recording the
  revocation check as passed unconditionally — the fail-open pattern, hardcoded,
  with no warning. SPEC.md §Revocation explicitly prohibits this: fail-open must
  require explicit configuration, must surface in the trace as a warning, and must
  not be the default. All five stubs now pass ok=false with the message
  'not implemented — stub only, revocation not checked'. This is the correct
  fail-closed-by-default posture for a documented stub.

batch_size hardcoded (Go, TypeScript, Rust)
  The trust config JSON carries a batch_size field so deployments with non-default
  batch sizes do not require code changes. Go, TypeScript SDK, and Rust all had
  const BATCH_SIZE = 16 ignoring the trust config value. Java already read it.
  Fixed:
  - Go: TrustAnchor gains BatchSize int field; loadTrustConfigFromBytes reads and
    stores it (default 16 if absent or zero); Verify() uses anchor.BatchSize
  - TypeScript SDK: TrustConfig interface gains batchSize: number; fromJSON reads
    it from raw.batch_size (default 16); verifier uses this.trust.batchSize
  - Rust: TrustConfig gains batch_size: usize; TrustConfigJSON gains
    #[serde(default)] batch_size: Option<usize>; from_json sets default 16;
    verifier uses self.trust.batch_size

revocation_url added to trust structs (Go, TypeScript, Rust)
  The trust config JSON has had revocation_url since PR #26. All four runtime
  trust structs (Go TrustAnchor, TS TrustConfig, Rust TrustConfig, Java
  TrustConfig) now carry the field and populate it from the JSON. Go verifier
  stub TODO already referenced anchor.RevocationURL before this change; now
  the field actually exists.

ARCHITECTURE.md
  'revocation range keys' → 'revocation cache keys' (stale phrase from the
  index-range design)
@rmhrisk rmhrisk merged commit 30da373 into main Mar 15, 2026
7 checks passed
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.

1 participant