Skip to content

Comments

JS: Fix FP in js/build-artifact-leak when keys come from an array of constants#20397

Merged
asgerf merged 3 commits intogithub:mainfrom
asgerf:js/build-artifact-leak-fp
Oct 28, 2025
Merged

JS: Fix FP in js/build-artifact-leak when keys come from an array of constants#20397
asgerf merged 3 commits intogithub:mainfrom
asgerf:js/build-artifact-leak-fp

Conversation

@asgerf
Copy link
Contributor

@asgerf asgerf commented Sep 10, 2025

No description provided.

The tests already have the annotations, it just seems to have been disable by accident
@github-actions github-actions bot added the JS label Sep 10, 2025
@asgerf asgerf force-pushed the js/build-artifact-leak-fp branch from 14274c1 to d75fddf Compare September 10, 2025 09:05
@asgerf asgerf force-pushed the js/build-artifact-leak-fp branch from d75fddf to 2a4d683 Compare September 10, 2025 09:07
@asgerf asgerf added the no-change-note-required This PR does not need a change note label Oct 27, 2025
@asgerf asgerf marked this pull request as ready for review October 27, 2025 09:43
@asgerf asgerf requested a review from a team as a code owner October 27, 2025 09:43
Copilot AI review requested due to automatic review settings October 27, 2025 09:43
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 false positive in the js/build-artifact-leak security query where environment variable keys are filtered through an array of safe constants. The fix adds logic to recognize when a reduce operation iterates over an array of constant strings, indicating intentional filtering rather than a security vulnerability.

Key Changes

  • Added test case demonstrating the false positive scenario with array-based key filtering
  • Extended the sanitizer logic to recognize arrays of constant strings as safe sources
  • Updated test expectations to use inline expectations format

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
javascript/ql/test/query-tests/Security/CWE-312/build-leaks.js Added test case getFilteredEnv4() demonstrating safe filtering via constant array
javascript/ql/test/query-tests/Security/CWE-312/BuildArtifactLeak.qlref Enabled inline expectations test postprocessing
javascript/ql/test/query-tests/Security/CWE-312/BuildArtifactLeak.expected Updated expected results with inline expectations format
javascript/ql/lib/semmle/javascript/security/dataflow/CleartextLoggingCustomizations.qll Implemented isArrayOfConstants predicate to recognize constant array sources

💡 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.

LGTM. 👍

@asgerf asgerf merged commit 8d49f26 into github:main Oct 28, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

JS no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants