Feat: on_shared_thread_view is only used to verify that thread can be… by peleccom · Pull Request #2925 · Chainlit/chainlit

peleccom · 2026-05-16T10:00:31Z

Title
Add on_shared_thread_access_allowed

Description

Problem

The GET /project/share/{thread_id} endpoint had a dual-gate authorization model:

An on_shared_thread_view callback (app-defined, flexible)
A hardcoded is_shared metadata flag check
This was redundant and restrictive. Apps that implemented a custom on_shared_thread_view
callback to control access (e.g., allowing admin users to view any thread regardless of
the is_shared flag) were blocked by the hardcoded check. The docstring claimed the
endpoint checked is_shared=True, but the semantic intent was always to let the
callback control access.
It was no way to restrict access to shared thread Once thread was shared anyone can view it. No matter the result of on_shared_thread_view call, even if we return False, thread is still visible to anyone. It is quite big security issue, in case we want to specify who can view thread

Change

Added new callback on_shared_thread_access_allowed.
It has the same call signature as on_shared_thread_view but can restict access to any shared thread.

If it return False user will see 404 error.
If not defined or returns True then access to thread is allowed.

This does not break existing logic with is_shared and on_shared_thread_view.

Summary by cubic

Adjust shared thread access. GET /project/share/{thread_id} keeps is_shared as an allow flag, uses on_shared_thread_view only when the thread isn’t shared, and adds on_shared_thread_access_allowed to optionally block even shared threads.

New Features
- Added on_shared_thread_access_allowed; if defined and returns False or raises, respond 404.
- Exposed in chainlit.callbacks, added to CodeSettings, and exported via chainlit.__init__.
Bug Fixes
- Shared threads can now be denied at the app level via the new callback. Tests cover is_shared fallback, on_shared_thread_view gating, and on_shared_thread_access_allowed paths.

^{Written for commit 4f6ee71. Summary will update on new commits.}

Review in cubic

… viewved

cubic-dev-ai

No issues found across 2 files

_{Re-trigger cubic}

Copilot

Pull request overview

This PR fixes the authorization behavior of GET /project/share/{thread_id} by making the app-defined on_shared_thread_view callback the only access gate, removing the previously hardcoded metadata.is_shared check that could bypass callback denials.

Changes:

Updated get_shared_thread to raise 404 whenever on_shared_thread_view denies access or errors (removing the is_shared fallback).
Updated callback documentation to reflect the callback as the sole authorizer for the shared-thread endpoint.
Added parameterized server tests to validate access outcomes across is_shared values and callback results.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
backend/chainlit/server.py	Removes the `is_shared` metadata gate and makes `on_shared_thread_view` the sole access controller.
backend/chainlit/callbacks.py	Updates `on_shared_thread_view` docs to match the endpoint’s new authorization semantics.
backend/tests/test_server.py	Adds tests covering allowed/denied/error outcomes for shared thread access.

+    _app.dependency_overrides[_get_current_user] = lambda: viewer
+
+    dl = AsyncMock()
+    dl.get_thread.return_value = {
+        "id": "shared-thread-1",
+        "name": "Shared Thread",
+        "userIdentifier": "author",
+        "metadata": {"is_shared": is_shared, "chat_profile": "pro"},
+    }
+    dl.get_thread_author.return_value = "author"
+    dl.build_debug_url.return_value = ""
+
+    data_mod._data_layer = dl
+    data_mod._data_layer_initialized = True
+
+    async def deny_cb(thread, user):
+        if isinstance(on_shared_thread_view_result, Exception):
+            raise on_shared_thread_view_result
+        return on_shared_thread_view_result
+
+    test_config.code.on_shared_thread_view = deny_cb
+
+    r = test_client.get("/project/share/shared-thread-1")
+
+    if allowed:
+        assert r.status_code == 200
+        assert r.json()["id"] == "shared-thread-1"
+    else:
+        assert r.status_code == 404
+        assert r.json() == {"detail": "Thread not found"}
+
+    # Cleanup
+    del _app.dependency_overrides[_get_current_user]
+    data_mod._data_layer = None
+    data_mod._data_layer_initialized = False
+    test_config.code.on_shared_thread_view = None


 def on_shared_thread_view(
    func: Callable[[ThreadDict, Optional[User]], Awaitable[bool]],
 ) -> Callable[[ThreadDict, Optional[User]], Awaitable[bool]]:
    """Hook to authorize viewing a shared thread.

    Users must implement and return True to allow a non-author to view a thread.
-    Thread metadata contains "is_shared" boolean flag and "shared_at" timestamp for custom thread sharing.
+    This callback is the sole gatekeeper for the GET /project/share/{thread_id} endpoint.
+    Thread metadata may contain "is_shared" boolean flag and "shared_at" timestamp for custom thread sharing.
    Signature: async (thread: ThreadDict, viewer: Optional[User]) -> bool


        except Exception:
            user_can_view = False

-    is_shared = bool(metadata.get("is_shared"))
-
-    # Proceed only raise an error if both conditions are False.
-    if (not user_can_view) and (not is_shared):
+    # Proceed only raise an error if user_can_view return False or exception
+    if not user_can_view:
        raise HTTPException(status_code=404, detail="Thread not found")


+        return on_shared_thread_view_result
+
+    test_config.code.on_shared_thread_view = deny_cb
+
+    r = test_client.get("/project/share/shared-thread-1")
+


dokterbob

I see where you're coming from and the intended cleanup.

But your PR seems to pull out the is_shared metadata key entirely. Whereas that does seem to be a prominent feature in the UI. It's spread out in quite a few places: https://github.com/search?q=repo%3AChainlit%2Fchainlit+is_shared&type=code

Can we address these security concerns perhaps through an approach which will not break pre-existing functionality?

dokterbob · 2026-05-26T16:30:05Z

+    (True, False, False),
+    (True, ValueError("error"), False),
+    (False, True, True),
+    (False, False, False),


So this removes the metadata["is_shared"] feature, right?
(Please forgive me if I don't fully follow, this codebase is a ... complex beast.)

If we remove this, should it still be in the tests?

… to threads

cubic-dev-ai

2 issues found across 5 files (changes from recent commits).

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="backend/chainlit/server.py">

<violation number="1" location="backend/chainlit/server.py:1029">
P0: Authorization bypass: is_shared=True grants access even when on_shared_thread_view callback denies access, undermining the stated security goal</violation>
</file>

<file name="backend/tests/test_server.py">

<violation number="1" location="backend/tests/test_server.py:1161">
P1: Callback-denied and callback-error shared-thread cases are incorrectly expected to succeed, so this test would not catch a regression where `on_shared_thread_view` is ignored.</violation>
</file>

_{Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic}

cubic-dev-ai · 2026-05-27T22:38:39Z

    is_shared = bool(metadata.get("is_shared"))

-    # Proceed only raise an error if both conditions are False.
    if (not user_can_view) and (not is_shared):


P0: Authorization bypass: is_shared=True grants access even when on_shared_thread_view callback denies access, undermining the stated security goal

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At backend/chainlit/server.py, line 1029: <comment>Authorization bypass: is_shared=True grants access even when on_shared_thread_view callback denies access, undermining the stated security goal</comment> <file context> @@ -1025,11 +1024,23 @@ async def get_shared_thread( - # Proceed only raise an error if user_can_view return False or exception - if not user_can_view: + if (not user_can_view) and (not is_shared): raise HTTPException(status_code=404, detail="Thread not found") </file context>

cubic-dev-ai · 2026-05-27T22:38:39Z

+        (True, False, True),
+        (True, ValueError("error"), True),


P1: Callback-denied and callback-error shared-thread cases are incorrectly expected to succeed, so this test would not catch a regression where on_shared_thread_view is ignored.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At backend/tests/test_server.py, line 1161: <comment>Callback-denied and callback-error shared-thread cases are incorrectly expected to succeed, so this test would not catch a regression where `on_shared_thread_view` is ignored.</comment> <file context> @@ -1154,13 +1154,16 @@ def test_health_check(test_client: TestClient): + ("is_shared", "on_shared_thread_view_result", "allowed"), + [ + (True, True, True), + (True, False, True), + (True, ValueError("error"), True), + (False, True, True), </file context>

Suggested change

(True, False, True),

(True, ValueError("error"), True),

(True, False, False),

(True, ValueError("error"), False),

peleccom · 2026-05-28T08:49:07Z

@dokterbob Hi, thanks for review and comments. I have checked this logic again and now it looks like feature, nothing in current behaviour is broken in case we know the idea behind on_shared_thread_view.

In this PR I suggest adding new callback on_shared_thread_access_allowed which allow to block access to shared thread (For example based on current user permissions)

Feat: on_shared_thread_view is only used to verify that thread can be…

a06d7b6

… viewved

peleccom requested review from asvishnyakov, hayescode and sandangel as code owners May 16, 2026 10:00

dosubot Bot added size:S This PR changes 10-29 lines, ignoring generated files. security labels May 16, 2026

cubic-dev-ai Bot reviewed May 16, 2026

View reviewed changes

Chore: Add tests for get shared thread permissions

e2c88f9

dokterbob added the unit-tests Has unit tests. label May 26, 2026

dokterbob requested a review from Copilot May 26, 2026 16:25

Copilot started reviewing on behalf of dokterbob May 26, 2026 16:25 View session

Copilot AI reviewed May 26, 2026

View reviewed changes

dokterbob requested changes May 26, 2026

View reviewed changes

Feat: Add on_shared_thread_access_allowed callback to restrict access…

e90f752

… to threads

dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels May 27, 2026

cubic-dev-ai Bot reviewed May 27, 2026

View reviewed changes

peleccom requested a review from dokterbob May 28, 2026 08:49

Merge branch 'main' into fix/fix-shared-thread-permisions

4f6ee71

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: on_shared_thread_view is only used to verify that thread can be…#2925

Feat: on_shared_thread_view is only used to verify that thread can be…#2925
peleccom wants to merge 4 commits into
Chainlit:mainfrom
peleccom:fix/fix-shared-thread-permisions

peleccom commented May 16, 2026 •

edited by cubic-dev-ai Bot

Loading

Uh oh!

cubic-dev-ai Bot left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

dokterbob left a comment

Uh oh!

Uh oh!

dokterbob May 26, 2026

Uh oh!

cubic-dev-ai Bot left a comment

Uh oh!

cubic-dev-ai Bot May 27, 2026

Uh oh!

cubic-dev-ai Bot May 27, 2026

Uh oh!

peleccom commented May 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

peleccom commented May 16, 2026 • edited by cubic-dev-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Title Add on_shared_thread_access_allowed

Problem

Change

Summary by cubic

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

dokterbob left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dokterbob May 26, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot May 27, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot May 27, 2026

Choose a reason for hiding this comment

Uh oh!

peleccom commented May 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

peleccom commented May 16, 2026 •

edited by cubic-dev-ai Bot

Loading

Title
Add on_shared_thread_access_allowed