Skip to content

Go: mass-enable diff-informed queries phase 2 - getASelected{Source,Sink}Location() { none() } - #19760

Merged
d10c merged 1 commit into
github:mainfrom
d10c:d10c/go/diff-informed-2
Jun 19, 2025
Merged

Go: mass-enable diff-informed queries phase 2 - getASelected{Source,Sink}Location() { none() }#19760
d10c merged 1 commit into
github:mainfrom
d10c:d10c/go/diff-informed-2

Conversation

@d10c

@d10cd10c commented Jun 13, 2025

Copy link
Copy Markdown
Contributor

Stacks on top of earlier PR: #19659
Uses patch from: https://github.com/github/codeql-patch/pull/88/commits/ec5681e740c18c792443099fb3e413446616a0ee

Adds getASelected{Source,Sink}Location() { none() } override to queries that select a dataflow source or sink as a location, but not both.

@d10c
d10c marked this pull request as ready for review June 17, 2025 08:39
CopilotAI review requested due to automatic review settings June 17, 2025 08:39
@d10c
d10c requested a review from a team as a code ownerJune 17, 2025 08:39
@d10cd10c added the no-change-note-required This PR does not need a change note label Jun 17, 2025

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 mass-enables diff-informed queries in Go CodeQL packs by adding a no-op observeDiffInformedIncrementalMode predicate and default getASelected{Source,Sink}Location() overrides where only one side is selected.

  • Introduces predicate observeDiffInformedIncrementalMode() { any() } in numerous Config modules.
  • Adds getASelectedSourceLocation(...) { none() } and/or getASelectedSinkLocation(...) { none() } overrides in cases where only one of source or sink is selected.
  • Builds on phase 1 to ensure diff-informed incremental mode compatibility.

Reviewed Changes

Copilot reviewed 32 out of 31 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
go/ql/src/experimental/frameworks/DecompressionBombs.qllAdded observeDiffInformedIncrementalMode predicate
go/ql/src/experimental/CWE-74/DsnInjectionCustomizations.qllAdded observeDiffInformedIncrementalMode predicate
go/ql/src/experimental/CWE-369/DivideByZero.qlAdded observeDiffInformedIncrementalMode predicate and getASelectedSourceLocation override
go/ql/src/experimental/CWE-327/WeakCryptoAlgorithmCustomizations.qllAdded observeDiffInformedIncrementalMode predicate
go/ql/src/experimental/CWE-321-V2/HardCodedKeys.qlAdded observeDiffInformedIncrementalMode predicate
go/ql/src/experimental/CWE-287/ImproperLdapAuthCustomizations.qllAdded observeDiffInformedIncrementalMode predicate
go/ql/src/experimental/CWE-285/PamAuthBypass.qlAdded observeDiffInformedIncrementalMode predicate and getASelectedSinkLocation override
go/ql/src/experimental/CWE-203/Timing.qlAdded observeDiffInformedIncrementalMode predicate
go/ql/src/experimental/CWE-090/LDAPInjection.qllAdded observeDiffInformedIncrementalMode predicate
go/ql/src/Security/CWE-640/EmailInjection.qllAdded observeDiffInformedIncrementalMode predicate
go/ql/src/Security/CWE-352/ConstantOauth2State.qlAdded observeDiffInformedIncrementalMode predicate
go/ql/src/Security/CWE-326/InsufficientKeySize.qlAdded observeDiffInformedIncrementalMode predicate and getASelectedSourceLocation override
go/ql/src/Security/CWE-209/StackTraceExposure.qlAdded observeDiffInformedIncrementalMode predicate
go/ql/src/Security/CWE-079/HtmlTemplateEscapingBypassXss.qlAdded observeDiffInformedIncrementalMode predicate
go/ql/src/Security/CWE-020/SuspiciousCharacterInRegexp.qlAdded observeDiffInformedIncrementalMode predicate
go/ql/src/Security/CWE-020/MissingRegexpAnchor.qlAdded observeDiffInformedIncrementalMode predicate and getASelectedSinkLocation override
go/ql/src/Security/CWE-020/IncompleteHostnameRegexp.qlAdded observeDiffInformedIncrementalMode predicate
go/ql/lib/semmle/go/security/ZipSlip.qllAdded observeDiffInformedIncrementalMode predicate
go/ql/lib/semmle/go/security/XPathInjection.qllAdded observeDiffInformedIncrementalMode predicate
go/ql/lib/semmle/go/security/UnsafeUnzipSymlink.qllAdded observeDiffInformedIncrementalMode predicate
go/ql/lib/semmle/go/security/UncontrolledAllocationSize.qllAdded observeDiffInformedIncrementalMode predicate
go/ql/lib/semmle/go/security/TaintedPath.qllAdded observeDiffInformedIncrementalMode predicate
go/ql/lib/semmle/go/security/StringBreak.qllAdded observeDiffInformedIncrementalMode predicate
go/ql/lib/semmle/go/security/StoredXss.qllAdded observeDiffInformedIncrementalMode predicate
go/ql/lib/semmle/go/security/StoredCommand.qllAdded observeDiffInformedIncrementalMode predicate
go/ql/lib/semmle/go/security/SqlInjection.qllAdded observeDiffInformedIncrementalMode predicate
go/ql/lib/semmle/go/security/OpenUrlRedirect.qllAdded observeDiffInformedIncrementalMode predicate
go/ql/lib/semmle/go/security/MissingJwtSignatureCheck.qllAdded observeDiffInformedIncrementalMode predicate
go/ql/lib/semmle/go/security/LogInjection.qllAdded observeDiffInformedIncrementalMode predicate
go/ql/lib/semmle/go/security/ExternalAPIs.qllAdded observeDiffInformedIncrementalMode predicate
go/ql/lib/semmle/go/security/CleartextLogging.qllAdded observeDiffInformedIncrementalMode predicate
Comments suppressed due to low confidence (2)

go/ql/src/experimental/CWE-369/DivideByZero.ql:51

  • No tests currently cover the new getASelectedSourceLocation override; consider adding test cases to validate that diff-informed incremental mode correctly handles source-only location configurations.
Location getASelectedSourceLocation(DataFlow::Node sink) { none() }

go/ql/src/experimental/frameworks/DecompressionBombs.qll:60

  • [nitpick] Add a brief comment above observeDiffInformedIncrementalMode explaining its purpose and how it interacts with diff-informed incremental runs, so future maintainers understand why this no-op override is needed.
predicate observeDiffInformedIncrementalMode() { any() }

)
}

predicate observeDiffInformedIncrementalMode() { any() }

CopilotAIJun 17, 2025

Copy link

Choose a reason for hiding this comment

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

The observeDiffInformedIncrementalMode predicate is duplicated across many configs; consider extracting it into a shared mixin or base module to reduce boilerplate and improve maintainability.

Copilot uses AI. Check for mistakes.
@d10c
d10c requested a review from michaelnebelJune 17, 2025 12:53
@d10c
d10cforce-pushed the d10c/go/diff-informed-2 branch from 1d3f49a to 51826c7CompareJune 17, 2025 15:02

@michaelnebelmichaelnebel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@d10c
d10c merged commit b62a6db into github:mainJun 19, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Gono-change-note-requiredThis PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@d10c@michaelnebel