Summary
Every permission check in routers/collections.py was inert. PermissionResponse defined no __bool__, so an instance was always truthy, and this router was the only module that tested the response object directly instead of reading .permitted.
Fixed in #821.
Root cause
>>> denied = PermissionResponse(False, 404, "not found")
>>> denied.permitted
False
>>> bool(denied)
True
Consequently:
[assoc for assoc in item.score_set_associations if has_permission(...)] # keeps everything
if not has_permission(user_data, item, Action.ADD_ROLE): # never taken
Every other module in the codebase reads .permitted. collections.py did not, at all 24 sites.
Impact
Collection user roster disclosed to non-admins — the more serious half
The narrowing branch at collections.py:152 never executed, so the full roster was returned to anyone who could read the collection, including anonymous callers on a public collection:
"viewers": [{"orcidId": "1234-5678-8765-4321", "firstName": "Extra", "lastName": "User"}]
Names and ORCID iDs of every editor and viewer. No navigation required — the disclosure is complete in a single GET. For private-but-shared collections this reveals collaboration structure.
The intended rule was implemented correctly in list_my_collections, which gates on the caller's own collection role. Only fetch_collection and the write endpoints were affected.
Private member URNs disclosed — bounded
The 18 association filters kept every member, so a readable collection listed the URNs of private score sets and experiments:
"scoreSetUrns": ["tmp:398c098d-24ae-483d-bc9e-7f7f8da970ed"]
Bounded in severity: the collection view model emits URNs only, and GET /score-sets/{urn} still returns 404 for an unentitled caller because assert_permission reads .permitted correctly. So this is existence-and-membership disclosure that dead-ends — an unentitled caller learns a private score set exists and belongs to this collection, and that it is unpublished (tmp: URN), but cannot retrieve it.
Access to private collections was never affected.
Affected endpoints
fetch_collection, update_collection, add_score_set_to_collection, delete_score_set_from_collection, add_experiment_to_collection, delete_experiment_from_collection, add_user_to_collection, remove_user_from_collection, and the association filters in list_my_collections.
Secondary risk introduced by fixing it
Both association relationships carry cascade="all, delete-orphan", and the filters are spelled as assignments back to the mapped collection. While the filters were inert nothing was ever dropped, so no orphans were staged. Once they work, real orphan deletes are staged on every affected request — unpersisted only because get_db never commits, and in 8 sites only because the narrowing happens after the handler's db.commit().
That invariant held by accident and had no test. #821 adds test_narrowing_associations_does_not_delete_them to pin it. #812 and #818 remove the idiom structurally.
Why this matters beyond the fix
This represents additional evidence for #808.
Summary
Every permission check in
routers/collections.pywas inert.PermissionResponsedefined no__bool__, so an instance was always truthy, and this router was the only module that tested the response object directly instead of reading.permitted.Fixed in #821.
Root cause
Consequently:
Every other module in the codebase reads
.permitted.collections.pydid not, at all 24 sites.Impact
Collection user roster disclosed to non-admins — the more serious half
The narrowing branch at
collections.py:152never executed, so the full roster was returned to anyone who could read the collection, including anonymous callers on a public collection:Names and ORCID iDs of every editor and viewer. No navigation required — the disclosure is complete in a single
GET. For private-but-shared collections this reveals collaboration structure.The intended rule was implemented correctly in
list_my_collections, which gates on the caller's own collection role. Onlyfetch_collectionand the write endpoints were affected.Private member URNs disclosed — bounded
The 18 association filters kept every member, so a readable collection listed the URNs of private score sets and experiments:
Bounded in severity: the collection view model emits URNs only, and
GET /score-sets/{urn}still returns 404 for an unentitled caller becauseassert_permissionreads.permittedcorrectly. So this is existence-and-membership disclosure that dead-ends — an unentitled caller learns a private score set exists and belongs to this collection, and that it is unpublished (tmp:URN), but cannot retrieve it.Access to private collections was never affected.
Affected endpoints
fetch_collection,update_collection,add_score_set_to_collection,delete_score_set_from_collection,add_experiment_to_collection,delete_experiment_from_collection,add_user_to_collection,remove_user_from_collection, and the association filters inlist_my_collections.Secondary risk introduced by fixing it
Both association relationships carry
cascade="all, delete-orphan", and the filters are spelled as assignments back to the mapped collection. While the filters were inert nothing was ever dropped, so no orphans were staged. Once they work, real orphan deletes are staged on every affected request — unpersisted only becauseget_dbnever commits, and in 8 sites only because the narrowing happens after the handler'sdb.commit().That invariant held by accident and had no test. #821 adds
test_narrowing_associations_does_not_delete_themto pin it. #812 and #818 remove the idiom structurally.Why this matters beyond the fix
This represents additional evidence for #808.