Skip to content

ci(codeql): skip py/log-injection on full Python scans - #44001

Closed
pylaterreur wants to merge 1 commit into
BerriAI:mainfrom
pylaterreur:fix-codeql-full-scan-log-injection
Closed

pylaterreur wants to merge 1 commit into
BerriAI:mainfrom
pylaterreur:fix-codeql-full-scan-log-injection

Conversation

@pylaterreur

@pylaterreur pylaterreur commented Oct 1, 2026 •

Copy link
Copy Markdown

TLDR

Problem this solves:

  • Analyze (python) fails on every CodeQL run for main
  • py/log-injection passes CodeQL's 2 GiB result-set limit there
  • That one failure throws away the other 167 queries' results

How it solves it:

No full Python scan has finished since March 11. The nightly runs hit the job's 30 minute timeout until July 8, and since July 9 the push and nightly runs fail on this one query, in all 25 runs I sampled up to October 1. Pull request scans are diff-informed, so the same query finishes there in about a minute and a half and keeps reporting log-injection findings on changed lines

User Flow

Before: every merge to main gets a red CodeQL check, and Python code scanning never gets results from main

  1. A pull request merges into main and CodeQL runs Analyze (python) on the push
  2. About 20 minutes later it fails with Query failed: codeql/python-queries/Security/CWE-117/LogInjection.ql (Result set is larger than the limit of 2GiB)
  3. Code scanning gets a failed-analysis record for Python instead of results

After: the push scan finishes and main's Python results reach code scanning

  1. A pull request merges into main and CodeQL runs Analyze (python) on the push
  2. It runs the other 167 Python queries and the check passes
  3. Code scanning gets main's Python results, and pull requests still get log-injection findings

Relevant issues

#41142 kept py/log-injection on purpose for the findings it reports on pull requests. This keeps those and only drops the query where it can't finish

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have added meaningful tests
  • The handful of test files covering my change pass locally, e.g. uv run pytest tests/unit/<your_test_file>.py -v. Leave the suites (make test-unit-*, make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more
  • My PR passes all required CI/CD checks (e.g., lint, schema.d.ts sync check, etc.)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

tests/unit/test_codeql_workflow.py locks the change in. One test checks that the step runs only on full Python scans. The other runs the step's own yq command on a copy of codeql-config.yml, and checks that it appends the py/log-injection exclusion and changes nothing else. Each fails if the step also runs on pull requests, excludes another query, or is removed. The misc shard runs it on runners that ship yq. check_workflow_startup_safety.py, check_workflow_job_name_collisions.py and assert_workflow_dir_hygiene.py pass, and actionlint 1.7.12 and zizmor 1.24.1 with online audits report nothing new

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

Screenshots / Proof of Fix

CodeQL only runs full scans on pushes to main and on its nightly schedule, so this PR's own checks can't run one. Before uses main's runs, and After shows the config the full scan will read and the evidence that the other 167 queries already finish

Before (aa601ce)

Push scan

  1. CodeQL on the aa601ce push, Analyze (python), job 110629596307
  2. The log ends the query phase with [168/168 eval 13m36s] Query failed: codeql/python-queries/Security/CWE-117/LogInjection.ql (Result set is larger than the limit of 2GiB) and 1 of 168 queries failed to evaluate
  3. Error running analysis for python with Exit code was 34, and the job fails after 19 minutes

Pull request scan

  1. CodeQL on test(ci): repair stale tests and flaky CI infrastructure #43983, Analyze (python), job 110236745113
  2. The log shows Calculating diff ranges for main...BerriAI:litellm_ci_stale_tests_1001, then [125/168 eval 1m24s] Evaluation done; writing results to codeql/python-queries/Security/CWE-117/LogInjection.bqrs
  3. The job passes in 8 minutes

After (8b47fd3)

Push scan

  1. Run the new step's yq -i '.query-filters += [{"exclude": {"id": "py/log-injection"}}]' .github/codeql/codeql-config.yml with yq 4.53.6, the version on the ubuntu-24.04 runner image these jobs use
  2. The config now ends query-filters with exclude: id: py/log-injection, and every other key and filter is unchanged
  3. The six queries the config already excludes never load in the Before log, so this filter leaves the 167 queries that finished there

Pull request scan

  1. The step only runs when matrix.language == 'python' && github.event_name != 'pull_request'
  2. So pull request scans, this PR's own CodeQL check included, read the config unchanged and still run LogInjection.ql
  3. This PR's Analyze (python), job 110789587352, shows Calculating diff ranges for main...pylaterreur:fix-codeql-full-scan-log-injection, then [126/168 eval 1m34s] Evaluation done; writing results to codeql/python-queries/Security/CWE-117/LogInjection.bqrs, and passes in 8 minutes

Type

Infrastructure

Caveats (if any)

Medium

  • Push and nightly scans no longer run py/log-injection
    • They never finished it, so no findings are lost
    • Pull requests still report it on changed lines
  • The first finished scan uploads all of main's current Python findings
    • Nothing has refreshed them since March 11, so expect a large batch

Low

  • Only the first push after merge proves the full scan finishes
  • Relies on the yq that ships on GitHub's Ubuntu runner images
  • The step edits the checked-out codeql-config.yml in place
    • Only CodeQL reads that file afterwards

Final Attestation

  • The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

@pylaterreur
pylaterreur requested a review from a team October 1, 2026 09:45
@greptile-apps

greptile-apps Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

[Medium risk] Modifies security scanning configuration for Python code.

The change appears functionally sound, but the repository’s regression-test requirement should be met before merging

Findings

  1. P2 Missing regression test ▶

Summary

The workflow now adds a CodeQL query exclusion before initialization on push and scheduled Python scans, while leaving pull request scans unchanged. The new behavior lacks a functional regression check

Reviews (1) · Last reviewed commit: "ci(codeql): skip py/log-injection on ful..."

Comment thread .github/workflows/codeql.yml
@codecov

codecov Bot commented Oct 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@pylaterreur
pylaterreur force-pushed the fix-codeql-full-scan-log-injection branch from 05fd771 to 77b92d0 Compare October 1, 2026 23:21
Analyze (python) fails on every CodeQL run for main that is not
cancelled. The py/log-injection query hits CodeQL's 2 GiB result-set
limit after 10 to 15 minutes, and that one failure throws away the
results of the other 167 Python queries. No full Python scan has
finished since March 11: nightly runs hit the 30 minute job timeout
until July 8 and have failed on this query since July 9

Pull request scans are diff-informed, so the same query finishes there
in about a minute and a half, and BerriAI#41142 kept py/log-injection for
those findings. The new step drops it from push and schedule runs
only, before CodeQL reads the config. codeql-config.yml already
excludes two other queries from every run for passing the same limit

tests/unit/test_codeql_workflow.py checks that the step runs only on full
Python scans, then runs its yq command on a copy of codeql-config.yml
and checks that it adds that one exclusion and changes nothing else
@pylaterreur

Copy link
Copy Markdown
Author

This got closed automatically when #44426 was merged: its description says "separate fix #44001 remains open", and GitHub reads "fix #44001" as a closing keyword. The problem is still there: the latest Python CodeQL run on main still fails on py/log-injection ("Result set is larger than the limit of 2GiB", https://github.com/BerriAI/litellm/actions/runs/37180796266). I can't reopen it myself; @moe-berri, could you reopen it, please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant