Skip to content

fix(multiuser): redact other users' current-item identifiers from queue status - #9087

Merged
lstein merged 3 commits into
invoke-ai:mainfrom
lstein:fix/queue-status-cross-user-leak
Apr 30, 2026
Merged

fix(multiuser): redact other users' current-item identifiers from queue status#9087
lstein merged 3 commits into
invoke-ai:mainfrom
lstein:fix/queue-status-cross-user-leak

Conversation

@lstein

@lstein lstein commented Apr 25, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR closes a minor security hole in the multiuser implementation which allowed item_id, session_id and batch_id identifiers from one user's running generation session to leak to another user.

QueueItemStatusChangedEvent embeds a SessionQueueStatus that includes the
currently-running item's item_id, session_id, and batch_id. The full event
ships to user:{owner} and admin rooms. When user A's item changed status while
user B's item was the one in progress, owner A's frontend received the event with
B's identifiers exposed in the embedded queue_status.

Fix

In _set_queue_item_status, after building queue_status, scrub item_id /
session_id / batch_id when the in-progress item belongs to a different user
than the changed item. Aggregate counts stay global (not user-sensitive). The
frontend never reads those fields off the event payload (only batch_status.batch_id,
which is the changed item's own batch — no leak), so no UI behavior changes.

Test plan

  • New regression test test_event_redacts_other_users_current_item_identifiers
    verifies that when user B's item is in_progress and user A's item is canceled,
    A's emitted event has queue_status.item_id/session_id/batch_id == None.
  • test_event_preserves_owner_current_item_identifiers confirms no over-redaction
    when there's no in-progress item.
  • test_event_preserves_identifiers_when_current_item_is_the_changed_item confirms
    identifiers ARE exposed when the in-progress item is the changed item itself.
  • Verified the regression test fails without the fix and passes with it.
  • All 16 session-queue + sanitization tests pass.

…ue status events

QueueItemStatusChangedEvent embeds the SessionQueueStatus, which includes the
currently-running item's item_id, session_id, and batch_id. The event ships to
user:{owner} and admin rooms. When user A's item changed status while user B's
item was the one in progress, owner A's frontend received the event with B's
identifiers exposed.

In _set_queue_item_status, scrub item_id/session_id/batch_id from the embedded
queue_status when the in-progress item belongs to a different user than the
changed item. Aggregate counts remain global (not user-sensitive).

Identified out-of-scope in the security audit of #127.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added python PRs that change python files services PRs that change app services python-tests PRs that change python tests labels Apr 25, 2026
@lstein lstein added the 6.14.0 label Apr 25, 2026
@lstein lstein moved this to 6.14.x Theme: LIBRARY UPDATES in Invoke - Community Roadmap Apr 25, 2026

@JPPhoto JPPhoto left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • invokeai/app/services/session_queue/session_queue_sqlite.py:319:
    The redaction decision is based on a second get_current() read after get_queue_status() has already embedded the current item's identifiers. These reads are not atomic: the DB lock is only held inside each transaction, and _set_queue_item_status() releases it between get_queue_status() and the new get_current() call. If user B is current when line 319 builds queue_status, then B completes or is canceled before line 326, current_item becomes None and the stale B item_id, session_id, and batch_id remain in the event sent to user A. This leaves a race-window version of the leak the PR is trying to close. To expose this issue, add a test that makes the first current-item read used by get_queue_status() return another user's in-progress item, then makes the second redaction read return None, and asserts the emitted event is still scrubbed.

@lstein lstein moved this from 6.14.x Theme: LIBRARY UPDATES to 6.13.x Theme: MODELS in Invoke - Community Roadmap Apr 30, 2026
@lstein lstein added v6.13.x and removed 6.14.0 labels Apr 30, 2026
@lstein

lstein commented Apr 30, 2026

Copy link
Copy Markdown
Collaborator Author

@JPPhoto

  • invokeai/app/services/session_queue/session_queue_sqlite.py:319:
    The redaction decision is based on a second get_current() read after get_queue_status() has already embedded the current item's identifiers. These reads are not atomic: the DB lock is only held inside each transaction, and _set_queue_item_status() releases it between get_queue_status() and the new get_current() call. If user B is current when line 319 builds queue_status, then B completes or is canceled before line 326, current_item becomes None and the stale B item_id, session_id, and batch_id remain in the event sent to user A. This leaves a race-window version of the leak the PR is trying to close. To expose this issue, add a test that makes the first current-item read used by get_queue_status() return another user's in-progress item, then makes the second redaction read return None, and asserts the emitted event is still scrubbed.

I've added a acting_user_id field to get_queue_status() which contains the ID of the original user, preventing the second call to get_current() and the race condition. I added your suggested test for this behavior and confirmed that the test failed before the fix, and succeeded afterward.

@JPPhoto
JPPhoto self-requested a review April 30, 2026 21:00

@JPPhoto JPPhoto left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@lstein
lstein merged commit 47d0952 into invoke-ai:main Apr 30, 2026
16 checks passed
lstein added a commit that referenced this pull request Jun 23, 2026
…r non-admins (#9262)

* fix(multiuser): restore global queue counts + redacted entries for non-admins

PR #9018 changed the queue status/list endpoints from "global counts +
per-user counts" to "user-scoped only" for non-admins. This silently broke
two multiuser behaviors:

1. The hamburger-menu badge lost its "X/Y" form. #9018 dropped the
   user_pending/user_in_progress fields, made pending/in_progress/total
   user-scoped (so a non-admin had no global total), and reduced the badge
   to a single number.

2. The virtualized queue list stopped showing other users' redacted
   entries. #9018 added `AND user_id = ?` to get_queue_item_ids, so a
   non-admin only ever received their own item ids — the (intact)
   sanitize_queue_item_for_user redaction never ran for other users' items.

Restore the original design (PR #8822) while keeping the #9087 current-item
identifier redaction:

- SessionQueueStatus regains optional user_pending/user_in_progress.
- get_queue_status always computes GLOBAL aggregate counts and, when
  user_id is provided, additionally returns that user's own counts. The
  current item's identifiers are still gated to the owner/admin via a
  single get_current() snapshot (race-free, #9087 preserved).
- get_queue_item_ids no longer filters by user; ids carry no sensitive
  data and items are redacted at hydration by sanitize_queue_item_for_user,
  so non-admins again see partially-redacted entries for other users' jobs.
- QueueCountBadge renders "<own>/<global total>" in multiuser mode and the
  plain total for admins / single-user mode.
- Regenerate the frontend OpenAPI schema.

Aggregate counts are global but non-identifying (no session_id/batch_id/
params/graph), consistent with #9087's "counts remain global" decision.

Tests:
- Update the model-shape test to assert user_pending/user_in_progress exist.
- Add session-queue integration tests: global counts + per-user subcounts,
  admin/global omission of subcounts, current-item redaction with global
  counts intact, and get_queue_item_ids returning every user's ids.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* chore(frontend): typegen + openapi

* chore(backend): ruff

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Alexander Eichhorn <alex@eichhorn.dev>
dunkeroni pushed a commit to dunkeroni/InvokeAI that referenced this pull request Jun 29, 2026
…r non-admins (invoke-ai#9262)

* fix(multiuser): restore global queue counts + redacted entries for non-admins

PR invoke-ai#9018 changed the queue status/list endpoints from "global counts +
per-user counts" to "user-scoped only" for non-admins. This silently broke
two multiuser behaviors:

1. The hamburger-menu badge lost its "X/Y" form. invoke-ai#9018 dropped the
   user_pending/user_in_progress fields, made pending/in_progress/total
   user-scoped (so a non-admin had no global total), and reduced the badge
   to a single number.

2. The virtualized queue list stopped showing other users' redacted
   entries. invoke-ai#9018 added `AND user_id = ?` to get_queue_item_ids, so a
   non-admin only ever received their own item ids — the (intact)
   sanitize_queue_item_for_user redaction never ran for other users' items.

Restore the original design (PR invoke-ai#8822) while keeping the invoke-ai#9087 current-item
identifier redaction:

- SessionQueueStatus regains optional user_pending/user_in_progress.
- get_queue_status always computes GLOBAL aggregate counts and, when
  user_id is provided, additionally returns that user's own counts. The
  current item's identifiers are still gated to the owner/admin via a
  single get_current() snapshot (race-free, invoke-ai#9087 preserved).
- get_queue_item_ids no longer filters by user; ids carry no sensitive
  data and items are redacted at hydration by sanitize_queue_item_for_user,
  so non-admins again see partially-redacted entries for other users' jobs.
- QueueCountBadge renders "<own>/<global total>" in multiuser mode and the
  plain total for admins / single-user mode.
- Regenerate the frontend OpenAPI schema.

Aggregate counts are global but non-identifying (no session_id/batch_id/
params/graph), consistent with invoke-ai#9087's "counts remain global" decision.

Tests:
- Update the model-shape test to assert user_pending/user_in_progress exist.
- Add session-queue integration tests: global counts + per-user subcounts,
  admin/global omission of subcounts, current-item redaction with global
  counts intact, and get_queue_item_ids returning every user's ids.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* chore(frontend): typegen + openapi

* chore(backend): ruff

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Alexander Eichhorn <alex@eichhorn.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python PRs that change python files python-tests PRs that change python tests services PRs that change app services v6.13.x

Projects

Status: 6.13.x Theme: MODELS

Development

Successfully merging this pull request may close these issues.

2 participants