release: 1.7.0

CI/lint hardening since v1.6.14: clippy on stable with the MSRV single-sourced
from Cargo.toml, cargo-deny dependency-audit gate with first-party git sources
allowed (#15), and audience-based comment-guard (#17). No library code changes.

.github/workflows/ci.yml

@@ -32,12 +32,26 @@ on:
 # libfreemkv path-deps ../freemkv-unlock, which resolves to the workspace root
 # under this layout — hence the third checkout.
 #
-# `--locked` is deliberately absent: Cargo.lock is gitignored here (library
-# convention), so there is nothing to lock against and it fails rather than
-# resolves. That is what was breaking this workflow. The lock that has to hold
-# is the one release.sh regenerates in each consuming binary.
+# `--locked` is deliberately absent: Cargo.lock is now committed (issue #15) and
+# the `deny` job audits the resolved graph, but CI's sibling path-patch layout
+# resolves the freemkv crates as path packages, which `--locked` can't satisfy —
+# the same reason the binary crates omit it. The lock that has to hold is the one
+# release.sh regenerates in each consuming binary.
 
 jobs:
+  # Single source of truth for the MSRV — read rust-version from Cargo.toml once
+  # and export it; MSRV-bound jobs consume needs.resolve-msrv.outputs.v. clippy
+  # runs on stable instead (see CI.md's Toolchain policy).
+  resolve-msrv:
+    runs-on: ubuntu-latest
+    outputs:
+      v: ${{ steps.msrv.outputs.v }}
+    steps:
+      - uses: actions/checkout@v7
+      - id: msrv
+        shell: bash
+        run: echo "v=$(grep -m1 '^rust-version' Cargo.toml | sed -E 's/.*\"([^\"]+)\".*/\1/')" >> "$GITHUB_OUTPUT"
+
   # CI owns the changelog: main may never carry a released version without a
   # CHANGELOG.md entry for it. The release orchestrator dates an existing
   # `## [X.Y.Z]` heading — it does NOT invent one — so a version bump that
@@ -65,6 +79,7 @@ jobs:
           echo "::warning::${msg} (allowed on ${GITHUB_REF_NAME:-unknown}; required before it can reach main)."
 
   lint:
+    needs: resolve-msrv
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v7
@@ -89,19 +104,27 @@ jobs:
           [patch.crates-io]
           libfreemkv = { path = "../libfreemkv" }
           EOF
-      - uses: dtolnay/rust-toolchain@1.88.0
+      # fmt runs on the MSRV pin; clippy runs on stable so new-lint debt is a
+      # blocking gate the pinned toolchain would hide. Stable is installed first
+      # for clippy; the MSRV pin goes second so it stays fmt's default toolchain.
+      - uses: dtolnay/rust-toolchain@stable
         with:
-          components: clippy, rustfmt
+          components: clippy
+      - uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: ${{ needs.resolve-msrv.outputs.v }}
+          components: rustfmt
       - uses: Swatinem/rust-cache@v2
         with:
           workspaces: freemkv-keysources
       - run: cargo fmt --check
         working-directory: freemkv-keysources
       # --all-targets so the test code is linted too, not just the library.
-      - run: cargo clippy --all-targets -- -D warnings
+      - run: cargo +stable clippy --all-targets -- -D warnings
         working-directory: freemkv-keysources
 
   test:
+    needs: resolve-msrv
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v7
@@ -126,13 +149,50 @@ jobs:
           [patch.crates-io]
           libfreemkv = { path = "../libfreemkv" }
           EOF
-      - uses: dtolnay/rust-toolchain@1.88.0
+      - uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: ${{ needs.resolve-msrv.outputs.v }}
       - uses: Swatinem/rust-cache@v2
         with:
           workspaces: freemkv-keysources
       - run: cargo test --tests
         working-directory: freemkv-keysources
 
+  # Dependency-audit gate. Mirrors the `test` job's sibling-checkout layout and
+  # generated .cargo/config.toml so `cargo metadata` resolves the git-tag patch
+  # to the local sibling paths; cargo-deny then audits advisories, bans,
+  # licenses, and sources against the committed deny.toml.
+  deny:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v7
+        with:
+          path: freemkv-keysources
+      - uses: actions/checkout@v7
+        with:
+          repository: freemkv/libfreemkv
+          ref: "${{ github.ref_name == 'qa' && 'qa' || 'dev' }}"
+          path: libfreemkv
+      - uses: actions/checkout@v7
+        with:
+          repository: freemkv/freemkv-unlock
+          ref: "${{ github.ref_name == 'qa' && 'qa' || 'dev' }}"
+          path: freemkv-unlock
+      - name: Resolve siblings from their dev tips
+        shell: bash
+        working-directory: freemkv-keysources
+        run: |
+          mkdir -p .cargo
+          cat > .cargo/config.toml <<'EOF'
+          [patch.crates-io]
+          libfreemkv = { path = "../libfreemkv" }
+          EOF
+      - uses: dtolnay/rust-toolchain@stable
+      - uses: EmbarkStudios/cargo-deny-action@v2
+        with:
+          command: check advisories bans licenses sources
+          manifest-path: freemkv-keysources/Cargo.toml
+
   # This crate ships wherever freemkv and autorip ship, and it drags in all of
   # libfreemkv's platform code (SCSI transports included). Build the tests, not
   # `cargo check` — check skips codegen and never compiles test-only code.
@@ -149,6 +209,7 @@ jobs:
     # than a slow one: release.sh's CI gate refuses while any run for the
     # commit is still in progress, so a never-scheduled job blocks releases
     # silently -- see the note on real-media in qa.yml.
+    needs: resolve-msrv
     if: github.ref_name == 'qa' || github.ref_name == 'main'
     runs-on: macos-latest
     steps:
@@ -174,7 +235,9 @@ jobs:
           [patch.crates-io]
           libfreemkv = { path = "../libfreemkv" }
           EOF
-      - uses: dtolnay/rust-toolchain@1.88.0
+      - uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: ${{ needs.resolve-msrv.outputs.v }}
       - uses: Swatinem/rust-cache@v2
         with:
           workspaces: freemkv-keysources
@@ -185,9 +248,10 @@ jobs:
   # layout and pinned toolchain (nothing in the release cascade depends on it,
   # and codecov upload is tokenless with fail_ci_if_error:false, so a flaky
   # upload never reddens the run). `--locked` is deliberately absent here for
-  # the same reason it is in `test`: Cargo.lock is gitignored, so there is no
-  # lockfile to lock against on a fresh runner.
+  # the same reason it is in `test`: CI's sibling path-patch layout resolves the
+  # freemkv crates as path packages, which `--locked` can't satisfy.
   coverage:
+    needs: resolve-msrv
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v7
@@ -212,8 +276,9 @@ jobs:
           [patch.crates-io]
           libfreemkv = { path = "../libfreemkv" }
           EOF
-      - uses: dtolnay/rust-toolchain@1.88.0
+      - uses: dtolnay/rust-toolchain@master
         with:
+          toolchain: ${{ needs.resolve-msrv.outputs.v }}
           components: llvm-tools-preview
       - uses: Swatinem/rust-cache@v2
         with:
@@ -241,6 +306,7 @@ jobs:
     # than a slow one: release.sh's CI gate refuses while any run for the
     # commit is still in progress, so a never-scheduled job blocks releases
     # silently -- see the note on real-media in qa.yml.
+    needs: resolve-msrv
     if: github.ref_name == 'qa' || github.ref_name == 'main'
     runs-on: windows-latest
     steps:
@@ -266,7 +332,9 @@ jobs:
           [patch.crates-io]
           libfreemkv = { path = "../libfreemkv" }
           EOF
-      - uses: dtolnay/rust-toolchain@1.88.0
+      - uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: ${{ needs.resolve-msrv.outputs.v }}
       - uses: Swatinem/rust-cache@v2
         with:
           workspaces: freemkv-keysources

.github/workflows/comment-guard.yml

@@ -0,0 +1,17 @@
+name: comment-guard
+
+# Caps explanatory prose by COMMENT INTENT (not visibility):
+#   //! / /*! module docs  — unbounded (module-scoped, can't hide per-item text)
+#   ///  / /** */ item docs — 8 prose lines (fenced ``` examples excluded)
+#   //   / /* */  asides    — 3 lines (single-blank-separated runs count as one)
+# Overflow belongs in the module //! or docs/<topic>.md with a one-line // pointer.
+
+on: [push, pull_request]
+
+jobs:
+  comment-guard:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v7
+      - name: Run comment-guard
+        run: python3 ci/comment-guard.py

.github/workflows/qa.yml

@@ -24,6 +24,19 @@ on:
   workflow_dispatch:
 
 jobs:
+  # Single source of truth for the MSRV — see ci.yml. Jobs that build/test AT
+  # the MSRV read needs.resolve-msrv.outputs.v from Cargo.toml's rust-version;
+  # clippy (cross-lint) runs on stable instead.
+  resolve-msrv:
+    runs-on: ubuntu-latest
+    outputs:
+      v: ${{ steps.msrv.outputs.v }}
+    steps:
+      - uses: actions/checkout@v7
+      - id: msrv
+        shell: bash
+        run: echo "v=$(grep -m1 '^rust-version' Cargo.toml | sed -E 's/.*\"([^\"]+)\".*/\1/')" >> "$GITHUB_OUTPUT"
+
   # ── Name the candidate ────────────────────────────────────────────────
   #
   # Every push to `qa` is a release candidate, so every push gets a tag:
@@ -65,6 +78,7 @@ jobs:
   # changes what the optimiser can prove. A test that only passes in debug is
   # a test that never guarded the binary anyone actually ships.
   release-tests:
+    needs: resolve-msrv
     strategy:
       fail-fast: false
       matrix:
@@ -93,7 +107,9 @@ jobs:
           [patch.crates-io]
           libfreemkv = { path = "../libfreemkv" }
           EOF
-      - uses: dtolnay/rust-toolchain@1.88.0
+      - uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: ${{ needs.resolve-msrv.outputs.v }}
       - uses: Swatinem/rust-cache@v2
         with:
           workspaces: freemkv-keysources
@@ -128,7 +144,7 @@ jobs:
           [patch.crates-io]
           libfreemkv = { path = "../libfreemkv" }
           EOF
-      - uses: dtolnay/rust-toolchain@1.88.0
+      - uses: dtolnay/rust-toolchain@stable
         with:
           components: clippy
       - uses: Swatinem/rust-cache@v2
@@ -143,6 +159,7 @@ jobs:
   # of the SCSI transport and platform layers compile on no other runner, so
   # without this they are first built at release time.
   windows-build:
+    needs: resolve-msrv
     runs-on: windows-latest
     steps:
       - uses: actions/checkout@v7
@@ -167,7 +184,9 @@ jobs:
           [patch.crates-io]
           libfreemkv = { path = "../libfreemkv" }
           EOF
-      - uses: dtolnay/rust-toolchain@1.88.0
+      - uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: ${{ needs.resolve-msrv.outputs.v }}
       - uses: Swatinem/rust-cache@v2
         with:
           workspaces: freemkv-keysources

.github/workflows/release.yml

@@ -29,10 +29,11 @@ jobs:
 
   # Tests run as a PARALLEL TRIPWIRE: they fail the run if they fail, but the
   # release job does NOT `needs:` this job. The tag decision was already gated by
-  # the local precommit (same Rust 1.86, same commit).
+  # the local precommit (same MSRV, same commit).
   #
-  # freemkv-keysources is a library — Cargo.lock isn't tracked, so --locked
-  # would always fail (no lockfile to lock against on a fresh runner). Its
+  # Cargo.lock is now committed (issue #15), but --locked is still omitted: on
+  # the runner the libfreemkv dep resolves from crates.io, not the local path
+  # patch, which --locked can't reconcile. Its
   # libfreemkv dep resolves from crates.io (the dev .cargo/config.toml patch is
   # gitignored and absent on the runner), so libfreemkv must already be published
   # at this version — the release script enforces that ordering before tagging.
@@ -41,7 +42,12 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v7
-      - uses: dtolnay/rust-toolchain@1.97.0
+      - id: msrv
+        shell: bash
+        run: echo "v=$(grep -m1 '^rust-version' Cargo.toml | sed -E 's/.*\"([^\"]+)\".*/\1/')" >> "$GITHUB_OUTPUT"
+      - uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: ${{ steps.msrv.outputs.v }}
       - uses: Swatinem/rust-cache@v2
       - run: cargo test
 

.gitignore

@@ -1,5 +1,4 @@
 /target
-Cargo.lock
 
 # session scratch — never track (may contain RE breadcrumbs)
 scratch/

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## [1.7.0] — 2026-09-02
+
+### Changed
+
+- Version aligned to 1.7.0 for the unified release. Internal CI/lint hardening only (stable-clippy MSRV split, cargo-deny dependency-audit gate, audience-based comment-guard); no functional changes.
+
 ## [1.6.14] — 2026-08-31
 
 ### Changed

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "freemkv-keysources"
-version = "1.6.14"
+version = "1.7.0"
 edition = "2024"
 rust-version = "1.88"
 license = "MIT"

ci/comment-guard.py

@@ -0,0 +1,252 @@
+#!/usr/bin/env python3
+"""comment-guard: cap explanatory prose by COMMENT INTENT, not visibility.
+
+Three tiers, keyed on the syntax the author reached for:
+
+  * `//!` / `/*!` — a module (or crate) design doc. UNBOUNDED. There is one per
+    file, at the top, documenting the module as a whole, so it can't be used to
+    smuggle per-item rationale the way item docs can. Long correctness / design
+    essays (e.g. a memory-ordering proof) legitimately live here.
+  * `///` / `/** */` — item documentation. Up to DOC_CAP *prose* lines. Fenced
+    ``` code blocks do NOT count, so real examples aren't punished. This holds
+    whether or not the item is `pub`: a `pub(crate)` fn deserves a real doc
+    paragraph just like a `pub` one. Capping `///` here (rather than exempting
+    it, as the old guard did — issue #17) is what stops rationale being hidden
+    behind a doc sigil, while still leaving room for a genuine paragraph.
+  * `//` / `/* */` — a quick internal aside. Up to INLINE_CAP lines. This is the
+    shape that sprawls into walls, so it's kept tight: stanzas separated by a
+    single blank line count CUMULATIVELY, so one essay can't be split into runs.
+
+Overflow belongs in the module `//!`, or in docs/<topic>.md with a one-line `//`
+pointer — both show up in review.
+
+Scans src/ and tests/ under the given root (default cwd). `--selftest` runs
+built-in cases. Exit non-zero if any block exceeds its cap.
+"""
+import re
+import sys
+from pathlib import Path
+
+INLINE_CAP = 3
+DOC_CAP = 8
+
+_RAW_OPEN = re.compile(r'r(#*)"')  # raw string opener: r"…", r#"…"#, br##"…"## …
+
+
+def _strip_line_comment(s):
+    """Return (kind, body) for a whole-line comment, else (None, None).
+
+    kind is 'doc' for `///`/`//!`, 'plain' for other `//`. A trailing comment on
+    a code line is not a whole-line comment and is ignored (single-line, cheap).
+    """
+    if s.startswith("//!"):
+        return "doc", s[3:]
+    if s.startswith("///") and not s.startswith("////"):
+        return "doc", s[3:]
+    if s.startswith("//"):
+        return "plain", s[2:]
+    return None, None
+
+
+def _fence_toggle(body):
+    """True if this doc body line opens/closes a ``` fenced code block."""
+    return body.strip().startswith("```")
+
+
+def _classify(lines):
+    """Yield per-line dicts: comment kind ('doc'/'plain'/None), doc-ness of
+    block comments, and code info. Block comments are tracked across lines.
+    """
+    info = []
+    in_block = False
+    block_is_doc = False
+    raw_hashes = None  # None = not in a raw string; int N = open, needs '"' + N*'#'
+    for raw in lines:
+        s = raw.lstrip()
+        rec = {"raw": raw, "s": s, "comment": None, "is_doc": False, "code": False}
+        # Inside a raw string literal (e.g. an embedded HTML/CSS/JS asset): the
+        # bytes are string data, not Rust comments, so the guard is blind to them
+        # until the string closes. Prevents `/* */` or `//` lines in an embedded
+        # page from being counted as comment blocks.
+        if raw_hashes is not None:
+            if ('"' + "#" * raw_hashes) in raw:
+                raw_hashes = None
+            info.append(rec)
+            continue
+        if in_block:
+            rec["comment"] = "doc" if block_is_doc else "plain"
+            rec["is_doc"] = block_is_doc
+            if "*/" in raw:
+                in_block = False
+            info.append(rec)
+            continue
+        # A block comment only starts a comment-line if it's the first token.
+        if s.startswith("/*"):
+            rec["is_doc"] = s.startswith("/**") or s.startswith("/*!")
+            rec["comment"] = "doc" if rec["is_doc"] else "plain"
+            if "*/" not in s[2:]:
+                in_block = True
+                block_is_doc = rec["is_doc"]
+            info.append(rec)
+            continue
+        kind, _ = _strip_line_comment(s)
+        if kind is not None:
+            rec["comment"] = kind
+            rec["is_doc"] = kind == "doc"
+            info.append(rec)
+            continue
+        # Code (or blank). Blank = empty after strip.
+        rec["code"] = s != ""
+        # A code line may OPEN a raw string that spans following lines. Detected
+        # only on code (comments were handled above), so `// see r"x"` is safe.
+        m = _RAW_OPEN.search(raw)
+        if m and ('"' + "#" * len(m.group(1))) not in raw[m.end():]:
+            raw_hashes = len(m.group(1))
+        info.append(rec)
+    return info
+
+
+def violations(path):
+    lines = path.read_text(encoding="utf-8", errors="replace").splitlines()
+    info = _classify(lines)
+    n = len(info)
+    out = []
+
+    i = 0
+    while i < n:
+        rec = info[i]
+        if rec["comment"] is None:
+            i += 1
+            continue
+
+        start = i
+        if rec["is_doc"]:
+            # Doc run: contiguous doc lines. `//!` / `/*!` module docs are
+            # UNBOUNDED; any other doc run caps PROSE (fenced ``` excluded) at
+            # DOC_CAP, regardless of the documented item's visibility.
+            j = i
+            prose = 0
+            in_fence = False
+            module_doc = False
+            while j < n and info[j]["comment"] == "doc":
+                s = info[j]["s"]
+                if s.startswith("//!") or s.startswith("/*!"):
+                    module_doc = True
+                body = (
+                    s[3:] if (s.startswith("///") or s.startswith("//!")) else s
+                )
+                if _fence_toggle(body):
+                    in_fence = not in_fence
+                elif not in_fence and body.strip() != "":
+                    prose += 1
+                j += 1
+            if not module_doc and prose > DOC_CAP:
+                out.append((start + 1, prose, DOC_CAP, "doc prose"))
+            i = j
+            continue
+
+        # Internal (plain) run, with single-blank bridging (cumulative).
+        j = i
+        count = 0
+        while j < n:
+            r = info[j]
+            if r["comment"] == "plain":
+                count += 1
+                j += 1
+                continue
+            if r["comment"] == "doc":
+                break
+            if r["comment"] is None and not r["code"]:
+                # blank: bridge only a SINGLE blank, else stop
+                if j + 1 < n and info[j + 1]["comment"] == "plain":
+                    j += 1
+                    continue
+                break
+            break  # code
+        if count > INLINE_CAP:
+            out.append((start + 1, count, INLINE_CAP, "inline comment"))
+        i = j
+    return out
+
+
+def main(argv):
+    if "--selftest" in argv:
+        return _selftest()
+    root = Path(argv[1]) if len(argv) > 1 else Path.cwd()
+    files = sorted(p for d in ("src", "tests") for p in (root / d).rglob("*.rs"))
+    total = 0
+    for f in files:
+        for start, length, cap, kind in violations(f):
+            rel = f.relative_to(root)
+            print(f"{rel}:{start}: {kind} block is {length} lines (max {cap})")
+            total += 1
+    if total:
+        print(f"\ncomment-guard: {total} comment block(s) exceed their cap.")
+        print(
+            f"Inline // asides cap at {INLINE_CAP}; /// item-doc prose caps at "
+            f"{DOC_CAP} (fenced examples excluded); //! module docs are unbounded. "
+            "Trim, move the rationale into the module //!, or docs/<topic>.md "
+            "with a // pointer."
+        )
+        return 1
+    return 0
+
+
+def _selftest():
+    import tempfile
+
+    cases = [
+        # (rust source, expected number of violations)
+        # short /// doc -> ok
+        ("/// short doc\npub fn a() {}\n", 0),
+        # /// on a PRIVATE item now gets the doc cap (8), so 4 lines is fine
+        ("/// l1\n/// l2\n/// l3\n/// l4\nfn priv_a() {}\n", 0),
+        # ...but /// is NOT unlimited: 9 prose lines on a private item fails
+        ("".join(f"/// l{k}\n" for k in range(9)) + "fn priv_b() {}\n", 1),
+        # 8 prose /// on a pub item -> ok (boundary)
+        ("".join(f"/// l{k}\n" for k in range(8)) + "pub fn c() {}\n", 0),
+        # 9 prose /// on a pub item -> violation
+        ("".join(f"/// l{k}\n" for k in range(9)) + "pub fn d() {}\n", 1),
+        # /// with a long fenced example -> example excluded, ok
+        (
+            "/// summary\n/// # Examples\n/// ```\n"
+            + "".join("/// code\n" for _ in range(12))
+            + "/// ```\npub fn e() {}\n",
+            0,
+        ),
+        # pub(crate) gets the SAME doc cap as pub (no pub/private split anymore)
+        ("".join(f"/// l{k}\n" for k in range(6)) + "pub(crate) fn f() {}\n", 0),
+        # bare inline 4 lines -> violation
+        ("// a\n// b\n// c\n// d\nlet x = 1;\n", 1),
+        # blank-split dodge: 3 + blank + 3 inline -> cumulative -> violation
+        ("// a\n// b\n// c\n\n// d\n// e\n// f\nfn z() {}\n", 1),
+        # plain /* */ block, 5 lines -> inline cap -> violation
+        ("/* one\n two\n three\n four\n five */\nfn q() {}\n", 1),
+        # comment-like lines INSIDE a raw string are string data, not comments
+        (
+            'const H: &str = r##"<style>\n'
+            + "".join("/* css note */\n" for _ in range(8))
+            + '</style>"##;\nfn h() {}\n',
+            0,
+        ),
+        # //! module doc may run long -> EXEMPT (no cap)
+        ("".join(f"//! l{k}\n" for k in range(40)) + "\npub fn m() {}\n", 0),
+        # /*! block module doc may run long -> EXEMPT
+        ("/*! l0\n" + "".join(f" l{k}\n" for k in range(30)) + "*/\nfn n() {}\n", 0),
+    ]
+    ok = True
+    with tempfile.TemporaryDirectory() as d:
+        for idx, (src, want) in enumerate(cases):
+            p = Path(d) / f"case{idx}.rs"
+            p.write_text(src)
+            got = len(violations(p))
+            flag = "ok" if got == want else "FAIL"
+            if got != want:
+                ok = False
+            print(f"  selftest {idx}: want {want} got {got} [{flag}]")
+    print("selftest PASSED" if ok else "selftest FAILED")
+    return 0 if ok else 1
+
+
+if __name__ == "__main__":
+    sys.exit(main(sys.argv))

deny.toml

@@ -0,0 +1,42 @@
+# cargo-deny — gates the RESOLVED dependency graph (issue #15).
+# Runs in CI against the committed Cargo.lock: RUSTSEC advisories, a permissive
+# licence allow-list, and source/ban hygiene. Update the lock (cargo update)
+# to clear an advisory rather than adding an `ignore` entry without cause.
+
+[advisories]
+yanked = "deny"
+# ignore = ["RUSTSEC-0000-0000"]  # only with a written justification
+
+[licenses]
+# The whole tree is permissive. r-efi's LGPL arm is satisfied by Apache-2.0
+# (its expression is "Apache-2.0 OR LGPL-2.1-or-later"), so no copyleft is
+# allowed here. Add a licence only when a real dependency introduces it.
+allow = [
+    "MIT",
+    "Apache-2.0",
+    "Apache-2.0 WITH LLVM-exception",
+    "0BSD",
+    "Unicode-3.0",
+    "Unlicense",
+    "Zlib",
+    "BSD-2-Clause",
+    "BSD-3-Clause",
+    "ISC",
+    "CDLA-Permissive-2.0",
+]
+confidence-threshold = 0.9
+
+[bans]
+multiple-versions = "warn"   # transitive dupes (digest 0.10/0.11 etc.) are noise, not a gate
+wildcards = "deny"
+# The freemkv siblings resolve as path deps under CI's path-patch layout, which
+# reads as a wildcard version req. That is internal, not a supply-chain wildcard.
+allow-wildcard-paths = true
+
+[sources]
+unknown-registry = "deny"
+unknown-git = "deny"
+# The freemkv siblings are first-party git-tag deps; allow git sources from
+# our own org only (CI resolves them as git, not the local path override).
+[sources.allow-org]
+github = ["freemkv"]

docs/keydb-format.md

@@ -0,0 +1,157 @@
+# keydb_format.rs — design notes
+
+## Full parser API retained on purpose
+
+The parser is copied verbatim, so it carries the full KeyDb/DiscEntry API
+even though this crate's consumer (`keydb.rs`) only exercises a subset
+(`load`, `find_disc`, `iter_disc_entries`, and the public fields read by
+`KeydbSource::unit_keys_from` / `KeydbSource::host_certs`). NOTE: an earlier
+revision of this comment cited a `candidates_from` function — no such
+function has ever existed in this crate; the real consumer is
+`KeydbSource::unit_keys_from` (keydb.rs), which reads `unit_keys`, `vuk`,
+`media_key` AND `vid` (`vid` is load-bearing for the MK+VID derivation the
+kat_c / kat_d KATs pin — it is NOT unused). The genuinely unused items —
+`empty`, `find_vuk`, `DiscEntry::title` — are part of the faithful copy and
+are retained rather than pruned; allow dead_code so the byte-for-byte copy
+compiles clean without diverging from the libfreemkv original.
+
+## ParseStats — what `KeyDb::parse` threw away
+
+The parser is fed a THIRD-PARTY file: a mirror can serve a truncated or
+byte-shifted download that is still valid UTF-8, still contains enough
+recognisable rows to pass `KeydbSource::save`'s "at least one entry" check,
+and is then atomically committed as the new keydb — after which every disc
+past the corruption point silently resolves nothing. Before this counter,
+each malformed `| DK` / `| PK` / `| HC` / `| HC2` / `0x…` row was dropped by
+an `if let Some(..) = .. { }` with no `else`, no counter and no log, so that
+file was indistinguishable from a healthy one at every layer above.
+
+The counts are logged ONCE per parse (a per-line log would be 181k lines on
+a real keydb, which is why the rate limit is "one summary", never "hide the
+count").
+
+## `KeyDb::load_counted` — same-descriptor stamp + parse
+
+Two callers need this seam, both in `crate::KeydbSource`:
+
+* The identity stamp and the bytes must come from the SAME descriptor.
+  Stat-the-path-then-load-the-path is two independent resolutions of one
+  name, so an atomic rename landing between them stored a stamp
+  describing one file next to the parsed contents of another — and the
+  cache then served that pairing until the stamp changed again. `fstat`
+  on the open handle cannot be pointed at a different inode.
+* The rejection counts have to be RETAINED, not just logged, so a cache
+  hit can re-emit them.
+
+## `to_keydb_cfg` — trailing-comment placement
+
+The trailing `; <comment>` (MKB version / volume size / UHD) is emitted
+ONLY after a `U` (unit-keys) field — that is the one place the parser
+splits the value on `;`. Gluing a comment onto an `M`/`I`/`V` value would
+make `parse_hex16` reject the whole field, so a comment-bearing entry that
+has no unit keys drops its comment (keys always survive; the metadata is a
+derivable hint). Real per-disc rows that carry metadata also carry keys.
+
+## `parse_disc_entry` title handling
+
+Title = everything between `= ` and the first ` | ` field (or the trailing
+`;` comment), kept VERBATIM (trimmed). This is a FAITHFUL copy of the keydb
+title, so it must round-trip exactly: a previous version extracted a `(...)`
+substring as a "display title", but that TRUNCATED real titles that
+legitimately contain parentheses ("Lawrence of Arabia (Restored Version) –
+Disc 2 …" → "Restored Version") and broke serialize→parse idempotence.
+Display prettification, if wanted, belongs in the title-display layer, NOT
+this codec.
+
+## Real-data test notes
+
+`to_keydb_cfg_is_idempotent_on_real_keydb`: parse the full keydb -> serialize
+(S1) -> parse S1 -> serialize again (S2). S1 MUST equal S2 byte-for-byte.
+This is the right invariant: a raw keydb.cfg has formatting variance
+(whitespace, optional fields, comment style) that our CANONICAL serializer
+normalizes, so `text == to_keydb_cfg` is NOT expected — but once normalized,
+a re-load+re-serialize must be stable. Idempotence here proves `parse` is
+lossless on its own output and `to_keydb_cfg` is deterministic. Also asserts
+no rows are dropped. It used to carry no `#[ignore]` and `return` early when
+`KEYDB_PATH` was unset — so CI ran it, asserted NOTHING about 181k-entry
+behaviour, and reported green. A skip has to be visible in the test output
+to mean anything, and an ignored test that is RUN without its fixture must
+fail loudly rather than silently pass.
+
+`real_keydb_path`: the old pattern (`match keydb_path() { Some(p) => p, None
+=> return }`) on a non-ignored test is the worst of both worlds: CI ran the
+test, asserted nothing, and reported success. These tests are `#[ignore]`d,
+so running one at all is an explicit request — and an explicit request with
+no fixture must fail, not quietly pass.
+
+## `load_counted_reads_the_handle_not_the_path` test
+
+`load_counted` must read the HANDLE it was given, never re-resolve the
+path. `KeydbSource::cached_db` stamps the open file with `fstat` and then
+parses from that same descriptor precisely so the recorded identity and
+the parsed bytes cannot describe two different files; the moment this
+function re-opens the path, that guarantee is gone and an atomic rename
+landing in between stores one file's stamp beside another file's keys —
+a mismatch the cache then serves as if it were valid.
+
+The race itself cannot be scheduled deterministically in-process, so the
+SEAM is what is pinned: with the path already replaced, the handle's
+contents must still win. Catches the mutation that restores an internal
+`File::open(path)`.
+
+## `host_cert_debug_is_redacted_including_the_sibling_wrapper` test
+
+The SIBLING type. `KeyDb`'s hand-written `Debug` prints `host_certs_len`, so
+it protects the whole-db rendering and nothing else — `KeyDb::host_certs` is
+a public field, and `{:?}` on ONE element of it walks a completely different
+code path: `KeydbHostCert`'s derive, then `HostCert`'s impl. That path
+carries the AACS host PRIVATE key, the only key material in the crate that
+authenticates US rather than opening a disc.
+
+It is safe today only because `libfreemkv::HostCert` hand-writes a redacting
+`Debug` (`aacs::types`, `private_key: "<redacted>"` + `certificate_len`) —
+an EXTERNAL guarantee this crate silently inherits through the derive, with
+nothing on this side pinning it. This test pins it: it catches a libfreemkv
+upgrade that swaps that impl for `#[derive(Debug)]`, which would leak the
+host private key out of this crate's public API with no diff in this
+repository at all.
+
+Rendered three ways because callers reach the cert three ways: the whole
+`Vec` (`db.host_certs`), one element, and the inner `HostCert` a caller
+gets back from `KeyDb::host_certs(mkb)`.
+
+## KEYDB-parser integration tests relocated from libfreemkv
+
+These exercise the parser (`KeyDb::load`) end-to-end against a real
+keydb.cfg and feed its material into libfreemkv's AACS crypto
+(`derive_vuk`, then `decrypt_unit` + `is_clean_ts`). They live here now
+that the parser lives here. All are `KEYDB_PATH`-env-gated and no-op in CI
+when the env is unset; they must still COMPILE.
+
+## `test_decrypt_real_unit` — no-decrypt outcome
+
+Reaching the fallback path is the EXPECTED outcome for the AACS 2.0 (BEE)
+sample this test was written against: the unit is still bus-encrypted, so
+no unit key alone can open it. That makes "no key worked" an unusable
+assertion on its own — which is exactly why this test used to end in an
+`eprintln!` and fall off the end, i.e. pass unconditionally. A no-op
+`decrypt_unit`, an `is_clean` stuck at `false`, or a parser that yielded
+zero keys would all have produced this same clean exit.
+
+What CAN be asserted regardless of the sample: the pipeline actually ran,
+and the decrypt primitive actually transformed the bytes.
+
+## Hand-written redacting `Debug`
+
+Key material must never reach a log, a panic message, or a bug report. Both
+`KeyDb` and `DiscEntry` carry raw AACS key bytes, and both are public API, so
+a `#[derive(Debug)]` on either is a leak of the crown jewels: `{:?}` on a
+loaded `KeyDb` printed every processing key plus every disc's Media Key,
+VID, VUK and Unit Keys. libfreemkv's equivalent types (`aacs::types`:
+`DeviceKey`, `HostCert`, `Vid`, `MediaKey`, `Vuk`, `ProcessingKey`,
+`UnitKey`, its own `DiscEntry`) all hand-write a redacting `Debug` for
+exactly this reason; this crate's copy of the parser dropped that
+convention when it was relocated. These restore it, field-for-field in the
+same style (`<redacted>` for key bytes, a `_len` for key-bearing
+collections). Pinned by `keydb_debug_is_redacted` /
+`disc_entry_debug_is_redacted`.

docs/keydb.md

@@ -0,0 +1,432 @@
+# `keydb.rs` design notes
+
+Long-form rationale moved out of `src/keydb.rs` doc comments to satisfy the
+repo's comment-guard line caps. Each section is pointed to from the file by a
+one-line `// See docs/keydb.md#<anchor>` comment.
+
+## candidate-order
+
+Full derivation path this source walks, cheapest-first:
+
+1. per-disc **Unit Keys** (hash hit) → returned terminal, no derivation.
+2. per-disc **VUK** (hash hit) → `uks_from_vuk` over the disc's encrypted
+   title keys.
+3. a **Media Key**, then `derive_vuk` → `uks_from_vuk`. The MK comes from, in
+   order: the disc's stored MK (hash hit); the keydb's **Processing Key**
+   pool walked against THIS disc's MKB via `derive_media_key_from_pk`; or the
+   device-key pool via `derive_media_key_from_dk`. The PK and DK pools
+   resolve the Media Key WITHOUT a VID; the final `derive_vuk` still needs
+   one. The VID is the unlocker's physical VID (`ResolveCtx::vid`) when
+   present, else the keydb entry's OWN stored VID (the `I` field, `vid`) for
+   the non-physical / ISO path. With no VID from either source the MK path
+   cannot complete — return nothing.
+
+The cross-disc MK-pool brute (trying OTHER discs' stored media keys against
+this disc) stays RETIRED: every MK path here is anchored to the matched
+disc's own MKB or stored material.
+
+The library still OWNS the crypto; this source owns only which primitive to
+call with which material. Returning an empty `Vec` is a genuine "no key for
+this disc here".
+
+## file-identity
+
+`(len, modified)` ALONE IS NOT AN IDENTITY. mtime is stored at whole-second
+resolution on HFS+, on many container overlays and on most network
+filesystems, so a writer that replaces `keydb.cfg` within the same second
+with a file of the same byte length leaves `(len, modified)` bit-identical —
+and the cache then serves the superseded parse for the rest of the process's
+life. For a key database that is not a stale page: it is serving retired or
+since-corrected keys while reporting success. The writers that hit this are
+real and named in this crate's own docs: the daily-refresh job, a second
+freemkv process, an operator's editor, a sync tool.
+
+Two independent discriminators close it:
+
+1. `dev` + `ino`. Every writer that publishes ATOMICALLY (temp file, fsync,
+   rename — what `KeydbSource::save` does, and what any correct publisher
+   does) creates a NEW inode, so the stamp differs immediately and
+   unconditionally, whatever the clock or the mtime resolution did. Zero on
+   platforms without them (see `FileStamp::of`), which costs nothing: rule 2
+   stands alone.
+2. A freshness rule on the cache entry — `CacheEntry::is_settled` — which
+   covers the residual in-place rewrite that reuses the inode.
+
+What this still does NOT catch: a writer that FORGES the timestamp, i.e.
+restores an mtime equal to the cached one (`touch -t`, `rsync --times`, a
+tar/backup restore) into the SAME inode with the SAME length. Nothing short
+of hashing the content can see that, and hashing 62 MiB per lookup is
+precisely the cost the cache exists to remove (~141 ms × ≥3 loads per rip).
+`KeydbSource::save` therefore keeps invalidating the cache directly, so the
+one write path this crate owns never depends on any of the above.
+
+## settle-proof
+
+Whether a cache entry's stamp can be TRUSTED to change if the file changes.
+
+The proof, for a filesystem storing mtime at granularity `G`: suppose this
+entry was stamped at `stamped_at` with `stamped_at - modified >= G`, and some
+writer later rewrites the file in place at wall time `T > stamped_at`. The
+recorded mtime `m'` of that write is within one granule of `T`, so
+`m' >= T - G > stamped_at - G >= modified`. Hence `m' != modified` and the
+stamp DOES change. The ambiguous window is therefore exactly the one where
+the cached entry was stamped less than `G` after the file's own mtime — i.e.
+we looked at the file while it was still "hot" — and in that window the
+entry is refused and the file is re-read. Once the file has been quiet for
+`G`, its entry settles and the cache is fully effective again.
+
+Cost: after every keydb write, lookups re-parse for up to `G` (2 s). The
+published keydb is rewritten daily, so that is ~2 s of re-parsing per day
+against a cache that saves ~420 ms per rip.
+
+`modified: None` (a filesystem that reports no mtime) never settles: the only
+two discriminators left would be size and inode, so the entry is re-read
+every time rather than being trusted on a weaker basis.
+
+CLOCK CAVEAT: `stamped_at` is the local clock and `modified` is the
+filesystem's. On a network share whose server clock runs AHEAD of the
+client's by more than `G`, entries never settle (safe: more re-parsing). A
+server clock BEHIND the client's makes an entry settle early, which re-opens
+the same-second window it closes — the inode rule (1) is what covers the
+realistic writers there.
+
+## keydbsource-cache
+
+`KeydbSource` caches the parsed database behind the file's identity stamp.
+The published keydb is ~62 MiB / ~181k entries and a single AACS-cert rip
+parses it repeatedly — `host_certs()` for the drive handshake (freemkv's
+`pipe.rs`/`engine.rs`, autorip's `keysource.rs` each build their own source),
+then the trait `host_certs(mkb)` during resolution, then `get_unit_keys` —
+and every one of those calls used to re-read all 62 MiB from disk and
+re-parse it from scratch, because the struct held nothing but a `PathBuf`.
+
+A keydb replaced underneath this source is picked up on the next call, with
+ONE stated exception: a writer that rewrites the file in place, into the
+same inode, at the same byte length, having restored the previous mtime
+exactly (`touch -t` / `rsync --times` / a backup restore). Everything else —
+any atomic rename, any length change, any real clock advance — changes the
+stamp. `FileStamp` and `CacheEntry::is_settled` carry the full argument and
+the clock caveats; do not weaken either without re-reading them. (The
+earlier wording here — "an externally replaced keydb is still picked up" —
+was simply false for a same-second, same-length replacement, which is the
+common shape of a daily-refresh job on a whole-second-mtime filesystem.)
+
+MEMORY RESIDENCY (accepted, and deliberately recorded rather than fixed
+here): a hit keeps the entire parsed keydb — every disc's Media Key / VID /
+VUK / Unit Keys, the processing- and device-key pools, and the host-cert
+PRIVATE keys — resident for the process's life, where the pre-cache code
+held one parse transiently. Nothing in this crate zeroizes on drop, so that
+material was always exposed to a core dump or a swapped page; the cache
+widens the WINDOW, it does not create the class. Closing it properly means
+zeroize-on-drop across `KeyDb`, `DiscEntry` and libfreemkv's `aacs::types`
+(which owns most of the buffers) — a cross-crate change, tracked separately,
+NOT smuggled in behind a cache fix.
+
+## cached-db-one-open
+
+Errors are the same ones `KeyDb::load` produces, unchanged: a missing file
+is `NotFound` (the documented benign case), an over-cap or non-UTF-8 file is
+`InvalidData`. The stamp costs an `fstat`, not a 62 MiB read.
+
+ONE OPEN, ONE IDENTITY. The file is opened once and both the stamp (`fstat`
+on that handle) and, on a miss, the bytes come from it. The previous shape —
+`std::fs::metadata(path)` and then a separate `KeyDb::load(path)` — resolved
+the path TWICE with no atomicity between them, so a rename landing in the
+gap cached the new file's stamp beside the old file's contents (or the
+reverse); the pairing was then served, looking perfectly valid, until the
+stamp changed again. The stamp is read BEFORE the bytes on purpose: a write
+racing the read then leaves the OLD stamp cached, which the next call
+detects, whereas stamping afterwards would file torn content under a
+fresh-looking identity and pin it.
+
+## cached-db-reemit
+
+Re-emit the parser's rejection summary on EVERY hit, not just on the parse
+that produced it. `KeyDb::parse` is what logs it, and a hit skips `parse`
+entirely — so a daemon that cached a corrupt keydb logged its rejected-row
+counts exactly once, at whatever moment it first read the file, and then
+served that same damaged database to every later rip in silence. Repetition
+is the correct failure mode here: the line is emitted only when the file
+really does have rejected or duplicate rows, and a warning an operator can
+still see on the tenth rip is worth more than one they had to catch on the
+first.
+
+## load-failure
+
+A MISSING keydb is the documented benign case: the app may simply not have
+one, another source may hold the key, and "no keydb" is genuinely "no key
+here". Everything else is NOT that: `InvalidData` is a keydb over the
+128 MiB cap or one that is not valid UTF-8 — i.e. corrupt, truncated, or a
+half-finished download — and a permission/IO failure means the file could
+not be read at all. Reporting either as an empty result made a broken keydb
+indistinguishable from "this disc has no key" (the same conflation as the
+seven-hour 502), with no log to notice it by. Both are now logged AND
+surfaced as errors so the resolver reports a source failure instead of
+`E7022`.
+
+## save-mirror-parse
+
+Mirror `KeyDb::parse`'s disc-entry rule EXACTLY by CALLING it
+(`is_disc_entry_line`), so `save()` never validates + persists content that
+parses to zero usable entries (e.g. a stray "0xDEADBEEF" comment line) and
+the two can no longer drift — they did drift once already, on the `0X` case
+rule.
+
+## unit-keys-from
+
+CPS-unit numbering: a returned `UnitKey::idx` is the POSITIONAL index
+libfreemkv's `resolve_and_apply` turns into the canonical CPS-unit number
+`idx + 1`. For the terminal per-disc unit-key path we therefore map the
+keydb's stored CPS number `num` to `idx = num - 1`, so the committed number
+is byte-identical to the keydb's `num` (and to what the OLD
+`Key::Unit(entry.unit_keys)` path committed). For the VUK / MK paths the
+boil primitive already yields 0-based positional indices, matching
+`parse_unit_key_ro`'s `(i + 1)` after the resolver's `+ 1`.
+
+## unit-keys-per-disc-hit
+
+Per-disc hit (most specific). `find_disc` normalizes the hash form. Without
+a matched entry this keydb has no per-disc material (Unit Keys / VUK / Media
+Key / stored VID) to anchor a derivation for the disc, so it resolves
+nothing. (The PK and DK pools are global, but the cross-disc MK-pool brute —
+trying OTHER discs' media keys against this disc — stays retired; a PK/DK
+pool only ever resolves a disc reached through its own matched entry below.)
+
+## unit-keys-union
+
+UNION every source of terminal keys, then dedup — never first-hit. A stored
+`unit_keys` list can be PARTIAL (the key-import tool only ever sampled the
+CPS units reachable from a playlist, so an orphan unit's key may be
+missing), while the per-disc VUK boils EVERY declared CPS unit. Taking the
+stored list alone (the old return-at-first-path) would shadow the VUK and
+silently drop the orphan unit's key. So gather both and keep a unique-by-key
+list: the read path tries every key per unit, so an extra or stale key is
+harmless — only a MISSING key hurts.
+
+## unit-keys-vuk-or-mk
+
+2. Per-disc VUK — one step, no VID needed; boils ALL declared units.
+3. Else a Media Key path (stored MK / PK pool / DK pool) → VUK → all
+   declared units. The MK itself carries no VID, but the final `vuk_from_mk`
+   needs one: physical (unlocker) VID first, else the entry's stored VID
+   (`I` field), else cannot derive. Either branch yields the COMPLETE
+   declared set, so we take the first that resolves (VUK preferred —
+   cheapest).
+
+## write-atomic
+
+keydb.cfg is the single source of AACS truth, and save/update run
+unattended (first-boot download + daily-refresh thread, with a container
+restart on every release). A bare in-place `fs::write` truncates the file
+before writing, so a SIGKILL (docker stop's grace window), OOM-kill, power
+loss, or ENOSPC mid-write would leave the keydb half-written — the prior
+good copy already gone. A truncated keydb doesn't error at write time; it
+silently breaks key resolution on every later AACS rip. Writing to a temp
+file then renaming (POSIX rename is atomic within a filesystem) means an
+interrupted update leaves the previous keydb fully intact.
+
+The fsync MUST succeed before the rename: a `sync_all` failure (ENOSPC,
+ESTALE on the bind-mounted volume) means the kernel never guaranteed the
+bytes reached stable storage, so publishing them via rename would defeat
+crash-safety. The temp name is unique per call (pid + monotonic counter) so
+a concurrent update can't share a fixed temp path and rename a mangled file
+over the keydb.
+
+## get-unit-keys-trait
+
+A MISSING keydb is not an error — it simply yields no keys (another source
+may have them), the same as the library's own loader. A keydb that exists
+but cannot be USED (over the size cap, not UTF-8, unreadable) is a source
+failure and returns `Err`: it says nothing about whether this disc has a
+key, and reporting it as an empty result is the same looks-like-success
+failure as a 502 reported as `E7022`.
+
+The keydb carries no AACS 2.1 forensic index keys today, so it does not
+override `get_fmts_indexes` — the default (empty) opts it out, and an FMTS
+disc's forensic set comes from the online source.
+
+## host-certs-trait
+
+Expose the keydb's host certs through the trait — the OEM/AACS cert-auth
+route collects them across every source via this method. Wires the disc's
+MKB generation through for revocation filtering (the keydb parser's
+`; Revoked in MKBv<N>` annotation): a cert revoked at generation `R` is
+withheld once the disc's generation reaches `R`.
+
+## test-pinned-mtime
+
+Force a file's mtime to `PINNED_MTIME`, so two different files can be made
+bit-identical to a `(len, mtime)` stamp on purpose. The cache tests that
+matter all depend on FORCING the indistinguishable case rather than hoping
+the filesystem produces it: on APFS (nanosecond mtimes) a same-second
+replacement is otherwise detected by mtime alone, and a test that relies on
+that passes with the fix reverted — which is worth nothing.
+
+## test-committed
+
+The committed `(cps, key)` pairs libfreemkv's `resolve_and_apply` derives
+from a source's Unit Keys: positional `idx` → canonical CPS number
+`idx + 1`. The KATs compare against THIS to prove byte-identical parity with
+the OLD `Key::Unit` / resolver-derived commit.
+
+## test-mock-ctx-stub
+
+`MockCtx` implements the full `ResolveCtx` trait so it can stand in for a
+real disc, but `KeydbSource` itself never calls `title()`/`samples()` (those
+matter to the online source, not the local keydb). Exercise them directly
+so the mock's trait surface is proven correct too.
+
+## test-kat-a
+
+Here `enc_title_keys` is empty, so the VUK can't derive anything — only the
+stored list contributes, and it commits byte-identically to the stored
+`(cps, key)` pairs.
+
+## test-orphan-union
+
+Orphan-unit completeness (the real keydb bug): an entry stores only `uk1`
+(the key-import tool sampled one reachable CPS unit) but ALSO carries the
+VUK, which boils BOTH declared units. The old return-at-first-path handed
+back just `[uk1]`, shadowing the VUK and silently dropping the orphan unit.
+The union must return BOTH — the stored uk1 AND the VUK-derived second unit.
+
+## test-kat-c
+
+A hash hit with a Media Key and a physical VID (from the unlocker) derives
+`MK → VUK → UK`. The PHYSICAL VID must be used in preference to the keydb's
+stored VID — proven by giving the entry a DIFFERENT stored VID and showing
+the result tracks the physical one.
+
+## test-kat-f
+
+Owner decision #1 (AACS): a keydb Processing Key must be walked against the
+matched disc's own MKB to recover the Media Key, then driven down the full
+chain `PK → MK → VUK → UK`. The disc entry carries NO stored MK/VUK/UK — the
+only key material is a global `PK` row — and the result must be the real
+Unit Keys, byte-identical to deriving from the recovered MK.
+
+The MKB + PK use a known-answer construction (a planted PK whose derived MK
+satisfies the synthetic verify record); the constants are precomputed AES
+vectors so this crate needs no AES primitive of its own. They mirror
+libfreemkv's `boil::mk_from_pk_drives_full_chain_to_uks` KAT.
+
+## test-repeated-lookups
+
+A single AACS-cert rip drives this source at least three times — the
+scan-options builder's inherent `host_certs()`, the trait `host_certs(mkb)`
+during the handshake, then `get_unit_keys` — and each call used to re-read
+and re-parse the WHOLE file, because `KeydbSource` held nothing but a
+`PathBuf`. Measured on the real 62 MiB / 184,860-entry keydb: ~140 ms per
+parse in `--release`, i.e. ~420 ms and ~186 MiB of file reads per rip,
+against ~1 µs for a cache hit.
+
+Catches the mutation that drops the cache (every call re-parses: count 3)
+and the mutation that makes the cache unconditional (see the invalidation
+tests below).
+
+The mtime is pinned into the PAST first, because a cache entry is only
+trusted once the file has been quiet for the mtime granularity
+(`CacheEntry::is_settled`) — a keydb written microseconds ago is
+deliberately re-read. Unix-only: pinning an mtime needs `touch -t` (no
+`filetime` dependency for a test).
+
+## test-changed-keydb
+
+The cache must not go stale: a keydb replaced UNDERNEATH the source (the
+daily-refresh thread writing through a different `KeydbSource`, or an
+operator dropping in a new file) has a different size/mtime stamp and must
+be re-read. Catches the mutation that caches on first load and never
+re-stats — which would pin a rip to a keydb deleted hours earlier.
+
+## test-two-discriminators
+
+The defect both of the inode/settle tests below pin: `(len, mtime)` is not a
+file identity. An external writer — the daily-refresh job, a second freemkv
+process, an editor, a sync tool — that replaces `keydb.cfg` inside one mtime
+granule with a file of the SAME length leaves that pair bit-identical, and
+the source then serves the superseded parse for the rest of the process's
+life: retired or since-corrected keys, reported as success.
+
+Each test disables the OTHER discriminator through `mtime_granularity`, so
+neither can pass on the strength of the one it is not testing, and neither
+depends on the host filesystem's real mtime resolution (on APFS the naive
+scenario is not reproducible at all).
+
+## test-inode-rename
+
+A keydb replaced by ATOMIC RENAME — how every correct publisher, including
+this crate's own `save()`, installs a new file — must be re-read even when
+length and mtime are identical. `mtime_granularity: 0` makes every entry
+settle, so ONLY dev+ino can distinguish the two files here.
+
+Catches the mutation that drops `dev`/`ino` from `FileStamp` (the original
+`(len, modified)` stamp): the source then keeps answering with the previous
+keydb's keys.
+
+## test-inplace-rewrite
+
+A keydb rewritten IN PLACE — same inode, same length, mtime forced back to
+the same value — must still be re-read while the cached entry is too young
+to trust. `mtime_granularity` is set to a decade so nothing ever settles,
+which is the same condition a real same-second, whole-second-mtime
+replacement creates and the only way to reproduce it deterministically on a
+nanosecond-mtime filesystem.
+
+Catches the mutation that deletes the `is_settled` check from `cached_db`
+(every stamp match trusted unconditionally, which is what the original code
+did): dev, ino, len and mtime are ALL identical here, so without the
+freshness rule the source serves the old keys forever.
+
+## test-corrupt-warns
+
+`ParseStats::log` runs inside `KeyDb::parse`, and a cache hit skips `parse`
+— so once the cache landed, the rejected-row summary for a given file was
+emitted exactly ONCE for the entire life of the process, while that same
+damaged file went on being served to every subsequent rip. A long-running
+daemon (autorip) logs it at startup, the operator misses it in the startup
+noise or loses it to log rotation, and there is no second chance for as long
+as the process runs.
+
+Catches the mutation that deletes the `emit_parse_stats` call from the
+cache-HIT arm of `cached_db`: warnings would drop to 1 while the file is
+still served three times.
+
+## test-healthy-silent
+
+Catches the mutation that makes `emit_parse_stats` unconditional (drops
+`ParseStats::log`'s empty-counts early return).
+
+## test-fresh-not-trusted
+
+Catches the mutation that widens the window to something unbounded (a
+granularity of hours would re-parse 62 MiB on every lookup all day) by
+pairing with `repeated_lookups_parse_the_keydb_once`, which proves the entry
+DOES settle once the mtime is in the past.
+
+## test-save-invalidates
+
+`save()` replaces the very file this source reads, so it drops the cached
+parse ITSELF rather than trusting the (size, mtime) stamp: a same-size
+rewrite that lands inside the filesystem's timestamp granularity is the one
+change the stamp cannot see — and it is exactly the change this crate
+performs (the daily-refresh thread re-saving a keydb through the source it
+then reads from).
+
+The test forces that indistinguishable case rather than hoping for it: the
+file is written, stamped to a FIXED mtime, cached, re-saved with different
+keys of the SAME length, and stamped back to the same mtime. To the stamp
+the two files are identical, so only the explicit invalidation can produce
+the new key. Unix-only because pinning an mtime needs `touch -t` (no
+`filetime` dependency for one test).
+
+## test-corrupt-is-error
+
+`KeyDb::load` fails distinguishably for a keydb that is over the 128 MiB cap
+or not valid UTF-8 — a truncated download, a half-written file, a binary
+blob dropped in by mistake. Both used to collapse into `Ok(Vec::new())` with
+ZERO tracing, i.e. into the benign "no key for this disc" the resolver
+reports as `E7022` — the same looks-like-success failure as the seven-hour
+502. Only a MISSING file is genuinely benign.
+
+Catches the mutation that restores `Err(_) => Ok(Vec::new())`.
+</content>

docs/lib-scope.md

@@ -0,0 +1,49 @@
+# freemkv-keysources: scope and design notes
+
+## Mechanism vs. policy
+
+Applications (autorip, the `freemkv` CLI) choose and order the key sources
+from their own config — the local-vs-online policy is just which impls they
+plug in — then resolve and hand the resulting key to `Disc::decrypt_with`.
+
+Each source resolves a disc's terminal **Unit Keys** in one shot via
+`KeySource::get_unit_keys`, driving libfreemkv's boil-down crypto primitives
+for whatever level of material it holds. Compose several with `MultiSource`
+in the caller's chosen order.
+
+Reading the encrypted content-sample units a key server validates on, and
+applying the resolved keys against a disc, is decryption *mechanism* — it
+lives in the library (`libfreemkv::resolve_and_apply`,
+`libfreemkv::read_encrypted_units`), not here.
+
+## `MultiSource` failure semantics
+
+`MultiSource::get_unit_keys` tries each inner source in order and returns the
+first non-empty Unit Key set (and `get_fmts_indexes` does the same for the
+forensic set). The caller supplies the list AND the order — local-first
+`[Keydb, Online]`, online-first `[Online, Keydb]`, etc. — so the "which
+sources, in what order" policy lives entirely with the application, not the
+library. `MultiSource` is itself a `KeySource`, so it nests and composes.
+
+**A source that could not ANSWER is not a source that answered "no key".**
+When no inner source produces keys, the composition returns `Err` if any
+inner source failed, and `Ok(empty)` only when every source genuinely
+answered and none held a key. This mirrors libfreemkv's own resolver
+(`keysource::drive_unit_keys`, which tracks an `errored` flag, and
+`source_failure`, which stamps the first failure onto the disc) — a
+composition that swallowed the `Err` would hand the resolver a clean
+`Ok(empty)`, and an outage would once again be reported to the operator as
+`E7022 No key source has a decryption key for this disc`.
+
+## `first_non_empty` ordering rule
+
+A source failure never blocks the chain (a later source is still tried, and
+a later success still wins outright, error or no error), but it is not
+forgotten either: with no keys anywhere, the FIRST failure is returned.
+First (not last) matches libfreemkv's `source_failure` rule, so the
+most-preferred source's reason is the one the operator is told about, in the
+same order the caller expressed a preference for successes.
+
+`get` selects which trait method to drive, so the base and forensic paths
+share ONE implementation of this rule and cannot drift apart — the forensic
+half carried the identical bug.

docs/multi-source-failure-collapse.md

@@ -0,0 +1,13 @@
+# `MultiSource`: a source that could not answer is not a source that said no
+
+`MultiSource` used to re-collapse the Ok/Err distinction `OnlineSource` was
+engineered to draw: `if let Ok(uks) = s.get_unit_keys(ctx)` discarded every
+`Err`, so a composition whose sources ALL failed returned `Ok(Vec::new())` —
+which the resolver reports as `E7022 No key source has a decryption key for
+this disc`. That is the seven-hour-502 incident reproduced one layer up.
+
+The regression test `multi_source_reports_a_source_failure_instead_of_a_clean_no_key`
+is THE regression check, one layer up from `interpret_reply`: when no source
+holds a key AND a source could not answer, the composition must report the
+FAILURE, never a clean "no key". It catches the mutation that restores
+`if let Ok(uks) = ..` (which drops the `Err` and returns `Ok(empty)`).

docs/online-agent-cache.md

@@ -0,0 +1,39 @@
+# `src/online.rs` — `OnlineSource::agent` caching
+
+## Why cache at all
+
+`query` used to build a FRESH `ureq::Agent` every call, and an FMTS disc
+makes two calls per rip (`get_unit_keys` + `get_fmts_indexes`), so it paid
+two TLS handshakes to the same host for no reason — an agent owns the
+connection pool, and a discarded agent discards the pooled, already
+negotiated TLS connection with it.
+
+## Security invariant
+
+The agent is reused ONLY when the freshly resolved + guarded address SET is
+identical to the one the cached agent is pinned to. Every query still
+re-resolves and re-runs the SSRF guard — that is the whole anti-rebinding
+defence and is deliberately not cached; what is reused is the connection,
+and only to addresses just re-validated this call.
+
+## Why the cache key is a sorted, deduped SET, not an ordered sequence
+
+`to_socket_addrs` hands back whatever order the resolver felt like, and a
+round-robin keyserver reorders its A/AAAA records on essentially every
+lookup — an ordered `==` therefore missed the cache every single time
+against exactly the deployment that most needs it, silently degrading to
+the per-query TLS handshake this field exists to avoid.
+
+Set equality is also the honest predicate: the security property being
+asserted is "these are the same validated addresses", which says nothing
+about their order. The agent itself stays pinned to the order it was BUILT
+with; that is safe because an equal set means every address it holds was
+re-resolved and re-guarded this call.
+
+## `a_reordered_but_identical_address_set_reuses_the_agent`
+
+Catches the mutation that drops the sort/dedup from `agent_for`'s key (the
+reversed order would then build a second agent) and, via the second half of
+the test, the mutation that "fixes" order-sensitivity by comparing only
+lengths or the first element — which would wrongly reuse an agent pinned to
+a DIFFERENT address.

docs/online-guardfail.md

@@ -0,0 +1,41 @@
+# `src/online.rs` — `GuardFail` classification details
+
+## Malformed-port authority
+
+A `host:port` authority whose port is not a u16 is a TYPO in the operator's
+config — `https://example.com:notaport/keys`. This used to fall back to the
+WHOLE authority as the hostname (port text included), which of course never
+resolves, so the URL surfaced as `GuardFail::Unreachable` →
+`Err(KeyServiceUnavailable)`: a standing misconfiguration reported as a
+transient outage, the exact collapse `GuardFail`'s own doc says must not
+happen, because the two halves demand OPPOSITE operator actions (fix the URL
+vs. wait).
+
+Reject it as `Config`, identical to the bracketed-IPv6 branch a few lines
+above, which already returns `Config` for an unparseable port. Silently
+substituting `default_port` would be worse still — it would ship the bearer
+token and key material to a port the operator never configured.
+
+## Guard-blocked address (`Config` arm, `resolve_and_guard`)
+
+Log THAT the URL was rejected, never WHY: the guard's message names the
+resolved address, and an internal address must not reach a log an operator
+may paste into a bug report. The static label is enough to separate
+"misconfigured/blocked key-service URL" from "this disc has no key", which
+is the only distinction the operator needs here.
+
+`error!`, not `warn!`, and `Err`, not `Ok(empty)`. This arm used to return
+`Ok(Vec::new())`, which made the PERMANENT fault quieter than the transient
+one directly above it: the service was never asked, yet the composition was
+handed a clean empty and — with no other source holding the key — told the
+operator `E7022 no key for this disc`. Same lie as the seven-hour 502, from
+a config typo that never self-heals. A failure to ask is never evidence
+about the disc, so it leaves as `Err`; `MultiSource::first_non_empty` still
+lets a later source's real key win outright, so this cannot fail a rip that
+the local keydb could have served.
+
+NOTE (reported upstream): libfreemkv has no `KeyServiceMisconfigured` code,
+so the permanent fault borrows the transient E7028. E7028's contract — "the
+source never got as far as answering the question" — is exactly true here;
+only its "transient, retry later" hint is wrong, and the `error!` log
+carries the correcting detail until a 70xx config code exists.

docs/online-preflight-guard-tests.md

@@ -0,0 +1,20 @@
+# `src/online.rs` tests — pre-flight guards in `query`
+
+Each of the three guards (cleartext scheme, over-cap MKB, under-sampled)
+fires BEFORE any address is resolved and before anything is sent. None was
+exercised before these tests: a reordering that let the cleartext POST
+through would have shipped green, and that POST carries the bearer token
+plus base64 key material.
+
+Their VERDICTS differ on purpose (see docs/online-query-guards.md): the
+cleartext-scheme guard is an operator fault and yields `Err`, while the two
+input-shaped guards yield `Ok(empty)` — this source has nothing for THIS
+disc, permanently, and calling that an outage would send the operator
+waiting for nothing.
+
+The discriminator in all three: the configured host is `.test`, which RFC
+6761 guarantees never resolves. On the guarded path nothing resolves it, so
+the test touches no network and returns `Ok(empty)`. Remove the guard and
+control reaches `resolve_and_guard`, which fails as `Unreachable` ->
+`Err(KeyServiceUnavailable)` — a different Result, so the mutation cannot
+pass.

docs/online-query-contract.md

@@ -0,0 +1,19 @@
+# `src/online.rs` — `OnlineSource::query` return contract
+
+The return type draws THE distinction this source exists to draw:
+
+* `Ok(non-empty)` — the service answered with a key.
+* `Ok(empty)` — the service ANSWERED and holds no key for this disc, or this
+  source had nothing to ask with (no service configured, a misconfigured
+  URL, an over-cap MKB, too few samples). A genuine miss; the resolver moves
+  to the next source and `E7022` is the right verdict.
+* `Err(..)` — the service could not answer: unreachable, timed out, DNS
+  failed, returned 5xx, rejected the token (401/403), rate-limited (429), or
+  replied with something unreadable. NOTHING is known about whether a key
+  exists. Collapsing this into `Ok(empty)` is the bug this signature fixes:
+  a seven-hour run of HTTP 502s was reported to operators as
+  `E7022 No key source has a decryption key for this disc`, and they went
+  hunting for a VUK when the correct action was to wait.
+
+`&self`: one-shot is the resolver's contract (each source's
+`get_unit_keys` is called once), so no per-call latch is needed.

docs/online-query-guards.md

@@ -0,0 +1,57 @@
+# `src/online.rs` — `OnlineSource::query` pre-flight guards
+
+## Refusing cleartext (`http://`)
+
+The request body always carries base64 AACS key material
+(inf/mkb/units/vid) and, when configured, a bearer token in a header — so an
+`http://` POST hands both to any on-path observer, and the token is
+replayable.
+
+There is no legitimate plaintext deployment to preserve here: the address
+guard rejects loopback, RFC1918, ULA and link-local, so an `http://` URL can
+only ever reach the PUBLIC internet — the single worst case for cleartext. A
+self-hosted keyserver on a LAN is already impossible by that guard, whatever
+its scheme.
+
+Still falls THROUGH to the next key source (a local keydb) — an `Err` here
+does not block the chain, because `MultiSource::first_non_empty` returns any
+later source's non-empty key set outright and only surfaces the failure
+when nothing anywhere resolved. What it does stop is the silent case: a
+cleartext URL is the same permanent operator fault as the address-guard
+rejection (see `GuardFail`), so it gets the same verdict. Returning
+`Ok(empty)` here made "we refused to ask" indistinguishable from "the
+service answered and has no key".
+
+## The three-way skip/fault/miss line (MKB cap, sample count)
+
+Written out after the `Config`-was-quieter-than-`Unreachable` bug, in three
+parts:
+
+* NO URL AT ALL is not a fault: the operator did not configure an online
+  source, so there is nothing to report. `Ok(empty)`, silently (the first
+  guard in `query`).
+* A bad URL — wrong scheme, mistyped port, guard-blocked address — is an
+  OPERATOR FAULT. Every disc is affected, nothing was asked, and the fix is
+  to edit the config. That is `Err`.
+* An over-cap MKB or too few content samples is a property of THIS DISC's
+  inputs, not a fault: the request cannot be *formed* for this disc, so this
+  source has nothing for it and never will, whatever the operator does and
+  however long they wait. Reporting that as `KeyServiceUnavailable` would
+  send the operator waiting out an outage that does not exist — the same
+  mislabelling in the opposite direction. These stay `Ok(empty)`, and stay
+  logged.
+
+Logging on the MKB-cap skip matters: a silent empty return there is
+indistinguishable from "no key", so the real cause is surfaced (the cap is
+64 MiB, far above any real trimmed MKB).
+
+## `cleartext_http_url_is_refused_before_anything_is_sent`
+
+Catches two mutations: sending the request anyway (the host is `.test` and
+would surface as `Unreachable`, but the assertion would still hold, which is
+why `too_few_samples_skips_the_request` guards the send-path ordering
+separately) and — the one this test was rewritten for — returning
+`Ok(Vec::new())`, which made a permanent cleartext misconfiguration
+indistinguishable from a genuine miss. A later source's real key still wins
+(`MultiSource::first_non_empty`), so refusing does not fail a rip the local
+keydb can serve.

docs/online-ssrf-guard.md

@@ -0,0 +1,44 @@
+# `src/online.rs` — SSRF guard
+
+## Threat model
+
+The keyserver URL is operator-supplied and the bearer token is confidential.
+After `validate_keyserver_url` checked the host at config time, an attacker
+who controls the keyserver's DNS can rebind it to 169.254.169.254 (cloud
+metadata) or an RFC1918 host in the window between validation and the actual
+POST, exfiltrating the key material and the Authorization token.
+
+## Defence
+
+Resolve the host once just before the POST, reject any blocked IP, and pin
+the ureq connection to those validated addresses so a subsequent DNS flip
+cannot redirect the request. Use `redirects(0)` so a public URL can't
+30x-redirect to an internal host.
+
+## Resolver wiring pitfall (`PinnedResolver`)
+
+ureq 3 replaced v2's resolver closure with the `Resolver` trait. The agent
+MUST be built through `Agent::with_parts` to take a custom one:
+`Agent::new_with_config` silently keeps the default resolver, which would
+send the request to live DNS and quietly reopen the rebinding window this
+module exists to close. That failure is invisible — it has no symptom short
+of an actual attack — so it is pinned by the test
+`hardened_agent_connects_to_the_pinned_address_not_dns`.
+
+## Why that test is structured the way it is
+
+`resolve_and_guard` VALIDATES addresses; `hardened_agent` PINS them, so a
+hostname cannot re-resolve to an internal address between validation and
+connection. The validation half is well covered by other tests — but every
+one of those still passes if the pinned resolver is never consulted, because
+none of them makes a connection. Nothing else in the file proves the agent
+honours the pin, and a mis-wired resolver fails OPEN: the connection quietly
+falls back to live DNS and the rebinding window this module exists to close
+is reopened, with no visible symptom.
+
+The discriminator: pin the agent to a loopback listener the test owns, then
+ask it for a host that CANNOT resolve — `.test` is reserved by RFC 6761 and
+never resolves in the public DNS. Only a consulted resolver can turn that
+name into a connection; a fallback to live DNS fails instead. No network is
+touched, and `query` (whose guard blocks loopback by design) is not
+involved — this drives `hardened_agent` directly.

docs/paths.md

@@ -0,0 +1,13 @@
+# `keydb.cfg` path policy (`src/paths.rs`)
+
+Key-path policy belongs with the key sources (this crate), not the library:
+libfreemkv is handed a path and reads it. The CLI/app asks here for the
+list of locations to *search* (first existing wins) and for the single
+*default* location to write to (e.g. `update-keys`/save).
+
+freemkv is a portable, standalone binary: the `keydb.cfg` lives *next to*
+the executable — `<dir of current exe>/keydb.cfg` — and nowhere else. There
+is no OS-specific config-dir lookup (`%APPDATA%`, `%USERPROFILE%\.config`,
+`$XDG_CONFIG_HOME`, `$HOME/.config`). Drop the exe and its `keydb.cfg` in
+the same folder and it works. Callers needing a custom location pass
+`--keydb PATH`, which bypasses this module entirely.

src/keydb.rs

@@ -1,33 +1,12 @@
 //! `keydb.cfg` key source (source #1).
 //!
 //! Parses a local `keydb.cfg`, looks the disc up by hash, and derives the
-//! disc's terminal **Unit Keys** itself by composing libfreemkv's raw
-//! `aacs::derive` primitives (`derive_vuk` / `decrypt_unit_key` /
-//! `derive_media_key_from_pk` / `derive_media_key_from_dk`) — never
-//! re-implementing AES. The path it picks mirrors the OLD candidate order
-//! (which libfreemkv's resolver used to walk) EXACTLY, cheapest-first:
-//!
-//! 1. per-disc **Unit Keys** (hash hit)  → returned terminal, no derivation.
-//! 2. per-disc **VUK** (hash hit)        → `uks_from_vuk` over the disc's
-//!    encrypted title keys.
-//! 3. a **Media Key**, then `derive_vuk` → `uks_from_vuk`. The MK comes
-//!    from, in order: the disc's stored MK (hash hit); the keydb's
-//!    **Processing Key** pool walked against THIS disc's MKB via
-//!    `derive_media_key_from_pk`; or the device-key pool via
-//!    `derive_media_key_from_dk`. The PK and DK pools resolve the
-//!    Media Key WITHOUT a VID; the final `derive_vuk` still needs one. The
-//!    VID is the unlocker's physical VID ([`ResolveCtx::vid`]) when present, else
-//!    the keydb entry's OWN stored VID (the `I` field, `vid`) for the
-//!    non-physical / ISO path. With no VID from either source the MK path cannot
-//!    complete — return nothing.
-//!
-//! The cross-disc MK-pool brute (trying OTHER discs' stored media keys against
-//! this disc) stays RETIRED: every MK path here is anchored to the matched
-//! disc's own MKB or stored material.
-//!
-//! The library still OWNS the crypto; this source owns only which primitive to
-//! call with which material. Returning an empty `Vec` is a genuine "no key for
-//! this disc here".
+//! disc's terminal **Unit Keys** by composing libfreemkv's raw
+//! `aacs::derive` primitives — never re-implementing AES. The path mirrors
+//! the OLD candidate order EXACTLY, cheapest-first: per-disc Unit Keys, then
+//! VUK, then a Media Key (stored / PK pool / DK pool) via `derive_vuk`. See
+//! docs/keydb.md#candidate-order for the full path and VID rules. The
+//! library owns the crypto; this source owns only which primitive to call.
 
 use std::io::{Read, Write};
 use std::path::{Path, PathBuf};
@@ -42,11 +21,9 @@ use libfreemkv::{Error, KeySource};
 
 use crate::keydb_format::KeyDb;
 
-/// Upper bound on decompressed keydb size. The published keydb is a few MiB;
-/// 128 MiB is a generous ceiling that still caps a decompression bomb (a tiny
-/// zip/gz can otherwise inflate to GiB and OOM the daily refresh thread). The
-/// public UHD keydb is ~62 MiB and growing, so 64 MiB was getting tight;
-/// 128 MiB leaves years of headroom.
+// Upper bound on decompressed keydb size: caps a decompression bomb (a tiny
+// zip/gz could otherwise inflate to GiB and OOM the daily refresh thread).
+// The public UHD keydb is ~62 MiB and growing; 128 MiB leaves headroom.
 const MAX_KEYDB_BYTES: u64 = 128 * 1024 * 1024;
 
 /// Result of a KEYDB save/update -- path written, entry count, and byte size.
@@ -57,42 +34,14 @@ pub struct UpdateResult {
     pub bytes: usize,
 }
 
-/// Widest observed granularity of a filesystem's stored mtime. HFS+, many
-/// container overlay filesystems and most network filesystems record whole
-/// seconds; 2 s leaves room for that plus rounding. Used by
-/// [`CacheEntry::is_settled`] — see the argument there.
+// Widest observed granularity of a filesystem's stored mtime (HFS+, many
+// container/network filesystems record whole seconds); 2 s leaves rounding
+// room. See docs/keydb.md#settle-proof for CacheEntry::is_settled's argument.
 const MTIME_GRANULARITY: std::time::Duration = std::time::Duration::from_secs(2);
 
-/// The identity of the keydb file a cache entry was parsed from.
-///
-/// `(len, modified)` ALONE IS NOT AN IDENTITY. mtime is stored at whole-second
-/// resolution on HFS+, on many container overlays and on most network
-/// filesystems, so a writer that replaces `keydb.cfg` within the same second
-/// with a file of the same byte length leaves `(len, modified)` bit-identical —
-/// and the cache then serves the superseded parse for the rest of the process's
-/// life. For a key database that is not a stale page: it is serving retired or
-/// since-corrected keys while reporting success. The writers that hit this are
-/// real and named in this crate's own docs: the daily-refresh job, a second
-/// freemkv process, an operator's editor, a sync tool.
-///
-/// Two independent discriminators close it:
-///
-/// 1. `dev` + `ino`. Every writer that publishes ATOMICALLY (temp file, fsync,
-///    rename — what [`KeydbSource::save`] does, and what any correct publisher
-///    does) creates a NEW inode, so the stamp differs immediately and
-///    unconditionally, whatever the clock or the mtime resolution did. Zero on
-///    platforms without them (see [`Self::of`]), which costs nothing: rule 2
-///    stands alone.
-/// 2. A freshness rule on the cache entry — [`CacheEntry::is_settled`] — which
-///    covers the residual in-place rewrite that reuses the inode.
-///
-/// What this still does NOT catch: a writer that FORGES the timestamp, i.e.
-/// restores an mtime equal to the cached one (`touch -t`, `rsync --times`, a
-/// tar/backup restore) into the SAME inode with the SAME length. Nothing short
-/// of hashing the content can see that, and hashing 62 MiB per lookup is
-/// precisely the cost the cache exists to remove (~141 ms × ≥3 loads per rip).
-/// [`KeydbSource::save`] therefore keeps invalidating the cache directly, so the
-/// one write path this crate owns never depends on any of the above.
+// The identity of the keydb file a cache entry was parsed from. `(len,
+// modified)` alone is not an identity (see docs/keydb.md#file-identity for
+// why); `dev`+`ino` plus CacheEntry::is_settled close the gap.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 struct FileStamp {
     /// Filesystem device id. `0` where unavailable.
@@ -109,11 +58,9 @@ impl FileStamp {
     /// handle (an `fstat`), never from a second `std::fs::metadata` on the path:
     /// only the handle guarantees the stamp and the bytes describe one file.
     fn of(meta: &std::fs::Metadata) -> Self {
-        // dev/ino are a Unix concept. Windows has a rough equivalent
-        // (volume serial + file index) but exposes it only through
-        // `MetadataExt` methods that are documented as unreliable on some
-        // filesystems; rule 2 alone is correct there, so the cross-platform
-        // fallback is simply "no inode discriminator" rather than a shaky one.
+        // dev/ino are a Unix concept; Windows's rough equivalent is documented
+        // unreliable on some filesystems, so the cross-platform fallback is
+        // simply "no inode discriminator" (rule 2 alone is correct there).
         #[cfg(unix)]
         let (dev, ino) = {
             use std::os::unix::fs::MetadataExt;
@@ -145,34 +92,9 @@ struct CacheEntry {
 }
 
 impl CacheEntry {
-    /// Whether this entry's stamp can be TRUSTED to change if the file changes.
-    ///
-    /// The proof, for a filesystem storing mtime at granularity `G`: suppose
-    /// this entry was stamped at `stamped_at` with `stamped_at - modified >= G`,
-    /// and some writer later rewrites the file in place at wall time
-    /// `T > stamped_at`. The recorded mtime `m'` of that write is within one
-    /// granule of `T`, so `m' >= T - G > stamped_at - G >= modified`. Hence
-    /// `m' != modified` and the stamp DOES change. The ambiguous window is
-    /// therefore exactly the one where the cached entry was stamped less than
-    /// `G` after the file's own mtime — i.e. we looked at the file while it was
-    /// still "hot" — and in that window the entry is refused and the file is
-    /// re-read. Once the file has been quiet for `G`, its entry settles and the
-    /// cache is fully effective again.
-    ///
-    /// Cost: after every keydb write, lookups re-parse for up to `G` (2 s).
-    /// The published keydb is rewritten daily, so that is ~2 s of re-parsing per
-    /// day against a cache that saves ~420 ms per rip.
-    ///
-    /// `modified: None` (a filesystem that reports no mtime) never settles: the
-    /// only two discriminators left would be size and inode, so the entry is
-    /// re-read every time rather than being trusted on a weaker basis.
-    ///
-    /// CLOCK CAVEAT: `stamped_at` is the local clock and `modified` is the
-    /// filesystem's. On a network share whose server clock runs AHEAD of the
-    /// client's by more than `G`, entries never settle (safe: more re-parsing).
-    /// A server clock BEHIND the client's makes an entry settle early, which
-    /// re-opens the same-second window it closes — the inode rule (1) is what
-    /// covers the realistic writers there.
+    // Whether this entry's stamp can be TRUSTED to change if the file changes.
+    // Proof, cost, and the clock caveat are in docs/keydb.md#settle-proof;
+    // don't weaken this without re-reading them.
     fn is_settled(&self, granularity: std::time::Duration) -> bool {
         self.stamp
             .modified
@@ -183,58 +105,29 @@ impl CacheEntry {
 
 /// A [`KeySource`] backed by a local `keydb.cfg` file.
 ///
-/// The parsed database is CACHED behind the file's identity stamp. The
-/// published keydb is ~62 MiB / ~181k entries and a single AACS-cert rip parses
-/// it repeatedly — `host_certs()` for the drive handshake (freemkv's
-/// `pipe.rs`/`engine.rs`, autorip's `keysource.rs` each build their own source),
-/// then the trait `host_certs(mkb)` during resolution, then `get_unit_keys` —
-/// and every one of those calls used to re-read all 62 MiB from disk and re-parse
-/// it from scratch, because the struct held nothing but a `PathBuf`.
-///
-/// A keydb replaced underneath this source is picked up on the next call, with
-/// ONE stated exception: a writer that rewrites the file in place, into the same
-/// inode, at the same byte length, having restored the previous mtime exactly
-/// (`touch -t` / `rsync --times` / a backup restore). Everything else — any
-/// atomic rename, any length change, any real clock advance — changes the stamp.
-/// [`FileStamp`] and [`CacheEntry::is_settled`] carry the full argument and the
-/// clock caveats; do not weaken either without re-reading them. (The earlier
-/// wording here — "an externally replaced keydb is still picked up" —
-/// was simply false for a same-second, same-length replacement, which is the
-/// common shape of a daily-refresh job on a whole-second-mtime filesystem.)
-///
-/// MEMORY RESIDENCY (accepted, and deliberately recorded rather than fixed
-/// here): a hit keeps the entire parsed keydb — every disc's Media Key / VID /
-/// VUK / Unit Keys, the processing- and device-key pools, and the host-cert
-/// PRIVATE keys — resident for the process's life, where the pre-cache code held
-/// one parse transiently. Nothing in this crate zeroizes on drop, so that
-/// material was always exposed to a core dump or a swapped page; the cache
-/// widens the WINDOW, it does not create the class. Closing it properly means
-/// zeroize-on-drop across `KeyDb`, `DiscEntry` and libfreemkv's `aacs::types`
-/// (which owns most of the buffers) — a cross-crate change, tracked separately,
-/// NOT smuggled in behind a cache fix.
+/// The parsed database is CACHED behind the file's identity stamp: a single
+/// AACS-cert rip calls `host_certs()`, the trait `host_certs(mkb)`, and
+/// `get_unit_keys`, and each used to re-read + re-parse the whole
+/// ~62 MiB file. A replaced keydb is picked up on the next call, with one
+/// narrow exception; see docs/keydb.md#keydbsource-cache for that exception
+/// and the memory-residency tradeoff — do not weaken [`FileStamp`] or
+/// [`CacheEntry::is_settled`] without re-reading it.
 pub struct KeydbSource {
     path: PathBuf,
-    /// `Mutex` (not `RwLock`): the guarded section is a stamp comparison plus an
-    /// `Arc` clone, so there is nothing for concurrent readers to win. Poisoning
-    /// is recovered from rather than propagated — a panic elsewhere must not
-    /// turn every later key lookup into a panic.
+    // Mutex, not RwLock: the guarded section is a stamp compare + Arc clone,
+    // so there's nothing for concurrent readers to win. Poisoning recovers
+    // rather than propagates — a panic elsewhere must not poison every lookup.
     cache: Mutex<Option<CacheEntry>>,
-    /// How many times the file was actually read + parsed (cache MISSES). The
-    /// only honest way to assert the cache from a test: timing is flaky and the
-    /// parsed value is identical either way. Cheap enough to keep in release.
+    // Cache MISSES (real reads+parses). The only honest way to assert the
+    // cache from a test, since timing is flaky and the parsed value is
+    // identical either way.
     parses: AtomicUsize,
-    /// The `G` of [`CacheEntry::is_settled`]. A field, not the constant inlined,
-    /// so the two discriminators can be tested ONE AT A TIME: with `G` at zero
-    /// every entry settles, which isolates the inode/len rule; with `G` enormous
-    /// nothing settles, which isolates the freshness rule. Testing them together
-    /// is how a stale-cache test ends up passing whether or not the fix is
-    /// present — on APFS (nanosecond mtimes) the naive same-second scenario is
-    /// simply not reproducible, so a test that "reproduces" it there is
-    /// asserting nothing.
+    // The `G` of CacheEntry::is_settled, as a field so tests can isolate the
+    // two staleness discriminators one at a time (0 vs. a huge duration). See
+    // docs/keydb.md#test-two-discriminators.
     mtime_granularity: std::time::Duration,
-    /// How many corruption summaries were emitted (see
-    /// [`KeydbSource::emit_parse_stats`]). Same rationale as `parses`: the
-    /// alternative instrument is a `tracing` subscriber in dev-dependencies.
+    // Corruption summaries emitted (see emit_parse_stats). Same rationale as
+    // `parses`: avoids pulling a `tracing` subscriber into dev-dependencies.
     warnings: AtomicUsize,
 }
 
@@ -250,11 +143,9 @@ impl KeydbSource {
         }
     }
 
-    /// Log the parser's rejection summary and count the emission.
-    ///
-    /// The count is the only way to assert, without pulling a `tracing`
-    /// subscriber into dev-dependencies, that the corruption warning is NOT
-    /// swallowed by the cache. See [`Self::cached_db`].
+    // Log the parser's rejection summary and count the emission — the count
+    // is the only way to assert, without a `tracing` dev-dependency, that the
+    // corruption warning is NOT swallowed by the cache. See `cached_db`.
     fn emit_parse_stats(&self, stats: &crate::keydb_format::ParseStats) {
         if stats.log() {
             self.warnings.fetch_add(1, Ordering::Relaxed);
@@ -276,22 +167,9 @@ impl KeydbSource {
         self
     }
 
-    /// The parsed keydb, from cache when the file is provably unchanged.
-    ///
-    /// Errors are the same ones [`KeyDb::load`] produces, unchanged: a missing
-    /// file is `NotFound` (the documented benign case), an over-cap or non-UTF-8
-    /// file is `InvalidData`. The stamp costs an `fstat`, not a 62 MiB read.
-    ///
-    /// ONE OPEN, ONE IDENTITY. The file is opened once and both the stamp
-    /// (`fstat` on that handle) and, on a miss, the bytes come from it. The
-    /// previous shape — `std::fs::metadata(path)` and then a separate
-    /// `KeyDb::load(path)` — resolved the path TWICE with no atomicity between
-    /// them, so a rename landing in the gap cached the new file's stamp beside
-    /// the old file's contents (or the reverse); the pairing was then served,
-    /// looking perfectly valid, until the stamp changed again. The stamp is read
-    /// BEFORE the bytes on purpose: a write racing the read then leaves the OLD
-    /// stamp cached, which the next call detects, whereas stamping afterwards
-    /// would file torn content under a fresh-looking identity and pin it.
+    // The parsed keydb, from cache when unchanged (errors mirror KeyDb::load).
+    // ONE OPEN, ONE IDENTITY: stamp (fstat) and bytes share one handle. See
+    // docs/keydb.md#cached-db-one-open for why a separate metadata()+load() pair was unsound.
     fn cached_db(&self) -> std::io::Result<Arc<KeyDb>> {
         let file = std::fs::File::open(&self.path)?;
         let stamp = FileStamp::of(&file.metadata()?);
@@ -301,16 +179,9 @@ impl KeydbSource {
             && entry.stamp == stamp
             && entry.is_settled(self.mtime_granularity)
         {
-            // Re-emit the parser's rejection summary on EVERY hit, not just on
-            // the parse that produced it. `KeyDb::parse` is what logs it, and a
-            // hit skips `parse` entirely — so a daemon that cached a corrupt
-            // keydb logged its rejected-row counts exactly once, at whatever
-            // moment it first read the file, and then served that same damaged
-            // database to every later rip in silence. Repetition is the correct
-            // failure mode here: the line is emitted only when the file really
-            // does have rejected or duplicate rows, and a warning an operator
-            // can still see on the tenth rip is worth more than one they had to
-            // catch on the first.
+            // Re-emit the rejection summary on EVERY hit, not just the parse
+            // that produced it (a hit skips `KeyDb::parse`) — otherwise a
+            // corrupt keydb warns once, then serves silently. See docs/keydb.md#cached-db-reemit.
             self.emit_parse_stats(&entry.stats);
             return Ok(entry.db.clone());
         }
@@ -340,18 +211,9 @@ impl KeydbSource {
         self.parses.load(Ordering::Relaxed)
     }
 
-    /// Turn a [`KeyDb::load`] failure into this source's verdict.
-    ///
-    /// A MISSING keydb is the documented benign case: the app may simply not
-    /// have one, another source may hold the key, and "no keydb" is genuinely
-    /// "no key here". Everything else is NOT that: `InvalidData` is a keydb over
-    /// the 128 MiB cap or one that is not valid UTF-8 — i.e. corrupt, truncated,
-    /// or a half-finished download — and a permission/IO failure means the file
-    /// could not be read at all. Reporting either as an empty result made a
-    /// broken keydb indistinguishable from "this disc has no key" (the same
-    /// conflation as the seven-hour 502), with no log to notice it by. Both are
-    /// now logged AND surfaced as errors so the resolver reports a source
-    /// failure instead of `E7022`.
+    // Turn a KeyDb::load failure into this source's verdict: MISSING is the
+    // documented benign case (Ok/None); everything else is logged and
+    // surfaced as an error, not silently reported as "no key". See docs/keydb.md#load-failure.
     fn load_failure(&self, e: &std::io::Error) -> Option<Error> {
         match e.kind() {
             std::io::ErrorKind::NotFound => None,
@@ -380,13 +242,12 @@ impl KeydbSource {
     /// Validate, decompress, and crash-safely persist raw keydb bytes (plain
     /// text, `.zip`, or `.gz`) to THIS source's own [`path`](Self::path).
     ///
-    /// The bytes are decompressed (zip / gz / plain), the result is checked for
-    /// at least one recognisable keydb entry, then atomically written to the
-    /// source's path (sibling-temp + fsync + rename + parent-dir fsync). The
-    /// decompressed size is capped at [`MAX_KEYDB_BYTES`] so a decompression
-    /// bomb can't OOM the caller (e.g. the daily-refresh thread). Writing to the
-    /// source's own path — not a hardcoded default — means the caller decides
-    /// the destination (CLI `--keydb`, the autorip service path, …).
+    /// The bytes are decompressed (zip / gz / plain), checked for at least
+    /// one recognisable keydb entry, then atomically written to the source's
+    /// path (sibling-temp + fsync + rename + parent-dir fsync). Decompressed
+    /// size is capped at [`MAX_KEYDB_BYTES`] (decompression-bomb guard).
+    /// Writes to the source's own path, not a hardcoded default, so the
+    /// caller decides the destination (CLI `--keydb`, the autorip service path).
     pub fn save(&self, bytes: &[u8]) -> Result<UpdateResult, Error> {
         let text = if bytes.starts_with(b"PK\x03\x04") {
             extract_zip(bytes)?
@@ -403,11 +264,9 @@ impl KeydbSource {
             .lines()
             .filter(|l| {
                 let t = l.trim();
-                // Mirror KeyDb::parse's disc-entry rule EXACTLY by CALLING it
-                // (`is_disc_entry_line`), so save() never validates + persists
-                // content that parses to zero usable entries (e.g. a stray
-                // "0xDEADBEEF" comment line) and the two can no longer drift —
-                // they did drift once already, on the `0X` case rule.
+                // Mirror KeyDb::parse's disc-entry rule EXACTLY by CALLING it,
+                // so save() never persists content that parses to zero usable
+                // entries. See docs/keydb.md#save-mirror-parse.
                 crate::keydb_format::is_disc_entry_line(t)
                     || t.starts_with("| DK")
                     || t.starts_with("| PK")
@@ -468,38 +327,20 @@ impl KeydbSource {
         }
     }
 
-    /// Derive this disc's terminal Unit Keys from a parsed keydb. Pure (no I/O),
-    /// so it is unit-testable against an in-memory `KeyDb` without a file on
-    /// disk. Empty `Vec` = no key for this disc from this keydb.
-    ///
-    /// CPS-unit numbering: a returned [`UnitKey::idx`] is the POSITIONAL index
-    /// libfreemkv's `resolve_and_apply` turns into the canonical CPS-unit number
-    /// `idx + 1`. For the terminal per-disc unit-key path we therefore map the
-    /// keydb's stored CPS number `num` to `idx = num - 1`, so the committed
-    /// number is byte-identical to the keydb's `num` (and to what the OLD
-    /// `Key::Unit(entry.unit_keys)` path committed). For the VUK / MK paths the
-    /// boil primitive already yields 0-based positional indices, matching
-    /// `parse_unit_key_ro`'s `(i + 1)` after the resolver's `+ 1`.
+    // Derive this disc's terminal Unit Keys from a parsed keydb. Pure (no
+    // I/O). Empty Vec = no key for this disc from this keydb. CPS-unit
+    // numbering (idx = num - 1) is explained in docs/keydb.md#unit-keys-from.
     fn unit_keys_from(db: &KeyDb, ctx: &dyn ResolveCtx) -> Vec<UnitKey> {
-        // Per-disc hit (most specific). find_disc normalizes the hash form.
-        // Without a matched entry this keydb has no per-disc material (Unit Keys
-        // / VUK / Media Key / stored VID) to anchor a derivation for the disc, so
-        // it resolves nothing. (The PK and DK pools are global, but the
-        // cross-disc MK-pool brute — trying OTHER discs' media keys against this
-        // disc — stays retired; a PK/DK pool only ever resolves a disc reached
-        // through its own matched entry below.)
+        // Per-disc hit (most specific); find_disc normalizes the hash form.
+        // Without a match there is no per-disc anchor, so the global PK/DK
+        // pools are never consulted. See docs/keydb.md#unit-keys-per-disc-hit.
         let Some(entry) = db.find_disc(ctx.disc_hash()) else {
             return Vec::new();
         };
 
-        // UNION every source of terminal keys, then dedup — never first-hit. A
-        // stored `unit_keys` list can be PARTIAL (the key-import tool only ever
-        // sampled the CPS units reachable from a playlist, so an orphan unit's key may be
-        // missing), while the per-disc VUK boils EVERY declared CPS unit. Taking
-        // the stored list alone (the old return-at-first-path) would shadow the
-        // VUK and silently drop the orphan unit's key. So gather both and keep a
-        // unique-by-key list: the read path tries every key per unit, so an extra
-        // or stale key is harmless — only a MISSING key hurts.
+        // UNION every source of terminal keys, then dedup — never first-hit,
+        // since a stored `unit_keys` list can be PARTIAL while the VUK boils
+        // every declared unit. See docs/keydb.md#unit-keys-union.
         let mut keys: Vec<UnitKey> = Vec::new();
 
         // 1. Terminal Unit Keys stored in the entry — directly usable, no
@@ -508,19 +349,13 @@ impl KeydbSource {
             keys.push(UnitKey::new(num.saturating_sub(1), *key));
         }
 
-        // The disc's encrypted title keys (from Unit_Key_RO.inf) — what every
-        // VUK-or-deeper path decrypts into the terminal keys. Empty when the scan
-        // captured no Unit_Key_RO.inf, in which case only the stored list (1)
+        // The disc's encrypted title keys (from Unit_Key_RO.inf). Empty when
+        // the scan captured none, in which case only the stored list (1)
         // contributes.
         let enc_title_keys = ctx.enc_title_keys().unwrap_or(&[]);
         if !enc_title_keys.is_empty() {
-            // 2. Per-disc VUK — one step, no VID needed; boils ALL declared units.
-            //    3. Else a Media Key path (stored MK / PK pool / DK pool) → VUK →
-            //       all declared units. The MK itself carries no VID, but the
-            //       final `vuk_from_mk` needs one: physical (unlocker) VID first,
-            //       else the entry's stored VID (`I` field), else cannot derive.
-            //       Either branch yields the COMPLETE declared set, so we take the
-            //       first that resolves (VUK preferred — cheapest).
+            // VUK path, else MK path (stored/PK/DK) → VUK. See
+            // docs/keydb.md#unit-keys-vuk-or-mk for the VID rules.
             let derived = if let Some(vuk) = entry.vuk {
                 uks_from_vuk(&vuk, enc_title_keys)
             } else {
@@ -585,25 +420,9 @@ fn extract_zip(data: &[u8]) -> Result<String, Error> {
     Err(Error::KeydbInvalid)
 }
 
-/// Write `text` to `path` crash-safely (create parent dir, write a sibling temp
-/// file, fsync, then atomic rename, then fsync the parent dir).
-///
-/// keydb.cfg is the single source of AACS truth, and save/update run unattended
-/// (first-boot download + daily-refresh thread, with a container restart on
-/// every release). A bare in-place `fs::write` truncates the file before
-/// writing, so a SIGKILL (docker stop's grace window), OOM-kill, power loss, or
-/// ENOSPC mid-write would leave the keydb half-written — the prior good copy
-/// already gone. A truncated keydb doesn't error at write time; it silently
-/// breaks key resolution on every later AACS rip. Writing to a temp file then
-/// renaming (POSIX rename is atomic within a filesystem) means an interrupted
-/// update leaves the previous keydb fully intact.
-///
-/// The fsync MUST succeed before the rename: a `sync_all` failure (ENOSPC,
-/// ESTALE on the bind-mounted volume) means the kernel never guaranteed the
-/// bytes reached stable storage, so publishing them via rename would defeat
-/// crash-safety. The temp name is unique per call (pid + monotonic counter) so a
-/// concurrent update can't share a fixed temp path and rename a mangled file
-/// over the keydb.
+// Write `text` to `path` crash-safely (temp file, fsync, atomic rename) so an
+// interrupted update never leaves a half-written keydb. See
+// docs/keydb.md#write-atomic for the fsync-before-rename argument.
 fn write_atomic(path: &Path, text: &str) -> Result<(), Error> {
     let werr = || Error::KeydbWrite {
         path: path.display().to_string(),
@@ -649,18 +468,9 @@ fn write_atomic(path: &Path, text: &str) -> Result<(), Error> {
 }
 
 impl KeySource for KeydbSource {
-    /// Resolve this disc's base per-CPS-unit Unit Keys from the keydb.
-    ///
-    /// A MISSING keydb is not an error — it simply yields no keys (another
-    /// source may have them), the same as the library's own loader. A keydb that
-    /// exists but cannot be USED (over the size cap, not UTF-8, unreadable) is a
-    /// source failure and returns `Err`: it says nothing about whether this disc
-    /// has a key, and reporting it as an empty result is the same
-    /// looks-like-success failure as a 502 reported as `E7022`.
-    ///
-    /// The keydb carries no AACS 2.1 forensic index keys today, so it does not
-    /// override `get_fmts_indexes` — the default (empty) opts it out, and an FMTS
-    /// disc's forensic set comes from the online source.
+    // Resolve this disc's base per-CPS-unit Unit Keys from the keydb. A
+    // MISSING keydb yields no keys (Ok(empty)); an UNUSABLE one is a source
+    // failure (Err) — never conflated. See docs/keydb.md#get-unit-keys-trait.
     fn get_unit_keys(&self, ctx: &dyn ResolveCtx) -> Result<Vec<UnitKey>, Error> {
         match self.cached_db() {
             Ok(db) => Ok(Self::unit_keys_from(&db, ctx)),
@@ -673,11 +483,9 @@ impl KeySource for KeydbSource {
         }
     }
 
-    /// Expose the keydb's host certs through the trait — the OEM/AACS cert-auth
-    /// route collects them across every source via this method. Wires the disc's
-    /// MKB generation through for revocation filtering (the keydb parser's
-    /// `; Revoked in MKBv<N>` annotation): a cert revoked at generation `R` is
-    /// withheld once the disc's generation reaches `R`.
+    // Expose the keydb's host certs through the trait, wiring the disc's MKB
+    // generation through for revocation filtering. See
+    // docs/keydb.md#host-certs-trait.
     fn host_certs(&self, mkb: Option<u32>) -> Vec<HostCert> {
         match self.cached_db() {
             Ok(db) => db.host_certs(mkb),
@@ -703,12 +511,9 @@ mod tests {
     #[cfg(unix)]
     const PINNED_MTIME: &str = "202601011200.00";
 
-    /// Force a file's mtime to [`PINNED_MTIME`], so two different files can be
-    /// made bit-identical to a `(len, mtime)` stamp on purpose. The cache tests
-    /// that matter all depend on FORCING the indistinguishable case rather than
-    /// hoping the filesystem produces it: on APFS (nanosecond mtimes) a
-    /// same-second replacement is otherwise detected by mtime alone, and a test
-    /// that relies on that passes with the fix reverted — which is worth nothing.
+    // Force a file's mtime to PINNED_MTIME so two different files can be
+    // made bit-identical to a (len, mtime) stamp on purpose, rather than
+    // hoping the filesystem produces that case. See docs/keydb.md#test-pinned-mtime.
     #[cfg(unix)]
     fn pin_mtime(p: &std::path::Path) {
         let ok = std::process::Command::new("touch")
@@ -797,10 +602,9 @@ mod tests {
         }
     }
 
-    /// The committed `(cps, key)` pairs libfreemkv's `resolve_and_apply` derives
-    /// from a source's Unit Keys: positional `idx` → canonical CPS number
-    /// `idx + 1`. The KATs compare against THIS to prove byte-identical parity
-    /// with the OLD `Key::Unit` / resolver-derived commit.
+    // The committed (cps, key) pairs resolve_and_apply derives from a
+    // source's Unit Keys (idx -> idx + 1). KATs compare against THIS. See
+    // docs/keydb.md#test-committed.
     fn committed(uks: &[UnitKey]) -> Vec<(u32, [u8; 16])> {
         uks.iter()
             .map(|u| (u.idx.saturating_add(1), u.key))
@@ -809,10 +613,9 @@ mod tests {
 
     const HASH: &str = "0xaabb";
 
-    /// `MockCtx` implements the full `ResolveCtx` trait so it can stand in for
-    /// a real disc, but `KeydbSource` itself never calls `title()`/`samples()`
-    /// (those matter to the online source, not the local keydb). Exercise
-    /// them directly so the mock's trait surface is proven correct too.
+    // MockCtx implements the full trait so it can stand in for a real disc,
+    // but KeydbSource never calls title()/samples(); exercise them directly
+    // so the mock's trait surface is proven correct too.
     #[test]
     fn mock_ctx_title_and_samples_are_stubbed_correctly() {
         let c = ctx(HASH, Vec::new(), None);
@@ -820,11 +623,9 @@ mod tests {
         assert_eq!(c.samples(4).unwrap(), Vec::<Vec<u8>>::new());
     }
 
-    // ── KAT (a): disc with terminal Unit Keys, no enc_title_keys ──────────────
-    /// Stored terminal unit keys are returned with their CPS numbering preserved.
-    /// Here `enc_title_keys` is empty, so the VUK can't derive anything — only the
-    /// stored list contributes, and it commits byte-identically to the stored
-    /// `(cps, key)` pairs.
+    // KAT (a): disc with terminal Unit Keys, no enc_title_keys. Stored
+    // terminal unit keys are returned with CPS numbering preserved. See
+    // docs/keydb.md#test-kat-a.
     #[test]
     fn kat_a_disc_with_unit_keys_is_terminal_and_preserves_cps_numbering() {
         let mut e = blank_entry(HASH);
@@ -841,11 +642,9 @@ mod tests {
         );
     }
 
-    /// Orphan-unit completeness (the real keydb bug): an entry stores only `uk1`
-    /// (the key-import tool sampled one reachable CPS unit) but ALSO carries the VUK,
-    /// which boils BOTH declared units. The old return-at-first-path handed back
-    /// just `[uk1]`, shadowing the VUK and silently dropping the orphan unit. The
-    /// union must return BOTH — the stored uk1 AND the VUK-derived second unit.
+    // Orphan-unit completeness (the real keydb bug): a PARTIAL stored list
+    // plus a VUK that boils BOTH declared units must return BOTH, not shadow
+    // the VUK with the partial list. See docs/keydb.md#test-orphan-union.
     #[test]
     fn union_partial_stored_plus_vuk_yields_all_declared_units() {
         let vuk = [0x5Au8; 16];
@@ -899,11 +698,9 @@ mod tests {
         );
     }
 
-    // ── KAT (c): disc with MK + physical (unlock) VID ─────────────────────────
-    /// A hash hit with a Media Key and a physical VID (from the unlocker) derives
-    /// `MK → VUK → UK`. The PHYSICAL VID must be used in preference to the keydb's
-    /// stored VID — proven by giving the entry a DIFFERENT stored VID and showing
-    /// the result tracks the physical one.
+    // KAT (c): MK + physical (unlock) VID derives MK -> VUK -> UK. The
+    // PHYSICAL VID must win over the keydb's stored VID. See
+    // docs/keydb.md#test-kat-c.
     #[test]
     fn kat_c_disc_with_mk_uses_physical_vid_over_keydb_vid() {
         let mk = [0x77u8; 16];
@@ -987,17 +784,9 @@ mod tests {
         rec
     }
 
-    // ── KAT (f): disc with NO per-disc MK, resolved via the keydb PK pool ──────
-    /// Owner decision #1 (AACS): a keydb Processing Key must be walked against
-    /// the matched disc's own MKB to recover the Media Key, then driven down the
-    /// full chain `PK → MK → VUK → UK`. The disc entry carries NO stored MK/VUK/
-    /// UK — the only key material is a global `PK` row — and the result must be
-    /// the real Unit Keys, byte-identical to deriving from the recovered MK.
-    ///
-    /// The MKB + PK use a known-answer construction (a planted PK whose derived
-    /// MK satisfies the synthetic verify record); the constants are precomputed
-    /// AES vectors so this crate needs no AES primitive of its own. They mirror
-    /// libfreemkv's `boil::mk_from_pk_drives_full_chain_to_uks` KAT.
+    // KAT (f): disc with NO per-disc MK, resolved via the keydb PK pool
+    // (PK -> MK -> VUK -> UK) with a known-answer MKB/PK construction. See
+    // docs/keydb.md#test-kat-f.
     #[test]
     fn kat_f_disc_with_pk_pool_yields_uks() {
         // Planted PK and the MK it resolves to (see libfreemkv boil.rs KAT).
@@ -1163,25 +952,9 @@ mod tests {
         std::fs::remove_dir_all(&dir).ok();
     }
 
-    // ── Caching: the 62 MiB / 181k-entry keydb is parsed ONCE ────────────────
-
-    /// A single AACS-cert rip drives this source at least three times — the
-    /// scan-options builder's inherent `host_certs()`, the trait
-    /// `host_certs(mkb)` during the handshake, then `get_unit_keys` — and each
-    /// call used to re-read and re-parse the WHOLE file, because `KeydbSource`
-    /// held nothing but a `PathBuf`. Measured on the real 62 MiB / 184,860-entry
-    /// keydb: ~140 ms per parse in `--release`, i.e. ~420 ms and ~186 MiB of
-    /// file reads per rip, against ~1 µs for a cache hit.
-    ///
-    /// Catches the mutation that drops the cache (every call re-parses: count 3)
-    /// and the mutation that makes the cache unconditional (see the
-    /// invalidation tests below).
-    ///
-    /// The mtime is pinned into the PAST first, because a cache entry is only
-    /// trusted once the file has been quiet for the mtime granularity
-    /// ([`CacheEntry::is_settled`]) — a keydb written microseconds ago is
-    /// deliberately re-read. Unix-only: pinning an mtime needs `touch -t` (no
-    /// `filetime` dependency for a test).
+    // Caching: the 62 MiB / 181k-entry keydb is parsed ONCE. A single
+    // AACS-cert rip drives this source at least three times; the mtime is
+    // pinned into the PAST first, since an entry is only trusted once settled. See docs/keydb.md#test-repeated-lookups.
     #[cfg(unix)]
     #[test]
     fn repeated_lookups_parse_the_keydb_once() {
@@ -1210,11 +983,9 @@ mod tests {
         );
     }
 
-    /// The cache must not go stale: a keydb replaced UNDERNEATH the source (the
-    /// daily-refresh thread writing through a different `KeydbSource`, or an
-    /// operator dropping in a new file) has a different size/mtime stamp and
-    /// must be re-read. Catches the mutation that caches on first load and never
-    /// re-stats — which would pin a rip to a keydb deleted hours earlier.
+    // The cache must not go stale: a keydb replaced UNDERNEATH the source has
+    // a different size/mtime stamp and must be re-read. See
+    // docs/keydb.md#test-changed-keydb.
     #[test]
     fn a_changed_keydb_file_is_reparsed() {
         let dir = scratch("cache-stamp");
@@ -1241,29 +1012,9 @@ mod tests {
         assert_eq!(src.parse_count(), 2, "one parse per distinct file");
     }
 
-    // ── The two stale-cache discriminators, tested ONE AT A TIME ────────────
-    //
-    // The defect both of these pin: `(len, mtime)` is not a file identity. An
-    // external writer — the daily-refresh job, a second freemkv process, an
-    // editor, a sync tool — that replaces `keydb.cfg` inside one mtime granule
-    // with a file of the SAME length leaves that pair bit-identical, and the
-    // source then serves the superseded parse for the rest of the process's
-    // life: retired or since-corrected keys, reported as success.
-    //
-    // Each test disables the OTHER discriminator through `mtime_granularity`, so
-    // neither can pass on the strength of the one it is not testing, and neither
-    // depends on the host filesystem's real mtime resolution (on APFS the naive
-    // scenario is not reproducible at all).
-
-    /// Discriminator 1, the inode: a keydb replaced by ATOMIC RENAME — how
-    /// every correct publisher, including this crate's own `save()`, installs a
-    /// new file — must be re-read even when length and mtime are identical.
-    /// `mtime_granularity: 0` makes every entry settle, so ONLY dev+ino can
-    /// distinguish the two files here.
-    ///
-    /// Catches the mutation that drops `dev`/`ino` from `FileStamp` (the
-    /// original `(len, modified)` stamp): the source then keeps answering with
-    /// the previous keydb's keys.
+    // Two stale-cache discriminators, tested ONE AT A TIME (each test
+    // disables the other via `mtime_granularity`). Discriminator 1, the
+    // inode: an ATOMIC RENAME must be re-read even with identical length and mtime. See docs/keydb.md#test-inode-rename.
     #[cfg(unix)]
     #[test]
     fn a_same_length_same_mtime_rename_is_still_re_read() {
@@ -1298,17 +1049,9 @@ mod tests {
         assert_eq!(src.parse_count(), 2, "the replacement must be re-parsed");
     }
 
-    /// Discriminator 2, the settle window: a keydb rewritten IN PLACE — same
-    /// inode, same length, mtime forced back to the same value — must still be
-    /// re-read while the cached entry is too young to trust. `mtime_granularity`
-    /// is set to a decade so nothing ever settles, which is the same condition a
-    /// real same-second, whole-second-mtime replacement creates and the only way
-    /// to reproduce it deterministically on a nanosecond-mtime filesystem.
-    ///
-    /// Catches the mutation that deletes the `is_settled` check from
-    /// `cached_db` (every stamp match trusted unconditionally, which is what the
-    /// original code did): dev, ino, len and mtime are ALL identical here, so
-    /// without the freshness rule the source serves the old keys forever.
+    // Discriminator 2, the settle window: a keydb rewritten IN PLACE (same
+    // inode/length, mtime forced back) must still be re-read while too young
+    // to trust. See docs/keydb.md#test-inplace-rewrite.
     #[cfg(unix)]
     #[test]
     fn an_in_place_rewrite_under_an_unsettled_stamp_is_still_re_read() {
@@ -1352,18 +1095,9 @@ mod tests {
         );
     }
 
-    /// A corrupt-but-parseable keydb must keep saying so. `ParseStats::log` runs
-    /// inside `KeyDb::parse`, and a cache hit skips `parse` — so once the cache
-    /// landed, the rejected-row summary for a given file was emitted exactly
-    /// ONCE for the entire life of the process, while that same damaged file
-    /// went on being served to every subsequent rip. A long-running daemon
-    /// (autorip) logs it at startup, the operator misses it in the startup noise
-    /// or loses it to log rotation, and there is no second chance for as long as
-    /// the process runs.
-    ///
-    /// Catches the mutation that deletes the `emit_parse_stats` call from the
-    /// cache-HIT arm of `cached_db`: warnings would drop to 1 while the file is
-    /// still served three times.
+    // A corrupt-but-parseable keydb must keep saying so on EVERY hit, not
+    // just the parse that produced the summary (a cache hit skips `parse`).
+    // See docs/keydb.md#test-corrupt-warns.
     #[cfg(unix)]
     #[test]
     fn a_corrupt_keydb_warns_on_every_lookup_not_only_on_the_parse() {
@@ -1401,11 +1135,9 @@ mod tests {
         );
     }
 
-    /// A HEALTHY keydb must stay silent — the re-emission above must not turn
-    /// into a per-lookup log line for every operator with an intact file.
-    ///
-    /// Catches the mutation that makes `emit_parse_stats` unconditional (drops
-    /// `ParseStats::log`'s empty-counts early return).
+    // A HEALTHY keydb must stay silent — the re-emission above must not turn
+    // into a per-lookup log line for every operator with an intact file. See
+    // docs/keydb.md#test-healthy-silent.
     #[cfg(unix)]
     #[test]
     fn a_healthy_keydb_warns_on_no_lookup() {
@@ -1429,16 +1161,9 @@ mod tests {
         );
     }
 
-    /// The cost side of the settle rule, asserted so it is a decision and not an
-    /// accident: a keydb written just now is re-read on every lookup until it
-    /// has been quiet for the granularity. That is the price of never serving a
-    /// same-second replacement, and it is bounded — 2 s after each write of a
-    /// file that is rewritten daily.
-    ///
-    /// Catches the mutation that widens the window to something unbounded (a
-    /// granularity of hours would re-parse 62 MiB on every lookup all day) by
-    /// pairing with `repeated_lookups_parse_the_keydb_once`, which proves the
-    /// entry DOES settle once the mtime is in the past.
+    // The cost side of the settle rule, asserted as a decision: a keydb
+    // written just now is re-read on every lookup until quiet for the
+    // granularity. See docs/keydb.md#test-fresh-not-trusted.
     #[test]
     fn a_freshly_written_keydb_is_not_trusted_from_cache() {
         let dir = scratch("cache-fresh");
@@ -1467,19 +1192,9 @@ mod tests {
         );
     }
 
-    /// `save()` replaces the very file this source reads, so it drops the cached
-    /// parse ITSELF rather than trusting the (size, mtime) stamp: a same-size
-    /// rewrite that lands inside the filesystem's timestamp granularity is the
-    /// one change the stamp cannot see — and it is exactly the change this crate
-    /// performs (the daily-refresh thread re-saving a keydb through the source
-    /// it then reads from).
-    ///
-    /// The test forces that indistinguishable case rather than hoping for it:
-    /// the file is written, stamped to a FIXED mtime, cached, re-saved with
-    /// different keys of the SAME length, and stamped back to the same mtime. To
-    /// the stamp the two files are identical, so only the explicit invalidation
-    /// can produce the new key. Unix-only because pinning an mtime needs
-    /// `touch -t` (no `filetime` dependency for one test).
+    // `save()` replaces the very file this source reads, so it drops the
+    // cached parse ITSELF rather than trusting the (size, mtime) stamp — the
+    // one change the stamp cannot see. See docs/keydb.md#test-save-invalidates.
     #[cfg(unix)]
     #[test]
     fn save_invalidates_the_cache_even_when_the_file_stamp_is_unchanged() {
@@ -1513,16 +1228,9 @@ mod tests {
         );
     }
 
-    // ── A corrupt keydb is a source FAILURE, not "this disc has no key" ──────
-
-    /// `KeyDb::load` fails distinguishably for a keydb that is over the 128 MiB
-    /// cap or not valid UTF-8 — a truncated download, a half-written file, a
-    /// binary blob dropped in by mistake. Both used to collapse into
-    /// `Ok(Vec::new())` with ZERO tracing, i.e. into the benign "no key for this
-    /// disc" the resolver reports as `E7022` — the same looks-like-success
-    /// failure as the seven-hour 502. Only a MISSING file is genuinely benign.
-    ///
-    /// Catches the mutation that restores `Err(_) => Ok(Vec::new())`.
+    // A corrupt keydb is a source FAILURE, not "this disc has no key": both
+    // used to collapse into Ok(Vec::new()) with ZERO tracing. Only a MISSING
+    // file is genuinely benign. See docs/keydb.md#test-corrupt-is-error.
     #[test]
     fn a_corrupt_keydb_is_an_error_not_an_empty_answer() {
         let dir = scratch("corrupt");
@@ -1648,10 +1356,8 @@ mod tests {
         assert!(matches!(src.save(garbage), Err(Error::KeydbInvalid)));
     }
 
-    /// A keydb.cfg that is over the size cap or not valid UTF-8
-    /// (`std::io::ErrorKind::InvalidData`) must surface as a SOURCE FAILURE
-    /// (`Error::KeydbInvalid`), never as a silent empty result — a corrupt
-    /// keydb is not the same thing as "this disc has no key".
+    // A keydb.cfg over the size cap or not valid UTF-8 must surface as a
+    // SOURCE FAILURE (KeydbInvalid), never a silent empty result.
     #[test]
     fn get_unit_keys_reports_invalid_utf8_keydb_as_a_failure() {
         let dir = scratch("load-failure-invalid");
@@ -1720,10 +1426,8 @@ mod tests {
         }
     }
 
-    /// `extract_zip` on a WELL-FORMED zip that carries no `*.cfg`/`*.CFG`
-    /// member must fall through the whole member loop and return
-    /// `Error::KeydbInvalid` — never a parse error, and never silently pick a
-    /// non-keydb member.
+    // `extract_zip` on a WELL-FORMED zip with no `*.cfg`/`*.CFG` member must
+    // return `Error::KeydbInvalid` — never a parse error or a silent pick.
     #[test]
     fn save_rejects_a_valid_zip_with_no_cfg_member() {
         use std::io::Write as _;
@@ -1768,10 +1472,8 @@ mod tests {
         assert_eq!(result.entries, 1);
     }
 
-    /// `write_atomic` when the temp file cannot be CREATED (permission
-    /// denied on an existing, unwritable parent directory) must clean up and
-    /// surface `Error::KeydbWrite`, distinct from the create_dir_all failure
-    /// already pinned by `write_atomic_failure_preserves_prior_keydb`.
+    // `write_atomic` when the temp file cannot be CREATED (unwritable
+    // parent) must clean up and surface `Error::KeydbWrite`.
     #[cfg(unix)]
     #[test]
     fn write_atomic_failure_when_temp_file_cannot_be_created() {

src/keydb_format.rs

@@ -6,18 +6,7 @@
 //! standalone [`std::io::Result`] here instead of `libfreemkv::error::Result`
 //! (so the format crate carries no dependency on libfreemkv's error type).
 //
-// The parser is copied verbatim, so it carries the full KeyDb/DiscEntry API
-// even though this crate's consumer (`keydb.rs`) only exercises a subset
-// (`load`, `find_disc`, `iter_disc_entries`, and the public fields read by
-// `KeydbSource::unit_keys_from` / `KeydbSource::host_certs`). NOTE: an earlier
-// revision of this comment cited a `candidates_from` function — no such
-// function has ever existed in this crate; the real consumer is
-// `KeydbSource::unit_keys_from` (keydb.rs), which reads `unit_keys`, `vuk`,
-// `media_key` AND `vid` (`vid` is load-bearing for the MK+VID derivation the
-// kat_c / kat_d KATs pin — it is NOT unused). The genuinely unused items —
-// `empty`, `find_vuk`, `DiscEntry::title` — are part of the faithful copy and
-// are retained rather than pruned; allow dead_code so the byte-for-byte copy
-// compiles clean without diverging from the libfreemkv original.
+// See docs/keydb-format.md — why the full parser API (incl. dead_code items) is retained.
 #![allow(dead_code)]
 
 use std::collections::HashMap;
@@ -28,10 +17,9 @@ use libfreemkv::aacs::types::{DeviceKey, HostCert};
 /// A keydb per-disc unit key: the CPS-unit number paired with its 16-byte key.
 pub type NumberedUnitKey = (u32, [u8; 16]);
 
-/// Upper bound on the on-disk keydb.cfg size accepted by [`KeyDb::load`].
-/// The public UHD keydb is ~62 MiB and growing; 128 MiB is generous
-/// headroom while still bounding the worst-case allocation from a hostile or
-/// corrupt file.
+// Upper bound on the on-disk keydb.cfg size accepted by `KeyDb::load`. The
+// public UHD keydb is ~62 MiB and growing; 128 MiB bounds worst-case
+// allocation from a hostile or corrupt file.
 const MAX_KEYDB_BYTES: u64 = 128 * 1024 * 1024;
 
 /// Upper bound on parsed disc entries. The real public keydb carries
@@ -64,18 +52,9 @@ pub struct KeyDb {
     pub disc_entries: HashMap<Arc<str>, DiscEntry>,
 }
 
-// Key material must never reach a log, a panic message, or a bug report. Both
-// types below carry raw AACS key bytes, and both are public API, so a
-// `#[derive(Debug)]` on either is a leak of the crown jewels: `{:?}` on a
-// loaded `KeyDb` printed every processing key plus every disc's Media Key,
-// VID, VUK and Unit Keys. libfreemkv's equivalent types (`aacs::types`:
-// `DeviceKey`, `HostCert`, `Vid`, `MediaKey`, `Vuk`, `ProcessingKey`,
-// `UnitKey`, its own `DiscEntry`) all hand-write a redacting `Debug` for
-// exactly this reason; this crate's copy of the parser dropped that
-// convention when it was relocated. These restore it, field-for-field in the
-// same style (`<redacted>` for key bytes, a `_len` for key-bearing
-// collections). Pinned by `keydb_debug_is_redacted` /
-// `disc_entry_debug_is_redacted`.
+// Key material must never leak via `{:?}`; both types below carry raw AACS
+// key bytes. See docs/keydb-format.md for why these hand-write a redacting
+// `Debug` instead of deriving it. Pinned by `keydb_debug_is_redacted` / `disc_entry_debug_is_redacted`.
 
 impl std::fmt::Debug for KeyDb {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
@@ -149,25 +128,17 @@ pub struct DiscEntry {
     pub is_uhd: bool,
 }
 
-/// Parse a hex string like "0xABCD..." into bytes.
-///
-/// Operates on bytes, not `&str` char boundaries: the keydb is
-/// third-party content, so a non-ASCII scalar (e.g. a 4-byte UTF-8
-/// codepoint) must not panic on a mid-codepoint slice. Any non-hex
-/// byte yields `None`.
+// Parse a hex string like "0xABCD..." into bytes. Operates on bytes, not
+// `&str` char boundaries: the keydb is third-party content, so a non-ASCII
+// scalar must not panic on a mid-codepoint slice. Non-hex byte -> `None`.
 pub(crate) fn parse_hex(s: &str) -> Option<Vec<u8>> {
     // The one workspace hex parser (strips an optional 0x/0X, byte-based).
     libfreemkv::hex::parse_hex_bytes(s)
 }
 
-/// Read the run of consecutive ASCII decimal digits immediately following the
-/// first occurrence of `marker` in `text`, parsing them with `parse`.
-///
-/// Operates on raw bytes so untrusted third-party comment text (which may carry
-/// non-ASCII scalars) never panics on a char boundary. Returns `None` when the
-/// marker is absent or no digits follow it. Whitespace between the marker and
-/// the digits is skipped, so this serves both `MKBv<N>` (no gap) and
-/// `VolumeSize: <N>` (a space before the number).
+// Read digits right after the first `marker` in `text`. Byte-based so
+// untrusted comment text never panics on a char boundary; skips whitespace
+// before the digits, serving both `MKBv<N>` and `VolumeSize: <N>`.
 fn parse_digits_after<T: std::str::FromStr>(text: &str, marker: &str) -> Option<T> {
     let bytes = text.as_bytes();
     let start = text.find(marker)? + marker.len();
@@ -205,20 +176,9 @@ pub(crate) fn parse_hex20(s: &str) -> Option<[u8; 20]> {
     libfreemkv::hex::parse_hex_fixed::<20>(s)
 }
 
-/// What [`KeyDb::parse`] threw away, by reason.
-///
-/// The parser is fed a THIRD-PARTY file: a mirror can serve a truncated or
-/// byte-shifted download that is still valid UTF-8, still contains enough
-/// recognisable rows to pass `KeydbSource::save`'s "at least one entry" check,
-/// and is then atomically committed as the new keydb — after which every disc
-/// past the corruption point silently resolves nothing. Before this counter,
-/// each malformed `| DK` / `| PK` / `| HC` / `| HC2` / `0x…` row was dropped by
-/// an `if let Some(..) = .. { }` with no `else`, no counter and no log, so that
-/// file was indistinguishable from a healthy one at every layer above.
-///
-/// The counts are logged ONCE per parse (a per-line log would be 181k lines on
-/// a real keydb, which is why the rate limit is "one summary", never "hide the
-/// count").
+// What `KeyDb::parse` threw away, by reason. See docs/keydb-format.md — why
+// this counter exists (silent corruption on a third-party keydb) and why
+// it's logged once per parse rather than per-line.
 #[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
 pub(crate) struct ParseStats {
     /// `| DK` rows that parsed as neither a positioned nor an orphan device key.
@@ -251,16 +211,9 @@ impl ParseStats {
             + self.disc_over_cap
     }
 
-    /// Emit the one summary line, if there is anything to say. Returns whether
-    /// a line was emitted — the only way a test can observe the warning without
-    /// a `tracing` subscriber dependency, and the seam
-    /// [`crate::KeydbSource::cached_db`] counts to prove the warning survives
-    /// the cache.
-    ///
-    /// `pub(crate)` because [`crate::KeydbSource`] re-emits it on a CACHE HIT
-    /// too: the cache short-circuits `parse`, so without that the corruption
-    /// summary for a given file fired once per process and a long-running daemon
-    /// went on serving the damaged keydb in silence forever after.
+    // Emit the one summary line, if there is anything to say; returns whether
+    // it fired (tests use this). `pub(crate)` so `KeydbSource` can re-emit it
+    // on a cache hit too, keeping the warning from going silent forever.
     pub(crate) fn log(&self) -> bool {
         if self.rejected() == 0 && self.disc_duplicate == 0 {
             return false;
@@ -280,13 +233,9 @@ impl ParseStats {
     }
 }
 
-/// True when `line` opens a per-disc row. The `0x` prefix is matched
-/// CASE-INSENSITIVELY: `parse_hex` in this same file deliberately tolerates
-/// `0x` and `0X` (and is tested for it), so a case-sensitive `starts_with("0x")`
-/// here would drop an uppercase `0X…` disc row on the floor with no diagnostic
-/// while every other hex field in the format accepted it. Shared with
-/// `KeydbSource::save`, whose entry count must stay in EXACT lockstep with what
-/// this parser accepts.
+// True when `line` opens a per-disc row. Matches `0x`/`0X` case-insensitively
+// (like `parse_hex`) so an uppercase row isn't silently dropped. Shared with
+// `KeydbSource::save`, whose entry count must match what this parser accepts.
 pub(crate) fn is_disc_entry_line(line: &str) -> bool {
     (line.starts_with("0x") || line.starts_with("0X")) && line.contains(" = ")
 }
@@ -333,14 +282,9 @@ impl KeyDb {
                 continue;
             }
 
-            // Device Key.
-            // Two shapes are accepted:
-            //   1. Positioned DK: `| DK | DEVICE_KEY 0x... | DEVICE_NODE 0x... | KEY_UV 0x... | KEY_U_MASK_SHIFT 0x...`
-            //      → loaded into `device_keys` (deterministic tree walk via `calc_pk_from_dk`).
-            //   2. Orphan DK: `| DK | DEVICE_KEY 0x...` with no position fields.
-            //      → loaded into `processing_keys` (brute walker / terminal validation).
-            // Per AACS spec a "PK" IS a DK at terminal position, so both row types
-            // are DKs in the unified model; only the metadata differs.
+            // Device Key: positioned rows -> `device_keys` (tree walk); orphan
+            // rows (no position fields) -> `processing_keys` (brute walker).
+            // Per AACS a "PK" IS a DK at terminal position, so both are DKs here.
             if line.starts_with("| DK") {
                 if let Some(dk) = Self::parse_device_key(line) {
                     db.device_keys.push(dk);
@@ -362,23 +306,17 @@ impl KeyDb {
                 continue;
             }
 
-            // Host Certificate (AACS 2.0).
-            //
-            // An HC2 row normally augments the preceding HC (AACS 1.0) row.
-            // KEYDB line ordering is third-party, so an HC2 row may appear
-            // before any HC row; rather than silently dropping the AACS 2.0
-            // credentials, carry them on a fresh HostCert with an empty v1
-            // cert (the v1 private_key/certificate stay zero/empty and are
-            // ignored by the v1 handshake, which guards on cert length).
+            // Host Certificate (AACS 2.0). Normally augments the preceding HC
+            // (AACS 1.0) row, but an HC2 row may appear first (third-party line
+            // ordering); then it's carried on a fresh HostCert with an empty v1 cert.
             if line.starts_with("| HC2") {
                 if let Some((pk, cert, revoked_at_mkb)) = Self::parse_host_cert_v2(line) {
                     if let Some(hc) = db.host_certs.last_mut() {
                         hc.cert.private_key_v2 = Some(pk);
                         hc.cert.certificate_v2 = Some(cert);
-                        // The `; Revoked in MKBv<N>` annotation can live on the
-                        // HC2 line rather than the preceding HC line; carry it
-                        // onto the combined cert if the HC line had none, so the
-                        // revocation isn't silently dropped.
+                        // The revocation annotation can live on the HC2 line
+                        // instead of the HC line; carry it onto the combined
+                        // cert if the HC line had none, so it isn't lost.
                         if hc.revoked_at_mkb.is_none() {
                             hc.revoked_at_mkb = revoked_at_mkb;
                         }
@@ -452,30 +390,16 @@ impl KeyDb {
         Ok(db)
     }
 
-    /// [`Self::load`] from an ALREADY-OPEN file, without the log.
-    ///
-    /// Two callers need this seam, both in [`crate::KeydbSource`]:
-    ///
-    /// * The identity stamp and the bytes must come from the SAME descriptor.
-    ///   Stat-the-path-then-load-the-path is two independent resolutions of one
-    ///   name, so an atomic rename landing between them stored a stamp
-    ///   describing one file next to the parsed contents of another — and the
-    ///   cache then served that pairing until the stamp changed again. `fstat`
-    ///   on the open handle cannot be pointed at a different inode.
-    /// * The rejection counts have to be RETAINED, not just logged, so a cache
-    ///   hit can re-emit them.
+    // `Self::load` from an ALREADY-OPEN file, without the log. See
+    // docs/keydb-format.md — why the same descriptor must supply both the
+    // identity stamp and the bytes, and why rejection counts are retained here.
     pub(crate) fn load_counted(
         f: std::fs::File,
         path: &std::path::Path,
     ) -> std::io::Result<(Self, ParseStats)> {
-        // Read through a hard `take` cap rather than trusting a stat. A
-        // `metadata` pre-check is only advisory: it is skipped entirely when the
-        // stat fails, and it reports len 0 for a FIFO or character device, so
-        // `--keydb /dev/zero` passed the guard and then read forever (NUL is
-        // valid UTF-8, so it did not even fail the decode — it just grew until
-        // the process was OOM-killed). Reading one byte past the cap accepts an
-        // exactly-at-cap file and rejects anything larger, matching
-        // `read_capped_to_string` on the download path.
+        // Read through a hard `take` cap rather than trusting a stat: a
+        // `metadata` pre-check reports len 0 for a FIFO/char device (e.g. would
+        // read `/dev/zero` forever). One byte past cap matches `read_capped_to_string`.
         use std::io::Read;
         let mut buf = Vec::new();
         f.take(MAX_KEYDB_BYTES + 1).read_to_end(&mut buf)?;
@@ -504,10 +428,9 @@ impl KeyDb {
             .to_lowercase()
             .trim_start_matches("0x")
             .to_string();
-        // Try with 0x prefix and without. parse_disc_entry only stores keys
-        // from lines that began with "0x", so every stored key carries the
-        // prefix and the no-prefix fallback is currently unreachable; it is
-        // retained as a defensive match for the prefix-agnostic lookup contract.
+        // Try with 0x prefix and without; every stored key currently carries
+        // the prefix, so the no-prefix fallback is a defensive match kept for
+        // the prefix-agnostic lookup contract.
         self.disc_entries
             .get(format!("0x{hash}").as_str())
             .or_else(|| self.disc_entries.get(hash.as_str()))
@@ -521,9 +444,8 @@ impl KeyDb {
             .to_lowercase()
             .trim_start_matches("0x")
             .to_string();
-        // The no-prefix fallback below is currently unreachable (every stored
-        // key carries the "0x" prefix, see find_vuk); kept as a defensive
-        // match for the prefix-agnostic lookup contract.
+        // No-prefix fallback below (every stored key carries "0x", see
+        // find_vuk); kept as a defensive match for the lookup contract.
         self.disc_entries
             .get(format!("0x{hash}").as_str())
             .or_else(|| self.disc_entries.get(hash.as_str()))
@@ -586,12 +508,8 @@ impl KeyDb {
     /// every field (see `round_trips_through_parse`). Used by the key-import tool
     /// to export a complete keydb.cfg (keys + host certs + VIDs).
     ///
-    /// The trailing `; <comment>` (MKB version / volume size / UHD) is emitted
-    /// ONLY after a `U` (unit-keys) field — that is the one place the parser
-    /// splits the value on `;`. Gluing a comment onto an `M`/`I`/`V` value would
-    /// make `parse_hex16` reject the whole field, so a comment-bearing entry that
-    /// has no unit keys drops its comment (keys always survive; the metadata is a
-    /// derivable hint). Real per-disc rows that carry metadata also carry keys.
+    /// See docs/keydb-format.md for why the trailing `; <comment>` is only
+    /// emitted after a `U` (unit-keys) field.
     pub fn to_keydb_cfg(&self) -> String {
         fn hx(b: &[u8]) -> String {
             use std::fmt::Write;
@@ -739,12 +657,9 @@ impl KeyDb {
         None
     }
 
-    /// Parse an orphan DK row: a `| DK |` line carrying only the
-    /// `DEVICE_KEY` field (no position metadata). The key is then
-    /// treated like a terminal/unpositioned label by the resolver
-    /// (Path 2's brute walker). Returns `None` if the line carries
-    /// any position field — those are positioned DKs and parsed by
-    /// [`Self::parse_device_key`] instead.
+    // Parse an orphan DK row: `| DK |` with only `DEVICE_KEY` (no position
+    // metadata), treated as terminal/unpositioned by the brute walker. `None`
+    // if a position field is present — those are positioned DKs (`parse_device_key`).
     fn parse_orphan_dk(line: &str) -> Option<[u8; 16]> {
         if line.contains("DEVICE_NODE")
             || line.contains("KEY_UV")
@@ -838,26 +753,16 @@ impl KeyDb {
         let (hash_part, rest) = line.split_once(" = ")?;
         let disc_hash: Arc<str> = Arc::from(hash_part.trim().to_lowercase());
 
-        // The trailing `;` comment (e.g.
-        // "; MKBv76/BEE/FindVUK 1.74 - VolumeSize: 81309007872 (UHD)") carries
-        // metadata the key fields don't. Capture everything after the FIRST ';'
-        // on the line, then extract MKB version / volume size / UHD flag.
+        // The trailing `;` comment (e.g. "; MKBv76/BEE/FindVUK 1.74 -
+        // VolumeSize: 81309007872 (UHD)") carries metadata the key fields
+        // don't; capture everything after the FIRST ';' on the line.
         let comment = line.split_once(';').map(|(_, c)| c).unwrap_or("");
-        // MKBv token: literal "MKBv" immediately followed by decimal digits.
         let mkb_version: Option<u32> = parse_digits_after(comment, "MKBv");
-        // VolumeSize token: "VolumeSize:" then whitespace then a byte count.
         let volume_size: Option<u64> = parse_digits_after(comment, "VolumeSize:");
-        // UHD flag: literal "(UHD)" anywhere in the comment.
         let is_uhd = comment.contains("(UHD)");
 
-        // Title = everything between `= ` and the first ` | ` field (or the
-        // trailing `;` comment), kept VERBATIM (trimmed). This is a FAITHFUL copy
-        // of the keydb title, so it must round-trip exactly: a previous version
-        // extracted a `(...)` substring as a "display title", but that TRUNCATED
-        // real titles that legitimately contain parentheses ("Lawrence of Arabia
-        // (Restored Version) – Disc 2 …" → "Restored Version") and broke
-        // serialize→parse idempotence. Display prettification, if wanted, belongs
-        // in the title-display layer, NOT this codec.
+        // Title kept VERBATIM (trimmed), a faithful copy for exact round-trip.
+        // See docs/keydb-format.md — why display prettification does NOT belong here.
         let before_fields = rest.split(" | ").next().unwrap_or("");
         // A title-only entry (no key fields) carries its `;` comment on the same
         // chunk — strip it so the comment doesn't leak into the title.
@@ -875,19 +780,15 @@ impl KeyDb {
         let mut unit_keys = Vec::new();
 
         let parts: Vec<&str> = rest.split(" | ").collect();
-        // Field scan starts at index 1: `parts[0]` is ALWAYS the title chunk and
-        // must be excluded, otherwise a disc whose title happens to be a field tag
-        // letter ("M", "I", "V", "U", "D") — e.g. `= M | M | 0x…` — would have the
-        // title eaten as a tag and shadow the real field. (Broke round-trip.)
+        // Field scan starts at index 1: `parts[0]` is ALWAYS the title chunk;
+        // excluding it stops a title matching a field-tag letter ("M"/"I"/"V"/
+        // "U"/"D") from being eaten as a tag and shadowing the real field.
         let mut i = 1;
         while i < parts.len() {
             match parts[i].trim() {
-                // M/I/V strip a trailing `; comment` before hex-parsing, exactly
-                // as the U arm below does: when the disc row's LAST field is one
-                // of these (no U field after it), a real keydb.cfg glues the
-                // `; MKBv…` comment onto that value, and parse_hex16 of
-                // "0x… ; MKBv64" fails — silently dropping a key the file
-                // actually holds. A hex16 never contains ';', so the split is safe.
+                // M/I/V strip a trailing `; comment` before hex-parsing (like the
+                // U arm below): when one of these is the row's LAST field, the
+                // comment gets glued on and parse_hex16 would otherwise fail.
                 "M" => {
                     if i + 1 < parts.len() {
                         media_key =
@@ -944,15 +845,9 @@ impl KeyDb {
 mod tests {
     use super::*;
 
-    /// `load` must enforce `MAX_KEYDB_BYTES` through a bounded read, not a stat.
-    ///
-    /// Regression: the cap used to be a `std::fs::metadata` pre-check followed by
-    /// an unbounded `read_to_string`. That is not a cap — it is skipped when the
-    /// stat fails, and a FIFO or character device reports len 0, so the guard
-    /// passed and the read never terminated. NUL bytes are valid UTF-8, so
-    /// `--keydb /dev/zero` did not even fail the decode; it allocated until the
-    /// process died. Asserts both edges of the boundary so a future cap change
-    /// cannot quietly re-introduce an off-by-one.
+    // `load` must enforce `MAX_KEYDB_BYTES` through a bounded read, not a stat
+    // (a prior metadata-based cap was skippable via a FIFO/char device
+    // reporting len 0). Asserts both edges of the boundary.
     #[test]
     fn load_enforces_the_size_cap_at_both_edges() {
         let dir = std::env::temp_dir()
@@ -981,11 +876,9 @@ mod tests {
         std::fs::remove_dir_all(&dir).ok();
     }
 
-    /// `to_keydb_cfg` is the exact inverse of `parse`: parse a known line set,
-    /// serialize it, re-parse, and every field survives — device key, processing
-    /// key, host cert (priv key + cert + revocation), and the per-disc M/I(vid)/V/U
-    /// keys plus the MKBv/UHD comment metadata. Both sides go through `parse`, so
-    /// internal key forms (e.g. the `0x`-prefixed disc-hash) match by construction.
+    // `to_keydb_cfg` is the exact inverse of `parse`: parse a known line set,
+    // serialize, re-parse, and every field survives (keys, host cert, and the
+    // MKBv/UHD comment metadata).
     #[test]
     fn to_keydb_cfg_round_trips_through_parse() {
         let h = |b: u8, n: usize| format!("{b:02x}").repeat(n);
@@ -1053,23 +946,9 @@ mod tests {
         assert_eq!(b.host_certs[0].revoked_at_mkb, Some(72));
     }
 
-    /// REAL-DATA IDEMPOTENCE — the "load + serialize back-to-back" check.
-    ///
-    /// Parse the full keydb → serialize (S1) → parse S1 → serialize again (S2).
-    /// S1 MUST equal S2 byte-for-byte. This is the right invariant: a raw
-    /// keydb.cfg has formatting variance (whitespace, optional fields, comment
-    /// style) that our CANONICAL serializer normalizes, so `text == to_keydb_cfg`
-    /// is NOT expected — but once normalized, a re-load+re-serialize must be
-    /// stable. Idempotence here proves `parse` is lossless on its own output and
-    /// `to_keydb_cfg` is deterministic. Also asserts no rows are dropped.
-    /// REQUIRES a real keydb; `#[ignore]` by default. Run with:
-    /// `KEYDB_PATH=/path/to/keydb.cfg cargo test --release -- --ignored`.
-    ///
-    /// It used to carry no `#[ignore]` and `return` early when `KEYDB_PATH` was
-    /// unset — so CI ran it, asserted NOTHING about 181k-entry behaviour, and
-    /// reported green. A skip has to be visible in the test output to mean
-    /// anything, and an ignored test that is RUN without its fixture must fail
-    /// loudly rather than silently pass.
+    // REAL-DATA IDEMPOTENCE: parse → serialize (S1) → parse → serialize (S2);
+    // S1 must equal S2 byte-for-byte. REQUIRES a real keydb (KEYDB_PATH env);
+    // `#[ignore]` by default. See docs/keydb-format.md.
     #[test]
     #[ignore = "needs a real keydb.cfg: KEYDB_PATH=<path> cargo test -- --ignored"]
     fn to_keydb_cfg_is_idempotent_on_real_keydb() {
@@ -1095,12 +974,9 @@ mod tests {
         assert_eq!(db1.host_certs.len(), db2.host_certs.len(), "HC drift");
     }
 
-    /// REAL-SCALE parser health: a genuine published keydb must parse with ZERO
-    /// rejected rows. This is the assertion the rejection counter exists to
-    /// make possible — a corrupt or byte-shifted mirror download differs from a
-    /// healthy one precisely here, and nothing above the parser can tell them
-    /// apart. `#[ignore]` by default; run with:
-    /// `KEYDB_PATH=/path/to/keydb.cfg cargo test --release -- --ignored`.
+    // REAL-SCALE parser health: a genuine published keydb must parse with ZERO
+    // rejected rows — the assertion the rejection counter exists to make
+    // possible. `#[ignore]` by default; run with KEYDB_PATH set.
     #[test]
     #[ignore = "needs a real keydb.cfg: KEYDB_PATH=<path> cargo test -- --ignored"]
     fn a_real_keydb_parses_with_no_rejected_rows() {
@@ -1129,13 +1005,9 @@ mod tests {
         if path.exists() { Some(path) } else { None }
     }
 
-    /// The real keydb for the `#[ignore]`d real-scale tests, or a LOUD failure.
-    ///
-    /// The old pattern (`match keydb_path() { Some(p) => p, None => return }`)
-    /// on a non-ignored test is the worst of both worlds: CI ran the test,
-    /// asserted nothing, and reported success. These tests are `#[ignore]`d, so
-    /// running one at all is an explicit request — and an explicit request with
-    /// no fixture must fail, not quietly pass.
+    // The real keydb for the `#[ignore]`d real-scale tests, or a LOUD failure.
+    // See docs/keydb-format.md — why silent `return` on a missing fixture is
+    // wrong here.
     fn real_keydb_path() -> std::path::PathBuf {
         keydb_path().expect(
             "KEYDB_PATH must point at a real keydb.cfg; run: \
@@ -1161,11 +1033,9 @@ mod tests {
 
     #[test]
     fn disc_entry_strips_a_comment_glued_to_a_trailing_v_field() {
-        // A real keydb.cfg row whose LAST field is V (no U after it) with the
-        // `; MKBv…` comment glued straight onto the hex value. The U arm already
-        // strips such a comment; before M/I/V did too, parse_hex16 saw
-        // "0x00… ; MKBv64" and silently dropped the VUK — the disc then reported
-        // no key despite the file holding one.
+        // A row whose LAST field is V (no U after it) with the `; MKBv…`
+        // comment glued onto the hex value; before M/I/V stripped it too,
+        // parse_hex16 silently dropped the VUK.
         let z40 = "00".repeat(20);
         let z32 = "00".repeat(16);
         let line =
@@ -1178,10 +1048,9 @@ mod tests {
         );
     }
 
-    // NOTE: key fields below use obvious repeated-byte / zero placeholders
-    // (0x01.., 0x02.., 0x03.., 0x00..). NEVER put real — or real-looking — host,
-    // device, or processing key material in code; these tests exercise the
-    // parser's field-splitting only, not any genuine key.
+    // NOTE: key fields below use obvious repeated-byte / zero placeholders.
+    // NEVER put real — or real-looking — key material in code; these tests
+    // exercise the parser's field-splitting only.
 
     #[test]
     fn test_parse_device_key() {
@@ -1193,10 +1062,8 @@ mod tests {
 
     #[test]
     fn test_orphan_dk_row_loads_into_processing_keys() {
-        // `| DK |` row without position fields = an orphan DK. Per the
-        // unified model the resolver treats it like a terminal/PK
-        // candidate: it lands in `processing_keys` and the brute walker
-        // handles it.
+        // `| DK |` row without position fields = an orphan DK; it lands in
+        // `processing_keys` and the brute walker handles it.
         let cfg = r#"
 | DK | DEVICE_KEY 0x01010101010101010101010101010101 ; orphan, no position fields
 | DK | DEVICE_KEY 0x02020202020202020202020202020202 | DEVICE_NODE 0x0800 | KEY_UV 0x00000400 | KEY_U_MASK_SHIFT 0x17 ; positioned MKBv01-MKBv48
@@ -1340,18 +1207,9 @@ mod tests {
         );
     }
 
-    /// `load_counted` must read the HANDLE it was given, never re-resolve the
-    /// path. `KeydbSource::cached_db` stamps the open file with `fstat` and then
-    /// parses from that same descriptor precisely so the recorded identity and
-    /// the parsed bytes cannot describe two different files; the moment this
-    /// function re-opens the path, that guarantee is gone and an atomic rename
-    /// landing in between stores one file's stamp beside another file's keys —
-    /// a mismatch the cache then serves as if it were valid.
-    ///
-    /// The race itself cannot be scheduled deterministically in-process, so the
-    /// SEAM is what is pinned: with the path already replaced, the handle's
-    /// contents must still win. Catches the mutation that restores an internal
-    /// `File::open(path)`.
+    // `load_counted` must read the HANDLE it was given, never re-resolve the
+    // path (identity stamp + bytes must match). See docs/keydb-format.md.
+    // With the path already replaced, the handle's contents must still win.
     #[cfg(unix)]
     #[test]
     fn load_counted_reads_the_handle_not_the_path() {
@@ -1401,14 +1259,9 @@ mod tests {
             !rendered.contains("5a") && !rendered.contains("5A"),
             "{what} Debug leaked a hex key byte: {rendered}"
         );
-        // A derived `Debug` over `[u8; 16]` / `Vec<[u8; 16]>` renders a byte
-        // array — `[90, 90, ...]`. A redacting impl never emits one. Matched as
-        // "a `[` immediately followed by a digit" rather than "any `[`", because
-        // callers legitimately render a `Vec` OF redacting structs
-        // (`{:?}` on `KeyDb::host_certs`), whose own `[` opens a list of
-        // `Name { .. }` — never a byte. Every byte-array rendering, decimal or
-        // nested, still starts `[<digit>` (or `[[<digit>`), and the two byte
-        // checks above catch the payload independently.
+        // A derived `Debug` over a byte array renders `[90, 90, ...]`; matched
+        // as "`[` immediately followed by a digit" rather than "any `[`",
+        // since callers legitimately render a `Vec` of redacting structs.
         let bytes = rendered.as_bytes();
         assert!(
             !rendered
@@ -1418,10 +1271,8 @@ mod tests {
         );
     }
 
-    /// `{:?}` on a `DiscEntry` must not print the Media Key, VID, VUK or any
-    /// Unit Key. Catches the mutation that restores `#[derive(Debug)]`: the
-    /// derive renders `media_key: Some([90, 90, ...])`, so a single `{:?}` in a
-    /// caller's log or panic message published a disc's whole key set.
+    // `{:?}` on a `DiscEntry` must not print the Media Key, VID, VUK or any
+    // Unit Key. Catches the mutation that restores `#[derive(Debug)]`.
     #[test]
     fn disc_entry_debug_is_redacted() {
         let e = DiscEntry {
@@ -1489,25 +1340,9 @@ mod tests {
         );
     }
 
-    /// The SIBLING type. `KeyDb`'s hand-written `Debug` prints
-    /// `host_certs_len`, so it protects the whole-db rendering and nothing else
-    /// — [`KeyDb::host_certs`] is a public field, and `{:?}` on ONE element of
-    /// it walks a completely different code path: `KeydbHostCert`'s derive,
-    /// then `HostCert`'s impl. That path carries the AACS host PRIVATE key, the
-    /// only key material in the crate that authenticates US rather than opening
-    /// a disc.
-    ///
-    /// It is safe today only because `libfreemkv::HostCert` hand-writes a
-    /// redacting `Debug` (`aacs::types`, `private_key: "<redacted>"` +
-    /// `certificate_len`) — an EXTERNAL guarantee this crate silently inherits
-    /// through the derive, with nothing on this side pinning it. This test pins
-    /// it: it catches a libfreemkv upgrade that swaps that impl for
-    /// `#[derive(Debug)]`, which would leak the host private key out of this
-    /// crate's public API with no diff in this repository at all.
-    ///
-    /// Rendered three ways because callers reach the cert three ways: the whole
-    /// `Vec` (`?db.host_certs`), one element, and the inner `HostCert` a caller
-    /// gets back from `KeyDb::host_certs(mkb)`.
+    // The SIBLING type: `KeyDb`'s hand-written `Debug` protects the whole-db
+    // rendering only; `{:?}` on one `KeydbHostCert` element walks a different
+    // path carrying the AACS host PRIVATE key. See docs/keydb-format.md.
     #[test]
     fn host_cert_debug_is_redacted_including_the_sibling_wrapper() {
         let hc = KeydbHostCert {
@@ -1553,14 +1388,9 @@ mod tests {
         assert_no_key_bytes("Vec<DeviceKey>", &format!("{:?}", vec![dk]));
     }
 
-    // ── Parser rejection accounting ────────────────────────────────────────
-
-    /// Every malformed row the parser drops must be COUNTED, per reason. The
-    /// mutation this catches: restoring the bare `if let Some(x) = .. { }` with
-    /// no `else`, which is how a byte-shifted mirror download passed `save()`'s
-    /// "at least one entry" validation, was atomically committed as the new
-    /// keydb, and then resolved nothing for every disc past the damage — with
-    /// not one line of log to notice it by.
+    // ── Parser rejection accounting ── every malformed row must be COUNTED,
+    // per reason. Catches restoring a bare `if let Some(x) = .. { }` with no
+    // `else`, which let a byte-shifted download resolve nothing with zero log.
     #[test]
     fn parse_counts_every_rejected_row_by_reason() {
         let good_pk = format!("| PK | 0x{}", "ab".repeat(16));
@@ -1629,26 +1459,18 @@ mod tests {
     /// dropped by the cap.
     #[test]
     fn parse_counts_unparseable_disc_row() {
-        // ` = ` present (so it IS a disc row) but `split_once(" = ")` succeeds
-        // and the entry parses — to get a genuine rejection the row must fail
-        // `parse_disc_entry`, which only returns None without a " = ". Nothing
-        // in today's grammar can be a disc row AND fail, so the counter is
-        // proven by construction elsewhere; assert the healthy case is zero so
-        // a future parser change that starts rejecting rows cannot do it
-        // silently.
+        // Nothing in today's grammar can be a disc row AND fail
+        // `parse_disc_entry`, so assert the healthy case is zero, so a future
+        // parser change that starts rejecting rows cannot do it silently.
         let k = "01".repeat(16);
         let (_, stats) = KeyDb::parse_counted(&format!("0xAAAA = T | U | 1-0x{k}\n"));
         assert_eq!(stats.rejected(), 0, "a healthy keydb rejects nothing");
         assert_eq!(stats.disc_duplicate, 0);
     }
 
-    // ── `0X` disc rows / the save() lockstep ────────────────────────────────
-
-    /// The disc-row test is `starts_with("0x")` — CASE-SENSITIVE, while
-    /// `parse_hex` in this same file deliberately accepts `0x` AND `0X` (and is
-    /// tested for it). An uppercase `0X…` row was therefore dropped with no
-    /// diagnostic at all: not parsed, not counted, not logged. Catches the
-    /// mutation that removes the `0X` arm of `is_disc_entry_line`.
+    // ── `0X` disc rows / the save() lockstep ── an uppercase row was once
+    // dropped with no diagnostic. Catches the mutation that removes the `0X`
+    // arm of `is_disc_entry_line`.
     #[test]
     fn disc_row_with_uppercase_0x_prefix_is_parsed() {
         let v = "33".repeat(16);
@@ -1666,12 +1488,9 @@ mod tests {
         assert!(!is_disc_entry_line("0XAABBCC"));
     }
 
-    // ── One allocation per disc hash, not two ──────────────────────────────
-
-    /// The map key and the entry's own `disc_hash` must be the SAME allocation.
-    /// They used to be two `String`s (`insert(entry.disc_hash.clone(), entry)`),
-    /// ~13 MB of duplicate hash text across the real keydb's 181k entries — now
-    /// resident for the process lifetime, since `KeydbSource` caches the parse.
+    // ── One allocation per disc hash, not two ── map key and entry's own
+    // `disc_hash` must be the SAME allocation (used to be a clone, ~13 MB of
+    // duplicate text across the real keydb's 181k entries).
     #[test]
     fn disc_hash_is_stored_once_not_twice() {
         let v = "33".repeat(16);
@@ -1683,11 +1502,7 @@ mod tests {
         );
     }
 
-    // ════════════════════════════════════════════════════════════════════
-    // Hardening additions
-    // ════════════════════════════════════════════════════════════════════
-
-    // ── parse_hex / parse_hex16 / parse_hex20 ──────────────────────────────
+    // ══ Hardening additions ══ parse_hex / parse_hex16 / parse_hex20 ══
 
     #[test]
     fn parse_hex_strips_lower_and_upper_prefixes() {
@@ -1870,11 +1685,9 @@ mod tests {
 
     #[test]
     fn parse_device_key_requires_all_four_fields() {
-        // Missing KEY_U_MASK_SHIFT → parse_device_key returns None; with no
-        // position fields at all it would be an orphan DK instead. Here the
-        // line has DEVICE_NODE + KEY_UV but no shift → neither parser accepts
-        // it as a positioned DK, and parse_orphan_dk rejects it (has position
-        // fields), so nothing is loaded.
+        // Missing KEY_U_MASK_SHIFT → parse_device_key returns None; it has
+        // DEVICE_NODE + KEY_UV so parse_orphan_dk also rejects it (has
+        // position fields), so nothing is loaded.
         let line = "| DK | DEVICE_KEY 0x00000000000000000000000000000000 | DEVICE_NODE 0x0800 | KEY_UV 0x00000400";
         assert!(KeyDb::parse_device_key(line).is_none());
         let db = KeyDb::parse(line);
@@ -1884,10 +1697,9 @@ mod tests {
 
     #[test]
     fn parse_device_key_accepts_uppercase_0x_prefix() {
-        // Regression: node/uv/shift parsed via a case-sensitive
-        // `trim_start_matches("0x")`, so an uppercase `0X` prefix was not
-        // stripped, `from_str_radix` failed, and the WHOLE device key was
-        // silently dropped. All four fields must parse regardless of prefix case.
+        // Regression: a case-sensitive `trim_start_matches("0x")` left an
+        // uppercase `0X` prefix unstripped, so `from_str_radix` failed and
+        // the WHOLE device key was silently dropped.
         let line = "| DK | DEVICE_KEY 0x000102030405060708090A0B0C0D0E0F \
                     | DEVICE_NODE 0X0001 | KEY_UV 0X00000002 | KEY_U_MASK_SHIFT 0X03";
         let dk = KeyDb::parse_device_key(line).expect("uppercase 0X prefix must parse");
@@ -2116,10 +1928,9 @@ mod tests {
         assert_eq!(hashes, vec!["0xaaaa", "0xbbbb"]);
     }
 
-    /// `to_keydb_cfg` emits the sibling `| HC2 |` line for a host cert that
-    /// carries AACS 2.0 credentials, and a `parse` of the output recovers
-    /// them. Also: an empty title serializes as the literal `Unknown` (the
-    /// inverse of the parser's own convention).
+    // `to_keydb_cfg` emits the sibling `| HC2 |` line for AACS 2.0
+    // credentials, recovered by re-`parse`; also an empty title serializes
+    // as the literal `Unknown`.
     #[test]
     fn to_keydb_cfg_emits_hc2_and_unknown_for_empty_title() {
         let h = |b: u8, n: usize| format!("{b:02x}").repeat(n);
@@ -2160,10 +1971,9 @@ mod tests {
         );
     }
 
-    /// `to_keydb_cfg`'s comment clause is written piecewise (MKBv, then
-    /// VolumeSize, then UHD) — exercise each optional sub-field independently
-    /// (not just all-three-true) so every combination of the trailing
-    /// `; comment` assembly is proven, not just the "everything present" case.
+    // `to_keydb_cfg`'s comment clause is written piecewise (MKBv, VolumeSize,
+    // UHD); exercise each optional sub-field independently, not just
+    // all-three-true.
     #[test]
     fn to_keydb_cfg_comment_fields_are_independent() {
         let k = "0a".repeat(16);
@@ -2193,15 +2003,8 @@ mod tests {
         assert_eq!(eb2.volume_size, None);
     }
 
-    // ════════════════════════════════════════════════════════════════════
-    // KEYDB-parser integration tests relocated from libfreemkv.
-    //
-    // These exercise the parser (KeyDb::load) end-to-end against a real
-    // keydb.cfg and feed its material into libfreemkv's AACS crypto
-    // (derive_vuk, then decrypt_unit + is_clean_ts). They live here now that the
-    // parser lives here. All are KEYDB_PATH-env-gated and no-op in CI when
-    // the env is unset; they must still COMPILE.
-    // ════════════════════════════════════════════════════════════════════
+    // KEYDB-parser integration tests relocated from libfreemkv; exercise the
+    // parser end-to-end against a real keydb.cfg. See docs/keydb-format.md.
 
     /// REQUIRES a real keydb; `#[ignore]` by default. Run with:
     /// `KEYDB_PATH=/path/to/keydb.cfg cargo test --release -- --ignored`.
@@ -2241,10 +2044,8 @@ mod tests {
     #[ignore = "needs KEYDB_PATH + ENCRYPTED_UNIT_PATH: cargo test -- --ignored"]
     fn test_decrypt_real_unit() {
         // Try decrypting a real encrypted aligned unit from a UHD sample.
-        // This disc is AACS 2.0 (BEE) so unit key alone won't work —
-        // we need bus decryption first. But this verifies the pipeline.
-        // Path comes from ENCRYPTED_UNIT_PATH (same env-driven pattern as the
-        // KEYDB_PATH fixture); no-ops in CI when unset.
+        // This disc is AACS 2.0 (BEE) so unit key alone won't work — bus
+        // decryption is needed first — but this verifies the pipeline.
         let unit_path = std::path::PathBuf::from(
             std::env::var("ENCRYPTED_UNIT_PATH")
                 .expect("ENCRYPTED_UNIT_PATH must point at an encrypted aligned unit"),
@@ -2302,16 +2103,9 @@ mod tests {
             }
         }
 
-        // Reaching here is the EXPECTED outcome for the AACS 2.0 (BEE) sample
-        // this test was written against: the unit is still bus-encrypted, so no
-        // unit key alone can open it. That makes "no key worked" an unusable
-        // assertion — which is exactly why this test used to end in an
-        // `eprintln!` and fall off the end, i.e. pass unconditionally. A no-op
-        // `decrypt_unit`, an `is_clean` stuck at `false`, or a parser that
-        // yielded zero keys would all have produced this same clean exit.
-        //
-        // What CAN be asserted regardless of the sample: the pipeline actually
-        // ran, and the decrypt primitive actually transformed the bytes.
+        // Reaching here is the EXPECTED outcome for this AACS 2.0 (BEE) sample.
+        // See docs/keydb-format.md for why "no key worked" alone would be an
+        // unusable assertion, and what is asserted instead.
         assert!(attempts > 0, "no unit key was tried at all");
         assert!(
             any_key_changed_the_unit,
@@ -2333,13 +2127,9 @@ mod tests {
         let path = real_keydb_path();
         let db = KeyDb::load(&path).unwrap();
 
-        // Find any BD entry that carries a VUK and unit keys, then exercise
-        // the lookup-by-hash + VUK-derivation chain against it.
-        // `expect`, not an early `return`: returning here asserted NOTHING while
-        // reporting success, so a parser regression that stopped populating
-        // `vid`/`vuk` — the very fields this test exists to exercise — would make
-        // the `find` miss and ship green. If the fixture genuinely has no such
-        // entry, that is a broken fixture and must be loud.
+        // Find any BD entry with a VUK and unit keys. `expect`, not an early
+        // `return`: a `return` here would ship green even if a parser
+        // regression stopped populating `vid`/`vuk` entirely.
         let entry = db
             .disc_entries
             .values()

src/lib.rs

@@ -1,24 +1,11 @@
 //! Pluggable AACS key sources for libfreemkv.
 //!
-//! libfreemkv owns the AACS crypto; this crate provides the published
-//! [`KeySource`] implementations that look a disc up and drive the boil-down
-//! primitives down to terminal Unit Keys:
-//!
-//! - [`KeydbSource`] — a local `keydb.cfg` (source #1).
-//! - [`OnlineSource`] — a remote key service (source #2).
-//!
-//! Applications (autorip, the `freemkv` CLI) choose and order the sources from
-//! their own config — the local-vs-online policy is just which impls they plug
-//! in — then resolve and hand the resulting key to `Disc::decrypt_with`.
-//!
-//! Each source resolves a disc's terminal **Unit Keys** in one shot via
-//! [`KeySource::get_unit_keys`], driving libfreemkv's boil-down crypto primitives for
-//! whatever level of material it holds. Compose several with [`MultiSource`] in
-//! the caller's chosen order. Reading the encrypted content-sample units a key
-//! server validates on, and applying the resolved keys against a disc, is
-//! decryption *mechanism* — it lives in the library
-//! (`libfreemkv::resolve_and_apply`, `libfreemkv::read_encrypted_units`), not
-//! here.
+//! libfreemkv owns the AACS crypto; this crate provides [`KeySource`] impls
+//! that look a disc up and drive the boil-down primitives to terminal Unit
+//! Keys: [`KeydbSource`] (local `keydb.cfg`) and [`OnlineSource`] (remote key
+//! service). Applications choose and order sources, then resolve and hand
+//! the key to `Disc::decrypt_with`; compose several with [`MultiSource`].
+//! See docs/lib-scope.md for the mechanism-vs-policy split with libfreemkv.
 
 mod keydb;
 /// The `keydb.cfg` parser (`KeyDb`, `DiscEntry`, …). Public: parsing the keydb
@@ -39,10 +26,9 @@ pub use libfreemkv::aacs::types::UnitKey;
 pub use libfreemkv::keysource::ResolveCtx;
 pub use libfreemkv::{DiscInputs, KeySource};
 
-/// VUK → the disc's terminal Unit Keys (positional index), one AES-ECB-decrypt
-/// per encrypted title key. Composes the raw `aacs::derive::decrypt_unit_key`
-/// primitive directly — replaces the removed libfreemkv `aacs::boil::uk_from_vuk`
-/// wrapper (that veneer is gone; libfreemkv owns only the AES).
+// VUK -> the disc's terminal Unit Keys (positional index), one AES-ECB-decrypt
+// per encrypted title key, via `aacs::derive::decrypt_unit_key`. Replaces the
+// removed libfreemkv `aacs::boil::uk_from_vuk` wrapper.
 pub(crate) fn uks_from_vuk(vuk: &[u8; 16], enc_title_keys: &[[u8; 16]]) -> Vec<UnitKey> {
     enc_title_keys
         .iter()
@@ -52,24 +38,14 @@ pub(crate) fn uks_from_vuk(vuk: &[u8; 16], enc_title_keys: &[[u8; 16]]) -> Vec<U
 }
 
 /// An ordered composition of key sources, driven as one.
-/// [`MultiSource::get_unit_keys`] tries each inner source in order and returns
-/// the first non-empty Unit Key set (and [`MultiSource::get_fmts_indexes`] does
-/// the same for the forensic set). **The caller supplies the list AND the
-/// order** — local-first `[Keydb,
-/// Online]`, online-first `[Online, Keydb]`, etc. —
-/// so the "which sources, in what order" policy lives entirely with the
-/// application, not the library. `MultiSource` is itself a [`KeySource`], so it
-/// nests and composes.
 ///
-/// **A source that could not ANSWER is not a source that answered "no key".**
-/// When no inner source produces keys, the composition returns `Err` if any
-/// inner source failed, and `Ok(empty)` only when every source genuinely
-/// answered and none held a key. This mirrors libfreemkv's own resolver
-/// (`keysource::drive_unit_keys`, which tracks an `errored` flag, and
-/// `source_failure`, which stamps the first failure onto the disc) — a
-/// composition that swallowed the `Err` would hand the resolver a clean
-/// `Ok(empty)`, and an outage would once again be reported to the operator as
-/// `E7022 No key source has a decryption key for this disc`.
+/// [`MultiSource::get_unit_keys`] tries each inner source in order and
+/// returns the first non-empty Unit Key set (and
+/// [`MultiSource::get_fmts_indexes`] does the same for the forensic set). The
+/// caller supplies the list AND the order — local-first `[Keydb, Online]`,
+/// online-first `[Online, Keydb]`, etc. `MultiSource` is itself a
+/// [`KeySource`], so it nests and composes. See docs/lib-scope.md for the
+/// `Err`-vs-`Ok(empty)` failure contract.
 pub struct MultiSource {
     sources: Vec<Box<dyn KeySource>>,
 }
@@ -81,19 +57,9 @@ impl MultiSource {
     }
 }
 
-/// Drive `sources` in order and return the first non-empty result — preserving
-/// the Ok/Err distinction when nothing resolves.
-///
-/// A source failure never BLOCKS the chain (a later source is still tried, and a
-/// later success still wins outright, error or no error), but it is not
-/// forgotten either: with no keys anywhere, the FIRST failure is returned. First
-/// (not last) matches libfreemkv's `source_failure` rule, so the most-preferred
-/// source's reason is the one the operator is told about, in the same order the
-/// caller expressed a preference for successes.
-///
-/// `get` selects which trait method to drive, so the base and forensic paths
-/// share ONE implementation of this rule and cannot drift apart — the forensic
-/// half carried the identical bug.
+// Drive `sources` in order, returning the first non-empty result and
+// preserving Ok/Err when nothing resolves. `get` selects the trait method so
+// the base and forensic paths share one implementation. See docs/lib-scope.md.
 fn first_non_empty(
     sources: &[Box<dyn KeySource>],
     get: impl Fn(&dyn KeySource, &dyn ResolveCtx) -> Result<Vec<UnitKey>, libfreemkv::Error>,
@@ -121,19 +87,16 @@ fn first_non_empty(
 }
 
 impl KeySource for MultiSource {
-    /// Try each inner source in order; the FIRST to return a non-empty base Unit
-    /// Key set wins, even if an earlier source failed. All sources exhausted →
-    /// `Ok(empty)` if they all ANSWERED, or the first source failure if any could
-    /// not (see [`MultiSource`]).
+    // First inner source to return a non-empty base Unit Key set wins. All
+    // exhausted -> `Ok(empty)` if all answered, else the first failure (see
+    // [`MultiSource`]).
     fn get_unit_keys(&self, ctx: &dyn ResolveCtx) -> Result<Vec<UnitKey>, libfreemkv::Error> {
         first_non_empty(&self.sources, |s, c| s.get_unit_keys(c), ctx)
     }
 
-    /// Forensic-index counterpart: try each inner source's `get_fmts_indexes` in
-    /// the same order and return the first non-empty set, under the same
-    /// failure-preserving rule. A source with no forensic material (the keydb,
-    /// via the trait default) contributes empty and is skipped; on today's discs
-    /// the online source answers.
+    // Forensic-index counterpart to `get_unit_keys`, same failure-preserving
+    // rule. A source with no forensic material (keydb, via the trait default)
+    // contributes empty and is skipped; today the online source answers.
     fn get_fmts_indexes(&self, ctx: &dyn ResolveCtx) -> Result<Vec<UnitKey>, libfreemkv::Error> {
         first_non_empty(&self.sources, |s, c| s.get_fmts_indexes(c), ctx)
     }

src/online.rs

@@ -16,46 +16,28 @@ use ureq::unversioned::resolver::{ResolvedSocketAddrs, Resolver};
 use ureq::unversioned::transport::{DefaultConnector, NextTimeout};
 
 // Upper bound on the MKB forwarded to the key service — kept in lockstep with
-// libfreemkv's `read_mkb_content` MAX_BYTES (64 MiB) so an MKB the library is
-// willing to capture is never silently un-forwardable here (a trimmed MKB
-// record stream is normally a few MiB; this is headroom, not an expected size).
+// libfreemkv's `read_mkb_content` MAX_BYTES (64 MiB), so a capturable MKB is
+// never silently un-forwardable here (headroom, not an expected size).
 const MAX_MKB_BYTES: usize = 64 * 1024 * 1024;
 const TIMEOUT_SECS: u64 = 180;
 /// Minimum encrypted-content samples the online source will send in one key
-/// request — re-exported from the base crate ([`libfreemkv::keysource::MIN_SAMPLE_UNITS`])
-/// so this crate and libfreemkv's own FMTS forensic query share ONE value.
+/// request — re-exported from the base crate
+/// ([`libfreemkv::keysource::MIN_SAMPLE_UNITS`]) so this crate and
+/// libfreemkv's own FMTS forensic query share ONE value.
 ///
-/// The service identifies the key by which of the submitted units it decrypts,
-/// so too few samples — especially on FMTS, where a segment interleaves several
-/// variants at the unit level — can return a key that matches an incidental unit
-/// rather than the one asked about (a false positive). A request carrying fewer
-/// is refused (empty result → the resolver moves to the next source) rather than
-/// sent and trusted. Kept public so callers that GATHER the samples (the CLI,
-/// autorip) sample at least this many — sampling fewer guarantees the request is
-/// skipped and the online source never consulted.
+/// A request carrying fewer samples is refused (empty result, never sent)
+/// since the service identifies the key by which submitted unit it decrypts
+/// and too few risks a false-positive match. Kept public so gathering
+/// callers (the CLI, autorip) sample at least this many.
 pub use libfreemkv::keysource::MIN_SAMPLE_UNITS;
 /// Hard cap on the key-service response body. A real unit-key reply is a few
 /// hundred bytes; bound the read so a malicious/compromised server can't drive
 /// the client to OOM with an unbounded body.
 const MAX_RESPONSE_BYTES: usize = 1024 * 1024;
 
-// ── SSRF guard ──────────────────────────────────────────────────────────────
-//
-// The keyserver URL is operator-supplied and the bearer token is confidential.
-// After validate_keyserver_url checked the host at config time, an attacker
-// who controls the keyserver's DNS can rebind it to 169.254.169.254 (cloud
-// metadata) or an RFC1918 host in the window between validation and the
-// actual POST, exfiltrating the key material and the Authorization token.
-//
-// Defence: resolve the host once just before the POST, reject any blocked IP,
-// and pin the ureq connection to those validated addresses so a subsequent DNS
-// flip cannot redirect the request. Use redirects(0) so a public URL can't
-// 30x-redirect to an internal host.
-
-/// True when `ip` must never be the target of an outbound key-service POST.
-/// Blocks loopback, link-local (incl. 169.254.0.0/16 cloud metadata), all
-/// RFC1918 private ranges, carrier-grade NAT, multicast, unspecified, and
-/// IPv4-mapped equivalents for all of the above.
+// ── SSRF guard — see docs/online-ssrf-guard.md ───────────────────────────────
+// is_blocked_ip: true when `ip` must never be an outbound key-service POST
+// target (loopback, link-local incl. cloud metadata, RFC1918, IPv4-mapped).
 fn is_blocked_ip(ip: &IpAddr) -> bool {
     match ip {
         IpAddr::V4(v4) => {
@@ -92,39 +74,22 @@ fn is_blocked_ip(ip: &IpAddr) -> bool {
     }
 }
 
-/// Why [`resolve_and_guard`] rejected a URL. The two halves demand OPPOSITE
-/// operator actions, so they must not be collapsed: a `Config` rejection is a
-/// standing misconfiguration that will never fix itself, while `Unreachable` is
-/// the key service being down right now — exactly the condition this module's
-/// callers must not report as "this disc has no key".
-///
-/// SEPARABLE, NOT RANKED. Distinguishing the two is about the *message*, never
-/// about whether the caller hears anything at all. BOTH variants mean the
-/// service was never asked, so BOTH are `Err` out of [`OnlineSource::query`]
-/// (see the match there). The first cut of this enum routed `Config` to
-/// `Ok(Vec::new())` and only `Unreachable` to `Err`, which made a permanent
-/// fault — a mistyped port, an `http://` URL, a host that resolves into RFC1918
-/// — strictly QUIETER than a transient one: one `warn!` and then a benign-looking
-/// empty that `MultiSource::first_non_empty` cannot tell from "this keydb simply
-/// has no row for this disc". That is the seven-hour-502 collapse with the
-/// polarity reversed: the outage at least self-heals, the typo never does.
+// Why resolve_and_guard rejected a URL, split by the operator action each
+// demands: Config is a standing misconfiguration (never self-heals),
+// Unreachable is the service down now. Both are Err from query, never Ok(empty).
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 enum GuardFail {
-    /// Malformed URL, bad scheme, or a host that is (or resolves to) a
-    /// non-public address. Operator configuration; retrying changes nothing.
+    // Malformed URL, bad scheme, or a host that resolves to a non-public
+    // address. Operator configuration; retrying changes nothing.
     Config,
-    /// The host did not resolve — DNS failure or DNS timeout. The service is
-    /// unreachable; nothing is known about this disc's key. Transient.
+    // Host did not resolve — DNS failure or timeout. Service unreachable;
+    // nothing known about this disc's key. Transient.
     Unreachable,
 }
 
-/// Resolve `url`'s host and validate every resulting address against the SSRF
-/// guard. Returns the pinned socket addresses (for use with a custom ureq
-/// resolver) on success, or the rejection reason plus an explanatory message.
-///
-/// SECURITY: the message names the resolved address on an SSRF rejection and is
-/// for the CONFIG-time caller only — never log it (see the call site in
-/// [`OnlineSource::query`]).
+// Resolve `url`'s host, validating every address against the SSRF guard;
+// returns pinned socket addrs or a rejection reason + message. SECURITY: the
+// message names the address — log only at config time, never in `query`.
 fn resolve_and_guard(url: &str) -> Result<Vec<SocketAddr>, (GuardFail, String)> {
     let rest = if let Some(r) = url.strip_prefix("https://") {
         (r, 443u16)
@@ -160,19 +125,9 @@ fn resolve_and_guard(url: &str) -> Result<Vec<SocketAddr>, (GuardFail, String)>
     } else if let Some((h, p)) = authority.rsplit_once(':') {
         match p.parse::<u16>() {
             Ok(p) => (h.to_string(), p),
-            // A `host:port` authority whose port is not a u16 is a TYPO in the
-            // operator's config — `https://example.com:notaport/keys`. This used
-            // to fall back to the WHOLE authority as the hostname (port text
-            // included), which of course never resolves, so the URL surfaced as
-            // `GuardFail::Unreachable` → `Err(KeyServiceUnavailable)`: a standing
-            // misconfiguration reported as a transient outage, the exact
-            // collapse `GuardFail`'s own doc says must not happen, because the
-            // two halves demand OPPOSITE operator actions (fix the URL vs. wait).
-            // Reject it as `Config`, identical to the bracketed-IPv6 branch a few
-            // lines above, which already returns `Config` for an unparseable
-            // port. Silently substituting `default_port` would be worse still —
-            // it would ship the bearer token and key material to a port the
-            // operator never configured.
+            // A malformed port (e.g. `:notaport`) is an operator config typo,
+            // not a service outage — reject as `Config`, same as the
+            // bracketed-IPv6 branch above. See docs/online-guardfail.md.
             Err(_) => return Err((GuardFail::Config, "invalid port".into())),
         }
     } else {
@@ -182,10 +137,8 @@ fn resolve_and_guard(url: &str) -> Result<Vec<SocketAddr>, (GuardFail, String)>
         return Err((GuardFail::Config, "URL has no host".into()));
     }
     // `to_socket_addrs` is a BLOCKING DNS lookup that can hang for the OS
-    // resolver timeout (tens of seconds) and freeze the rip thread that called
-    // query(). Run it on a spawned thread and join with a bounded deadline;
-    // on timeout return Err so query() yields None and the rip proceeds
-    // (mirrors the bounded-resolve in autorip/libfreemkv).
+    // resolver timeout and freeze the calling rip thread, so run it on a
+    // spawned thread with a bounded deadline (mirrors autorip/libfreemkv).
     let addrs: Vec<SocketAddr> = {
         use std::sync::mpsc;
         const DNS_TIMEOUT: Duration = Duration::from_secs(10);
@@ -236,31 +189,20 @@ fn resolve_and_guard(url: &str) -> Result<Vec<SocketAddr>, (GuardFail, String)>
 /// metadata-exfiltration guard). Returns `Ok(())` on success so a caller can
 /// gate `OnlineSource` construction; the error string explains the rejection.
 ///
-/// This is the *config-time* check. [`OnlineSource`] independently re-resolves
-/// and re-guards the host immediately before each POST (and pins the validated
-/// addresses), so a DNS rebind between this check and the request can't redirect
-/// the key material. The two share the SAME `is_blocked_ip` classifier and the
-/// SAME bounded-resolve, so their verdicts never diverge — the reason this lives
-/// here, in the key-source crate, rather than being re-rolled per application.
+/// This is the *config-time* check; [`OnlineSource`] re-resolves and
+/// re-guards the host again before each POST, closing the DNS-rebind window.
 pub fn validate_keyserver_url(url: &str) -> Result<(), String> {
     resolve_and_guard(url).map(|_| ()).map_err(|(_, msg)| msg)
 }
 
-/// ureq's `ResolvedSocketAddrs` is a fixed 16-slot array and its `push` writes
-/// straight into that array, so handing it a 17th address is an out-of-bounds
-/// panic — in the rip thread, on a host that merely publishes a lot of A
-/// records. Keep the first 16; every one of them was validated by
-/// [`resolve_and_guard`], so a subset is still safe, just less redundant.
+// ureq's `ResolvedSocketAddrs` is a fixed 16-slot array; `push`ing a 17th
+// address panics (out of bounds) on a host with many A records. Keep the
+// first 16 — each already validated by `resolve_and_guard`.
 const MAX_PINNED_ADDRS: usize = 16;
 
-/// The pinned-address resolver behind [`hardened_agent`].
-///
-/// ureq 3 replaced v2's resolver closure with this trait. The agent MUST be
-/// built through `Agent::with_parts` to take one: `Agent::new_with_config`
-/// silently keeps the default resolver, which would send the request to live
-/// DNS and quietly reopen the rebinding window this module exists to close.
-/// That failure is invisible — it has no symptom short of an actual attack —
-/// so it is pinned by `hardened_agent_connects_to_the_pinned_address_not_dns`.
+// The pinned-address resolver behind `hardened_agent`. Must be wired via
+// `Agent::with_parts` — `new_with_config` silently keeps live DNS and
+// reopens the rebind window. See docs/online-ssrf-guard.md.
 #[derive(Debug)]
 struct PinnedResolver(Vec<SocketAddr>);
 
@@ -299,31 +241,13 @@ fn hardened_agent(pinned: Vec<SocketAddr>) -> ureq::Agent {
 pub struct OnlineSource {
     base_url: String,
     secret: String,
-    /// The last agent built, with the address set it was pinned to.
-    ///
-    /// A `query` used to build a FRESH `ureq::Agent` every call, and an FMTS disc
-    /// makes two calls per rip (`get_unit_keys` + `get_fmts_indexes`), so it paid
-    /// two TLS handshakes to the same host for no reason — an agent owns the
-    /// connection pool, and a discarded agent discards the pooled, already
-    /// negotiated TLS connection with it.
-    ///
-    /// SECURITY: the agent is reused ONLY when the freshly resolved + guarded
-    /// address SET is identical to the one the cached agent is pinned to. Every
-    /// query still re-resolves and re-runs the SSRF guard — that is the whole
-    /// anti-rebinding defence and is deliberately not cached; what is reused is
-    /// the connection, and only to addresses just re-validated this call.
-    ///
-    /// The stored `Vec<SocketAddr>` is the SET KEY: sorted and deduped, so the
-    /// comparison is order-insensitive. `to_socket_addrs` hands back whatever
-    /// order the resolver felt like, and a round-robin keyserver reorders its
-    /// A/AAAA records on essentially every lookup — an ordered `==` therefore
-    /// missed the cache every single time against exactly the deployment that
-    /// most needs it, silently degrading to the per-query TLS handshake this
-    /// field exists to avoid. Set equality is also the honest predicate: the
-    /// security property being asserted is "these are the same validated
-    /// addresses", which says nothing about their order. The agent itself stays
-    /// pinned to the order it was BUILT with; that is safe because an equal set
-    /// means every address it holds was re-resolved and re-guarded this call.
+    /// The last agent built, with the address set (sorted, deduped — an
+    /// order-insensitive SET KEY) it was pinned to. Reused only when a fresh
+    /// resolve + SSRF-guard of the host yields the identical address set, so
+    /// the anti-rebinding guarantee is untouched: only the pooled TLS
+    /// connection is reused, never a stale, un-reguarded address. See
+    /// docs/online-agent-cache.md for why this exists and why the key is a
+    /// set rather than an ordered sequence.
     agent: Mutex<Option<(Vec<SocketAddr>, Arc<ureq::Agent>)>>,
 }
 
@@ -336,10 +260,9 @@ impl OnlineSource {
         }
     }
 
-    /// The agent pinned to `pinned` — the cached one when the address set is
-    /// unchanged, a fresh one otherwise (which then becomes the cached one).
-    /// Poisoning is recovered from: a panic elsewhere must not make every later
-    /// key request panic.
+    // The agent pinned to `pinned` — the cached one when the address set is
+    // unchanged, a fresh one otherwise. Poisoning is recovered from: a panic
+    // elsewhere must not make every later key request panic.
     fn agent_for(&self, pinned: Vec<SocketAddr>) -> Arc<ureq::Agent> {
         // Compare SETS, not sequences — see the `agent` field's doc.
         let key = {
@@ -359,52 +282,17 @@ impl OnlineSource {
         agent
     }
 
-    /// The server-resolved Unit Keys for this disc. Runs exactly one network
-    /// round-trip. The service returns either a terminal `UK` (used directly) or
-    /// a `VUK` (derived to Unit Keys locally via the disc's encrypted title keys
-    /// from `ctx`).
-    ///
-    /// The return type draws THE distinction this source exists to draw:
-    ///
-    /// * `Ok(non-empty)` — the service answered with a key.
-    /// * `Ok(empty)` — the service ANSWERED and holds no key for this disc, or
-    ///   this source had nothing to ask with (no service configured, a
-    ///   misconfigured URL, an over-cap MKB, too few samples). A genuine miss;
-    ///   the resolver moves to the next source and `E7022` is the right verdict.
-    /// * `Err(..)` — the service could not answer: unreachable, timed out, DNS
-    ///   failed, returned 5xx, rejected the token (401/403), rate-limited (429),
-    ///   or replied with something unreadable. NOTHING is known about whether a
-    ///   key exists. Collapsing this into `Ok(empty)` is the bug this signature
-    ///   fixes: a seven-hour run of HTTP 502s was reported to operators as
-    ///   `E7022 No key source has a decryption key for this disc`, and they went
-    ///   hunting for a VUK when the correct action was to wait.
-    ///
-    /// `&self`: one-shot is the resolver's contract (each source's
-    /// `get_unit_keys` is called once), so no per-call latch is needed.
+    // The server-resolved Unit Keys for this disc: one round-trip, returning
+    // a terminal `UK` or a `VUK` derived locally. `Ok`/`Err` draw the
+    // miss-vs-outage distinction — see docs/online-query-contract.md.
     fn query(&self, ctx: &dyn ResolveCtx) -> Result<Vec<UnitKey>, Error> {
         // No configured service: nothing to resolve.
         if self.base_url.is_empty() {
             return Ok(Vec::new());
         }
-        // Refuse to transmit over plaintext. The request body always carries
-        // base64 AACS key material (inf/mkb/units/vid) and, when configured, a
-        // bearer token in a header — so an `http://` POST hands both to any
-        // on-path observer, and the token is replayable.
-        //
-        // There is no legitimate plaintext deployment to preserve here: the
-        // address guard below rejects loopback, RFC1918, ULA and link-local, so
-        // an `http://` URL can only ever reach the PUBLIC internet — the single
-        // worst case for cleartext. A self-hosted keyserver on a LAN is already
-        // impossible by that guard, whatever its scheme.
-        //
-        // Still falls THROUGH to the next key source (a local keydb) — an `Err`
-        // here does not block the chain, because `MultiSource::first_non_empty`
-        // returns any later source's non-empty key set outright and only
-        // surfaces the failure when nothing anywhere resolved. What it does stop
-        // is the silent case: a cleartext URL is the same permanent operator
-        // fault as the address-guard rejection below (see [`GuardFail`]), so it
-        // gets the same verdict. Returning `Ok(empty)` here made "we refused to
-        // ask" indistinguishable from "the service answered and has no key".
+        // Refuse to transmit over plaintext: the body carries base64 key
+        // material and, when configured, a replayable bearer token. `Err`,
+        // not `Ok(empty)` — see docs/online-query-guards.md.
         if !self.base_url.starts_with("https://") {
             tracing::error!(
                 target: "freemkv::keysource",
@@ -415,26 +303,9 @@ impl OnlineSource {
             return Err(Error::KeyServiceUnavailable);
         }
         let mkb = ctx.mkb().unwrap_or(&[]);
-        // THE LINE this source draws between its skips (written out after the
-        // `Config`-was-quieter-than-`Unreachable` bug), in three parts:
-        //
-        // * NO URL AT ALL is not a fault: the operator did not configure an
-        //   online source, so there is nothing to report. `Ok(empty)`, silently
-        //   (the first guard in this function).
-        // * A bad URL — wrong scheme, mistyped port, guard-blocked address — is
-        //   an OPERATOR FAULT. Every disc is affected, nothing was asked, and
-        //   the fix is to edit the config. That is `Err` (above).
-        // * An over-cap MKB or too few content samples is a property of THIS
-        //   DISC's inputs, not a fault: the request cannot be *formed* for this
-        //   disc, so this source has nothing for it and never will, whatever the
-        //   operator does and however long they wait. Reporting that as
-        //   `KeyServiceUnavailable` would send the operator waiting out an
-        //   outage that does not exist — the same mislabelling in the opposite
-        //   direction. These stay `Ok(empty)`, and stay logged.
-        //
-        // Log it: a silent empty return here is indistinguishable from "no key",
-        // so surface the real cause (the cap is 64 MiB, far above any real
-        // trimmed MKB).
+        // No-URL / bad-URL / over-cap-or-under-sampled draw three different
+        // verdicts — see docs/online-query-guards.md. Logged: a silent empty
+        // here reads as "no key", so the real cause (64 MiB cap) is surfaced.
         if mkb.len() > MAX_MKB_BYTES {
             tracing::warn!(
                 target: "freemkv::keysource",
@@ -444,12 +315,9 @@ impl OnlineSource {
             );
             return Ok(Vec::new());
         }
-        // Gather encrypted-content samples and prove the minimum by TYPE: a
-        // `DecodeSampleSet` only exists with >= MIN_SAMPLE_UNITS units, so from here
-        // on the request cannot be built under-sized. The service resolves a key by
-        // which submitted unit it decrypts, so a request carrying too few can return
-        // a key matching an incidental unit (a false positive, seen on FMTS variant
-        // units) — too few → skip this source and fall through to the next.
+        // Prove the minimum by TYPE: `DecodeSampleSet` only exists with >=
+        // MIN_SAMPLE_UNITS units, so the request can't be built under-sized
+        // (too few risks the service matching an incidental unit — FMTS).
         let gathered = ctx.samples(64).unwrap_or_default();
         let n = gathered.len();
         let Some(samples) = DecodeSampleSet::new(gathered) else {
@@ -488,15 +356,12 @@ impl OnlineSource {
             body["title"] = serde_json::Value::String(label.to_string());
         }
         // Resolve + SSRF-guard the host just before the POST; pin the
-        // validated addresses into a redirect-disabled agent so a DNS
-        // rebind between config time and fetch time can't redirect the
-        // request (and the bearer token) to an internal/metadata host.
+        // validated addresses so a DNS rebind between config time and fetch
+        // time can't redirect the request to an internal/metadata host.
         let pinned = match resolve_and_guard(&self.base_url) {
             Ok(addrs) => addrs,
-            // A host that did not RESOLVE is the service being unreachable, not a
-            // bad URL — DNS failure and DNS timeout are exactly what a key-service
-            // outage looks like from here, and reporting them as a genuine miss is
-            // the same conflation as reporting a 502 that way.
+            // Did-not-RESOLVE is the service unreachable, not a bad URL — see
+            // docs/online-guardfail.md.
             Err((GuardFail::Unreachable, _)) => {
                 tracing::warn!(
                     target: "freemkv::keysource",
@@ -507,37 +372,16 @@ impl OnlineSource {
                 return Err(Error::KeyServiceUnavailable);
             }
             Err((GuardFail::Config, _)) => {
-                // Log THAT the URL was rejected, never WHY: the guard's message
-                // names the resolved address, and an internal address must not
-                // reach a log an operator may paste into a bug report. The
-                // static label is enough to separate "misconfigured/blocked
-                // key-service URL" from "this disc has no key", which is the
-                // only distinction the operator needs here.
-                //
-                // `error!`, not `warn!`, and `Err`, not `Ok(empty)`. This arm
-                // used to return `Ok(Vec::new())`, which made the PERMANENT
-                // fault quieter than the transient one directly above it: the
-                // service was never asked, yet the composition was handed a
-                // clean empty and — with no other source holding the key — told
-                // the operator `E7022 no key for this disc`. Same lie as the
-                // seven-hour 502, from a config typo that never self-heals. A
-                // failure to ask is never evidence about the disc, so it leaves
-                // as `Err`; `MultiSource::first_non_empty` still lets a later
-                // source's real key win outright, so this cannot fail a rip that
-                // the local keydb could have served.
+                // Log THAT the URL was rejected, never WHY (the message names
+                // the address). `error!` + `Err`, not `Ok(empty)` — see
+                // docs/online-guardfail.md for the full rationale.
                 tracing::error!(
                     target: "freemkv::keysource",
                     phase = "keyserver_post",
                     "key-service URL failed the address guard — the service was NOT asked about this disc; \
                      this is a standing misconfiguration (fix the URL), not a disc without a key"
                 );
-                // NOTE (reported upstream): libfreemkv has no
-                // `KeyServiceMisconfigured` code, so the permanent fault borrows
-                // the transient E7028. E7028's contract — "the source never got
-                // as far as answering the question" — is exactly true here; only
-                // its "transient, retry later" hint is wrong, and the `error!`
-                // above carries the correcting detail until a 70xx config code
-                // exists.
+                // Borrows transient E7028 pending a 70xx config code upstream.
                 return Err(Error::KeyServiceUnavailable);
             }
         };
@@ -546,12 +390,9 @@ impl OnlineSource {
         if let Some(value) = bearer_header(&self.secret) {
             req = req.header("Authorization", &value);
         }
-        // Begin/end around the keyserver round-trip — a slow or unresponsive
-        // service is the suspected DVD-scan hang. The agent is built with a
-        // 10s connect + bounded read timeout (see `hardened_agent`), so this
-        // call can never block forever; we log the timing so a slow round-trip
-        // is visible. SECURITY: never log `body` — it carries base64 key
-        // material.
+        // Begin/end around the keyserver round-trip: bounded by
+        // `hardened_agent`'s connect/read timeouts, so this can never block
+        // forever. SECURITY: never log `body` — it carries base64 key material.
         tracing::info!(target: "freemkv::keysource", phase = "keyserver_post", "begin");
         let post_t0 = std::time::Instant::now();
         let sent = req.send_json(body);
@@ -559,17 +400,9 @@ impl OnlineSource {
     }
 }
 
-/// Map a key-service HTTP status the client never asked for into the operator
-/// action it implies. THE table this fix exists for — each arm is a different
-/// thing for a person to do, and none of them is "your disc has no key":
-///
-/// * 401 / 403 → the token is wrong or expired. Fix the credentials.
-/// * 429 → rate limited. Back off and retry more slowly.
-/// * 5xx → the service is down. Wait; this is the seven-hour-502 case.
-/// * anything else non-2xx → the service is misbehaving in a way this client
-///   cannot interpret. Treated as unavailable, deliberately: the genuine miss is
-///   a **200 with no key in the body**, so a non-2xx status is never evidence
-///   about this disc, and guessing "no key" from one is the original bug.
+// Map a key-service HTTP status into the operator action it implies: 401/403
+// fix credentials, 429 back off, 5xx wait — none of them is "no key" (the
+// genuine miss is a 200 with an empty body), the original bug this fixes.
 fn classify_http_status(code: u16) -> Error {
     match code {
         401 | 403 => Error::KeyServiceUnauthorized,
@@ -578,20 +411,9 @@ fn classify_http_status(code: u16) -> Error {
     }
 }
 
-/// Turn the raw outcome of the key-service POST into this source's answer.
-///
-/// Split out of [`OnlineSource::query`] so the reply → verdict mapping is
-/// testable WITHOUT a network: the SSRF guard in `resolve_and_guard` blocks
-/// loopback by design, so a stub HTTP server on 127.0.0.1 can never be reached
-/// through `query`, and this seam is the only honest way to drive "the service
-/// returned 502" and "the service returned 200 with no entry" through the same
-/// code the rip uses.
-///
-/// `elapsed_ms` is the round-trip time, for the timing logs only.
-///
-/// SECURITY: logs status codes, byte counts and static labels only. The request
-/// body carries base64 AACS key material and the reply carries keys — neither is
-/// ever logged, and nor is a serde parse error (it quotes the offending input).
+// Turn the raw key-service POST outcome into this source's answer. Split
+// out of `query` so the reply -> verdict mapping is testable WITHOUT a
+// network. SECURITY: logs status/byte-count/labels only — never `body`.
 fn interpret_reply(
     sent: Result<ureq::http::Response<ureq::Body>, ureq::Error>,
     ctx: &dyn ResolveCtx,
@@ -600,17 +422,9 @@ fn interpret_reply(
     let mut resp = match sent {
         Ok(r) => r,
         Err(e) => {
-            // Record the HTTP status when the service actually answered:
-            // 401/403 (bad or expired token), 429 (rate limited) and
-            // 5xx (service down) are completely different operator actions,
-            // and collapsing them into one line is why a 502 was read as
-            // "this disc has no key". The status is the service's own
-            // response code — it carries no key material and no address.
-            // A transport error has no status; report it as such rather
-            // than inventing one.
-            //
-            // Each arm now RETURNS the classified error instead of an empty
-            // vec, so the distinction survives past this function.
+            // 401/403/429/5xx demand different operator actions; collapsing
+            // them is why a 502 was read as "no key". Each arm RETURNS the
+            // classified error, never an empty vec, so it survives past here.
             return Err(match e {
                 ureq::Error::StatusCode(code) => {
                     let err = classify_http_status(code);
@@ -624,13 +438,9 @@ fn interpret_reply(
                     );
                     err
                 }
-                // ureq 3 fans v2's single `Transport` variant out into `Io`,
-                // `Timeout`, `Tls`, `Protocol`, `HostNotFound` and more, and the
-                // enum is non_exhaustive. Every one of them means the same thing
-                // here — nothing answered, so nothing is known about this disc —
-                // and a catch-all is the only shape that stays correct as the
-                // enum grows. The status arm above is the sole case where the
-                // service actually replied.
+                // ureq 3's non_exhaustive transport-error enum (`Io`, `Timeout`,
+                // `Tls`, ...) all mean the same thing here — nothing answered —
+                // so a catch-all stays correct as the enum grows.
                 _ => {
                     tracing::warn!(
                         target: "freemkv::keysource",
@@ -674,10 +484,9 @@ fn interpret_reply(
     let json: serde_json::Value = match serde_json::from_slice(&buf) {
         Ok(j) => j,
         Err(_) => {
-            // Never log the parse error or the payload: a serde message
-            // quotes the offending input, which here is key material.
-            // Unparseable is not "no key" — it is the service answering in a
-            // language this client does not speak. Transient / a service bug.
+            // Never log the parse error or payload (a serde message quotes
+            // the offending input, i.e. key material). Unparseable is a
+            // service bug, not "no key".
             tracing::warn!(
                 target: "freemkv::keysource",
                 phase = "keyserver_post",
@@ -686,12 +495,9 @@ fn interpret_reply(
             return Err(Error::KeyServiceUnavailable);
         }
     };
-    // `UK` is an ARRAY of hex keys (the service always returns an array now,
-    // even of one). A single element is the base Unit Key. A full set (one per
-    // forensic index, ordered index 1..N) is returned for a forensic sample.
-    // Preserve array order and tag each key with its array position, so the
-    // caller can map position → index (element i = index i+1). A bare string is
-    // still accepted for backward compatibility.
+    // `UK` is an ARRAY of hex keys — one for the base Unit Key, or an
+    // ordered forensic-index set. Array position tags each key (i -> index
+    // i+1). A bare string is still accepted for backward compatibility.
     if let Some(uk) = json.get("UK") {
         let mut out = Vec::new();
         if let Some(s) = uk.as_str() {
@@ -699,12 +505,9 @@ fn interpret_reply(
                 out.push(UnitKey::new(0, k));
             }
         } else if let Some(arr) = uk.as_array() {
-            // A forensic set is only usable COMPLETE: the mux trusts any
-            // non-empty result as the whole set and never assumes 32 (see
-            // `get_fmts_indexes`). Skipping an unparseable element would
-            // hand back a short set that silently omits an index, so a
-            // malformed element rejects the whole reply — that is a service
-            // bug, not a usable answer, and therefore a source FAILURE.
+            // A forensic set is only usable COMPLETE (the mux trusts any
+            // non-empty result as the whole set). Skipping a bad element
+            // would silently omit an index, so reject the whole reply.
             let mut bad = false;
             for (i, v) in arr.iter().enumerate() {
                 match v.as_str().and_then(parse_uk) {
@@ -735,10 +538,8 @@ fn interpret_reply(
         match ctx.enc_title_keys() {
             Ok(enc) => return Ok(uks_from_vuk(&vuk, enc)),
             Err(_) => {
-                // The SERVICE answered correctly; the DISC's encrypted title keys
-                // are what could not be read. Not a service failure, so not an
-                // `Err` from this source — the disc-side reason is the library's
-                // to report.
+                // The SERVICE answered; the DISC's encrypted title keys are
+                // what could not be read — a disc-side reason, not `Err` here.
                 tracing::warn!(
                     target: "freemkv::keysource",
                     phase = "keyserver_post",
@@ -749,11 +550,9 @@ fn interpret_reply(
             }
         }
     }
-    // The genuine miss — and the ONLY path that returns `Ok(empty)` from a
-    // completed round-trip. Logged distinctly from every failure above so an
-    // operator can tell "the service has no key for this disc" from "the
-    // service did not answer"; collapsing the two is what made a 502 look like
-    // a missing key. This is the one case where `E7022` is the truth.
+    // The genuine miss — the ONLY path returning `Ok(empty)` from a completed
+    // round-trip, logged distinctly from every failure above so a 502 can
+    // never again look like a missing key. `E7022` is the truth here.
     tracing::info!(
         target: "freemkv::keysource",
         phase = "keyserver_post",
@@ -763,23 +562,16 @@ fn interpret_reply(
 }
 
 impl KeySource for OnlineSource {
-    /// Base per-CPS-unit Unit Keys: submit the ctx's content samples and take the
-    /// service's reply (a terminal `UK`, or a `VUK` derived locally). One network
-    /// round-trip. An empty `Ok` means the service ANSWERED and holds no key; an
-    /// `Err` means it could not answer (see [`OnlineSource::query`]). Both let the
-    /// resolver try the next source — the difference is what the operator is told
-    /// when no source has one.
+    // Base per-CPS-unit Unit Keys via `query`: `Ok(empty)` means the service
+    // answered with no key, `Err` means it could not answer — see
+    // docs/online-query-contract.md.
     fn get_unit_keys(&self, ctx: &dyn ResolveCtx) -> Result<Vec<UnitKey>, Error> {
         self.query(ctx)
     }
 
-    /// AACS 2.1 forensic index set: the mux injects an index-1 single-phase anchor
-    /// batch as the ctx's samples; the service maps it to the full ordered set of
-    /// forensic index keys, tagged by array position (element `i` → forensic index
-    /// `i + 1`). Same one round-trip as [`get_unit_keys`](Self::get_unit_keys) —
-    /// the difference is purely which samples the mux gathered and how the caller
-    /// reads the reply. The count is whatever the service returns; the mux trusts
-    /// any non-empty result as the complete set and never assumes 32.
+    // AACS 2.1 forensic index set: same `query` round-trip as
+    // `get_unit_keys`, but the mux's samples are a single-phase anchor batch
+    // and the service's array position tags each forensic index.
     fn get_fmts_indexes(&self, ctx: &dyn ResolveCtx) -> Result<Vec<UnitKey>, Error> {
         self.query(ctx)
     }
@@ -788,16 +580,13 @@ impl KeySource for OnlineSource {
         "online"
     }
 
-    // host_certs: the no-op default. The online service does not serve host
-    // certs today (no client-side fetch, no server-side endpoint), so the OEM
-    // cert route falls back to whatever other source (e.g. the keydb) supplies.
-    // No network is touched. (Future task: online host-cert serving.)
+    // host_certs: no-op default. No online cert fetch/endpoint today, so
+    // OEM certs fall back to another source (e.g. keydb); no network touched.
 }
 
-/// The `Authorization` header value for a key-service request, or `None` when no
-/// secret/token is configured (the request then goes out unauthenticated). The
-/// token — passed as `--key-auth` on the CLI or `keyserver_secret` in autorip —
-/// is sent verbatim as an HTTP Bearer credential.
+// The `Authorization` header value, or `None` when no secret is configured
+// (request goes out unauthenticated). Sent verbatim as an HTTP Bearer
+// credential — the token comes from `--key-auth` (CLI) / `keyserver_secret`.
 fn bearer_header(secret: &str) -> Option<String> {
     if secret.is_empty() {
         None
@@ -869,11 +658,9 @@ mod tests {
         ))));
     }
 
-    /// The online source serves NO host certs today (no fetch, no endpoint).
-    /// `host_certs()` must return empty WITHOUT touching the network, so the OEM
-    /// route falls back to whatever else (the keydb) supplies. Uses a non-empty
-    /// base URL to prove the empty result isn't merely "no service configured" —
-    /// it's the deliberate no-op stub.
+    // host_certs() must return empty WITHOUT touching the network. Uses a
+    // non-empty base URL to prove the empty result is the deliberate no-op
+    // stub, not merely "no service configured".
     #[test]
     fn host_certs_is_noop_empty_no_network() {
         let src = OnlineSource::new("http://example.test/keys", "secret");
@@ -907,10 +694,8 @@ mod tests {
         assert!(resolve_and_guard("").is_err());
     }
 
-    /// Malformed-authority edges that `resolve_and_guard` must reject BEFORE
-    /// ever touching DNS: an empty host after the scheme (`https://`), an
-    /// unterminated IPv6 literal (missing `]`), and a bare `host:` with an
-    /// empty host part.
+    // Malformed-authority edges rejected BEFORE touching DNS: empty host,
+    // unterminated IPv6 literal, and a bare `host:` with an empty host part.
     #[test]
     fn resolve_and_guard_rejects_malformed_authorities() {
         // Scheme with nothing after it at all.
@@ -923,11 +708,8 @@ mod tests {
         assert!(resolve_and_guard("https://:8080/keys").is_err());
     }
 
-    /// A host that genuinely does not resolve (RFC 6761 reserves `.test`
-    /// to never resolve in the public DNS) must be reported as
-    /// `GuardFail::Unreachable` — "the service is down", not
-    /// `GuardFail::Config` ("fix the URL"). No real network traffic beyond a
-    /// local DNS lookup that is guaranteed to fail.
+    // A host that genuinely does not resolve (RFC 6761 `.test`) must report
+    // `GuardFail::Unreachable` ("service is down"), not `Config`.
     #[test]
     fn resolve_and_guard_reports_unreachable_for_a_host_that_never_resolves() {
         let (kind, msg) = resolve_and_guard("https://this-host-does-not-exist.test/keys")
@@ -938,10 +720,8 @@ mod tests {
         assert!(msg.contains("resolve"), "message should explain: {msg}");
     }
 
-    /// `PinnedResolver` with an EMPTY address set — the state `agent_for`
-    /// would build if `resolve_and_guard` somehow returned no addresses —
-    /// must fail the resolve step with `HostNotFound` rather than silently
-    /// falling back to live DNS.
+    // An EMPTY pinned address set must fail the resolve step with
+    // `HostNotFound` rather than silently falling back to live DNS.
     #[test]
     fn pinned_resolver_with_no_addresses_fails_the_connection() {
         let result = hardened_agent(Vec::new())
@@ -966,24 +746,9 @@ mod tests {
         assert_eq!(addrs[0].port(), 8080);
     }
 
-    // ── the pin is actually consulted ──────────────────────────────────────
-    //
-    // `resolve_and_guard` VALIDATES addresses; `hardened_agent` PINS them, so a
-    // hostname cannot re-resolve to an internal address between validation and
-    // connection. The validation half is well covered above — but every one of
-    // those tests still passes if the pinned resolver is never consulted,
-    // because none of them makes a connection. Nothing else in this file proves
-    // the agent honours the pin, and a mis-wired resolver fails OPEN: the
-    // connection quietly falls back to live DNS and the rebinding window this
-    // module exists to close is reopened, with no visible symptom.
-    //
-    // The discriminator: pin the agent to a loopback listener this test owns,
-    // then ask it for a host that CANNOT resolve — `.test` is reserved by
-    // RFC 6761 and never resolves in the public DNS. Only a consulted resolver
-    // can turn that
-    // name into a connection; a fallback to live DNS fails instead. No network
-    // is touched, and `query` (whose guard blocks loopback by design) is not
-    // involved — this drives `hardened_agent` directly.
+    // The pin is actually consulted (a mis-wired resolver fails OPEN to
+    // live DNS with no symptom — see docs/online-ssrf-guard.md): pin to a
+    // loopback listener, then ask for a `.test` host that CANNOT resolve.
     #[test]
     fn hardened_agent_connects_to_the_pinned_address_not_dns() {
         use std::io::Write as _;
@@ -1068,13 +833,9 @@ mod tests {
         assert!(validate_keyserver_url("https://8.8.8.8/keys").is_ok());
     }
 
-    // ── the reply → verdict mapping (THE defect) ───────────────────────────
-    //
-    // Driven through `interpret_reply`, the seam `query` hands its round-trip
-    // to. A stub HTTP server is NOT usable here: `resolve_and_guard` blocks
-    // loopback by design (SSRF guard), so 127.0.0.1 can never be reached through
-    // `query`, and pointing the test at a public host would make it a network
-    // test. Everything from the response onward is this function.
+    // ── the reply → verdict mapping (THE defect) ──────────────────────────
+    // Driven through `interpret_reply` directly: a stub HTTP server is NOT
+    // usable here since `resolve_and_guard` blocks loopback by design.
 
     /// A `ResolveCtx` that carries nothing — enough for the reply paths that do
     /// not derive from a VUK.
@@ -1110,10 +871,9 @@ mod tests {
             .expect("stub response")
     }
 
-    /// THE regression. The key service returned HTTP 502 for ~seven hours and
-    /// the CLI told operators the disc had no key, so they went hunting for a
-    /// VUK that was never missing. A 5xx must be a source FAILURE, and it must
-    /// be distinguishable from the service answering 200 with no entry.
+    // THE regression: a 5xx for ~seven hours was reported as "no key" and
+    // sent operators hunting a VUK that was never missing. A 5xx must be a
+    // source FAILURE, distinguishable from a 200 with no entry.
     #[test]
     fn http_5xx_is_a_source_failure_not_a_missing_key() {
         for status in [500u16, 502, 503, 504] {
@@ -1139,10 +899,8 @@ mod tests {
         );
     }
 
-    /// Each status the service can return maps to a DIFFERENT operator action:
-    /// fix the token (401/403), back off (429), wait (5xx). A non-2xx status is
-    /// never evidence about this disc, so anything else is "unavailable" too —
-    /// guessing "no key" from a status code is the original bug.
+    // Each status maps to a DIFFERENT operator action: fix the token
+    // (401/403), back off (429), wait (5xx) — never "no key" from a status.
     #[test]
     fn http_status_maps_to_the_operator_action() {
         let cases: &[(u16, u16)] = &[
@@ -1168,10 +926,9 @@ mod tests {
         }
     }
 
-    /// A transport failure — nothing answered at all — is the same verdict as a
-    /// 5xx and equally not a missing key. Uses a refused connection to
-    /// 127.0.0.1:1 to obtain a REAL transport-class error; this touches no
-    /// network and never reaches `query`, so the SSRF guard is not involved.
+    // A transport failure is the same verdict as a 5xx. Uses a refused
+    // connection to 127.0.0.1:1 for a REAL transport-class error; never
+    // reaches `query`, so the SSRF guard is not involved.
     #[test]
     fn transport_failure_is_a_source_failure() {
         let config = Config::builder()
@@ -1180,11 +937,9 @@ mod tests {
         let sent = ureq::Agent::new_with_config(config)
             .post("http://127.0.0.1:1/")
             .send("{}");
-        // ureq 3 split v2's single `Transport` variant across `Io`,
-        // `ConnectionFailed`, `Timeout` and friends, and which one a refused
-        // connection produces is a platform detail. The claim that matters is
-        // unchanged and is what this asserts: it failed, and NOT with a status
-        // code — nothing on the other end ever answered.
+        // Which transport variant a refused connection produces is a
+        // platform detail; what matters (asserted below) is that it failed,
+        // and NOT with a status code — nothing on the other end answered.
         assert!(
             !matches!(sent, Ok(_) | Err(ureq::Error::StatusCode(_))),
             "a refused connection must fail as transport, never as an HTTP status"
@@ -1260,10 +1015,8 @@ mod tests {
         assert_eq!(keys[0].key[0], 0x00);
     }
 
-    /// A malformed `"UK"` STRING (not the array form) parses to nothing and
-    /// falls through to the genuine-miss path — proven distinct from the
-    /// array form's "reject the whole reply" behaviour, since a bad scalar
-    /// string has no partial set to protect.
+    // A malformed `"UK"` STRING falls through to the genuine-miss path,
+    // distinct from the array form's "reject the whole reply" behaviour.
     #[test]
     fn uk_as_an_unparseable_string_falls_through_to_a_miss() {
         let keys = interpret_reply(Ok(reply(200, r#"{"UK":"not hex"}"#)), &BareCtx, 1)
@@ -1369,25 +1122,9 @@ mod tests {
         }
     }
 
-    // ── The pre-flight guards in `query` (nothing leaves the process) ───────
-    //
-    // Each of the three guards below fires BEFORE any address is resolved and
-    // before anything is sent. None was exercised: a reordering that let the
-    // cleartext POST through would have shipped green, and that POST carries the
-    // bearer token plus base64 key material.
-    //
-    // Their VERDICTS differ on purpose (see the comment above the MKB cap in
-    // `query`): the cleartext-scheme guard is an operator fault and yields
-    // `Err`, while the two input-shaped guards yield `Ok(empty)` — this source
-    // has nothing for THIS disc, permanently, and calling that an outage would
-    // send the operator waiting for nothing.
-    //
-    // The discriminator in all three: the configured host is `.test`, which
-    // RFC 6761 guarantees never resolves. On the guarded path nothing resolves
-    // it, so the test touches no network and returns `Ok(empty)`. Remove the
-    // guard and control reaches `resolve_and_guard`, which fails as
-    // `Unreachable` → `Err(KeyServiceUnavailable)` — a different Result, so the
-    // mutation cannot pass.
+    // ── The pre-flight guards in `query` (nothing leaves the process) ──────
+    // See docs/online-preflight-guard-tests.md — why each guard's verdict
+    // differs and why `.test` is the discriminator host used below.
 
     /// A `ResolveCtx` whose MKB size and sample COUNT are dialled per guard.
     struct GuardCtx {
@@ -1415,20 +1152,9 @@ mod tests {
         }
     }
 
-    /// An `http://` key-service URL must never be POSTed to: the body carries
-    /// base64 AACS key material and the header carries a replayable bearer
-    /// token. The source refuses — and, because refusing means the service was
-    /// never asked, says so with `Err` rather than an empty that reads as "this
-    /// disc has no key". A later source's real key still wins
-    /// (`MultiSource::first_non_empty`), so refusing does not fail a rip the
-    /// local keydb can serve.
-    ///
-    /// Catches two mutations: sending the request anyway (the host is `.test`
-    /// and would surface as `Unreachable`, but the assertion below would still
-    /// hold, which is why `too_few_samples_skips_the_request` guards the
-    /// send-path ordering) and — the one this test was rewritten for — returning
-    /// `Ok(Vec::new())`, which made a permanent cleartext misconfiguration
-    /// indistinguishable from a genuine miss.
+    // An `http://` key-service URL must never be POSTed to; the source
+    // refuses with `Err`, not an empty that reads as "no key" — see
+    // docs/online-query-guards.md.
     #[test]
     fn cleartext_http_url_is_refused_before_anything_is_sent() {
         let src = OnlineSource::new("http://keyserver.test/keys", "s3cr3t");
@@ -1481,20 +1207,13 @@ mod tests {
         );
     }
 
-    // ── MAX_RESPONSE_BYTES: the stated anti-OOM defence ─────────────────────
-
-    /// The reply cap is the crate's only defence against a hostile or broken
-    /// key service driving the client to OOM with an unbounded body, and it had
-    /// never been exercised. Both edges are asserted so the `+1` read cannot
-    /// quietly become a truncating read (which would hand a half-body to the
-    /// JSON parser) or an off-by-one that rejects a legal reply.
+    // ── MAX_RESPONSE_BYTES: the anti-OOM defence, both edges asserted so
+    // `+1` can't quietly truncate, nor an off-by-one reject a legal reply.
     #[test]
     fn over_cap_reply_is_rejected_and_an_at_cap_reply_still_parses() {
-        // The over-cap body is deliberately VALID, key-bearing JSON. A body of
-        // junk would be rejected by the JSON parser whether or not the cap
-        // exists, so the test would pass with the cap deleted — worthless. This
-        // one is only rejectable BY the cap: remove the bounded read and it
-        // parses into a key.
+        // The over-cap body is deliberately VALID, key-bearing JSON — junk
+        // would be rejected by the parser regardless of the cap. This one is
+        // only rejectable BY the cap.
         let head = r#"{"UK":["000102030405060708090a0b0c0d0e0f"],"pad":""#;
         let tail = r#""}"#;
         let over = format!(
@@ -1521,12 +1240,8 @@ mod tests {
 
     // ── A bad port is CONFIG, not an outage ────────────────────────────────
 
-    /// `https://example.com:notaport/keys` is a typo in the operator's config.
-    /// The unparseable port used to make the WHOLE authority (port text
-    /// included) the hostname, which of course never resolved — so a standing
-    /// misconfiguration was reported as `Unreachable`, i.e. as the key service
-    /// being down. The two demand OPPOSITE actions (fix the URL vs. wait), which
-    /// is precisely what `GuardFail`'s doc says must not be collapsed.
+    // A typo'd port must be `Config`, not `Unreachable` — see
+    // docs/online-guardfail.md for why collapsing the two is the bug.
     #[test]
     fn unparseable_port_is_a_config_fault_not_an_outage() {
         for url in [
@@ -1545,15 +1260,9 @@ mod tests {
         assert_eq!(addrs[0].port(), 8443);
     }
 
-    /// The same distinction as seen by a caller — and the CRITICAL half of it:
-    /// separating a config fault from an outage must never make the config fault
-    /// quieter. A mistyped port means the service was never asked, so the caller
-    /// gets `Err`, exactly like an outage; only the log text differs.
-    ///
-    /// Catches the mutation that returns `Ok(Vec::new())` from the
-    /// `GuardFail::Config` arm of `query` (which is what this code did, and is
-    /// how a permanent misconfiguration became an `E7022 no key for this disc`
-    /// with no `Err` for `MultiSource::first_non_empty` to surface).
+    // The caller-visible half: a mistyped port must get `Err`, exactly like
+    // an outage (only the log text differs) — catches the `Ok(Vec::new())`
+    // regression from `GuardFail::Config` (docs/online-guardfail.md).
     #[test]
     fn query_with_a_mistyped_port_reports_a_failure_not_a_miss() {
         let src = OnlineSource::new("https://example.com:notaport/keys", "s3cr3t");
@@ -1569,14 +1278,9 @@ mod tests {
         );
     }
 
-    /// A guard-BLOCKED address (the SSRF/metadata case) travels the same
-    /// `GuardFail::Config` arm and must be just as loud: the POST never left the
-    /// process, so nothing is known about this disc. Distinct from the mistyped
-    /// port above because it fails AFTER resolution, on the address check.
-    ///
-    /// Catches the mutation that makes only the parse-level config faults `Err`
-    /// while letting the resolved-to-private-address case fall back to
-    /// `Ok(empty)`.
+    // A guard-BLOCKED address travels the same `GuardFail::Config` arm and
+    // must be just as loud — distinct from the mistyped port above because
+    // it fails AFTER resolution, on the address check.
     #[test]
     fn query_against_a_guard_blocked_address_reports_a_failure_not_a_miss() {
         // 127.0.0.1 needs no DNS and is unconditionally rejected by is_blocked_ip.
@@ -1593,9 +1297,8 @@ mod tests {
         );
     }
 
-    /// `get_fmts_indexes` shares `query` with `get_unit_keys` — never
-    /// separately exercised. Same guard-blocked, no-network path as the
-    /// `get_unit_keys` tests above proves it is wired up.
+    // `get_fmts_indexes` shares `query` with `get_unit_keys` — same
+    // guard-blocked, no-network path proves it is wired up.
     #[test]
     fn get_fmts_indexes_shares_the_same_query_path() {
         let src = OnlineSource::new("https://127.0.0.1/keys", "s3cr3t");
@@ -1611,10 +1314,8 @@ mod tests {
         );
     }
 
-    /// A host that genuinely does not resolve travels `query`'s OWN
-    /// `GuardFail::Unreachable` arm (distinct from the `Config` arm covered
-    /// above) — still `Err`, since "did not resolve" is what a key-service
-    /// outage looks like from here, never a genuine miss.
+    // A host that does not resolve travels `query`'s OWN `Unreachable` arm
+    // (distinct from `Config` above) — still `Err`, never a genuine miss.
     #[test]
     fn query_against_an_unresolvable_host_reports_unreachable_as_a_failure() {
         let src = OnlineSource::new("https://this-host-does-not-exist.test/keys", "s3cr3t");
@@ -1630,11 +1331,9 @@ mod tests {
         );
     }
 
-    /// The request body's `vid_b64` and `title` fields are assembled BEFORE
-    /// the address guard runs (see `query`'s ordering), so a ctx that
-    /// supplies a VID and a title still exercises that assembly even when
-    /// the guard then rejects the address — proven through the same
-    /// deterministic guard-blocked path the tests above use (no network).
+    // `vid_b64`/`title` are assembled BEFORE the address guard runs, so a
+    // VID+title ctx still exercises that assembly even when the guard then
+    // rejects the address (same deterministic guard-blocked path, no network).
     #[test]
     fn query_assembles_vid_and_title_before_the_address_guard_runs() {
         struct VidTitleCtx;
@@ -1700,19 +1399,9 @@ mod tests {
         assert!(src.get_unit_keys(&WhitespaceTitleCtx).is_err());
     }
 
-    // ── One agent per address set, not one per query ────────────────────────
-
-    /// A round-robin keyserver returns the SAME addresses in a different order
-    /// on each lookup. That is the same validated set, so it must reuse the
-    /// agent — an ordered `==` on the cache key missed on every single query
-    /// against exactly the deployment the cache was written for, silently
-    /// reverting to a fresh TLS handshake per request.
-    ///
-    /// Catches the mutation that drops the sort/dedup from `agent_for`'s key
-    /// (the reversed order would then build a second agent) and, via the second
-    /// half, the mutation that "fixes" order-sensitivity by comparing only
-    /// lengths or the first element — which would wrongly reuse an agent pinned
-    /// to a DIFFERENT address.
+    // ── One agent per address set, not one per query ──────────────────────
+    // A round-robin keyserver reorders the SAME addresses; still the same
+    // set, so it must reuse the agent — see docs/online-agent-cache.md.
     #[test]
     fn a_reordered_but_identical_address_set_reuses_the_agent() {
         let src = OnlineSource::new("https://keyserver.test/keys", "");
@@ -1742,12 +1431,9 @@ mod tests {
         );
     }
 
-    /// An FMTS disc calls `query` twice per rip (`get_unit_keys` +
-    /// `get_fmts_indexes`), and a fresh `ureq::Agent` per call throws away the
-    /// connection pool with it — a second full TLS handshake to the same host
-    /// for nothing. The agent is reused only while the freshly guarded address
-    /// set is IDENTICAL, so the anti-rebinding guarantee is untouched: a
-    /// different address set builds (and re-pins) a new agent.
+    // An FMTS disc calls `query` twice per rip; the agent is reused only
+    // while the freshly guarded address set is IDENTICAL — see
+    // docs/online-agent-cache.md.
     #[test]
     fn the_agent_is_reused_per_address_set_only() {
         let src = OnlineSource::new("https://keyserver.test/keys", "");

src/paths.rs

@@ -1,16 +1,12 @@
 //! Where the `keydb.cfg` lives: local to the executable, local ONLY.
 //!
-//! Key-path policy belongs with the key sources (this crate), not the library:
-//! libfreemkv is handed a path and reads it. The CLI/app asks here for the
-//! list of locations to *search* (first existing wins) and for the single
-//! *default* location to write to (e.g. `update-keys`/save).
+//! Key-path policy belongs here (key sources), not in libfreemkv, which
+//! just reads a handed-in path. This module gives the *search* list (first
+//! existing wins) and the single *default* write location.
 //!
-//! freemkv is a portable, standalone binary: the `keydb.cfg` lives *next to*
-//! the executable — `<dir of current exe>/keydb.cfg` — and nowhere else. There
-//! is no OS-specific config-dir lookup (`%APPDATA%`, `%USERPROFILE%\.config`,
-//! `$XDG_CONFIG_HOME`, `$HOME/.config`). Drop the exe and its `keydb.cfg` in
-//! the same folder and it works. Callers needing a custom location pass
-//! `--keydb PATH`, which bypasses this module entirely.
+//! freemkv is portable: `keydb.cfg` lives next to the exe —
+//! `<dir of current exe>/keydb.cfg` — with no OS config-dir fallback.
+//! `--keydb PATH` bypasses this module. See docs/paths.md for rationale.
 
 use std::path::PathBuf;
 

tests/key_sources.rs

@@ -1,15 +1,12 @@
 //! Fixture-based integration tests for the published key sources.
 //!
 //! These exercise the *public* surface of `freemkv-keysources` end-to-end —
-//! real files on disk, the real `KeyDb` parser from libfreemkv, and
-//! the `KeySource` trait (`get_unit_keys` over a `ResolveCtx`) the applications drive.
-//!
-//! Covered:
-//! - `KeydbSource`: terminal unit-key lookup by disc hash through a real
-//!   `keydb.cfg`; the exe-local path helpers; MKB-aware host-cert serving.
-//! - `OnlineSource`: SSRF/scheme validation and the unconfigured no-op.
-//! - `MultiSource`: caller-supplied ordering / precedence, host-cert UNION,
-//!   and nesting.
+//! real files on disk, the real `KeyDb` parser from libfreemkv, and the
+//! `KeySource` trait (`get_unit_keys` over a `ResolveCtx`) the applications
+//! drive. Covered: `KeydbSource` terminal unit-key lookup by disc hash
+//! (plus exe-local path helpers and MKB-aware host-cert serving);
+//! `OnlineSource` SSRF/scheme validation and the unconfigured no-op; and
+//! `MultiSource` ordering/precedence, host-cert UNION, and nesting.
 
 use std::io::Write;
 use std::path::{Path, PathBuf};
@@ -372,12 +369,8 @@ fn multi_source_real_keydb_resolves_through_chain() {
 }
 
 // ── MultiSource: a source that could not ANSWER is not a source that said no ─
-//
-// `MultiSource` re-collapsed the Ok/Err distinction `OnlineSource` was
-// engineered to draw: `if let Ok(uks) = s.get_unit_keys(ctx)` discarded every
-// `Err`, so a composition whose sources ALL failed returned `Ok(Vec::new())` —
-// which the resolver reports as `E7022 No key source has a decryption key for
-// this disc`. That is the seven-hour-502 incident reproduced one layer up.
+// See docs/multi-source-failure-collapse.md — the Ok/Err collapse incident
+// this section's tests exist to catch.
 
 /// A source that always fails, with a chosen error — the key-service outage.
 struct FailingSource {
@@ -405,10 +398,8 @@ fn unauthorized() -> libfreemkv::Error {
     libfreemkv::Error::KeyServiceUnauthorized
 }
 
-/// THE regression, one layer up from `interpret_reply`: when no source holds a
-/// key AND a source could not answer, the composition must report the FAILURE,
-/// never a clean "no key". Catches the mutation that restores
-/// `if let Ok(uks) = ..` (which drops the `Err` and returns `Ok(empty)`).
+// See docs/multi-source-failure-collapse.md — must report FAILURE, not a
+// clean "no key", when no source holds a key and one could not answer.
 #[test]
 fn multi_source_reports_a_source_failure_instead_of_a_clean_no_key() {
     let multi = MultiSource::new(vec![