Skip to content

Exclude some queries from query suites by lowering their precision. #19507

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

Merged
merged 3 commits into from
May 21, 2025

Conversation

michaelnebel
Copy link
Contributor

@michaelnebel michaelnebel commented May 16, 2025

The linked issue describes why the precisions are lowered (and it also includes a list of specific query IDs).

@michaelnebel michaelnebel force-pushed the removehardcodedpassword branch from e8a4b9f to dabeddb Compare May 19, 2025 07:27
@github-actions github-actions bot added the Java label May 19, 2025
@michaelnebel michaelnebel marked this pull request as ready for review May 19, 2025 09:53
@Copilot Copilot AI review requested due to automatic review settings May 19, 2025 09:53
@michaelnebel michaelnebel requested review from a team as code owners May 19, 2025 09:53
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 lowers the precision of several hardcoded credentials queries across Java, Go, and C#, excludes them from all query suites, and updates the corresponding integration-test expectations and change notes.

  • Lowered precision from medium to low for hardcoded credential queries in Java, Go, and C#.
  • Removed these queries from all query-suite expected listings and added them to not_included_in_qls.expected.
  • Added change notes in Java, Go, and C# to document the removals.

Reviewed Changes

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

Show a summary per file
File Description
javascript/ql/integration-tests/query-suite/*.qls.expected Removed HardcodedCredentials entries from JS query suites
java/ql/src/change-notes/2025-05-16-hardcoded-credentials.md Added note for removed Java hardcoded-credential-api-call query
java/ql/src/Security/CWE/CWE-798/HardcodedCredentialsApiCall.ql Lowered precision from medium to low
java/ql/integration-tests/java/query-suite/*.expected Updated not_included_in_qls.expected, removed from suites
go/ql/src/change-notes/2025-05-16-hardcoded-credentials.md Added note for removed Go hardcoded-credentials query
go/ql/src/Security/CWE-798/HardcodedCredentials.ql Lowered precision from medium to low
go/ql/integration-tests/query-suite/*.expected Updated not_included_in_qls.expected, removed from suites
csharp/ql/src/change-notes/2025-05-16-hardcoded-credentials.md Added note for removed C# credential-related queries
csharp/ql/src/Security Features/CWE-798/*.ql Lowered precision from medium to low
csharp/ql/integration-tests/posix/query-suite/*.expected Updated not_included_in_qls.expected, removed from suites
Comments suppressed due to low confidence (2)

javascript/ql/src/Security/CWE-798/HardcodedCredentials.ql:7

  • The JavaScript HardcodedCredentials query still has @precision medium—it should be lowered to @precision low to match the updates in other languages.
* @precision medium

Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

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

Ruby / C# 👍

@tamasvajk
Copy link
Contributor

Should we freeze the security and quality suite? Is it okay that we're removing queries from it?

@michaelnebel
Copy link
Contributor Author

Should we freeze the security and quality suite? Is it okay that we're removing queries from it?

It is my understanding that these queries are to be removed from all suites that customers use, but I will ask concretely about this suite in the issue.

@jonjanego
Copy link
Member

Should we freeze the security and quality suite? Is it okay that we're removing queries from it?

It is my understanding that these queries are to be removed from all suites that customers use, but I will ask concretely about this suite in the issue.

Responding here - Yes, this is fine, mostly unrelated to the quality product

Copy link
Contributor

@owen-mc owen-mc left a comment

Choose a reason for hiding this comment

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

Go 👍🏻

Copy link
Contributor

@yoff yoff left a comment

Choose a reason for hiding this comment

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

Python 👍

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.

Javascript LGTM 👍

Copy link
Contributor

@redsun82 redsun82 left a comment

Choose a reason for hiding this comment

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

Swift LGTM!

@michaelnebel michaelnebel merged commit 2952c0d into github:main May 21, 2025
75 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.

9 participants