fix: apply object access checks in tag bulk_create and update commands - #41457
Conversation
The single-object tag command checks that the caller may modify the target object before creating a tag relationship. Apply the same ownership check in the bulk_create and update commands through a shared current_user_can_modify_object helper, so all three tag-relationship paths enforce one consistent boundary. Add regression coverage for the bulk_create and update paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: sha174n <shaitan@zx48.net>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #41457 +/- ##
==========================================
+ Coverage 63.95% 64.50% +0.55%
==========================================
Files 2664 2664
Lines 146153 146117 -36
Branches 33713 33700 -13
==========================================
+ Hits 93474 94259 +785
+ Misses 50964 50140 -824
- Partials 1715 1718 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Code Review Agent Run #1a1e6fActionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
rusackas
left a comment
There was a problem hiding this comment.
Thanks @sha174n, this LGTM. Factoring the ownership check into current_user_can_modify_object and looking objects up with skip_base_filter=True closes the gap in the bulk_create and update paths cleanly, and lines up with the single-object path from #40333.
One thing worth pinning before merge: the new test only checks the resulting row counts, so it'd still pass if Gamma ever lost tag-write and the requests 403'd. An assert on the response status would keep it honest about what it's actually exercising.
Also worth grabbing the bot's note on update.py line 79 while you're in there... that to_object_model lookup isn't wrapped the way the create.py one is, so the two paths handle a DAO error differently.
SUMMARY
Tag relationships can be created through three command paths: the single-object
CreateCustomTagCommand, the bulkCreateCustomTagWithRelationshipsCommand, andUpdateTagCommand. The single-object path already checks that the caller may modify the target object before creating the relationship (added in #40333).This PR factors that check into a shared
current_user_can_modify_objecthelper incommands/tag/utils.pyand applies it across all three paths so they enforce one consistent ownership boundary. Objects are looked up withskip_base_filter=Trueso the explicit ownership check decides which objects are tagged.TESTING INSTRUCTIONS
pytest tests/integration_tests/tags/api_tests.pyAdds
test_bulk_create_and_update_skip_inaccessible_objects: an object owned by another user is skipped by a non-owner'sbulk_createandupdatecalls.ADDITIONAL INFORMATION