Session image cleanup deletes gallery rows without checking who owns them

[ashvinctrl]

Keeping this light on the exploitation side per SECURITY.md. Private reporting isn't enabled on the repo so I can't file an advisory, and the policy says a minimal issue is the fallback.

cleanup_session_images in src/session_image_cleanup.py picks what to delete like this:

clauses = [GalleryImage.session_id == session_id]
if image_ids:
    clauses.append(GalleryImage.id.in_(list(image_ids)))
if filenames:
    clauses.append(GalleryImage.filename.in_(list(filenames)))
query = db.query(GalleryImage).filter(or_(*clauses))

First clause is fine. The other two aren't scoped to anything at all, no owner and no session, and the ids and filenames they match on come from session_image_refs, which reads them out of ChatMessage.meta_data under tool_events.

That metadata isn't computed server-side. POST /api/session/{sid}/inject_messages writes whatever metadata object the client sends straight through, and reserve_message_upload_references only looks at upload references, so anything else in the dict is stored as-is.

Matched rows get is_active = False and the file unlinked from GENERATED_IMAGES_DIR, and this runs on a normal session delete. So the set of images a chat can delete ends up wider than the set it owns, in two separate directions:

• across users, since nothing compares owners at all
• across chats, since a row still attached to a different session_id can be matched on the reference alone

The second one doesn't need anyone to be doing anything clever. Reference an image in a second chat, delete that chat, and the row that still belongs to the first chat is gone, file and all. The function's own docstring says "owned by a chat", and a row whose session_id points at a different chat isn't owned by this one.

The thing that made me fairly confident this is an oversight rather than intended is serve_generated_image in app.py, which already guards the read path and says why:

# SECURITY: filename is the only key, so anyone who knows / guesses a
# 12-hex content hash could pull another user's image bytes. Require
# auth and verify ownership via the gallery row (when one exists).

Same identifier. Checked before you can read the image, not checked before you can delete it.

I don't think the reference scan should just be removed, it's clearly there for a reason. tests/test_session_image_cleanup.py covers a row with no session_id that nothing but the metadata can find, and that's a real case. It just needs to be constrained to the session's owner, and to rows that aren't already attached to some other chat.

delete_all_sessions in routes/session_routes.py builds the same unscoped query, but it's require_admin and deletes every session anyway, so I'd treat that as separate and lower priority.

Have a fix with tests, can open a PR.