Skip to content

BLD: add more static analysis#31661

Open
tacaswell wants to merge 17 commits into
matplotlib:mainfrom
tacaswell:bld/static_analysis
Open

BLD: add more static analysis#31661
tacaswell wants to merge 17 commits into
matplotlib:mainfrom
tacaswell:bld/static_analysis

Conversation

@tacaswell
Copy link
Copy Markdown
Member

PR summary

This adds more static analysis to our CI / prek config.

  • run clang-tidy on c/c++/objectiveC code (and fix or ignore issues found). This found a couple of real issues and will only run on CI (+ a helper script to run it locally). The dependencies are too heavy to try and launder through prek's virtual environments.
  • run zizmor in prek (and hence automatically on CI). Had to explictily allow the 4 places we use pull_request_target and a couple of places we allow caching. Also enabled cooldown windows on our dependabot config. This also flagged we had a redundant checkout step in
  • enable shellcheck in prek to hit the handful of bash scripts we ship and applied the hardening it found.
  • run ruff on our checked in notebooks as well
  • added static validation of svg files to prek (I got in my head that this could be used to get information out of developer's machines which is probably not possible (beyond what you give up by going to any website)), but now it will be harder for someone to sneak an active SVG into our baseline images.

Not reflected in the commits here, but I did look at https://github.com/PyCQA/bandit but every single thing it flagged was a false positive (either flagging things like exec in the plot directive or being too simplistic to notice that we had already mitigated the thing it was flagging in the code).

AI Disclosure

I started from the prompt

Analyze this repository and identify any programming languages, markup languages, document formats, build artifacts, generated files, embedded assets, or other non-core technologies that could be overlooked by traditional SAST or standard code-scanning tools.

Focus especially on:

languages or file types that are not part of the repository's primary tech stack
obscure, uncommon, legacy, or less frequently scanned formats
executable or scriptable content hidden in non-obvious places
document or graphics formats that may contain logic, macros, scripts, links, or embedded content
Include examples such as LaTeX, PostScript, SVG, and similar formats, but do not limit the analysis to those.

For each item you identify, provide:

the language, format, or artifact type
why it may be missed by classic SAST or scanning tools
the security relevance or potential risk
typical file extensions or repository locations where it may appear
whether it is likely source, generated output, embedded content, or supporting artifact

with opus 4.6 and then pushing on addressing the coverage with a mix of opus 4.6 and sonnet 4.6. I reviewed everything as I committed it and took a very heavy editing pass at the initial version of the documentation.

PR checklist

tacaswell added 14 commits May 11, 2026 11:28
These were false positives, but add code to give the analyzer a hint + asserts
in a debugging build rather than NOLINT to be defensive.

Aided by 🤖
When for no-change to code with the qhull wrapper rather than adjusting the
code.

Aided by 🤖
Fixes an issue found by clang-tidy

Aided by 🤖
Added exceptions for things it thinks are questionable but we are OK with
doing.

Aided by 🤖
@github-advanced-security
Copy link
Copy Markdown

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

  • The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
  • Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
  • You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

Comment thread tools/run_clang_tidy.py
@@ -0,0 +1,246 @@
#!/usr/bin/env python3
"""
Run clang-tidy on Matplotlib's C/C++ and Objective-C sources.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure this script is fully necessary; meson will create a clang-tidy target for you (but only if you have .clang-tidy in the root), so you just need to move it, make an editable install, and run ninja -C build/cp313 clang-tidy or similar

Comment thread .pre-commit-config.yaml
hooks:
- id: yamllint
args: ["--strict", "--config-file=.yamllint.yml"]
- repo: https://github.com/nbQA-dev/nbQA
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm confused why this is needed; https://github.com/astral-sh/ruff-pre-commit has a note on how not to run on notebooks, so I thought it's enabled by default.

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