Close CWE Top 25 / OWASP Top 10 gaps: 12 semgrep rules, actionlint + zizmor, generated coverage matrix - #4
Merged
Merged
Conversation
Drops clj-holmes-action for a direct binary install. The action's entrypoint hardcodes a single fetch-rules and never passes -d, which blocked custom rules; clj-holmes itself reads rules from any local directory, so upstream, cleancoders, and consumer rules union with a cp. Also pins the upstream rules ref. The action fetched git://clj-holmes/clj-holmes-rules#main at runtime, leaving detection rules unpinned in a workflow that SHA-pins every other third-party action. Adds a rule-count floor: a scan with no rules exits 0 and looks like a clean build, which is worse than no scan. First rule is cc-hiccup-raw (CWE-79/A05), chosen because it exercises namespace-alias resolution — the clj-holmes capability that ruled out semgrep, whose Clojure support is experimental and cannot resolve aliases. Fixture harness fails on missing AND unexpected findings; a silently non-matching rule would still appear in the coverage matrix. The safe corpus initially tripped the rule on a constant raw-string call: real signal, since clj-holmes has no dataflow. Resolved by documenting the limitation in the rule message and removing that case from the corpus, not by loosening the pattern. self-test passes rules-ref: github.sha so a PR validates its own rules rather than released ones, and holmes-ignored-paths so the deliberately-vulnerable fixtures do not fail this repo's own scan. Adding bin/ removed the shellcheck self-skip coverage, so a second security-skips invocation restores it. Root .clj-kondo/config.edn excludes test/fixtures from linting — the files are bad code by construction and are never loaded. Force-added, matching how clj/.clj-kondo/config.edn is tracked despite the .clj-kondo/ ignore rule.
CWE-89 is #2 on the CWE Top 25 and had no detection. Keys on a (str ...) form reaching a jdbc execute call rather than on SQL-looking text, so the safe fixture's allowlisted ORDER BY — which legitimately uses str — stays clean.
…es only clj-holmes reads only *.clj. It silently skips .cljs and .cljc, and its edamame call omits :read-cond, so a .cljc file fails to parse even when renamed — and code-str->code catches the exception, prints, and returns nil. A repo of .cljc therefore scans clean. That is disqualifying here: CWE-79 is #1 on the CWE Top 25 and is largely a ClojureScript problem, and .cljc is where c3kit puts shared domain logic. The fix upstream is two lines (widen clj-file?, pass :read-cond :allow :features). But clj-holmes' last real commit was October 2022, with open PRs from 2022 and 2023, so there is nobody to merge it, and owning a GraalVM fork of a security-critical binary is a permanent cost. So all 12 custom rules become semgrep rules. semgrep reads all three extensions, is maintained, and carries first-class metadata.cwe / metadata.owasp that flow into SARIF. semgrep cannot resolve namespace aliases, so every rule enumerates them and every vulnerable fixture exercises more than one spelling. check-rule-tags.sh enforces at least two enumerated prefixes unless the rule declares metadata.alias-exempt with a reason (js/ is reserved; :dangerouslySetInnerHTML is a keyword). That makes alias blindness fail a test instead of going silent, which is the condition the engine choice depends on. Fixtures moved test/fixtures -> spec-fixtures: semgrep's default .semgrepignore excludes test/ paths, so the corpus scanned zero targets and every rule looked like it was passing. Two latent bugs fixed while reworking the jobs: - returns 1 when the input is empty (the default) and would exit the step under set -e. Now an if-statement. - the rule-count floor of 10 sat exactly on the upstream rule count once the cleancoders rules left the union; lowered to 5. holmes-ignored-paths renamed ignored-paths, now applied to both engines. Spec and plan carry a Revision 2 section recording the whole decision.
CWE-78 (#9) and CWE-77 (#23) via clojure.java.shell/sh through a shell interpreter; CWE-94 (#10) via load-string, load-file, and requiring-resolve on a constructed symbol. cc-shell-exec keys on the "-c" flag rather than enumerating shell names: -c is what makes the interpreter parse the next argument as a command line, and it is common to bash/sh/ksh/zsh. Both safe fixtures use the same functions benignly (fixed argv, quoted symbol literal) so the rules must key on the dangerous shape, not the function name. Fixes a pipefail bug in check-rule-tags.sh surfaced by the first rule with two CWE entries: `yq ... | grep -q` fails the pipeline because grep -q closes the pipe on the first match and yq takes SIGPIPE. Capture then match.
CWE-22 (#6) and CWE-636/396 (OWASP A10). Both are severity: WARNING by design. Without dataflow, (io/file base x) cannot be proven user-derived and most generic catches are legitimate, so these feed /security-audit triage rather than blocking. Their messages say so explicitly. A10 Mishandling of Exceptional Conditions is new in OWASP 2025 and maps unusually well onto Clojure: CWE-396 is literally (catch Exception e ...) and CWE-636 is a permissive default on a swallowed security decision.
CWE-502 is #15 on the CWE Top 25. Upstream clj-holmes covers read-string but not nippy/thaw or the SnakeYAML no-arg constructor, both common in c3kit codebases. Patterns pin arity, which is what separates safe from unsafe here: the one-arg (nippy/thaw b) has no options map and therefore no :incl-class-allowlist, and (Yaml.) has no SafeConstructor. The safe fixture uses the two-arg forms and stays clean. CWE-502 maps to OWASP A08 only — it is absent from A05's 37-CWE injection list.
CWE-209, OWASP A10. Keys on the value appearing in :body rather than on the call itself, so the safe pattern — explain server-side, return field names — stays clean. The safe fixture calls s/explain-data deliberately to prove that discrimination holds. Completes the 12-rule set.
Covers OWASP A02 Security Misconfiguration and A03 Supply Chain. CI config is
itself attack surface, and this repo's entire product is workflows, so it
dogfoods both. actionlint hard-fails (syntax and expression correctness, not
opinion). zizmor is advisory via zizmor-blocking because its defaults light up
existing repos and blocking on adoption would wedge consumers. Both self-skip
with no .github/workflows, matching the shellcheck-dir idiom.
Running zizmor on our own workflows found 28 issues, now 0:
- HIGH unpinned-images: the semgrep container was :latest. semgrep is now the
PRIMARY detection engine, so an unpinned image lets the thing that decides
whether the build is secure change underneath us — the same hole as the
unpinned upstream clj-holmes rules. Pinned by digest.
- excessive-permissions: self-test.yml had no permissions block, so its jobs
inherited repo defaults. Added contents: read.
- artipacked x11: actions/checkout persists GITHUB_TOKEN into .git/config,
which matters here specifically because these jobs upload SARIF artifacts.
None of our checkouts push, so persist-credentials: false throughout.
Confirmed cleancoders/github-actions is public, so the cross-repo rules
checkout works with the default token for every consumer.
Hand-maintaining this table is how a coverage doc starts lying: a rule gets renamed or deleted and the table keeps claiming it, which is worse than no table once it reaches an auditor. Generated from metadata.cwe / metadata.owasp / metadata.class and checked in CI, so the scanner half physically cannot overstate. Verified --check fails on a mutated tag and passes when current. The blocking column is derived from severity, so the two WARNING triage rules are visibly non-blocking rather than implied so by prose. Manual-review rows stay in the clojure-security class index — the two halves live in different repos and each is authoritative for its own. Pins a yq install in the rule-tests job: jq ships on the runner image, yq does not reliably, and both scripts parse rule YAML.
Eight scanners now, six new inputs. States plainly what the coverage does not claim: no taint analysis anywhere, semgrep cannot resolve namespace aliases so each rule enumerates them, clj-holmes rules are .clj-only, A06 uncovered, two rules non-blocking, and 10 of 19 applicable CWE Top 25 entries reachable only by a manual audit run. Also documents the rules-ref footgun: a reusable workflow cannot determine its own ref, so consuming @v2 or a SHA without setting rules-ref silently gets v1 rules.
It was tagged dynamic-eval, whose skill-index row records CWE-94 code injection — but shell injection is CWE-78/77, a different weakness with a different fix. Cross-checking rule metadata.class against the clojure-security index caught the mismatch; the skill gains a command-injection class in the same change. Coverage table regenerated.
The workflow declared pull-requests: write for clj-holmes-action, which posted PR summary comments. That action is gone: clj-holmes and semgrep now emit SARIF to files that upload as artifacts, gitleaks/actionlint/zizmor are plain binaries, and action-shellcheck annotates via workflow commands, which needs no permission. Nothing left writes to a PR. This caused a startup_failure on PR #4, not just an over-broad grant. A called reusable workflow cannot request more than its caller grants, and self-test.yml now declares least privilege (contents: read) after zizmor flagged its absence. Asking for pull-requests: write made the workflow uncallable from any caller practising least privilege — including every consumer that does.
CI caught a clj-holmes bug: --fail-on-result combined with -t sarif exits 3 even with ZERO findings. -t json and -t stdout both exit 0 correctly, which pins it to that one combination. Upstream is unmaintained since Oct 2022, so it will not be fixed there. We need SARIF for the evidence artifact, so the scan now runs --no-fail-on-result and the job counts results with jq to decide the exit. That also fixes a second problem the failure exposed: with SARIF written to a file, the failing step printed nothing at all, so the log gave no reason for the red. Findings are now echoed with rule id and file:line before the job fails. Verified both directions locally: a clean tree passes, and a tree containing (read-string s) fails with the finding named.
security-skips duplicated all nine scanner jobs (18 checks total) to exercise
two conditionals. The main security job already covers most skip paths for real
— this repo has no deps.edn or src/, and the logs show those notices firing —
so the duplicate bought coverage of shellcheck-dir and extra-rules-dir only.
Replaced with bin/detect.sh plus bin/test-detect.sh (17 cases, milliseconds),
and a single detect job that runs it once and exposes the answers as job
outputs. That also deletes three duplicated inline detect blocks: shellcheck,
actionlint, and zizmor now gate on needs.detect.outputs instead of each
re-implementing the same 8-line predicate.
Why these predicates get tests at all: a wrong answer makes a scanner skip
silently while the build stays green, which is indistinguishable from 'scanned
and found nothing'. Mutation-checked — inverting the src-paths filter fails 4
tests.
Node 20 deprecations cleared:
upload-artifact v4.6.2 -> v7.0.1 (x3)
setup-uv v6.4.3 -> v9.0.0
checkout v5/v6 -> v7.0.1 (unified across both workflows)
setup-clojure 13.4 -> 13.6.1 (self-test's clj-lib was pinned to an
older version than security.yml, which
its own comment claimed was impossible)
actionlint 1.7.7 -> 1.7.12
18 checks -> 10.
Picks up 248b62b (release-library operator messaging) so CI verifies the state that will actually land, not a stale base.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 2 of the CWE/OWASP coverage work. Phase 1 (the
clojure-securityskillrestructure and route-inventory sweep) is in
cleancoders/agent-plugins.Spec:
docs/superpowers/specs/2026-07-27-cwe-owasp-coverage-design.mdPlan:
docs/superpowers/plans/2026-07-27-security-workflow-cwe-owasp-coverage.mdWhat changed
cc-*semgrep rules insecurity-rules/semgrep/, each carryingmetadata.cwe/metadata.owasp/metadata.class, built TDD against a pairedvulnerable/safe fixture corpus.
actionlint(hard-fail) andzizmor(advisory viazizmor-blocking) —CI config is itself attack surface, and this repo's product is workflows.
if: always()so a failing scan still leaves evidence.that no longer exists.
zizmor-blocking,extra-rules-dir,rules-ref,holmes-upstream-ref,ignored-paths.The spec changed during implementation
The spec chose clj-holmes as the primary engine on pattern-DSL expressiveness
(it resolves namespace aliases; semgrep does not). That was decided without
checking which files clj-holmes reads. It reads only
.clj:.cljsand.cljcare skipped silently, and its edamame call omits:read-cond, so.cljcfails to parse even when renamed — with the failureswallowed. A
.cljc-heavy repo scans clean. CWE-79 is #1 on the Top 25 andlargely a ClojureScript problem;
.cljcis where shared domain logic lives.The upstream fix is two lines, but clj-holmes' last real commit was October 2022.
So all 12 custom rules are semgrep rules; clj-holmes stays for its upstream
crypto/XXE/
read-stringrules,.clj-only. Recorded as "Revision 2" in bothspec and plan.
The tradeoff: semgrep cannot resolve aliases, so each rule enumerates them.
That is enforced, not aspirational —
bin/check-rule-tags.shfails any rulewithout ≥2 enumerated prefixes unless it declares
metadata.alias-exemptwith areason, and every vulnerable fixture exercises more than one spelling. Alias
blindness fails a test instead of going silent.
Defects found and fixed along the way
:latesttest/scanned 0 targets.semgrepignoreexcludestest/; every rule looked like it passed against nothing. Moved tospec-fixtures/[ -n "$IGNORED" ] && args+=(...)set -e— would have killed both scan steps for every consumer#mainat runtimeholmes-upstream-refartipacked×11GITHUB_TOKENinto.git/config— relevant here specifically because these jobs upload artifactscc-shell-execfiled underdynamic-evalzizmor on our own workflows: 28 findings → 0.
What this does not claim
Stated in the README, not just here:
.clj-only./security-auditrun.cc-path-traversalandcc-generic-catchareWARNINGand do not block.rules-ref, defaultv1.Verification
Local: rule tags, fixture corpus, coverage-table freshness, shellcheck, actionlint,
zizmor (0 findings), clj-kondo on fixtures, and
clj-libspecs (80 examples, 0failures) all pass.
Nothing has run in CI yet — the cross-repo rules checkout, the semgrep
container, and the yq install have never executed on a runner. That is what this
PR is for.
v1is deliberately not retagged; consumers are unaffected until it is.🤖 Generated with Claude Code