ast-grep rules: a 469-rule security pack and its honest limits

Scanning the nginx echo module takes 0.83 seconds and produces 31 findings across 4,661 lines of C. Twenty-three of those 31 come from one rule, nginx-unchecked-palloc, and that rule cannot tell you whether a single one of them is a bug. It was built that way on purpose, and it’s the most useful thing in the pack.

We run a curated set of ast-grep rules across every C, Go, PHP, Python and Lua tree we build. 469 rule files, 938 fixtures, 18 languages. It lives at labs/ast-grep-essentials and it is the least glamorous piece of tooling we own.

What does ast-grep actually match?

ast-grep parses source with tree-sitter and matches patterns against the syntax tree instead of the bytes. You write $L || $R and it matches a binary expression, not a string containing a pipe character. Comments don’t match. String literals don’t match. A reformatted line that grep would miss because someone broke it across two lines still matches, because the tree doesn’t care about your newlines.

That last point is the whole pitch. Here’s a real rule from the pack:

rule:
  kind: binary_expression
  all:
    - pattern: $L || $R
    - any:
        - has: {field: right, pattern: $A && $B}
        - has: {field: left, pattern: $A && $B}

That’s c-and-or-mixed-without-parens. C binds && tighter than ||, so a || b && c means a || (b && c), which is frequently not the grouping the author read off the page. A guard chain meant to reject unless every clause holds instead accepts on the first disjunct alone. Try writing that check in grep. You can’t, because the thing that makes it a finding is a tree relationship: an && node sitting as a direct child of a || node. Wrap either operand in parentheses and it becomes a parenthesized_expression, no longer a direct child, and the finding clears without changing a single instruction the compiler emits.

Which is also the rule’s honest limitation, and it says so in its own note field: it cannot know which grouping you meant. The finding is not “your precedence is wrong”. The finding is “your code doesn’t say which precedence you wanted, and the next person to read it will guess”.

Why are 370 of 469 rules only warnings?

The severity split is 85 error, 370 warning, 13 info, one off. If you’ve used a linter that ships everything at error and then asks you to suppress the noise, that ratio probably looks cowardly. It’s the honest answer to a question most static analysis tools dodge: does a match prove a bug?

Usually not. Take c-memcmp-on-secret:

rule:
  kind: call_expression
  all:
    - has:
        field: function
        regex: '^(memcmp|strcmp|strncmp|ngx_memcmp|ngx_strcmp|ngx_strncmp)$'
    - has:
        field: arguments
        has:
          stopBy: end
          kind: identifier
          regex: '(?i)(secret|token|key|hmac|mac|password|passwd|digest|signature|sig|hash|nonce)'

memcmp stops at the first differing byte. The time it takes leaks how long a common prefix the attacker supplied, which is enough to recover a forged authenticator one byte at a time. Real bug class, real CVEs, thoroughly documented.

And the detection is an identifier-name regex. That’s it. It sees hash in a hash-table lookup and reports it. It reports a config string compared against a variable somebody called key. It misses key material held in a buffer named buf entirely. Every rule in the pack carries a note: field stating this in plain language, because the alternative is a reviewer rediscovering the limitation from scratch after twenty minutes on a hash-table lookup.

That note is what makes the finding actionable. “Variable-time comparison on possible key material, gate is a name regex, dismiss if the compared value isn’t an authenticator” is a thing a junior triages in ten seconds. A bare SEVERITY: HIGH with no stated gate is a thing they escalate to you at 3 a.m., and they’re right to.

The 23 findings that are deliberately not findings

Back to nginx-unchecked-palloc, the rule that fired 23 times on one small module. Here it is, entire:

rule:
  kind: call_expression
  has:
    field: function
    regex: ^(ngx_palloc|ngx_pnalloc|ngx_pcalloc)$

It matches every pool allocation. All of them. Severity info, and its note reads: “Candidate only — this flags every pool-alloc call, it does NOT prove the guard is missing.”

We tried to write the real rule first. ngx_palloc returns NULL under memory pressure, and dereferencing without if (p == NULL) return NGX_ERROR; is a crash you’ll meet on the worst possible day. But the guard shows up as a declaration-with-initializer, or as an assignment followed by an if, or a goto, or a check three statements later after two unrelated assignments, or an early return with NGX_HTTP_INTERNAL_SERVER_ERROR. Each variant we encoded made the rule wrong in the other direction. We shipped the enumerator instead.

Twenty-three candidates in a 4,661-line module is a ten-minute review, and it’s a review that actually happens, because nobody is arguing with the tool about whether it’s right. Compare that to the version of this rule that tries to be clever: it misses the goto form, so you trust it, so the one real NULL deref in the tree never gets looked at.

There’s a position in here that people do disagree with: a static analysis rule that can’t prove a bug should still ship, provided it says so out loud. The common practice is the opposite. Suppress anything under some confidence threshold, keep the signal-to-noise ratio pretty for the dashboard, and quietly ship the NULL deref. Give me 23 honest candidates over one confident rule that skips the case where the guard sat three statements down.

How the deprecated-alias invariant broke

A rule ID is a contract. Once it’s in someone’s CI config, renaming it breaks their build, so renamed rules keep a deprecated alias pointing at the replacement.

Those aliases only work while the alias’s matcher stays identical to its target’s. We maintained that by hand. It broke exactly the way hand-maintained invariants break: enriching c-string-sizeof-includes-nul to also catch the pointer form left nginx-string-sizeof-includes-nul sitting on the old single-pattern matcher. The alias still existed. It still ran. It just stopped catching half of what its target caught, and nothing anywhere said so.

The fix (commit 02b2f82, 23 September) removes the remembering. An alias declares metadata.deprecated_alias_of, and ci/sync_deprecated_aliases.py rebuilds its matcher from the target, preserving only the fields an alias legitimately owns. --check fails on drift.

The part worth stealing: the check runs from the trusted base checkout, not from the PR’s tree. A pull request that edits the sync script cannot disable the gate validating that same pull request. Five-minute change in the workflow file. It’s the difference between a gate and a suggestion, and most repositories I’ve read have the suggestion.

Running the scan without stepping on the config traps

npm ci
npx ast-grep scan -c sgconfig.yml /path/to/source

The engine is pinned at @ast-grep/cli 0.45.3. Pin yours too. Rule semantics drift when tree-sitter grammars change under you, and an unpinned engine means a CI verdict that flips because somebody else shipped a release on a Tuesday.

Two config details that cost us time. sgconfig.yml lists every native language directory explicitly rather than pointing at the parent rules/, because rules/powershell/ needs a custom parser configured separately in sgconfig.powershell.yml, and importing the parent tries to load those rules with a parser that isn’t there. PHP needs the languageGlobs mapping copied across too, or .phtml files go unparsed:

languageGlobs:
  php:
    - '*.php'
    - '*.phtml'

Unparsed and silent. The scan comes back clean on a directory where half the templates were never read, which is worse than an error, because a clean scan is something you act on. This release has no built-in Perl parser either, so the pack doesn’t pretend to cover Perl.

The 31 nginx-internals rules

Of the 469, 31 are nginx-internals rules, and those are the ones we wrote rather than curated. They encode conventions no general C linter knows about. nginx-str-data-passed-to-libc: an ngx_str_t .data pointer isn’t NUL-terminated, so handing it to strlen reads off the end of your buffer into whatever the pool allocated next. nginx-table-missing-sentinel. nginx-use-after-finalize. nginx-conf-return-code-confusion, which catches the config handler returning NGX_ERROR where the caller expects a char *.

nginx-finalize-plus-return-rc is my favourite. Call ngx_http_finalize_request() and then return a status code, and you’ve double-finalized the request. The symptom is a worker that dies under load with nothing useful in the error log. You find it once by reading a core dump at an hour you’d rather not discuss. You find it every time after that in 0.8 seconds. If you’re writing a module from scratch, the same class of mistake is why we keep a skeleton nginx module with the boilerplate already correct and a test harness that exercises it under load.

184 of the 469 rules are copied from CodeRabbit’s ast-grep essentials at commit 73120109, each recording its source and Apache-2.0 notice in its leading comments, with the upstream license kept under LICENSES/. We didn’t write those and don’t claim them.

Fixtures, or the rule doesn’t ship

Every rule has a mirrored fixture under tests/<language>/<category>/<id>.yml. 938 of them. The shape:

valid:
  - 'int f(const unsigned char *hmac, const unsigned char *e, size_t n) { return CRYPTO_memcmp(hmac, e, n) == 0; }'
  - 'void f(void) { /* memcmp(hmac, expected, n); */ }'
  - 'const char *s = "memcmp(hmac, expected, n)";'
invalid:
  - 'int f(const unsigned char *hmac, const unsigned char *e, size_t n) { return memcmp(hmac, e, n) == 0; }'

Look at the third valid entry. A string literal containing what looks exactly like a match. The second is the same code sitting in a comment. Neither may match, and both are in the fixture because a grep-shaped implementation of this rule fires on both, and because a future edit that quietly makes the rule grep-shaped again needs to fail loudly rather than pass.

Near misses are the point of the fixture. Positive examples prove the rule fires. Only the near misses prove it fires for the right reason. A fixture with four positives and no lookalikes tells you nothing you didn’t already know while writing the pattern. It’s the same discipline that keeps a YARA corpus honest: without a negative control you’re measuring your own optimism.

CI validates only changed rules, each copied into an isolated config alongside its fixture, so one rule cannot pass because another language happened to load cleanly. For that check it installs the current latest ast-grep from npm, against the pinned engine used for scanning. Pinning gives reproducible verdicts. Testing on latest tells us a grammar change is coming before it lands on everyone.

Authoring, enrichment and scheduling live in a separate private harness repository, which is part of a larger workflow that keeps rule creation reviewable. The public pack is rules, fixtures, config and licenses, because those are the parts anyone consuming the rules actually needs, and a rule that only works inside our tooling isn’t a rule we should be shipping.

Where ast-grep rules don’t help at all

There’s no dataflow here, and none is coming. A rule sees one syntax tree and matches shapes in it. If your bug is “this attacker-controlled value reaches that sink four functions away”, ast-grep is the wrong tool and you want CodeQL, or Semgrep with taint mode. Runtime request inspection is a different layer again, which is what a WAF in front of nginx is for.

What it’s genuinely good at is the local, visible, known bug: the precedence trap, the wrong sizeof operand, the off-by-one in strncat, the getenv result going straight into a %s. Those ship constantly, in code written by people who know better, at 17:40 on a Friday. 0.8 seconds against a module is cheap enough to run on every save, and cheap enough that nobody negotiates about whether to run it.

The pack is under the MyGuard Internal Use License 1.0: internal use including commercial, no distribution to third parties outside GitHub, with a carve-out for forks and branches used to prepare pull requests.

Anyway. Go count how many ngx_palloc calls in your own module have a NULL check under them. I’ll wait.

Related reading

• Writing an nginx module from a skeleton – the boilerplate these C rules are checking.
• An nginx module test harness – what to run after static analysis comes back clean.
• YARA rules for mail scanning – the same fixture-and-negative-control discipline, different matcher.
• The MyGuard review workflow – where rule authoring and enrichment actually sit.