fix: skip external image srcs when duplicating page assets by pablohashescobar · Pull Request #9938 · makeplane/plane

pablohashescobar · 2026-10-04T17:56:03Z

Description

Duplicating a page that contains an external image (e.g. https://media.docs.plane.so/seed_assets/31.png) failed with "... is not a valid UUID". extract_asset_ids returned every src it found, and those were passed to FileAsset.objects.filter(id__in=...), which aborted the whole asset copy.

extract_asset_ids now keeps only src values that are valid UUIDs (using the existing is_valid_uuid helper). Uploaded assets are still copied; external URLs are left as-is in the duplicated page.

Type of Change

Bug fix (non-breaking change which fixes an issue)
Feature (non-breaking change which adds functionality)
Improvement (change that would cause existing functionality to not work as expected)
Code refactoring
Performance improvements
Documentation update

Screenshots and Media (if applicable)

Test Scenarios

Added unit test test_extract_asset_ids_skips_external_urls: a UUID src is returned, an external URL src and a missing src are skipped.
Duplicate a page containing both an uploaded image and an external image URL; verify the duplicate succeeds and the uploaded image is copied.
Duplicate a page with only external images; verify no error.

References

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- External image URLs and elements without a source are no longer treated as assets to copy. Only valid asset IDs are included.

- External image URLs (e.g. https://media.docs.plane.so/...) were passed through to FileAsset.objects.filter(id__in=...), which raised "is not a valid UUID" and aborted the whole asset copy on page duplicate. - Filter extract_asset_ids to UUID srcs only, since uploaded assets are the only ones that carry a UUID. - Add a unit test covering UUID, external URL, and missing-src tags.

coderabbitai · 2026-10-04T17:56:57Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 2ad9c3c2-372b-44ae-83ef-d51f8153a3c1

📥 Commits

Reviewing files that changed from the base of the PR and between c7a5afe and f665b49.

📒 Files selected for processing (2)

apps/api/plane/bgtasks/copy_s3_object.py
apps/api/plane/tests/unit/bg_tasks/test_copy_s3_objects.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

Walkthrough

extract_asset_ids now returns only nonempty image sources that are valid UUIDs. A unit test checks that external URLs and sources that are missing are excluded.

Changes

Asset ID filtering

Layer / File(s)	Summary
UUID source filtering `apps/api/plane/bgtasks/copy_s3_object.py`, `apps/api/plane/tests/unit/bg_tasks/test_copy_s3_objects.py`	`extract_asset_ids` filters sources to nonempty valid UUIDs. The unit test checks a valid UUID, an external URL, and an image without a `src`.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~5 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to f665b

Normal uploaded asset references remain eligible for copying, while external image URLs are excluded. No actionable merge risk is established.

Security Architecture Review

Security architecture risk: 🔵 Low · up to f665b

The change tightens image-ID handling while preserving existing permissions and asset-copy restrictions. No introduced vulnerability was established, but external-image handling during conversion and recovery from interrupted duplication were not fully verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

observed — Storage-copy selection remains restricted to FileAsset rows matching the entity workspace, supplied project, and extracted IDs. External URL strings do not become storage-copy keys. The query does not check source-entity ownership separately; that predicate predates this change, and its intended policy remains unestablished.

Trust Boundaries and Controls

observed — The worker sends preserved description HTML to a configured conversion endpoint, not to an image-source-derived destination. The repository converter validates the request and invokes local document-format conversion. This does not establish direct image-URL fetching, but transitive conversion-helper behavior and deployed service configuration remain incompletely covered.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: skipping external image sources when duplicating page assets.
Description check	✅ Passed	The description covers the bug, the fix, the change type, and test scenarios. The References section has no entry, but no related issue is required, and screenshots are not necessary for this change.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

Copilot

Copilot review overview

🟢 Approval recommended

The focused validation fix correctly addresses the failure and includes appropriate regression coverage.

Review effort: Balanced
Findings: None

What changed in this PR

Filters page asset sources to valid UUIDs, preventing external image URLs from breaking page duplication.

Changes:

Validates extracted asset IDs before database queries.
Adds regression coverage for external and missing image sources.

File	Description
`apps/api/plane/bgtasks/copy_s3_object.py`	Skips non-UUID image sources during asset extraction.
`apps/api/plane/tests/unit/bg_tasks/test_copy_s3_objects.py`	Tests UUID extraction with external and missing sources.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

pablohashescobar requested a review from dheeru0198 as a code owner October 4, 2026 17:56

Copilot AI balanced review requested due to automatic review settings October 4, 2026 17:56

Copilot started reviewing on behalf of pablohashescobar October 4, 2026 17:56 View session

Copilot AI reviewed Oct 4, 2026

View reviewed changes

Navigation Menu

Repository navigation

fix: skip external image srcs when duplicating page assets - #9938

fix: skip external image srcs when duplicating page assets#9938
pablohashescobar wants to merge 1 commit into
previewfrom
fix/page-duplicate-external-assets

pablohashescobar commented Oct 4, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 4, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

Copilot AI left a comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

pablohashescobar commented Oct 4, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of Change

Screenshots and Media (if applicable)

Test Scenarios

References

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Security Architecture Review

❌ Failed checks (1 warning)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pablohashescobar commented Oct 4, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 4, 2026 •

edited

Loading