Skip to content

JS: moved execa out of experimental #19858

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

Napalys
Copy link
Contributor

@Napalys Napalys commented Jun 24, 2025

No description provided.

@Napalys Napalys marked this pull request as ready for review June 24, 2025 08:18
@Copilot Copilot AI review requested due to automatic review settings June 24, 2025 08:18
@Napalys Napalys requested a review from a team as a code owner June 24, 2025 08:18
Copy link
Contributor

@Copilot 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 promotes the execa library model from experimental to stable, migrating its tests into the main query-tests directories and updating the QL framework import.

  • Added execa.js under Security/CWE-078 and Security/CWE-022 with appropriate $Source/$Alert tags
  • Updated expected result files to include execa.js entries for both command and path injection
  • Removed experimental Execa tests and updated javascript.qll to import the stable Execa framework; added a change note

Reviewed Changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
javascript/ql/test/query-tests/Security/CWE-078/CommandInjection/execa.js New tests for command injection with various execa calls
javascript/ql/test/query-tests/Security/CWE-078/CommandInjection/CommandInjection.expected Updated expected alerts for execa.js entries
javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/execa.js New tests for path injection via execa input/output options
javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected Updated expected alerts for path injection tests
javascript/ql/lib/javascript.qll Imported semmle.javascript.frameworks.Execa for stable model
javascript/ql/lib/change-notes/2025-06-20-execa.md Added change note for Execa promotion
javascript/ql/test/experimental/Execa/** Removed obsolete experimental Execa tests

Copy link
Contributor

@asgerf asgerf left a comment

Choose a reason for hiding this comment

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

We already have a model of execa in SystemCommandExecutors.qll. I'd rather have only one model in one place. Could you make sure the new model covers all the same cases as the old one and then remove the old one?

@Napalys Napalys requested a review from asgerf June 24, 2025 11:49
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