From e796fb1a1752670a4f59f82c9eda5fa6e64d0bc5 Mon Sep 17 00:00:00 2001 From: Alex Root-Roatch Date: Mon, 27 Jul 2026 17:47:28 -0500 Subject: [PATCH 01/14] feat: union clj-holmes rule sources and add the first custom rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .clj-kondo/config.edn | 13 ++++ .github/workflows/security.yml | 86 ++++++++++++++++++++- .github/workflows/self-test.yml | 44 ++++++++++- bin/check-rule-tags.sh | 33 ++++++++ bin/test-rules.sh | 55 +++++++++++++ security-rules/clj-holmes/cc-hiccup-raw.yml | 27 +++++++ test/fixtures/expectations.tsv | 2 + test/fixtures/safe/hiccup_raw.clj | 14 ++++ test/fixtures/vulnerable/hiccup_raw.clj | 12 +++ 9 files changed, 278 insertions(+), 8 deletions(-) create mode 100644 .clj-kondo/config.edn create mode 100644 bin/check-rule-tags.sh create mode 100644 bin/test-rules.sh create mode 100644 security-rules/clj-holmes/cc-hiccup-raw.yml create mode 100644 test/fixtures/expectations.tsv create mode 100644 test/fixtures/safe/hiccup_raw.clj create mode 100644 test/fixtures/vulnerable/hiccup_raw.clj diff --git a/.clj-kondo/config.edn b/.clj-kondo/config.edn new file mode 100644 index 0000000..5b6e731 --- /dev/null +++ b/.clj-kondo/config.edn @@ -0,0 +1,13 @@ +;; Root clj-kondo config for the github-actions repo. +;; +;; test/fixtures/ holds deliberately-vulnerable Clojure used as the detection +;; corpus for the cc-* clj-holmes rules (bin/test-rules.sh). Linting it is +;; wrong by construction: the files exist precisely because they are bad code, +;; they are never loaded or compiled, and their namespaces intentionally do not +;; correspond to a source root. Excluding them here is the lint-side twin of +;; the `holmes-ignored-paths` input that keeps them out of security.yml's own +;; scan of this repo. +;; +;; The release library under clj/ carries its own .clj-kondo/config.edn; this +;; file does not apply to it. +{:output {:exclude-files ["test/fixtures/"]}} diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 9d71abe..0e4423b 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -19,6 +19,28 @@ on: description: "Fail the workflow on semgrep findings (default: advisory only)" type: boolean default: false + extra-rules-dir: + description: "Consumer-supplied clj-holmes rules; unioned in when the directory exists" + type: string + default: ".security-rules" + rules-ref: + description: >- + Ref of cleancoders/github-actions to source detection rules from. Must match + the ref this workflow is consumed at; a reusable workflow cannot determine + 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" + holmes-ignored-paths: + description: "Regex of paths clj-holmes must skip (e.g. deliberately-vulnerable test fixtures)" + type: string + default: "" secrets: private-git-ssh-key: description: >- @@ -115,12 +137,68 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + - name: Check out cleancoders detection rules + # A reusable workflow cannot reference files from its own repo: `uses: ./` + # resolves against the CALLER's checkout, and GitHub exposes no reliable + # "what ref am I running at" variable for reusable workflows. Hence an + # explicit ref. A consumer on a non-v1 ref must set rules-ref to match. + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + with: + repository: cleancoders/github-actions + ref: ${{ inputs.rules-ref }} + path: .cc-security-rules + - name: Install clj-holmes + # Direct binary install rather than clj-holmes-action: the action's + # entrypoint hardcodes a single `fetch-rules` and never passes `-d`, so + # custom rules cannot be unioned in. Same 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: Assemble rule set + # Three-way union: upstream + cleancoders + optional consumer. Rules are + # plain YAML in a directory, so unioning is a cp; `scan -d` reads any + # local dir. + shell: bash + env: + HOLMES_UPSTREAM_REF: ${{ inputs.holmes-upstream-ref }} + EXTRA_RULES_DIR: ${{ inputs.extra-rules-dir }} + run: | + set -euo pipefail + clj-holmes fetch-rules -r "$HOLMES_UPSTREAM_REF" -o /tmp/rules + cp -r .cc-security-rules/security-rules/clj-holmes/. /tmp/rules/ + if [ -d "$EXTRA_RULES_DIR" ]; then + echo "::notice::adding consumer rules from $EXTRA_RULES_DIR" + cp -r "$EXTRA_RULES_DIR"/. /tmp/rules/ + fi + # A scan with no rules exits 0 and looks like a clean build. Floor set + # well below the real count (9 upstream + 12 cleancoders) so upstream + # pruning a rule does not false-alarm; this catches catastrophic loss. + count=$(find /tmp/rules -name '*.yml' | wc -l) + echo "loaded $count rules" + if [ "$count" -lt 10 ]; then + echo "::error::only $count rules loaded; refusing to scan" + exit 1 + fi - name: clj-holmes SAST - uses: clj-holmes/clj-holmes-action@53daa4da4ff495cccf791e4ba4222a8317ddae9e # main @ 2026-07-09 + shell: bash + env: + IGNORED: ${{ inputs.holmes-ignored-paths }} + run: | + set -euo pipefail + args=(scan -p . -d /tmp/rules --fail-on-result -t sarif -o clj-holmes.sarif) + [ -n "$IGNORED" ] && args+=(-i "$IGNORED") + clj-holmes "${args[@]}" + - name: Upload SARIF + if: always() # evidence of a FAILING scan is the evidence most worth keeping + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 with: - path: '.' - output-type: 'stdout' - fail-on-result: 'true' + name: clj-holmes-sarif + path: clj-holmes.sarif + retention-days: 90 shellcheck: runs-on: ubuntu-latest diff --git a/.github/workflows/self-test.yml b/.github/workflows/self-test.yml index 541a158..956b27c 100644 --- a/.github/workflows/self-test.yml +++ b/.github/workflows/self-test.yml @@ -6,12 +6,48 @@ on: jobs: security: uses: ./.github/workflows/security.yml - # no inputs: exercises defaults. This repo has no src/, bin/, or deps.edn, - # so clj-kondo, shellcheck, and clj-watson must SKIP gracefully - # (portability guard test); gitleaks / clj-holmes / semgrep run against the - # repo tree. + with: + # Without this the rules checkout would fetch v1 from GitHub and a PR + # would be validated against RELEASED rules instead of its own. + rules-ref: ${{ github.sha }} + # test/fixtures/ holds deliberate vulnerabilities used as the detection + # corpus; scanning them would fail this repo's own build. + holmes-ignored-paths: "test/fixtures" + # This repo has no src/ or deps.edn, so clj-kondo and clj-watson must SKIP + # gracefully. bin/ now exists (the rule tooling), so shellcheck runs here + # for real; its skip path is covered by security-skips below. # No secrets passed: security.yml requires none (GITHUB_TOKEN is auto- # provided), so `secrets: inherit` would violate least privilege. + + security-skips: + # Portability guard: every job must self-skip cleanly on a repo missing the + # thing it scans. bin/ now exists, so point shellcheck at a path that does + # not, and likewise for the consumer rules directory. + uses: ./.github/workflows/security.yml + with: + rules-ref: ${{ github.sha }} + holmes-ignored-paths: "test/fixtures" + shellcheck-dir: "./no-such-dir" + extra-rules-dir: "./no-such-rules" + + rule-tests: + # Guards the cc-* detection rules themselves. Without this a rule could stop + # matching and still appear in the README coverage matrix — the exact false + # confidence this repo exists to prevent. + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 + - name: Install clj-holmes + shell: bash + run: | + set -euo pipefail + curl -fsSL "https://github.com/clj-holmes/clj-holmes/releases/download/v1.4.3/clj-holmes-ubuntu-latest" \ + -o /tmp/clj-holmes + sudo install -m 755 /tmp/clj-holmes /usr/local/bin/clj-holmes + - name: Check rule tags + run: bash bin/check-rule-tags.sh + - name: Run rule fixture tests + run: bash bin/test-rules.sh clj-lib: # Guards the release library the c3kit repos consume as a git dep. Note the # working-directory: the library lives under clj/ so this repo keeps no diff --git a/bin/check-rule-tags.sh b/bin/check-rule-tags.sh new file mode 100644 index 0000000..f50d4ae --- /dev/null +++ b/bin/check-rule-tags.sh @@ -0,0 +1,33 @@ +#!/usr/bin/env bash +# Every cleancoders rule must carry class-, cwe-, and owasp- tags. The coverage +# matrix in README.md is generated from these tags, so an untagged rule is a +# detection that exists but is invisible to the evidence trail. +set -euo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +RULES="${ROOT}/security-rules/clj-holmes" + +command -v yq >/dev/null || { echo "yq not installed"; exit 1; } + +status=0 +for f in "${RULES}"/*.yml; do + id="$(yq -r '.[0].id' "${f}")" + tags="$(yq -r '.[0].properties.tags[]' "${f}" 2>/dev/null || true)" + + case "${id}" in + cc-*) ;; + *) echo "${f}: rule id '${id}' must be prefixed cc-"; status=1 ;; + esac + + echo "${tags}" | grep -qE '^class-[a-z0-9-]+$' \ + || { echo "${f}: missing a class- tag"; status=1; } + echo "${tags}" | grep -qE '^cwe-[0-9]+$' \ + || { echo "${f}: missing a cwe- tag"; status=1; } + + owasp_count="$(echo "${tags}" | grep -cE '^owasp-a(0[1-9]|10)-2025$' || true)" + [ "${owasp_count}" -eq 1 ] \ + || { echo "${f}: needs exactly one owasp-aNN-2025 tag (found ${owasp_count})"; status=1; } +done + +[ "${status}" -eq 0 ] && echo "all rules tagged" +exit "${status}" diff --git a/bin/test-rules.sh b/bin/test-rules.sh new file mode 100644 index 0000000..2a4367d --- /dev/null +++ b/bin/test-rules.sh @@ -0,0 +1,55 @@ +#!/usr/bin/env bash +# Scans the fixture corpus with ONLY the cleancoders rules and diffs the result +# against expectations.tsv. Fails on missing findings (a rule stopped matching) +# and on unexpected ones (a rule got too broad, or an upstream change altered +# behaviour). Both directions matter: a silently non-matching rule still appears +# in the coverage matrix, which is exactly the false confidence this repo exists +# to prevent. +set -euo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +RULES="${ROOT}/security-rules/clj-holmes" +FIXTURES="${ROOT}/test/fixtures" +EXPECTED="${FIXTURES}/expectations.tsv" +WORK="$(mktemp -d)" +trap 'rm -rf "${WORK}"' EXIT + +command -v clj-holmes >/dev/null || { echo "clj-holmes not installed"; exit 1; } +command -v jq >/dev/null || { echo "jq not installed"; exit 1; } + +# --- vulnerable corpus: every expected finding must appear ------------------- +clj-holmes scan -p "${FIXTURES}/vulnerable" -d "${RULES}" \ + --no-fail-on-result -t sarif -o "${WORK}/vuln.sarif" >/dev/null + +# SARIF: ruleId plus the basename of the file it fired on. +jq -r '.runs[].results[] + | .ruleId + "\t" + (.locations[0].physicalLocation.artifactLocation.uri + | split("/") | last)' \ + "${WORK}/vuln.sarif" | sort -u > "${WORK}/actual.tsv" + +grep -v '^#' "${EXPECTED}" | grep -v '^[[:space:]]*$' | sort -u > "${WORK}/expected.tsv" + +missing="$(comm -23 "${WORK}/expected.tsv" "${WORK}/actual.tsv")" +unexpected="$(comm -13 "${WORK}/expected.tsv" "${WORK}/actual.tsv")" + +status=0 +if [ -n "${missing}" ]; then + echo "MISSING findings (rule stopped matching):"; echo "${missing}"; status=1 +fi +if [ -n "${unexpected}" ]; then + echo "UNEXPECTED findings (rule too broad):"; echo "${unexpected}"; status=1 +fi + +# --- safe corpus: must be completely clean ----------------------------------- +clj-holmes scan -p "${FIXTURES}/safe" -d "${RULES}" \ + --no-fail-on-result -t sarif -o "${WORK}/safe.sarif" >/dev/null + +safe_hits="$(jq -r '.runs[].results[] + | .ruleId + " in " + .locations[0].physicalLocation.artifactLocation.uri' \ + "${WORK}/safe.sarif")" +if [ -n "${safe_hits}" ]; then + echo "FALSE POSITIVES on the safe corpus:"; echo "${safe_hits}"; status=1 +fi + +[ "${status}" -eq 0 ] && echo "rule tests passed" +exit "${status}" diff --git a/security-rules/clj-holmes/cc-hiccup-raw.yml b/security-rules/clj-holmes/cc-hiccup-raw.yml new file mode 100644 index 0000000..23cf1f6 --- /dev/null +++ b/security-rules/clj-holmes/cc-hiccup-raw.yml @@ -0,0 +1,27 @@ +- id: cc-hiccup-raw + name: Unescaped HTML via hiccup raw / raw-string + severity: error + message: >- + hiccup.util/raw-string and hiccup.core/raw bypass Hiccup's auto-escaping. If + the value is user-controlled this is stored or reflected XSS. Sanitize with + the OWASP Java HTML Sanitizer, or drop the raw call and let Hiccup escape. + Note this rule has no dataflow: it also fires on a raw call whose argument is + a compile-time constant, which is safe. Trace provenance before acting — see + the hiccup-injection class in the clojure-security skill. + properties: + precision: high + tags: + - security + - class-hiccup-injection + - cwe-79 + - owasp-a05-2025 + patterns: + - patterns-either: + - pattern: "($& $custom-function $&)" + namespace: hiccup.util + function: raw-string + custom-function?: true + - pattern: "($& $custom-function $&)" + namespace: hiccup.core + function: raw + custom-function?: true diff --git a/test/fixtures/expectations.tsv b/test/fixtures/expectations.tsv new file mode 100644 index 0000000..fc67a4c --- /dev/null +++ b/test/fixtures/expectations.tsv @@ -0,0 +1,2 @@ +# — tab-separated. One line per expected finding. +cc-hiccup-raw hiccup_raw.clj diff --git a/test/fixtures/safe/hiccup_raw.clj b/test/fixtures/safe/hiccup_raw.clj new file mode 100644 index 0000000..1099292 --- /dev/null +++ b/test/fixtures/safe/hiccup_raw.clj @@ -0,0 +1,14 @@ +(ns fixtures.hiccup-raw-safe) + +;; Ordinary hiccup auto-escapes string content — no raw call at all. +(defn render-bio [user] + [:div.bio (:bio user)]) + +;; A constant divider expressed as data rather than raw markup. +(defn divider [] [:hr.rule]) + +;; NOTE: a `raw-string` call on a compile-time constant is genuinely safe, but +;; it is deliberately NOT in this corpus. clj-holmes has no dataflow analysis, +;; so it cannot distinguish a constant argument from a request-derived one and +;; will fire on both. That limitation is documented in the rule's message and +;; triaged by /security-audit, not papered over by loosening the pattern. diff --git a/test/fixtures/vulnerable/hiccup_raw.clj b/test/fixtures/vulnerable/hiccup_raw.clj new file mode 100644 index 0000000..a3c257b --- /dev/null +++ b/test/fixtures/vulnerable/hiccup_raw.clj @@ -0,0 +1,12 @@ +(ns fixtures.hiccup-raw + (:require [hiccup.util :as hu] + [hiccup.core :as h])) + +;; Aliased call — the whole reason this repo uses clj-holmes rather than +;; semgrep. Semgrep's experimental Clojure tree-sitter cannot resolve `hu` back +;; to hiccup.util and would need one literal pattern per alias. +(defn render-bio [user] + [:div.bio (hu/raw-string (:bio user))]) + +(defn render-post [post] + [:article (h/raw (:body post))]) From 89eed72e8df78c701465330c78a06ff94529cf11 Mon Sep 17 00:00:00 2001 From: Alex Root-Roatch Date: Mon, 27 Jul 2026 17:48:12 -0500 Subject: [PATCH 02/14] feat(rules): detect SQL built by string concatenation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../clj-holmes/cc-sql-string-concat.yml | 29 +++++++++++++++++++ test/fixtures/expectations.tsv | 1 + test/fixtures/safe/sql_injection.clj | 14 +++++++++ test/fixtures/vulnerable/sql_injection.clj | 9 ++++++ 4 files changed, 53 insertions(+) create mode 100644 security-rules/clj-holmes/cc-sql-string-concat.yml create mode 100644 test/fixtures/safe/sql_injection.clj create mode 100644 test/fixtures/vulnerable/sql_injection.clj diff --git a/security-rules/clj-holmes/cc-sql-string-concat.yml b/security-rules/clj-holmes/cc-sql-string-concat.yml new file mode 100644 index 0000000..e316742 --- /dev/null +++ b/security-rules/clj-holmes/cc-sql-string-concat.yml @@ -0,0 +1,29 @@ +- id: cc-sql-string-concat + name: SQL built by string concatenation + severity: error + message: >- + A jdbc execute call receives a (str ...)-built query. Use a parameterized + vector ["SELECT ... WHERE x = ?" v]. Where parameters cannot help — ORDER BY + columns, table names, dynamic WHERE fragments — allowlist the identifier + against a static map rather than interpolating it. + properties: + precision: medium + tags: + - security + - class-sql-injection + - cwe-89 + - owasp-a05-2025 + patterns: + - patterns-either: + - pattern: "($& $custom-function $& (str $&) $&)" + namespace: next.jdbc + function: execute! + custom-function?: true + - pattern: "($& $custom-function $& (str $&) $&)" + namespace: next.jdbc + function: execute-one! + custom-function?: true + - pattern: "($& $custom-function $& (str $&) $&)" + namespace: clojure.java.jdbc + function: query + custom-function?: true diff --git a/test/fixtures/expectations.tsv b/test/fixtures/expectations.tsv index fc67a4c..8d3dc47 100644 --- a/test/fixtures/expectations.tsv +++ b/test/fixtures/expectations.tsv @@ -1,2 +1,3 @@ # — tab-separated. One line per expected finding. cc-hiccup-raw hiccup_raw.clj +cc-sql-string-concat sql_injection.clj diff --git a/test/fixtures/safe/sql_injection.clj b/test/fixtures/safe/sql_injection.clj new file mode 100644 index 0000000..0e23715 --- /dev/null +++ b/test/fixtures/safe/sql_injection.clj @@ -0,0 +1,14 @@ +(ns fixtures.sql-injection-safe + (:require [next.jdbc :as jdbc])) + +(defn find-user [db name] + (jdbc/execute! db ["SELECT * FROM users WHERE name = ?" name])) + +(def ^:private +sortable+ {"created" "created_at" "title" "title"}) + +;; Deliberately still builds a string with `str`: this proves the rule keys on +;; a (str ...) form reaching an execute call, not on `str` appearing near SQL +;; text. The identifier is allowlisted, so the query is safe. +(defn list-sorted [db col] + (let [safe-col (get +sortable+ col "created_at")] + (jdbc/execute! db [(str "SELECT * FROM posts ORDER BY " safe-col)]))) diff --git a/test/fixtures/vulnerable/sql_injection.clj b/test/fixtures/vulnerable/sql_injection.clj new file mode 100644 index 0000000..5dbe6a1 --- /dev/null +++ b/test/fixtures/vulnerable/sql_injection.clj @@ -0,0 +1,9 @@ +(ns fixtures.sql-injection + (:require [next.jdbc :as jdbc])) + +(defn find-user [db name] + (jdbc/execute! db (str "SELECT * FROM users WHERE name = '" name "'"))) + +(defn list-sorted [db col] + ;; The dynamic-identifier trap: parameters cannot help here. + (jdbc/execute! db (str "SELECT * FROM posts ORDER BY " col))) From b1ff1bd41ce99e68960664d99227353070fa5086 Mon Sep 17 00:00:00 2001 From: Alex Root-Roatch Date: Mon, 27 Jul 2026 18:12:49 -0500 Subject: [PATCH 03/14] refactor: move custom rules to semgrep; clj-holmes keeps upstream rules only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .clj-kondo/config.edn | 4 +- .github/workflows/security.yml | 104 ++++++++++++------ .github/workflows/self-test.yml | 12 +- bin/check-rule-tags.sh | 47 +++++--- bin/test-rules.sh | 42 +++---- ...27-security-workflow-cwe-owasp-coverage.md | 33 ++++++ .../2026-07-27-cwe-owasp-coverage-design.md | 60 +++++++++- security-rules/clj-holmes/cc-hiccup-raw.yml | 27 ----- .../clj-holmes/cc-sql-string-concat.yml | 29 ----- security-rules/semgrep/cc-cljs-eval.yaml | 22 ++++ security-rules/semgrep/cc-cljs-innerhtml.yaml | 23 ++++ .../semgrep/cc-dangerously-set-html.yaml | 18 +++ security-rules/semgrep/cc-hiccup-raw.yaml | 28 +++++ .../semgrep/cc-sql-string-concat.yaml | 29 +++++ .../expectations.tsv | 3 + spec-fixtures/safe/cljs_xss.cljs | 15 +++ .../safe/hiccup_raw.clj | 0 .../safe/sql_injection.clj | 0 spec-fixtures/vulnerable/cljs_xss.cljs | 21 ++++ spec-fixtures/vulnerable/hiccup_raw.clj | 17 +++ .../vulnerable/sql_injection.clj | 7 +- test/fixtures/vulnerable/hiccup_raw.clj | 12 -- 22 files changed, 409 insertions(+), 144 deletions(-) delete mode 100644 security-rules/clj-holmes/cc-hiccup-raw.yml delete mode 100644 security-rules/clj-holmes/cc-sql-string-concat.yml create mode 100644 security-rules/semgrep/cc-cljs-eval.yaml create mode 100644 security-rules/semgrep/cc-cljs-innerhtml.yaml create mode 100644 security-rules/semgrep/cc-dangerously-set-html.yaml create mode 100644 security-rules/semgrep/cc-hiccup-raw.yaml create mode 100644 security-rules/semgrep/cc-sql-string-concat.yaml rename {test/fixtures => spec-fixtures}/expectations.tsv (60%) create mode 100644 spec-fixtures/safe/cljs_xss.cljs rename {test/fixtures => spec-fixtures}/safe/hiccup_raw.clj (100%) rename {test/fixtures => spec-fixtures}/safe/sql_injection.clj (100%) create mode 100644 spec-fixtures/vulnerable/cljs_xss.cljs create mode 100644 spec-fixtures/vulnerable/hiccup_raw.clj rename {test/fixtures => spec-fixtures}/vulnerable/sql_injection.clj (52%) delete mode 100644 test/fixtures/vulnerable/hiccup_raw.clj diff --git a/.clj-kondo/config.edn b/.clj-kondo/config.edn index 5b6e731..4bb0d44 100644 --- a/.clj-kondo/config.edn +++ b/.clj-kondo/config.edn @@ -1,6 +1,6 @@ ;; Root clj-kondo config for the github-actions repo. ;; -;; test/fixtures/ holds deliberately-vulnerable Clojure used as the detection +;; spec-fixtures/ holds deliberately-vulnerable Clojure used as the detection ;; corpus for the cc-* clj-holmes rules (bin/test-rules.sh). Linting it is ;; wrong by construction: the files exist precisely because they are bad code, ;; they are never loaded or compiled, and their namespaces intentionally do not @@ -10,4 +10,4 @@ ;; ;; The release library under clj/ carries its own .clj-kondo/config.edn; this ;; file does not apply to it. -{:output {:exclude-files ["test/fixtures/"]}} +{:output {:exclude-files ["spec-fixtures/"]}} diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 0e4423b..2cc49a4 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -20,7 +20,7 @@ on: type: boolean default: false extra-rules-dir: - description: "Consumer-supplied clj-holmes rules; unioned in when the directory exists" + description: "Consumer-supplied semgrep rules; added as an extra --config when the directory exists" type: string default: ".security-rules" rules-ref: @@ -37,8 +37,8 @@ on: SHA-pins everything else. type: string default: "git://clj-holmes/clj-holmes-rules#main" - holmes-ignored-paths: - description: "Regex of paths clj-holmes must skip (e.g. deliberately-vulnerable test fixtures)" + ignored-paths: + description: "Paths both clj-holmes and semgrep must skip (e.g. deliberately-vulnerable fixtures)" type: string default: "" secrets: @@ -137,20 +137,11 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 - - name: Check out cleancoders detection rules - # A reusable workflow cannot reference files from its own repo: `uses: ./` - # resolves against the CALLER's checkout, and GitHub exposes no reliable - # "what ref am I running at" variable for reusable workflows. Hence an - # explicit ref. A consumer on a non-v1 ref must set rules-ref to match. - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 - with: - repository: cleancoders/github-actions - ref: ${{ inputs.rules-ref }} - path: .cc-security-rules - name: Install clj-holmes # Direct binary install rather than clj-holmes-action: the action's - # entrypoint hardcodes a single `fetch-rules` and never passes `-d`, so - # custom rules cannot be unioned in. Same pattern the gitleaks job uses. + # 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 @@ -158,39 +149,44 @@ jobs: 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: Assemble rule set - # Three-way union: upstream + cleancoders + optional consumer. Rules are - # plain YAML in a directory, so unioning is a cp; `scan -d` reads any - # local dir. + - 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 }} - EXTRA_RULES_DIR: ${{ inputs.extra-rules-dir }} run: | set -euo pipefail clj-holmes fetch-rules -r "$HOLMES_UPSTREAM_REF" -o /tmp/rules - cp -r .cc-security-rules/security-rules/clj-holmes/. /tmp/rules/ - if [ -d "$EXTRA_RULES_DIR" ]; then - echo "::notice::adding consumer rules from $EXTRA_RULES_DIR" - cp -r "$EXTRA_RULES_DIR"/. /tmp/rules/ - fi - # A scan with no rules exits 0 and looks like a clean build. Floor set - # well below the real count (9 upstream + 12 cleancoders) so upstream - # pruning a rule does not false-alarm; this catches catastrophic loss. + # 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 10 ]; then + 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.holmes-ignored-paths }} + IGNORED: ${{ inputs.ignored-paths }} run: | set -euo pipefail args=(scan -p . -d /tmp/rules --fail-on-result -t sarif -o clj-holmes.sarif) - [ -n "$IGNORED" ] && args+=(-i "$IGNORED") + # 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[@]}" - name: Upload SARIF if: always() # evidence of a FAILING scan is the evidence most worth keeping @@ -308,6 +304,12 @@ 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. + # 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. + # # semgrep ALWAYS runs with --error, so any finding turns this job red and # stays visible — its "Blocking" label is a policy tag, NOT an exit code, so # --error is what actually makes `semgrep scan` exit non-zero. continue-on- @@ -318,5 +320,41 @@ jobs: image: semgrep/semgrep steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 - - name: Semgrep (OWASP Top 10 + default) - run: semgrep scan --config p/owasp-top-ten --config p/default --error + - name: Check out cleancoders detection rules + # A reusable workflow cannot reference files from its own repo: `uses: ./` + # resolves against the CALLER's checkout, and GitHub exposes no reliable + # "what ref am I running at" variable for reusable workflows. Hence an + # explicit ref. A consumer on a non-v1 ref must set rules-ref to match. + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + with: + repository: cleancoders/github-actions + ref: ${{ inputs.rules-ref }} + path: .cc-security-rules + - name: Semgrep (cleancoders rules + OWASP Top 10 + default) + shell: bash + env: + IGNORED: ${{ inputs.ignored-paths }} + EXTRA_RULES_DIR: ${{ inputs.extra-rules-dir }} + run: | + set -euo pipefail + args=(scan --error --sarif --output semgrep.sarif + --config .cc-security-rules/security-rules/semgrep + --config p/owasp-top-ten + --config p/default) + if [ -d "$EXTRA_RULES_DIR" ]; then + echo "::notice::adding consumer rules from $EXTRA_RULES_DIR" + args+=(--config "$EXTRA_RULES_DIR") + fi + # NOT `[ -n "$IGNORED" ] && args+=(...)`: that statement returns 1 when + # the input is empty (the default), and `set -e` would exit here. + if [ -n "$IGNORED" ]; then + args+=(--exclude "$IGNORED") + fi + semgrep "${args[@]}" + - name: Upload SARIF + if: always() # evidence of a FAILING scan is the evidence most worth keeping + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: semgrep-sarif + path: semgrep.sarif + retention-days: 90 diff --git a/.github/workflows/self-test.yml b/.github/workflows/self-test.yml index 956b27c..bc8944a 100644 --- a/.github/workflows/self-test.yml +++ b/.github/workflows/self-test.yml @@ -10,9 +10,9 @@ jobs: # Without this the rules checkout would fetch v1 from GitHub and a PR # would be validated against RELEASED rules instead of its own. rules-ref: ${{ github.sha }} - # test/fixtures/ holds deliberate vulnerabilities used as the detection + # spec-fixtures/ holds deliberate vulnerabilities used as the detection # corpus; scanning them would fail this repo's own build. - holmes-ignored-paths: "test/fixtures" + ignored-paths: "spec-fixtures" # This repo has no src/ or deps.edn, so clj-kondo and clj-watson must SKIP # gracefully. bin/ now exists (the rule tooling), so shellcheck runs here # for real; its skip path is covered by security-skips below. @@ -26,7 +26,7 @@ jobs: uses: ./.github/workflows/security.yml with: rules-ref: ${{ github.sha }} - holmes-ignored-paths: "test/fixtures" + ignored-paths: "spec-fixtures" shellcheck-dir: "./no-such-dir" extra-rules-dir: "./no-such-rules" @@ -37,13 +37,11 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 - - name: Install clj-holmes + - name: Install semgrep shell: bash run: | set -euo pipefail - curl -fsSL "https://github.com/clj-holmes/clj-holmes/releases/download/v1.4.3/clj-holmes-ubuntu-latest" \ - -o /tmp/clj-holmes - sudo install -m 755 /tmp/clj-holmes /usr/local/bin/clj-holmes + python3 -m pip install --quiet semgrep - name: Check rule tags run: bash bin/check-rule-tags.sh - name: Run rule fixture tests diff --git a/bin/check-rule-tags.sh b/bin/check-rule-tags.sh index f50d4ae..e12938c 100644 --- a/bin/check-rule-tags.sh +++ b/bin/check-rule-tags.sh @@ -1,32 +1,53 @@ #!/usr/bin/env bash -# Every cleancoders rule must carry class-, cwe-, and owasp- tags. The coverage -# matrix in README.md is generated from these tags, so an untagged rule is a -# detection that exists but is invisible to the evidence trail. +# Every cleancoders rule must carry metadata.cwe, metadata.owasp, and +# metadata.class. The coverage matrix in README.md is generated from these, so an +# untagged rule is a detection that exists but is invisible to the evidence trail. +# +# Also enforces the alias mitigation: semgrep cannot resolve namespace aliases, so +# a rule that matches a namespaced function must enumerate its aliases. Rules that +# opt out declare metadata.alias-exempt with a reason (special forms and interop +# like js/eval or .-innerHTML cannot be aliased). set -euo pipefail ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" -RULES="${ROOT}/security-rules/clj-holmes" +RULES="${ROOT}/security-rules/semgrep" command -v yq >/dev/null || { echo "yq not installed"; exit 1; } status=0 -for f in "${RULES}"/*.yml; do - id="$(yq -r '.[0].id' "${f}")" - tags="$(yq -r '.[0].properties.tags[]' "${f}" 2>/dev/null || true)" +for f in "${RULES}"/*.yaml; do + id="$(yq -r '.rules[0].id' "${f}")" case "${id}" in cc-*) ;; *) echo "${f}: rule id '${id}' must be prefixed cc-"; status=1 ;; esac - echo "${tags}" | grep -qE '^class-[a-z0-9-]+$' \ - || { echo "${f}: missing a class- tag"; status=1; } - echo "${tags}" | grep -qE '^cwe-[0-9]+$' \ - || { echo "${f}: missing a cwe- tag"; status=1; } + [ "$(yq -r '.rules[0].languages | contains(["clojure"])' "${f}")" = "true" ] \ + || { echo "${f}: must declare languages: [clojure]"; status=1; } - owasp_count="$(echo "${tags}" | grep -cE '^owasp-a(0[1-9]|10)-2025$' || true)" + yq -r '.rules[0].metadata.cwe[]' "${f}" 2>/dev/null | grep -qE '^CWE-[0-9]+' \ + || { echo "${f}: metadata.cwe must list at least one 'CWE-: ...' entry"; status=1; } + + owasp_count="$(yq -r '.rules[0].metadata.owasp[]' "${f}" 2>/dev/null \ + | grep -cE '^A(0[1-9]|10):2025' || true)" [ "${owasp_count}" -eq 1 ] \ - || { echo "${f}: needs exactly one owasp-aNN-2025 tag (found ${owasp_count})"; status=1; } + || { echo "${f}: needs exactly one 'A:2025 - ...' owasp entry (found ${owasp_count})"; status=1; } + + cls="$(yq -r '.rules[0].metadata.class // ""' "${f}")" + echo "${cls}" | grep -qE '^[a-z0-9-]+$' \ + || { echo "${f}: metadata.class must name a clojure-security class"; status=1; } + + exempt="$(yq -r '.rules[0].metadata.alias-exempt // ""' "${f}")" + if [ -z "${exempt}" ]; then + # A rule matching a namespaced fn must enumerate aliases, because semgrep + # cannot resolve them. Count distinct "(prefix/" tokens in the pattern lines; + # fewer than two means the rule only matches one spelling of the sink. + prefixes="$(grep -E '^\s+- pattern' -A0 "${f}" \ + | grep -oE '\(([a-zA-Z0-9._-]+)/' | sort -u | wc -l | tr -d ' ')" + [ "${prefixes}" -ge 2 ] \ + || { echo "${f}: enumerate at least 2 namespace aliases, or set metadata.alias-exempt with a reason"; status=1; } + fi done [ "${status}" -eq 0 ] && echo "all rules tagged" diff --git a/bin/test-rules.sh b/bin/test-rules.sh index 2a4367d..6fd7d1d 100644 --- a/bin/test-rules.sh +++ b/bin/test-rules.sh @@ -5,28 +5,37 @@ # behaviour). Both directions matter: a silently non-matching rule still appears # in the coverage matrix, which is exactly the false confidence this repo exists # to prevent. +# +# semgrep cannot resolve namespace aliases, so every rule enumerates the aliases +# it expects and every vulnerable fixture exercises more than one of them. That +# is the whole mitigation for choosing semgrep over clj-holmes — if it stops +# working, these tests must fail rather than the scan going quiet. set -euo pipefail ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" -RULES="${ROOT}/security-rules/clj-holmes" -FIXTURES="${ROOT}/test/fixtures" +RULES="${ROOT}/security-rules/semgrep" +FIXTURES="${ROOT}/spec-fixtures" EXPECTED="${FIXTURES}/expectations.tsv" WORK="$(mktemp -d)" trap 'rm -rf "${WORK}"' EXIT -command -v clj-holmes >/dev/null || { echo "clj-holmes not installed"; exit 1; } -command -v jq >/dev/null || { echo "jq not installed"; exit 1; } +command -v semgrep >/dev/null || { echo "semgrep not installed"; exit 1; } +command -v jq >/dev/null || { echo "jq not installed"; exit 1; } + +# semgrep namespaces SARIF ruleId with the config directory: "semgrep.cc-foo". +# Strip everything up to the last dot to get the bare rule id. +scan_to_tsv() { + local target="$1" out="$2" + semgrep scan --config "${RULES}" --no-git-ignore --sarif -q "${target}" \ + > "${out}.sarif" 2>/dev/null + jq -r '.runs[].results[] + | ((.ruleId | split(".") | last) + "\t" + + (.locations[0].physicalLocation.artifactLocation.uri + | split("/") | last))' "${out}.sarif" | sort -u +} # --- vulnerable corpus: every expected finding must appear ------------------- -clj-holmes scan -p "${FIXTURES}/vulnerable" -d "${RULES}" \ - --no-fail-on-result -t sarif -o "${WORK}/vuln.sarif" >/dev/null - -# SARIF: ruleId plus the basename of the file it fired on. -jq -r '.runs[].results[] - | .ruleId + "\t" + (.locations[0].physicalLocation.artifactLocation.uri - | split("/") | last)' \ - "${WORK}/vuln.sarif" | sort -u > "${WORK}/actual.tsv" - +scan_to_tsv "${FIXTURES}/vulnerable" "${WORK}/vuln" > "${WORK}/actual.tsv" grep -v '^#' "${EXPECTED}" | grep -v '^[[:space:]]*$' | sort -u > "${WORK}/expected.tsv" missing="$(comm -23 "${WORK}/expected.tsv" "${WORK}/actual.tsv")" @@ -41,12 +50,7 @@ if [ -n "${unexpected}" ]; then fi # --- safe corpus: must be completely clean ----------------------------------- -clj-holmes scan -p "${FIXTURES}/safe" -d "${RULES}" \ - --no-fail-on-result -t sarif -o "${WORK}/safe.sarif" >/dev/null - -safe_hits="$(jq -r '.runs[].results[] - | .ruleId + " in " + .locations[0].physicalLocation.artifactLocation.uri' \ - "${WORK}/safe.sarif")" +safe_hits="$(scan_to_tsv "${FIXTURES}/safe" "${WORK}/safe")" if [ -n "${safe_hits}" ]; then echo "FALSE POSITIVES on the safe corpus:"; echo "${safe_hits}"; status=1 fi diff --git a/docs/superpowers/plans/2026-07-27-security-workflow-cwe-owasp-coverage.md b/docs/superpowers/plans/2026-07-27-security-workflow-cwe-owasp-coverage.md index 80761fb..199344f 100644 --- a/docs/superpowers/plans/2026-07-27-security-workflow-cwe-owasp-coverage.md +++ b/docs/superpowers/plans/2026-07-27-security-workflow-cwe-owasp-coverage.md @@ -12,6 +12,39 @@ **Depends on:** Phase 1 (`cleancoders/agent-plugins`, plan `2026-07-27-clojure-security-cwe-owasp-coverage.md`) for the class-name and taxonomy vocabulary only — not for code. Phase 1 must be merged before Task 2, because rule `class-*` tags must match its class index. +## REVISION 2 — semgrep owns the custom rules + +Discovered while executing Task 3: **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 — and the parse failure is swallowed, so the scan +exits clean. Upstream's last real commit was October 2022, so there is nobody to +merge the two-line fix. See the spec's "Revision 2" section for the full record. + +**All 12 custom rules are semgrep rules.** clj-holmes stays in the pipeline with +upstream rules only. + +Amendments to the tasks below, which are otherwise unchanged: + +| item | was | now | +|---|---|---| +| rule directory | `security-rules/clj-holmes/` | `security-rules/semgrep/` | +| rule schema | clj-holmes shape-shifter YAML | semgrep YAML, `languages: [clojure]` | +| CWE/OWASP carrier | `properties.tags` strings | first-class `metadata.cwe` / `metadata.owasp` | +| alias handling | resolved automatically | **enumerate aliases per rule**, with an alias variant in the vulnerable fixture | +| `bin/test-rules.sh` | `clj-holmes scan -d` | `semgrep scan --config`, strip the `.` prefix semgrep adds to SARIF `ruleId` | +| `bin/check-rule-tags.sh` | reads `properties.tags` | reads `metadata.cwe` / `metadata.owasp` / `metadata.class` | +| `extra-rules-dir` | consumer clj-holmes rules | consumer semgrep rules | +| `holmes-ignored-paths` | clj-holmes only | renamed `ignored-paths`, applies to both engines | +| clj-holmes job | union 3 rule sources | upstream rules only; keep the pinned ref, binary install, and SARIF | +| Task 8 | "retarget semgrep as non-Clojure engine" | semgrep is now the **primary** engine; still add `--sarif` | + +Every rule task gains one step: **the vulnerable fixture must exercise at least two +different aliases for the same sink**, so alias blindness fails a test instead of +passing silently. This is the mitigation the engine choice depends on; without it +the decision is unsafe. + +Fixtures and `expectations.tsv` written in Tasks 1–2 carry over unchanged. + ## Global Constraints - **Reference editions:** CWE Top 25 (2025), OWASP Top 10:2025. Never cite 2021 or 2024. diff --git a/docs/superpowers/specs/2026-07-27-cwe-owasp-coverage-design.md b/docs/superpowers/specs/2026-07-27-cwe-owasp-coverage-design.md index 4970969..1484466 100644 --- a/docs/superpowers/specs/2026-07-27-cwe-owasp-coverage-design.md +++ b/docs/superpowers/specs/2026-07-27-cwe-owasp-coverage-design.md @@ -264,7 +264,65 @@ false alarms when upstream prunes a rule). It exists to catch *catastrophic* rul (zero, or a handful), not to assert an exact inventory. `bin/check-rule-tags.sh` asserts the cleancoders rules specifically. -### Why clj-holmes rather than semgrep for Clojure rules +### REVISION 2 (2026-07-27, during implementation): semgrep owns the custom rules + +The section below is superseded. It was written on an assumption verified during +Phase 2 implementation to be false. + +**What was wrong.** The engine choice was made on pattern-DSL expressiveness +(clj-holmes resolves namespace aliases; semgrep cannot). Nobody checked which +*files* clj-holmes reads. It reads only `.clj`: + +```clojure +;; clj_holmes/diplomat/code_reader.clj +(defn ^:private clj-file? [^File file] + (and (.isFile file) (-> file .toString (.endsWith ".clj")))) +``` + +`.cljs` and `.cljc` are skipped silently. Worse, `logic/reader.clj` calls edamame +without `:read-cond`, so a `.cljc` file renamed to `.clj` fails to parse — and +`code-str->code` catches the exception, prints, and returns `nil`. A repo of +`.cljc` therefore scans **clean**. + +That matters disproportionately 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. + +**Why not fix it upstream.** The fix is genuinely two lines (widen `clj-file?`, add +`:read-cond :allow :features` to the edamame opts). But clj-holmes' last real commit +was **October 2022**, with open PRs from 2022 and 2023. There is no maintainer to +merge it, and owning a GraalVM fork of a security-critical binary is a permanent +cost we declined. + +**Decision.** All 12 custom rules are **semgrep** rules. Verified during +implementation: + +| | `.clj` | `.cljs` | `.cljc` | ns aliases | maintained | +|---|---|---|---|---|---| +| clj-holmes | yes | no | no | yes | **no, since 2022** | +| semgrep | yes | yes | yes | **no** | yes | + +semgrep's alias blindness is mitigated by enumerating aliases per rule +(`hiccup.util/raw-string`, `hu/raw-string`, `html/raw-string` — all verified to +match) and, crucially, is **testable**: the fixture corpus includes alias variants, +so a miss shows up as a failing test rather than silence. clj-holmes' `.cljc` +blindness is silent and untestable without a fork. + +semgrep also carries first-class `metadata.cwe` / `metadata.owasp` fields, which +flow into SARIF as `properties.tags` — strictly better for the evidence trail than +clj-holmes' free-form `properties.tags`. + +**clj-holmes stays in the pipeline** running upstream rules only. It still hard-fails +on MD5, SHA-1, Blowfish, DESede, ECB, weak SSL context, insecure hostname verifiers, +XXE, and `read-string`. Those remain `.clj`-only, which the coverage matrix must say. + +**Consequences for the plan:** `bin/test-rules.sh` and `bin/check-rule-tags.sh` are +rewritten against semgrep's schema; rules live in `security-rules/semgrep/`; the +`extra-rules-dir` input feeds semgrep; `holmes-ignored-paths` becomes +`ignored-paths` and applies to both engines. Fixtures and `expectations.tsv` carry +over unchanged. Note semgrep prefixes SARIF `ruleId` with the config directory name +(`cleancoders.cc-hiccup-raw`), which the harness strips. + +### Why clj-holmes rather than semgrep for Clojure rules (SUPERSEDED — see Revision 2) clj-holmes rules support namespace-aware resolution: diff --git a/security-rules/clj-holmes/cc-hiccup-raw.yml b/security-rules/clj-holmes/cc-hiccup-raw.yml deleted file mode 100644 index 23cf1f6..0000000 --- a/security-rules/clj-holmes/cc-hiccup-raw.yml +++ /dev/null @@ -1,27 +0,0 @@ -- id: cc-hiccup-raw - name: Unescaped HTML via hiccup raw / raw-string - severity: error - message: >- - hiccup.util/raw-string and hiccup.core/raw bypass Hiccup's auto-escaping. If - the value is user-controlled this is stored or reflected XSS. Sanitize with - the OWASP Java HTML Sanitizer, or drop the raw call and let Hiccup escape. - Note this rule has no dataflow: it also fires on a raw call whose argument is - a compile-time constant, which is safe. Trace provenance before acting — see - the hiccup-injection class in the clojure-security skill. - properties: - precision: high - tags: - - security - - class-hiccup-injection - - cwe-79 - - owasp-a05-2025 - patterns: - - patterns-either: - - pattern: "($& $custom-function $&)" - namespace: hiccup.util - function: raw-string - custom-function?: true - - pattern: "($& $custom-function $&)" - namespace: hiccup.core - function: raw - custom-function?: true diff --git a/security-rules/clj-holmes/cc-sql-string-concat.yml b/security-rules/clj-holmes/cc-sql-string-concat.yml deleted file mode 100644 index e316742..0000000 --- a/security-rules/clj-holmes/cc-sql-string-concat.yml +++ /dev/null @@ -1,29 +0,0 @@ -- id: cc-sql-string-concat - name: SQL built by string concatenation - severity: error - message: >- - A jdbc execute call receives a (str ...)-built query. Use a parameterized - vector ["SELECT ... WHERE x = ?" v]. Where parameters cannot help — ORDER BY - columns, table names, dynamic WHERE fragments — allowlist the identifier - against a static map rather than interpolating it. - properties: - precision: medium - tags: - - security - - class-sql-injection - - cwe-89 - - owasp-a05-2025 - patterns: - - patterns-either: - - pattern: "($& $custom-function $& (str $&) $&)" - namespace: next.jdbc - function: execute! - custom-function?: true - - pattern: "($& $custom-function $& (str $&) $&)" - namespace: next.jdbc - function: execute-one! - custom-function?: true - - pattern: "($& $custom-function $& (str $&) $&)" - namespace: clojure.java.jdbc - function: query - custom-function?: true diff --git a/security-rules/semgrep/cc-cljs-eval.yaml b/security-rules/semgrep/cc-cljs-eval.yaml new file mode 100644 index 0000000..4129493 --- /dev/null +++ b/security-rules/semgrep/cc-cljs-eval.yaml @@ -0,0 +1,22 @@ +rules: + - id: cc-cljs-eval + languages: [clojure] + severity: ERROR + message: >- + js/eval and (js/Function. ...) execute arbitrary JavaScript. Never call either + on a runtime value. Dispatch through a hard-coded map keyed by a user-facing + string and reject unknown keys. + metadata: + cwe: ["CWE-94: Improper Control of Generation of Code ('Code Injection')"] + owasp: ["A05:2025 - Injection"] + class: cljs-dom-xss + confidence: HIGH + # js/ is ClojureScript's reserved host namespace. It cannot be aliased or + # required, so there is no second spelling to enumerate. + alias-exempt: "js/ is a reserved host namespace and cannot be aliased" + references: + - https://owasp.org/Top10/2025/A05_2025-Injection/ + pattern-either: + - pattern: (js/eval $X) + - pattern: (js/Function. ...) + - pattern: (new js/Function ...) diff --git a/security-rules/semgrep/cc-cljs-innerhtml.yaml b/security-rules/semgrep/cc-cljs-innerhtml.yaml new file mode 100644 index 0000000..ba2587a --- /dev/null +++ b/security-rules/semgrep/cc-cljs-innerhtml.yaml @@ -0,0 +1,23 @@ +rules: + - id: cc-cljs-innerhtml + languages: [clojure] + severity: ERROR + message: >- + Assigning to .-innerHTML (or dommy/set-html!) parses the value as HTML. If it + crosses a trust boundary this is DOM XSS. Use .-textContent, or sanitize with + DOMPurify first. + metadata: + cwe: ["CWE-79: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')"] + owasp: ["A05:2025 - Injection"] + class: cljs-dom-xss + confidence: HIGH + references: + - https://owasp.org/Top10/2025/A05_2025-Injection/ + pattern-either: + # Host interop — cannot be aliased. + - pattern: (set! (.-innerHTML $EL) $X) + - pattern: (set! (.-outerHTML $EL) $X) + # dommy CAN be aliased; enumerate the spellings seen in the cleancoders repos. + - pattern: (dommy.core/set-html! $EL $X) + - pattern: (dommy/set-html! $EL $X) + - pattern: (d/set-html! $EL $X) diff --git a/security-rules/semgrep/cc-dangerously-set-html.yaml b/security-rules/semgrep/cc-dangerously-set-html.yaml new file mode 100644 index 0000000..34b2e6d --- /dev/null +++ b/security-rules/semgrep/cc-dangerously-set-html.yaml @@ -0,0 +1,18 @@ +rules: + - id: cc-dangerously-set-html + languages: [clojure] + severity: ERROR + message: >- + :dangerouslySetInnerHTML bypasses Reagent's escaping. Render the value as a + string child, or sanitize with DOMPurify before setting __html. + metadata: + cwe: ["CWE-79: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')"] + owasp: ["A05:2025 - Injection"] + class: cljs-dom-xss + confidence: HIGH + # A bare keyword in a Reagent props map. Keywords have no namespace alias + # to vary, so there is no second spelling to enumerate. + alias-exempt: ":dangerouslySetInnerHTML is a keyword, not a namespaced call" + references: + - https://owasp.org/Top10/2025/A05_2025-Injection/ + pattern: ":dangerouslySetInnerHTML" diff --git a/security-rules/semgrep/cc-hiccup-raw.yaml b/security-rules/semgrep/cc-hiccup-raw.yaml new file mode 100644 index 0000000..bd26515 --- /dev/null +++ b/security-rules/semgrep/cc-hiccup-raw.yaml @@ -0,0 +1,28 @@ +rules: + - id: cc-hiccup-raw + languages: [clojure] + severity: ERROR + message: >- + hiccup.util/raw-string and hiccup.core/raw bypass Hiccup's auto-escaping. If + the value is user-controlled this is stored or reflected XSS. Sanitize with + the OWASP Java HTML Sanitizer, or drop the raw call and let Hiccup escape. + This rule has no dataflow: it also fires on a raw call whose argument is a + compile-time constant, which is safe. Trace provenance before acting — see + the hiccup-injection class in the clojure-security skill. + metadata: + cwe: ["CWE-79: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')"] + owasp: ["A05:2025 - Injection"] + class: hiccup-injection + confidence: HIGH + references: + - https://owasp.org/Top10/2025/A05_2025-Injection/ + pattern-either: + # semgrep cannot resolve namespace aliases, so enumerate them. Fully + # qualified first, then the aliases seen across the cleancoders repos. + - pattern: (hiccup.util/raw-string $X) + - pattern: (hu/raw-string $X) + - pattern: (html/raw-string $X) + - pattern: (util/raw-string $X) + - pattern: (hiccup.core/raw $X) + - pattern: (h/raw $X) + - pattern: (hiccup/raw $X) diff --git a/security-rules/semgrep/cc-sql-string-concat.yaml b/security-rules/semgrep/cc-sql-string-concat.yaml new file mode 100644 index 0000000..d316565 --- /dev/null +++ b/security-rules/semgrep/cc-sql-string-concat.yaml @@ -0,0 +1,29 @@ +rules: + - id: cc-sql-string-concat + languages: [clojure] + severity: ERROR + message: >- + A jdbc execute call receives a (str ...)-built query. Use a parameterized + vector ["SELECT ... WHERE x = ?" v]. Where parameters cannot help — ORDER BY + columns, table names, dynamic WHERE fragments — allowlist the identifier + against a static map rather than interpolating it. + metadata: + cwe: ["CWE-89: Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection')"] + owasp: ["A05:2025 - Injection"] + class: sql-injection + confidence: MEDIUM + references: + - https://owasp.org/Top10/2025/A05_2025-Injection/ + pattern-either: + # Aliases enumerated: semgrep cannot resolve them. next.jdbc is commonly + # aliased jdbc/sql/db; clojure.java.jdbc likewise. + - pattern: (next.jdbc/execute! $DB (str ...)) + - pattern: (jdbc/execute! $DB (str ...)) + - pattern: (sql/execute! $DB (str ...)) + - pattern: (db/execute! $DB (str ...)) + - pattern: (next.jdbc/execute-one! $DB (str ...)) + - pattern: (jdbc/execute-one! $DB (str ...)) + - pattern: (sql/execute-one! $DB (str ...)) + - pattern: (clojure.java.jdbc/query $DB (str ...)) + - pattern: (jdbc/query $DB (str ...)) + - pattern: (sql/query $DB (str ...)) diff --git a/test/fixtures/expectations.tsv b/spec-fixtures/expectations.tsv similarity index 60% rename from test/fixtures/expectations.tsv rename to spec-fixtures/expectations.tsv index 8d3dc47..74a19cf 100644 --- a/test/fixtures/expectations.tsv +++ b/spec-fixtures/expectations.tsv @@ -1,3 +1,6 @@ # — tab-separated. One line per expected finding. cc-hiccup-raw hiccup_raw.clj cc-sql-string-concat sql_injection.clj +cc-cljs-innerhtml cljs_xss.cljs +cc-cljs-eval cljs_xss.cljs +cc-dangerously-set-html cljs_xss.cljs diff --git a/spec-fixtures/safe/cljs_xss.cljs b/spec-fixtures/safe/cljs_xss.cljs new file mode 100644 index 0000000..cdd50df --- /dev/null +++ b/spec-fixtures/safe/cljs_xss.cljs @@ -0,0 +1,15 @@ +(ns fixtures.cljs-xss-safe) + +;; textContent escapes; no HTML parsing occurs. +(defn show-note [el note] + (set! (.-textContent el) note)) + +;; Reagent escapes string children by default. +(defn bio-panel [user] + [:div.bio (:bio user)]) + +;; Dispatch through a hard-coded map, never eval. +(def ^:private +actions+ {"greet" (fn [] "hi") "bye" (fn [] "bye")}) + +(defn run-action [k] + (when-let [f (get +actions+ k)] (f))) diff --git a/test/fixtures/safe/hiccup_raw.clj b/spec-fixtures/safe/hiccup_raw.clj similarity index 100% rename from test/fixtures/safe/hiccup_raw.clj rename to spec-fixtures/safe/hiccup_raw.clj diff --git a/test/fixtures/safe/sql_injection.clj b/spec-fixtures/safe/sql_injection.clj similarity index 100% rename from test/fixtures/safe/sql_injection.clj rename to spec-fixtures/safe/sql_injection.clj diff --git a/spec-fixtures/vulnerable/cljs_xss.cljs b/spec-fixtures/vulnerable/cljs_xss.cljs new file mode 100644 index 0000000..12ba213 --- /dev/null +++ b/spec-fixtures/vulnerable/cljs_xss.cljs @@ -0,0 +1,21 @@ +(ns fixtures.cljs-xss + (:require [dommy.core :as dommy])) + +(defn show-note [el note] + (set! (.-innerHTML el) note)) + +(defn show-via-dommy [el note] + (dommy/set-html! el note)) + +;; Second alias for the same sink — guards the rule's enumerated alias list. +(defn show-via-d [el note] + (d/set-html! el note)) + +(defn run-expr [expr] + (js/eval expr)) + +(defn make-fn [src] + ((js/Function. "x" src) 1)) + +(defn bio-panel [user] + [:div {:dangerouslySetInnerHTML #js {:__html (:bio user)}}]) diff --git a/spec-fixtures/vulnerable/hiccup_raw.clj b/spec-fixtures/vulnerable/hiccup_raw.clj new file mode 100644 index 0000000..cd591ef --- /dev/null +++ b/spec-fixtures/vulnerable/hiccup_raw.clj @@ -0,0 +1,17 @@ +(ns fixtures.hiccup-raw + (:require [hiccup.util :as hu] + [hiccup.core :as h])) + +;; Two DIFFERENT aliases for the same sink, on purpose. semgrep cannot resolve +;; namespace aliases, so the rule enumerates them explicitly. If someone trims +;; that list, one of these stops matching and bin/test-rules.sh fails — which is +;; the entire safety net for choosing semgrep over clj-holmes. +(defn render-bio [user] + [:div.bio (hu/raw-string (:bio user))]) + +(defn render-post [post] + [:article (h/raw (:body post))]) + +;; Fully-qualified, no alias at all. +(defn render-footer [site] + [:footer (hiccup.util/raw-string (:footer-html site))]) diff --git a/test/fixtures/vulnerable/sql_injection.clj b/spec-fixtures/vulnerable/sql_injection.clj similarity index 52% rename from test/fixtures/vulnerable/sql_injection.clj rename to spec-fixtures/vulnerable/sql_injection.clj index 5dbe6a1..cfa06de 100644 --- a/test/fixtures/vulnerable/sql_injection.clj +++ b/spec-fixtures/vulnerable/sql_injection.clj @@ -1,5 +1,6 @@ (ns fixtures.sql-injection - (:require [next.jdbc :as jdbc])) + (:require [next.jdbc :as jdbc] + [next.jdbc :as sql])) (defn find-user [db name] (jdbc/execute! db (str "SELECT * FROM users WHERE name = '" name "'"))) @@ -7,3 +8,7 @@ (defn list-sorted [db col] ;; The dynamic-identifier trap: parameters cannot help here. (jdbc/execute! db (str "SELECT * FROM posts ORDER BY " col))) + +;; A second alias for the same namespace — guards the enumerated alias list. +(defn count-by-type [db t] + (sql/execute-one! db (str "SELECT count(*) FROM events WHERE type = '" t "'"))) diff --git a/test/fixtures/vulnerable/hiccup_raw.clj b/test/fixtures/vulnerable/hiccup_raw.clj deleted file mode 100644 index a3c257b..0000000 --- a/test/fixtures/vulnerable/hiccup_raw.clj +++ /dev/null @@ -1,12 +0,0 @@ -(ns fixtures.hiccup-raw - (:require [hiccup.util :as hu] - [hiccup.core :as h])) - -;; Aliased call — the whole reason this repo uses clj-holmes rather than -;; semgrep. Semgrep's experimental Clojure tree-sitter cannot resolve `hu` back -;; to hiccup.util and would need one literal pattern per alias. -(defn render-bio [user] - [:div.bio (hu/raw-string (:bio user))]) - -(defn render-post [post] - [:article (h/raw (:body post))]) From 8d5096ecfe689bb7a68b8b127d2aa8421fff1b78 Mon Sep 17 00:00:00 2001 From: Alex Root-Roatch Date: Mon, 27 Jul 2026 18:25:37 -0500 Subject: [PATCH 04/14] feat(rules): detect shell and runtime-code-loading sinks 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. --- bin/check-rule-tags.sh | 7 +++++- security-rules/semgrep/cc-load-string.yaml | 25 ++++++++++++++++++++ security-rules/semgrep/cc-shell-exec.yaml | 27 ++++++++++++++++++++++ spec-fixtures/expectations.tsv | 2 ++ spec-fixtures/safe/dynamic_exec.clj | 11 +++++++++ spec-fixtures/vulnerable/dynamic_exec.clj | 16 +++++++++++++ 6 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 security-rules/semgrep/cc-load-string.yaml create mode 100644 security-rules/semgrep/cc-shell-exec.yaml create mode 100644 spec-fixtures/safe/dynamic_exec.clj create mode 100644 spec-fixtures/vulnerable/dynamic_exec.clj diff --git a/bin/check-rule-tags.sh b/bin/check-rule-tags.sh index e12938c..57d680b 100644 --- a/bin/check-rule-tags.sh +++ b/bin/check-rule-tags.sh @@ -26,7 +26,12 @@ for f in "${RULES}"/*.yaml; do [ "$(yq -r '.rules[0].languages | contains(["clojure"])' "${f}")" = "true" ] \ || { echo "${f}: must declare languages: [clojure]"; status=1; } - yq -r '.rules[0].metadata.cwe[]' "${f}" 2>/dev/null | grep -qE '^CWE-[0-9]+' \ + # Capture first, then match. `yq ... | grep -q` breaks under `set -o pipefail`: + # grep -q exits on the first match and closes the pipe, yq takes SIGPIPE, and + # pipefail reports the whole pipeline as failed. Only shows up on rules with + # more than one CWE entry, which makes it a nasty intermittent. + cwes="$(yq -r '.rules[0].metadata.cwe[]' "${f}" 2>/dev/null || true)" + echo "${cwes}" | grep -qE '^CWE-[0-9]+' \ || { echo "${f}: metadata.cwe must list at least one 'CWE-: ...' entry"; status=1; } owasp_count="$(yq -r '.rules[0].metadata.owasp[]' "${f}" 2>/dev/null \ diff --git a/security-rules/semgrep/cc-load-string.yaml b/security-rules/semgrep/cc-load-string.yaml new file mode 100644 index 0000000..864188a --- /dev/null +++ b/security-rules/semgrep/cc-load-string.yaml @@ -0,0 +1,25 @@ +rules: + - id: cc-load-string + languages: [clojure] + severity: ERROR + message: >- + load-string, load-file, and requiring-resolve on a constructed symbol compile + and run code chosen at runtime. Replace with a hard-coded map from a + user-facing key to a resolved var, and reject unknown keys with a 4xx. + metadata: + cwe: ["CWE-94: Improper Control of Generation of Code ('Code Injection')"] + owasp: ["A05:2025 - Injection"] + class: dynamic-eval + confidence: MEDIUM + # load-string / load-file / requiring-resolve / resolve are clojure.core, + # referred by default and effectively never alias-qualified in practice. + alias-exempt: "clojure.core fns referred by default; no alias to enumerate" + references: + - https://owasp.org/Top10/2025/A05_2025-Injection/ + pattern-either: + - pattern: (load-string ...) + - pattern: (load-file ...) + # A quoted symbol literal is safe and must NOT match; only a constructed + # (symbol ...) call is dangerous. + - pattern: (requiring-resolve (symbol ...)) + - pattern: (resolve (symbol ...)) diff --git a/security-rules/semgrep/cc-shell-exec.yaml b/security-rules/semgrep/cc-shell-exec.yaml new file mode 100644 index 0000000..42cb930 --- /dev/null +++ b/security-rules/semgrep/cc-shell-exec.yaml @@ -0,0 +1,27 @@ +rules: + - id: cc-shell-exec + languages: [clojure] + severity: ERROR + message: >- + clojure.java.shell/sh invoked with a "-c" flag runs its argument through a + shell interpreter, so any interpolated value is command injection. Pass a + fixed argv instead — (sh "convert" path "out.png") — so the OS never parses + user data as syntax. + metadata: + cwe: + - "CWE-78: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection')" + - "CWE-77: Improper Neutralization of Special Elements used in a Command ('Command Injection')" + owasp: ["A05:2025 - Injection"] + class: dynamic-eval + confidence: MEDIUM + references: + - https://owasp.org/Top10/2025/A05_2025-Injection/ + # Keys on the "-c" flag rather than enumerating every shell name: "-c" is + # what makes the interpreter parse the following string as a command line, + # and it is common to bash/sh/ksh/zsh alike. Aliases are still enumerated — + # semgrep cannot resolve them. + pattern-either: + - pattern: (clojure.java.shell/sh $SHELL "-c" ...) + - pattern: (sh/sh $SHELL "-c" ...) + - pattern: (shell/sh $SHELL "-c" ...) + - pattern: (sh $SHELL "-c" ...) diff --git a/spec-fixtures/expectations.tsv b/spec-fixtures/expectations.tsv index 74a19cf..bd5cab3 100644 --- a/spec-fixtures/expectations.tsv +++ b/spec-fixtures/expectations.tsv @@ -4,3 +4,5 @@ cc-sql-string-concat sql_injection.clj cc-cljs-innerhtml cljs_xss.cljs cc-cljs-eval cljs_xss.cljs cc-dangerously-set-html cljs_xss.cljs +cc-shell-exec dynamic_exec.clj +cc-load-string dynamic_exec.clj diff --git a/spec-fixtures/safe/dynamic_exec.clj b/spec-fixtures/safe/dynamic_exec.clj new file mode 100644 index 0000000..17b45ce --- /dev/null +++ b/spec-fixtures/safe/dynamic_exec.clj @@ -0,0 +1,11 @@ +(ns fixtures.dynamic-exec-safe + (:require [clojure.java.shell :as sh])) + +;; Fixed argv — no shell interprets the value, so no injection surface. +(defn convert [path] + (sh/sh "convert" path "out.png")) + +;; Static symbol literal, resolved once at load, never from runtime input. +(def ^:private report-fn (requiring-resolve 'clojure.string/upper-case)) + +(defn run-report [s] (report-fn s)) diff --git a/spec-fixtures/vulnerable/dynamic_exec.clj b/spec-fixtures/vulnerable/dynamic_exec.clj new file mode 100644 index 0000000..b38f667 --- /dev/null +++ b/spec-fixtures/vulnerable/dynamic_exec.clj @@ -0,0 +1,16 @@ +(ns fixtures.dynamic-exec + (:require [clojure.java.shell :as sh] + [clojure.java.shell :as shell])) + +(defn convert [path] + (sh/sh "bash" "-c" (str "convert " path " out.png"))) + +;; Second alias for the same sink — guards the enumerated alias list. +(defn thumbnail [path] + (shell/sh "sh" "-c" (str "convert -thumbnail 64 " path))) + +(defn run-report [ns-name fn-name] + ((requiring-resolve (symbol ns-name fn-name)))) + +(defn eval-rule [src] + (load-string src)) From a35b04e1d6486d0af0161cf1703883c59379b57e Mon Sep 17 00:00:00 2001 From: Alex Root-Roatch Date: Mon, 27 Jul 2026 18:26:36 -0500 Subject: [PATCH 05/14] feat(rules): add low-precision triage rules for traversal and fail-open 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. --- security-rules/semgrep/cc-generic-catch.yaml | 29 ++++++++++++++++++ security-rules/semgrep/cc-path-traversal.yaml | 30 +++++++++++++++++++ spec-fixtures/expectations.tsv | 2 ++ spec-fixtures/safe/triage_low.clj | 16 ++++++++++ spec-fixtures/vulnerable/triage_low.clj | 23 ++++++++++++++ 5 files changed, 100 insertions(+) create mode 100644 security-rules/semgrep/cc-generic-catch.yaml create mode 100644 security-rules/semgrep/cc-path-traversal.yaml create mode 100644 spec-fixtures/safe/triage_low.clj create mode 100644 spec-fixtures/vulnerable/triage_low.clj diff --git a/security-rules/semgrep/cc-generic-catch.yaml b/security-rules/semgrep/cc-generic-catch.yaml new file mode 100644 index 0000000..3fa1c00 --- /dev/null +++ b/security-rules/semgrep/cc-generic-catch.yaml @@ -0,0 +1,29 @@ +rules: + - id: cc-generic-catch + languages: [clojure] + severity: WARNING + message: >- + A catch of Exception/Throwable that returns true or nil. When the guarded + expression is a security decision this fails OPEN — the error path grants what + the success path would have denied. Catch narrowly, log, and return the + restrictive value. LOW PRECISION — many generic catches are legitimate; + triage with /security-audit before acting. + metadata: + cwe: + - "CWE-636: Not Failing Securely ('Failing Open')" + - "CWE-396: Declaration of Catch for Generic Exception" + owasp: ["A10:2025 - Mishandling of Exceptional Conditions"] + class: fail-open + confidence: LOW + # `catch` is a special form and Exception/Throwable are imported java.lang + # classes; neither has a namespace alias to vary. + alias-exempt: "catch is a special form; Exception/Throwable are java.lang" + references: + - https://owasp.org/Top10/2025/A10_2025-Mishandling_of_Exceptional_Conditions/ + # WARNING, not ERROR: plenty of generic catches are legitimate. This feeds + # triage rather than gating the build. + pattern-either: + - pattern: (catch Exception $E true) + - pattern: (catch Throwable $E true) + - pattern: (catch Exception $E nil) + - pattern: (catch Throwable $E nil) diff --git a/security-rules/semgrep/cc-path-traversal.yaml b/security-rules/semgrep/cc-path-traversal.yaml new file mode 100644 index 0000000..d5ab1f3 --- /dev/null +++ b/security-rules/semgrep/cc-path-traversal.yaml @@ -0,0 +1,30 @@ +rules: + - id: cc-path-traversal + languages: [clojure] + severity: WARNING + message: >- + A filesystem path built from a request map. If the value reaches this point + unvalidated, "../" escapes the intended directory. Canonicalize and assert the + prefix, or index a static allowlist map. LOW PRECISION — semgrep has no + dataflow, so provenance is not proven here; triage with /security-audit before + acting. + metadata: + cwe: ["CWE-22: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')"] + owasp: ["A01:2025 - Broken Access Control"] + class: path-traversal + confidence: LOW + references: + - https://owasp.org/Top10/2025/A01_2025-Broken_Access_Control/ + # WARNING, not ERROR: without dataflow this cannot be precise enough to gate + # a build. It exists to feed triage. Keys on the request-map accessor keys + # that actually carry filenames. + pattern-either: + - pattern: (clojure.java.io/file ... (:name $P)) + - pattern: (io/file ... (:name $P)) + - pattern: (jio/file ... (:name $P)) + - pattern: (clojure.java.io/file ... (:path $P)) + - pattern: (io/file ... (:path $P)) + - pattern: (jio/file ... (:path $P)) + - pattern: (clojure.java.io/file ... (:filename $P)) + - pattern: (io/file ... (:filename $P)) + - pattern: (jio/file ... (:filename $P)) diff --git a/spec-fixtures/expectations.tsv b/spec-fixtures/expectations.tsv index bd5cab3..9be3d3e 100644 --- a/spec-fixtures/expectations.tsv +++ b/spec-fixtures/expectations.tsv @@ -6,3 +6,5 @@ cc-cljs-eval cljs_xss.cljs cc-dangerously-set-html cljs_xss.cljs cc-shell-exec dynamic_exec.clj cc-load-string dynamic_exec.clj +cc-path-traversal triage_low.clj +cc-generic-catch triage_low.clj diff --git a/spec-fixtures/safe/triage_low.clj b/spec-fixtures/safe/triage_low.clj new file mode 100644 index 0000000..0d4fbe2 --- /dev/null +++ b/spec-fixtures/safe/triage_low.clj @@ -0,0 +1,16 @@ +(ns fixtures.triage-low-safe + (:require [clojure.java.io :as io] + [clojure.tools.logging :as log])) + +(def ^:private +docs+ {"terms" "terms.md" "privacy" "privacy.md"}) + +;; Allowlist lookup: no request text ever reaches the path. +(defn read-doc [k] + (when-let [f (get +docs+ k)] (slurp (io/resource (str "docs/" f))))) + +;; Fails CLOSED, narrow catch, logged. +(defn authorized? [user] + (try (check-permissions user) + (catch java.sql.SQLException e + (log/warn e "permission lookup failed") + false))) diff --git a/spec-fixtures/vulnerable/triage_low.clj b/spec-fixtures/vulnerable/triage_low.clj new file mode 100644 index 0000000..6c66610 --- /dev/null +++ b/spec-fixtures/vulnerable/triage_low.clj @@ -0,0 +1,23 @@ +(ns fixtures.triage-low + (:require [clojure.java.io :as io] + [clojure.java.io :as jio] + [clojure.tools.logging :as log])) + +(defn read-upload [params] + (slurp (io/file "uploads" (:name params)))) + +;; Second alias for the same sink — guards the enumerated alias list. +(defn read-attachment [params] + (slurp (jio/file "attachments" (:filename params)))) + +(defn authorized? [user] + ;; Fails OPEN: an exception in the permission lookup grants access. + (try (check-permissions user) + (catch Exception _ true))) + +(defn audit! [event] + ;; Swallows the failure; the caller reads nil as "no problem". + (try (write-audit-log! event) + (catch Exception _ nil))) + +(defn log-it [e] (log/warn e "ignored")) From ef3f978dfdbab1b85fd4875260b1836727328ec5 Mon Sep 17 00:00:00 2001 From: Alex Root-Roatch Date: Mon, 27 Jul 2026 18:27:17 -0500 Subject: [PATCH 06/14] feat(rules): detect nippy and SnakeYAML deserialization sinks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- security-rules/semgrep/cc-nippy-thaw.yaml | 21 +++++++++++++++++++ .../semgrep/cc-snakeyaml-unsafe.yaml | 20 ++++++++++++++++++ spec-fixtures/expectations.tsv | 2 ++ spec-fixtures/safe/deser.clj | 10 +++++++++ spec-fixtures/vulnerable/deser.clj | 14 +++++++++++++ 5 files changed, 67 insertions(+) create mode 100644 security-rules/semgrep/cc-nippy-thaw.yaml create mode 100644 security-rules/semgrep/cc-snakeyaml-unsafe.yaml create mode 100644 spec-fixtures/safe/deser.clj create mode 100644 spec-fixtures/vulnerable/deser.clj diff --git a/security-rules/semgrep/cc-nippy-thaw.yaml b/security-rules/semgrep/cc-nippy-thaw.yaml new file mode 100644 index 0000000..d864f2a --- /dev/null +++ b/security-rules/semgrep/cc-nippy-thaw.yaml @@ -0,0 +1,21 @@ +rules: + - id: cc-nippy-thaw + languages: [clojure] + severity: ERROR + message: >- + nippy/thaw on untrusted bytes without :incl-class-allowlist can instantiate + arbitrary classes, reaching JVM gadget chains. Pass an explicit allowlist, or + move to a transit/EDN envelope for anything crossing a trust boundary. + metadata: + cwe: ["CWE-502: Deserialization of Untrusted Data"] + owasp: ["A08:2025 - Software or Data Integrity Failures"] + class: java-deserialization + confidence: HIGH + references: + - https://owasp.org/Top10/2025/A08_2025-Software_or_Data_Integrity_Failures/ + # Single-argument arity only: the two-argument form carries an options map, + # which is where :incl-class-allowlist goes. Aliases enumerated. + pattern-either: + - pattern: (taoensso.nippy/thaw $B) + - pattern: (nippy/thaw $B) + - pattern: (np/thaw $B) diff --git a/security-rules/semgrep/cc-snakeyaml-unsafe.yaml b/security-rules/semgrep/cc-snakeyaml-unsafe.yaml new file mode 100644 index 0000000..f0e711b --- /dev/null +++ b/security-rules/semgrep/cc-snakeyaml-unsafe.yaml @@ -0,0 +1,20 @@ +rules: + - id: cc-snakeyaml-unsafe + languages: [clojure] + severity: ERROR + message: >- + The no-arg Yaml constructor deserializes arbitrary classes named in the + document. Use (Yaml. (SafeConstructor.)), or SnakeYAML 2.0+ where the safe + behaviour is the default. + metadata: + cwe: ["CWE-502: Deserialization of Untrusted Data"] + owasp: ["A08:2025 - Software or Data Integrity Failures"] + class: java-deserialization + confidence: HIGH + # Java interop on an imported class; there is no Clojure namespace alias. + alias-exempt: "Java interop constructor, not a namespaced Clojure call" + references: + - https://owasp.org/Top10/2025/A08_2025-Software_or_Data_Integrity_Failures/ + pattern-either: + - pattern: (Yaml.) + - pattern: (new Yaml) diff --git a/spec-fixtures/expectations.tsv b/spec-fixtures/expectations.tsv index 9be3d3e..a8a18bf 100644 --- a/spec-fixtures/expectations.tsv +++ b/spec-fixtures/expectations.tsv @@ -8,3 +8,5 @@ cc-shell-exec dynamic_exec.clj cc-load-string dynamic_exec.clj cc-path-traversal triage_low.clj cc-generic-catch triage_low.clj +cc-nippy-thaw deser.clj +cc-snakeyaml-unsafe deser.clj diff --git a/spec-fixtures/safe/deser.clj b/spec-fixtures/safe/deser.clj new file mode 100644 index 0000000..d6091ff --- /dev/null +++ b/spec-fixtures/safe/deser.clj @@ -0,0 +1,10 @@ +(ns fixtures.deser-safe + (:require [taoensso.nippy :as nippy]) + (:import [org.yaml.snakeyaml Yaml] + [org.yaml.snakeyaml.constructor SafeConstructor])) + +(defn load-session [^bytes b] + (nippy/thaw b {:incl-class-allowlist #{"clojure.lang.PersistentArrayMap"}})) + +(defn parse-config [s] + (.load (Yaml. (SafeConstructor.)) s)) diff --git a/spec-fixtures/vulnerable/deser.clj b/spec-fixtures/vulnerable/deser.clj new file mode 100644 index 0000000..8a4424a --- /dev/null +++ b/spec-fixtures/vulnerable/deser.clj @@ -0,0 +1,14 @@ +(ns fixtures.deser + (:require [taoensso.nippy :as nippy] + [taoensso.nippy :as np]) + (:import [org.yaml.snakeyaml Yaml])) + +(defn load-session [^bytes b] + (nippy/thaw b)) + +;; Second alias for the same sink — guards the enumerated alias list. +(defn load-cache [^bytes b] + (np/thaw b)) + +(defn parse-config [s] + (.load (Yaml.) s)) From c93c20ae904ebdc21f953fe83c8e595e24eb70c6 Mon Sep 17 00:00:00 2001 From: Alex Root-Roatch Date: Mon, 27 Jul 2026 18:27:59 -0500 Subject: [PATCH 07/14] feat(rules): detect spec/malli explain output in response bodies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../semgrep/cc-explain-data-response.yaml | 25 +++++++++++++++++++ spec-fixtures/expectations.tsv | 1 + spec-fixtures/safe/error_leak.clj | 15 +++++++++++ spec-fixtures/vulnerable/error_leak.clj | 12 +++++++++ 4 files changed, 53 insertions(+) create mode 100644 security-rules/semgrep/cc-explain-data-response.yaml create mode 100644 spec-fixtures/safe/error_leak.clj create mode 100644 spec-fixtures/vulnerable/error_leak.clj diff --git a/security-rules/semgrep/cc-explain-data-response.yaml b/security-rules/semgrep/cc-explain-data-response.yaml new file mode 100644 index 0000000..c271ae1 --- /dev/null +++ b/security-rules/semgrep/cc-explain-data-response.yaml @@ -0,0 +1,25 @@ +rules: + - id: cc-explain-data-response + languages: [clojure] + severity: ERROR + message: >- + s/explain-data and me/humanize embed the offending value, so returning them in + a response body dumps internal structures and likely PII to the client. Log the + detail server-side and return field names only. + metadata: + cwe: ["CWE-209: Generation of Error Message Containing Sensitive Information"] + owasp: ["A10:2025 - Mishandling of Exceptional Conditions"] + class: spec-malli-leak + confidence: MEDIUM + references: + - https://owasp.org/Top10/2025/A10_2025-Mishandling_of_Exceptional_Conditions/ + # Keys on the value landing in :body, not on the call itself — explaining + # server-side and returning field names is the correct pattern and must stay + # clean. Aliases enumerated for both spec and malli. + pattern-either: + - pattern: '{..., :body (clojure.spec.alpha/explain-data ...), ...}' + - pattern: '{..., :body (s/explain-data ...), ...}' + - pattern: '{..., :body (spec/explain-data ...), ...}' + - pattern: '{..., :body (malli.error/humanize ...), ...}' + - pattern: '{..., :body (me/humanize ...), ...}' + - pattern: '{..., :body (m/explain ...), ...}' diff --git a/spec-fixtures/expectations.tsv b/spec-fixtures/expectations.tsv index a8a18bf..9be2672 100644 --- a/spec-fixtures/expectations.tsv +++ b/spec-fixtures/expectations.tsv @@ -10,3 +10,4 @@ cc-path-traversal triage_low.clj cc-generic-catch triage_low.clj cc-nippy-thaw deser.clj cc-snakeyaml-unsafe deser.clj +cc-explain-data-response error_leak.clj diff --git a/spec-fixtures/safe/error_leak.clj b/spec-fixtures/safe/error_leak.clj new file mode 100644 index 0000000..5b9e3ad --- /dev/null +++ b/spec-fixtures/safe/error_leak.clj @@ -0,0 +1,15 @@ +(ns fixtures.error-leak-safe + (:require [clojure.spec.alpha :as s] + [clojure.tools.logging :as log])) + +(defn- field-names [ed] + (mapv #(-> % :path first name) (::s/problems ed))) + +;; explain-data IS called — but its output never reaches :body. Only field +;; names go back to the client; the detail is logged server-side. +(defn create-user [req] + (if (s/valid? ::user (:body req)) + {:status 201} + (let [ed (s/explain-data ::user (:body req))] + (log/warn "validation failed" {:fields (field-names ed)}) + {:status 400 :body {:errors (field-names ed)}}))) diff --git a/spec-fixtures/vulnerable/error_leak.clj b/spec-fixtures/vulnerable/error_leak.clj new file mode 100644 index 0000000..2728f47 --- /dev/null +++ b/spec-fixtures/vulnerable/error_leak.clj @@ -0,0 +1,12 @@ +(ns fixtures.error-leak + (:require [clojure.spec.alpha :as s] + [malli.error :as me])) + +(defn create-user [req] + (if (s/valid? ::user (:body req)) + {:status 201} + {:status 400 :body (s/explain-data ::user (:body req))})) + +;; Second namespace/alias for the same class of leak. +(defn update-user [_req errors] + {:status 400 :body (me/humanize errors)}) From 0f5d242a0b00449c138c69f54bf1806cb6269ad2 Mon Sep 17 00:00:00 2001 From: Alex Root-Roatch Date: Mon, 27 Jul 2026 18:31:11 -0500 Subject: [PATCH 08/14] feat: add actionlint and zizmor jobs, and fix what they found MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .github/workflows/security.yml | 95 ++++++++++++++++++++++++++++++++- .github/workflows/self-test.yml | 10 ++++ 2 files changed, 104 insertions(+), 1 deletion(-) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 2cc49a4..eb09afa 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -19,6 +19,13 @@ on: description: "Fail the workflow on semgrep findings (default: advisory only)" type: boolean default: false + zizmor-blocking: + description: >- + Fail the workflow on zizmor findings (default: advisory). Advisory by default + because zizmor's default settings light up existing repos; blocking on + adoption would wedge consumers. + type: boolean + default: false extra-rules-dir: description: "Consumer-supplied semgrep rules; added as an extra --config when the directory exists" type: string @@ -60,6 +67,8 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + with: + persist-credentials: false - name: Set up SSH for private git deps # Mirrors the consumer's build workflow: write the deploy key to # ~/.ssh/id_ed25519 so `clojure` can clone private git dependencies @@ -137,6 +146,8 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + 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 @@ -200,6 +211,8 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + with: + persist-credentials: false - name: Detect shell scripts id: detect shell: bash @@ -225,6 +238,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 with: + persist-credentials: false fetch-depth: 0 # full history — .gitleaksignore baselines the pre-redaction backlog - name: Install gitleaks shell: bash @@ -247,6 +261,8 @@ jobs: continue-on-error: ${{ !inputs.clj-watson-blocking }} steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + with: + persist-credentials: false - name: Set up SSH for private git deps # Mirrors the consumer's build workflow: write the deploy key to # ~/.ssh/id_ed25519 so `clojure` can clone private git dependencies @@ -317,9 +333,15 @@ jobs: # surfaces it without blocking; semgrep-blocking: true fails the build. continue-on-error: ${{ !inputs.semgrep-blocking }} container: - image: semgrep/semgrep + # 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). + image: semgrep/semgrep@sha256:98c2572fced2474539fd27cab3207ebd8e95e4e7aab4c3b381fdc5e2641d9941 # latest @ 2026-07-22 steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + with: + persist-credentials: false - name: Check out cleancoders detection rules # A reusable workflow cannot reference files from its own repo: `uses: ./` # resolves against the CALLER's checkout, and GitHub exposes no reliable @@ -330,6 +352,10 @@ jobs: repository: cleancoders/github-actions ref: ${{ inputs.rules-ref }} path: .cc-security-rules + # cleancoders/github-actions is public, so no credential is needed to + # read it — and not persisting one keeps it out of .git/config, which + # this job's SARIF artifact upload would otherwise be able to carry. + persist-credentials: false - name: Semgrep (cleancoders rules + OWASP Top 10 + default) shell: bash env: @@ -358,3 +384,70 @@ jobs: name: semgrep-sarif path: semgrep.sarif retention-days: 90 + + actionlint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + with: + persist-credentials: false + - name: Detect workflows + id: detect + shell: bash + run: | + set -euo pipefail + if [ -n "$(find .github/workflows -name '*.y*ml' 2>/dev/null | head -1)" ]; then + echo "run=true" >> "$GITHUB_OUTPUT" + else + echo "::notice::no .github/workflows; skipping actionlint" + echo "run=false" >> "$GITHUB_OUTPUT" + fi + - name: actionlint + # Hard-fails: this is workflow syntax and expression correctness, not + # opinion. Catches broken ${{ }} refs and shell bugs inside run: blocks. + if: steps.detect.outputs.run == 'true' + shell: bash + run: | + set -euo pipefail + VER=1.7.7 + curl -fsSL "https://github.com/rhysd/actionlint/releases/download/v${VER}/actionlint_${VER}_linux_amd64.tar.gz" \ + | sudo tar -xz -C /usr/local/bin actionlint + actionlint + + zizmor: + runs-on: ubuntu-latest + # Advisory by default: zizmor's default settings light up existing repos, so + # blocking on adoption would wedge every consumer on day one. Covers OWASP + # A02 Security Misconfiguration and A03 Supply Chain — CI config is itself + # attack surface, and this repo's entire product is workflows. + continue-on-error: ${{ !inputs.zizmor-blocking }} + steps: + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + with: + persist-credentials: false + - name: Detect workflows + id: detect + shell: bash + run: | + set -euo pipefail + if [ -n "$(find .github/workflows -name '*.y*ml' 2>/dev/null | head -1)" ]; then + echo "run=true" >> "$GITHUB_OUTPUT" + else + echo "::notice::no .github/workflows; skipping zizmor" + echo "run=false" >> "$GITHUB_OUTPUT" + fi + - uses: astral-sh/setup-uv@e92bafb6253dcd438e0484186d7669ea7a8ca1cc # v6.4.3 + if: steps.detect.outputs.run == 'true' + - name: zizmor + if: steps.detect.outputs.run == 'true' + shell: bash + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: uvx zizmor@1.11.0 --format sarif . > zizmor.sarif + - name: Upload SARIF + if: always() && steps.detect.outputs.run == 'true' + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: zizmor-sarif + path: zizmor.sarif + retention-days: 90 diff --git a/.github/workflows/self-test.yml b/.github/workflows/self-test.yml index bc8944a..94b7f36 100644 --- a/.github/workflows/self-test.yml +++ b/.github/workflows/self-test.yml @@ -3,6 +3,12 @@ on: pull_request: {} push: branches: [ master ] +# Least privilege by default. Reusable workflows called from here declare their +# own permissions; these apply to the jobs defined in this file. zizmor flags +# the absence as excessive-permissions. +permissions: + contents: read + jobs: security: uses: ./.github/workflows/security.yml @@ -37,6 +43,8 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 + with: + persist-credentials: false - name: Install semgrep shell: bash run: | @@ -57,6 +65,8 @@ jobs: working-directory: clj steps: - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 + with: + persist-credentials: false - uses: actions/setup-java@03ad4de0992f5dab5e18fcb136590ce7c4a0ac95 # v5 with: java-version: 21 From 5c937e4d9f0c3f926784e11bc4f18febfbe1d425 Mon Sep 17 00:00:00 2001 From: Alex Root-Roatch Date: Mon, 27 Jul 2026 18:32:27 -0500 Subject: [PATCH 09/14] feat: generate the scanner coverage matrix from rule metadata MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .github/workflows/self-test.yml | 14 +++++++ README.md | 28 +++++++++++++ bin/gen-coverage-matrix.sh | 71 +++++++++++++++++++++++++++++++++ 3 files changed, 113 insertions(+) create mode 100644 bin/gen-coverage-matrix.sh diff --git a/.github/workflows/self-test.yml b/.github/workflows/self-test.yml index 94b7f36..632955a 100644 --- a/.github/workflows/self-test.yml +++ b/.github/workflows/self-test.yml @@ -50,10 +50,24 @@ jobs: run: | set -euo pipefail python3 -m pip install --quiet semgrep + - name: Install yq + # check-rule-tags.sh and gen-coverage-matrix.sh both parse rule YAML. + # jq ships on the runner image; yq does not reliably, so pin it here + # rather than depending on whatever the image happens to carry. + shell: bash + run: | + set -euo pipefail + VER=4.44.3 + sudo curl -fsSL "https://github.com/mikefarah/yq/releases/download/v${VER}/yq_linux_amd64" \ + -o /usr/local/bin/yq + sudo chmod +x /usr/local/bin/yq + yq --version - name: Check rule tags run: bash bin/check-rule-tags.sh - name: Run rule fixture tests run: bash bin/test-rules.sh + - name: Check coverage table is current + run: bash bin/gen-coverage-matrix.sh --check clj-lib: # Guards the release library the c3kit repos consume as a git dep. Note the # working-directory: the library lives under clj/ so this repo keeps no diff --git a/README.md b/README.md index 4c68e93..53d93cf 100644 --- a/README.md +++ b/README.md @@ -36,6 +36,34 @@ jobs: | `clj-watson-blocking` | `false` | When `true`, clj-watson dependency-CVE findings fail the workflow. Default `false` = advisory (reported, never blocks). | | `semgrep-blocking` | `false` | When `true`, semgrep findings fail the workflow. Default `false` = advisory (reported, never blocks). | +### Coverage + +Scanner rows below are generated from rule metadata by +`bin/gen-coverage-matrix.sh` and checked in CI, so the table cannot claim a rule +that no longer exists. The manual-review rows — access control, insecure design, +and everything else no scanner reaches — live in the `clojure-security` plugin's +class index, because those two halves are maintained in different repos and each +is authoritative for its own. + + + +| rule | class | CWE | OWASP 2025 | blocking | +|------|-------|-----|------------|----------| +| `cc-cljs-eval` | `cljs-dom-xss` | 94 | A05 | yes | +| `cc-cljs-innerhtml` | `cljs-dom-xss` | 79 | A05 | yes | +| `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-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-shell-exec` | `dynamic-eval` | 78, 77 | A05 | yes | +| `cc-snakeyaml-unsafe` | `java-deserialization` | 502 | A08 | yes | +| `cc-sql-string-concat` | `sql-injection` | 89 | A05 | yes | + + + ### gitleaks Scans full history and honors a repo-local `.gitleaksignore`. Generate a baseline diff --git a/bin/gen-coverage-matrix.sh b/bin/gen-coverage-matrix.sh new file mode 100644 index 0000000..0cbf537 --- /dev/null +++ b/bin/gen-coverage-matrix.sh @@ -0,0 +1,71 @@ +#!/usr/bin/env bash +# Emits the scanner rows of the coverage matrix from rule metadata. 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. --check fails CI when the committed table no longer +# matches the rules on disk. +# +# gen-coverage-matrix.sh rewrite the table in README.md +# gen-coverage-matrix.sh --emit print the table to stdout +# gen-coverage-matrix.sh --check exit 1 if README.md is stale +set -euo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +RULES="${ROOT}/security-rules/semgrep" +README="${ROOT}/README.md" +BEGIN='' +END='' + +command -v yq >/dev/null || { echo "yq not installed"; exit 1; } + +generate() { + echo "${BEGIN}" + echo + echo "| rule | class | CWE | OWASP 2025 | blocking |" + echo "|------|-------|-----|------------|----------|" + for f in "${RULES}"/*.yaml; do + id="$(yq -r '.rules[0].id' "${f}")" + cls="$(yq -r '.rules[0].metadata.class' "${f}")" + sev="$(yq -r '.rules[0].severity' "${f}")" + # "CWE-79: Improper ..." -> "79"; join multiples with ", " + cwe="$(yq -r '.rules[0].metadata.cwe[]' "${f}" \ + | sed -E 's/^CWE-([0-9]+).*/\1/' | paste -sd', ' - | sed 's/,/, /g')" + # "A05:2025 - Injection" -> "A05" + owasp="$(yq -r '.rules[0].metadata.owasp[0]' "${f}" | sed -E 's/^(A[0-9]+):.*/\1/')" + # WARNING rules are triage-only by design and must not gate a build. + if [ "${sev}" = "WARNING" ]; then blocking="no (triage)"; else blocking="yes"; fi + echo "| \`${id}\` | \`${cls}\` | ${cwe} | ${owasp} | ${blocking} |" + done + echo + echo "${END}" +} + +case "${1:-}" in + --emit) + generate + ;; + --check) + current="$(sed -n "/${BEGIN}/,/${END}/p" "${README}")" + if [ "${current}" != "$(generate)" ]; then + echo "README coverage table is stale. Run: bash bin/gen-coverage-matrix.sh" + diff <(echo "${current}") <(generate) || true + exit 1 + fi + echo "coverage table current" + ;; + *) + tmp="$(mktemp)" + generate > "${tmp}" + python3 - "${README}" "${tmp}" <<'PY' +import re, sys +readme, table = sys.argv[1], sys.argv[2] +body = open(table).read().strip() +src = open(readme).read() +new = re.sub(r".*?", lambda _: body, + src, flags=re.S) +open(readme, "w").write(new) +PY + rm -f "${tmp}" + echo "README coverage table regenerated" + ;; +esac From dbd131e728a741151cc5e008bd308064416d0b51 Mon Sep 17 00:00:00 2001 From: Alex Root-Roatch Date: Mon, 27 Jul 2026 18:33:20 -0500 Subject: [PATCH 10/14] docs: document the new inputs, scanners, and coverage limits 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. --- README.md | 46 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 53d93cf..4cf8a45 100644 --- a/README.md +++ b/README.md @@ -4,10 +4,19 @@ Shared reusable GitHub Actions workflows for cleancoders repos. ## `security.yml` — reusable security-scan workflow -Runs six scanners. **Hard-fail** (block the caller): `clj-kondo`, `clj-holmes`, -`shellcheck`, `gitleaks`. **Advisory by default** (report, never block): -`clj-watson`, `semgrep` — each can be made blocking per-consumer via the -`clj-watson-blocking` / `semgrep-blocking` inputs. +Runs eight scanners. **Hard-fail** (block the caller): `clj-kondo`, `clj-holmes`, +`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). ### Usage @@ -35,6 +44,11 @@ jobs: | `shellcheck-dir` | `"./bin"` | shellcheck scandir. The job self-skips when the directory is absent or empty (e.g. library repos with no `bin/`). | | `clj-watson-blocking` | `false` | When `true`, clj-watson dependency-CVE findings fail the workflow. Default `false` = advisory (reported, never blocks). | | `semgrep-blocking` | `false` | When `true`, semgrep findings fail the workflow. Default `false` = advisory (reported, never blocks). | +| `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. | ### Coverage @@ -64,6 +78,30 @@ is authoritative for its own. +#### What this coverage does not claim + +A coverage table that overstates is worse than none, so: + +1. **No taint analysis anywhere.** Every scanned row is pattern matching. Neither + semgrep OSS nor clj-holmes tracks dataflow, so a sink reached by an unusual + path is missed. The table says "we look for this shape," not "we would catch + this bug." +2. **semgrep cannot resolve namespace aliases.** Each rule enumerates the aliases + it expects (`hu/`, `html/`, `hiccup.util/`, …). An unusual alias is a silent + 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. +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. +7. **Rules track `rules-ref`, default `v1`.** Consuming another ref without + setting it gets v1 rules. + ### gitleaks Scans full history and honors a repo-local `.gitleaksignore`. Generate a baseline From 0b33e4dd2a49a1b165366e2abff88a77c29769de Mon Sep 17 00:00:00 2001 From: Alex Root-Roatch Date: Mon, 27 Jul 2026 18:35:44 -0500 Subject: [PATCH 11/14] fix(rules): file cc-shell-exec under the command-injection class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- README.md | 2 +- security-rules/semgrep/cc-shell-exec.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 4cf8a45..cd37e39 100644 --- a/README.md +++ b/README.md @@ -72,7 +72,7 @@ is authoritative for its own. | `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-shell-exec` | `dynamic-eval` | 78, 77 | 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 | diff --git a/security-rules/semgrep/cc-shell-exec.yaml b/security-rules/semgrep/cc-shell-exec.yaml index 42cb930..65c4273 100644 --- a/security-rules/semgrep/cc-shell-exec.yaml +++ b/security-rules/semgrep/cc-shell-exec.yaml @@ -12,7 +12,7 @@ rules: - "CWE-78: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection')" - "CWE-77: Improper Neutralization of Special Elements used in a Command ('Command Injection')" owasp: ["A05:2025 - Injection"] - class: dynamic-eval + class: command-injection confidence: MEDIUM references: - https://owasp.org/Top10/2025/A05_2025-Injection/ From 24f96d13d1aa60ae4b9f2498ec42a2d9dcc1e04c Mon Sep 17 00:00:00 2001 From: Alex Root-Roatch Date: Tue, 28 Jul 2026 10:22:35 -0500 Subject: [PATCH 12/14] fix: drop vestigial pull-requests: write from security.yml MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .github/workflows/security.yml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index eb09afa..61629be 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -59,8 +59,16 @@ on: required: false 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 + # action-shellcheck annotates via workflow commands, which needs no permission. + # + # This is not cosmetic: a reusable workflow cannot request more than its caller + # grants, so a caller practising least privilege (`contents: read`) could not + # call this workflow at all while it asked for pull-requests: write. contents: read - pull-requests: write # allow scanners to post PR summary comments jobs: clj-kondo: From a3c0a09bbc6c642105c1f1da8786f520e6f00aa9 Mon Sep 17 00:00:00 2001 From: Alex Root-Roatch Date: Tue, 28 Jul 2026 10:26:29 -0500 Subject: [PATCH 13/14] fix: decide clj-holmes pass/fail ourselves instead of --fail-on-result 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. --- .github/workflows/security.yml | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 61629be..fb453d7 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -200,13 +200,32 @@ jobs: IGNORED: ${{ inputs.ignored-paths }} run: | set -euo pipefail - args=(scan -p . -d /tmp/rules --fail-on-result -t sarif -o clj-holmes.sarif) + # --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. + args=(scan -p . -d /tmp/rules --no-fail-on-result -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[@]}" + + 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@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 From d6addabc108b3a9b4691ed3c2b2fa4c1a03ae47f Mon Sep 17 00:00:00 2001 From: Alex Root-Roatch Date: Tue, 28 Jul 2026 11:06:59 -0500 Subject: [PATCH 14/14] refactor: one tested detect job; drop security-skips; clear Node 20 pins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .github/workflows/security.yml | 114 +++++++++++++++++--------------- .github/workflows/self-test.yml | 31 ++++----- bin/detect.sh | 65 ++++++++++++++++++ bin/test-detect.sh | 83 +++++++++++++++++++++++ 4 files changed, 221 insertions(+), 72 deletions(-) create mode 100644 bin/detect.sh create mode 100644 bin/test-detect.sh diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index fb453d7..36f4f46 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -71,10 +71,49 @@ permissions: contents: read jobs: + detect: + # One tested place for every self-skip decision. Each scanner must skip + # cleanly on a repo lacking the thing it scans, and a wrong answer means the + # job silently skips while the build stays green — indistinguishable from + # "scanned and found nothing". + # + # This used to be an inline `if [ -d ... ]` duplicated across jobs, which + # could not be tested and drifted between copies. bin/detect.sh is covered by + # bin/test-detect.sh (17 cases). + runs-on: ubuntu-latest + outputs: + has-shellcheck-target: ${{ steps.d.outputs.has-shellcheck-target }} + has-workflows: ${{ steps.d.outputs.has-workflows }} + has-extra-rules: ${{ steps.d.outputs.has-extra-rules }} + has-deps-edn: ${{ steps.d.outputs.has-deps-edn }} + has-src-dirs: ${{ steps.d.outputs.has-src-dirs }} + src-dirs: ${{ steps.d.outputs.src-dirs }} + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - name: Check out cleancoders toolkit + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + repository: cleancoders/github-actions + ref: ${{ inputs.rules-ref }} + path: .cc-security-rules + persist-credentials: false + - name: Detect what this repo has + id: d + shell: bash + env: + SHELLCHECK_DIR: ${{ inputs.shellcheck-dir }} + EXTRA_RULES_DIR: ${{ inputs.extra-rules-dir }} + SRC_PATHS: ${{ inputs.src-paths }} + run: | + set -euo pipefail + bash .cc-security-rules/bin/detect.sh outputs | tee -a "$GITHUB_OUTPUT" + clj-kondo: runs-on: ubuntu-latest steps: - - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false - name: Set up SSH for private git deps @@ -153,7 +192,7 @@ jobs: clj-holmes: runs-on: ubuntu-latest steps: - - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false - name: Install clj-holmes @@ -228,33 +267,21 @@ jobs: 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@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + 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' runs-on: ubuntu-latest steps: - - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false - - name: Detect shell scripts - id: detect - shell: bash - env: - SC_DIR: ${{ inputs.shellcheck-dir }} - run: | - set -euo pipefail - if [ -d "$SC_DIR" ] && [ -n "$(find "$SC_DIR" -type f 2>/dev/null | head -1)" ]; then - echo "run=true" >> "$GITHUB_OUTPUT" - else - echo "::notice::no files in '$SC_DIR'; skipping shellcheck" - echo "run=false" >> "$GITHUB_OUTPUT" - fi - name: ShellCheck - if: steps.detect.outputs.run == 'true' uses: ludeeus/action-shellcheck@00cae500b08a931fb5698e11e79bfbd38e612a38 # v2.0.0 with: scandir: ${{ inputs.shellcheck-dir }} @@ -263,7 +290,7 @@ jobs: gitleaks: runs-on: ubuntu-latest steps: - - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false fetch-depth: 0 # full history — .gitleaksignore baselines the pre-redaction backlog @@ -287,7 +314,7 @@ jobs: # propagates it and fails the build. continue-on-error: ${{ !inputs.clj-watson-blocking }} steps: - - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false - name: Set up SSH for private git deps @@ -366,7 +393,7 @@ jobs: # clj-holmes rules. zizmor flags this as unpinned-images (high). image: semgrep/semgrep@sha256:98c2572fced2474539fd27cab3207ebd8e95e4e7aab4c3b381fdc5e2641d9941 # latest @ 2026-07-22 steps: - - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false - name: Check out cleancoders detection rules @@ -374,7 +401,7 @@ jobs: # resolves against the CALLER's checkout, and GitHub exposes no reliable # "what ref am I running at" variable for reusable workflows. Hence an # explicit ref. A consumer on a non-v1 ref must set rules-ref to match. - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: repository: cleancoders/github-actions ref: ${{ inputs.rules-ref }} @@ -406,42 +433,34 @@ jobs: semgrep "${args[@]}" - name: Upload SARIF if: always() # evidence of a FAILING scan is the evidence most worth keeping - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 (node24) with: name: semgrep-sarif path: semgrep.sarif retention-days: 90 actionlint: + needs: detect + if: needs.detect.outputs.has-workflows == 'true' runs-on: ubuntu-latest steps: - - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false - - name: Detect workflows - id: detect - shell: bash - run: | - set -euo pipefail - if [ -n "$(find .github/workflows -name '*.y*ml' 2>/dev/null | head -1)" ]; then - echo "run=true" >> "$GITHUB_OUTPUT" - else - echo "::notice::no .github/workflows; skipping actionlint" - echo "run=false" >> "$GITHUB_OUTPUT" - fi - name: actionlint # Hard-fails: this is workflow syntax and expression correctness, not # opinion. Catches broken ${{ }} refs and shell bugs inside run: blocks. - if: steps.detect.outputs.run == 'true' shell: bash run: | set -euo pipefail - VER=1.7.7 + VER=1.7.12 curl -fsSL "https://github.com/rhysd/actionlint/releases/download/v${VER}/actionlint_${VER}_linux_amd64.tar.gz" \ | sudo tar -xz -C /usr/local/bin actionlint actionlint zizmor: + needs: detect + if: needs.detect.outputs.has-workflows == 'true' runs-on: ubuntu-latest # Advisory by default: zizmor's default settings light up existing repos, so # blocking on adoption would wedge every consumer on day one. Covers OWASP @@ -449,31 +468,18 @@ jobs: # attack surface, and this repo's entire product is workflows. continue-on-error: ${{ !inputs.zizmor-blocking }} steps: - - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false - - name: Detect workflows - id: detect - shell: bash - run: | - set -euo pipefail - if [ -n "$(find .github/workflows -name '*.y*ml' 2>/dev/null | head -1)" ]; then - echo "run=true" >> "$GITHUB_OUTPUT" - else - echo "::notice::no .github/workflows; skipping zizmor" - echo "run=false" >> "$GITHUB_OUTPUT" - fi - - uses: astral-sh/setup-uv@e92bafb6253dcd438e0484186d7669ea7a8ca1cc # v6.4.3 - if: steps.detect.outputs.run == 'true' + - uses: astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9 # v9.0.0 (node24) - name: zizmor - if: steps.detect.outputs.run == 'true' shell: bash env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: uvx zizmor@1.11.0 --format sarif . > zizmor.sarif - name: Upload SARIF - if: always() && steps.detect.outputs.run == 'true' - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 (node24) with: name: zizmor-sarif path: zizmor.sarif diff --git a/.github/workflows/self-test.yml b/.github/workflows/self-test.yml index 632955a..bf47329 100644 --- a/.github/workflows/self-test.yml +++ b/.github/workflows/self-test.yml @@ -19,30 +19,20 @@ jobs: # spec-fixtures/ holds deliberate vulnerabilities used as the detection # corpus; scanning them would fail this repo's own build. ignored-paths: "spec-fixtures" - # This repo has no src/ or deps.edn, so clj-kondo and clj-watson must SKIP - # gracefully. bin/ now exists (the rule tooling), so shellcheck runs here - # for real; its skip path is covered by security-skips below. + # This repo has no src/ or deps.edn, so clj-kondo and clj-watson exercise + # their skip paths here for real. bin/ exists (the rule tooling), so + # shellcheck runs. The skip predicates themselves are covered by + # bin/test-detect.sh in rule-tests, not by a duplicate workflow invocation. # No secrets passed: security.yml requires none (GITHUB_TOKEN is auto- # provided), so `secrets: inherit` would violate least privilege. - security-skips: - # Portability guard: every job must self-skip cleanly on a repo missing the - # thing it scans. bin/ now exists, so point shellcheck at a path that does - # not, and likewise for the consumer rules directory. - uses: ./.github/workflows/security.yml - with: - rules-ref: ${{ github.sha }} - ignored-paths: "spec-fixtures" - shellcheck-dir: "./no-such-dir" - extra-rules-dir: "./no-such-rules" - rule-tests: # Guards the cc-* detection rules themselves. Without this a rule could stop # matching and still appear in the README coverage matrix — the exact false # confidence this repo exists to prevent. runs-on: ubuntu-latest steps: - - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false - name: Install semgrep @@ -64,10 +54,15 @@ jobs: yq --version - name: Check rule tags run: bash bin/check-rule-tags.sh + - name: Run detect tests + # Replaces the old security-skips job, which duplicated all nine scanner + # jobs to exercise two conditionals. 17 cases, milliseconds. + run: bash bin/test-detect.sh - name: Run rule fixture tests run: bash bin/test-rules.sh - name: Check coverage table is current run: bash bin/gen-coverage-matrix.sh --check + clj-lib: # Guards the release library the c3kit repos consume as a git dep. Note the # working-directory: the library lives under clj/ so this repo keeps no @@ -78,14 +73,14 @@ jobs: run: working-directory: clj steps: - - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false - - uses: actions/setup-java@03ad4de0992f5dab5e18fcb136590ce7c4a0ac95 # v5 + - uses: actions/setup-java@0f481fcb613427c0f801b606911222b5b6f3083a # v5 with: java-version: 21 distribution: 'temurin' - - uses: DeLaGuardo/setup-clojure@3fe9b3ae632c6758d0b7757b0838606ef4287b08 # 13.4 + - uses: DeLaGuardo/setup-clojure@4c7a6f613e5089821bb3bb2a33a3ee115578580d # v13.6.1 (node24) with: cli: 'latest' - name: Install clj-kondo diff --git a/bin/detect.sh b/bin/detect.sh new file mode 100644 index 0000000..d6ac69f --- /dev/null +++ b/bin/detect.sh @@ -0,0 +1,65 @@ +#!/usr/bin/env bash +# Self-skip predicates for security.yml, in one tested place. +# +# Every scanner job must skip cleanly on a repo that lacks the thing it scans — +# a library with no bin/, a repo with no deps.edn. That logic used to be an +# inline `if [ -d ... ]` duplicated across jobs, which meant it could not be +# tested and drifted between copies. The `detect` job runs this once and exposes +# the answers as job outputs. +# +# Usage: +# detect.sh dir-has-files exit 0 if dir exists and holds >=1 file +# detect.sh existing-dirs print the subset of paths that are dirs +# detect.sh outputs emit key=value lines for GITHUB_OUTPUT +# +# `outputs` reads SHELLCHECK_DIR, EXTRA_RULES_DIR, and SRC_PATHS from the env so +# the workflow passes inputs in one place. +set -euo pipefail + +dir_has_files() { + local d="${1:-}" + [ -n "$d" ] || return 1 + [ -d "$d" ] || return 1 + [ -n "$(find "$d" -type f 2>/dev/null | head -1)" ] +} + +# Filters a space-separated list down to paths that exist as directories. Used +# for src-paths, where the default names three roots and most repos have one. +existing_dirs() { + local out=() + for p in "$@"; do + [ -d "$p" ] && out+=("$p") + done + [ ${#out[@]} -gt 0 ] && printf '%s\n' "${out[@]}" + return 0 +} + +emit_outputs() { + local sc="${SHELLCHECK_DIR:-}" extra="${EXTRA_RULES_DIR:-}" src="${SRC_PATHS:-}" + + if dir_has_files "$sc"; then echo "has-shellcheck-target=true" + else echo "has-shellcheck-target=false"; fi + + if dir_has_files ".github/workflows"; then echo "has-workflows=true" + else echo "has-workflows=false"; fi + + if [ -n "$extra" ] && [ -d "$extra" ]; then echo "has-extra-rules=true" + else echo "has-extra-rules=false"; fi + + if [ -f deps.edn ]; then echo "has-deps-edn=true" + else echo "has-deps-edn=false"; fi + + # shellcheck disable=SC2086 # deliberate word-splitting: src is a path list + local dirs + dirs="$(existing_dirs $src | paste -sd' ' - || true)" + echo "src-dirs=${dirs}" + if [ -n "$dirs" ]; then echo "has-src-dirs=true"; else echo "has-src-dirs=false"; fi +} + +case "${1:-}" in + dir-has-files) shift; dir_has_files "$@" ;; + existing-dirs) shift; existing_dirs "$@" ;; + outputs) emit_outputs ;; + *) echo "usage: detect.sh {dir-has-files |existing-dirs |outputs}" >&2 + exit 2 ;; +esac diff --git a/bin/test-detect.sh b/bin/test-detect.sh new file mode 100644 index 0000000..17b6cf6 --- /dev/null +++ b/bin/test-detect.sh @@ -0,0 +1,83 @@ +#!/usr/bin/env bash +# Tests bin/detect.sh. These predicates decide whether a scanner runs at all, so +# a wrong answer means a job silently skips and the build still goes green — +# indistinguishable from "scanned and found nothing". That is the failure mode +# this whole repo exists to prevent, which is why one-line predicates get tests. +# +# Replaces the old `security-skips` job in self-test.yml, which duplicated all 9 +# scanner jobs to exercise two conditionals. This covers more cases in +# milliseconds. +set -uo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +DETECT="${ROOT}/bin/detect.sh" +pass=0; fail=0 + +ok() { pass=$((pass+1)); } +bad() { fail=$((fail+1)); echo "FAIL: $1"; } +check(){ if [ "$2" = "$3" ]; then ok; else bad "$1 — expected '$3', got '$2'"; fi; } + +WORK="$(mktemp -d)" +trap 'rm -rf "${WORK}"' EXIT + +# --- dir-has-files ----------------------------------------------------------- +mkdir -p "${WORK}/empty" "${WORK}/full" "${WORK}/nested/deep" +touch "${WORK}/full/a.sh" +touch "${WORK}/nested/deep/b.sh" + +bash "${DETECT}" dir-has-files "${WORK}/full" && r=yes || r=no +check "dir with a file" "$r" "yes" +bash "${DETECT}" dir-has-files "${WORK}/empty" && r=yes || r=no +check "empty dir" "$r" "no" +bash "${DETECT}" dir-has-files "${WORK}/absent" && r=yes || r=no +check "absent dir" "$r" "no" +bash "${DETECT}" dir-has-files "" && r=yes || r=no +check "empty string" "$r" "no" +bash "${DETECT}" dir-has-files "${WORK}/nested" && r=yes || r=no +check "file only in subdirectory" "$r" "yes" + +# --- existing-dirs ----------------------------------------------------------- +mkdir -p "${WORK}/src/clj" "${WORK}/src/cljc" +cd "${WORK}" +r="$(bash "${DETECT}" existing-dirs src/clj src/cljs src/cljc | paste -sd' ' -)" +check "filters out the absent root" "$r" "src/clj src/cljc" +r="$(bash "${DETECT}" existing-dirs src/nope other/nope | paste -sd' ' -)" +check "all absent yields empty" "$r" "" + +# --- outputs ----------------------------------------------------------------- +# A library-shaped repo: no bin/, no deps.edn, no workflows, one source root. +mkdir -p "${WORK}/lib/src/clj" && cd "${WORK}/lib" +out="$(SHELLCHECK_DIR=./bin EXTRA_RULES_DIR=.security-rules \ + SRC_PATHS="src/clj src/cljs src/cljc" bash "${DETECT}" outputs)" +check "no bin/ -> shellcheck skips" \ + "$(echo "$out" | grep '^has-shellcheck-target=')" "has-shellcheck-target=false" +check "no workflows -> actionlint/zizmor skip" \ + "$(echo "$out" | grep '^has-workflows=')" "has-workflows=false" +check "no consumer rules dir" \ + "$(echo "$out" | grep '^has-extra-rules=')" "has-extra-rules=false" +check "no deps.edn -> clj-watson skips" \ + "$(echo "$out" | grep '^has-deps-edn=')" "has-deps-edn=false" +check "src-dirs narrowed to the one that exists" \ + "$(echo "$out" | grep '^src-dirs=')" "src-dirs=src/clj" + +# An app-shaped repo: everything present. +mkdir -p "${WORK}/app/bin" "${WORK}/app/.github/workflows" \ + "${WORK}/app/.security-rules" "${WORK}/app/src/cljs" +touch "${WORK}/app/bin/run.sh" "${WORK}/app/.github/workflows/ci.yml" \ + "${WORK}/app/deps.edn" +cd "${WORK}/app" +out="$(SHELLCHECK_DIR=./bin EXTRA_RULES_DIR=.security-rules \ + SRC_PATHS="src/clj src/cljs src/cljc" bash "${DETECT}" outputs)" +check "bin/ present -> shellcheck runs" \ + "$(echo "$out" | grep '^has-shellcheck-target=')" "has-shellcheck-target=true" +check "workflows present -> actionlint runs" \ + "$(echo "$out" | grep '^has-workflows=')" "has-workflows=true" +check "consumer rules dir found" \ + "$(echo "$out" | grep '^has-extra-rules=')" "has-extra-rules=true" +check "deps.edn present" \ + "$(echo "$out" | grep '^has-deps-edn=')" "has-deps-edn=true" +check "src-dirs narrowed to cljs" \ + "$(echo "$out" | grep '^src-dirs=')" "src-dirs=src/cljs" + +echo "detect tests: ${pass} passed, ${fail} failed" +[ "${fail}" -eq 0 ]