Skip to content

Fix shared conversation AI access and add participant file approvals - #1303

Merged
Paul Lizer (paullizer) merged 6 commits into
Developmentfrom
paullizer-shared-conversation-file-approvals
Aug 19, 2026
Merged

Fix shared conversation AI access and add participant file approvals#1303
Paul Lizer (paullizer) merged 6 commits into
Developmentfrom
paullizer-shared-conversation-file-approvals

Conversation

@paullizer

@paullizer Paul Lizer (paullizer) commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Problem

In a shared (collaborative) conversation, an invited participant asking the assistant to generate a CSV got:

Stream interrupted before any content was received.
Stream interrupted: Forbidden

The failure is broader than file generation. A collaborative conversation is backed by a hidden source conversation (conversation_kind: 'collaboration_source') whose user_id is always the shared conversation creator. /api/collaboration/conversations/<id>/stream proxies into chat_stream_api using that source conversation but keeps the requesting participant's session, so every owner-equality check failed for participants.

Four gates were involved:

# Location Effect
1 chat_stream_api_authorize_personal_conversation_access 403 Forbidden. Blocked all AI invocation by participants, not just files.
2 _upload_generated_chat_artifact_for_current_user PermissionError, swallowed by maybe_create_generated_file_output, so the file silently vanished.
3 _resolve_group_upload_target_for_current_user Plain group User could not save generated documents to a group workspace.
4 _get_authorized_chat_artifact_message Participants could not download an artifact even once it existed.

Gate 1 only surfaced on explicit AI requests because shared conversations default to ai_invocation_mode: 'explicit_only' — ordinary participant messages never reach the bridge, which is why it looked file-specific.

Changes

Authorization fix. All owner-only checks now resolve through one collaboration-aware helper, build_conversation_participation_context, mirroring the pattern already used by chat file uploads in _resolve_chat_upload_context. Personal conversations with no collaboration link remain strictly owner-only.

Approval gate. Downloadable files generated by a non-owner participant are written immediately in a pending_approval state rather than failing, so approval is a cheap state flip and the model never re-runs.

  • Personal shared conversation → the conversation owner approves
  • Group shared conversation → any group Owner/Admin/DocumentManager approves
  • A requester is never their own approver
  • Staged files are not downloadable by anyone, including the requester
  • Inline approve/deny card on the artifact, plus a notification for approvers
  • Denial deletes the blob and records who declined; unapproved files auto-deny after 3 days on the existing approval expiry sweep
  • Only downloadable deliverables are gated (CSV, XLSX, XLS, XLSM, DOCX, PDF, JSON, XML). Generated images and charts stay ungated so ordinary chat is unaffected.
  • New admin setting require_shared_conversation_file_approval, default on

Additional bugs fixed

Both affect the reported case directly, since 900 rows exceeds the 500-row inline export threshold and takes the background export path:

  • assert_generated_chat_artifact_is_published_for_user read the export run using the caller's id as the partition key. A large CSV queued by a participant would have been unreadable by the owner even after approval. The run owner is now recorded on the artifact, with a fallback to the caller for pre-existing artifacts.
  • commit_generated_chat_artifact_publication_for_user and delete_generated_chat_artifact_for_user carried the same owner-only check and would have broken publication and rollback for participant-queued exports.

Three more were caught in review and fixed before this PR:

  • Requester self-approval. Group-scope approvers were resolved purely by group role, so a group Admin/DocumentManager who was only a participant could stage a file and approve it themselves. The requester check now runs before the scope branch.
  • Bypass via /api/enhanced_citations/tabular. That route streams any blob-backed file message after a single conversation-ownership check and never consulted the gate. A plain group User who created a group shared conversation — explicitly not an approver — could fetch a staged CSV/XLSX directly. The gate now runs there before the blob is read, returning 403 rather than 500.
  • Truncation before authorization. list_pending_generated_file_approvals_for_user applied TOP @limit across the whole messages container and filtered by approver afterwards in Python, so a tenant with >50 pending files could return an empty list to an approver with items waiting. Candidates are now narrowed to the caller's own approval scopes inside the query.

Deliberate scope decision

Gate 3 (saving a generated document into a group workspace) is not staged. That path feeds the group search index, so holding it back means withholding indexing — a materially larger change. It now returns an actionable message naming the roles that can complete the request. The group scenario from the report — a member asking for a file in a group conversation — is fully covered by the approval flow, since those artifacts carry group scope and route to group document roles. Noted as follow-up in the feature doc.

Validation

Suite Result
test_shared_conversation_file_approval_fix.py (new) 16/16
functional_tests/route_tests/ (3 suites) 12/12
test_generated_artifact_lifecycle_authorization.py 6/6
test_assistant_table_csv_artifact.py 35/35
test_generated_json_xml_exports.py 7/7
test_tabular_row_orchestration_scale.py pass

Development was merged into this branch to resolve conflicts in config.py and release_notes.md. The two suites that arrived with it — test_collaboration_mention_tab_autocomplete.py (5/5) and test_chat_new_conversation_documents_drawer_reset.py (7/7) — pass against the merged code.

Two existing tests that extract these functions via AST needed the new dependency stubbed and were updated: test_generated_artifact_lifecycle_authorization.py and test_tabular_row_orchestration_scale.py.

Confirmed against a clean baseline as pre-existing and unrelated to this change: test_mixed_source_hardening.py, test_tabular_generated_output_exports.py, and test_enhanced_citations_blob_and_collaboration_fix.py. The latter two fail on stale exact-version assertions pinned to 0.241.144, which conflict with the repo rule against exact version equality in tests — worth a separate cleanup, left untouched here.

Behavior change summary

Scenario Before After
Participant asks the AI anything in a shared conversation Stream interrupted: Forbidden, no content Normal response
Participant asks for a CSV Forbidden, or artifact silently dropped File created, held for approval, approvers notified
Approver approves Not possible File becomes downloadable
Approver denies Not possible Blob deleted, decision shown in the card
Nobody responds Not possible Auto-denied after 3 days, blob deleted
Owner generates their own file Worked Unchanged

Notes

  • Version bumped to 0.260.006 (rebased over 0.260.005 from Development, which had already claimed 0.260.004). Nothing under deployers/ changed, so deployers/version.txt is untouched.
  • New browser JS is a local static asset under static/js/chat/, uses textContent only, Bootstrap d-none and alerts, no CDN or dynamic imports.
  • All new routes carry @swagger_route(security=get_auth_security()), @login_required, @user_required.
  • Docs: docs/explanation/features/SHARED_CONVERSATION_FILE_APPROVALS.md, docs/explanation/fixes/SHARED_CONVERSATION_FILE_GENERATION_FORBIDDEN_FIX.md, release notes, and both indexes.

Review focus

The diff touches shared conversation authorization used well beyond collaborative conversations. The highest-value review target is build_conversation_participation_context and its call sites. The invariant to check is that non-shared personal conversations remain strictly owner-only — asserted in the new tests, but worth a human read.

Paul Lizer (paullizer) and others added 2 commits August 19, 2026 12:50
A collaborative conversation is backed by a hidden source conversation whose
user_id is always the shared conversation creator. The collaboration stream
bridge proxies into chat_stream_api using that source conversation while keeping
the requesting participant's session, so every owner-equality check failed for
participants. The visible symptom was "Stream interrupted: Forbidden" with no
content whenever an invited user invoked the AI.

Four gates were involved:

- chat_stream_api returned 403 Forbidden, blocking all AI invocation by
  participants. This only surfaced on explicit AI requests because shared
  conversations default to ai_invocation_mode 'explicit_only'.
- The generated artifact write raised PermissionError, which
  maybe_create_generated_file_output swallowed, so the file silently vanished.
- Group workspace document saves required Owner/Admin/DocumentManager.
- Artifact downloads were owner-only, so participants could not retrieve files
  even once they existed.

Authorization is now resolved through a single collaboration-aware helper,
build_conversation_participation_context, mirroring the pattern already used by
chat file uploads in _resolve_chat_upload_context. Personal conversations with
no collaboration link remain strictly owner-only.

On top of that, downloadable files generated by a non-owner participant are now
staged for approval instead of failing. The artifact is written immediately in a
pending_approval state and released by an approver, so the model never re-runs.
The conversation owner approves in personal shared conversations; any group
Owner, Admin, or DocumentManager approves in group ones. A requester is never
their own approver. Denial deletes the stored blob and records the decision, and
anything left unapproved auto-denies after three days on the existing approval
expiry sweep. Only downloadable deliverables are gated; generated images and
charts stay ungated. Behavior is controlled by a new admin setting that defaults
to enabled.

Also fixed while in this path, both of which affect the reported case because
900 rows exceeds the 500-row inline export threshold:

- assert_generated_chat_artifact_is_published_for_user read the export run using
  the caller's id as the partition key, so an approved large CSV queued by a
  participant would have been unreadable by the owner. The run owner is now
  recorded on the artifact.
- commit_generated_chat_artifact_publication_for_user and
  delete_generated_chat_artifact_for_user carried the same owner-only check and
  would have broken publication and rollback for participant-queued exports.

Group workspace writes are deliberately not staged. They feed the group search
index, so withholding them would mean withholding indexing. That path now
returns an actionable message naming the roles that can complete the request.

Version bumped to 0.260.004.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ed-conversation-file-approvals

# Conflicts:
#	application/single_app/config.py
#	docs/explanation/release_notes.md
Comment thread application/single_app/route_backend_collaboration.py Outdated
Comment thread application/single_app/route_backend_collaboration.py Outdated
Comment thread application/single_app/route_backend_collaboration.py Dismissed
Comment thread application/single_app/route_enhanced_citations.py Outdated
Comment thread application/single_app/route_enhanced_citations.py Outdated
@paullizer

Copy link
Copy Markdown
Contributor Author

CI note: Release Notes Check

This check is red, but the failure is not caused by this PR. It fails on every PR branch right now — including paullizer-fix-1299-mention-tab-autocomplete, which merged as #1301, and fix/tabular-passthrough-schema-and-assistant-text.

The release-notes validation itself passes. The job dies on a later step, Post PR comment (when latest features likely needed but missing), with:

POST /repos/microsoft/simplechat/issues/1303/comments
403 Resource not accessible by integration

That is a workflow token permission problem (issues: write / pull-requests: write not granted to the job), not a content problem. Worth fixing separately.

On the Latest Features card

The comment that step was trying to post is the non-blocking reminder that support_menu_config.py was not updated. I did try to add a card, then backed it out deliberately.

The current-release tier has a strict contract enforced by test_support_menu_user_feature.py and test_admin_latest_features_tab.py: exactly 20 cards, and every card must carry exactly 3 screenshots. Using include_media=False breaks the 3-image assertion, and adding a 21st card breaks the count. I have no way to produce genuine screenshots of this UI, and fabricating placeholder PNGs to satisfy the assertions would put misleading assets in the catalog.

So this is left for whoever picks it up with real captures. It is a good candidate for a card — the approval flow is visible to both end users and admins. The three natural shots are: a participant requesting a file in a shared conversation, the approver's Approve/Deny card, and the approved file downloadable.

All other checks pass, including broken-access-control-check, malicious-pr-security-review, swagger-route-check, xss-sink-check, and CodeQL.

Paul Lizer (paullizer) and others added 4 commits August 19, 2026 13:08
… through an exception'

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
… through an exception'

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
… through an exception'

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
… through an exception'

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@paullizer
Paul Lizer (paullizer) merged commit 6232883 into Development Aug 19, 2026
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants