Update tests for shared helpers and fix test nits

- Cover the new utilities helpers (unpad, crc32, checksumPid type
  preservation, pidFromSerial, safe_join) and drop the now-redundant
  per-module crc32 tests.
- Rename test_unpad_removes_pkcs7_padding to reflect that unpad only
  trusts the trailing pad-length byte rather than validating PKCS#7.
- Replace the KOBO_HASH_KEYS change-detector with a test that documents
  the intentional value pin and checks the ASCII-salt contract.
- Note kgenpids' hybrid import in the test-loader docstring.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

tests/dedrm_test_utils.py

@@ -4,14 +4,16 @@ The DeDRM and Obok plugins normally run inside calibre's bundled Python and use
 intra-package (``from .x import y``) imports. To unit-test the pure logic we:
 
   * put the plugin source directories on ``sys.path`` (for absolute imports such
-    as ``import kgenpids`` / ``import aescbc``);
+    as ``import kgenpids`` / ``import alfcrypto``);
   * provide a minimal ``calibre`` stub for the few modules that import it;
   * register a synthetic ``dedrm`` package whose ``__path__`` points at the
     DeDRM_plugin directory, so modules that use relative imports resolve without
     executing the real (calibre-dependent) ``__init__.py``.
 
-Modules with no relative imports (alfcrypto, kgenpids, argv_utils, aescbc, obok)
-are loaded top-level; modules with ``from .`` imports are loaded as ``dedrm.X``.
+Modules with no relative imports (alfcrypto, argv_utils, obok) are loaded
+top-level; modules with ``from .`` imports are loaded as ``dedrm.X``. kgenpids
+is a hybrid: it is imported absolutely by other worker modules, so it falls back
+to an absolute ``from utilities import ...`` and is still loaded top-level here.
 """
 
 import importlib

tests/test_ineptepub.py

@@ -5,5 +5,5 @@ import dedrm_test_utils as U
 epub = U.load("ineptepub", package="dedrm")
 
 
-def test_unpad_removes_pkcs7_padding():
+def test_unpad_strips_trailing_pad_length():
     assert epub.unpad(b"data\x04\x04\x04\x04") == b"data"

tests/test_ineptpdf.py

@@ -14,5 +14,5 @@ def test_nunpack_lengths():
     assert pdf.nunpack(b"\x01\x00\x00\x00") == 16777216
 
 
-def test_unpad_removes_pkcs7_padding():
+def test_unpad_strips_trailing_pad_length():
     assert pdf.unpad(b"hello\x03\x03\x03") == b"hello"

tests/test_kgenpids.py

@@ -33,7 +33,3 @@ def test_pid_encryption_table_is_crc32_table():
 def test_generate_device_pid_known_vector():
     table = kg.generatePidEncryptionTable()
     assert kg.generateDevicePID(table, b"\x12\x34\x56\x78", 4) == b"22I8IQVF"
-
-
-def test_crc32_known_vector():
-    assert kg.crc32(b"test") == 4181434640

tests/test_kindlepid.py

@@ -5,10 +5,6 @@ import dedrm_test_utils as U
 kp = U.load("kindlepid", package="dedrm")
 
 
-def test_letters_charset():
-    assert kp.letters == b"ABCDEFGHIJKLMNPQRSTUVWXYZ123456789"
-
-
 def test_checksum_pid_appends_two_chars():
     pid = kp.checksumPid(b"12345678")
     assert pid == b"12345678EL"
@@ -17,7 +13,3 @@ def test_checksum_pid_appends_two_chars():
 
 def test_pid_from_serial_known_vector():
     assert kp.pidFromSerial(b"B00212345678", 8) == b"VBKTRX7Q"
-
-
-def test_crc32_known_vector():
-    assert kp.crc32(b"test") == 4181434640

tests/test_mobidedrm.py

@@ -30,5 +30,9 @@ def test_trailing_data_entries():
     assert md.getSizeOfTrailingDataEntries(b"ABCDE", 5, 1) == 2
 
 
-def test_crc32_known_vector():
-    assert md.crc32(b"test") == 4181434640
+def test_checksum_pid_str_contract():
+    # mobidedrm's call sites pass and expect ``str``; the shared helper must
+    # preserve that (unlike the bytes contract used by kgenpids/kindlepid).
+    out = md.checksumPid("12345678")
+    assert out == "12345678EL"
+    assert isinstance(out, str)

tests/test_obok.py

@@ -7,12 +7,23 @@ import dedrm_test_utils as U
 obok = U.load("obok")
 
 
-def test_unpad_removes_pkcs7_padding():
+def test_unpad_strips_trailing_pad_length():
+    # obok.unpad trusts the final byte as the pad length; it is not a validating
+    # PKCS#7 implementation, so the name reflects what it actually does.
     assert obok.unpad(b"data\x04\x04\x04\x04") == b"data"
 
 
-def test_kobo_hash_keys_present():
-    assert obok.KOBO_HASH_KEYS == ["88b3a2e13", "XzUhGYdFp", "NoCanLook", "QJhwzAtXL"]
+def test_kobo_hash_keys_are_stable_ascii_salts():
+    # These four salts are baked into Kobo's userkey derivation
+    # (sha256(hash + macaddr) -> deviceid). Pinning the exact values is
+    # intentional: a silent typo here would break decryption for every user, so
+    # we want a hard regression guard rather than only a shape check.
+    keys = obok.KOBO_HASH_KEYS
+    assert keys == ["88b3a2e13", "XzUhGYdFp", "NoCanLook", "QJhwzAtXL"]
+    # The derivation feeds each salt to ``.encode('ascii')``; pin that contract.
+    for k in keys:
+        assert isinstance(k, str) and k
+        k.encode("ascii")
 
 
 class _FakeStream:

tests/test_utilities.py

@@ -1,4 +1,6 @@
-"""Tests for DeDRM utilities (uStrCmp)."""
+"""Tests for DeDRM utilities (uStrCmp and the shared crypto/path helpers)."""
+
+import os
 
 import dedrm_test_utils as U
 
@@ -17,3 +19,52 @@ def test_ustrcmp_caseless():
 def test_ustrcmp_unicode_normalisation():
     # Composed 'é' (U+00E9) vs decomposed 'e' + combining acute (U+0065 U+0301).
     assert ut.uStrCmp("é", "é", caseless=False) is True
+
+
+def test_unpad_strips_trailing_pad_length():
+    assert ut.unpad(b"data\x04\x04\x04\x04") == b"data"
+    assert ut.unpad(b"hello\x03\x03\x03") == b"hello"
+
+
+def test_crc32_known_vector():
+    assert ut.crc32(b"test") == 4181434640
+
+
+def test_pid_alphabet():
+    assert ut.PID_ALPHABET == b"ABCDEFGHIJKLMNPQRSTUVWXYZ123456789"
+
+
+def test_checksum_pid_preserves_bytes():
+    out = ut.checksumPid(b"12345678")
+    assert out == b"12345678EL"
+    assert isinstance(out, bytes)
+
+
+def test_checksum_pid_preserves_str():
+    out = ut.checksumPid("12345678")
+    assert out == "12345678EL"
+    assert isinstance(out, str)
+
+
+def test_pid_from_serial_known_vector():
+    assert ut.pidFromSerial(b"B00212345678", 8) == b"VBKTRX7Q"
+
+
+def test_safe_join_plain_name(tmp_path):
+    assert ut.safe_join(str(tmp_path), "book0000.dat") == os.path.join(
+        str(tmp_path), "book0000.dat"
+    )
+
+
+def test_safe_join_strips_directory_components(tmp_path):
+    # Any directory part (including ``../``) is reduced to a bare basename.
+    result = ut.safe_join(str(tmp_path), "../../etc/evil")
+    assert os.path.basename(result) == "evil"
+    assert os.path.dirname(os.path.abspath(result)) == os.path.abspath(str(tmp_path))
+
+
+def test_safe_join_result_always_contained(tmp_path):
+    root = os.path.abspath(str(tmp_path))
+    for name in ["../../evil", "a/b/c", "..\\..\\win", "/abs/path", "plain"]:
+        result = os.path.abspath(ut.safe_join(str(tmp_path), name))
+        assert result == root or result.startswith(root + os.sep)