Skip to content

Commit 3af0426

Browse files
mkannwischermkannwischer
committed
lint: Add ast-grep check for visibility-macro annotations
Add an ast-grep lint for the MLD_INTERNAL_API / MLD_EXTERNAL_API / MLD_API_QUALIFIER convention on externally-linked declarations under mldsa/src/ (functions for now; data definitions not yet covered). Rule lives in scripts/.ast-grep/mldsa-declarations.yml as a multi-document YAML with two rules (function definition + function prototype). Each rule matches on tree-sitter-c node kinds (function_definition / declaration), excludes the three accepted annotation macros, static functions, and assembly entry points (*_asm, empty_cu_*). scripts/lint gains a check-ast-grep step; nix/util.nix's linters package picks up the ast-grep binary. Invocation: ast-grep scan --rule scripts/.ast-grep/mldsa-declarations.yml . Rationale for ast-grep over alternatives considered in this spike: - CodeQL: powerful but requires a full build + database; overkill for a lint rule and pulls in a non-OSS CLI. - Semgrep: C parser chokes on unknown macros and lacks sibling- relationship matchers, forcing return-type enumeration and implicit reliance on parser quirks. - ast-grep: tree-sitter-c parses reliably; rule matches on AST node kinds and names required annotations literally via regex. Known limitations: - Unlike CodeQL (which only sees compiled code), ast-grep scans all source text, so declarations inside #if blocks (e.g. MLDSA_DEBUG, native backend headers) are also flagged. Either annotate them or scope the `files:` globs narrower in follow-up. - A `static MLD_INLINE ...` combination parses with `static` outside the function_definition AST node, so the rule falls back to a text regex to exclude those. Temporary CI trimming: - .github/workflows/all.yml has every non-base job commented out so this branch validates the lint integration in isolation. Restore before merge. Signed-off-by: Matthias J. Kannwischer <matthias@zerorisc.com>
1 parent 55c9fb6 commit 3af0426

4 files changed

Lines changed: 221 additions & 90 deletions

File tree

‎.github/workflows/all.yml‎

Lines changed: 92 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -13,99 +13,102 @@ on:
1313
types: [ "opened", "synchronize" ]
1414

1515
jobs:
16+
# TEMPORARY: all non-base jobs disabled on the semgrep branch so the
17+
# semgrep-in-lint integration can be validated in isolation. Restore
18+
# before merge.
1619
base:
1720
name: Base
1821
permissions:
1922
contents: 'read'
2023
id-token: 'write'
2124
uses: ./.github/workflows/base.yml
2225
secrets: inherit
23-
lint-markdown:
24-
name: Lint Markdown
25-
permissions:
26-
contents: 'read'
27-
id-token: 'write'
28-
uses: ./.github/workflows/lint_markdown.yml
29-
nix:
30-
name: Nix
31-
permissions:
32-
actions: 'write'
33-
contents: 'read'
34-
id-token: 'write'
35-
uses: ./.github/workflows/nix.yml
36-
secrets: inherit
37-
ci:
38-
name: Extended
39-
permissions:
40-
contents: 'read'
41-
id-token: 'write'
42-
needs: [ base, nix ]
43-
uses: ./.github/workflows/ci.yml
44-
secrets: inherit
45-
cbmc:
46-
name: CBMC
47-
permissions:
48-
contents: 'read'
49-
id-token: 'write'
50-
pull-requests: 'write'
51-
needs: [ base, nix ]
52-
uses: ./.github/workflows/cbmc.yml
53-
secrets: inherit
54-
oqs_integration:
55-
name: libOQS
56-
permissions:
57-
contents: 'read'
58-
id-token: 'write'
59-
needs: [ base ]
60-
uses: ./.github/workflows/integration-liboqs.yml
61-
secrets: inherit
62-
awslc_integration:
63-
name: AWS-LC
64-
permissions:
65-
contents: 'read'
66-
id-token: 'write'
67-
needs: [ base ]
68-
uses: ./.github/workflows/integration-awslc.yml
69-
with:
70-
commit: v1.67.0
71-
secrets: inherit
72-
ct-test:
73-
name: Constant-time
74-
permissions:
75-
contents: 'read'
76-
id-token: 'write'
77-
needs: [ base, nix ]
78-
uses: ./.github/workflows/ct-tests.yml
79-
secrets: inherit
80-
slothy:
81-
name: SLOTHY
82-
permissions:
83-
contents: 'read'
84-
id-token: 'write'
85-
needs: [ base, nix ]
86-
uses: ./.github/workflows/slothy.yml
87-
secrets: inherit
88-
baremetal:
89-
name: Baremetal
90-
permissions:
91-
contents: 'read'
92-
id-token: 'write'
93-
needs: [ base ]
94-
uses: ./.github/workflows/baremetal.yml
95-
secrets: inherit
96-
opentitan_integration:
97-
name: OpenTitan
98-
permissions:
99-
contents: 'read'
100-
id-token: 'write'
101-
needs: [ base ]
102-
uses: ./.github/workflows/integration-opentitan.yml
103-
secrets: inherit
104-
isabelle:
105-
name: Isabelle
106-
permissions:
107-
contents: 'read'
108-
id-token: 'write'
109-
needs: [ base ]
110-
uses: ./.github/workflows/isabelle.yml
111-
secrets: inherit
26+
# lint-markdown:
27+
# name: Lint Markdown
28+
# permissions:
29+
# contents: 'read'
30+
# id-token: 'write'
31+
# uses: ./.github/workflows/lint_markdown.yml
32+
# nix:
33+
# name: Nix
34+
# permissions:
35+
# actions: 'write'
36+
# contents: 'read'
37+
# id-token: 'write'
38+
# uses: ./.github/workflows/nix.yml
39+
# secrets: inherit
40+
# ci:
41+
# name: Extended
42+
# permissions:
43+
# contents: 'read'
44+
# id-token: 'write'
45+
# needs: [ base, nix ]
46+
# uses: ./.github/workflows/ci.yml
47+
# secrets: inherit
48+
# cbmc:
49+
# name: CBMC
50+
# permissions:
51+
# contents: 'read'
52+
# id-token: 'write'
53+
# pull-requests: 'write'
54+
# needs: [ base, nix ]
55+
# uses: ./.github/workflows/cbmc.yml
56+
# secrets: inherit
57+
# oqs_integration:
58+
# name: libOQS
59+
# permissions:
60+
# contents: 'read'
61+
# id-token: 'write'
62+
# needs: [ base ]
63+
# uses: ./.github/workflows/integration-liboqs.yml
64+
# secrets: inherit
65+
# awslc_integration:
66+
# name: AWS-LC
67+
# permissions:
68+
# contents: 'read'
69+
# id-token: 'write'
70+
# needs: [ base ]
71+
# uses: ./.github/workflows/integration-awslc.yml
72+
# with:
73+
# commit: v1.67.0
74+
# secrets: inherit
75+
# ct-test:
76+
# name: Constant-time
77+
# permissions:
78+
# contents: 'read'
79+
# id-token: 'write'
80+
# needs: [ base, nix ]
81+
# uses: ./.github/workflows/ct-tests.yml
82+
# secrets: inherit
83+
# slothy:
84+
# name: SLOTHY
85+
# permissions:
86+
# contents: 'read'
87+
# id-token: 'write'
88+
# needs: [ base, nix ]
89+
# uses: ./.github/workflows/slothy.yml
90+
# secrets: inherit
91+
# baremetal:
92+
# name: Baremetal
93+
# permissions:
94+
# contents: 'read'
95+
# id-token: 'write'
96+
# needs: [ base ]
97+
# uses: ./.github/workflows/baremetal.yml
98+
# secrets: inherit
99+
# opentitan_integration:
100+
# name: OpenTitan
101+
# permissions:
102+
# contents: 'read'
103+
# id-token: 'write'
104+
# needs: [ base ]
105+
# uses: ./.github/workflows/integration-opentitan.yml
106+
# secrets: inherit
107+
# isabelle:
108+
# name: Isabelle
109+
# permissions:
110+
# contents: 'read'
111+
# id-token: 'write'
112+
# needs: [ base ]
113+
# uses: ./.github/workflows/isabelle.yml
114+
# secrets: inherit

‎nix/util.nix‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,8 @@ rec {
8888
shfmt
8989
shellcheck
9090
actionlint
91-
ruff;
91+
ruff
92+
ast-grep;
9293

9394
inherit (pkgs.python3Packages)
9495
mpmath sympy pyparsing pyyaml rich;

‎scripts/.ast-grep/mldsa-declarations.yml‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
# Copyright (c) The mldsa-native project authors
2+
# SPDX-License-Identifier: Apache-2.0 OR ISC OR MIT
3+
#
4+
# ast-grep rules enforcing visibility-macro annotations on every
5+
# externally-linked declaration under mldsa/src/. See mldsa/src/common.h
6+
# for the macro definitions.
7+
#
8+
# Invoked by scripts/lint via `ast-grep scan --rule`.
9+
10+
id: mldsa-function-def-missing-visibility
11+
language: c
12+
files:
13+
- 'mldsa/src/**/*.c'
14+
- 'mldsa/src/**/*.h'
15+
rule:
16+
all:
17+
- kind: function_definition
18+
- not:
19+
has:
20+
kind: type_identifier
21+
regex: '^(MLD_INTERNAL_API|MLD_EXTERNAL_API|MLD_API_QUALIFIER)$'
22+
stopBy: end
23+
- not:
24+
follows:
25+
stopBy: neighbor
26+
has:
27+
kind: identifier
28+
regex: '^(MLD_INTERNAL_API|MLD_EXTERNAL_API|MLD_API_QUALIFIER)$'
29+
stopBy: end
30+
- not:
31+
has:
32+
kind: storage_class_specifier
33+
regex: '^static$'
34+
stopBy: end
35+
- not:
36+
follows:
37+
stopBy: neighbor
38+
has:
39+
kind: storage_class_specifier
40+
regex: '^static$'
41+
stopBy: end
42+
- not:
43+
regex: '^\s*(static|MLD_STATIC_TESTABLE)\b'
44+
- not:
45+
has:
46+
kind: identifier
47+
regex: '^(empty_cu_|.*_asm$)'
48+
stopBy: end
49+
severity: error
50+
message: |-
51+
Function definition has external linkage but is not preceded by
52+
MLD_INTERNAL_API, MLD_EXTERNAL_API, or MLD_API_QUALIFIER
53+
(see mldsa/src/common.h).
54+
55+
---
56+
57+
id: mldsa-function-proto-missing-visibility
58+
language: c
59+
files:
60+
- 'mldsa/src/**/*.c'
61+
- 'mldsa/src/**/*.h'
62+
rule:
63+
all:
64+
- kind: declaration
65+
- has:
66+
kind: function_declarator
67+
stopBy: end
68+
- not:
69+
has:
70+
kind: type_identifier
71+
regex: '^(MLD_INTERNAL_API|MLD_EXTERNAL_API|MLD_API_QUALIFIER)$'
72+
stopBy: end
73+
- not:
74+
follows:
75+
stopBy: neighbor
76+
has:
77+
kind: identifier
78+
regex: '^(MLD_INTERNAL_API|MLD_EXTERNAL_API|MLD_API_QUALIFIER)$'
79+
stopBy: end
80+
- not:
81+
has:
82+
kind: storage_class_specifier
83+
regex: '^static$'
84+
stopBy: end
85+
- not:
86+
follows:
87+
stopBy: neighbor
88+
has:
89+
kind: storage_class_specifier
90+
regex: '^static$'
91+
stopBy: end
92+
- not:
93+
regex: '^\s*(static|MLD_STATIC_TESTABLE)\b'
94+
- not:
95+
has:
96+
kind: identifier
97+
regex: '^(empty_cu_|.*_asm$)'
98+
stopBy: end
99+
severity: error
100+
message: |-
101+
Function prototype has external linkage but is not preceded by
102+
MLD_INTERNAL_API, MLD_EXTERNAL_API, or MLD_API_QUALIFIER
103+
(see mldsa/src/common.h).

‎scripts/lint‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,30 @@ lint-c-files()
195195
checkerr "Lint C" "$(lint-c-files)"
196196
gh_group_end
197197

198+
check-ast-grep()
199+
{
200+
if ! command -v ast-grep >/dev/null; then
201+
gh_error_simple "ast-grep missing" "ast-grep is not installed"
202+
error "Check ast-grep"
203+
SUCCESS=false
204+
gh_summary_failure "Check ast-grep"
205+
return 0
206+
fi
207+
208+
if (cd "$ROOT" && ast-grep scan --rule scripts/.ast-grep/mldsa-declarations.yml .); then
209+
info "Check ast-grep"
210+
gh_summary_success "Check ast-grep"
211+
else
212+
error "Check ast-grep"
213+
gh_summary_failure "Check ast-grep"
214+
SUCCESS=false
215+
fi
216+
}
217+
218+
gh_group_start "Check ast-grep (visibility-macro annotations)"
219+
check-ast-grep
220+
gh_group_end
221+
198222
check-eol-dry-run()
199223
{
200224
for file in $(git ls-files -- ":/" ":/!:*.png"); do

0 commit comments

Comments
 (0)