feat: drop clj-holmes; replace its detections with semgrep rules

.github/workflows/security.yml

@@ -37,15 +37,8 @@ on:
           its own ref.
         type: string
         default: "v1"
-      holmes-upstream-ref:
-        description: >-
-          Upstream clj-holmes rules repo. Pinned rather than floating: the stock action
-          fetched #main at runtime, leaving detection rules unpinned in a workflow that
-          SHA-pins everything else.
-        type: string
-        default: "git://clj-holmes/clj-holmes-rules#main"
       ignored-paths:
-        description: "Paths both clj-holmes and semgrep must skip (e.g. deliberately-vulnerable fixtures)"
+        description: "Paths semgrep must skip (e.g. deliberately-vulnerable fixtures)"
         type: string
         default: ""
     secrets:
@@ -61,8 +54,8 @@ on:
 permissions:
   # contents: read only. `pull-requests: write` was here for clj-holmes-action,
   # which posted PR summary comments; that action is gone. Nothing left in this
-  # workflow writes to a PR — clj-holmes and semgrep emit SARIF to files that go
-  # up as artifacts, gitleaks/actionlint/zizmor are plain binaries, and
+  # workflow writes to a PR — semgrep emits SARIF to a file that goes up as an
+  # artifact, gitleaks/actionlint/zizmor are plain binaries, and
   # action-shellcheck annotates via workflow commands, which needs no permission.
   #
   # This is not cosmetic: a reusable workflow cannot request more than its caller
@@ -189,102 +182,6 @@ jobs:
           echo "Linting: ${paths[*]}"
           clj-kondo --lint "${paths[@]}" --fail-level error
 
-  clj-holmes:
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1  # v7.0.1
-        with:
-          persist-credentials: false
-      - name: Install clj-holmes
-        # Direct binary install rather than clj-holmes-action: the action's
-        # entrypoint fetches its rules from an unpinned #main at runtime, which
-        # left detection rules floating in a workflow that SHA-pins every other
-        # third-party action. Same install pattern the gitleaks job uses.
-        shell: bash
-        run: |
-          set -euo pipefail
-          VER=1.4.3
-          curl -fsSL "https://github.com/clj-holmes/clj-holmes/releases/download/v${VER}/clj-holmes-ubuntu-latest" \
-            -o /tmp/clj-holmes
-          sudo install -m 755 /tmp/clj-holmes /usr/local/bin/clj-holmes
-      - name: Fetch upstream rules
-        # Upstream rules ONLY. The cleancoders custom rules are semgrep rules,
-        # not clj-holmes rules: clj-holmes reads only *.clj — it silently skips
-        # .cljs and .cljc, and its edamame call omits :read-cond so .cljc fails
-        # to parse even when renamed, with the failure swallowed. Upstream has
-        # been unmaintained since Oct 2022, so that is not getting fixed.
-        #
-        # What clj-holmes still earns its place for: MD5, SHA-1, Blowfish,
-        # DESede, ECB, weak SSL context, insecure hostname verifiers, XXE, and
-        # read-string — all hard-failing, all .clj-only.
-        shell: bash
-        env:
-          HOLMES_UPSTREAM_REF: ${{ inputs.holmes-upstream-ref }}
-        run: |
-          set -euo pipefail
-          clj-holmes fetch-rules -r "$HOLMES_UPSTREAM_REF" -o /tmp/rules
-          # A scan with no rules exits 0 and looks like a clean build. Upstream
-          # ships ~10 rules; floor well below that so pruning one does not
-          # false-alarm, while still catching catastrophic loss (fetch failed,
-          # ref renamed, empty tarball).
-          count=$(find /tmp/rules -name '*.yml' | wc -l)
-          echo "loaded $count rules"
-          if [ "$count" -lt 5 ]; then
-            echo "::error::only $count rules loaded; refusing to scan"
-            exit 1
-          fi
-      - name: clj-holmes SAST
-        shell: bash
-        env:
-          IGNORED: ${{ inputs.ignored-paths }}
-        run: |
-          set -euo pipefail
-          # --no-fail-on-result on purpose. `--fail-on-result -t sarif` exits 3
-          # even with ZERO findings (clj-holmes bug; unmaintained since 2022, so
-          # it will not be fixed). -t json and -t stdout exit 0 correctly, which
-          # pins it to that one combination. We need SARIF for the evidence
-          # artifact, so we take the exit decision ourselves below.
-          #
-          # This also fixes a second problem: with SARIF going to a file, a
-          # failing scan printed nothing, so the log gave no reason. Now the
-          # findings are echoed before the job fails.
-          # --no-verbose is REQUIRED, not cosmetic. clj-holmes renders a progrock
-          # progress bar whose ETA calculation overflows an int on large repos:
-          #   IllegalArgumentException: Value out of range for int: 35050732419
-          #     progrock.core$interval_str -> clj_holmes.logic.progress
-          # That kills the scan with exit 255 and writes no SARIF at all. It is
-          # silent on small repos and fatal on real ones (~1300 Clojure files).
-          # The stock clj-holmes-action always passed --no-verbose; dropping it
-          # when replacing that action is what reintroduced this.
-          args=(scan -p . -d /tmp/rules --no-fail-on-result --no-verbose -t sarif -o clj-holmes.sarif)
-          # NOT `[ -n "$IGNORED" ] && args+=(...)`: that whole statement returns 1
-          # when the input is empty (the default), and `set -e` would exit here.
-          if [ -n "$IGNORED" ]; then
-            args+=(-i "$IGNORED")
-          fi
-          clj-holmes "${args[@]}"
-
-          if [ ! -f clj-holmes.sarif ]; then
-            echo "::error::clj-holmes produced no SARIF; the scan did not complete"
-            exit 1
-          fi
-          count=$(jq '[.runs[].results[]?] | length' clj-holmes.sarif)
-          if [ "$count" -gt 0 ]; then
-            jq -r '.runs[].results[]
-                   | "  \(.ruleId)  \(.locations[0].physicalLocation.artifactLocation.uri):\(.locations[0].physicalLocation.region.startLine // 0)"' \
-              clj-holmes.sarif
-            echo "::error::clj-holmes found $count finding(s); see above and the clj-holmes-sarif artifact"
-            exit 1
-          fi
-          echo "clj-holmes: no findings"
-      - name: Upload SARIF
-        if: always()   # evidence of a FAILING scan is the evidence most worth keeping
-        uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a  # v7.0.1 (node24)
-        with:
-          name: clj-holmes-sarif
-          path: clj-holmes.sarif
-          retention-days: 90
-
   shellcheck:
     needs: detect
     if: needs.detect.outputs.has-shellcheck-target == 'true'
@@ -386,8 +283,14 @@ jobs:
 
   semgrep:
     runs-on: ubuntu-latest
-    # PRIMARY engine for the cleancoders custom rules. semgrep reads .clj, .cljs
-    # AND .cljc; clj-holmes reads only .clj and is unmaintained since Oct 2022.
+    # The ONLY Clojure scanner. clj-holmes was dropped: it read just .clj
+    # (silently skipping .cljs and .cljc), was unmaintained since Oct 2022, and
+    # cost three bugs — exiting 3 on zero findings with -t sarif, hiding findings
+    # in a file, and crashing on a progress-bar int overflow that blocked a
+    # production deploy. Its unique detections (read-string, clojure.xml XXE,
+    # weak crypto, insecure TLS) are now cc-* rules that measurably outperform
+    # it: 8 of 8 fixture cases versus its 6, and they cover .cljs/.cljc too.
+    #
     # semgrep's weakness is that it cannot resolve namespace aliases, so each
     # rule enumerates them and spec-fixtures/ exercises more than one spelling —
     # see bin/test-rules.sh, which fails if that stops working.
@@ -399,10 +302,10 @@ jobs:
     # surfaces it without blocking; semgrep-blocking: true fails the build.
     continue-on-error: ${{ !inputs.semgrep-blocking }}
     container:
-      # Digest-pinned, not :latest. semgrep is now the PRIMARY detection engine,
-      # so an unpinned image would let the thing that decides whether the build
-      # is secure change underneath us — the same hole as the unpinned upstream
-      # clj-holmes rules. zizmor flags this as unpinned-images (high).
+      # Digest-pinned, not :latest. semgrep is now the ONLY Clojure detection
+      # engine, so an unpinned image would let the thing that decides whether the
+      # build is secure change underneath us. zizmor flags this as
+      # unpinned-images (high).
       image: semgrep/semgrep@sha256:98c2572fced2474539fd27cab3207ebd8e95e4e7aab4c3b381fdc5e2641d9941  # latest @ 2026-07-22
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1  # v7.0.1

README.md

@@ -4,19 +4,26 @@ Shared reusable GitHub Actions workflows for cleancoders repos.
 
 ## `security.yml` — reusable security-scan workflow
 
-Runs eight scanners. **Hard-fail** (block the caller): `clj-kondo`, `clj-holmes`,
+Runs seven scanners. **Hard-fail** (block the caller): `clj-kondo`,
 `shellcheck`, `gitleaks`, `actionlint`. **Advisory by default** (report, never
 block): `clj-watson`, `semgrep`, `zizmor` — each can be made blocking
 per-consumer via the `clj-watson-blocking` / `semgrep-blocking` /
 `zizmor-blocking` inputs.
 
-**semgrep is the primary Clojure detection engine.** It carries the 12
-cleancoders `cc-*` rules in `security-rules/semgrep/` and reads `.clj`, `.cljs`,
-and `.cljc`. `clj-holmes` runs upstream rules only — it covers weak crypto,
-XXE, and `read-string`, but reads **only `.clj`**: it silently skips `.cljs` and
-`.cljc`, and its parser rejects reader conditionals. Upstream has been
-unmaintained since October 2022. That split is deliberate; see
-`docs/superpowers/specs/2026-07-27-cwe-owasp-coverage-design.md` (Revision 2).
+**semgrep is the only Clojure detection engine.** It carries the 16 cleancoders
+`cc-*` rules in `security-rules/semgrep/` and reads `.clj`, `.cljs`, and
+`.cljc`.
+
+`clj-holmes` was removed. It read only `.clj` — silently skipping `.cljs` and
+`.cljc`, and rejecting reader conditionals outright — had been unmaintained
+since October 2022, and produced three failures in its first week: exiting 3 on
+zero findings under `-t sarif`, writing findings to a file so a red build gave
+no reason, and crashing on a progress-bar integer overflow that blocked a
+production deploy. Its unique detections are now `cc-read-string`,
+`cc-clojure-xml-xxe`, `cc-weak-crypto`, and `cc-insecure-tls`, which catch 8 of
+8 fixture cases against its 6 — it shipped rules for Blowfish and DESede and
+matched neither. Rationale: `docs/superpowers/specs/2026-07-27-cwe-owasp-coverage-design.md`
+(Revision 2).
 
 ### Usage
 
@@ -47,8 +54,7 @@ jobs:
 | `zizmor-blocking` | `false` | When `true`, zizmor Actions-security findings fail the workflow. Advisory by default because zizmor's defaults light up existing repos. |
 | `extra-rules-dir` | `".security-rules"` | Consumer-supplied semgrep rules, added as an extra `--config`. Self-skips when the directory is absent. |
 | `rules-ref` | `"v1"` | Ref of this repo to source the `cc-*` rules from. **Must match the ref you consume the workflow at** — a reusable workflow cannot determine its own ref, so consuming `@v2` or a SHA without setting this gets you `v1` rules. |
-| `holmes-upstream-ref` | `"git://clj-holmes/clj-holmes-rules#main"` | Upstream clj-holmes rules source. Override to pin a SHA. |
-| `ignored-paths` | `""` | Paths both clj-holmes and semgrep must skip, e.g. deliberately-vulnerable fixtures. |
+| `ignored-paths` | `""` | Paths semgrep must skip, e.g. deliberately-vulnerable fixtures. |
 
 ### Coverage
 
@@ -65,16 +71,20 @@ is authoritative for its own.
 |------|-------|-----|------------|----------|
 | `cc-cljs-eval` | `cljs-dom-xss` | 94 | A05 | yes |
 | `cc-cljs-innerhtml` | `cljs-dom-xss` | 79 | A05 | yes |
+| `cc-clojure-xml-xxe` | `xxe` | 611 | A02 | no (triage) |
 | `cc-dangerously-set-html` | `cljs-dom-xss` | 79 | A05 | yes |
 | `cc-explain-data-response` | `spec-malli-leak` | 209 | A10 | yes |
 | `cc-generic-catch` | `fail-open` | 636, 396 | A10 | no (triage) |
 | `cc-hiccup-raw` | `hiccup-injection` | 79 | A05 | yes |
+| `cc-insecure-tls` | `insecure-tls-verification` | 295 | A07 | yes |
 | `cc-load-string` | `dynamic-eval` | 94 | A05 | yes |
 | `cc-nippy-thaw` | `java-deserialization` | 502 | A08 | yes |
 | `cc-path-traversal` | `path-traversal` | 22 | A01 | no (triage) |
+| `cc-read-string` | `read-string-rce` | 94 | A05 | yes |
 | `cc-shell-exec` | `command-injection` | 78, 77 | A05 | yes |
 | `cc-snakeyaml-unsafe` | `java-deserialization` | 502 | A08 | yes |
 | `cc-sql-string-concat` | `sql-injection` | 89 | A05 | yes |
+| `cc-weak-crypto` | `weak-crypto` | 327, 328 | A04 | yes |
 
 <!-- END COVERAGE -->
 
@@ -91,14 +101,19 @@ A coverage table that overstates is worse than none, so:
    miss. `spec-fixtures/` exercises more than one alias per sink and
    `bin/test-rules.sh` fails if any stops matching, so the enumeration is
    test-guarded rather than aspirational — but it is still enumeration.
-3. **clj-holmes rules apply to `.clj` only** — not `.cljs`, not `.cljc`. That
-   covers the crypto, XXE, and `read-string` rows.
+3. **`cc-weak-crypto` and `cc-insecure-tls` match algorithm names textually**
+   (`pattern-regex`), because semgrep does not bind metavariables inside Clojure
+   string literals. They can therefore fire inside a comment or an unrelated
+   string. Weaker than the structural rules, but the thing being checked *is* a
+   literal.
 4. **OWASP A06 Insecure Design is uncovered.** It is a threat-modeling category.
 5. **10 of 19 applicable CWE Top 25 entries depend on a manual
    `/security-audit` run** — access control above all. CI cannot invoke it.
    Expected cadence is once per release; nothing enforces that.
-6. **`cc-path-traversal` and `cc-generic-catch` do not block** (`severity:
-   WARNING`). Without dataflow they cannot be precise enough to gate a build.
+6. **`cc-path-traversal`, `cc-generic-catch`, and `cc-clojure-xml-xxe` do not
+   block** (`severity: WARNING`). Without dataflow they cannot be precise enough
+   to gate a build — `cc-clojure-xml-xxe` in particular fires on every XML parse
+   because verifying that the factory was hardened requires tracking the object.
 7. **Rules track `rules-ref`, default `v1`.** Consuming another ref without
    setting it gets v1 rules.
 

security-rules/semgrep/cc-clojure-xml-xxe.yaml

@@ -0,0 +1,23 @@
+rules:
+  - id: cc-clojure-xml-xxe
+    languages: [clojure]
+    severity: WARNING
+    message: >-
+      clojure.xml/parse and clojure.data.xml/parse use JVM SAX defaults that permit
+      external-entity resolution, so parsing untrusted XML is XXE. Harden the factory
+      explicitly (disallow-doctype-decl true, external entities false).
+      LOW PRECISION — this fires on every parse call, hardened or not: verifying
+      that the factory was configured requires dataflow neither engine has. Triage
+      with /security-audit rather than treating it as a defect.
+    metadata:
+      cwe: ["CWE-611: Improper Restriction of XML External Entity Reference"]
+      owasp: ["A02:2025 - Security Misconfiguration"]
+      class: xxe
+      confidence: LOW
+    pattern-either:
+      - pattern: (clojure.xml/parse ...)
+      - pattern: (xml/parse ...)
+      - pattern: (cxml/parse ...)
+      - pattern: (clojure.data.xml/parse ...)
+      - pattern: (data.xml/parse ...)
+      - pattern: (dxml/parse ...)

security-rules/semgrep/cc-insecure-tls.yaml

@@ -0,0 +1,23 @@
+rules:
+  - id: cc-insecure-tls
+    languages: [clojure]
+    severity: ERROR
+    message: >-
+      TLS configured without authentication: an obsolete protocol, a permissive
+      hostname verifier, or a trust-all manager. This stops passive eavesdropping
+      but not an active machine-in-the-middle. Use the platform default verifier and
+      TLSv1.2+; pin a custom trust store if a private CA is genuinely required.
+    metadata:
+      cwe: ["CWE-295: Improper Certificate Validation"]
+      owasp: ["A07:2025 - Authentication Failures"]
+      class: insecure-tls-verification
+      confidence: HIGH
+      alias-exempt: "Java interop on javax.net.ssl; no Clojure alias"
+    # pattern-regex for the protocol string: semgrep does not bind metavariables
+    # inside Clojure string literals, so (SSLContext/getInstance "$P") never matches.
+    pattern-either:
+      - pattern-regex: \(SSLContext/getInstance\s+"(?i)(SSL|SSLv2|SSLv3|TLSv1|TLSv1\.1)"
+      - pattern: (reify HostnameVerifier (verify [...] true))
+      - pattern: (reify javax.net.ssl.HostnameVerifier (verify [...] true))
+      - pattern: (setDefaultHostnameVerifier ...)
+      - pattern: "{... :insecure? true ...}"

security-rules/semgrep/cc-read-string.yaml

@@ -0,0 +1,19 @@
+rules:
+  - id: cc-read-string
+    languages: [clojure]
+    severity: ERROR
+    message: >-
+      clojure.core/read-string honors the #= reader-eval tag, so reading
+      attacker-controlled text is remote code execution. Use clojure.edn/read-string.
+      Note edn is only safe with an empty :readers map — a custom reader on
+      attacker-controlled tag data executes too.
+    metadata:
+      cwe: ["CWE-94: Improper Control of Generation of Code ('Code Injection')"]
+      owasp: ["A05:2025 - Injection"]
+      class: read-string-rce
+      confidence: MEDIUM
+      alias-exempt: "clojure.core/read-string is referred by default; no alias to enumerate"
+    pattern-either:
+      - pattern: (read-string $X)
+      - pattern: (clojure.core/read-string $X)
+    pattern-not: (clojure.edn/read-string $X)

security-rules/semgrep/cc-weak-crypto.yaml

@@ -0,0 +1,23 @@
+rules:
+  - id: cc-weak-crypto
+    languages: [clojure]
+    severity: ERROR
+    message: >-
+      Broken or reversible cryptographic primitive. Use SHA-256+ for digests,
+      AES/GCM for encryption, and bcrypt or argon2 for passwords — a bare digest is
+      never a password hash regardless of algorithm strength.
+    metadata:
+      cwe:
+        - "CWE-327: Use of a Broken or Risky Cryptographic Algorithm"
+        - "CWE-328: Use of Weak Hash"
+      owasp: ["A04:2025 - Cryptographic Failures"]
+      class: weak-crypto
+      confidence: HIGH
+      alias-exempt: "Java interop on javax.crypto / java.security; no Clojure alias"
+    # pattern-regex, not a metavariable: semgrep does not bind metavariables inside
+    # string literals in Clojure, so (Cipher/getInstance "$ALG") never matches. The
+    # thing being checked IS the literal, so a regex is the honest instrument.
+    pattern-either:
+      - pattern-regex: \(MessageDigest/getInstance\s+"(?i)(MD2|MD4|MD5|SHA-?1)"
+      - pattern-regex: \(Cipher/getInstance\s+"[^"]*(?i)(ECB|Blowfish|DESede|RC2|RC4)[^"]*"
+      - pattern-regex: \(Cipher/getInstance\s+"(?i)DES(/[^"]*)?"

spec-fixtures/expectations.tsv

@@ -11,3 +11,7 @@ cc-generic-catch	triage_low.clj
 cc-nippy-thaw	deser.clj
 cc-snakeyaml-unsafe	deser.clj
 cc-explain-data-response	error_leak.clj
+cc-read-string	crypto_tls.clj
+cc-clojure-xml-xxe	crypto_tls.clj
+cc-weak-crypto	crypto_tls.clj
+cc-insecure-tls	crypto_tls.clj

spec-fixtures/safe/crypto_tls.clj

@@ -0,0 +1,27 @@
+(ns fixtures.crypto-tls-safe
+  (:require [clojure.edn :as edn])
+  (:import [javax.crypto Cipher]
+           [javax.net.ssl SSLContext]
+           [java.security MessageDigest]))
+
+;; edn/read-string does not honor #= — both the aliased and fully-qualified
+;; spellings must stay clean.
+(defn parse-config  [s] (edn/read-string s))
+(defn parse-config2 [s] (clojure.edn/read-string s))
+
+;; Modern primitives.
+(defn digest      [] (MessageDigest/getInstance "SHA-256"))
+(defn digest-512  [] (MessageDigest/getInstance "SHA-512"))
+(defn cipher-gcm  [] (Cipher/getInstance "AES/GCM/NoPadding"))
+(defn cipher-cbc  [] (Cipher/getInstance "AES/CBC/PKCS5Padding"))
+
+;; Protocol negotiation left to the platform, or pinned forward.
+(defn ctx-default [] (SSLContext/getInstance "TLS"))
+(defn ctx-13      [] (SSLContext/getInstance "TLSv1.3"))
+(defn ctx-12      [] (SSLContext/getInstance "TLSv1.2"))
+
+;; NOTE: no hardened clojure.xml/parse call appears here. cc-clojure-xml-xxe is
+;; severity WARNING precisely because it cannot tell a hardened factory from a
+;; default one — verifying that needs dataflow. Putting a hardened parse in this
+;; corpus would fail the false-positive check for a limitation the rule already
+;; documents, so the class is triaged by /security-audit instead.