fix: gate public sharing of skills behind sharing.public_skills on create/update (#24494)

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>
This commit is contained in:
Classic298 2026-05-09 16:19:03 +02:00 committed by GitHub
parent 8a0018cf96
commit 9918ab6265
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -176,6 +176,19 @@ async def create_new_skill(
detail=ERROR_MESSAGES.ID_TAKEN,
)
# Strip public/user grants the requesting user is not permitted to assign
# (matches the channel/notes/calendar pattern). Without this, a user with
# workspace.skills permission could attach principal_id='*' read/write
# grants in the create payload, bypassing the sharing.public_skills gate
# that the dedicated /access/update endpoint already enforces.
form_data.access_grants = await filter_allowed_access_grants(
request.app.state.config.USER_PERMISSIONS,
user.id,
user.role,
form_data.access_grants,
'sharing.public_skills',
)
try:
skill = await Skills.insert_new_skill(user.id, form_data, db=db)
if skill:
@ -276,6 +289,19 @@ async def update_skill_by_id(
detail=ERROR_MESSAGES.UNAUTHORIZED,
)
# Strip public/user grants the requesting user is not permitted to assign
# (matches the channel/notes/calendar pattern). The access check above only
# restricts WHO can write to the skill; this filter restricts WHICH grants
# they may set, so a non-admin owner cannot make their own skill publicly
# readable/writable without sharing.public_skills permission.
form_data.access_grants = await filter_allowed_access_grants(
request.app.state.config.USER_PERMISSIONS,
user.id,
user.role,
form_data.access_grants,
'sharing.public_skills',
)
try:
updated = {
**form_data.model_dump(exclude={'id'}),