UAT: DatabaseResourceHandler._get_checkpoint_conn() uses fragile path suffix matching — can return wrong connection for databases with similar path suffixes #5937

Open
opened 2026-04-09 11:59:27 +00:00 by HAL9000 · 1 comment
Owner

Bug Report

Feature Area: Resource Management — Database Resources / Resource Lifecycle Management
Milestone Scope: v3.6.0 (database resource handler checkpoint, issue #1241)
Severity: Low — edge case that can cause incorrect checkpoint connection reuse


What Was Tested

Code-level analysis of src/cleveragents/resource/handlers/database.py DatabaseResourceHandler._get_checkpoint_conn() method.

Expected Behavior (from spec)

When write() is called on a SQLite database that has an open checkpoint (SAVEPOINT), the write should reuse the checkpoint connection so the write is within the SAVEPOINT transaction and can be rolled back by rollback_to().

The connection lookup should reliably identify the correct connection for the given database location.

Actual Behavior

_get_checkpoint_conn() uses this path matching logic:

for _seq, _name, db_file in rows:
    if db_file and (
        db_file == location
        or db_file.endswith(location.lstrip("/"))  # ← FRAGILE
    ):
        return conn

The db_file.endswith(location.lstrip("/")) check is fragile and can produce false positives. For example:

  • location = "/data/foo.db"location.lstrip("/") = "data/foo.db"
  • A different database at /other/data/foo.db would match because "/other/data/foo.db".endswith("data/foo.db") is True

This means a checkpoint connection for /other/data/foo.db could be returned when looking up /data/foo.db, causing writes to be executed on the wrong database connection.

Code Location

src/cleveragents/resource/handlers/database.py, DatabaseResourceHandler._get_checkpoint_conn() method.

Steps to Reproduce

  1. Create two SQLite databases: /data/foo.db and /other/data/foo.db
  2. Create a checkpoint on /other/data/foo.db (stores connection in _sqlite_checkpoints)
  3. Call write() on a resource with location /data/foo.db
  4. _get_checkpoint_conn("/data/foo.db") incorrectly returns the connection for /other/data/foo.db
  5. The write executes on the wrong database

Impact

  • Writes intended for one SQLite database may be executed on a different database's connection
  • This would corrupt the checkpoint state and potentially the wrong database
  • The bug only manifests when multiple SQLite databases with path suffix overlap are open simultaneously with active checkpoints

Fix

Use exact path comparison only, or use os.path.realpath() for canonical path comparison:

import os

def _get_checkpoint_conn(self, location: str) -> sqlite3.Connection | None:
    """Return the open checkpoint connection for a given SQLite location, if any."""
    canonical_location = os.path.realpath(location) if location != ":memory:" else location
    for conn, _savepoint in self._sqlite_checkpoints.values():
        try:
            rows = conn.execute("PRAGMA database_list").fetchall()
            for _seq, _name, db_file in rows:
                if db_file:
                    canonical_db = os.path.realpath(db_file)
                    if canonical_db == canonical_location:
                        return conn
                elif location == ":memory:":
                    # In-memory databases match by connection identity
                    return conn
        except Exception:
            pass
    return None

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: uat-tester

## Bug Report **Feature Area**: Resource Management — Database Resources / Resource Lifecycle Management **Milestone Scope**: v3.6.0 (database resource handler checkpoint, issue #1241) **Severity**: Low — edge case that can cause incorrect checkpoint connection reuse --- ## What Was Tested Code-level analysis of `src/cleveragents/resource/handlers/database.py` `DatabaseResourceHandler._get_checkpoint_conn()` method. ## Expected Behavior (from spec) When `write()` is called on a SQLite database that has an open checkpoint (SAVEPOINT), the write should reuse the checkpoint connection so the write is within the SAVEPOINT transaction and can be rolled back by `rollback_to()`. The connection lookup should reliably identify the correct connection for the given database location. ## Actual Behavior `_get_checkpoint_conn()` uses this path matching logic: ```python for _seq, _name, db_file in rows: if db_file and ( db_file == location or db_file.endswith(location.lstrip("/")) # ← FRAGILE ): return conn ``` The `db_file.endswith(location.lstrip("/"))` check is fragile and can produce **false positives**. For example: - `location = "/data/foo.db"` → `location.lstrip("/")` = `"data/foo.db"` - A different database at `/other/data/foo.db` would match because `"/other/data/foo.db".endswith("data/foo.db")` is `True` This means a checkpoint connection for `/other/data/foo.db` could be returned when looking up `/data/foo.db`, causing writes to be executed on the wrong database connection. ## Code Location `src/cleveragents/resource/handlers/database.py`, `DatabaseResourceHandler._get_checkpoint_conn()` method. ## Steps to Reproduce 1. Create two SQLite databases: `/data/foo.db` and `/other/data/foo.db` 2. Create a checkpoint on `/other/data/foo.db` (stores connection in `_sqlite_checkpoints`) 3. Call `write()` on a resource with location `/data/foo.db` 4. `_get_checkpoint_conn("/data/foo.db")` incorrectly returns the connection for `/other/data/foo.db` 5. The write executes on the wrong database ## Impact - Writes intended for one SQLite database may be executed on a different database's connection - This would corrupt the checkpoint state and potentially the wrong database - The bug only manifests when multiple SQLite databases with path suffix overlap are open simultaneously with active checkpoints ## Fix Use exact path comparison only, or use `os.path.realpath()` for canonical path comparison: ```python import os def _get_checkpoint_conn(self, location: str) -> sqlite3.Connection | None: """Return the open checkpoint connection for a given SQLite location, if any.""" canonical_location = os.path.realpath(location) if location != ":memory:" else location for conn, _savepoint in self._sqlite_checkpoints.values(): try: rows = conn.execute("PRAGMA database_list").fetchall() for _seq, _name, db_file in rows: if db_file: canonical_db = os.path.realpath(db_file) if canonical_db == canonical_location: return conn elif location == ":memory:": # In-memory databases match by connection identity return conn except Exception: pass return None ``` --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: uat-tester
Author
Owner

🏷️ Label compliance fix applied by backlog groomer (cycle 60)

This issue was missing its State/ label. The following label has been added:

  • State/Verified — UAT-confirmed fragile path suffix matching bug; existing Priority/Backlog and Type/Bug labels retained

Automated by CleverAgents Bot
Supervisor: Label Management | Agent: forgejo-label-manager

🏷️ **Label compliance fix applied by backlog groomer (cycle 60)** This issue was missing its `State/` label. The following label has been added: - `State/Verified` — UAT-confirmed fragile path suffix matching bug; existing `Priority/Backlog` and `Type/Bug` labels retained --- **Automated by CleverAgents Bot** Supervisor: Label Management | Agent: forgejo-label-manager
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
cleveragents/cleveragents-core#5937
No description provided.