Skip to content

[permissions] Fix the access holes and status codes found by the spec review - #1327

Merged
frankrousseau merged 44 commits into
cgwire:mainfrom
frankrousseau:main
Oct 29, 2026
Merged

[permissions] Fix the access holes and status codes found by the spec review#1327
frankrousseau merged 44 commits into
cgwire:mainfrom
frankrousseau:main

Conversation

@frankrousseau

@frankrousseau frankrousseau commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

Problem

  • Several routes read the global role instead of the project role, so per-project vendors, clients, managers and supervisors got the wrong access
  • Some writes were badly guarded: task creation, share link passwords and guest names, guest annotations, sequence and entity deletion, budgets, previews and working files for clients, breakdown links, parent ids on PUT, comment replies and attachments, shared filters
  • Managers' CSV export leaked the daily salary, and the test event route was open to all
  • Wrong status codes: 500 on bad dates, 400 instead of 403 on Kitsu imports, raw database errors returned by imports
  • The OpenAPI specs did not follow these fixes

Solution

  • Resolve the project role before each role check, and scope listings to the projects the caller manages or supervises
  • Add the missing permission and parent checks, and protect the parent ids on PUT
  • Keep the daily salary and the test event route to administrators
  • Answer 400 on wrong dates, 403 on Kitsu imports, and sanitize import database errors
  • Update the OpenAPI specs to match

frankrousseau and others added 30 commits October 28, 2026 19:18
@frankrousseau @claude
[tasks] Restrict task creation to managers of the entity project
f4abece 
POST /data/tasks ran no permission check. It now requires an admin or a
manager of the entity project, and the task project_id follows the
entity instead of the body.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@frankrousseau @claude
[playlists] Take the share link password on every shared route
44dee24 
Only four shared routes read the password query parameter: a protected
link answered 404 to guest creation, media and comment edition even with
the right password. The password check moves out of validate_share_token,
which the services call again without it, into the route decorator.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@frankrousseau @claude
[imports] Sanitize database errors returned by CSV and Kitsu imports
300719c 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@frankrousseau @claude
[imports] Deny Kitsu imports to non-admins with a 403
e1ea5a0 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@frankrousseau @claude
[persons] Check admin permissions before the user limit on creation
d2f1024 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@frankrousseau @claude
[playlists] Stop reusing a guest by name on a share link
8d8857f 
Without a valid guest_id, guest creation returned the existing guest
carrying the same name on the link: typing a reviewer's name took their
identity. A guest is now reused through its id only.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@frankrousseau @claude
[playlists] Keep a guest to its own annotations
d1a5597 
A guest could update or delete every drawing object of a preview,
including those of the team. Guest additions are now stamped with the
guest id and guest updates and deletions are kept to the objects stamped
with it, keyed by time and id.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@frankrousseau @claude
[shots] Fix the inverted creator check on sequence deletion
f7b54b3 
Any team member who was not the creator could delete a sequence, and the
creator needed manager access.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@frankrousseau @claude
[time-spents] Fix DELETE status, admin access and undated listing
29e35b1 
- Return 200 instead of 201 when deleting a time spent
- Let admins handle the time of a person no longer assigned to the task
- List every time spent of a person when no date is given

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@frankrousseau @claude
[imports] Return an empty body when deleting a Shotgun import error
0cd7a7e 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@frankrousseau @claude
[persons] Answer 400 to wrong dates on studio day offs and desktop lo…
dbb2a0d 
…gins

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@frankrousseau @claude
[entities] Deny entity deletion to a creator out of the team
b9fed7a 
The eight entity deletion routes ignored the result of
check_belong_to_project, so the creator of an asset, shot, scene,
episode, sequence, concept or edit deleted it after leaving the project.
The rule now lives in check_entity_delete_access, which raises.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@frankrousseau @claude
[tests] Cover a supervisor with departments sending no metadata
39f427a 
The fix itself (no KeyError when the body has no data) landed in
b9fed7a.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@frankrousseau @claude
[projects] Scope the budget routes to their project and budget
940dbfe 
ProductionBudgetResource PUT and DELETE took a budget of any project,
and ProductionBudgetEntryResource an entry of any budget. get_budget and
get_budget_entry now take the parent id and answer 404 on a mismatch.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@frankrousseau @claude
[departments] Fix the route getting a hardware item of a department
f97f106 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@frankrousseau @claude
[assets] Compare entity type names without LIKE wildcards
206dcba 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@frankrousseau @claude
[entities] Scope the vendor entity list to their tasks
e1d90a3 
join(Task) after join(Project) joined the tasks on the project: a vendor
got every entity of the project.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@frankrousseau @claude
[files] Document the output file list, update and delete routes
526edb5 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@frankrousseau @claude
[breakdown] Fix the docstring of get_production_episodes_casting
4066906 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@frankrousseau @claude
[comments] Let a manager out of the team delete their own comment
7b28b9b 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@frankrousseau @claude
[tasks] Keep clients out of the previews and working files
856e993 
check_task_action_access lets a client through, as a client comments on
a task. The routes adding, removing, uploading or repositioning previews
and the working file updates now go through check_task_work_access,
which also blocks the client role resolved on the project.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@frankrousseau @claude
[breakdown] Keep clients and vendors out of scene links and asset ins…
796d1e6 
…tances

A client or a vendor of the project could link or unlink shots and
scenes, with a shot of another project, and create or remove asset
instances. These writes now go through check_project_edition_access and
the shot must belong to the scene project.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@frankrousseau @claude
[crud] Protect the parent ids of preview files, asset instances and c…
888fb79 
…omments

A PUT moved a preview file to another task, an asset instance to another
asset, scene or target asset, and a comment to another task: the update
checks read the project off the stored parent. The asset instance update
also took any member of the project, clients included.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@frankrousseau @claude
[comments] Check the comment access on attachment file reads
8864ed0 
AttachmentFile GET checked the access to the task, not to the comment:
a client read the attachments of the internal comments.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@frankrousseau @claude
[comments] Check the task of a comment on ack and reply deletion
3630515 
Acknowledging a comment and deleting a reply took any comment id under
any task. Both now load the comment through get_task_comment, and the
reply deletion checks the task access, which it never did.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@frankrousseau @claude
[comments] Keep the task access check for the author of a comment
4e13f77 
The author of a comment skipped every check when replying to it, out of
the team included. The reply now checks the task access first.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@frankrousseau @claude
[persons] Keep the daily salary out of the managers' CSV export
97b6240 
The persons export is open to the managers and carried the daily salary
column. The column now shows to the admins only.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@frankrousseau @claude
[shots] Keep a vendor to its own shots, scenes and previews
41d7168 
A vendor got every shot of an episode and every scene of a sequence, and
read the preview of an assigned task without being on the team. The two
listings now drop the entities the vendor holds no task on, the episode
shots the way the episode sequences already did, and the preview read
checks the project access first.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@frankrousseau @claude
[tests] Put the client in the scene project before making the other one
8fa7044 
generate_fixture_project moves self.project_id onto the new project, so
the client landed in the other team and the 403 held for another reason.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@frankrousseau @claude
[filters] Leave the filters of the other members when a group goes
19dd211 
Removing a filter group deleted every filter it held, those of the other
members of a shared group included. Theirs are now detached from the
group, only the owner's go with it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
frankrousseau and others added 14 commits October 28, 2026 20:00
@frankrousseau @claude
[filters] Keep the shared filters within the teams of their project
ac31f69 
A shared filter or group reached every open project, not only the ones
the caller is on the team of. The listings now keep them to the caller's
teams, or to everyone when they name no project; admins are not scoped.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@frankrousseau @claude
[files] File a new working file as the current user
327ae93 
NewWorkingFile took any person_id as the author. Only a manager of the
project may file on behalf of someone else now.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@frankrousseau @claude
[projects] Validate the default preview background on update
5086e6d 
default_preview_background_file_id was checked against the project
backgrounds on creation only. The update now checks both background
fields, reading the stored project when the body carries no list.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@frankrousseau @claude
[status] Keep the test event route to the admins
42fe991 
/status/test-event asked for no authentication and broadcast an event to
every connected client.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@frankrousseau @claude
[entities] Read the vendor role per project in the entity listing
01472fb 
The listing read the global role: a vendor on one project only got every
entity of it. Each entity now follows the role held on its own project,
through the new get_project_ids_with_roles helper.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@frankrousseau @claude
[tasks] Read the vendor and client roles per project in the task listing
1fb94e3 
The vendor filter and the client blanking read the global role. Each
task now follows the role held on its own project, and a vendor must be
on the team as everywhere else.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@frankrousseau @claude
[files] Read the vendor role per project in the output file listing
bc0cb3e 
OutputFilesResource blocked the global vendor role only. The listing now
leaves out the projects the user is a vendor on.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@frankrousseau @claude
[playlists] Let a manager of the project handle its share links
8d3c27f 
The share link routes checked the global manager role before resolving
the project, so a manager on the project only got a 403.
check_manager_project_access already covers the rule.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@frankrousseau @claude
[persons] Scope the time spents to the projects managed or supervised
b150572 
The time spent routes read the global role and opened every project of
the caller. They now keep to the projects where the caller is a manager
or a supervisor, read per project.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@frankrousseau @claude
[tasks] Scope the tasks to check to the projects managed or supervised
d9860a5 
The to-check scope read the global role. It now keeps to the projects
where the caller is a manager or a supervisor, read per project.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@frankrousseau @claude
[notifications] Read the client and artist roles per project when bla…
b84410f 
…nking

A client comment is blanked for an artist, but both roles were read
globally: a client on the project only kept their text visible, and an
artist on the project only was not covered. Both are read on the project
of the notification now.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@frankrousseau @claude
[persons] Answer 400 to wrong dates on the day offs of a person
f960d8b 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@frankrousseau @claude
[persons] Read the department restriction of a supervisor per project
0161e2b 
A supervisor of one project who also manages another had the department
filter of their supervision applied to every project, so they missed the
other departments of the projects they manage, in the time spents of a
person and in the tasks to check.

user_service.build_supervision_scope_filter builds one task filter from
the roles held per project: every task of a managed project, the tasks of
the supervisor's departments on a supervised one. The person time spent
routes and the tasks to check pass this filter instead of a flat list of
departments, the explicit department_id query parameter keeps its own
filter.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@frankrousseau @claude
[docs] Match the OpenAPI specs to the permission and status fixes
667fe4b 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@frankrousseau
frankrousseau merged commit 530f8a0 into cgwire:main Oct 29, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@frankrousseau