Skip to content

Commit ea35ee5

Browse files
fix(dbapi): harden commit/rollback against message-only errors; forward max_total_rows
- ISSUE-97: commit/rollback silent no-op is now gated on the SQLite result code first (SQLITE_ERROR=1 or SQLITE_MISUSE=21), then on the "no transaction is active" substring. Previously, a malicious or impostor server could force the client to silently succeed on any failure just by crafting a message containing that substring — even SQLITE_FULL (disk full), SQLITE_CONSTRAINT, or other fatal codes. Extract the check into a shared _is_no_transaction_error helper used by both the sync and async connection classes. - ISSUE-111: module-level connect() forwards max_total_rows to the underlying Connection. Previously users of the PEP 249 convenience function silently got the default; only direct Connection() use let them override. SQLAlchemy's URL already forwarded this — the dbapi convenience was the gap. Adds regression tests in tests/test_cycle3_hardening.py covering the disk-full-crafted-substring case, the constraint-crafted-substring case, and connect(max_total_rows=...) forwarding. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 8df2c28 commit ea35ee5

File tree

4 files changed

+100
-5
lines changed

4 files changed

+100
-5
lines changed

src/dqlitedbapi/__init__.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ def connect(
9494
*,
9595
database: str = "default",
9696
timeout: float = 10.0,
97+
max_total_rows: int | None = 10_000_000,
9798
) -> Connection:
9899
"""Connect to a dqlite database.
99100
@@ -104,6 +105,9 @@ def connect(
104105
finite number. ``0``, negatives, and non-finite values are
105106
rejected here rather than silently passed through to the
106107
underlying connection.
108+
max_total_rows: Cumulative row cap across continuation frames
109+
for a single query. Forwarded to the underlying
110+
:class:`Connection` (ISSUE-111). ``None`` disables the cap.
107111
108112
Returns:
109113
A Connection object
@@ -112,4 +116,9 @@ def connect(
112116

113117
if not math.isfinite(timeout) or timeout <= 0:
114118
raise ProgrammingError(f"timeout must be a positive finite number, got {timeout}")
115-
return Connection(address, database=database, timeout=timeout)
119+
return Connection(
120+
address,
121+
database=database,
122+
timeout=timeout,
123+
max_total_rows=max_total_rows,
124+
)

src/dqlitedbapi/aio/connection.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
from dqliteclient import DqliteConnection
1010
from dqliteclient.protocol import _validate_max_total_rows
1111
from dqlitedbapi.aio.cursor import AsyncCursor
12+
from dqlitedbapi.connection import _is_no_transaction_error
1213
from dqlitedbapi.exceptions import InterfaceError, OperationalError, ProgrammingError
1314

1415

@@ -130,7 +131,7 @@ async def commit(self) -> None:
130131
try:
131132
await self._async_conn.execute("COMMIT")
132133
except (OperationalError, _client_exc.OperationalError) as e:
133-
if "no transaction is active" not in str(e).lower():
134+
if not _is_no_transaction_error(e):
134135
raise
135136

136137
async def rollback(self) -> None:
@@ -144,7 +145,7 @@ async def rollback(self) -> None:
144145
try:
145146
await self._async_conn.execute("ROLLBACK")
146147
except (OperationalError, _client_exc.OperationalError) as e:
147-
if "no transaction is active" not in str(e).lower():
148+
if not _is_no_transaction_error(e):
148149
raise
149150

150151
def cursor(self) -> AsyncCursor:

src/dqlitedbapi/connection.py

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,30 @@
1414
from dqlitedbapi.cursor import Cursor
1515
from dqlitedbapi.exceptions import InterfaceError, OperationalError, ProgrammingError
1616

17+
# SQLite result codes for "you tried to COMMIT/ROLLBACK but there's no
18+
# transaction active." SQLite returns ``SQLITE_ERROR`` (1) most of the
19+
# time; some code paths return ``SQLITE_MISUSE`` (21). Check the
20+
# numeric code first so a malicious or impostor server cannot silence
21+
# unrelated errors just by crafting a message string that contains the
22+
# magic substring (ISSUE-97). The substring remains as a secondary
23+
# filter because SQLite has many uses of code=1.
24+
_NO_TX_CODES = frozenset({1, 21})
25+
_NO_TX_SUBSTRING = "no transaction is active"
26+
27+
28+
def _is_no_transaction_error(exc: Exception) -> bool:
29+
"""True if ``exc`` is a genuine "no active transaction" server reply.
30+
31+
Gates the silent swallow on the SQLite result code in addition to
32+
the English wording. A disk-full / constraint / IO error whose
33+
message happens to include the magic substring will not be
34+
swallowed.
35+
"""
36+
code = getattr(exc, "code", None)
37+
if code is not None and code not in _NO_TX_CODES:
38+
return False
39+
return _NO_TX_SUBSTRING in str(exc).lower()
40+
1741

1842
def _cleanup_loop_thread(
1943
loop: asyncio.AbstractEventLoop,
@@ -273,7 +297,7 @@ async def _commit_async(self) -> None:
273297
try:
274298
await self._async_conn.execute("COMMIT")
275299
except (OperationalError, _client_exc.OperationalError) as e:
276-
if "no transaction is active" not in str(e).lower():
300+
if not _is_no_transaction_error(e):
277301
raise
278302

279303
def rollback(self) -> None:
@@ -295,7 +319,7 @@ async def _rollback_async(self) -> None:
295319
try:
296320
await self._async_conn.execute("ROLLBACK")
297321
except (OperationalError, _client_exc.OperationalError) as e:
298-
if "no transaction is active" not in str(e).lower():
322+
if not _is_no_transaction_error(e):
299323
raise
300324

301325
def cursor(self) -> Cursor:

tests/test_cycle3_hardening.py

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,67 @@ def test_out_of_range_raises_data_error(self) -> None:
183183
_datetime_from_unixtime(2**63)
184184

185185

186+
class TestIsNoTransactionError:
187+
"""ISSUE-97: commit/rollback no-op is gated on SQLite result code first.
188+
189+
A malicious/impostor server must not be able to silence an unrelated
190+
error just by crafting a ``message`` that contains the magic
191+
substring.
192+
"""
193+
194+
def test_substring_only_with_sqlite_error_is_no_tx(self) -> None:
195+
from dqliteclient.exceptions import OperationalError as ClientOpError
196+
from dqlitedbapi.connection import _is_no_transaction_error
197+
198+
# code=1 (SQLITE_ERROR) + substring → true
199+
err = ClientOpError(1, "cannot commit - no transaction is active")
200+
assert _is_no_transaction_error(err)
201+
202+
# code=21 (SQLITE_MISUSE) + substring → true
203+
err = ClientOpError(21, "misuse: no transaction is active")
204+
assert _is_no_transaction_error(err)
205+
206+
def test_disk_full_with_matching_substring_is_not_silenced(self) -> None:
207+
from dqliteclient.exceptions import OperationalError as ClientOpError
208+
from dqlitedbapi.connection import _is_no_transaction_error
209+
210+
# Code 13 (SQLITE_FULL) must NOT be silenced even if the message
211+
# happens to contain the magic substring (attacker-controlled).
212+
err = ClientOpError(13, "disk full — but no transaction is active")
213+
assert not _is_no_transaction_error(err)
214+
215+
def test_constraint_violation_is_not_silenced(self) -> None:
216+
from dqliteclient.exceptions import OperationalError as ClientOpError
217+
from dqlitedbapi.connection import _is_no_transaction_error
218+
219+
err = ClientOpError(19, "constraint: no transaction is active anywhere")
220+
assert not _is_no_transaction_error(err)
221+
222+
223+
class TestConnectForwardsMaxTotalRows:
224+
"""ISSUE-111: module-level connect() forwards max_total_rows."""
225+
226+
def test_connect_forwards_max_total_rows(self) -> None:
227+
from dqlitedbapi import connect
228+
from dqlitedbapi.connection import Connection
229+
230+
conn = connect("localhost:19001", max_total_rows=500)
231+
try:
232+
assert isinstance(conn, Connection)
233+
assert conn._max_total_rows == 500
234+
finally:
235+
conn.close()
236+
237+
def test_connect_forwards_none_for_max_total_rows(self) -> None:
238+
from dqlitedbapi import connect
239+
240+
conn = connect("localhost:19001", max_total_rows=None)
241+
try:
242+
assert conn._max_total_rows is None
243+
finally:
244+
conn.close()
245+
246+
186247
class TestIsRowReturning:
187248
@pytest.mark.parametrize(
188249
"sql",

0 commit comments

Comments
 (0)