-
Notifications
You must be signed in to change notification settings - Fork 104
feat: prevent export of results from non-downloadable datasets #3772
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request implements a feature to prevent export of workflow results that depend on non-downloadable datasets the user doesn't own, ensuring dataset access controls cannot be bypassed through workflow execution and result export.
- Adds server-side validation to analyze workflow dependencies and prevent export of operators dependent on restricted datasets
- Implements frontend UI changes to show restriction warnings and filter exportable operators
- Uses dataset ownership and downloadable flags to determine export eligibility with propagation to downstream operators
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| workflow-result-export.service.ts | Adds dataset metadata tracking, restriction analysis, and export filtering logic |
| result-exportation.component.ts | Updates export dialog to handle restricted operators and display warnings |
| result-exportation.component.html | Adds UI alerts for partial and complete export restrictions |
| WorkflowExecutionsResource.scala | Implements server-side validation to block export of restricted operators |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
core/gui/src/app/workspace/service/workflow-result-export/workflow-result-export.service.ts
Outdated
Show resolved
Hide resolved
core/gui/src/app/workspace/service/workflow-result-export/workflow-result-export.service.ts
Outdated
Show resolved
Hide resolved
...ala/edu/uci/ics/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResource.scala
Outdated
Show resolved
Hide resolved
...ala/edu/uci/ics/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResource.scala
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@seongjinyoon Can you add comments on top of each class and function so that reviewers can easily understand the code? Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a few comments. Some of them may be wrong. Please check.
...ala/edu/uci/ics/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResource.scala
Outdated
Show resolved
Hide resolved
...ala/edu/uci/ics/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResource.scala
Outdated
Show resolved
Hide resolved
...ala/edu/uci/ics/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResource.scala
Outdated
Show resolved
Hide resolved
...ala/edu/uci/ics/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResource.scala
Outdated
Show resolved
Hide resolved
...ala/edu/uci/ics/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResource.scala
Outdated
Show resolved
Hide resolved
...ala/edu/uci/ics/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResource.scala
Show resolved
Hide resolved
...ala/edu/uci/ics/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResource.scala
Outdated
Show resolved
Hide resolved
...ala/edu/uci/ics/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResource.scala
Outdated
Show resolved
Hide resolved
core/gui/src/app/workspace/component/result-exportation/result-exportation.component.html
Outdated
Show resolved
Hide resolved
core/gui/src/app/workspace/component/result-exportation/result-exportation.component.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a few more comments! @bobbai00 Please review the PR after @seongjinyoon resolves the comments.
core/workflow-core/src/main/scala/edu/uci/ics/amber/core/storage/FileResolver.scala
Show resolved
Hide resolved
...ala/edu/uci/ics/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResource.scala
Outdated
Show resolved
Hide resolved
|
I have addressed all comments. @bobbai00 please take a look. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments on the frontend code.
core/gui/src/app/workspace/service/workflow-result-export/workflow-result-export.service.ts
Outdated
Show resolved
Hide resolved
core/gui/src/app/workspace/service/workflow-result-export/workflow-result-export.service.ts
Outdated
Show resolved
Hide resolved
core/gui/src/app/workspace/service/workflow-result-export/workflow-result-export.service.ts
Show resolved
Hide resolved
core/gui/src/app/workspace/service/workflow-result-export/workflow-result-export.service.ts
Outdated
Show resolved
Hide resolved
|
@bobbai00 Please review the code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments
core/gui/src/app/workspace/service/workflow-result-export/workflow-result-export.service.ts
Show resolved
Hide resolved
core/gui/src/app/workspace/service/workflow-result-export/workflow-result-export.service.ts
Outdated
Show resolved
Hide resolved
core/gui/src/app/workspace/service/workflow-result-export/workflow-result-export.service.ts
Outdated
Show resolved
Hide resolved
|
@bobbai00 Please review the code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its in good shape now. Left few minor comments. Please let me know once you resolved them, and I will take another look
core/gui/src/app/workspace/service/workflow-result-export/workflow-result-export.service.ts
Outdated
Show resolved
Hide resolved
core/gui/src/app/workspace/component/result-exportation/result-exportation.component.ts
Outdated
Show resolved
Hide resolved
core/gui/src/app/workspace/component/result-exportation/result-exportation.component.ts
Outdated
Show resolved
Hide resolved
core/gui/src/app/workspace/service/workflow-result-export/workflow-result-export.service.ts
Outdated
Show resolved
Hide resolved
|
I removed myself from the reviewer list. We rely on other members to do the reviews. Feel free to merge the PR after all reviewers approved it. |
Signed-off-by: Seongjin Yoon <75426413+seongjinyoon@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Description:
Implemented restriction on
export resultto prevent users from exporting workflow results that depend on non-downloadable datasets they don't own. This ensures dataset download cannot be circumvented through workflow execution and result export.Closes #3766
Changes:
Backend
Frontend
information
Video:
The video demonstrates how
export resultbehaves on:Screen.Recording.2025-09-26.at.12.27.38.AM.mov