From 77a29a324b8bd9efaf59ae271d392e788f3e2e98 Mon Sep 17 00:00:00 2001
From: foxbyte <198093090+adiraj66132@users.noreply.github.com>
Date: Thu, 22 Oct 2026 21:13:54 +0000
Subject: [PATCH] fix(wal): reject non-string keys and values on the write path

#344 added the check only on replay, so a non-string key or value could
still be written. The next replay called that line corrupt and truncated
the log there, dropping every valid write after it.

Validate in SetEntry/DeleteEntry instead, before anything is written or
applied, so insert/delete raise TypeError and leave memory and the log
untouched.
---
 src/seriousdb/cache.py |  4 ++++
 src/seriousdb/wal.py   | 29 ++++++++++++++---------------
 tests/test_cache.py    | 24 ++++++++++++++++++++++++
 tests/test_wal.py      | 11 +++++++++++
 4 files changed, 53 insertions(+), 15 deletions(-)

diff --git a/src/seriousdb/cache.py b/src/seriousdb/cache.py
index 9c1d9c6..301a1a7 100644
--- a/src/seriousdb/cache.py
+++ b/src/seriousdb/cache.py
@@ -77,6 +77,8 @@ def insert(self, key: str, value: str) -> tuple[str, bool]:
 
         Raises
         ------
+        TypeError
+            If `key` or `value` is not a string.
         ServiceUnavailableError
             If no database has been loaded.
         OSError
@@ -137,6 +139,8 @@ def delete(self, key: str) -> str:
 
         Raises
         ------
+        TypeError
+            If `key` is not a string.
         ResourceNotFoundError
             If `key` does not exist.
         ServiceUnavailableError
diff --git a/src/seriousdb/wal.py b/src/seriousdb/wal.py
index 27a8e94..99684b3 100644
--- a/src/seriousdb/wal.py
+++ b/src/seriousdb/wal.py
@@ -41,23 +41,10 @@ def from_dict(data: dict) -> "WalEntry":
         op = data.get("op")
         try:
             if op == "set":
-                key = data["key"]
-                value = data["value"]
-
-                if not isinstance(key, str):
-                    raise TypeError("key must be a string")
-                if not isinstance(value, str):
-                    raise TypeError("value must be a string")
-
-                return SetEntry(key=key, value=value)
+                return SetEntry(key=data["key"], value=data["value"])
 
             if op == "delete":
-                key = data["key"]
-
-                if not isinstance(key, str):
-                    raise TypeError("key must be a string")
-
-                return DeleteEntry(key=key)
+                return DeleteEntry(key=data["key"])
 
         except KeyError as e:
             raise ValueError(f"missing required field: {e}") from e
@@ -72,6 +59,13 @@ class SetEntry(WalEntry):
     key: str
     value: str
 
+    def __post_init__(self) -> None:
+        """Reject keys and values that are not strings."""
+        if not isinstance(self.key, str):
+            raise TypeError("key must be a string")
+        if not isinstance(self.value, str):
+            raise TypeError("value must be a string")
+
     def to_dict(self) -> dict:
         """Return the JSON-serializable representation of this entry."""
         return {"op": "set", "key": self.key, "value": self.value}
@@ -87,6 +81,11 @@ class DeleteEntry(WalEntry):
 
     key: str
 
+    def __post_init__(self) -> None:
+        """Reject keys that are not strings."""
+        if not isinstance(self.key, str):
+            raise TypeError("key must be a string")
+
     def to_dict(self) -> dict:
         """Return the JSON-serializable representation of this entry."""
         return {"op": "delete", "key": self.key}
diff --git a/tests/test_cache.py b/tests/test_cache.py
index 6c4e71d..b7b3f22 100644
--- a/tests/test_cache.py
+++ b/tests/test_cache.py
@@ -413,6 +413,30 @@ def boom(op):
     assert reloaded.db["name"] == "Alice"
 
 
+def test_rejected_write_leaves_the_wal_healthy(db_path):
+    """A non-string write must be rejected, and must not lose later writes."""
+    cache = Cache()
+    cache.load(str(db_path))
+
+    assert cache.db is not None
+
+    cache.insert("a", "1")
+
+    with pytest.raises(TypeError):
+        cache.insert(5, "bad")  # ty: ignore[invalid-argument-type]
+    with pytest.raises(TypeError):
+        cache.insert("b", 2)  # ty: ignore[invalid-argument-type]
+
+    assert cache.db == {"a": "1"}
+
+    cache.insert("b", "2")
+
+    reloaded = Cache()
+    reloaded.load(str(db_path))
+
+    assert reloaded.db == {"a": "1", "b": "2"}
+
+
 def test_replay_repairs_torn_wal_before_later_appends(db_path):
     """Remove a torn final record so later WAL appends remain valid."""
     cache = Cache()
diff --git a/tests/test_wal.py b/tests/test_wal.py
index 7c8a680..4fe8a11 100644
--- a/tests/test_wal.py
+++ b/tests/test_wal.py
@@ -72,6 +72,7 @@ def test_replay_drops_and_truncates_a_torn_last_entry(wal, wal_path):
         b'{"op": "set", "key": null, "value": "b"}\n',  # Key is null
         b'{"op": "set", "key": 5, "value": "b"}\n',  # Key is not a string
         b'{"op": "set", "key": "b", "value": [1, 2]}\n',  # Value is not a string
+        b'{"op": "delete", "key": 5}\n',  # Delete key is not a string
     ],
 )
 def test_replay_recovers_from_malformed_entry(wal, wal_path, malformed):
@@ -95,6 +96,16 @@ def test_replay_recovers_from_malformed_entry(wal, wal_path, malformed):
     ]
 
 
+def test_entries_reject_non_string_key_or_value():
+    """Reject a key or value that is not a string."""
+    with pytest.raises(TypeError):
+        SetEntry(key=5, value="b")  # ty: ignore[invalid-argument-type]
+    with pytest.raises(TypeError):
+        SetEntry(key="b", value=[1, 2])  # ty: ignore[invalid-argument-type]
+    with pytest.raises(TypeError):
+        DeleteEntry(key=5)  # ty: ignore[invalid-argument-type]
+
+
 def test_append_after_repaired_truncation_stays_valid(wal, wal_path):
     wal.append(SetEntry(key="a", value="1"))
     wal.append(SetEntry(key="b", value="2"))