[merttoprak1]

What changed and why

test_auth_api.py and test_auth_security.py called create_all and drop_all on app.db.session.engine. That engine uses DATABASE_URL. When DATABASE_URL points at a local database, every auth test drops its tables in teardown. This happened during #132.

• Both modules now use the client and engine fixtures from tests/conftest.py. These fixtures use an in-memory SQLite database, like the other integration tests. The three seeding tests pass that engine to seed_test_users.
• tests/conftest.py gets an autouse fixture, _application_database_is_untouched. It listens on the application engine with before_cursor_execute and raises before a write statement executes: INSERT, UPDATE, DELETE, CREATE, DROP, ALTER, REPLACE or TRUNCATE, with or without a leading WITH clause. The statement never reaches the database. This is the regression test. It also covers a future test that writes through SessionLocal or get_db_context, which the get_db override does not reach.

No new dependency. The guard uses the SQLAlchemy event API.

Closes #299

How it was verified

I set up a real local database with python -m app.db.init_db (24 tables, two dev accounts) and set DATABASE_URL to it.

Run	Result	Tables left
Auth tests on master	54 passed	1 of 24 (only alembic_version)
Auth tests on master + guard	26 failed, 54 teardown errors: Test tried to write to the application database: INSERT INTO app_user ...	24 of 24
Auth tests on this branch	54 passed	24 of 24
Full suite on this branch, with coverage	1087 passed, coverage 85.27% (ratchet 76%)	24 of 24

uv run --no-sync mypy app: no issues.

Watch for

• The guard runs for every test, unit and integration. It blocks writes only. Reads are allowed.
• The guard checks SQL text, so it is not a hard boundary. It does not catch a write that starts with a comment, SELECT ... INTO, MERGE, COPY, VACUUM, PRAGMA writes, setval or EXPLAIN ANALYZE. SQLAlchemy does not generate these. If application code catches the error, the write is still blocked, but the test can stay green.
• After this change, one test still reads the application database. /health calls get_db() directly instead of through Depends, so the override does not apply. As a result, test_health_reports_database_connected_when_it_is checks the real database with SELECT 1. It is read-only, so I left it out of this PR.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

[lucaosti]

Thanks, this fixes #299: I ran the auth tests against a real SQLite file as DATABASE_URL and all 23 tables survive (54 passed). Reusing the client/engine fixtures and documenting the before/after is exactly right.

One change before merging:

1. The guard detects the write after it has happened. _record only collects statements and the assertion runs at teardown, so the statement still reaches the developer's database. I tried a probe test that runs CREATE TABLE probe through SessionLocal: it failed at teardown, but the table was created in the real file. Raising inside the before_cursor_execute listener blocks the statement and points at the line that issued it, which is what #299 needs.

Nit:

2. The prefix check misses WITH ... INSERT (my probe inserted a row and passed), REPLACE and TRUNCATE. Matching the verbs after an optional leading WITH would cover them.

[merttoprak1]

Thanks, both points were right. Fixed in the second commit.

Blocking. The listener now raises AssertionError inside before_cursor_execute. SQLAlchemy calls it before cursor.execute, outside the try around it, so the statement never reaches the database. To check, I ran the master version of the two auth modules with the new guard against a database from python -m app.db.init_db: 26 failed and 54 teardown errors, and all 24 tables and both dev accounts were still there. With the first version of the guard, only alembic_version was left.

Nit. The match now accepts an optional leading WITH (and WITH RECURSIVE) clause, and adds REPLACE and TRUNCATE. WITH ... SELECT still passes.

Limits. This is a check on SQL text, so it is not a hard boundary:

• It does not catch writes that start with a comment, SELECT ... INTO, MERGE, COPY, VACUUM, PRAGMA writes, setval or EXPLAIN ANALYZE. SQLAlchemy does not generate these. A test needs hand-written SQL to get them.
• If application code catches the error, the write is still blocked, but the test can stay green. On master, the API error handler turned it into a 500, and the tests failed on the status code.

The full fix for these limits is to set DATABASE_URL to a throwaway database in tests/conftest.py before the app is imported, as it already does for QVAC_SERVICE_URL. The cost is that CI would stop building the engine from the Postgres URL, which is how #307 found the postgresql:// driver error. I kept that out of this PR. It is a separate decision.

Full suite on this branch: 1087 passed, coverage 85.27%, mypy app clean.

[lucaosti]

Thanks Mert, this does exactly what #299 needed. I reran the probe against a real SQLite file: CREATE TABLE through SessionLocal now fails at the statement and the table is not created. WITH ... INSERT, REPLACE, TRUNCATE and INSERT OR REPLACE are blocked too, while reads and the in-memory fixtures work as before, and the full suite (1087 tests) leaves the file byte-identical.

One optional nit: a statement starting with a SQL comment (/* x */ INSERT or -- x\nINSERT) still gets through. SQLAlchemy never generates that, so it only matters for hand-written text() SQL; stripping leading comments before the match would close it. Approving.