[SECUR-242] fix(api): scope bulk-asset associate by uploader, not project_id (regression from #9288) - #9495
Conversation
…ject_id Regression from #9288 (WEB-7776, cross-project IDOR scoping): adding project_id=project_id to ProjectBulkAssetEndpoint.post broke project creation — the "enable features" step 404s because the freshly-uploaded cover/feature asset still has project_id=NULL (this endpoint is what sets it). master had no such filter. Scope the lookup by created_by=request.user instead. This still closes the IDOR (#9288) — a caller can only touch assets they uploaded, and @allow_permission already scopes them to the project — and is stricter than the original master code (which had no ownership check), while allowing not-yet-associated assets to be linked. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe bulk asset association endpoint now selects requester-owned assets within the target workspace, including assets already associated with the target project and unassociated assets. ChangesBulk asset association
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Linked to Plane Work Item(s) References This comment was auto-generated by Plane |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/plane/app/views/asset/v2.py`:
- Around line 711-718: Update the asset queryset in the association flow to
retain assets whose project_id is NULL or already matches the requested
project_id, while preserving the existing id, workspace, and created_by
constraints. Apply this boundary before the PROJECT_COVER and ISSUE_DESCRIPTION
branches use the assets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2077629d-a091-49f6-977f-f90a0a42d7c6
📒 Files selected for processing (1)
apps/api/plane/app/views/asset/v2.py
…roject (CodeRabbit) Address CodeRabbit: created_by alone let a user move their own asset from another project into this one via the PROJECT_COVER/ISSUE_DESCRIPTION update branches. Add an unassociated-or-same-project bound (project_id=project_id OR project_id IS NULL) alongside created_by, so freshly-uploaded (NULL) assets still link but cross-project moves are rejected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks @coderabbitai — valid, fixed in f5a5163. Scoping by Now bounded to unassociated or already-in-this-project assets, in addition to the uploader check: assets = FileAsset.objects.filter(
id__in=asset_ids,
workspace__slug=slug,
created_by=request.user,
).filter(Q(project_id=project_id) | Q(project_id__isnull=True))
|
|
Tip For best results, initiate chat on the files or code changes.
🐇✨ ✏️ Learnings added
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
Problem
Release-blocking regression in v1.4.0-rc1. Creating a project from the frontend fails at the "enable features" step:
Root cause
#9288 (WEB-7776, "scope FileAsset queries to prevent cross-project IDOR — Cluster F") added
project_id=project_idto the lookup inProjectBulkAssetEndpoint.post:But this endpoint's job is to associate freshly-uploaded assets with a project/entity. A cover/feature asset uploaded during project creation still has
project_id=NULLat this point — in fact this endpoint is what sets it (assets.update(project_id=project_id)forPROJECT_COVER). Requiringproject_idto already match means the asset is never found → 404.master(v1.3.1) had no such filter, so the flow worked there.Present in both
previewandcanary→ ships in v1.4.0 unless patched.Fix
Scope by
created_by=request.user(ownership) instead ofproject_id=project_id(association state):This still closes the IDOR #9288 targeted — a caller can only touch assets they uploaded, and
@allow_permission([ADMIN, MEMBER, GUEST])already scopes them to the target project — and it is stricter than the original master code (which had no ownership check at all), while allowing a not-yet-associated asset to be linked.Testing
canaryfor v1.4.0-rc2 (the RC is already deployed with the bug).🤖 Generated with Claude Code
Summary by CodeRabbit