Skip to content

Ruby: Exclude vendored library parameters from taint sources. - #22282

Open
aschackmull wants to merge 2 commits into
github:mainfrom
aschackmull:ruby/vendored-lib-exclude
Open

Ruby: Exclude vendored library parameters from taint sources.#22282
aschackmull wants to merge 2 commits into
github:mainfrom
aschackmull:ruby/vendored-lib-exclude

Conversation

@aschackmull

Copy link
Copy Markdown
Contributor

5 queries (PolynomialReDoS.ql, MissingFullAnchor.ql, UnsafeCodeConstruction.ql, UnsafeHtmlConstruction.ql, and UnsafeShellCommandConstruction.ql) use Gem::getALibraryInput() as a taint source, but when the gem is vendored this leads to FPs, since then those parameters aren't external input, but instead just ordinary call targets from a fixed set of call sites.

Copilot AI balanced review requested due to automatic review settings August 5, 2026 08:50
@aschackmull
aschackmull requested a review from a team as a code owner August 5, 2026 08:50
@github-actions github-actions Bot added the Ruby label Aug 5, 2026

Copilot AI 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.

🟢 Ready to approve

The focused change matches the described false-positive scenario without introducing review findings.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Excludes vendored Ruby gems from library-input taint source modeling, reducing false positives across five security queries.

Changes:

  • Detects gemspecs beneath a vendor directory.
  • Excludes their parameters from getALibraryInput().
File summaries
File Description
ruby/ql/lib/codeql/ruby/frameworks/core/Gem.qll Filters vendored gems from external library inputs.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

@hvitved hvitved 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.

Looks plausible to me.

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.

3 participants