diff --git a/fleet/_async/resources/sqlite.py b/fleet/_async/resources/sqlite.py index c2719c9a..e7a0ed25 100644 --- a/fleet/_async/resources/sqlite.py +++ b/fleet/_async/resources/sqlite.py @@ -458,18 +458,18 @@ async def _expect_no_changes(self): removed_tables = before_tables - after_tables for table in added_tables: - if not self.ignore_config.should_ignore_table(table): + if not self.ignore_config.should_ignore_table(table) and not self.ignore_config.is_incidental_table(table): raise AssertionError(f"Unexpected table added: {table}") for table in removed_tables: - if not self.ignore_config.should_ignore_table(table): + if not self.ignore_config.should_ignore_table(table) and not self.ignore_config.is_incidental_table(table): raise AssertionError(f"Unexpected table removed: {table}") # Prepare tables to check tables_to_check = [] all_tables = before_tables | after_tables for table in all_tables: - if not self.ignore_config.should_ignore_table(table): + if not self.ignore_config.should_ignore_table(table) and not self.ignore_config.is_incidental_table(table): tables_to_check.append(table) # If no tables to check, we're done @@ -736,6 +736,7 @@ async def check_row( if ( table not in changes_by_table and not self.ignore_config.should_ignore_table(table) + and not self.ignore_config.is_incidental_table(table) ): tables_to_verify.append(table) @@ -805,8 +806,11 @@ def _is_change_allowed( # Collect all unexpected changes unexpected_changes = [] + mentioned_tables = {c.get("table") for c in allowed_changes} for tbl, report in diff.items(): + if tbl not in mentioned_tables and self.ignore_config.is_incidental_table(tbl): + continue for row in report.get("modified_rows", []): for f, vals in row["changes"].items(): if self.ignore_config.should_ignore_field(tbl, f): @@ -1185,6 +1189,7 @@ async def check_row( if ( table not in changes_by_table and not self.ignore_config.should_ignore_table(table) + and not self.ignore_config.is_incidental_table(table) ): tables_to_verify.append(table) @@ -1469,8 +1474,11 @@ def _validate_modification_with_fields_spec( # Collect all unexpected changes for detailed reporting unexpected_changes = [] + mentioned_tables = {c.get("table") for c in allowed_changes} for tbl, report in diff.items(): + if tbl not in mentioned_tables and self.ignore_config.is_incidental_table(tbl): + continue for row in report.get("modified_rows", []): row_changes = row["changes"] diff --git a/fleet/resources/sqlite.py b/fleet/resources/sqlite.py index a2b4d99f..79a8ea53 100644 --- a/fleet/resources/sqlite.py +++ b/fleet/resources/sqlite.py @@ -454,18 +454,18 @@ def _expect_no_changes(self): removed_tables = before_tables - after_tables for table in added_tables: - if not self.ignore_config.should_ignore_table(table): + if not self.ignore_config.should_ignore_table(table) and not self.ignore_config.is_incidental_table(table): raise AssertionError(f"Unexpected table added: {table}") for table in removed_tables: - if not self.ignore_config.should_ignore_table(table): + if not self.ignore_config.should_ignore_table(table) and not self.ignore_config.is_incidental_table(table): raise AssertionError(f"Unexpected table removed: {table}") # Prepare tables to check tables_to_check = [] all_tables = before_tables | after_tables for table in all_tables: - if not self.ignore_config.should_ignore_table(table): + if not self.ignore_config.should_ignore_table(table) and not self.ignore_config.is_incidental_table(table): tables_to_check.append(table) # If no tables to check, we're done @@ -752,6 +752,7 @@ def check_row( if ( table not in changes_by_table and not self.ignore_config.should_ignore_table(table) + and not self.ignore_config.is_incidental_table(table) ): tables_to_verify.append(table) @@ -827,8 +828,11 @@ def _is_change_allowed( # Collect all unexpected changes unexpected_changes = [] + mentioned_tables = {c.get("table") for c in allowed_changes} for tbl, report in diff.items(): + if tbl not in mentioned_tables and self.ignore_config.is_incidental_table(tbl): + continue for row in report.get("modified_rows", []): for f, vals in row["changes"].items(): if self.ignore_config.should_ignore_field(tbl, f): @@ -1228,6 +1232,7 @@ def check_row( if ( table not in changes_by_table and not self.ignore_config.should_ignore_table(table) + and not self.ignore_config.is_incidental_table(table) ): tables_to_verify.append(table) @@ -1517,8 +1522,11 @@ def _validate_modification_with_fields_spec( # Collect all unexpected changes for detailed reporting unexpected_changes = [] + mentioned_tables = {c.get("table") for c in allowed_changes} for tbl, report in diff.items(): + if tbl not in mentioned_tables and self.ignore_config.is_incidental_table(tbl): + continue for row in report.get("modified_rows", []): row_changes = row["changes"] diff --git a/fleet/verifiers/db.py b/fleet/verifiers/db.py index 85f7a43b..dcd00b22 100644 --- a/fleet/verifiers/db.py +++ b/fleet/verifiers/db.py @@ -790,6 +790,64 @@ def __repr__(self): ################################################################################ +# Tables that are incidental to task completion: apps write them during normal +# UI interaction (login, session, audit, search history, navigation, tracking) +# or the framework writes them (migrations, metadata), but they are not the +# business writes a verifier intends to assert. ``expect_only`` / ``expect_only_v2`` +# treat incidental writes to these tables as ignored *only when the table is not +# explicitly listed in ``allowed_changes``*, so a correct agent is not flagged for +# routine side-effect rows. A verifier that genuinely wants to assert changes to +# one of these tables still can by listing it in ``allowed_changes`` (per-row +# validation runs regardless of this set). Pass ``incidental_tables=set()`` to +# opt out and restore strict behavior for every unmentioned table. +DEFAULT_INCIDENTAL_TABLES: frozenset[str] = frozenset( + { + # framework / migration / metadata + "__drizzle_migrations", + "sqlite_sequence", + "sqlite_stat1", + "_generation_state", + "_db_metadata", + "_db_meta", + # auth / login / session + "login_activities", + "login_activity", + "login_audit", + "login_history", + "user_sessions", + "sessions", + "session", + "session_tokens", + "password_reset_token", + "email_verification_token", + "captcha_challenge", + # audit / tracking / activity + "audit_log", + "audit_logs", + "user_actions", + "user_activity", + "recent_activity", + "recent_activities", + "navigation_items", + "search_history", + "activities", + "export_activity", + "shop_partner_visit", + "signature", + "teams_presence", + # misc incidental side-effect tables + "attachments", + "todo_task_lists", + "todo_tasks", + "todo_task_steps", + "sections", + "section_chats", + "section_channels", + "email_folders", + } +) + + class IgnoreConfig: """Configuration for ignoring specific tables, fields, or combinations during diff operations.""" @@ -798,21 +856,34 @@ def __init__( tables: Optional[Set[str]] = None, fields: Optional[Set[str]] = None, table_fields: Optional[Dict[str, Set[str]]] = None, + incidental_tables: Optional[Set[str]] = None, ): """ Args: tables: Set of table names to completely ignore fields: Set of field names to ignore across all tables table_fields: Dict mapping table names to sets of field names to ignore in that table + incidental_tables: Set of table names whose incidental writes are tolerated + when the table is not listed in ``allowed_changes``. Defaults to + :data:`DEFAULT_INCIDENTAL_TABLES`. Pass an empty set to opt out. """ self.tables = tables or set() self.fields = fields or set() self.table_fields = table_fields or {} + self.incidental_tables = ( + incidental_tables + if incidental_tables is not None + else set(DEFAULT_INCIDENTAL_TABLES) + ) def should_ignore_table(self, table: str) -> bool: """Check if a table should be completely ignored.""" return table in self.tables + def is_incidental_table(self, table: str) -> bool: + """Check if a table is an incidental side-effect table (case-insensitive).""" + return (table or "").lower() in self.incidental_tables + def should_ignore_field(self, table: str, field: str) -> bool: """Check if a specific field in a table should be ignored.""" # Global field ignore @@ -1036,6 +1107,8 @@ def _expect_only_targeted(self, allowed_changes: List[Dict[str, Any]]): continue if self.ignore_config.should_ignore_table(table): continue + if self.ignore_config.is_incidental_table(table): + continue before_count = self._get_row_count(self.before.db_path, table) after_count = self._get_row_count(self.after.db_path, table) if before_count != after_count: @@ -1249,6 +1322,8 @@ def _validate_modify_row( continue if self.ignore_config.should_ignore_table(table): continue + if self.ignore_config.is_incidental_table(table): + continue before_count = self._get_row_count(self.before.db_path, table) after_count = self._get_row_count(self.after.db_path, table) if before_count != after_count: @@ -1275,6 +1350,8 @@ def expect_only(self, allowed_changes: List[Dict[str, Any]]): if not allowed_changes: diff = self._collect() for tbl, report in diff.items(): + if self.ignore_config.is_incidental_table(tbl): + continue total = ( len(report.get("added_rows", [])) + len(report.get("removed_rows", [])) @@ -1322,8 +1399,11 @@ def _is_change_allowed( # Collect all unexpected changes for detailed reporting unexpected_changes = [] + mentioned_tables = {c.get("table") for c in allowed_changes} for tbl, report in diff.items(): + if tbl not in mentioned_tables and self.ignore_config.is_incidental_table(tbl): + continue for row in report.get("modified_rows", []): for f, vals in row["changes"].items(): if self.ignore_config.should_ignore_field(tbl, f): @@ -1474,6 +1554,8 @@ def expect_only_v2(self, allowed_changes: List[Dict[str, Any]]): if not allowed_changes: diff = self._collect() for tbl, report in diff.items(): + if self.ignore_config.is_incidental_table(tbl): + continue total = ( len(report.get("added_rows", [])) + len(report.get("removed_rows", [])) @@ -1696,8 +1778,11 @@ def _validate_modification_with_fields_spec( # Collect all unexpected changes for detailed reporting unexpected_changes = [] + mentioned_tables = {c.get("table") for c in allowed_changes} for tbl, report in diff.items(): + if tbl not in mentioned_tables and self.ignore_config.is_incidental_table(tbl): + continue for row in report.get("modified_rows", []): row_changes = row["changes"] diff --git a/tests/test_expect_only.py b/tests/test_expect_only.py index 96362e25..6fa9e786 100644 --- a/tests/test_expect_only.py +++ b/tests/test_expect_only.py @@ -2591,3 +2591,261 @@ def test_targeted_row_exists_both_sides_no_change(): finally: os.unlink(before_db) os.unlink(after_db) + + +# ============================================================================ +# Tests for incidental side-effect table handling (ENVT-139728) +# Verifiers must not fail when apps write incidental session/tracking/login/ +# audit rows during normal interaction, as long as those tables are not listed +# in allowed_changes. A verifier that DOES list an incidental table in +# allowed_changes still gets full per-row validation. +# ============================================================================ + + +def _make_tmp_dbs(): + before_db = tempfile.NamedTemporaryFile(suffix=".db", delete=False).name + after_db = tempfile.NamedTemporaryFile(suffix=".db", delete=False).name + return before_db, after_db + + +def test_expect_only_v2_ignores_incidental_session_writes(): + """Incidental session/tracking writes not in allowed_changes must not fail the diff.""" + before_db, after_db = _make_tmp_dbs() + try: + conn = sqlite3.connect(before_db) + conn.execute("CREATE TABLE account (id INTEGER PRIMARY KEY, balance REAL)") + conn.execute("CREATE TABLE transaction_record (id INTEGER PRIMARY KEY, account_id INTEGER, amount REAL)") + conn.execute("CREATE TABLE login_activities (id INTEGER PRIMARY KEY, user_id INTEGER, action TEXT)") + conn.execute("CREATE TABLE search_history (id INTEGER PRIMARY KEY, query TEXT)") + conn.execute("INSERT INTO account VALUES (1, 100.0)") + conn.execute("INSERT INTO transaction_record VALUES (1, 1, 50.0)") + conn.execute("INSERT INTO login_activities VALUES (1, 1, 'login')") + conn.execute("INSERT INTO search_history VALUES (1, 'home')") + conn.commit() + conn.close() + + conn = sqlite3.connect(after_db) + conn.execute("CREATE TABLE account (id INTEGER PRIMARY KEY, balance REAL)") + conn.execute("CREATE TABLE transaction_record (id INTEGER PRIMARY KEY, account_id INTEGER, amount REAL)") + conn.execute("CREATE TABLE login_activities (id INTEGER PRIMARY KEY, user_id INTEGER, action TEXT)") + conn.execute("CREATE TABLE search_history (id INTEGER PRIMARY KEY, query TEXT)") + conn.execute("INSERT INTO account VALUES (1, 90.0)") + conn.execute("INSERT INTO transaction_record VALUES (1, 1, 50.0)") + conn.execute("INSERT INTO transaction_record VALUES (2, 1, -60.0)") + conn.execute("INSERT INTO login_activities VALUES (1, 1, 'login')") + conn.execute("INSERT INTO login_activities VALUES (2, 1, 'login')") + conn.execute("INSERT INTO search_history VALUES (1, 'home')") + conn.execute("INSERT INTO search_history VALUES (2, 'rewards')") + conn.commit() + conn.close() + + before = DatabaseSnapshot(before_db) + after = DatabaseSnapshot(after_db) + + # Legitimate business writes are enumerated; incidental login/session + # writes are NOT listed and must be tolerated. + before.diff(after).expect_only_v2( + [ + { + "table": "account", + "pk": 1, + "type": "modify", + "resulting_fields": [("balance", 90.0)], + "no_other_changes": True, + }, + { + "table": "transaction_record", + "pk": 2, + "type": "insert", + "fields": [("id", 2), ("account_id", 1), ("amount", -60.0)], + }, + ] + ) + + finally: + os.unlink(before_db) + os.unlink(after_db) + + +def test_expect_only_v2_empty_changes_ignores_incidental_writes(): + """Harbor-style empty allowed_changes tolerates incidental writes.""" + before_db, after_db = _make_tmp_dbs() + try: + conn = sqlite3.connect(before_db) + conn.execute("CREATE TABLE login_activities (id INTEGER PRIMARY KEY, user_id INTEGER, action TEXT)") + conn.execute("CREATE TABLE search_history (id INTEGER PRIMARY KEY, query TEXT)") + conn.execute("INSERT INTO login_activities VALUES (1, 1, 'login')") + conn.execute("INSERT INTO search_history VALUES (1, 'home')") + conn.commit() + conn.close() + + conn = sqlite3.connect(after_db) + conn.execute("CREATE TABLE login_activities (id INTEGER PRIMARY KEY, user_id INTEGER, action TEXT)") + conn.execute("CREATE TABLE search_history (id INTEGER PRIMARY KEY, query TEXT)") + conn.execute("INSERT INTO login_activities VALUES (1, 1, 'login')") + conn.execute("INSERT INTO login_activities VALUES (2, 1, 'login')") + conn.execute("INSERT INTO search_history VALUES (1, 'home')") + conn.execute("INSERT INTO search_history VALUES (2, 'rewards')") + conn.commit() + conn.close() + + before = DatabaseSnapshot(before_db) + after = DatabaseSnapshot(after_db) + + # No business changes expected; only incidental writes happened. + before.diff(after).expect_only_v2([]) + + finally: + os.unlink(before_db) + os.unlink(after_db) + + +def test_expect_only_v2_empty_changes_still_flags_business_writes(): + """Empty allowed_changes must still flag writes to non-incidental tables.""" + before_db, after_db = _make_tmp_dbs() + try: + conn = sqlite3.connect(before_db) + conn.execute("CREATE TABLE account (id INTEGER PRIMARY KEY, balance REAL)") + conn.execute("INSERT INTO account VALUES (1, 100.0)") + conn.commit() + conn.close() + + conn = sqlite3.connect(after_db) + conn.execute("CREATE TABLE account (id INTEGER PRIMARY KEY, balance REAL)") + conn.execute("INSERT INTO account VALUES (1, 100.0)") + conn.execute("INSERT INTO account VALUES (2, 50.0)") + conn.commit() + conn.close() + + before = DatabaseSnapshot(before_db) + after = DatabaseSnapshot(after_db) + + with pytest.raises(AssertionError): + before.diff(after).expect_only_v2([]) + + finally: + os.unlink(before_db) + os.unlink(after_db) + + +def test_expect_only_v2_incidental_opt_out_is_strict(): + """IgnoreConfig(incidental_tables=set()) restores strict behavior for unmentioned tables.""" + before_db, after_db = _make_tmp_dbs() + try: + conn = sqlite3.connect(before_db) + conn.execute("CREATE TABLE account (id INTEGER PRIMARY KEY, balance REAL)") + conn.execute("CREATE TABLE login_activities (id INTEGER PRIMARY KEY, user_id INTEGER, action TEXT)") + conn.execute("INSERT INTO account VALUES (1, 100.0)") + conn.execute("INSERT INTO login_activities VALUES (1, 1, 'login')") + conn.commit() + conn.close() + + conn = sqlite3.connect(after_db) + conn.execute("CREATE TABLE account (id INTEGER PRIMARY KEY, balance REAL)") + conn.execute("CREATE TABLE login_activities (id INTEGER PRIMARY KEY, user_id INTEGER, action TEXT)") + conn.execute("INSERT INTO account VALUES (1, 90.0)") + conn.execute("INSERT INTO login_activities VALUES (1, 1, 'login')") + conn.execute("INSERT INTO login_activities VALUES (2, 1, 'login')") + conn.commit() + conn.close() + + before = DatabaseSnapshot(before_db) + after = DatabaseSnapshot(after_db) + + with pytest.raises(AssertionError): + before.diff(after, IgnoreConfig(incidental_tables=set())).expect_only_v2( + [ + { + "table": "account", + "pk": 1, + "type": "modify", + "resulting_fields": [("balance", 90.0)], + "no_other_changes": True, + }, + ] + ) + + finally: + os.unlink(before_db) + os.unlink(after_db) + + +def test_expect_only_v2_incidental_table_mentioned_still_validated(): + """Listing an incidental table in allowed_changes still runs per-row validation.""" + before_db, after_db = _make_tmp_dbs() + try: + conn = sqlite3.connect(before_db) + conn.execute("CREATE TABLE login_activities (id INTEGER PRIMARY KEY, user_id INTEGER, action TEXT)") + conn.execute("INSERT INTO login_activities VALUES (1, 1, 'login')") + conn.commit() + conn.close() + + conn = sqlite3.connect(after_db) + conn.execute("CREATE TABLE login_activities (id INTEGER PRIMARY KEY, user_id INTEGER, action TEXT)") + conn.execute("INSERT INTO login_activities VALUES (1, 1, 'login')") + conn.execute("INSERT INTO login_activities VALUES (2, 1, 'login')") + conn.commit() + conn.close() + + before = DatabaseSnapshot(before_db) + after = DatabaseSnapshot(after_db) + + # The verifier explicitly asserts on login_activities, so per-row + # validation must run and catch the wrong field value. + with pytest.raises(AssertionError): + before.diff(after).expect_only_v2( + [ + { + "table": "login_activities", + "pk": 2, + "type": "insert", + "fields": [("id", 2), ("user_id", 1), ("action", "WRONG_VALUE")], + }, + ] + ) + + finally: + os.unlink(before_db) + os.unlink(after_db) + + +def test_expect_only_v2_still_flags_unexpected_business_writes(): + """The incidental relaxation must not mask unexpected writes to business tables.""" + before_db, after_db = _make_tmp_dbs() + try: + conn = sqlite3.connect(before_db) + conn.execute("CREATE TABLE account (id INTEGER PRIMARY KEY, balance REAL)") + conn.execute("CREATE TABLE orders (id INTEGER PRIMARY KEY, total REAL)") + conn.execute("INSERT INTO account VALUES (1, 100.0)") + conn.execute("INSERT INTO orders VALUES (1, 10.0)") + conn.commit() + conn.close() + + conn = sqlite3.connect(after_db) + conn.execute("CREATE TABLE account (id INTEGER PRIMARY KEY, balance REAL)") + conn.execute("CREATE TABLE orders (id INTEGER PRIMARY KEY, total REAL)") + conn.execute("INSERT INTO account VALUES (1, 90.0)") + conn.execute("INSERT INTO orders VALUES (1, 10.0)") + conn.execute("INSERT INTO orders VALUES (2, 99.0)") + conn.commit() + conn.close() + + before = DatabaseSnapshot(before_db) + after = DatabaseSnapshot(after_db) + + with pytest.raises(AssertionError): + before.diff(after).expect_only_v2( + [ + { + "table": "account", + "pk": 1, + "type": "modify", + "resulting_fields": [("balance", 90.0)], + "no_other_changes": True, + }, + ] + ) + + finally: + os.unlink(before_db) + os.unlink(after_db)