Listing skills ran one database query per skill in the instance. A non-admin opening the list on a workspace with 500 skills issued over 500 queries, the paginated list re-resolved the caller's group membership once per row, and every chat message carrying a skill loaded every skill the user can read, full body and owner included, to use the two or three it actually referenced.
Skills now arrive already filtered: the owner-or-grant check runs in the query as an EXISTS subquery, the same way prompts and the search endpoints already do it, the per-item write flag uses the existing batch grant lookup, and the chat path asks only for the skill ids the request names.
Measured with 500 skills of which 3 are visible to the caller: 504 queries and ~300 ms before, 4 queries and ~2.6 ms after. The resulting set is unchanged for owner, public, direct-user, group and multi-grant entries, for both read and write.
A skill ID goes straight into the path of every mutating skill endpoint (/api/v1/skills/id/{id}/...), but create only replaced spaces with hyphens. An ID containing a "/" was stored verbatim as the primary key, so the route never matched, the request fell through to the SPA static mount and the client got 405 Method Not Allowed. The skill could not be opened, edited, toggled or deleted, by admins either, and since skill.name is UNIQUE it could not be recreated under a corrected ID. Percent-encoding does not help: uvicorn decodes the path before Starlette routes it, so the only remaining fix was a direct database write.
Create now rejects any ID outside [a-z0-9_-] with 400 instead of silently storing an unreachable one. Two frontend paths that fed unsanitized IDs into it are fixed as well: the manual "Skill ID" field, which was bound with no sanitization at all and is the path that reproduces on every version, and the markdown import, which put the raw frontmatter name into the ID before opening the editor in clone mode, where the reactive slugify is disabled.
Existing rows with an unreachable ID are not repaired here; rewriting a primary key would also have to re-point the access grants keyed on it.
Fixes#27655
The /create (L155-193) and /id/{id}/update (L248-297) endpoints in
routers/skills.py persisted form_data.access_grants directly to
AccessGrants.set_access_grants without filter_allowed_access_grants,
while every other shareable resource in the codebase (channels, knowledge,
models, notes, prompts, tools, calendars) and the dedicated
/id/{id}/access/update endpoint on this same router (L309-348) all do call
the filter. A user with workspace.skills permission (default False, but
admins can grant it to skill-creating users) could therefore attach
{"principal_type":"user","principal_id":"*","permission":"read"|"write"}
to the create or update payload and have it persisted unfiltered, bypassing
the sharing.public_skills gate that the rest of the cohort enforces.
Two changes:
- create_new_skill: call filter_allowed_access_grants with
'sharing.public_skills' immediately before insert, after the existing
permission check and ID-taken check.
- update_skill_by_id: call filter_allowed_access_grants with the same key
after the access check, before form_data.model_dump() flows into
Skills.update_skill_by_id. The pre-existing access check at L263-277 only
restricts WHO may modify the skill; the new filter restricts WHICH grants
they may set.
All supporting plumbing was already in place from prior PRs:
filter_allowed_access_grants is already imported at L22, the
USER_PERMISSIONS_WORKSPACE_SKILLS_ALLOW_PUBLIC_SHARING constant exists,
DEFAULT_USER_PERMISSIONS['sharing']['public_skills'] is wired up,
SharingPermissions.public_skills is in the Pydantic, and the admin UI
already renders the toggle. This is a pure 2-line router fix that closes
the cohort-consistency gap.
Same shape as the calendar fix in #24493, reported by Matteo Panzeri while
auditing the resource-cohort cohort during follow-up on #24493.
Co-authored-by: Matteo Panzeri <28739806+matte1782@users.noreply.github.com>
The sharePublic prop in editor components (Knowledge, Tools, Skills,
Prompts, Models) incorrectly included an "|| edit" / "|| write_access"
condition, allowing users with write access to see and use the "Public"
sharing option regardless of their actual public sharing permission.
Additionally, all backend access/update endpoints only verified write
authorization but did not check the corresponding sharing.public_*
permission, allowing direct API calls to bypass frontend restrictions
entirely.
Frontend: removed the edit/write_access bypass from sharePublic in all
five editor components so visibility is gated solely by the user's
sharing.public_* permission or admin role.
Backend: added has_public_read_access_grant checks to the access/update
endpoints in knowledge.py, tools.py, prompts.py, skills.py, models.py,
and notes.py. Public grants are silently stripped when the user lacks
the corresponding permission.
Fixes#21356
