Skip to content

Conversation

@asgerf
Copy link
Contributor

@asgerf asgerf commented Nov 25, 2025

In a Next.js app, a folder named pages has significant meaning, and it seems this folder can appear in different places. Some parts of the model did not support the location app/pages (and other parts did). This PR fixes that and ensures all parts of the model share the logic for how to locate such folders.

Some parts of the code did not handle case where 'pages' was located at 'app/pages'.
@asgerf asgerf marked this pull request as ready for review November 25, 2025 15:11
@asgerf asgerf requested a review from a team as a code owner November 25, 2025 15:11
Copilot AI review requested due to automatic review settings November 25, 2025 15:11
Copilot finished reviewing on behalf of asgerf November 25, 2025 15:14
Copy link
Contributor

Copilot AI left a 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 where server-side taint sources in the app/pages folder were not being detected. The fix centralizes the logic for locating Next.js special folders (pages, api, etc.) and ensures all folder structure variations are supported, including src/ prefixes and the app/pages configuration.

Key Changes:

  • Added helper predicates (packageRoot(), srcRoot(), appRoot(), pagesRoot(), apiRoot()) to systematically locate Next.js folders across different project layouts
  • Updated getAPagesFolder() and apiFolder() to use the new centralized logic instead of hardcoded paths
  • Added test case demonstrating the fix for app/pages/ folder detection

Reviewed changes

Copilot reviewed 5 out of 5 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 detection logic with new helper predicates to support all Next.js project layout variations
javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/app/pages/Next2.jsx Added test case to verify XSS detection in app/pages/ folder structure
javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected Updated test expectations with new alerts from app/pages/Next2.jsx
javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected Updated test expectations with new alerts from app/pages/Next2.jsx
javascript/ql/src/change-notes/2025-11-25-nextjs-project-layout.md Documented the bug fix for app/pages folder detection

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@Napalys Napalys left a comment

Choose a reason for hiding this comment

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

:shipit:

@asgerf asgerf merged commit 0245b9d into github:main Nov 26, 2025
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants