Skip to content

fix: clj-holmes crash on large repos; make semgrep findings visible - #5

Merged
arootroatch merged 1 commit into
masterfrom
fix-scanner-output
Jul 28, 2026
Merged

fix: clj-holmes crash on large repos; make semgrep findings visible#5
arootroatch merged 1 commit into
masterfrom
fix-scanner-output

Conversation

@arootroatch

Copy link
Copy Markdown
Collaborator

Two regressions from replacing clj-holmes-action, both surfaced by the first
real consumer run (cleancoders.com run 30379331916).

1. clj-holmes crashed and blocked a production deploy

IllegalArgumentException: Value out of range for int: 35050732419
  progrock.core$interval_str -> clj_holmes.logic.progress

Its progress bar overflows an int computing an ETA. Exit 255, no SARIF written
at all
. Silent on small repos, fatal on ~1300 Clojure files. Those nonsense
ETA: 15823642:26 values in every clj-holmes log were this bug not yet tipping
over.

The stock clj-holmes-action always passed --no-verbose; dropping that flag
when replacing the action reintroduced it. With the flag, the same scan on
cleancoders.com exits 0 with 0 findings — so a progress-bar bug, not a
security finding, blocked a deploy.

Also guards a missing SARIF explicitly rather than letting jq fail obscurely
on a file that was never created.

2. semgrep findings were invisible

The job went red and the log said, in full:

✅ Scan completed successfully.
 • Findings: 5 (5 blocking)
Ran 290 rules on 1391 files: 5 findings.
##[error]Process completed with exit code 1.

Five findings, no indication what or where. With --sarif --output the detail
goes to the file, so --error gave a red job and an unactionable log — the same
defect fixed for clj-holmes in the previous PR and missed for semgrep in the
same commit.

Now prints severity, rule, file and line, and computes the exit code here:

  [error] detected-jwt-token  spec/clj/cleancoders/jwplayer_spec.clj:37
  [error] detected-generic-secret  src/clj/cleancoders/config.clj:118
  [warning] cc-generic-catch  src/clj/cleancoders/db_browser/engine.clj:58
  [warning] cc-generic-catch  src/clj/cleancoders/mfa.clj:218
  [error] cc-cljs-innerhtml  src/cljs/cleancoders/contact.cljs:224
semgrep: 7 finding(s), 3 blocking

Only error-level findings fail the job. Severity lives on the rule
(tool.driver.rules[].defaultConfiguration.level), not the result — semgrep
leaves results[].level null — so the jq joins them. cc-path-traversal and
cc-generic-catch are WARNING on purpose and the README promises they do not
block; counting every result would have broken that promise. cleancoders.com has
7 findings but only 3 blocking.

Verification

Reproduced both against cleancoders.com locally: without --no-verbose, exit 255
and no SARIF; with it, exit 0 and a valid report. The new jq was validated
against the actual SARIF artifact from the failing run.

v1 needs retagging after merge — cleancoders.com deploys stay blocked until it
moves.

🤖 Generated with Claude Code

Both found by the first real consumer run (cleancoders.com), and both are
regressions from replacing clj-holmes-action in this branch.

clj-holmes crashed with exit 255 and wrote no SARIF at all:

  IllegalArgumentException: Value out of range for int: 35050732419
    progrock.core$interval_str -> clj_holmes.logic.progress

Its progress bar overflows an int computing an ETA. Silent on small repos,
fatal on ~1300 Clojure files. The stock action always passed --no-verbose;
dropping that flag reintroduced the bug. With it, the same scan exits 0 and
reports 0 findings — so a progress-bar bug was blocking a production deploy.
Also guards a missing SARIF explicitly instead of letting jq fail obscurely.

semgrep found 5 blocking issues and the log said only 'Findings: 5'. With
--sarif --output the detail goes to the file, so --error produced a red job
with no indication of what or where. Now the findings print with severity,
rule, file and line, and the exit code is computed here.

Only error-level findings fail the job. Severity lives on the rule
(tool.driver.rules[].defaultConfiguration.level), not the result, so the jq
joins them — cc-path-traversal and cc-generic-catch are WARNING on purpose and
the README promises they do not block. Counting every result would have broken
that promise: cleancoders.com has 7 findings but only 3 blocking.
@arootroatch
arootroatch merged commit 3802385 into master Jul 28, 2026
11 checks passed
@arootroatch
arootroatch deleted the fix-scanner-output branch July 28, 2026 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant