fix: copilot breaking change introduced in 2.8.5

[asvishnyakov]

Fixes #2641

• Custom auth and copilot token auth tests added
• Copilot tests refactored
• Added e2e test for previously fixed security vulnerability

[jbcallaghan]

The issue seems to be the following:

2.8.4 (works):

@sio.on("connect") # pyright: ignore [reportOptionalCall]
async def connect(sid, environ, auth):

2.8.5 (broken):

@sio.on("connect") # pyright: ignore [reportOptionalCall]
async def connect(sid: str, environ: WSGIEnvironment, auth: WebSocketSessionAuth):

[asvishnyakov]

@jbcallaghan These are just type annotations, they are ignored by Python at runtime.

Thread {thread_id} not found

can only be caused by passing non-existing thread ID to get_thread_author

Can you tell what value it has? Like a None, empty string or real GUID?

[jbcallaghan]

n only be caused by passing non-existing thread ID to get_thread_author

Sorry, that was a red herring. At first I thought that the token wasn't being retrieved, but it is.

I tried the following in socket.py and the user details are definitely being retrieved.

@sio.on("connect") # pyright: ignore [reportOptionalCall]
async def connect(sid: str, environ, auth):
user: User | PersistedUser | None = None
token: str | None = None
thread_id = auth.get("threadId")

if require_login():
    try:
        user, token = await _authenticate_connection(environ)
        print (f"user: {user}  token: {token}")

The issue occurs here, where the user.identifier is populated, there is a thread_id value, but the ChainlitDataLayer can't find the thread in the DB:

        if not (await data_layer.get_thread_author(thread_id) == user.identifier):
            logger.error("Authorization for the thread failed.")
            raise ConnectionRefusedError("authorization failed")

ChainlitDataLayer raises the error below as there is no matching thread in the DB at this stage when using co-pilot mode.

async def get_thread_author(self, thread_id: str) -> str:
    query = """
    SELECT u.identifier
    FROM "Thread" t
    JOIN "User" u ON t."userId" = u.id
    WHERE t.id = $1
    """
    results = await self.execute_query(query, {"thread_id": thread_id})
    if not results:
        raise ValueError(f"Thread {thread_id} not found")
    return results[0]["identifier"]                

[jbcallaghan]

In answer to your question, this is an example of the thread value:

ValueError: Thread d7472f51-b3d2-4361-835d-04a54c3f8cb0 not found

[jbcallaghan]

The following mod works, checking the clientType to see if it is copilot.

@sio.on("connect") # pyright: ignore [reportOptionalCall]
async def connect(sid: str, environ, auth):
user: User | PersistedUser | None = None
token: str | None = None
thread_id = auth.get("threadId")
client_type = auth.get("clientType")

if require_login():
    try:
        user, token = await _authenticate_connection(environ)

    except Exception as e:
        print("Exception authenticating connection: %s", e)
        logger.exception("Exception authenticating connection: %s", e)

    if not user:
        logger.error("Authentication failed in websocket connect.")
        raise ConnectionRefusedError("authentication failed")

    if thread_id and client_type != "copilot":
        data_layer = get_data_layer()
        if not data_layer:
            logger.error("Data layer is not initialized.")
            raise ConnectionRefusedError("data layer not initialized")

        if not (await data_layer.get_thread_author(thread_id) == user.identifier):
            logger.error("Authorization for the thread failed.")
            raise ConnectionRefusedError("authorization failed")

[cubic-dev-ai]

4 issues found across 7 files

Prompt for AI agents (all 4 issues)

Understand the root cause of the following 4 issues and fix them.


<file name="cypress/e2e/data_layer/spec.cy.ts">

<violation number="1" location="cypress/e2e/data_layer/spec.cy.ts:234">
The new access-control test asserts on the UI right after `startNewThread()` without waiting for the `@threadHijack` intercept to fire. Without a `cy.wait(&#39;@threadHijack&#39;)`, the hijack request might never be sent before the assertion, so the test can report success even if a stolen thread loads, defeating the purpose of the security regression.</violation>
</file>

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

<violation number="1" location="backend/chainlit/socket.py:137">
Connections with a threadId now proceed when the data layer is unavailable, skipping the previous authorization denial and allowing unvalidated thread access.</violation>
</file>

<file name="cypress/e2e/auth/spec.cy.ts">

<violation number="1" location="cypress/e2e/auth/spec.cy.ts:45">
This test waits on `@user`, but the alias is registered after the page has already loaded `/user` (support/e2e.ts visits `/` before every test) and the test never re-triggers the request, so the wait can never resolve. Move the intercept before a fresh `cy.visit(&#39;/&#39;)` or force another `/user` request before waiting.</violation>

<violation number="2" location="cypress/e2e/auth/spec.cy.ts:79">
The `/user` alias is registered only after the global `cy.visit(&#39;/&#39;)` has already fired the request, and the test never causes another `/user` call before `cy.wait(&#39;@user&#39;)`. As written this test always times out; register the intercept before a new navigation or explicitly trigger `/user` again before waiting.</violation>
</file>

_{Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR}

[cubic-dev-ai]

1 issue found across 2 files (reviewed changes from recent commits).

Prompt for AI agents (all 1 issues)

Understand the root cause of the following 1 issues and fix them.


<file name="cypress/e2e/data_layer/main.py">

<violation number="1" location="cypress/e2e/data_layer/main.py:160">
get_thread_author now assumes the thread exists and will crash with a TypeError if the thread cannot be found. Ensure the result of get_thread is validated before reading userIdentifier.</violation>
</file>

_{Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR}