Skip to content

Verify fee schedule file with PipeRoot.isUnderRoot - #1031

Merged
labkey-matthewb merged 2 commits into
developfrom
fb_docker_tar
Oct 29, 2026
Merged

labkey-matthewb merged 2 commits into
developfrom
fb_docker_tar

Conversation

@labkey-matthewb

Copy link
Copy Markdown
Contributor

Rationale

Keep fee schedule import working with LabKey/platform#8227, which no longer accepts a multi-segment path string passed as a single Path part.

Related Pull Requests

Changes

  • Check the selected file directly against the pipeline root, including supplemental roots, instead of re-resolving it from a relative URI.
  • Return a not-found error rather than a server error for a file outside the pipeline root.

new Path(relativeURI.getPath()) treats a multi-segment path as one name, which platform fb_docker_tar now rejects.

@labkey-bpatel labkey-bpatel left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude code finding maybe worth a look:

Finding #1 - [Severity: Low] — Error handling — snprc_ehr/src/org/labkey/snprc_ehr/controllers/FeeScheduleController.java:160

Issue: new File(filePath).toPath() can throw InvalidPathException for characters the OS doesn’t allow, such as NUL, or < and | on Windows. In that case, the request bypasses the new NotFoundException handling and returns a 500.

Why it matters: This PR is intended to return a not-found error for invalid paths, but some invalid paths can still result in a 500. The impact is low since this is admin-only and filePath comes from a hidden form field.

Suggestion: Use FileUtil.resolveFile(new File(filePath)), which handles . and .. without converting to a NIO path. isUnderRoot already uses this internally. Another option is to catch InvalidPathException and throw NotFoundException.

@labkey-matthewb
labkey-matthewb merged commit 167c91c into develop Oct 29, 2026
5 of 6 checks passed
@labkey-matthewb
labkey-matthewb deleted the fb_docker_tar branch October 29, 2026 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants