removeRun can delete a locked run: lock check and deletion use two different queries

[evanmarshall]

Describe the bug

A run that is protected by an active run lock can still be permanently deleted. The lock check in removeRun() and the actual deletion resolve the runs to act on with two separate queries, so a run that starts matching the filter between the two queries is deleted without its lock ever being checked.

In removeRun() (web/server/codechecker_server/api/report_server.py), the preflight resolves the matching run IDs and checks their locks:

codechecker/web/server/codechecker_server/api/report_server.py

Lines 4195 to 4197 in ac2c3ac

	matched_run_ids = get_run_ids_for_filter(session, run_filter)
	if matched_run_ids:
	check_remove_runs_lock(session, matched_run_ids)

            matched_run_ids = get_run_ids_for_filter(session, run_filter)
            if matched_run_ids:
                check_remove_runs_lock(session, matched_run_ids)

But the destructive query is then built from the RunFilter again, not from matched_run_ids, and the loop deletes and commits each returned row with no further lock check:

codechecker/web/server/codechecker_server/api/report_server.py

Lines 4199 to 4212 in ac2c3ac

	q = process_run_filter(session, session.query(Run), run_filter)
	# q.delete(synchronize_session=False) could also be used here,
	# however, a run deletion tends to be a slow operation due to
	# cascades and such. Deleting runs in separate transactions don't
	# exceed a potential statement timeout threshold in a DBMS.
	runs = []
	deleted_run_cnt = 0
	for run in q.all():
	try:
	runs.append(run.name)
	session.delete(run)
	session.commit()

            q = process_run_filter(session, session.query(Run), run_filter)
            ...
            for run in q.all():
                try:
                    runs.append(run.name)
                    session.delete(run)
                    session.commit()

Because each delete is committed in its own transaction, the set of rows returned by q.all() can differ from the set that was lock-checked a moment earlier. Any run that newly matches the filter in that window is deleted even if it holds an active RunLock.

CodeChecker version

master at ac2c3ac (code inspection; the window is timing-dependent so it is easiest to see in the source).

To Reproduce

Minimal failure scenario with a name-based filter:

1. A run named myrun exists and is not locked.
2. Client A calls removeRun(None, RunFilter(names=['myrun'])). The preflight resolves myrun's ID and check_remove_runs_lock passes.
3. Before q.all() executes, client B deletes the original myrun and starts a fresh store under the same name, which creates a new Run row holding an active RunLock.
4. Client A's second query matches the new row by name and deletes it, along with the in-progress store's data, even though it was locked.

Expected behaviour

Only the runs that passed the lock check should be deletable. The simplest fix is to build the destructive query from the already-resolved matched_run_ids (for example session.query(Run).filter(Run.id.in_(matched_run_ids))) instead of re-running process_run_filter. Alternatively, re-check check_remove_runs_lock on the exact rows returned by q.all() right before deleting them.

Desktop (please complete the following information)

• OS: Linux (server-side issue, not client or browser specific)

Additional context

This is a follow-up to the fix merged in #4999, which correctly moved the lock check from [run_id] to the filter-resolved IDs. The remaining gap is that the check and the deletion still resolve the filter independently, so the guarantee only holds for rows that matched at preflight time.

Found while running Ito (AI code review, free for open source) against recently merged PRs. Full analysis: https://app.ito.ai/share/d4939f58-5894-45e5-b1dc-9b62c14b9e4e.