Skip to content

Comments

fix: prevent path traversal for API endpoint URL#1950

Merged
dcshzj merged 2 commits intodevelopfrom
fix/endpoint-path-traversal
Dec 3, 2025
Merged

fix: prevent path traversal for API endpoint URL#1950
dcshzj merged 2 commits intodevelopfrom
fix/endpoint-path-traversal

Conversation

@dcshzj
Copy link
Contributor

@dcshzj dcshzj commented Nov 12, 2025

Problem

Closes ISOM-2132.

Solution

Breaking Changes

  • Yes - this PR contains breaking changes
  • No - this PR is backwards compatible with ALL of the following feature flags in this doc

Bug Fixes:

  • Adds a check on the input parameters to prevent any potential path traversals before crafting the final API endpoint.

Before & After Screenshots

BEFORE:

image

AFTER:

image

Tests

  • Unit tests (using npm run tests)
  • e2e tests (comment on this PR with the text !run e2e)
  • Smoke tests
    • Log in to the CMS, then try this URL: https://staging-cms.isomer.gov.sg/sites/isomer-vapt2025one/folders/example-folder/subfolders/example-subfolder/editPageSettings/x%5C..%5C..%5C..%5C..%5C..%5C..%5C..%5C..%5Cfoo
    • Verify in the Console tab that there is an error message that starts with "Error: Unsafe path detected in parameter"
    • Verify in the Network tab that there is no requests with a 403 response to a URL that ends with foo.

@linear
Copy link

linear bot commented Nov 12, 2025

@dcshzj dcshzj force-pushed the fix/endpoint-path-traversal branch from 8a7e39f to aa5bc10 Compare November 12, 2025 09:06
Copy link
Contributor

@seaerchin seaerchin left a comment

Choose a reason for hiding this comment

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

shuold we use starter-kitty?

@dcshzj
Copy link
Contributor Author

dcshzj commented Dec 3, 2025

shuold we use starter-kitty?

I tried previously but got some issues I think was the ESM/CJS thingy, but that was for the backend so probably frontend is quite ok.

But for this I think it's slightly different cos starter-kitty assumes backend but this is running on the frontend. It has a dependency on fs which is only available in nodejs.

@dcshzj dcshzj requested a review from seaerchin December 3, 2025 06:48
@dcshzj dcshzj merged commit b119f9d into develop Dec 3, 2025
7 of 8 checks passed
Copilot AI mentioned this pull request Dec 4, 2025
5 tasks
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