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
