-
Notifications
You must be signed in to change notification settings - Fork 1.9k
JS: Handle Next.js files named 'page' or 'route' #20916
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
base: main
Are you sure you want to change the base?
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 PR fixes a bug in the Next.js model to properly detect server-side taint sources in files named route.ts or page.tsx/.jsx that appear outside the traditional api and pages folders, specifically within the app directory structure used by Next.js 13+ App Router.
Key Changes
- Refactored folder resolution predicates to support recursive traversal of
appandpagesdirectories - Extended
getAPagesModule()to recognize files namedpageanywhere within the app folder hierarchy - Enhanced
NextAppRouteHandlerto recognize files namedrouteanywhere within the app folder hierarchy
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| javascript/ql/lib/semmle/javascript/frameworks/Next.qll | Refactored folder predicates and extended route/page file detection to support Next.js App Router structure |
| javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/app/blah/route.ts | Added test case for route handler in non-api folder |
| javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/app/blah/page.jsx | Added test case for page component with server-side functions in non-pages folder |
| javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected | Updated expected test results to include new test cases |
| javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected | Updated expected test results to include new test cases |
| javascript/ql/src/change-notes/2025-11-26-nextjs-page-route-files.md | Added change note documenting the bug fix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Napalys
left a comment
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. 👍
| deprecated predicate getAPagesFolder = pagesFolder/0; | ||
|
|
||
| /** | ||
| * Gets a module corrosponding to a `Next.js` page. |
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.
Nit existing typo, outside of this pr scope.
| * Gets a module corrosponding to a `Next.js` page. | |
| * Gets a module corresponding to a `Next.js` page. |
The Next.js model was missing cases for files named
route.tsorpage.tsoutside thepagesorapifolders.