A workspace model shared publicly could be used by any user even when its
base model was private. Unregistered base models (no row in the model
table) are admin-only for direct use — get_filtered_models hides them from
non-admins and check_model_access rejects them — but has_base_model_access
treated a missing row as "no ACL" and allowed the chained request through.
has_base_model_access now takes the caller's role and only allows an
unregistered base model hop for admins, so a shared preset can no longer
reach a base model the caller could not use directly. Registered base
models keep their existing grant-based enforcement.
Claude-Session: https://claude.ai/code/session_018toPfJW1hMXAhokGaL43Ep
Co-authored-by: Claude <noreply@anthropic.com>
Docker compose list-form environment syntax passes quotes through verbatim, so WEB_FETCH_FILTER_LIST="" reaches the backend as two literal quote characters rather than an empty string. Config parsing turned that into the filter entry '""', which has no "!" prefix and therefore landed in the allow list. A non-empty allow list requires every host to match one of its entries, and a quotes-only pattern can never match a hostname, so every fetch_url and web loader request was rejected with "URL blocked by filter list" and surfaced to the user as "The URL you provided is invalid".
get_allow_block_lists now strips surrounding quote characters from each entry and drops entries that are empty after normalisation. Quoted but otherwise valid entries such as "example.com" or !"example.com" now behave as their unquoted forms, and garbage entries no longer convert the default blocklist into a match-nothing allowlist that blocks everything.
Fixes#26908
get_accessible_folder_files is the server-side filter that reduces a folder's attached-knowledge list (and, once #26723 lands, a direct model's) to the entries the caller may read, before that list is handed to the builtin knowledge tools as `__model_knowledge__`. It validated `file` and `collection` entries but passed `note` entries through unchecked (they fell into the `else` keep-as-is branch), even though notes are a first-class attached-knowledge type that flows through this list.
No current caller is exploitable, because every note consumer (`query_knowledge_files`, `view_note`, and the legacy retrieval path) independently re-checks note access before returning content. But relying on each consumer to remember that check is exactly the fragility this helper exists to remove, and the same `_has_read_access_to_file` membership short-circuit that makes an unvalidated `file` entry dangerous would turn any future note path that trusts list membership into an IDOR. Validate notes here so the filter enforces its own contract instead of leaning on downstream re-checks.
A note entry is now kept only when the caller owns it or holds a read grant. Notes are private by default and carry no self-grant, so ownership is checked explicitly alongside the grant lookup. Admins still bypass all checks and genuinely unknown types are still kept as-is.
Related: #26723
When WEBSOCKET_MANAGER=redis, app.state.MODELS and the socket session/
usage pools are Redis-backed dicts, so every membership test and
getitem is a network round trip:
- generate_chat_completion checked `model_id not in models` (HEXISTS)
and then read `models[model_id]` (HGET) on every chat completion.
A single .get() now serves both, with the same not-found error.
- The direct-connection branch spread the pool with `{**MODELS, ...}`,
which iterates keys() then fetches each value — HKEYS plus one HGET
per model. dict(MODELS.items()) issues a single HGETALL instead.
- get_user_ids_from_room called SESSION_POOL.get(sid) twice per
session (once to filter, once for the value); the usage handler
checked membership then fetched the same key. Both now do one
lookup.
In non-Redis mode these are plain dicts and behavior is identical.
Claude-Session: https://claude.ai/code/session_01MHg5zs1VBjvRWQ54qHpfYD
Co-authored-by: Claude <noreply@anthropic.com>
The streaming handler's content accumulator is a closure cell (declared nonlocal in stream_body_handler), and CPython's in-place string append optimization only applies to plain local variables (STORE_FAST), never to cell variables (STORE_DEREF). The content += value form introduced in #27231 therefore still allocates and copies the full accumulated string on every delta, exactly like the f-string it replaced; whether that copy is cheap or expensive is up to the allocator, and measurements swing accordingly (6 to 83 ms of pure copying for a 400 KB response on Python 3.12, against 0.3 ms for this patch).
Accumulate the deltas in a list instead and join once at the single read site (publish_chat_finished_event at stream end). List append is amortized O(1) with no dependence on reference counts, bytecode specialization or allocator behaviour, so accumulation is O(n) by construction. The non-str fallback keeps the previous f-string coercion semantics.
Verified end to end against a mock SSE upstream: a streamed chat with a think-tag block plus 40 content deltas produces output items, message text, reasoning text and usage identical to current dev, with no errors in the server log.
The header value was truncated with int(), so every request faster than one second reported X-Process-Time: 0 and the header carried no information for exactly the requests it is meant to describe. Emit fractional seconds with microsecond precision instead, matching the pre-ASGI-refactor behavior where the raw float was sent.
With audit logging enabled, every audited request authenticated twice. The route dependency resolved the user once, and then _log_audit_entry called get_current_user again in the request's finally block: a second JWT decode, two more Redis revocation lookups, a second user row fetch with pydantic validation and, crucially, a second fire-and-forget last-active write transaction per request.
get_current_user now stashes the resolved user on the scope-backed request state (the same mechanism the auth middleware already uses for request.state.token), and the audit middleware reuses it, falling back to the old resolution only when no user was stashed (e.g. routes without an auth dependency). While in the file, the audit path patterns are compiled once in the constructor instead of per request, and the always-log endpoint set is a class attribute instead of a per-call literal; both are fixed for the process lifetime.
Benchmark:
| metric | before | after |
| --- | --- | --- |
| audit auth resolution, CPU floor (JWT decode + user validate only) | 16.7 us | 0.24 us |
| extra work per audited request | 2 Redis GETs + 1 user SELECT + 1 last-active write | none |
The before column understates the saving: it excludes the Redis and DB round trips listed in the second row, which dominate in real deployments.
Functionally verified with a stacked ASGI harness: when the route resolves a user the audit entry carries that user and the auth pipeline is not invoked again; without a stashed user the fallback path still resolves and logs correctly; the skip matrix (exclusions, whitelist mode, always-log auth endpoints, unauthenticated and non-audited methods) is unchanged.
get_all_models runs on every models refresh and, without the base-models cache (off by default), on every /api/models request. Several of its costs multiplied by the model count for no reason:
- The active action and filter id sets were derived from get_functions_by_type, which loads full function rows including plugin source and validates them, only for the ids and is_global flags. A generalized column-only query now returns (id, is_global) tuples; the existing filter-specific helper delegates to it.
- Action priorities were computed inside the per-model sort key, constructing a pydantic Valves object per action per model; with global actions in every model's list that was models x actions constructions per refresh. Priorities are now memoized per action.
- Global action and filter item dicts were rebuilt per model from the same modules. The item lists are now built once per function and shallow-copied per model, keeping per-model dicts independent exactly as before (nested values were already shared).
- Deactivated base-model overrides were dropped with models.remove, a linear scan and shift per removal; removals are now collected and filtered out in one identity-based pass, preserving list.remove's exact object semantics.
- RedisDict.set fingerprinted the payload by serializing the already-serialized mapping a second time plus a sha256; a direct dict comparison against the last written mapping has the same skip semantics without re-serializing anything.
- /api/models did tag normalization and profile-image stripping for every model before access filtering discarded the invisible ones, and always evaluated a json.dumps debug f-string; the work now runs only on visible models and the debug line is gated on the log level. The duplicate-id dedup keeps its position before filtering so the effective-model semantics are unchanged.
Benchmark:
| metric | before | after |
| --- | --- | --- |
| model-cache fingerprint, 200 models | 45 us | 1.4 us |
| action priority Valves builds, 200 models x 4 global actions | 0.37 ms (800 builds) | 0.002 ms (4 builds) |
| function-table payload for id sets | full rows incl. source | (id, is_global) tuples |
Functionally verified: the column-only id query matches the full-row query for actions and filters including inactive exclusion, and the fingerprint skip logic writes on first set, skips identical payloads, updates plus deletes stale keys on change and clears on empty, against a scripted fake Redis.
has_access_to_file runs for every non-owner file GET, per RAG file check and per shared-chat or model-attached file. Its final step called Models.get_models_by_user_id, which issued one grant query per non-owned workspace model, so a single file check on an instance with M workspace models cost M grant queries plus a group query, with the deny path always paying full price. Its collection_name step listed every knowledge base the user can access (itself one grant query per knowledge base) just to scan the list for one id. And get_accessible_folder_files repeated the whole pipeline per folder entry, refetching the caller's group memberships every time.
Three changes, all using parameters and helpers that already exist:
- Models.get_models_by_user_id resolves grants for all non-owned models in one get_accessible_resource_ids call and accepts prefetched user_group_ids.
- The collection_name check fetches the one referenced knowledge base and performs a single owner-or-grant check with the already-resolved group ids, preserving the write-requires-owner guard exactly (including its short-circuit before any grant query).
- get_accessible_folder_files resolves group ids once and threads them through every per-entry check.
Benchmark:
| metric | before | after |
| --- | --- | --- |
| filter loop CPU, 300 workspace models (queries stubbed) | 47 us | 19 us |
| grant queries per file-access check, M workspace models | M | 1 |
| group membership queries per folder listing, F files | F | 1 |
The stubbed CPU row understates the win: each removed query in the other two rows was a real database round trip.
Functionally verified with stubbed accessors: owned plus granted models are returned with owned ids excluded from the batch query; model-attached file access resolves through the batched path; the collection_name path does one KB fetch and one grant check with no full listing; a missing KB falls through; write access via a KB still requires the KB owner to own the file and short-circuits before the grant query; folder listings fetch groups exactly once.
Two independent sources of fixed per-request cost:
The async SQLite engine was created with pool_pre_ping=True. A pre-ping guards against server connections dropped by timeouts or restarts, which cannot happen to a local SQLite file; each ping still costs a hop into the aiosqlite worker thread plus a SELECT 1 on every connection checkout, and with session sharing off a single request checks out a connection for every model-layer call it makes. The Postgres engines keep their pre-ping, where it is actually protective.
CommitSessionMiddleware unconditionally ran ScopedSession.commit() plus remove() after every HTTP request. The scoped registry instantiates a session on first access, so on the vast majority of requests (which never touch the sync session, per the middleware's own docstring) this built a Session, opened and committed an empty transaction and tore everything down for nothing. The middleware now checks ScopedSession.registry.has() first: requests that used the sync session are committed and removed exactly as before, on success and on the rollback path alike, and idle requests skip the machinery entirely.
Benchmark (real SQLite database):
| metric | before | after |
| --- | --- | --- |
| user row fetch incl. session + connection checkout | 681 us | 514 us |
| idle-request sync session work (create + empty commit + teardown) | 12.3 us | 0.26 us |
The first row saves per model-layer call, not per request: a request making five DB calls saves the checkout ping five times.
Functionally verified: normal reads and writes work with pre-ping off; an idle request through the middleware leaves no sync session behind; a request that uses the sync session still gets committed and removed.
The branch that normalizes plain JSON error lines from streaming upstreams (lines without the SSE data: prefix) called Chats.upsert_message_to_chat_by_id_and_message_id without await. The coroutine was never executed, so the error was never written to the chat and Python emitted a "coroutine was never awaited" RuntimeWarning instead. The frontend still received the error event, but after a reload the message showed no trace of the failure.
The parallel error-persist branch further down the same handler already awaits the call; this aligns the two.
Several spots in the chat pipeline issued sequential single-key config
SELECTs, or fetched the same key twice back-to-back, on every request:
- chat_completion_tools_handler: task model default/external and the
tools prompt template were four sequential Config round trips (the
template was fetched twice). One batched Config.get_many now serves
all of them.
- chat_completion_files_handler: the six RAG settings (top_k,
top_k_reranker, relevance_threshold, hybrid_bm25_weight,
enable_hybrid_search, full_context) were six sequential round trips
inside the retrieval call. Batched into one get_many.
- Voice and code-interpreter prompt templates were each fetched twice
within one conditional; fetch once and reuse. The code-interpreter
engine was likewise fetched twice per execution.
- Skill resolution fetched the accessible-skills list, kept only the
ids, then re-fetched each mentioned skill by id (N+1). Reuse the
rows from the access query.
Value semantics are identical: get_many applies the same defaults as
the individual gets, and the pre-existing truthiness/empty-string
checks on templates are preserved exactly.
Claude-Session: https://claude.ai/code/session_01MHg5zs1VBjvRWQ54qHpfYD
Co-authored-by: Claude <noreply@anthropic.com>
The middleware code-interpreter path defines `restricted_import` as `async def` and assigns it to `builtins.__import__`, which Python's import machinery calls synchronously. Calling an async function returns a coroutine without running its body, so the blocklist check never executes and `_real_import` is never called. When `CODE_INTERPRETER_BLOCKED_MODULES` is set, blocked modules are therefore not blocked, and every subsequent import inside the interpreter binds a dangling coroutine instead of the module, breaking legitimate imports as well.
Define the hook as a regular `def`, matching the working implementation in `tools/builtin.py`. A blocked top-level import now raises `ImportError`, and all other imports pass through to the real importer.
SecurityHeadersMiddleware called set_security_headers() on every
response — 14 os.environ.get lookups plus a regex validation per
configured header, for values that are static for the process
lifetime. Compute the header list once at construction; when no
security env vars are set, skip wrapping send entirely.
RedirectMiddleware decoded and parse_qs'd the query string of every
GET, though it only acts on /watch?v= and ?shared= URLs. Add a cheap
path/substring precheck first; a false positive just falls through to
the previous full parse, so no redirect behavior changes.
Verified byte-identical responses (status, Location, header values)
against the previous implementations across redirect, passthrough,
and no-env cases.
Claude-Session: https://claude.ai/code/session_01MHg5zs1VBjvRWQ54qHpfYD
Co-authored-by: Claude <noreply@anthropic.com>
The streaming handler rebuilt the full accumulated response with
`content = f'{content}{value}'` on every content delta — a complete
string copy per chunk, making accumulation O(n^2) over the response
length. Use in-place `content += value` for the (universal) str case,
which CPython extends in place, keeping accumulation O(n); the
f-string fallback is preserved for non-str values so coercion
behavior is unchanged.
In the ENABLE_REALTIME_CHAT_SAVE branch, full_output() — which
concatenates the entire accumulated output — was called twice per
chunk (once for the DB upsert, once for the emitted delta). Compute
it once and reuse.
Claude-Session: https://claude.ai/code/session_01MHg5zs1VBjvRWQ54qHpfYD
Co-authored-by: Claude <noreply@anthropic.com>
The chat action route loaded a Function by its raw action_id and executed its action callable after only checking that the id and the requested model existed. The model list that the client renders actions from resolves each model's actions to the active action-type Functions that are global or assigned to that model, and the action route did not mirror that resolution, so a disabled, unassigned, or wrong-type Function, or an action on a model the caller cannot access, could be reached by calling the route directly.
Gate the route on the same rules the model resolution applies: the Function must be an active action, and for server-resolved models the caller must have model access and the action must be one the model actually surfaces (matched by function id, the prefix of each model actions entry, so single and sub-actions both resolve). Direct connections carry a client-supplied model the caller already owns, so the model-bound checks are scoped to non-direct calls; the active-action check always applies. Executing admin-authored Function code remains intended behaviour — this only keeps the route consistent with which actions each model exposes.
Co-authored-by: komyunghan <komyunghan@users.noreply.github.com>
get_all_models ran four function-table queries: global actions, active
actions, global filters, active filters. Global functions are by
definition (type, is_active=True, is_global=True) — a subset of the
active set — so the global id sets can be derived from the active-rows
queries' is_global flag. Four queries become two, and each dropped
query returned full rows including every plugin's source code.
Also folds the mid-function 'models.default_metadata' read into the
Config.get_many already issued at the top of the function (one fewer
round trip; the existing `or {}` default handling is preserved).
Claude-Session: https://claude.ai/code/session_01MHg5zs1VBjvRWQ54qHpfYD
Co-authored-by: Claude <noreply@anthropic.com>
