Repository navigation
fix: check dataset read permission in publisheddata formpopulate - #2887
Merged
Merged
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The new use of
checkPermissionsForDatasetExtendedchanges the behavior for unauthorized access; consider explicitly handling and standardizing the error/response (e.g., 403 vs empty form data) so clients get a clear, consistent signal. - The dataset lookup and form population logic in both v4 and non-v4 controllers are nearly identical; consider extracting a shared helper to avoid duplication and keep permission checks consistent across versions.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new use of `checkPermissionsForDatasetExtended` changes the behavior for unauthorized access; consider explicitly handling and standardizing the error/response (e.g., 403 vs empty form data) so clients get a clear, consistent signal.
- The dataset lookup and form population logic in both v4 and non-v4 controllers are nearly identical; consider extracting a shared helper to avoid duplication and keep permission checks consistent across versions.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Junjiequan
approved these changes
Aug 18, 2026
minottic
pushed a commit
that referenced
this pull request
Aug 19, 2026
* fix: check dataset read permission in publisheddata formpopulate * fix: add missing imports, fix formatting and test * fix: export DatasetsController from DatasetsModule * fix: e2e test --------- Co-authored-by: Omkar Zade <[email protected]>
omkar-ethz
reviewed
Aug 20, 2026
| where: { pid: pid[0] }, | ||
| })) as unknown as DatasetClass; | ||
| const dataset = | ||
| (await this.datasetsController.checkPermissionsForDatasetExtended( |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The publisheddata formpopulate endpoint looked up the dataset without a read check, leaking dataset and proposal information for datasets the user cannot read. This checks the dataset read permission first.
Summary by Sourcery
Protect published-data form population by checking dataset read access before retrieving related information.
Bug Fixes:
Tests: