lower MSRV to 1.90, trim two stray comments in aacs handshake

Bisected the true floor: 1.85 fails to compile (needs is_multiple_of,
stabilized 1.87, and let-chains, stabilized 1.88), 1.90 builds/clippy/tests
clean. Repin Cargo.toml rust-version and every CI dtolnay/rust-toolchain to
1.90.0, and note the change in CHANGELOG.md. Also removed a redundant
"// Hash the data" comment and a stale duplicate doc line on
generate_host_key_pair_p256 found during a pass over the crate's comments.

.github/dependabot.yml

@@ -0,0 +1,63 @@
+version: 2
+
+# Dependency updates land on `dev`, never on `main`.
+#
+# `main` here is a RELEASE POINTER that release.sh moves to each tag. A bot
+# commit on it would put work there that no tag contains, which is exactly the
+# state that aborted the 1.6.2 cascade at the last step -- so pointing
+# Dependabot at main would recreate that failure on a schedule.
+updates:
+  - package-ecosystem: cargo
+    directory: /
+    target-branch: dev
+    schedule:
+      interval: weekly
+    open-pull-requests-limit: 5
+    # One PR per week for the routine bumps instead of one per crate. Eight
+    # repos times a handful of crates is a volume nobody reads, and an
+    # unread PR queue is indistinguishable from no updates at all.
+    groups:
+      minor-and-patch:
+        update-types:
+          - minor
+          - patch
+    ignore:
+      # The freemkv crates depend on each other by GIT TAG, re-pinned by
+      # release.sh as part of the release commit. Dependabot cannot see that
+      # cascade, so a PR bumping one of these would fight the release process
+      # and could pin a version whose tag does not exist yet.
+      - dependency-name: freemkv-unlock
+      - dependency-name: libfreemkv
+      - dependency-name: freemkv-keysources
+      - dependency-name: freemkv-i18n
+      - dependency-name: freemkv-engine
+
+  # The workflows are now real infrastructure -- the release cascade, the
+  # cross-platform hash matrix, the disc gate -- so their actions need the same
+  # attention as the crates.
+  - package-ecosystem: github-actions
+    directory: /
+    target-branch: dev
+    schedule:
+      interval: weekly
+    open-pull-requests-limit: 5
+    groups:
+      actions:
+        update-types:
+          - minor
+          - patch
+    ignore:
+      # NOT a dependency: `dtolnay/rust-toolchain` is versioned by the RUST
+      # release it installs, and the tag we pin is the toolchain CI is pinned
+      # to on purpose -- precommit.sh runs the same one locally so a lint that
+      # passes on a developer's newer default cannot pass CI by accident.
+      #
+      # Dependabot reads those tags as semver and proposed 1.97.0 -> 1.100.0,
+      # a Rust version that does not exist. Every such PR 404s on toolchain
+      # download across all eight repos, and they regenerate weekly -- eight
+      # permanently-red PRs that promote.yml then has to special-case when it
+      # decides whether dev is green.
+      #
+      # Bumping the toolchain is a deliberate, all-eight-repos change, made by
+      # hand together with precommit.sh. There is nothing here for a bot.
+      - dependency-name: dtolnay/rust-toolchain

.github/workflows/ci.yml

@@ -21,11 +21,37 @@ on:
 # NOT used: with no committed lockfile it fails rather than resolves.
 
 jobs:
+  # 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
+  # forgot its changelog entry must fail here rather than ship a blank release.
+  # Hard-fails on main (and PRs targeting main); warns elsewhere so an
+  # in-progress dev/qa branch whose entry isn't written yet is not blocked.
+  changelog:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v7
+      - name: Require a CHANGELOG entry for the crate version on main
+        shell: bash
+        run: |
+          set -euo pipefail
+          version=$(grep -m1 '^version = ' Cargo.toml | sed -E 's/.*"([^"]+)".*/\1/')
+          if grep -qE "^## \[${version}\]" CHANGELOG.md; then
+            echo "CHANGELOG.md has an entry for ${version}."
+            exit 0
+          fi
+          msg="CHANGELOG.md has no '## [${version}]' entry"
+          if [ "${GITHUB_REF_NAME:-}" = "main" ] || [ "${GITHUB_BASE_REF:-}" = "main" ]; then
+            echo "::error::${msg} — main requires a changelog entry for the released version."
+            exit 1
+          fi
+          echo "::warning::${msg} (allowed on ${GITHUB_REF_NAME:-unknown}; required before it can reach main)."
+
   lint:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v5
-      - uses: dtolnay/rust-toolchain@1.97.0
+      - uses: actions/checkout@v7
+      - uses: dtolnay/rust-toolchain@1.90.0
         with:
           components: clippy, rustfmt
       - uses: Swatinem/rust-cache@v2
@@ -38,8 +64,8 @@ jobs:
   test:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v5
-      - uses: dtolnay/rust-toolchain@1.97.0
+      - uses: actions/checkout@v7
+      - uses: dtolnay/rust-toolchain@1.90.0
       - uses: Swatinem/rust-cache@v2
         with:
           shared-key: freemkv-unlock-ci
@@ -52,20 +78,46 @@ jobs:
   # compile there. Build the tests (not just `cargo check`) — that forces
   # codegen and catches errors in test-only code, which `check` skips.
   check-macos:
+    # dev is the FAST lane: this job still runs, but on the release-candidate
+    # branches rather than on every push to dev. Nothing is deleted and no
+    # platform stops being checked before a release -- qa.yml independently
+    # covers macOS and Windows, and the jobs unique to this file (the Intel
+    # macOS build, the Windows release build) run here on qa and main. A push
+    # to dev is meant to be cheap and frequent; waiting on three runner pools
+    # to agree is what a release candidate is for.
+    #
+    # `if` SKIPS the job (it does not queue). A queued job would be far worse
+    # 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.
+    if: github.ref_name == 'qa' || github.ref_name == 'main'
     runs-on: macos-latest
     steps:
-      - uses: actions/checkout@v5
-      - uses: dtolnay/rust-toolchain@1.97.0
+      - uses: actions/checkout@v7
+      - uses: dtolnay/rust-toolchain@1.90.0
       - uses: Swatinem/rust-cache@v2
         with:
           shared-key: freemkv-unlock-ci
       - run: cargo build --tests --all-features
 
   check-windows:
+    # dev is the FAST lane: this job still runs, but on the release-candidate
+    # branches rather than on every push to dev. Nothing is deleted and no
+    # platform stops being checked before a release -- qa.yml independently
+    # covers macOS and Windows, and the jobs unique to this file (the Intel
+    # macOS build, the Windows release build) run here on qa and main. A push
+    # to dev is meant to be cheap and frequent; waiting on three runner pools
+    # to agree is what a release candidate is for.
+    #
+    # `if` SKIPS the job (it does not queue). A queued job would be far worse
+    # 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.
+    if: github.ref_name == 'qa' || github.ref_name == 'main'
     runs-on: windows-latest
     steps:
-      - uses: actions/checkout@v5
-      - uses: dtolnay/rust-toolchain@1.97.0
+      - uses: actions/checkout@v7
+      - uses: dtolnay/rust-toolchain@1.90.0
       - uses: Swatinem/rust-cache@v2
         with:
           shared-key: freemkv-unlock-ci

.github/workflows/leak-guard.yml

@@ -11,7 +11,7 @@ jobs:
   leak-guard:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v5
+      - uses: actions/checkout@v7
         with:
           fetch-depth: 0
       - name: Compute commit range

.github/workflows/qa.yml

@@ -43,7 +43,7 @@ jobs:
     permissions:
       contents: write
     steps:
-      - uses: actions/checkout@v5
+      - uses: actions/checkout@v7
         with:
           fetch-depth: 0
       - name: Stamp the next rc
@@ -71,10 +71,10 @@ jobs:
         os: ['ubuntu-latest', 'macos-latest']
     runs-on: ${{ matrix.os }}
     steps:
-      - uses: actions/checkout@v5
+      - uses: actions/checkout@v7
         with:
           path: freemkv-unlock
-      - uses: dtolnay/rust-toolchain@1.97.0
+      - uses: dtolnay/rust-toolchain@1.90.0
       - uses: Swatinem/rust-cache@v2
         with:
           workspaces: freemkv-unlock
@@ -87,10 +87,10 @@ jobs:
   cross-lint:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v5
+      - uses: actions/checkout@v7
         with:
           path: freemkv-unlock
-      - uses: dtolnay/rust-toolchain@1.97.0
+      - uses: dtolnay/rust-toolchain@1.90.0
         with:
           components: clippy
       - uses: Swatinem/rust-cache@v2
@@ -107,10 +107,10 @@ jobs:
   windows-build:
     runs-on: windows-latest
     steps:
-      - uses: actions/checkout@v5
+      - uses: actions/checkout@v7
         with:
           path: freemkv-unlock
-      - uses: dtolnay/rust-toolchain@1.97.0
+      - uses: dtolnay/rust-toolchain@1.90.0
       - uses: Swatinem/rust-cache@v2
         with:
           workspaces: freemkv-unlock

CHANGELOG.md

@@ -1,5 +1,153 @@
 # Changelog
 
+## [1.6.10] — 2026-08-23
+
+### Changed
+
+- Version aligned to 1.6.10 for the unified release. No functional changes to
+  this crate; the release was driven by libfreemkv (TrueHD/MLP audio now resyncs
+  to the next major-sync access unit after a source transport-stream
+  discontinuity, instead of splicing post-gap audio mid-stream — fixing
+  decoder-choking seams on discs whose stream carries a continuity-counter gap;
+  see the libfreemkv 1.6.10 notes).
+- MSRV lowered from 1.97 to 1.90. The crate does not use anything newer than
+  1.90 stabilized; CI's pinned toolchain and `rust-version` are updated to
+  match.
+
+## [1.6.9] — 2026-08-22
+
+### Changed
+
+- Version aligned to 1.6.9 for the unified release. No functional changes to
+  this crate; the release was driven by autorip (automatic per-episode TV
+  ripping — each episode named `S{NN}E{MM}`, with TMDB runtime-aligned episode
+  numbering across multi-disc seasons — a Manual Rename option, and a unified
+  per-disc staging state file — see the autorip 1.6.9 notes).
+
+## [1.6.8] — 2026-08-21
+
+### Changed
+
+- Version aligned to 1.6.8 for the unified release. No functional changes to
+  this crate; the release was driven by autorip (webhooks now fire per pipeline
+  stage — Rip / Mux / Move — with the Rip hook firing the moment the drive is
+  free again, plus a Ripper-tab activity-banner fix so it also shows during
+  moves — see the autorip 1.6.8 notes).
+
+## [1.6.7] — 2026-08-21
+
+### Changed
+
+- Version aligned to 1.6.7 for the unified release. No functional changes to
+  this crate; the release was driven by autorip (per-webhook event selection,
+  a progress bar per moved artifact, and move-queue / webhook-error fixes —
+  see the autorip 1.6.7 notes).
+
+## [1.6.6] — 2026-08-20
+
+### Changed
+
+- Version aligned to 1.6.6 for the unified release. No functional changes
+  to this crate; the release was driven by autorip (webhooks may now target
+  private/LAN addresses — see the autorip 1.6.6 notes).
+
+## [1.6.5] — 2026-08-20
+
+### Added
+
+- **AACS 2.0 (P-256) drives now complete a live host-certificate
+  handshake.** The elliptic-curve (P-256) authentication path is wired
+  into the cert handshake for real, not left as dead code. A drive is
+  first tried with the AACS 1.0 certificate (a 2.0 drive accepts it for
+  the Volume ID); if the drive refuses it and the supplied host
+  certificate carries AACS 2.0 credentials, the crate falls back to the
+  native P-256 handshake. It is cert-agnostic — it uses whatever the host
+  certificate carries, with no separate feature gate — and a bus fault on
+  the 1.0 attempt short-circuits the fallback so a dead bus is not
+  hammered. The 2.0 path enforces the same chain-of-trust checks as the
+  1.0 path (drive-cert type gate, fatal certificate verification under the
+  real 2.0 Licensing Administrator key, on-curve validation of every
+  attacker-supplied point, and AGID release on every exit). The 2.0
+  certificate byte layout is documented as provisional pending a genuine
+  drive certificate to pin it against; a wrong layout fails closed and
+  never trusts.
+
+### Security
+
+- **A rogue drive or bridge can no longer forge its way to a trusted bus
+  key.** A drive certificate whose type was neither AACS 1.0 nor 2.0 used
+  to skip Licensing Administrator verification yet still have its own key
+  trusted for the bus-key signature check, letting an attacker inject a
+  chosen bus key; unknown cert types are now rejected at the type gate
+  before any key is trusted. ECDSA verification now validates the signer's
+  public key (on-curve, in-field, not the identity point) before use — a
+  key of (0,0) previously collapsed verification to a forgeable form. The
+  compiled-in AACS 1.0 and 2.0 Licensing Administrator anchor keys, which
+  were off-curve, are replaced with the real published keys (both
+  confirmed on-curve; the 1.0 key verifies a genuine LA-signed
+  certificate). The P-256 certificate-length check now matches the real
+  132-byte layout, so AACS 2.0 certificates verify at all.
+
+- **Sensitive material no longer leaks through debug output.** `Unlocked`
+  (which carries the bus key and Volume ID) and `DriveProfile` (which
+  carries the MediaTek drive firmware image and per-drive vendor SCSI
+  templates) now use hand-written redacting `Debug` implementations
+  instead of printing everything. The AACS 2.0 authentication path now
+  releases the drive AGID on every failure exit, matching the 1.0 path.
+
+### Fixed
+
+- **A dead or failing drive bus is no longer reported as a successful
+  unlock.** A transport fault during the AACS data-key read, during
+  firmware-unlock disc probing, and in both MediaTek speed-probe passes
+  used to be swallowed or flattened into a generic failure — in the
+  firmware-unlock case surfacing as `drive_unlocked: true` on a bus that
+  had actually died. These paths now abort with a transport error so the
+  fault is visible instead of masquerading as clear content.
+
+- **Transport faults are no longer misclassified as "wrong drive" or
+  "bad credentials."** A dead bus on the first probe command previously
+  read as "not a Renesas drive" or "not a supported DVD profile"; a bus
+  that died mid-handshake was reported as a credentials rejection after
+  three retries against dead hardware; and CSS bus-auth collapsed every
+  fault into a generic auth failure. Each call site now honours the
+  transport contract — abort on a real bus fault, fall through only on a
+  genuine "not applicable." A merely short probe response is no longer
+  fabricated into a transport failure, so rips that would have succeeded
+  are no longer aborted.
+
+- **A half-unlocked drive now falls through to certificate
+  authentication instead of claiming the bus is clear.** The firmware
+  unlocker set `drive_unlocked: true` whenever the handshake merely
+  completed, ignoring the computed unlock flag; a drive returning zero
+  bytes could even report a full unlock from a buffer it never wrote.
+  The unlock outcome is now actually consulted, so a partial unlock
+  correctly steers to the cert-auth fallback. Related read paths that
+  parsed the caller's own zero-filled buffer as drive data on a CHECK
+  CONDITION are fixed to inspect status and bytes transferred.
+
+### Changed
+
+- **Drive preparation issues 88 fewer SCSI commands per disc.** The
+  probe's two passes previously walked the same 88 addresses twice; the
+  redundant pass is removed.
+
+## [1.6.4] — 2026-08-15
+
+### Changed
+
+- **No functional change.** This crate ships alongside the rest of freemkv at a
+  matching version; its behaviour is untouched.
+
+## [1.6.3] — 2026-08-10
+
+### Changed
+
+- **Housekeeping only — nothing about how a drive is unlocked has changed.** The
+  cryptography and compression crates this crate builds on were moved to their
+  current releases and brought into line with the versions the rest of freemkv
+  uses, so one version of each is built instead of two.
+
 ## [1.6.2] — 2026-08-08
 
 Version sync with the workspace. No functional change in this crate.

CONTRIBUTING.md

@@ -0,0 +1,18 @@
+# Contributing
+
+Thanks for your interest!
+
+- **Report a bug** — open an issue
+- **Fix a bug** — fork, branch, PR
+- **Suggest a feature** — open an issue first
+
+## Development
+
+```bash
+cargo build
+cargo test
+```
+
+## License
+
+MIT

Cargo.toml

@@ -1,8 +1,8 @@
 [package]
 name = "freemkv-unlock"
-version = "1.6.1"
+version = "1.6.9"
 edition = "2024"
-rust-version = "1.97"
+rust-version = "1.90"
 license = "MIT"
 description = "Unlock layer for the freemkv toolchain: the Unlocker contract + self-contained firmware/AACS/CSS unlocker modules. libfreemkv depends on this and dispatches via all_unlockers()."
 repository = "https://github.com/freemkv/freemkv-unlock"
@@ -18,11 +18,11 @@ emulation = []
 [dependencies]
 serde = { version = "1", features = ["derive"] }
 serde_json = "1"
-base64 = "0.22.1"
+base64 = "0.23"
 tracing = "0.1"
-num-bigint = "0.4"
+num-bigint = "0.5"
 num-traits = "0.2"
 sha1 = "0.10"
-sha2 = "0.10"
-aes = "0.8"
-rand = "0.8"
+sha2 = "0.11"
+aes = "0.9"
+rand = "0.10"

SECURITY.md

@@ -0,0 +1,22 @@
+# Security Policy
+
+## Supported versions
+
+| Version | Supported |
+| ------- | --------- |
+| 1.6.x   | Yes       |
+| < 1.6   | No        |
+
+Only the current 1.6.x line receives security fixes.
+
+## Reporting a vulnerability
+
+Report vulnerabilities privately through GitHub Security Advisories:
+https://github.com/freemkv/freemkv-unlock/security/advisories/new
+
+Do not open a public issue for a security report. Include the affected
+version, steps to reproduce, and the impact you believe the issue has.
+
+## Response time
+
+You will get an initial response within 7 days.

src/aacs/error.rs

@@ -25,6 +25,15 @@ pub enum Error {
     AacsVidRead,
     HandshakeRejected,
     VidUnavailable,
+    /// A handshake step returned FEWER bytes than the protocol step requires.
+    /// The buffer the step would have parsed is the caller's own zero fill, so
+    /// consuming it would turn a failed command into a "successful" handshake
+    /// step built entirely out of zeros.
+    ShortTransfer {
+        opcode: u8,
+        expected: usize,
+        got: usize,
+    },
     /// A SCSI command failed. `status == SCSI_STATUS_TRANSPORT_FAILURE` with
     /// `sense: None` is a transport-layer fault; a CHECK CONDITION carries the
     /// parsed [`ScsiSense`].
@@ -54,6 +63,7 @@ impl Error {
             Error::AacsVidRead => 7012,
             Error::HandshakeRejected => 7013,
             Error::VidUnavailable => 7014,
+            Error::ShortTransfer { .. } => 7015,
             Error::Scsi { .. } => 7099,
         }
     }

src/aacs/mod.rs

@@ -9,7 +9,7 @@ mod error;
 mod handshake;
 
 use aes::Aes128;
-use aes::cipher::{BlockDecrypt, KeyInit, generic_array::GenericArray};
+use aes::cipher::{Array, BlockCipherDecrypt, KeyInit};
 
 use crate::scsi::ScsiTransport;
 use crate::{DiscKind, UnlockCtx, UnlockError, Unlocked, Unlocker};
@@ -17,8 +17,8 @@ use crate::{DiscKind, UnlockCtx, UnlockError, Unlocked, Unlocker};
 /// AES-128-ECB decrypt a single 16-byte block — used to decrypt the bus key /
 /// read_data_key the drive returns after the handshake.
 pub(crate) fn aes_ecb_decrypt(key: &[u8; 16], data: &[u8; 16]) -> [u8; 16] {
-    let cipher = Aes128::new(GenericArray::from_slice(key));
-    let mut block = GenericArray::clone_from_slice(data);
+    let cipher = Aes128::new(&(*key).into());
+    let mut block: Array<u8, _> = (*data).into();
     cipher.decrypt_block(&mut block);
     let mut out = [0u8; 16];
     out.copy_from_slice(&block);

src/css/error.rs

@@ -19,6 +19,31 @@ impl Error {
             Error::Scsi(_) => 7299,
         }
     }
+
+    /// True if this is a transport-layer SCSI failure (bus dead).
+    pub fn is_transport_failure(&self) -> bool {
+        matches!(
+            self,
+            Error::Scsi(s)
+                if s.status == crate::scsi::SCSI_STATUS_TRANSPORT_FAILURE && s.sense.is_none()
+        )
+    }
+}
+
+/// Collapse a SCSI-layer failure from a bus-auth step onto the CSS auth code —
+/// but NEVER a transport fault.
+///
+/// Every step used to do `.map_err(|_| Error::CssAuthFailed)?`, which discarded
+/// the `ScsiError` whole. That made the `Transport` arm of the conversion below
+/// UNREACHABLE from any real call site: a dead bus arrived at the consumer as
+/// `NotApplicable`, so it fell through and kept probing a bus that was gone.
+pub fn step_err(e: crate::scsi::ScsiError) -> Error {
+    let err = Error::Scsi(e);
+    if err.is_transport_failure() {
+        err
+    } else {
+        Error::CssAuthFailed
+    }
 }
 
 impl From<crate::scsi::ScsiError> for Error {
@@ -31,11 +56,37 @@ impl From<crate::scsi::ScsiError> for Error {
 /// "this unlocker didn't apply" (a transport fault still surfaces as Transport).
 impl From<Error> for crate::UnlockError {
     fn from(e: Error) -> Self {
-        match e {
-            Error::Scsi(s) if s.status == crate::scsi::SCSI_STATUS_TRANSPORT_FAILURE => {
-                crate::UnlockError::Transport
-            }
-            _ => crate::UnlockError::NotApplicable,
+        if e.is_transport_failure() {
+            crate::UnlockError::Transport
+        } else {
+            crate::UnlockError::NotApplicable
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::UnlockError;
+    use crate::scsi::{SCSI_STATUS_CHECK_CONDITION, SCSI_STATUS_TRANSPORT_FAILURE, ScsiError};
+
+    /// Catches restoring the `.map_err(|_| CssAuthFailed)` that made the
+    /// `Transport` arm unreachable: a dead bus must classify as `Transport`
+    /// (consumer aborts), a drive rejection as `NotApplicable` (fall through).
+    #[test]
+    fn transport_fault_survives_the_step_mapping() {
+        let dead = step_err(ScsiError {
+            status: SCSI_STATUS_TRANSPORT_FAILURE,
+            sense: None,
+        });
+        assert!(dead.is_transport_failure());
+        assert_eq!(UnlockError::from(dead), UnlockError::Transport);
+
+        let refused = step_err(ScsiError {
+            status: SCSI_STATUS_CHECK_CONDITION,
+            sense: Some([0u8; 32]),
+        });
+        assert!(matches!(refused, Error::CssAuthFailed));
+        assert_eq!(UnlockError::from(refused), UnlockError::NotApplicable);
+    }
+}

src/css/mod.rs

@@ -6,11 +6,54 @@
 //! actually opens scrambled-sector reads), then a best-effort, non-fatal
 //! disc-key REPORT KEY. The bytes are NOT used as keys: the descramble title
 //! key is recovered keylessly by the Stevenson known-plaintext attack (see
-//! [`super::crack_key`]).
+//! libfreemkv's `crack_key`).
 
 mod error;
-use crate::css::error::{Error, Result};
-use crate::scsi::ScsiTransport;
+use crate::css::error::{Error, Result, step_err};
+use crate::scsi::{DataDirection, ScsiTransport};
+
+/// Issue ONE CSS bus-auth CDB, honouring the transport contract.
+///
+/// The single seam every bus-auth step now goes through. It closes two holes
+/// that each step open-coded before:
+///   * `Err` was collapsed to `CssAuthFailed`, so a dead bus looked like "this
+///     unlocker doesn't apply" and the consumer kept probing it ([`step_err`]);
+///   * `Ok` was never inspected, but per the contract a CHECK CONDITION arrives
+///     as `Ok` with a non-zero `status` — so a refused command handed the step
+///     back its own zero-filled buffer to parse as drive data.
+///
+/// `min_bytes` is the number of bytes the step must actually receive (0 for a
+/// write / no-data command, whose transferred count no transport promises).
+fn css_scsi(
+    scsi: &mut dyn ScsiTransport,
+    cdb: &[u8],
+    dir: DataDirection,
+    buf: &mut [u8],
+    min_bytes: usize,
+) -> Result<()> {
+    let r = scsi.execute(cdb, dir, buf, 5_000).map_err(step_err)?;
+    if r.status != 0 {
+        tracing::debug!(
+            target: "freemkv::css",
+            phase = "css_step_check_condition",
+            opcode = cdb.first().copied().unwrap_or(0),
+            status = r.status,
+            "CSS bus-auth step returned a drive sense"
+        );
+        return Err(Error::CssAuthFailed);
+    }
+    if r.bytes_transferred < min_bytes {
+        tracing::debug!(
+            target: "freemkv::css",
+            phase = "css_step_short_response",
+            opcode = cdb.first().copied().unwrap_or(0),
+            bytes_transferred = r.bytes_transferred,
+            "CSS bus-auth step returned a short response"
+        );
+        return Err(Error::CssAuthFailed);
+    }
+    Ok(())
+}
 
 // ── CryptKey tables ───────────────────────────────────────────────────────
 
@@ -124,7 +167,7 @@ const PERM_VARIANT: [[u8; 32]; 2] = [
 /// what actually unlocks scrambled-sector reads), then a best-effort,
 /// non-fatal disc-key REPORT KEY. The title-key REPORT KEY is NOT issued: it
 /// is unnecessary (the descramble key is recovered keylessly by the Stevenson
-/// attack in [`super::crack_key`]) and its hard failure on some USB bridges
+/// attack in libfreemkv's `crack_key`) and its hard failure on some USB bridges
 /// used to abort the whole unlock (the 7014 bug). The bytes are discarded.
 pub fn unlock_css_reads(scsi: &mut dyn ScsiTransport, lba: u32) -> Result<()> {
     let t0 = std::time::Instant::now();
@@ -187,7 +230,7 @@ impl crate::Unlocker for DvdUnlocker {
         // DiscKind alone. If the drive does not report a DVD profile, refuse
         // (NotApplicable) WITHOUT issuing any CSS CDB, so a mis-routed
         // Blu-ray/UHD is never sent CSS bus-auth.
-        if !mounted_disc_is_dvd(scsi) {
+        if !mounted_disc_is_dvd(scsi)? {
             tracing::debug!(
                 target: "freemkv::css",
                 phase = "dvd_unlocker_not_dvd",
@@ -206,7 +249,14 @@ impl crate::Unlocker for DvdUnlocker {
 /// Transport-level "is the mounted disc a DVD?" probe (GET CONFIGURATION
 /// current-profile, DVD family `0x0010..=0x001F`). Lets the DvdUnlocker
 /// self-verify against the drive instead of trusting the caller's DiscKind.
-fn mounted_disc_is_dvd(scsi: &mut dyn ScsiTransport) -> bool {
+///
+/// `Err(Transport)` on a dead bus. This is the FIRST command the DVD unlocker
+/// issues, and its match had no `Err` arm at all: a transport fault fell into
+/// `_ => false`, which the caller turned into `NotApplicable` — a dead bus read
+/// as "not a DVD", with nothing logged anywhere on the path.
+fn mounted_disc_is_dvd(
+    scsi: &mut dyn ScsiTransport,
+) -> std::result::Result<bool, crate::UnlockError> {
     // RT=0: the 8-byte feature header carries the Current Profile in bytes 6-7.
     let cdb = [
         crate::scsi::SCSI_GET_CONFIGURATION,
@@ -221,17 +271,34 @@ fn mounted_disc_is_dvd(scsi: &mut dyn ScsiTransport) -> bool {
         0x00,
     ];
     let mut buf = [0u8; 8];
-    match scsi.execute(
-        &cdb,
-        crate::scsi::DataDirection::FromDevice,
-        &mut buf,
-        5_000,
-    ) {
-        Ok(r) if r.bytes_transferred >= 8 => {
+    match scsi.execute(&cdb, DataDirection::FromDevice, &mut buf, 5_000) {
+        Ok(r) if r.status == 0 && r.bytes_transferred >= 8 => {
             let profile = ((buf[6] as u16) << 8) | buf[7] as u16;
-            (0x0010..=0x001F).contains(&profile)
+            Ok((0x0010..=0x001F).contains(&profile))
+        }
+        // A drive sense or a short reply genuinely means "can't tell / not a
+        // DVD" — decline, but say so.
+        Ok(r) => {
+            tracing::debug!(
+                target: "freemkv::css",
+                phase = "dvd_profile_probe_inconclusive",
+                status = r.status,
+                bytes_transferred = r.bytes_transferred,
+                "GET CONFIGURATION did not report a current profile"
+            );
+            Ok(false)
+        }
+        Err(e) => {
+            let err = crate::css::error::step_err(e);
+            tracing::warn!(
+                target: "freemkv::css",
+                phase = "dvd_profile_probe_failed",
+                error_code = err.code(),
+                transport_failure = err.is_transport_failure(),
+                "GET CONFIGURATION failed while probing for a DVD"
+            );
+            Err(crate::UnlockError::from(err))
         }
-        _ => false,
     }
 }
 
@@ -268,39 +335,53 @@ fn unlock_css_reads_inner(scsi: &mut dyn ScsiTransport, _lba: u32) -> Result<()>
 /// derived: descrambling is keyless (the Stevenson known-plaintext attack), so
 /// the bus key has no consumer.
 fn establish_authenticated_session(scsi: &mut dyn ScsiTransport) -> Result<u8> {
-    // Invalidate all AGIDs via REPORT KEY format 0x3F
+    // Invalidate all AGIDs via REPORT KEY format 0x3F. This is also what makes
+    // an abandoned AGID self-heal — which is not a reason to abandon one.
     for agid in 0..4u8 {
-        let mut cdb = [0u8; 12];
-        cdb[0] = crate::scsi::SCSI_REPORT_KEY;
-        // alloc_len = 0 (no data transfer)
-        cdb[10] = (agid << 6) | 0x3F;
-        let mut buf = [0u8; 8];
-        let _ = scsi.execute(
-            &cdb,
-            crate::scsi::DataDirection::FromDevice,
-            &mut buf,
-            5_000,
-        );
+        release_agid(scsi, agid);
     }
 
     // Allocate AGID
     let mut buf = [0u8; 8];
-    scsi.execute(
+    css_scsi(
+        scsi,
         &report_key_cdb(0, 0x00, 8),
-        crate::scsi::DataDirection::FromDevice,
+        DataDirection::FromDevice,
         &mut buf,
-        5_000,
-    )
-    .map_err(|_| Error::CssAuthFailed)?;
+        8,
+    )?;
     let agid = (buf[7] >> 6) & 0x03;
 
+    // From here on we hold the AGID; release it on any failure (see
+    // [`release_agid`]) instead of abandoning it.
+    let r = authenticate_with_agid(scsi, agid);
+    if r.is_err() {
+        release_agid(scsi, agid);
+    }
+    r.map(|()| agid)
+}
+
+/// Release an AGID (REPORT KEY format 0x3F). Best-effort: a failure to release
+/// is not a failure of the operation that is already failing.
+fn release_agid(scsi: &mut dyn ScsiTransport, agid: u8) {
+    let mut cdb = [0u8; 12];
+    cdb[0] = crate::scsi::SCSI_REPORT_KEY;
+    cdb[10] = (agid << 6) | 0x3F;
+    let mut buf = [0u8; 8];
+    let _ = scsi.execute(&cdb, DataDirection::FromDevice, &mut buf, 5_000);
+}
+
+/// The challenge-response half of [`establish_authenticated_session`], with the
+/// AGID already allocated. Split out so its caller can release the AGID on any
+/// failure without a Drop guard or a release at each early return.
+fn authenticate_with_agid(scsi: &mut dyn ScsiTransport, agid: u8) -> Result<()> {
     // Host sends challenge. The spec wants a fresh per-session random nonce,
     // not a fixed constant — a predictable challenge weakens the bus-auth
     // handshake.
     let mut host_challenge = [0u8; 10];
     {
-        use rand::RngCore;
-        rand::thread_rng().fill_bytes(&mut host_challenge);
+        use rand::Rng;
+        rand::rng().fill_bytes(&mut host_challenge);
     }
     let mut hc_buf = [0u8; 16];
     hc_buf[0] = 0x00;
@@ -308,23 +389,23 @@ fn establish_authenticated_session(scsi: &mut dyn ScsiTransport) -> Result<u8> {
     for i in 0..10 {
         hc_buf[4 + i] = host_challenge[9 - i];
     }
-    scsi.execute(
+    css_scsi(
+        scsi,
         &send_key_cdb(agid, 0x01, 16),
-        crate::scsi::DataDirection::ToDevice,
+        DataDirection::ToDevice,
         &mut hc_buf,
-        5_000,
-    )
-    .map_err(|_| Error::CssAuthFailed)?;
+        0,
+    )?;
 
     // Get Key1 from drive
     let mut dk_buf = [0u8; 12];
-    scsi.execute(
+    css_scsi(
+        scsi,
         &report_key_cdb(agid, 0x02, 12),
-        crate::scsi::DataDirection::FromDevice,
+        DataDirection::FromDevice,
         &mut dk_buf,
-        5_000,
-    )
-    .map_err(|_| Error::CssAuthFailed)?;
+        12,
+    )?;
     let mut key1 = [0u8; 5];
     for i in 0..5 {
         key1[i] = dk_buf[4 + (4 - i)];
@@ -342,13 +423,13 @@ fn establish_authenticated_session(scsi: &mut dyn ScsiTransport) -> Result<u8> {
 
     // Get drive challenge
     let mut dc_buf = [0u8; 16];
-    scsi.execute(
+    css_scsi(
+        scsi,
         &report_key_cdb(agid, 0x01, 16),
-        crate::scsi::DataDirection::FromDevice,
+        DataDirection::FromDevice,
         &mut dc_buf,
-        5_000,
-    )
-    .map_err(|_| Error::CssAuthFailed)?;
+        16,
+    )?;
     let mut drive_challenge = [0u8; 10];
     for i in 0..10 {
         drive_challenge[i] = dc_buf[4 + (9 - i)];
@@ -362,25 +443,26 @@ fn establish_authenticated_session(scsi: &mut dyn ScsiTransport) -> Result<u8> {
     for i in 0..5 {
         hk_buf[4 + i] = key2[4 - i];
     }
-    scsi.execute(
+    css_scsi(
+        scsi,
         &send_key_cdb(agid, 0x03, 12),
-        crate::scsi::DataDirection::ToDevice,
+        DataDirection::ToDevice,
         &mut hk_buf,
-        5_000,
-    )
-    .map_err(|_| Error::CssAuthFailed)?;
+        0,
+    )?;
 
     // The authenticated session (ASF=1) is now established — scrambled-sector
     // reads are unlocked, which is the only thing we needed. The CSS bus key
     // would be CryptKey(2, variant, key1 || key2), but it has no consumer
     // (descrambling is keyless via the Stevenson attack), so it is not derived.
-    Ok(agid)
+    Ok(())
 }
 
 // ── Step 2: Disc Key ──────────────────────────────────────────────────────
 
-/// Issue READ DVD STRUCTURE format 0x02 (Copyright Information — opcode 0xAD,
-/// NOT the REPORT KEY 0xA4 disc-key block) purely for the bus-auth unlock side
+/// Issue READ DVD STRUCTURE format 0x02 (the Disc Key block — opcode 0xAD, NOT
+/// the REPORT KEY 0xA4 disc-key block; format 0x01 is Copyright Information)
+/// purely for the bus-auth unlock side
 /// effect. The returned block contents are not used — the descramble title key
 /// is recovered keylessly elsewhere, so the genuine disc-key REPORT KEY is
 /// intentionally skipped. (If a drive is ever found where bus-auth alone does
@@ -397,14 +479,10 @@ fn read_disc_key(scsi: &mut dyn ScsiTransport, agid: u8) -> Result<()> {
     cdb[9] = alloc_len as u8;
     cdb[10] = agid << 6;
 
+    // Best-effort by design (the caller logs and continues), but a transport
+    // fault still has to be distinguishable from a drive that declined.
     let mut buf = vec![0u8; alloc_len as usize];
-    let dvd_result = scsi.execute(
-        &cdb,
-        crate::scsi::DataDirection::FromDevice,
-        &mut buf,
-        5_000,
-    );
-    dvd_result.map_err(|_| Error::CssAuthFailed)?;
+    css_scsi(scsi, &cdb, DataDirection::FromDevice, &mut buf, 0)?;
 
     Ok(())
 }
@@ -1026,4 +1104,125 @@ mod tests {
             "no CSS CDB may be issued at a non-DVD drive"
         );
     }
+    // ── Transport-contract tests ────────────────────────────────────────────
+
+    use crate::scsi::mock::{MockTransport, Reply};
+    use crate::{DiscKind, DriveId, UnlockCtx, UnlockError, Unlocker};
+
+    /// THE defect-7 test. `mounted_disc_is_dvd` is the FIRST command the DVD
+    /// unlocker issues and its match had no `Err` arm: a transport fault fell
+    /// into `_ => false`, so a dead bus was reported as "not a DVD"
+    /// (`NotApplicable`) and the consumer kept probing it — with nothing logged
+    /// anywhere on the path. Catches restoring that catch-all arm.
+    #[test]
+    fn transport_fault_probing_for_a_dvd_aborts() {
+        let id = DriveId::default();
+        let mut t = MockTransport::always(Reply::TransportFault);
+        let r = DvdUnlocker::new().unlock_bus(&mut t, &UnlockCtx::new(&id, DiscKind::Css, &[]));
+        assert_eq!(r.unwrap_err(), UnlockError::Transport);
+        assert_eq!(
+            t.calls(),
+            1,
+            "must abort on the first command, not probe on"
+        );
+    }
+
+    /// A drive that REFUSES GET CONFIGURATION (CHECK CONDITION, delivered as
+    /// `Ok` per the contract) is inconclusive, not a dead bus → decline. Guards
+    /// against over-correcting defect 7 into "any probe failure aborts the rip".
+    #[test]
+    fn check_condition_probing_for_a_dvd_declines() {
+        let id = DriveId::default();
+        let mut t = MockTransport::always(Reply::illegal_request());
+        let r = DvdUnlocker::new().unlock_bus(&mut t, &UnlockCtx::new(&id, DiscKind::Css, &[]));
+        assert_eq!(r.unwrap_err(), UnlockError::NotApplicable);
+    }
+
+    /// A DVD is mounted, then the bus dies during bus-auth. THE defect-2 test:
+    /// every CSS step used to do `.map_err(|_| CssAuthFailed)`, discarding the
+    /// `ScsiError` and making the `Transport` arm of the error conversion
+    /// unreachable from any real call site — so a dead bus arrived as
+    /// `NotApplicable` and the consumer went on probing it.
+    #[test]
+    fn transport_fault_during_bus_auth_aborts() {
+        let id = DriveId::default();
+        // GET CONFIGURATION reports a DVD-ROM profile (0x0010), then the bus dies.
+        let mut config = vec![0u8; 8];
+        config[6] = 0x00;
+        config[7] = 0x10;
+        let mut t = MockTransport::scripted(vec![Reply::good(config)], Reply::TransportFault);
+        let r = DvdUnlocker::new().unlock_bus(&mut t, &UnlockCtx::new(&id, DiscKind::Css, &[]));
+        assert_eq!(r.unwrap_err(), UnlockError::Transport);
+    }
+
+    /// The same shape with the drive REFUSING the bus-auth commands: a CSS
+    /// auth failure is a fall-through, not an abort.
+    #[test]
+    fn drive_refusing_bus_auth_is_not_applicable() {
+        let id = DriveId::default();
+        let mut config = vec![0u8; 8];
+        config[7] = 0x10;
+        let mut t = MockTransport::scripted(vec![Reply::good(config)], Reply::illegal_request());
+        let r = DvdUnlocker::new().unlock_bus(&mut t, &UnlockCtx::new(&id, DiscKind::Css, &[]));
+        assert_eq!(r.unwrap_err(), UnlockError::NotApplicable);
+    }
+
+    /// A conforming transport answering the AGID allocation with a CHECK
+    /// CONDITION must NOT let the handshake carry on off the caller's own
+    /// zero-filled buffer (agid 0, a challenge answered by zeros). Catches
+    /// dropping the `status` check in `css_scsi`.
+    #[test]
+    fn agid_allocation_check_condition_fails_the_auth() {
+        // 4 AGID invalidations are best-effort; the 5th command is the alloc.
+        let mut t = MockTransport::scripted(
+            vec![
+                Reply::good(vec![0u8; 8]),
+                Reply::good(vec![0u8; 8]),
+                Reply::good(vec![0u8; 8]),
+                Reply::good(vec![0u8; 8]),
+            ],
+            Reply::illegal_request(),
+        );
+        let e = establish_authenticated_session(&mut t).expect_err("refused alloc");
+        assert!(matches!(e, Error::CssAuthFailed));
+    }
+
+    /// Defect 18: an AGID allocated and then lost to a failed challenge must be
+    /// RELEASED (REPORT KEY format 0x3F), not abandoned. A drive has four; the
+    /// leak self-heals because the next session invalidates all of them, but
+    /// leaving a drive short between attempts is a state we should not create.
+    #[test]
+    fn a_failed_handshake_releases_the_agid_it_allocated() {
+        let mut t = MockTransport::scripted(
+            vec![
+                Reply::good(vec![0u8; 8]), // 4 × AGID invalidate
+                Reply::good(vec![0u8; 8]),
+                Reply::good(vec![0u8; 8]),
+                Reply::good(vec![0u8; 8]),
+                Reply::good(vec![0u8; 8]), // AGID allocated
+            ],
+            Reply::illegal_request(), // every challenge step is refused
+        );
+        establish_authenticated_session(&mut t).expect_err("refused challenge");
+        let last = t.cdbs.last().expect("commands were issued");
+        assert_eq!(last[0], crate::scsi::SCSI_REPORT_KEY);
+        assert_eq!(last[10] & 0x3F, 0x3F, "AGID released on the failure path");
+    }
+
+    /// A short AGID-allocation response is equally unusable — the AGID would be
+    /// read out of bytes the drive never sent.
+    #[test]
+    fn short_agid_allocation_fails_the_auth() {
+        let mut t = MockTransport::scripted(
+            vec![
+                Reply::good(vec![0u8; 8]),
+                Reply::good(vec![0u8; 8]),
+                Reply::good(vec![0u8; 8]),
+                Reply::good(vec![0u8; 8]),
+            ],
+            Reply::short(vec![0u8; 8], 3),
+        );
+        let e = establish_authenticated_session(&mut t).expect_err("short alloc");
+        assert!(matches!(e, Error::CssAuthFailed));
+    }
 }

src/ld/error.rs

@@ -39,6 +39,17 @@ impl std::fmt::Display for Error {
 impl std::error::Error for Error {}
 
 impl Error {
+    /// Stable numeric code (logged). Values are local to this crate and share
+    /// the crate-wide space with `aacs` (70xx) and `css` (72xx).
+    pub fn code(&self) -> u16 {
+        match self {
+            Error::ProfileParse => 7101,
+            Error::UnlockFailed => 7102,
+            Error::SignatureMismatch { .. } => 7103,
+            Error::Scsi { .. } => 7199,
+        }
+    }
+
     /// True if this is a transport-layer SCSI failure (bus dead), as opposed to a
     /// drive sense or a logical failure.
     pub(crate) fn is_transport_failure(&self) -> bool {
@@ -74,3 +85,95 @@ impl From<Error> for crate::UnlockError {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::UnlockError;
+    use crate::scsi::{SCSI_STATUS_CHECK_CONDITION, SCSI_STATUS_TRANSPORT_FAILURE, ScsiError};
+
+    /// The abort/fall-through boundary, which had no test at all. Catches
+    /// widening `is_transport_failure` (every failure would abort every rip) or
+    /// narrowing it (a dead bus would be probed forever).
+    #[test]
+    fn only_a_senseless_transport_status_is_a_transport_failure() {
+        let dead_bus = Error::Scsi {
+            opcode: 0x3C,
+            status: SCSI_STATUS_TRANSPORT_FAILURE,
+            sense: None,
+        };
+        assert!(dead_bus.is_transport_failure());
+        assert_eq!(UnlockError::from(dead_bus), UnlockError::Transport);
+
+        // A CHECK CONDITION is the DRIVE refusing, not the bus dying.
+        let refused = Error::Scsi {
+            opcode: 0x3C,
+            status: SCSI_STATUS_CHECK_CONDITION,
+            sense: Some([0u8; 32]),
+        };
+        assert!(!refused.is_transport_failure());
+        assert_eq!(UnlockError::from(refused), UnlockError::NotApplicable);
+
+        // The transport-failure status WITH a sense is a drive that reported
+        // 0xFF, not a bus fault — it must not abort the consumer.
+        let odd = Error::Scsi {
+            opcode: 0x3C,
+            status: SCSI_STATUS_TRANSPORT_FAILURE,
+            sense: Some([0u8; 32]),
+        };
+        assert!(!odd.is_transport_failure());
+
+        for e in [
+            Error::ProfileParse,
+            Error::UnlockFailed,
+            Error::SignatureMismatch {
+                expected: [0; 4],
+                got: [1; 4],
+            },
+        ] {
+            assert!(!e.is_transport_failure(), "{e:?} is not a bus fault");
+            assert_eq!(UnlockError::from(e), UnlockError::NotApplicable);
+        }
+    }
+
+    /// `From<ScsiError>` must carry the status AND sense across the seam —
+    /// collapsing either one destroys the transport-vs-rejection distinction the
+    /// whole abort/fall-through split rests on.
+    #[test]
+    fn scsi_error_conversion_preserves_status_and_sense() {
+        let e = Error::from(ScsiError {
+            status: SCSI_STATUS_TRANSPORT_FAILURE,
+            sense: None,
+        });
+        assert!(e.is_transport_failure());
+
+        let e = Error::from(ScsiError {
+            status: SCSI_STATUS_CHECK_CONDITION,
+            sense: Some([0u8; 32]),
+        });
+        assert!(!e.is_transport_failure());
+        assert_eq!(UnlockError::from(e), UnlockError::NotApplicable);
+    }
+
+    /// Error codes are stable and distinct — they are the numeric contract the
+    /// logs carry instead of English.
+    #[test]
+    fn error_codes_are_distinct_and_stable() {
+        let codes = [
+            Error::ProfileParse.code(),
+            Error::UnlockFailed.code(),
+            Error::SignatureMismatch {
+                expected: [0; 4],
+                got: [0; 4],
+            }
+            .code(),
+            Error::Scsi {
+                opcode: 0,
+                status: 0,
+                sense: None,
+            }
+            .code(),
+        ];
+        assert_eq!(codes, [7101, 7102, 7103, 7199]);
+    }
+}

src/ld/mod.rs

@@ -10,7 +10,10 @@
 // needs (the real unlocker drives its CDBs from per-drive profile templates, not
 // these constants). Compile it only when the `emulation` feature exposes it, so
 // it never dead-codes in a normal build.
-#[cfg(feature = "emulation")]
+// Compiled under `cfg(test)` as well as the feature: its tests pin the unlock
+// wire format, and gating the whole module on a non-default feature meant CI
+// (which builds with default features) never ran them.
+#[cfg(any(feature = "emulation", test))]
 mod cdb;
 mod error;
 mod platform;
@@ -75,23 +78,42 @@ pub(crate) fn firmware_name(id: &DriveId) -> Option<&'static str> {
 impl LibreDrive {
     /// Read the OEM Volume ID via the matched profile's vendor CDB.
     ///
+    /// Takes the profile the caller ALREADY matched rather than re-running
+    /// `find_bundled`: the catalog is 206 entries and drive-prep used to scan it
+    /// twice per drive (once here, once in `firmware_unlock`) for the same
+    /// answer.
+    ///
     /// `Ok(Some(vid))` on a well-formed 36-byte response (signature `00 22 00`,
     /// VID at `[4..20]`); `Ok(None)` when there is no OEM-VID CDB or the response
-    /// is short / bad-signature (the drive is still unlocked, just no VID); `Err`
-    /// only on a transport fault.
-    fn read_oem_vid(&self, scsi: &mut dyn ScsiTransport, id: &DriveId) -> Result<Option<[u8; 16]>> {
+    /// is short / bad-signature / drive-rejected (the drive is still unlocked,
+    /// just no VID); `Err` only on a transport fault.
+    fn read_oem_vid(
+        &self,
+        scsi: &mut dyn ScsiTransport,
+        profile: &profile::DriveProfile,
+    ) -> Result<Option<[u8; 16]>> {
         const RESPONSE_LEN: usize = 36;
         const EXPECTED_HEADER: [u8; 3] = [0x00, 0x22, 0x00];
 
-        let Some(m) = profile::find_bundled(id) else {
-            return Ok(None);
-        };
-        let Some(cdb) = m.profile.read_vid_cdb else {
+        let Some(cdb) = profile.read_vid_cdb else {
             return Ok(None);
         };
 
         let mut buf = vec![0u8; RESPONSE_LEN];
         let result = scsi.execute(&cdb, DataDirection::FromDevice, &mut buf, 5_000)?;
+        // Per the transport contract a drive sense arrives as `Ok` with a
+        // non-zero `status`, NOT as `Err`. Without this check a CHECK CONDITION
+        // would be read as a successful 36-byte response and the caller's own
+        // zero fill parsed as a Volume ID.
+        if result.status != 0 {
+            tracing::warn!(
+                target: "freemkv::disc",
+                phase = "oem_vid_check_condition",
+                status = result.status,
+                "OEM VID CDB returned a drive sense"
+            );
+            return Ok(None);
+        }
         if result.bytes_transferred < RESPONSE_LEN {
             tracing::warn!(
                 target: "freemkv::disc",
@@ -141,13 +163,55 @@ impl LibreDrive {
         }
         let is_variant_b = matches!(m.platform, profile::Platform::Mt1959B);
         use platform::PlatformDriver;
-        let mut mt = platform::mt1959::Mt1959::new(m.profile, is_variant_b);
+        let mut mt = platform::mt1959::Mt1959::new(m.profile.clone(), is_variant_b);
         // A transport fault → UnlockError::Transport; any other firmware failure
         // → NotApplicable (via From<error::Error>).
         mt.init(scsi)?;
-        // Prime the per-region speed table (best-effort — must not fail the unlock).
-        let _ = mt.probe_disc(scsi);
-        let vid = self.read_oem_vid(scsi, id)?;
+        // `init` only proves the handshake COMPLETED — `do_unlock` returns Ok on
+        // a response that carried the per-drive signature but not both firmware
+        // markers. Only `is_unlocked()` means the drive actually reached the
+        // extended-access state and serves clear content. Reporting
+        // `drive_unlocked: true` off `init` alone told the consumer the bus was
+        // clear on a half-unlocked drive, which suppressed the cert-auth
+        // fallback and shipped ciphertext at rc=0. A partial unlock must fall
+        // through to the next unlocker.
+        if !mt.is_unlocked() {
+            tracing::warn!(
+                target: "freemkv::disc",
+                phase = "firmware_unlock_incomplete",
+                "firmware handshake completed but the drive is not in the extended-access state; falling through"
+            );
+            return Err(UnlockError::NotApplicable);
+        }
+        // Prime the per-region speed table. Best-effort — it must not fail the
+        // unlock — but NOT silent: speed calibration can fail completely, and an
+        // unlogged `let _ =` made a fully-failed calibration indistinguishable
+        // from a successful one in the rip log.
+        if let Err(e) = mt.probe_disc(scsi) {
+            // A transport fault here is a DEAD BUS, not a speed-calibration miss.
+            // The rest of this path (`read_oem_vid`) is a no-op for the 140/206
+            // profiles that carry no `read_vid_cdb`, so it never touches the bus
+            // again — meaning this swallowed fault was the ONLY dead-bus signal,
+            // and warn-and-continue turned it into `Ok(Unlocked{drive_unlocked:
+            // true})`: a dead bus rendered as a fully-unlocked drive (the flagship
+            // failure-that-looks-like-success). A genuine calibration miss (drive
+            // sense / short reply) still continues on the default speed table.
+            if e.is_transport_failure() {
+                tracing::warn!(
+                    target: "freemkv::disc",
+                    phase = "probe_disc_transport_fault",
+                    "transport fault during disc speed calibration; aborting"
+                );
+                return Err(UnlockError::Transport);
+            }
+            tracing::warn!(
+                target: "freemkv::disc",
+                phase = "probe_disc_failed",
+                transport_failure = false,
+                "disc speed calibration failed; continuing with the drive's default speed table"
+            );
+        }
+        let vid = self.read_oem_vid(scsi, &m.profile)?;
         Ok(Unlocked {
             vid,
             bus_key: None,
@@ -235,16 +299,21 @@ mod tests {
         }
     }
 
-    /// A well-formed 36-byte response (signature 00 22 00, VID at [4..20]) parses
-    /// to `Some(vid)`.
-    #[test]
-    fn read_oem_vid_parses_well_formed_response() {
+    /// The bundled profile of the fixture drive (the one carrying a real
+    /// `read_vid_cdb`).
+    fn known_vid_profile() -> profile::DriveProfile {
         let m = profile::find_bundled(&known_vid_drive_id()).expect("profile match");
         assert!(
             m.profile.read_vid_cdb.is_some(),
             "test fixture drive must carry an OEM VID CDB"
         );
+        m.profile
+    }
 
+    /// A well-formed 36-byte response (signature 00 22 00, VID at [4..20]) parses
+    /// to `Some(vid)`.
+    #[test]
+    fn read_oem_vid_parses_well_formed_response() {
         let mut payload = vec![0u8; 36];
         payload[0..3].copy_from_slice(&[0x00, 0x22, 0x00]);
         let vid = [0x3Cu8; 16];
@@ -254,7 +323,7 @@ mod tests {
             bytes_transferred: 36,
         };
         let got = LibreDrive::new()
-            .read_oem_vid(&mut t, &known_vid_drive_id())
+            .read_oem_vid(&mut t, &known_vid_profile())
             .expect("parse ok");
         assert_eq!(got, Some(vid), "VID parsed from [4..20]");
     }
@@ -267,7 +336,7 @@ mod tests {
             bytes_transferred: 20,
         };
         let got = LibreDrive::new()
-            .read_oem_vid(&mut t, &known_vid_drive_id())
+            .read_oem_vid(&mut t, &known_vid_profile())
             .expect("short response is Ok(None)");
         assert_eq!(got, None);
     }
@@ -282,21 +351,49 @@ mod tests {
             bytes_transferred: 36,
         };
         let got = LibreDrive::new()
-            .read_oem_vid(&mut t, &known_vid_drive_id())
+            .read_oem_vid(&mut t, &known_vid_profile())
             .expect("bad header is Ok(None)");
         assert_eq!(got, None);
     }
 
-    /// A drive with no matching profile → `read_oem_vid` is `Ok(None)`.
+    /// Catches dropping the `result.status` check: a CHECK CONDITION arrives as
+    /// `Ok` per the transport contract, so without it the caller's zero-filled
+    /// buffer is parsed as a real 36-byte response.
     #[test]
-    fn read_oem_vid_no_profile_is_none() {
+    fn read_oem_vid_check_condition_is_none_not_a_vid() {
+        use crate::scsi::mock::{MockTransport, Reply};
+        let mut t = MockTransport::always(Reply::illegal_request());
+        let got = LibreDrive::new()
+            .read_oem_vid(&mut t, &known_vid_profile())
+            .expect("a drive sense is not a transport fault");
+        assert_eq!(got, None, "a CHECK CONDITION must never yield a VID");
+    }
+
+    /// Catches swallowing a transport fault in the OEM-VID read: a dead bus must
+    /// propagate (→ `UnlockError::Transport`), never become `Ok(None)`.
+    #[test]
+    fn read_oem_vid_transport_fault_propagates() {
+        use crate::scsi::mock::{MockTransport, Reply};
+        let mut t = MockTransport::always(Reply::TransportFault);
+        let err = LibreDrive::new()
+            .read_oem_vid(&mut t, &known_vid_profile())
+            .expect_err("a dead bus must not be Ok(None)");
+        assert!(err.is_transport_failure());
+        assert_eq!(UnlockError::from(err), UnlockError::Transport);
+    }
+
+    /// A profile with no OEM-VID CDB → `Ok(None)` without touching the drive.
+    #[test]
+    fn read_oem_vid_no_cdb_is_none() {
+        let mut p = known_vid_profile();
+        p.read_vid_cdb = None;
         let mut t = FakeTransport {
             payload: vec![0u8; 36],
             bytes_transferred: 36,
         };
         let got = LibreDrive::new()
-            .read_oem_vid(&mut t, &make_drive_id("FAKE-VND", "9.99", "XX12345", ""))
-            .expect("no profile is Ok(None)");
+            .read_oem_vid(&mut t, &p)
+            .expect("no CDB is Ok(None)");
         assert_eq!(got, None);
     }
 
@@ -316,4 +413,128 @@ mod tests {
             .expect_err("no profile → NotApplicable");
         assert_eq!(err, UnlockError::NotApplicable);
     }
+
+    /// THE defect-1 test. The drive answers every unlock READ_BUFFER with a
+    /// response carrying the per-drive signature and the primary firmware marker
+    /// but NOT the secondary one — `do_unlock` returns `Ok` and `init()`
+    /// succeeds, yet the drive is NOT in the extended-access state. Reporting
+    /// `drive_unlocked: true` here suppresses the cert-auth fallback. Catches
+    /// removing the `is_unlocked()` gate in `firmware_unlock`.
+    #[test]
+    fn half_unlocked_drive_falls_through_instead_of_claiming_unlocked() {
+        use crate::scsi::mock::{MockTransport, Reply};
+        let id = known_vid_drive_id();
+        let sig = profile::find_bundled(&id)
+            .expect("profile")
+            .profile
+            .signature;
+
+        // 64-byte unlock response: signature + primary marker "MMkv" at [12..16],
+        // secondary marker at [16..20] left zeroed.
+        let mut resp = vec![0u8; 64];
+        resp[0..4].copy_from_slice(&sig);
+        resp[12..16].copy_from_slice(&[0x4D, 0x4D, 0x6B, 0x76]);
+
+        let mut t = MockTransport::always(Reply::good(resp));
+        let err = LibreDrive::new()
+            .unlock_features(&mut t, &ctx(&id))
+            .expect_err("a half-unlocked drive must fall through to cert-auth");
+        assert_eq!(err, UnlockError::NotApplicable);
+    }
+
+    /// The whole-unlock happy path still reports `drive_unlocked` when BOTH
+    /// firmware markers are present — the `is_unlocked()` gate must not have
+    /// made a genuinely unlocked drive fall through.
+    #[test]
+    fn fully_unlocked_drive_reports_drive_unlocked() {
+        use crate::scsi::mock::{MockTransport, Reply};
+        let id = known_vid_drive_id();
+        let sig = profile::find_bundled(&id)
+            .expect("profile")
+            .profile
+            .signature;
+
+        let mut resp = vec![0u8; 64];
+        resp[0..4].copy_from_slice(&sig);
+        resp[12..16].copy_from_slice(&[0x4D, 0x4D, 0x6B, 0x76]);
+        resp[16..20].copy_from_slice(&[0x4C, 0x62, 0x44, 0x72]);
+
+        let mut t = MockTransport::always(Reply::good(resp));
+        let u = LibreDrive::new()
+            .unlock_features(&mut t, &ctx(&id))
+            .expect("both markers → unlocked");
+        assert!(u.drive_unlocked);
+    }
+
+    /// THE probe-disc dead-bus test. The drive unlocks fully (both firmware
+    /// markers), then the bus DIES during disc-speed calibration. `probe_disc`
+    /// used to be warn-and-continued regardless of the fault, and — because the
+    /// 140/206 profiles without a `read_vid_cdb` never touch the bus again — a
+    /// dead bus was reported as `Ok(Unlocked{drive_unlocked:true})`: a dead bus
+    /// rendered as a successful unlock (the flagship failure-that-looks-like-
+    /// success). `firmware_unlock` must now abort with `Transport`.
+    /// MUTATION: reverting the `if e.is_transport_failure()` return in
+    /// `firmware_unlock` (warn-and-continue), OR flattening the probe loops'
+    /// transport classification, makes this go red.
+    #[test]
+    fn transport_fault_during_probe_disc_is_transport_not_a_successful_unlock() {
+        let id = known_vid_drive_id();
+        let sig = profile::find_bundled(&id)
+            .expect("profile")
+            .profile
+            .signature;
+
+        // A drive that unlocks fully but whose bus dies on the speed probe.
+        struct ProbeFaultsDrive {
+            resp: Vec<u8>,
+        }
+        impl ScsiTransport for ProbeFaultsDrive {
+            fn execute(
+                &mut self,
+                cdb: &[u8],
+                _dir: DataDirection,
+                data: &mut [u8],
+                _timeout_ms: u32,
+            ) -> crate::scsi::Result<ScsiResult> {
+                // READ_BUFFER (0x3C) / SUB_CMD_PROBE (0x14) is the speed probe.
+                if cdb.first() == Some(&0x3C) && cdb.get(3) == Some(&0x14) {
+                    return Err(crate::scsi::ScsiError {
+                        status: crate::scsi::SCSI_STATUS_TRANSPORT_FAILURE,
+                        sense: None,
+                    });
+                }
+                let n = self.resp.len().min(data.len());
+                data[..n].copy_from_slice(&self.resp[..n]);
+                Ok(ScsiResult {
+                    status: 0,
+                    bytes_transferred: n,
+                    sense: [0u8; 32],
+                })
+            }
+        }
+
+        let mut resp = vec![0u8; 64];
+        resp[0..4].copy_from_slice(&sig);
+        resp[12..16].copy_from_slice(&[0x4D, 0x4D, 0x6B, 0x76]); // primary marker
+        resp[16..20].copy_from_slice(&[0x4C, 0x62, 0x44, 0x72]); // secondary marker
+        let mut t = ProbeFaultsDrive { resp };
+
+        let err = LibreDrive::new()
+            .unlock_features(&mut t, &ctx(&id))
+            .expect_err("a dead bus during probe must abort, not report success");
+        assert_eq!(err, UnlockError::Transport);
+    }
+
+    /// Catches classifying a dead bus as "not this unlocker's drive": the very
+    /// first unlock command faulting at the transport layer must abort the
+    /// consumer (`Transport`), not fall through to the next unlocker.
+    #[test]
+    fn transport_fault_during_unlock_is_transport_not_not_applicable() {
+        use crate::scsi::mock::{MockTransport, Reply};
+        let mut t = MockTransport::always(Reply::TransportFault);
+        let err = LibreDrive::new()
+            .unlock_features(&mut t, &ctx(&known_vid_drive_id()))
+            .expect_err("dead bus");
+        assert_eq!(err, UnlockError::Transport);
+    }
 }

src/ld/platform/mod.rs

@@ -17,8 +17,10 @@ pub(crate) trait PlatformDriver: Send {
     #[allow(dead_code)]
     fn is_ready(&self) -> bool;
 
-    /// True if the drive is currently in the extended-access state.
-    #[allow(dead_code)]
+    /// True if the drive is currently in the extended-access state. THE gate
+    /// `LibreDrive::firmware_unlock` reports `drive_unlocked` from — `init()`
+    /// returning `Ok` only means the handshake completed, not that the drive
+    /// serves clear content.
     fn is_unlocked(&self) -> bool {
         false
     }

src/ld/platform/mt1959/mod.rs

@@ -37,6 +37,9 @@ const FIRMWARE_ACTIVE_SIG: [u8; 4] = [0x4D, 0x4D, 0x6B, 0x76];
 /// response, not a stale image's residual buffer.
 const FIRMWARE_MODE_OFFSET: usize = 16;
 const FIRMWARE_MODE_SIG: [u8; 4] = [0x4C, 0x62, 0x44, 0x72];
+/// Fewest bytes an unlock response must carry before ANY of its three checks
+/// mean anything — through the secondary marker at [16..20].
+const MIN_UNLOCK_RESPONSE: usize = FIRMWARE_MODE_OFFSET + 4;
 
 // ── Init address (per disc type) ──────────────────────────────────────
 const INIT_ADDR_BD: u16 = 0x0100;
@@ -123,11 +126,17 @@ impl Mt1959 {
         );
         let cdb = self.read_buffer_sub(sub_cmd, address, expected as u8);
         let result = scsi.execute(&cdb, DataDirection::FromDevice, buf, 5_000)?;
-        if result.bytes_transferred != expected {
+        // A drive sense arrives as `Ok` with a non-zero status (the transport
+        // contract), and a merely SHORT response is the drive answering badly —
+        // neither is a dead bus. Fabricating a transport-failure status here
+        // turned a short probe reply into a hard abort of a rip that would
+        // otherwise have succeeded; a real transport fault still propagates
+        // through the `?` above with its own status.
+        if result.status != 0 || result.bytes_transferred != expected {
             return Err(Error::Scsi {
                 opcode: SCSI_READ_BUFFER,
-                status: crate::scsi::SCSI_STATUS_TRANSPORT_FAILURE,
-                sense: None,
+                status: result.status,
+                sense: Some(result.sense),
             });
         }
         Ok(result.bytes_transferred)
@@ -158,23 +167,48 @@ impl Mt1959 {
         let mut response = vec![0u8; UNLOCK_RESPONSE_SIZE as usize];
         let result = scsi.execute(&cdb, DataDirection::FromDevice, &mut response, 30_000)?;
 
+        // A drive sense arrives as `Ok` with a non-zero status; treating it as
+        // a successful unlock would validate the caller's own zero fill.
+        if result.status != 0 {
+            tracing::debug!(
+                target: "freemkv::disc",
+                phase = "mt1959_unlock_check_condition",
+                status = result.status,
+                "unlock READ_BUFFER returned a drive sense"
+            );
+            return Err(Error::Scsi {
+                opcode: SCSI_READ_BUFFER,
+                status: result.status,
+                sense: Some(result.sense),
+            });
+        }
+
         // `response` is a fixed 64-byte buffer, so `response.len()` is
         // always >= every offset below — the meaningful bound is how many
-        // bytes the drive actually delivered. Validate against
-        // `bytes_transferred` so a short/partial transfer (stale trailing
-        // zeros) can't be read as if the drive sent real marker bytes.
+        // bytes the drive actually delivered. Every marker check MUST be
+        // reachable: when the checks were each guarded by their own `n >= ..`
+        // the drive delivering ZERO bytes skipped all three and `do_unlock`
+        // returned `Ok` on a buffer the drive never wrote. Require enough bytes
+        // to actually validate, up front.
         let n = result.bytes_transferred.min(response.len());
+        if n < MIN_UNLOCK_RESPONSE {
+            tracing::debug!(
+                target: "freemkv::disc",
+                phase = "mt1959_unlock_short_response",
+                bytes_transferred = result.bytes_transferred,
+                "unlock response too short to validate"
+            );
+            return Err(Error::UnlockFailed);
+        }
 
-        if n >= 4 && response[0..4] != self.profile.signature {
+        if response[0..4] != self.profile.signature {
             return Err(Error::SignatureMismatch {
                 expected: self.profile.signature,
                 got: response[0..4].try_into().unwrap_or([0; 4]),
             });
         }
 
-        if n >= FIRMWARE_ACTIVE_OFFSET + 4
-            && response[FIRMWARE_ACTIVE_OFFSET..FIRMWARE_ACTIVE_OFFSET + 4] != FIRMWARE_ACTIVE_SIG
-        {
+        if response[FIRMWARE_ACTIVE_OFFSET..FIRMWARE_ACTIVE_OFFSET + 4] != FIRMWARE_ACTIVE_SIG {
             return Err(Error::UnlockFailed);
         }
 
@@ -187,9 +221,8 @@ impl Mt1959 {
         // the rest of the response. Requiring both before we tell the
         // upper layer "OEM path is live" keeps any partial / corrupted
         // response from steering us off the cert-auth fallback.
-        self.unlocked = n >= FIRMWARE_MODE_OFFSET + 4
-            && response[FIRMWARE_ACTIVE_OFFSET..FIRMWARE_ACTIVE_OFFSET + 4] == FIRMWARE_ACTIVE_SIG
-            && response[FIRMWARE_MODE_OFFSET..FIRMWARE_MODE_OFFSET + 4] == FIRMWARE_MODE_SIG;
+        self.unlocked =
+            response[FIRMWARE_MODE_OFFSET..FIRMWARE_MODE_OFFSET + 4] == FIRMWARE_MODE_SIG;
 
         self.init_complete = true;
         Ok(response)
@@ -210,52 +243,116 @@ impl Mt1959 {
                 0x00,
             ];
             let mut resp = [0u8; 4];
-            if scsi
-                .execute(&cdb, DataDirection::FromDevice, &mut resp, 5_000)
-                .is_ok()
-            {
-                return Ok(());
+            match scsi.execute(&cdb, DataDirection::FromDevice, &mut resp, 5_000) {
+                // A drive sense arrives as `Ok`; only a GOOD status is a pass.
+                Ok(r) if r.status == 0 => return Ok(()),
+                Ok(r) => {
+                    tracing::debug!(
+                        target: "freemkv::disc",
+                        phase = "mt1959_validate_check_condition",
+                        status = r.status,
+                        "validate READ_BUFFER returned a drive sense"
+                    );
+                }
+                // A dead bus will not recover across five more retries, and
+                // labelling every other failure "transport" made the consumer
+                // abort rips it could have completed. Propagate the real fault.
+                Err(e) => {
+                    tracing::warn!(
+                        target: "freemkv::disc",
+                        phase = "mt1959_validate_transport_fault",
+                        "transport fault during validate; aborting"
+                    );
+                    return Err(Error::from(e));
+                }
             }
         }
-        Err(Error::Scsi {
-            opcode: SCSI_READ_BUFFER,
-            status: crate::scsi::SCSI_STATUS_TRANSPORT_FAILURE,
-            sense: None,
-        })
+        Err(Error::UnlockFailed)
     }
 
     // ── Init (unlock + firmware) ───────────────────────────────────────
 
     fn run_init(&mut self, scsi: &mut dyn ScsiTransport) -> Result<()> {
-        let mut succeeded = false;
-        for _attempt in 0..3 {
+        let mut last_err = Error::UnlockFailed;
+        for attempt in 0..3 {
             match self.do_unlock(scsi) {
                 Ok(_) => {
-                    succeeded = true;
-                    break;
+                    tracing::debug!(
+                        target: "freemkv::disc",
+                        phase = "mt1959_unlock_ok",
+                        attempt,
+                        unlocked = self.unlocked,
+                        "MT1959 unlock handshake completed"
+                    );
+                    return Ok(());
                 }
                 Err(Error::SignatureMismatch { .. }) => {
+                    tracing::debug!(
+                        target: "freemkv::disc",
+                        phase = "mt1959_signature_mismatch",
+                        attempt,
+                        "unlock response carried another drive's signature"
+                    );
                     return Err(Error::UnlockFailed);
                 }
-                Err(_) => {
+                // A transport fault means the bus is gone. Re-uploading firmware
+                // into a dead bus three times accomplishes nothing and the
+                // generic UnlockFailed it used to end on is classified as
+                // "unlocker not applicable", so the consumer kept probing a dead
+                // drive instead of aborting. Surface the fault unchanged.
+                Err(e) if e.is_transport_failure() => {
+                    tracing::warn!(
+                        target: "freemkv::disc",
+                        phase = "mt1959_transport_fault",
+                        attempt,
+                        "transport fault during unlock; aborting"
+                    );
+                    return Err(e);
+                }
+                Err(e) => {
+                    last_err = e;
                     let loaded = if self.mode == MODE_A {
-                        variant_a::load_firmware(self, scsi).is_ok()
+                        variant_a::load_firmware(self, scsi)
                     } else {
-                        variant_b::load_firmware(self, scsi).is_ok()
+                        variant_b::load_firmware(self, scsi)
                     };
-                    if !loaded {
-                        continue;
+                    match loaded {
+                        // Same rule on the upload path: a dead bus aborts.
+                        Err(e) if e.is_transport_failure() => {
+                            tracing::warn!(
+                                target: "freemkv::disc",
+                                phase = "mt1959_transport_fault",
+                                attempt,
+                                "transport fault during firmware upload; aborting"
+                            );
+                            return Err(e);
+                        }
+                        Err(e) => {
+                            tracing::debug!(
+                                target: "freemkv::disc",
+                                phase = "mt1959_firmware_upload_failed",
+                                attempt,
+                                error = %e,
+                                "firmware upload failed; retrying unlock"
+                            );
+                            last_err = e;
+                            continue;
+                        }
+                        Ok(()) => {
+                            // Firmware upload resets the drive. Give it time to
+                            // fully recover before retrying unlock.
+                            std::thread::sleep(std::time::Duration::from_secs(10));
+                        }
                     }
-                    // Firmware upload resets the drive. Give it time to
-                    // fully recover before retrying unlock.
-                    std::thread::sleep(std::time::Duration::from_secs(10));
                 }
             }
         }
-        if !succeeded {
-            return Err(Error::UnlockFailed);
-        }
-        Ok(())
+        tracing::warn!(
+            target: "freemkv::disc",
+            phase = "mt1959_unlock_exhausted",
+            "MT1959 unlock did not succeed after 3 attempts"
+        );
+        Err(last_err)
     }
 
     // ── Probe disc ─────────────────────────────────────────────────────
@@ -285,10 +382,12 @@ impl Mt1959 {
             0x00,
         ];
         let mut cap_buf = [0u8; READ_CAPACITY_RESPONSE_SIZE];
-        let disc_sectors = if scsi
-            .execute(&cap_cdb, DataDirection::FromDevice, &mut cap_buf, 5_000)
-            .is_ok()
-        {
+        // A transport fault here is a dead bus: propagate it (the caller
+        // classifies it as `Transport` and aborts) instead of silently
+        // defaulting the disc type. A drive sense (`Ok`, non-zero status) or a
+        // short reply just means "capacity unknown" -> assume BD.
+        let cap = scsi.execute(&cap_cdb, DataDirection::FromDevice, &mut cap_buf, 5_000)?;
+        let disc_sectors = if cap.status == 0 && cap.bytes_transferred >= 4 {
             // last_lba + 1 = sector count. A 0xFFFFFFFF last-LBA is the
             // READ CAPACITY(10) "capacity exceeds 32 bits" sentinel; saturate
             // rather than wrap to 0 (which would misclassify a huge disc as
@@ -317,7 +416,7 @@ impl Mt1959 {
         let mut addr: u16 = 0;
         while addr < PROBE_COARSE_END {
             let mut resp = [0u8; PROBE_RESPONSE_SIZE as usize];
-            if self
+            if let Err(e) = self
                 .read_buffer_probe(
                     scsi,
                     SUB_CMD_PROBE,
@@ -325,31 +424,68 @@ impl Mt1959 {
                     &mut resp,
                     PROBE_RESPONSE_SIZE as usize,
                 )
-                .is_err()
+                .inspect_err(|e| {
+                    tracing::debug!(
+                        target: "freemkv::disc",
+                        phase = "mt1959_probe_failed",
+                        addr,
+                        error = %e,
+                        "coarse speed probe failed"
+                    );
+                })
             {
-                return Err(Error::Scsi {
-                    opcode: SCSI_READ_BUFFER,
-                    status: crate::scsi::SCSI_STATUS_TRANSPORT_FAILURE,
-                    sense: None,
-                });
+                // A dead bus MUST keep its transport classification: this loop is
+                // reached from `probe_disc`, whose caller aborts the rip only when
+                // it sees `Transport`. Flattening every failure to `UnlockFailed`
+                // (which classifies as "not applicable") let a bus that died mid
+                // speed-probe fall through to `Ok(Unlocked{drive_unlocked:true})`.
+                // A short / rejected probe reply is still the drive's OWN failure —
+                // keep it as `UnlockFailed` so it doesn't hard-abort a rip that
+                // could continue on the default speed table.
+                if e.is_transport_failure() {
+                    return Err(e);
+                }
+                return Err(Error::UnlockFailed);
             }
             addr = addr.wrapping_add(PROBE_STEP);
         }
 
-        // Pass 2: fine scan
-        let mut addr: u32 = 0;
+        // Pass 2: continue past the coarse range. It used to restart at 0 with
+        // the SAME PROBE_STEP, re-issuing all 88 pass-1 addresses verbatim — 88
+        // redundant SCSI round-trips per disc, which dominates drive prep at
+        // real bus latency and teaches the drive's speed table nothing new.
+        let mut addr: u32 = PROBE_COARSE_END as u32;
         while addr < PROBE_FINE_END {
             let mut resp = [0u8; PROBE_RESPONSE_SIZE as usize];
-            if self
-                .read_buffer_probe(
-                    scsi,
-                    SUB_CMD_PROBE,
-                    addr as u16,
-                    &mut resp,
-                    PROBE_RESPONSE_SIZE as usize,
-                )
-                .is_err()
-            {
+            if let Err(e) = self.read_buffer_probe(
+                scsi,
+                SUB_CMD_PROBE,
+                addr as u16,
+                &mut resp,
+                PROBE_RESPONSE_SIZE as usize,
+            ) {
+                // Fine probing is best-effort — a short / rejected reply just ends
+                // the sweep early. But a bus that DIED here must not be swallowed:
+                // a bare `break` returned `Ok(())` from `run_probe`, so a dead bus
+                // mid-pass-2 vanished into `Ok(Unlocked{drive_unlocked:true})` with
+                // no log at all (the flagship failure-that-looks-like-success). A
+                // transport fault keeps its classification and aborts.
+                if e.is_transport_failure() {
+                    tracing::warn!(
+                        target: "freemkv::disc",
+                        phase = "mt1959_probe_transport_fault",
+                        addr,
+                        "transport fault during fine speed probe; aborting"
+                    );
+                    return Err(e);
+                }
+                tracing::debug!(
+                    target: "freemkv::disc",
+                    phase = "mt1959_probe_failed",
+                    addr,
+                    error = %e,
+                    "fine speed probe ended early"
+                );
                 break;
             }
             addr += PROBE_STEP as u32;
@@ -483,6 +619,330 @@ mod tests {
         assert_eq!(len, 2208, "MODE SELECT length = firmware.len(), not 2496");
     }
 
+    /// variant_a had no direct test at all (variant_b did). Pins its upload
+    /// sequence: WRITE_BUFFER carrying the firmware at its real per-drive
+    /// length in the CDB's 24-bit length field, then the 0x45 verify READ_BUFFER
+    /// whose result used to be discarded outright.
+    #[test]
+    fn variant_a_uploads_at_the_real_length_and_issues_the_verify_read() {
+        let mut profile = fixture_profile([0x11, 0x22, 0x33, 0x44]);
+        profile.firmware = vec![0u8; 2208];
+
+        let mut mt = Mt1959::new(profile, false);
+        let mut t = RecordingTransport { cdbs: Vec::new() };
+        let _ = variant_a::load_firmware(&mut mt, &mut t);
+
+        let wb = t
+            .cdbs
+            .iter()
+            .find(|c| c.first() == Some(&SCSI_WRITE_BUFFER))
+            .expect("WRITE_BUFFER issued");
+        let len = ((wb[6] as usize) << 16) | ((wb[7] as usize) << 8) | wb[8] as usize;
+        assert_eq!(len, 2208, "24-bit CDB length must be firmware.len()");
+
+        assert!(
+            t.cdbs
+                .iter()
+                .any(|c| c.first() == Some(&SCSI_READ_BUFFER) && c.get(2) == Some(&0x45)),
+            "the 0x45 verify READ_BUFFER must be issued"
+        );
+    }
+
+    /// An EMPTY firmware blob cannot be uploaded — catches a profile whose
+    /// firmware failed to decode being pushed at the drive as a zero-length
+    /// WRITE_BUFFER.
+    #[test]
+    fn variant_a_refuses_an_empty_firmware_blob() {
+        let mut mt = Mt1959::new(fixture_profile([0; 4]), false);
+        let mut t = RecordingTransport { cdbs: Vec::new() };
+        let e = variant_a::load_firmware(&mut mt, &mut t).expect_err("no firmware");
+        assert!(matches!(e, Error::UnlockFailed));
+        assert!(t.cdbs.is_empty(), "no CDB may reach the drive");
+    }
+
+    /// THE defect-15 test. A drive that returns ZERO bytes used to skip the
+    /// signature check, the firmware-active check AND the mode check — every
+    /// gate was individually guarded by its own `n >= ..` — so `do_unlock`
+    /// returned `Ok(vec![0u8; 64])` and (with the old `drive_unlocked: true`)
+    /// reported a fully unlocked drive. Catches restoring those per-check
+    /// guards.
+    #[test]
+    fn do_unlock_rejects_a_zero_length_response() {
+        let sig = [0x99, 0x9E, 0xC3, 0x75];
+        let mut t =
+            crate::scsi::mock::MockTransport::always(crate::scsi::mock::Reply::zero_transfer(64));
+        let mut mt = Mt1959::new(fixture_profile(sig), false);
+        let e = mt.do_unlock(&mut t).expect_err("no bytes is not an unlock");
+        assert!(matches!(e, Error::UnlockFailed));
+        assert!(!mt.init_complete);
+        assert!(!mt.is_unlocked());
+    }
+
+    /// A response truncated just below the secondary marker is equally
+    /// unverifiable — the checks must not run against the caller's zero fill.
+    #[test]
+    fn do_unlock_rejects_a_response_too_short_to_validate() {
+        let sig = [0x99, 0x9E, 0xC3, 0x75];
+        let response = build_response(sig, FIRMWARE_ACTIVE_SIG, FIRMWARE_MODE_SIG);
+        let mut t =
+            crate::scsi::mock::MockTransport::always(crate::scsi::mock::Reply::short(response, 19));
+        let mut mt = Mt1959::new(fixture_profile(sig), false);
+        let e = mt
+            .do_unlock(&mut t)
+            .expect_err("19 bytes cannot carry the markers");
+        assert!(matches!(e, Error::UnlockFailed));
+    }
+
+    /// A CHECK CONDITION arrives as `Ok` per the transport contract; treating it
+    /// as an unlock would validate the caller's own zero fill.
+    #[test]
+    fn do_unlock_rejects_a_check_condition() {
+        let mut t =
+            crate::scsi::mock::MockTransport::always(crate::scsi::mock::Reply::illegal_request());
+        let mut mt = Mt1959::new(fixture_profile([0; 4]), false);
+        let e = mt
+            .do_unlock(&mut t)
+            .expect_err("a drive sense is not an unlock");
+        assert!(!e.is_transport_failure(), "a sense is not a dead bus");
+    }
+
+    /// THE defect-16 test (the inverse misclassification). A merely SHORT probe
+    /// response used to be fabricated into a transport-failure status, which
+    /// hard-aborts a rip that would otherwise have succeeded. Catches
+    /// reintroducing the fabricated 0xFF.
+    #[test]
+    fn short_probe_response_is_not_a_transport_failure() {
+        let mut t = crate::scsi::mock::MockTransport::always(crate::scsi::mock::Reply::short(
+            vec![0u8; 4],
+            2,
+        ));
+        let mt = Mt1959::new(fixture_profile([0; 4]), false);
+        let mut buf = [0u8; 4];
+        let e = mt
+            .read_buffer_probe(&mut t, SUB_CMD_PROBE, 0, &mut buf, 4)
+            .expect_err("short probe");
+        assert!(
+            !e.is_transport_failure(),
+            "a short probe response must not abort the rip"
+        );
+        assert_eq!(
+            crate::UnlockError::from(e),
+            crate::UnlockError::NotApplicable
+        );
+    }
+
+    /// A genuine transport fault on the probe still propagates as one.
+    #[test]
+    fn transport_fault_on_probe_is_a_transport_failure() {
+        let mut t =
+            crate::scsi::mock::MockTransport::always(crate::scsi::mock::Reply::TransportFault);
+        let mt = Mt1959::new(fixture_profile([0; 4]), false);
+        let mut buf = [0u8; 4];
+        let e = mt
+            .read_buffer_probe(&mut t, SUB_CMD_PROBE, 0, &mut buf, 4)
+            .expect_err("dead bus");
+        assert!(e.is_transport_failure());
+    }
+
+    /// THE defect-4 test. A dead bus must abort `run_init` at once. It used to
+    /// match `Err(_)` and re-upload firmware into the dead bus on all three
+    /// attempts, then return the generic `UnlockFailed`, which classifies as
+    /// `NotApplicable` — so the consumer kept probing. Catches removing the
+    /// `is_transport_failure` arm.
+    #[test]
+    fn run_init_aborts_on_a_transport_fault_without_reloading_firmware() {
+        let mut t =
+            crate::scsi::mock::MockTransport::always(crate::scsi::mock::Reply::TransportFault);
+        let mut mt = Mt1959::new(fixture_profile([0; 4]), false);
+        let e = mt.init(&mut t).expect_err("dead bus");
+        assert!(e.is_transport_failure());
+        assert_eq!(crate::UnlockError::from(e), crate::UnlockError::Transport);
+        assert_eq!(t.calls(), 1, "one command, then abort — no firmware reload");
+    }
+
+    /// THE defect-14 test. The two probe passes used to walk the SAME addresses
+    /// at the SAME step: pass 2 restarted at 0 and re-issued all 88 pass-1
+    /// probes verbatim. Catches restoring the overlap.
+    #[test]
+    fn probe_passes_do_not_reissue_the_same_addresses() {
+        let mut t =
+            crate::scsi::mock::MockTransport::always(crate::scsi::mock::Reply::good(vec![0u8; 8]));
+        let mut mt = Mt1959::new(fixture_profile([0; 4]), false);
+        mt.init_complete = true;
+        let _ = mt.run_probe(&mut t);
+
+        // Every SUB_CMD_PROBE CDB carries its address at bytes 4-5.
+        let mut probes: Vec<u16> = t
+            .cdbs
+            .iter()
+            .filter(|c| c.first() == Some(&SCSI_READ_BUFFER) && c.get(3) == Some(&SUB_CMD_PROBE))
+            .map(|c| ((c[4] as u16) << 8) | c[5] as u16)
+            .collect();
+        let issued = probes.len();
+        probes.sort_unstable();
+        probes.dedup();
+        assert_eq!(
+            issued,
+            probes.len(),
+            "no probe address may be issued twice ({} duplicates)",
+            issued - probes.len()
+        );
+    }
+
+    /// Answers every command GOOD except the speed probe (READ_BUFFER /
+    /// SUB_CMD_PROBE) at address >= `fault_at`, which returns a transport fault —
+    /// a bus that dies partway through disc-speed calibration.
+    struct ProbeFaultsTransport {
+        fault_at: u16,
+    }
+    impl ScsiTransport for ProbeFaultsTransport {
+        fn execute(
+            &mut self,
+            cdb: &[u8],
+            _dir: DataDirection,
+            data: &mut [u8],
+            _timeout_ms: u32,
+        ) -> crate::scsi::Result<ScsiResult> {
+            if cdb.first() == Some(&SCSI_READ_BUFFER) && cdb.get(3) == Some(&SUB_CMD_PROBE) {
+                let addr = ((cdb[4] as u16) << 8) | cdb[5] as u16;
+                if addr >= self.fault_at {
+                    return Err(crate::scsi::ScsiError {
+                        status: crate::scsi::SCSI_STATUS_TRANSPORT_FAILURE,
+                        sense: None,
+                    });
+                }
+            }
+            for b in data.iter_mut() {
+                *b = 0;
+            }
+            Ok(ScsiResult {
+                status: 0,
+                bytes_transferred: data.len(),
+                sense: [0u8; 32],
+            })
+        }
+    }
+
+    /// THE probe pass-1 dead-bus test. A bus that dies during the coarse speed
+    /// probe must keep its transport classification out of `run_probe` so the
+    /// caller (`firmware_unlock`) aborts with `Transport` rather than reporting a
+    /// fully-unlocked drive.
+    /// MUTATION: flattening the pass-1 error to `Error::UnlockFailed` makes the
+    /// `is_transport_failure` assert go red (UnlockFailed maps to NotApplicable).
+    #[test]
+    fn transport_fault_during_pass1_probe_stays_a_transport_failure() {
+        let mut t = ProbeFaultsTransport { fault_at: 0 };
+        let mut mt = Mt1959::new(fixture_profile([0; 4]), false);
+        mt.init_complete = true;
+        let e = mt
+            .run_probe(&mut t)
+            .expect_err("dead bus during pass-1 probe");
+        assert!(e.is_transport_failure(), "pass-1 fault must stay transport");
+        assert_eq!(crate::UnlockError::from(e), crate::UnlockError::Transport);
+    }
+
+    /// THE probe pass-2 dead-bus test. Pass 1 completes; the bus dies in the fine
+    /// probe. The loop used to `break` on ANY error and return `Ok(())`, so a
+    /// dead bus mid-pass-2 vanished into `Ok`. It must now propagate a transport
+    /// fault (a short/rejected fine probe still ends the sweep benignly).
+    /// MUTATION: restoring the bare `break` makes `expect_err` panic (run_probe
+    /// returns Ok).
+    #[test]
+    fn transport_fault_during_pass2_probe_stays_a_transport_failure() {
+        let mut t = ProbeFaultsTransport {
+            fault_at: PROBE_COARSE_END,
+        };
+        let mut mt = Mt1959::new(fixture_profile([0; 4]), false);
+        mt.init_complete = true;
+        let e = mt
+            .run_probe(&mut t)
+            .expect_err("dead bus during pass-2 probe");
+        assert!(e.is_transport_failure(), "pass-2 fault must stay transport");
+        assert_eq!(crate::UnlockError::from(e), crate::UnlockError::Transport);
+    }
+
+    /// `validate`'s transport-abort branch had no test. A dead bus must propagate
+    /// as a transport fault on the FIRST attempt, not be retried five times.
+    /// MUTATION: turning the `Err(e) => return Err(Error::from(e))` arm into a
+    /// `continue` makes this exhaust to `UnlockFailed` and the assert go red.
+    #[test]
+    fn validate_propagates_a_transport_fault_without_retrying() {
+        let mut t =
+            crate::scsi::mock::MockTransport::always(crate::scsi::mock::Reply::TransportFault);
+        let mt = Mt1959::new(fixture_profile([0; 4]), false);
+        let e = mt.validate(&mut t).expect_err("dead bus");
+        assert!(e.is_transport_failure());
+        assert_eq!(t.calls(), 1, "abort on the first fault, no 5× retry");
+    }
+
+    /// `validate`'s drive-sense path: a CHECK CONDITION is `Ok` per the contract,
+    /// so validate retries and finally returns `UnlockFailed` — NOT a transport
+    /// abort (which would wrongly kill a rip the drive merely stalled on).
+    #[test]
+    fn validate_drive_sense_exhausts_to_unlock_failed() {
+        let mut t =
+            crate::scsi::mock::MockTransport::always(crate::scsi::mock::Reply::illegal_request());
+        let mt = Mt1959::new(fixture_profile([0; 4]), false);
+        let e = mt.validate(&mut t).expect_err("five senses, no pass");
+        assert!(!e.is_transport_failure(), "a sense is not a dead bus");
+        assert!(matches!(e, Error::UnlockFailed));
+        assert_eq!(t.calls(), 5, "retries the full five attempts");
+    }
+
+    /// variant_a's firmware-verify-read transport-abort branch (~variant_a.rs:97)
+    /// had no test. WRITE_BUFFER succeeds, the 0x45 verify read faults: a dead bus
+    /// must abort THERE, not be swallowed into the unlock retries.
+    /// MUTATION: dropping the `if err.is_transport_failure() { return Err }` lets
+    /// execution fall through to `do_unlock` (scripted to answer, so it fails on
+    /// the signature as `UnlockFailed`), and the `is_transport_failure` assert
+    /// goes red — a genuine red-before-green, not green-both-ways.
+    #[test]
+    fn variant_a_verify_read_transport_fault_aborts() {
+        let mut profile = fixture_profile([0; 4]);
+        profile.firmware = vec![0u8; 64];
+        let mut mt = Mt1959::new(profile, false);
+        // call 1 WRITE_BUFFER → good; call 2 verify read → dead bus; later
+        // do_unlock calls (only reached if the fix is absent) → good.
+        let mut t = crate::scsi::mock::MockTransport::scripted(
+            vec![
+                crate::scsi::mock::Reply::good(vec![0u8; 4]),
+                crate::scsi::mock::Reply::TransportFault,
+            ],
+            crate::scsi::mock::Reply::good(vec![0u8; 64]),
+        );
+        let e = variant_a::load_firmware(&mut mt, &mut t).expect_err("dead bus at verify");
+        assert!(
+            e.is_transport_failure(),
+            "verify-read dead bus must surface"
+        );
+    }
+
+    /// variant_b had no transport-abort test. A firmware-upload STEP that hits a
+    /// dead bus (here the metadata read) must abort via `trace_step`, not be
+    /// swallowed and carried into the unlock retries.
+    /// MUTATION: dropping `trace_step`'s `if err.is_transport_failure()` return
+    /// lets execution reach `do_unlock` (scripted good → signature fails as
+    /// `UnlockFailed`), so the `is_transport_failure` assert goes red.
+    #[test]
+    fn variant_b_upload_step_transport_fault_aborts() {
+        let mut profile = fixture_profile([0; 4]);
+        profile.firmware = vec![0u8; 64];
+        let mut mt = Mt1959::new(profile, true);
+        // call 1 MODE SELECT → good; call 2 metadata read → dead bus; rest good.
+        let mut t = crate::scsi::mock::MockTransport::scripted(
+            vec![
+                crate::scsi::mock::Reply::good(vec![0u8; 16]),
+                crate::scsi::mock::Reply::TransportFault,
+            ],
+            crate::scsi::mock::Reply::good(vec![0u8; 64]),
+        );
+        let e = variant_b::load_firmware(&mut mt, &mut t).expect_err("dead bus mid-upload");
+        assert!(
+            e.is_transport_failure(),
+            "an upload-step dead bus must surface"
+        );
+    }
+
     fn fixture_profile(signature: [u8; 4]) -> DriveProfile {
         DriveProfile {
             identity: Identity {

src/ld/platform/mt1959/variant_a.rs

@@ -43,7 +43,12 @@ pub(super) fn load_firmware(mt: &mut Mt1959, scsi: &mut dyn ScsiTransport) -> Re
     let mut data = firmware.clone();
     scsi.execute(&cdb, DataDirection::ToDevice, &mut data, 30_000)?;
 
-    // Verify firmware loaded (non-fatal — different buffer_id 0x45)
+    // Verify the firmware loaded (non-fatal — different buffer_id 0x45). There
+    // is no documented expected payload to compare against, so the outcome is
+    // TRACED rather than asserted: discarding it with `let _ =` meant a corrupt
+    // upload proceeded straight into the do_unlock retries with no diagnostic
+    // whatsoever, which is exactly the silent-failure shape this crate exists to
+    // avoid.
     let verify_cdb = [
         super::SCSI_READ_BUFFER,
         super::MODE_A,
@@ -57,12 +62,43 @@ pub(super) fn load_firmware(mt: &mut Mt1959, scsi: &mut dyn ScsiTransport) -> Re
         0x00,
     ];
     let mut verify_resp = [0u8; super::VALIDATE_RESPONSE_SIZE as usize];
-    let _ = scsi.execute(
+    match scsi.execute(
         &verify_cdb,
         DataDirection::FromDevice,
         &mut verify_resp,
         5_000,
-    );
+    ) {
+        Ok(r) if r.status == 0 && r.bytes_transferred == verify_resp.len() => {
+            tracing::debug!(
+                target: "freemkv::disc",
+                phase = "mt1959a_fw_verify_ok",
+                "variant-A firmware verify read returned a full response"
+            );
+        }
+        Ok(r) => {
+            tracing::warn!(
+                target: "freemkv::disc",
+                phase = "mt1959a_fw_verify_bad",
+                status = r.status,
+                bytes_transferred = r.bytes_transferred,
+                "variant-A firmware verify read did not return a full good response"
+            );
+        }
+        // A dead bus mid-upload must abort, not be swallowed into the unlock
+        // retries below.
+        Err(e) => {
+            let err = crate::ld::error::Error::from(e);
+            tracing::warn!(
+                target: "freemkv::disc",
+                phase = "mt1959a_fw_verify_failed",
+                transport_failure = err.is_transport_failure(),
+                "variant-A firmware verify read failed"
+            );
+            if err.is_transport_failure() {
+                return Err(err);
+            }
+        }
+    }
 
     // Double unlock after firmware upload. The first establishes the
     // unlock and is fatal on failure; the second is a confirmation pass and

src/ld/platform/mt1959/variant_b.rs

@@ -13,6 +13,47 @@ const FIRMWARE_EXTRA: [u8; 16] = [0; 16];
 /// that drive — real profiles must supply their own (see `DriveProfile`).
 const VENDOR_VERIFY: [u8; 10] = [0xF1, 0x01, 0x02, 0x00, 0x0D, 0x30, 0x01, 0xF3, 0xAD, 0x23];
 
+/// Trace the outcome of a best-effort firmware-upload step and swallow
+/// everything EXCEPT a transport fault. These steps are advisory (the unlock
+/// retries below are the real gate) but their results were previously thrown
+/// away entirely, so a firmware upload could fail end to end and leave nothing
+/// in the log to say so — and a dead bus kept the sequence running.
+fn trace_step(phase: &'static str, r: crate::scsi::Result<crate::scsi::ScsiResult>) -> Result<()> {
+    match r {
+        Ok(res) if res.status == 0 => {
+            tracing::debug!(
+                target: "freemkv::disc",
+                phase,
+                bytes_transferred = res.bytes_transferred,
+                "firmware upload step ok"
+            );
+            Ok(())
+        }
+        Ok(res) => {
+            tracing::warn!(
+                target: "freemkv::disc",
+                phase,
+                status = res.status,
+                "firmware upload step returned a drive sense"
+            );
+            Ok(())
+        }
+        Err(e) => {
+            let err = crate::ld::error::Error::from(e);
+            tracing::warn!(
+                target: "freemkv::disc",
+                phase,
+                transport_failure = err.is_transport_failure(),
+                "firmware upload step failed"
+            );
+            if err.is_transport_failure() {
+                return Err(err);
+            }
+            Ok(())
+        }
+    }
+}
+
 pub(super) fn load_firmware(mt: &mut Mt1959, scsi: &mut dyn ScsiTransport) -> Result<()> {
     let firmware = &mt.profile.firmware;
     if firmware.is_empty() {
@@ -58,12 +99,15 @@ pub(super) fn load_firmware(mt: &mut Mt1959, scsi: &mut dyn ScsiTransport) -> Re
         0x00,
     ];
     let mut meta_resp = [0u8; 16];
-    let _ = scsi.execute(
-        &read_meta_cdb,
-        DataDirection::FromDevice,
-        &mut meta_resp,
-        5_000,
-    );
+    trace_step(
+        "mt1959b_fw_meta",
+        scsi.execute(
+            &read_meta_cdb,
+            DataDirection::FromDevice,
+            &mut meta_resp,
+            5_000,
+        ),
+    )?;
 
     // Step 3: Write extra firmware data (all zeros)
     let write_extra_cdb = [
@@ -79,14 +123,24 @@ pub(super) fn load_firmware(mt: &mut Mt1959, scsi: &mut dyn ScsiTransport) -> Re
         0x00,
     ];
     let mut data2 = FIRMWARE_EXTRA.to_vec();
-    let _ = scsi.execute(&write_extra_cdb, DataDirection::ToDevice, &mut data2, 5_000);
+    trace_step(
+        "mt1959b_fw_extra",
+        scsi.execute(&write_extra_cdb, DataDirection::ToDevice, &mut data2, 5_000),
+    )?;
 
     // Step 4: Vendor verify (0xF1 — B-only, not standard SCSI). PER-DRIVE: take
     // it from the profile (39 distinct values across the 140 B drives). The
     // const is only a legacy fallback — it carries one drive's token.
+    // The result used to be discarded with `let _ =` despite the comment calling
+    // this a verify step: a corrupt upload proceeded straight to the do_unlock
+    // retries with no diagnostic. There is no documented expected payload, so
+    // the outcome is TRACED; only a dead bus aborts.
     let verify_cdb = mt.profile.fw_verify_cdb.unwrap_or(VENDOR_VERIFY);
     let mut dummy = [0u8; 0];
-    let _ = scsi.execute(&verify_cdb, DataDirection::None, &mut dummy, 5_000);
+    trace_step(
+        "mt1959b_fw_verify",
+        scsi.execute(&verify_cdb, DataDirection::None, &mut dummy, 5_000),
+    )?;
 
     // Step 5: Unlock retries (up to 5, then a final fatal attempt). On a
     // successful unlock we issue one confirmation pass; its result is
@@ -94,9 +148,14 @@ pub(super) fn load_firmware(mt: &mut Mt1959, scsi: &mut dyn ScsiTransport) -> Re
     // unlock state, so a hiccup on the redundant confirmation must not fail
     // an otherwise-good unlock.
     for _attempt in 0..5 {
-        if mt.do_unlock(scsi).is_ok() {
-            let _ = mt.do_unlock(scsi);
-            return Ok(());
+        match mt.do_unlock(scsi) {
+            Ok(_) => {
+                let _ = mt.do_unlock(scsi);
+                return Ok(());
+            }
+            // A dead bus will not recover across five more unlock attempts.
+            Err(e) if e.is_transport_failure() => return Err(e),
+            Err(_) => {}
         }
     }
     mt.do_unlock(scsi)?;

src/ld/profile.rs

@@ -19,7 +19,7 @@ pub struct Profiles {
 impl Profiles {
     /// The profile matching a drive identity, if this catalog supports that
     /// drive. Two-pass per platform section: exact (incl. firmware date) then a
-    /// looser vendor/revision/vendor-specific match. See [`find_by_drive_id`].
+    /// looser vendor/revision/vendor-specific match. See `find_by_drive_id`.
     pub fn get(&self, drive_id: &crate::DriveId) -> Option<ProfileMatch> {
         find_by_drive_id(self, drive_id)
     }
@@ -41,7 +41,23 @@ pub struct Identity {
 }
 
 /// Per-drive profile.
-#[derive(Debug, Clone, Deserialize)]
+///
+/// VISIBILITY: only `identity` and `signature` are public — they are what the
+/// catalog is public FOR ("is this drive supported?", and the emulator's
+/// impersonation). Everything below them is unlock MECHANISM: the MediaTek
+/// firmware image and the per-drive vendor CDB templates. `Cargo.toml` says this
+/// crate is never published because it carries drive firmware; re-exporting
+/// `DriveProfile` with a `pub firmware: Vec<u8>` handed that firmware, and every
+/// vendor CDB, to any downstream crate that called `ld::profile()`. They are
+/// `pub(crate)` so the mechanism stays inside the unlocker.
+///
+/// `allow(dead_code)`: this is the on-disk catalog schema, and several captured
+/// CDB templates have no in-crate consumer yet. They were `pub` before, so the
+/// compiler saw the (external) use; making them `pub(crate)` is what exposes
+/// them as unread. Deleting them would silently drop fields from the format the
+/// profile-extraction pipeline emits.
+#[allow(dead_code)]
+#[derive(Clone, Deserialize)]
 pub struct DriveProfile {
     pub identity: Identity,
     /// Expected first 4 bytes of the drive's unlock response — the
@@ -53,7 +69,7 @@ pub struct DriveProfile {
     /// firmware-load step). JSON-encoded as standard base64; empty when
     /// the profile carries no firmware blob.
     #[serde(default, deserialize_with = "deserialize_base64")]
-    pub firmware: Vec<u8>,
+    pub(crate) firmware: Vec<u8>,
 
     // ── OEM-extended-access CDB templates ──────────────────────────────
     //
@@ -62,39 +78,61 @@ pub struct DriveProfile {
     // in the JSON as lowercase hex strings without separators
     // (e.g. `"3c014410e29100002400"` for a 10-byte CDB).
     #[serde(default)]
-    pub unlock_init_value: u8,
+    pub(crate) unlock_init_value: u8,
     #[serde(default)]
-    pub unlock_response_size: u8,
+    pub(crate) unlock_response_size: u8,
 
     #[serde(default, deserialize_with = "deserialize_opt_hex_bytes_10")]
-    pub read_vid_cdb: Option<[u8; 10]>,
+    pub(crate) read_vid_cdb: Option<[u8; 10]>,
     #[serde(default, deserialize_with = "deserialize_opt_hex_bytes_10")]
-    pub read_disc_keys_cdb: Option<[u8; 10]>,
+    pub(crate) read_disc_keys_cdb: Option<[u8; 10]>,
     #[serde(default, deserialize_with = "deserialize_opt_hex_bytes_12")]
-    pub drive_nominal_speed_cdb: Option<[u8; 12]>,
+    pub(crate) drive_nominal_speed_cdb: Option<[u8; 12]>,
     #[serde(default, deserialize_with = "deserialize_opt_hex_bytes_12")]
-    pub set_speed_max_cdb: Option<[u8; 12]>,
+    pub(crate) set_speed_max_cdb: Option<[u8; 12]>,
     #[serde(default, deserialize_with = "deserialize_opt_hex_bytes_10")]
-    pub read10_raw_2sec_cdb: Option<[u8; 10]>,
+    pub(crate) read10_raw_2sec_cdb: Option<[u8; 10]>,
     #[serde(default, deserialize_with = "deserialize_opt_hex_bytes_10")]
-    pub read10_raw_1sec_cdb: Option<[u8; 10]>,
+    pub(crate) read10_raw_1sec_cdb: Option<[u8; 10]>,
     #[serde(default, deserialize_with = "deserialize_opt_hex_bytes_10")]
-    pub read_buffer_verify_cdb: Option<[u8; 10]>,
+    pub(crate) read_buffer_verify_cdb: Option<[u8; 10]>,
     #[serde(default, deserialize_with = "deserialize_opt_hex_bytes_10")]
-    pub write_buffer_cdb: Option<[u8; 10]>,
+    pub(crate) write_buffer_cdb: Option<[u8; 10]>,
     #[serde(default, deserialize_with = "deserialize_opt_hex_bytes_10")]
-    pub read_buffer_unlock_cdb: Option<[u8; 10]>,
+    pub(crate) read_buffer_unlock_cdb: Option<[u8; 10]>,
     /// Variant-B vendor verify (0xF1) CDB. PER-DRIVE: 39 distinct values across
     /// the 140 B drives, so it CANNOT be a hardcoded constant. `variant_b`'s old
     /// `VENDOR_VERIFY` const was one drive's token, wrong for the other ~139.
     #[serde(default, deserialize_with = "deserialize_opt_hex_bytes_10")]
-    pub fw_verify_cdb: Option<[u8; 10]>,
+    pub(crate) fw_verify_cdb: Option<[u8; 10]>,
 
     // Per-drive identifier tables — variable-length hex strings.
     #[serde(default, deserialize_with = "deserialize_opt_hex_bytes")]
-    pub speed_zone_table: Option<Vec<u8>>,
+    pub(crate) speed_zone_table: Option<Vec<u8>>,
     #[serde(default, deserialize_with = "deserialize_opt_hex_bytes")]
-    pub speed_calc_table: Option<Vec<u8>>,
+    pub(crate) speed_calc_table: Option<Vec<u8>>,
+}
+
+// Hand-written, REDACTING Debug. `DriveProfile` is `pub` and reachable through
+// the public `Profiles` catalog (`ld::bundled` / `ld::profiles`), whose own
+// derived `Debug` would recurse into this one. `Cargo.toml` says this crate is
+// NEVER published because it carries drive firmware; a derived `Debug` would
+// render that firmware image AND every captured per-drive vendor CDB (the unlock
+// MECHANISM) straight into any log. Show only the PUBLIC identity + signature
+// (what the catalog is public FOR); collapse the firmware to a byte length and
+// omit the vendor CDB templates entirely so no mechanism bytes reach a log.
+impl std::fmt::Debug for DriveProfile {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("DriveProfile")
+            .field("identity", &self.identity)
+            .field("signature", &self.signature)
+            .field(
+                "firmware",
+                &format_args!("[{} bytes redacted]", self.firmware.len()),
+            )
+            .field("cdb_templates", &"[redacted]")
+            .finish()
+    }
 }
 
 /// Chipset + variant — determined by which section the profile was found in.
@@ -266,7 +304,24 @@ pub fn bundled() -> Option<&'static Profiles> {
     use std::sync::OnceLock;
     static CACHE: OnceLock<Option<Profiles>> = OnceLock::new();
     CACHE
-        .get_or_init(|| load_from_str(BUNDLED_PROFILES).ok())
+        .get_or_init(|| match load_from_str(BUNDLED_PROFILES) {
+            Ok(p) => Some(p),
+            // The `None` is cached PERMANENTLY (correctly — the blob is fixed at
+            // compile time), which means a bad shipped profiles.json makes
+            // LibreDrive report "drive not supported" for every drive for the
+            // life of the process. Unlogged, that is indistinguishable from a
+            // genuinely uncataloged drive; log it once, loudly, at the one place
+            // the parse happens.
+            Err(e) => {
+                tracing::error!(
+                    target: "freemkv::disc",
+                    phase = "bundled_profiles_parse_failed",
+                    error_code = e.code(),
+                    "bundled LibreDrive profile catalog failed to parse; no drive can match"
+                );
+                None
+            }
+        })
         .as_ref()
 }
 
@@ -660,6 +715,40 @@ mod tests {
         );
     }
 
+    /// `DriveProfile`'s `Debug` must NOT render the firmware image or the
+    /// per-drive vendor CDB templates — this crate is unpublished precisely
+    /// because it carries drive firmware, and the profile is reachable through
+    /// the public `Profiles` catalog.
+    /// MUTATION: restoring `#[derive(Debug)]` prints the raw firmware bytes
+    /// (e.g. the 0xEE marker below) and the CDB bytes, so this goes red.
+    #[test]
+    fn drive_profile_debug_redacts_firmware_and_cdbs() {
+        use serde_json::json;
+        let profiles: Profiles = serde_json::from_str(
+            &json!({
+                "mt1959_a": [{
+                    "identity": {"vendor_id":"VIS","product_revision":"1.00",
+                        "vendor_specific":"AA00000","firmware_date":"200001010000"},
+                    "signature":"aabbccdd",
+                    "firmware":"7u7u7u7u", // base64 → six 0xEE bytes
+                    "read_vid_cdb":"3c014410e29100002400"
+                }]
+            })
+            .to_string(),
+        )
+        .unwrap();
+        let s = format!("{:?}", profiles.mt1959_a[0]);
+        // Firmware byte 0xEE renders as `238` under a derived Debug.
+        assert!(!s.contains("238"), "firmware bytes must not appear: {s}");
+        assert!(
+            !s.contains("read_vid_cdb"),
+            "CDB templates must not appear: {s}"
+        );
+        assert!(s.contains("redacted"), "must mark redaction: {s}");
+        // The PUBLIC fields stay visible.
+        assert!(s.contains("VIS"), "identity must remain visible: {s}");
+    }
+
     /// deserialize_hex4 of an empty string must produce [0;4] without error.
     /// This matches `deserialize_hex4`'s explicit early-return for empty strings.
     /// Mutation: treating empty string as an error prevents profiles where signature

src/lib.rs

@@ -21,7 +21,8 @@ mod css;
 // [`all_unlockers`]. `aacs` and `css` carry no such public catalog, so they
 // stay fully private.
 pub mod ld;
-// `renesis` is public for its `is_renesas` drive-probe; the unlocker impl
+// `renesis` is public for its `is_renesas` drive-probe (which reports a dead
+// bus as `Err(UnlockError::Transport)` rather than "not a Renesas drive"); the unlocker impl
 // (`Renesis`) is `pub(crate)` — reached only through [`all_unlockers`].
 pub mod renesis;
 
@@ -82,13 +83,30 @@ impl<'a> UnlockCtx<'a> {
 /// What removing bus encryption yielded. `drive_unlocked` means the drive now
 /// serves clear content (firmware route) — equivalent, for the gate, to a cert
 /// `bus_key`.
-#[derive(Debug, Clone, Default)]
+#[derive(Clone, Default)]
 pub struct Unlocked {
     pub vid: Option<[u8; 16]>,
     pub bus_key: Option<[u8; 16]>,
     pub drive_unlocked: bool,
 }
 
+// Hand-written, REDACTING Debug — `Unlocked` is the public carrier that leaves
+// this crate, and both `bus_key` (the AACS bus key) and `vid` (the Volume ID
+// that feeds VUK derivation) are key material that must never reach a log or a
+// test-failure message in plaintext. The `AacsAuth` / `CertHandshake` siblings
+// already redact; a derived `Debug` here would print the very bytes they hide.
+// Presence is still observable (Some/None) so a log can say WHETHER a key was
+// obtained without revealing WHAT it is.
+impl std::fmt::Debug for Unlocked {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("Unlocked")
+            .field("vid", &self.vid.map(|_| "[redacted]"))
+            .field("bus_key", &self.bus_key.map(|_| "[redacted]"))
+            .field("drive_unlocked", &self.drive_unlocked)
+            .finish()
+    }
+}
+
 /// Why an unlock produced no usable result. Only `Transport` is a hard error
 /// (bus dead → consumer aborts); the rest mean "fall through to the next
 /// unlocker".
@@ -138,7 +156,8 @@ pub trait Unlocker: Send + Sync {
     /// Remove BUS ENCRYPTION for the mounted disc (AACS host-cert handshake, or
     /// CSS scrambled-sector auth). The consumer runs this only when the bus isn't
     /// already clear, trying each unlocker until one handles it. Same `Ok` /
-    /// `Err(NotApplicable)` / `Err(Transport)` contract as [`unlock_features`].
+    /// `Err(NotApplicable)` / `Err(Transport)` contract as
+    /// [`Unlocker::unlock_features`].
     /// Default: not provided.
     fn unlock_bus(
         &self,
@@ -170,3 +189,88 @@ pub fn all_unlockers() -> Vec<Box<dyn Unlocker>> {
         Box::new(css::DvdUnlocker::new()),
     ]
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// The dispatch order is a documented contract (`firmware → cert → css`) and
+    /// had NO test: `all_unlockers` is the one place an unlocker is named, and
+    /// the consumer stops at the first that doesn't decline, so reordering this
+    /// list silently changes which unlocker claims a drive. Catches a reorder,
+    /// a rename, and an accidental addition/removal.
+    #[test]
+    fn all_unlockers_dispatch_order_is_firmware_then_cert_then_css() {
+        let names: Vec<&'static str> = all_unlockers().iter().map(|u| u.name()).collect();
+        assert_eq!(names, vec!["LibreDrive", "Renesas", "AACS", "DVD"]);
+    }
+
+    /// Every unlocker declines by default rather than claiming a capability it
+    /// does not provide — and declines WITHOUT touching the transport, so the
+    /// consumer can iterate the whole registry on any drive.
+    #[test]
+    fn unlockers_decline_capabilities_they_do_not_provide() {
+        struct DeadTransport;
+        impl scsi::ScsiTransport for DeadTransport {
+            fn execute(
+                &mut self,
+                _cdb: &[u8],
+                _dir: scsi::DataDirection,
+                _data: &mut [u8],
+                _timeout_ms: u32,
+            ) -> scsi::Result<scsi::ScsiResult> {
+                panic!("a declining unlocker must not issue a CDB");
+            }
+        }
+        let id = DriveId::default(); // matches no bundled profile
+        let ctx = UnlockCtx::new(&id, DiscKind::Unencrypted, &[]);
+        let mut t = DeadTransport;
+        for u in all_unlockers() {
+            // AACS/DVD provide no drive features; LibreDrive/Renesas are the
+            // feature unlockers but decline an unknown drive identity.
+            let name = u.name();
+            if name == "AACS" || name == "DVD" {
+                assert_eq!(
+                    u.unlock_features(&mut t, &ctx).unwrap_err(),
+                    UnlockError::NotApplicable,
+                    "{name} must not claim drive features"
+                );
+            }
+            if name == "Renesas" {
+                assert_eq!(
+                    u.unlock_bus(&mut t, &ctx).unwrap_err(),
+                    UnlockError::NotApplicable,
+                    "{name} leaves bus removal to the cert unlocker"
+                );
+            }
+        }
+    }
+
+    /// `unlocker_name` is a PURE lookup — it must answer from the `DriveId`
+    /// alone, and only the identity-keyed unlocker can claim a drive this way.
+    #[test]
+    fn unlocker_name_is_a_pure_identity_lookup() {
+        assert_eq!(unlocker_name(&DriveId::default()), None);
+    }
+
+    /// `Unlocked` is the public carrier that leaves the crate, and its `bus_key`
+    /// and `vid` are key material. Its `Debug` must NOT print those bytes.
+    /// MUTATION: restoring `#[derive(Debug)]` prints the raw byte arrays, so the
+    /// distinctive marker bytes appear in the output and this goes red.
+    #[test]
+    fn unlocked_debug_redacts_key_material() {
+        let u = Unlocked {
+            vid: Some([0xAB; 16]),
+            bus_key: Some([0xCD; 16]),
+            drive_unlocked: true,
+        };
+        let s = format!("{u:?}");
+        // A derived Debug renders `[171, 171, ...]` (0xAB) / `[205, ...]` (0xCD).
+        assert!(!s.contains("171"), "vid bytes must not appear: {s}");
+        assert!(!s.contains("205"), "bus_key bytes must not appear: {s}");
+        assert!(s.contains("[redacted]"), "must mark redaction: {s}");
+        // Presence (Some/None) stays observable.
+        assert!(s.contains("Some"), "must still show a key WAS present: {s}");
+        assert!(s.contains("drive_unlocked: true"));
+    }
+}

src/renesis.rs

@@ -16,23 +16,48 @@ const RB_F1_LEN: usize = 48;
 const RENESAS_MARKER: &[u8] = b"SAT";
 const RENESAS_MARKER_OFFSET: usize = 16;
 
-/// True if `scsi` is a Renesas-platform drive (Pioneer or HL-DT-ST Renesas).
+/// `Ok(true)` if `scsi` is a Renesas-platform drive (Pioneer or HL-DT-ST
+/// Renesas).
 ///
 /// Issues the vendor READ_BUFFER 0x02/0xF1 probe: a Renesas controller serves a
 /// 48-byte identity block whose bytes `[16..19]` are the ASCII `SAT` interface
-/// tag. A MediaTek drive rejects it (ILLEGAL REQUEST → `Err`), and a transport
-/// fault also yields `Err`; both return `false`. This is the definitive
-/// Renesas-vs-MediaTek split.
-pub fn is_renesas(scsi: &mut dyn ScsiTransport) -> bool {
+/// tag. A MediaTek drive rejects it — as a CHECK CONDITION (`Ok` with a
+/// non-zero status) or, through a non-conforming transport, as an `Err`
+/// carrying a sense — which is `Ok(false)`: not a Renesas drive. This is the
+/// definitive Renesas-vs-MediaTek split.
+///
+/// A TRANSPORT fault is `Err(Transport)`. It used to be folded into the same
+/// `false` as a drive rejection, and because this probe is the first SCSI
+/// command the unlocker issues, a dead bus read as "not a Renesas drive" — the
+/// consumer then walked the remaining unlockers against a bus that was gone.
+pub fn is_renesas(scsi: &mut dyn ScsiTransport) -> std::result::Result<bool, UnlockError> {
     let mut buf = [0u8; RB_F1_LEN];
     match scsi.execute(&RB_F1_CDB, DataDirection::FromDevice, &mut buf, 5_000) {
         Ok(r) => {
             let end = RENESAS_MARKER_OFFSET + RENESAS_MARKER.len();
-            r.status == 0
+            Ok(r.status == 0
                 && r.bytes_transferred >= end
-                && &buf[RENESAS_MARKER_OFFSET..end] == RENESAS_MARKER
+                && &buf[RENESAS_MARKER_OFFSET..end] == RENESAS_MARKER)
+        }
+        // Only a senseless transport-failure status is a dead bus; anything
+        // else the transport reports as `Err` is the drive refusing.
+        Err(e) => {
+            if e.status == crate::scsi::SCSI_STATUS_TRANSPORT_FAILURE && e.sense.is_none() {
+                tracing::warn!(
+                    target: "freemkv::disc",
+                    phase = "renesas_probe_transport_fault",
+                    "transport fault on the Renesas identity probe; aborting"
+                );
+                return Err(UnlockError::Transport);
+            }
+            tracing::debug!(
+                target: "freemkv::disc",
+                phase = "renesas_probe_rejected",
+                status = e.status,
+                "Renesas identity probe rejected by the drive; not a Renesas platform"
+            );
+            Ok(false)
         }
-        Err(_) => false,
     }
 }
 
@@ -60,7 +85,7 @@ impl Unlocker for Renesis {
         scsi: &mut dyn ScsiTransport,
         _ctx: &UnlockCtx,
     ) -> std::result::Result<Unlocked, UnlockError> {
-        if !is_renesas(scsi) {
+        if !is_renesas(scsi)? {
             return Err(UnlockError::NotApplicable);
         }
         tracing::debug!(
@@ -104,7 +129,11 @@ mod tests {
         }
     }
 
-    /// Rejects the command (a MediaTek drive → ILLEGAL REQUEST → transport `Err`).
+    /// Rejects the command the way a MediaTek drive does: ILLEGAL REQUEST. The
+    /// sense is what distinguishes this from a dead bus — the old fixture sent
+    /// `sense: None`, which is the wire shape of a TRANSPORT fault, and the test
+    /// built on it asserted `NotApplicable`, pinning exactly the
+    /// misclassification this file had.
     struct RejectingTransport;
     impl ScsiTransport for RejectingTransport {
         fn execute(
@@ -114,9 +143,12 @@ mod tests {
             _data: &mut [u8],
             _timeout_ms: u32,
         ) -> Result<ScsiResult> {
+            let mut sense = [0u8; 32];
+            sense[2] = 0x05; // ILLEGAL REQUEST
+            sense[12] = 0x20; // invalid command operation code
             Err(ScsiError {
                 status: crate::scsi::SCSI_STATUS_CHECK_CONDITION,
-                sense: None,
+                sense: Some(sense),
             })
         }
     }
@@ -133,13 +165,13 @@ mod tests {
         let mut t = RenesasTransport {
             payload: renesas_payload(),
         };
-        assert!(is_renesas(&mut t));
+        assert!(is_renesas(&mut t).expect("no transport fault"));
     }
 
     #[test]
     fn is_renesas_false_when_command_rejected() {
         let mut t = RejectingTransport;
-        assert!(!is_renesas(&mut t));
+        assert!(!is_renesas(&mut t).expect("a drive rejection is not a bus fault"));
     }
 
     #[test]
@@ -148,7 +180,7 @@ mod tests {
         let mut t = RenesasTransport {
             payload: vec![0u8; 48],
         };
-        assert!(!is_renesas(&mut t));
+        assert!(!is_renesas(&mut t).expect("no transport fault"));
     }
 
     #[test]
@@ -157,7 +189,7 @@ mod tests {
         let mut t = RenesasTransport {
             payload: vec![0x20u8; 8],
         };
-        assert!(!is_renesas(&mut t));
+        assert!(!is_renesas(&mut t).expect("no transport fault"));
     }
 
     #[test]
@@ -179,6 +211,8 @@ mod tests {
         assert_eq!(u.bus_key, None);
     }
 
+    /// A drive REJECTION (ILLEGAL REQUEST, with a sense) is "not a Renesas
+    /// drive" → fall through to the next unlocker.
     #[test]
     fn features_not_applicable_on_non_renesas() {
         let mut t = RejectingTransport;
@@ -188,6 +222,35 @@ mod tests {
         assert_eq!(err, UnlockError::NotApplicable);
     }
 
+    /// The same rejection delivered the way a CONFORMING transport delivers it —
+    /// `Ok` with a CHECK CONDITION status — must reach the same answer. Catches
+    /// dropping the `r.status == 0` check, which would read the probe buffer's
+    /// zero fill as an identity block.
+    #[test]
+    fn check_condition_is_not_a_renesas_drive() {
+        use crate::scsi::mock::{MockTransport, Reply};
+        let mut t = MockTransport::always(Reply::illegal_request());
+        assert!(!is_renesas(&mut t).expect("a drive sense is not a bus fault"));
+    }
+
+    /// THE defect-8 test: a dead bus on the FIRST command the unlocker issues
+    /// must abort the consumer, not be reported as "not a Renesas drive".
+    /// Catches restoring the `Err(_) => false` arm.
+    #[test]
+    fn transport_fault_aborts_instead_of_declining() {
+        use crate::scsi::mock::{MockTransport, Reply};
+        let mut t = MockTransport::always(Reply::TransportFault);
+        assert_eq!(is_renesas(&mut t).unwrap_err(), UnlockError::Transport);
+
+        let mut t = MockTransport::always(Reply::TransportFault);
+        let id = crate::DriveId::default();
+        let ctx = UnlockCtx::new(&id, DiscKind::Unknown, &[]);
+        assert_eq!(
+            Renesis::new().unlock_features(&mut t, &ctx).unwrap_err(),
+            UnlockError::Transport
+        );
+    }
+
     #[test]
     fn does_not_provide_bus_removal() {
         // Renesas leaves bus encryption to the cert: unlock_bus is the default.

src/scsi.rs

@@ -98,3 +98,270 @@ pub(crate) fn build_set_cd_speed(read_speed: u16) -> [u8; 12] {
         0x00,
     ]
 }
+
+// ── Test fixture ────────────────────────────────────────────────────────────
+//
+// The crate-wide mock transport. Before this existed, EVERY mock in the crate
+// returned `Ok` with status 0 and a full `bytes_transferred` — so no test could
+// ever exercise the three outcomes the contract actually distinguishes, and a
+// whole class of transport-fault misclassification bugs sat uncaught (a dead bus
+// read as "this unlocker doesn't apply", a zero-filled buffer read as valid
+// drive data). This fixture can express all of them, and is the red-before-green
+// vehicle for the transport-contract fixes across ld / aacs / css / renesis.
+#[cfg(test)]
+#[allow(dead_code)] // a fixture: each helper is used by a subset of the modules
+pub(crate) mod mock {
+    use super::*;
+    use std::collections::VecDeque;
+
+    /// One scripted answer to one `execute()` call.
+    #[derive(Debug, Clone)]
+    pub(crate) enum Reply {
+        /// `Ok` + GOOD status. `payload` is copied into the caller's buffer;
+        /// `bytes_transferred` defaults to the number of bytes copied.
+        Data {
+            payload: Vec<u8>,
+            bytes_transferred: Option<usize>,
+        },
+        /// `Ok` + a NON-ZERO SCSI status (CHECK CONDITION) with a drive sense.
+        /// Per the contract at [`ScsiTransport::execute`] this is NOT a
+        /// transport fault — the caller must inspect `status`, and a caller that
+        /// doesn't will consume the caller's zero-filled buffer as drive data.
+        Sense {
+            status: u8,
+            sense_key: u8,
+            asc: u8,
+            ascq: u8,
+        },
+        /// `Err` with the transport-failure status and no sense — a genuine
+        /// transport-layer fault (bridge crash / disconnect). MUST abort.
+        TransportFault,
+        /// `Err` carrying a real status + parsed sense. This is what a
+        /// NON-conforming transport does (libfreemkv's adapter returns `Err` for
+        /// any non-zero SCSI status); classification must still treat it as a
+        /// drive rejection, not a dead bus.
+        ErrWithSense {
+            status: u8,
+            sense_key: u8,
+            asc: u8,
+            ascq: u8,
+        },
+    }
+
+    impl Reply {
+        /// `Ok`, GOOD status, full transfer of `payload`.
+        pub(crate) fn good(payload: Vec<u8>) -> Reply {
+            Reply::Data {
+                payload,
+                bytes_transferred: None,
+            }
+        }
+        /// `Ok`, GOOD status, but the drive only delivered `n` bytes.
+        pub(crate) fn short(payload: Vec<u8>, n: usize) -> Reply {
+            Reply::Data {
+                payload,
+                bytes_transferred: Some(n),
+            }
+        }
+        /// `Ok`, GOOD status, ZERO bytes delivered — the buffer the caller reads
+        /// is entirely its own zero fill.
+        pub(crate) fn zero_transfer(len: usize) -> Reply {
+            Reply::Data {
+                payload: vec![0u8; len],
+                bytes_transferred: Some(0),
+            }
+        }
+        /// `Ok` + CHECK CONDITION / ILLEGAL REQUEST (0x05, ASC 0x20 invalid
+        /// command) — the ordinary way a drive refuses a vendor command.
+        pub(crate) fn illegal_request() -> Reply {
+            Reply::Sense {
+                status: SCSI_STATUS_CHECK_CONDITION,
+                sense_key: 0x05,
+                asc: 0x20,
+                ascq: 0x00,
+            }
+        }
+        /// `Err` + CHECK CONDITION / ILLEGAL REQUEST — the same drive refusal as
+        /// seen through a non-conforming transport.
+        pub(crate) fn illegal_request_as_err() -> Reply {
+            Reply::ErrWithSense {
+                status: SCSI_STATUS_CHECK_CONDITION,
+                sense_key: 0x05,
+                asc: 0x20,
+                ascq: 0x00,
+            }
+        }
+    }
+
+    fn sense_buf(sense_key: u8, asc: u8, ascq: u8) -> [u8; 32] {
+        let mut b = [0u8; 32];
+        b[2] = sense_key & 0x0F;
+        b[12] = asc;
+        b[13] = ascq;
+        b
+    }
+
+    /// A scripted transport: answers each `execute()` from `script`, falling
+    /// back to `default` once the script runs out, and records every CDB.
+    pub(crate) struct MockTransport {
+        pub(crate) script: VecDeque<Reply>,
+        pub(crate) default: Reply,
+        pub(crate) cdbs: Vec<Vec<u8>>,
+    }
+
+    impl MockTransport {
+        /// Every command gets the same answer.
+        pub(crate) fn always(reply: Reply) -> Self {
+            MockTransport {
+                script: VecDeque::new(),
+                default: reply,
+                cdbs: Vec::new(),
+            }
+        }
+        /// The first N commands follow `script`; the rest get `default`.
+        pub(crate) fn scripted(script: Vec<Reply>, default: Reply) -> Self {
+            MockTransport {
+                script: script.into(),
+                default,
+                cdbs: Vec::new(),
+            }
+        }
+        /// How many CDBs were issued (lets a test assert a dead bus aborted
+        /// instead of retrying).
+        pub(crate) fn calls(&self) -> usize {
+            self.cdbs.len()
+        }
+    }
+
+    impl ScsiTransport for MockTransport {
+        fn execute(
+            &mut self,
+            cdb: &[u8],
+            _direction: DataDirection,
+            data: &mut [u8],
+            _timeout_ms: u32,
+        ) -> Result<ScsiResult> {
+            self.cdbs.push(cdb.to_vec());
+            let reply = self
+                .script
+                .pop_front()
+                .unwrap_or_else(|| self.default.clone());
+            match reply {
+                Reply::Data {
+                    payload,
+                    bytes_transferred,
+                } => {
+                    let n = payload.len().min(data.len());
+                    data[..n].copy_from_slice(&payload[..n]);
+                    Ok(ScsiResult {
+                        status: 0,
+                        bytes_transferred: bytes_transferred.unwrap_or(n),
+                        sense: [0u8; 32],
+                    })
+                }
+                Reply::Sense {
+                    status,
+                    sense_key,
+                    asc,
+                    ascq,
+                } => Ok(ScsiResult {
+                    status,
+                    bytes_transferred: 0,
+                    sense: sense_buf(sense_key, asc, ascq),
+                }),
+                Reply::TransportFault => Err(ScsiError {
+                    status: SCSI_STATUS_TRANSPORT_FAILURE,
+                    sense: None,
+                }),
+                Reply::ErrWithSense {
+                    status,
+                    sense_key,
+                    asc,
+                    ascq,
+                } => Err(ScsiError {
+                    status,
+                    sense: Some(sense_buf(sense_key, asc, ascq)),
+                }),
+            }
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use mock::{MockTransport, Reply};
+
+    /// The fixed-format sense layout the whole crate reads its diagnostics off
+    /// (key at byte 2 low nibble, ASC at 12, ASCQ at 13). This file had no test
+    /// at all, so nothing pinned the offsets.
+    #[test]
+    fn sense_parses_at_the_fixed_format_offsets() {
+        let mut buf = [0u8; 32];
+        buf[2] = 0xF5; // sense key is the LOW nibble only
+        buf[12] = 0x24;
+        buf[13] = 0x01;
+        let s = ScsiSense::from_buf(&buf);
+        assert_eq!(s.sense_key, 0x05);
+        assert_eq!(s.asc, 0x24);
+        assert_eq!(s.ascq, 0x01);
+        assert!(s.is_illegal_request());
+
+        buf[2] = 0x02; // NOT READY
+        assert!(!ScsiSense::from_buf(&buf).is_illegal_request());
+    }
+
+    /// SET CD SPEED encodes the read speed big-endian at bytes 2-3 and leaves
+    /// the write speed at 0xFFFF.
+    #[test]
+    fn set_cd_speed_encodes_the_read_speed_big_endian() {
+        let cdb = build_set_cd_speed(0x1234);
+        assert_eq!(cdb[0], SCSI_SET_CD_SPEED);
+        assert_eq!([cdb[2], cdb[3]], [0x12, 0x34]);
+        assert_eq!([cdb[4], cdb[5]], [0xFF, 0xFF]);
+        assert_eq!(build_set_cd_speed(0xFFFF)[2..4], [0xFF, 0xFF]);
+    }
+
+    /// The fixture itself must honour the contract it exists to test: a drive
+    /// sense is `Ok` with a non-zero status, a bus fault is `Err`. If this ever
+    /// drifts, every transport-contract test built on it silently stops testing
+    /// anything.
+    #[test]
+    fn mock_transport_expresses_the_three_contract_outcomes() {
+        let mut t = MockTransport::scripted(
+            vec![
+                Reply::good(vec![0xAB; 4]),
+                Reply::illegal_request(),
+                Reply::zero_transfer(4),
+            ],
+            Reply::TransportFault,
+        );
+        let mut buf = [0u8; 4];
+
+        let r = t
+            .execute(&[0x3C], DataDirection::FromDevice, &mut buf, 0)
+            .expect("data reply is Ok");
+        assert_eq!(r.status, 0);
+        assert_eq!(r.bytes_transferred, 4);
+        assert_eq!(buf, [0xAB; 4]);
+
+        let r = t
+            .execute(&[0x3C], DataDirection::FromDevice, &mut buf, 0)
+            .expect("A DRIVE SENSE IS Ok, NOT Err — the load-bearing contract");
+        assert_eq!(r.status, SCSI_STATUS_CHECK_CONDITION);
+        assert!(ScsiSense::from_buf(&r.sense).is_illegal_request());
+
+        let r = t
+            .execute(&[0x3C], DataDirection::FromDevice, &mut buf, 0)
+            .expect("a zero-length transfer is still Ok");
+        assert_eq!(r.bytes_transferred, 0);
+
+        let e = t
+            .execute(&[0x3C], DataDirection::FromDevice, &mut buf, 0)
+            .expect_err("a transport fault is Err");
+        assert_eq!(e.status, SCSI_STATUS_TRANSPORT_FAILURE);
+        assert!(e.sense.is_none(), "a bus fault carries no drive sense");
+
+        assert_eq!(t.calls(), 4);
+    }
+}