Skip to content

Conversation

d10c
Copy link
Contributor

@d10c d10c commented Jun 13, 2025

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.

@github-actions github-actions bot added the Swift label Jun 13, 2025
@d10c d10c marked this pull request as ready for review June 17, 2025 08:39
@Copilot Copilot AI review requested due to automatic review settings June 17, 2025 08:39
@d10c d10c requested a review from a team as a code owner June 17, 2025 08:39
@d10c d10c requested a review from geoffw0 June 17, 2025 08:39
@d10c d10c added the no-change-note-required This PR does not need a change note label Jun 17, 2025
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 mass-enables diff-informed incremental mode for Swift security queries by adding a new predicate and stubbing out location selection where only one side is used.

  • Added observeDiffInformedIncrementalMode() predicate to all Swift security query config modules.
  • Introduced getASelectedSourceLocation(...) { none() } overrides in queries that only select sources.

Reviewed Changes

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

Show a summary per file
File Description
swift/ql/lib/codeql/swift/security/regex/RegexInjectionQuery.qll Add observeDiffInformedIncrementalMode
swift/ql/lib/codeql/swift/security/XXEQuery.qll Add observeDiffInformedIncrementalMode
swift/ql/lib/codeql/swift/security/WeakSensitiveDataHashingQuery.qll Add observeDiffInformedIncrementalMode
swift/ql/lib/codeql/swift/security/WeakPasswordHashingQuery.qll Add observeDiffInformedIncrementalMode
swift/ql/lib/codeql/swift/security/UnsafeUnpackQuery.qll Add observeDiffInformedIncrementalMode and getASelectedSourceLocation
swift/ql/lib/codeql/swift/security/UnsafeJsEvalQuery.qll Add observeDiffInformedIncrementalMode and getASelectedSourceLocation
swift/ql/lib/codeql/swift/security/UncontrolledFormatStringQuery.qll Add observeDiffInformedIncrementalMode
swift/ql/lib/codeql/swift/security/StringLengthConflationQuery.qll Add observeDiffInformedIncrementalMode and getASelectedSourceLocation
swift/ql/lib/codeql/swift/security/StaticInitializationVectorQuery.qll Add observeDiffInformedIncrementalMode and getASelectedSourceLocation
swift/ql/lib/codeql/swift/security/SqlInjectionQuery.qll Add observeDiffInformedIncrementalMode
swift/ql/lib/codeql/swift/security/PredicateInjectionQuery.qll Add observeDiffInformedIncrementalMode
swift/ql/lib/codeql/swift/security/PathInjectionQuery.qll Add observeDiffInformedIncrementalMode
swift/ql/lib/codeql/swift/security/InsufficientHashIterationsQuery.qll Add observeDiffInformedIncrementalMode and getASelectedSourceLocation
swift/ql/lib/codeql/swift/security/HardcodedEncryptionKeyQuery.qll Add observeDiffInformedIncrementalMode
swift/ql/lib/codeql/swift/security/ECBEncryptionQuery.qll Add observeDiffInformedIncrementalMode
swift/ql/lib/codeql/swift/security/ConstantSaltQuery.qll Add observeDiffInformedIncrementalMode
swift/ql/lib/codeql/swift/security/ConstantPasswordQuery.qll Add observeDiffInformedIncrementalMode and getASelectedSourceLocation
swift/ql/lib/codeql/swift/security/CommandInjectionQuery.qll Add observeDiffInformedIncrementalMode
swift/ql/lib/codeql/swift/security/CleartextTransmissionQuery.qll Add observeDiffInformedIncrementalMode
swift/ql/lib/codeql/swift/security/CleartextLoggingQuery.qll Add observeDiffInformedIncrementalMode
Comments suppressed due to low confidence (3)

swift/ql/lib/codeql/swift/security/RegexInjectionQuery.qll:26

  • The same observeDiffInformedIncrementalMode stub is duplicated across multiple modules; consider extracting this into a shared trait or base config to reduce repetition and simplify future updates.
predicate observeDiffInformedIncrementalMode() { any() }

swift/ql/lib/codeql/swift/security/regex/RegexInjectionQuery.qll:26

  • Add dedicated tests for observeDiffInformedIncrementalMode() to verify that diff-informed incremental flows are correctly activated and that existing queries still produce expected results.
predicate observeDiffInformedIncrementalMode() { any() }

swift/ql/lib/codeql/swift/security/UnsafeUnpackQuery.qll:30

  • [nitpick] You've overridden getASelectedSourceLocation but not getASelectedSinkLocation. If this query selects only sources or only sinks, ensure the complementary override is present to avoid unintended default location selection.
Location getASelectedSourceLocation(DataFlow::Node sink) { none() }

geoffw0
geoffw0 previously approved these changes Jun 17, 2025
Copy link
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

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

I'm a bit confused as to why we didn't merge https://github.com/github/codeql/pull/19662/files first, but LGTM.

@d10c
Copy link
Contributor Author

d10c commented Jun 17, 2025

Thanks! I was leaving the first phase of PRs open in case reviews on the other languages give us something else to change, but actually they are ready to merge (the Go, C++, and Actions PRs are still waiting for a reviewer though).

@d10c d10c dismissed geoffw0’s stale review June 17, 2025 12:53

The merge-base changed after approval.

@d10c d10c force-pushed the d10c/swift/diff-informed-2 branch from 7e4c0c4 to 67bccc3 Compare June 17, 2025 15:03
@d10c d10c merged commit 687e8d2 into github:main Jun 19, 2025
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-change-note-required This PR does not need a change note Swift
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants