fix the allowlist and the guard, and add the nightly driver the sweep needs to run from cron

[fabrizzio-dotCMS]

Found by pointing the sweep at our own repository for the first time, which is the thing we are about to schedule nightly. Three defects, one root cause: a check whose own inputs are hand-maintained.

1. Two indicators were still defined inline

REGEX_PATS in polinrider_scan.sh held helloipbot and the quoted header key directly, while taking the RPC host list from rules.sh. #6 migrated one and missed two. The shell-history probe was a literal in the script as well.

All three move to rules.sh.

2. The guard that exists to catch that had a hardcoded list

test_rules_single_source.sh searched the scripts for four literals, written out by hand. The two above were never in that list, so the guard passed while two indicators sat in a second place.

A guard with a hand-maintained list of what to guard is the same drift it exists to prevent, one level up. It now derives the list from rules.sh — every signature set, the build marker, the whitespace rule, the history probe — so a rule added tomorrow is covered without anyone remembering to edit the test.

Verified in the direction that matters: reinstating the inline REGEX_PATS makes the new guard fail. The old one passed.

3. The allowlist was exempting the wrong files

entry	carries an indicator today?
org_sweep.sh	no — sources rules.sh since #6
polinrider_scan.sh	no — same
rules.sh	yes — and was not listed
canaries.sh	yes — and was not listed

Two consequences. The nightly sweep would have reported our own repo INFECTED every night. And two paths were exempt for no reason — an implant injected into the scanner itself would have been suppressed.

Narrowed rather than widened: the comments in those two scripts quoted the literals verbatim, which is a hit like any other, since the sweep greps content and not code. They now describe the indicators and point at canaries.sh for the exact bytes.

sweep-allowlist.example becomes sweep-allowlist, shipped rather than templated. Which files are exempt from malware detection is a security decision and belongs in a reviewed diff, not in editable config.

4. The allowlist now has a ref dimension

Which is what the above ran into. refs/pull/1..3 froze the pre-#6 versions of both scripts, and pull-request refs are read-only — we cannot rewrite or delete them, so they match for as long as the repo exists. Tidying the current files cannot reach them.

Without a ref scope, the only way to stop that firing nightly is to exempt the path on every ref — which also suppresses an implant injected into the scanner on main. So entries may now be scoped:

owner/repo:path-glob              any ref, as before
owner/repo@ref-glob:path-glob     only matching refs

The exemption covers the bytes we cannot change; refs/heads/* stays covered. The frozen past and the living present are not the same hole — which is the incident's own structure, in miniature, on our own tooling.

Tests

test_org_sweep.sh gains five assertions: a refs/pull/* entry must not suppress on a branch, a matching scope must suppress, an entry with no ref part keeps meaning any ref, and a suppressed finding is still printed. An invisible hole is exactly what the allowlist's own header warns about.

All five suites pass — 22 and 26 assertions plus three qualitative.

Note on scope

This is the fourth time in this repo that a check has turned out to reassure without checking. The pattern each time: something derived by hand from something else, and no test on the derivation. That is why the fix here is the guard's input, not just the two files it missed.

---

Update: three more defects, found by running it for real

The sweep was pointed at the actual repositories for the first time. None of what follows could have been found any other way — no fixture has a wide README.

99% of the findings were markdown tables

Total findings	4,159
.md — pipe-aligned tables padding cells to 209 spaces	4,122
The real carriers, for comparison	507 spaces

Since any finding marks a repository INFECTED, dotCMS/core would have been reported infected every single night. A report that is never clean is a report nobody reads, and then somebody switches it off — worse than the rule not existing.

The whitespace rule no longer runs over .md, .txt, .rst, .csv, .svg and friends. Not a blind spot: the three literal signatures and the build marker still scan every file, so a carrier hidden in a .md is still caught by content. What is skipped is a shape hint in formats where the shape carries no information.

Confirmed live against dotCMS/core with the fix in place: 0 markdown hits, down from 4,122.

A shape hint is not a signature

The 37 hits left on core are one file — a 1.6 MB vendored bundle — across 37 refs, with no signature and no marker anywhere in it. Scott's brief warned about this class for line-length heuristics; it came back through whitespace.

A signature, the build marker, or config.bat added to .gitignore are the implant. A long whitespace run is how the implant hides — valuable, and the reason human review missed this campaign, but on its own it is a shape that vendored build output also has. Counted apart now; shape-only reports as SUSPICIOUS.

Nothing is silenced: findings still printed, repository still not called clean, and anything carrying both — which is what every real carrier does — is still INFECTED.

The trade, stated plainly: a future variant with no known signature, showing only the shape, would report SUSPICIOUS rather than INFECTED. Still reported. Against 4,000 nightly false positives, which kill the control outright, that is the better failure.

An empty repository is not an unreadable one

Three of ours are genuinely empty, 0 KB, and came back UNKNOWN — "not proven", which drives exit 2. Every nightly run would have carried three permanent warnings and a permanent failure for repositories with nothing in them. A clone that fails stays UNKNOWN; zero refs after a clone that succeeded is now EMPTY, counted separately, not affecting the exit code.

Tests

Eight new assertions, each verified failing against the unfixed sweep. 39 in this suite; all five suites green.

Sizing note for the schedule

49 small repositories took 3 minutes. dotCMS/core alone has 21,289 refs, 18,314 of them pull-request refs, and ran for over four hours. The cost is two or three large repositories, not the 217 — a single nightly job over everything is not viable as designed.

[sfreudenthaler]

Requesting changes. The allowlist work (#3), the ref-scope dimension (#4) and the markdown/whitespace split are right, and the derived single-source guard is a real improvement — I verified it fails when the inline REGEX_PATS is reinstated. All five suites pass here (22 + 39 + three qualitative).

But two of the changes reopen the exact failure this repo exists to close: a scan that reports nothing wrong while the check did not run. Both are reproducible.

---

Blocking 1 — nightly_sweep.sh discards the sweep's exit code, so an engine that refuses to run reports a clean night

nightly_sweep.sh:184

GH_TOKEN="$TOKEN" bash "$SWEEP" --allowlist "$ALLOWLIST" --repos "${chunk[*]}" 2>&1 \
  | tee -a "$WORK/log"

set -uo pipefail is on but -e is not, and the pipeline's status is never read. The entire report is then built by grepping ^RESULT lines out of $WORK/log — so a batch that produces no RESULT lines is indistinguishable from a batch that found nothing.

org_sweep.sh exits 2 and prints ABORT with no RESULT lines for: engine self-test failure, canaries not loading, missing rules.sh, no token, allowlist not found, unknown option. Every one of those is the sweep correctly refusing to certify anything — and the driver converts it into a green run.

Reproduced with a stub org_sweep.sh that aborts on every batch:

batch 1: 2 repositories
ABORT engine self-test failed - refusing to report anything as clean
batch 2: 2 repositories
ABORT engine self-test failed - refusing to report anything as clean
SWEEP COMPLETE: 4 scanned · 0 no ref hits · 0 empty · 0 shape-only · 0 infected · 0 not scanned · 0 known · 0s
>>> NIGHTLY EXIT=0
>>> heartbeat: {"last_success":"...","scanned":4,"took_seconds":0}

Zero repositories were scanned. The run notifies :white_check_mark: clean run, exits 0, and writes the success heartbeat — the artefact monitoring uses to decide the control is alive.

The partial case is worse because it is the likely one (token, disk, network blip on one batch):

batch 1: 2 repositories
RESULT dotCMS/r1 NO_REF_HITS 0
RESULT dotCMS/r2 NO_REF_HITS 0
batch 2: 2 repositories
ABORT cannot create workdir /tmp/org_sweep
SWEEP COMPLETE: 4 scanned · 2 no ref hits · ... · 0 not scanned
>>> NIGHTLY EXIT=0

Half the org never looked at, reported as 4 scanned, exit 0, clean. Note $N_SCAN in the summary is the count of repos in scope, not the count scanned — nothing ever reconciles the two.

Why checking $? is not enough

org_sweep.sh overloads exit 2: usage/engine failure and "at least one repo came back UNKNOWN" (org_sweep.sh:383). Treating rc=2 as fatal would fire on ordinary UNKNOWNs, which the driver already handles correctly from the RESULT lines. So the fix has to be coverage reconciliation, per batch:

before=$(grep -c '^RESULT ' "$WORK/log" || true)
GH_TOKEN="$TOKEN" bash "$SWEEP" --allowlist "$ALLOWLIST" --repos "${chunk[*]}" 2>&1 \
  | tee -a "$WORK/log"
after=$(grep -c '^RESULT ' "$WORK/log" || true)
[ $((after - before)) -eq ${#chunk[@]} ] \
  || die "batch $n: ${#chunk[@]} repositories sent, $((after - before)) reported - the sweep did not run to completion"

That catches the abort, a mid-batch kill (OOM, cron timeout), and truncated output, and die already gives exit 2 plus the broken notification, which is the documented contract. The summary should also print the reconciled count rather than $N_SCAN, and the heartbeat should not be written unless coverage reconciles.

---

Blocking 2 — the new EMPTY verdict is assigned without checking that ref enumeration succeeded

org_sweep.sh:228-249

git for-each-ref --format='%(refname)' > "$revfile" 2>/dev/null
nrefs=$(wc -l < "$revfile" | tr -d ' ')
if [ "$nrefs" -eq 0 ]; then ... EMPTY ...

The exit status of git for-each-ref (and git rev-list --all under --deep) is discarded along with its stderr. EMPTY is inferred purely from an empty file, and EMPTY is documented as not affecting the exit code.

The comment argues "a clone that fails has its own path above and remains UNKNOWN" — true for git clone itself, but it leaves the case where the clone succeeds and the enumeration fails: corrupt or truncated mirror, disk full while writing $revfile, a git failure in the mirror.

$ org_sweep.sh --local /tmp/notarepo        # not a git repository at all
SKIP  notarepo: clone succeeded and contained no refs - empty repository, nothing to scan
RESULT notarepo EMPTY 0
EXIT=0

$ org_sweep.sh --local /tmp/corrupt.git     # bare repo with refs/ and packed-refs removed
RESULT corrupt.git EMPTY 0
EXIT=0

On main the same corrupt repository gives EXIT=2. This PR turns "could not be read" into "nothing to read" — the one substitution the header of this file forbids.

Fix — separate the two facts, keep the legitimate win (three genuinely empty repos stop being UNKNOWN):

if [ "$DEEP" = 1 ]; then
  git rev-list --all > "$revfile" 2>/dev/null; enum_rc=$?
else
  git for-each-ref --format='%(refname)' > "$revfile" 2>/dev/null; enum_rc=$?
fi
if [ "$enum_rc" -ne 0 ]; then
  skip "$label: ref enumeration FAILED (rc=$enum_rc) - this repo is NOT proven clean"
  rm -f "$revfile"
  printf 'RESULT %s UNKNOWN 0\n' "$label"; printf 'UNKNOWN\n' >> "$VERDICTS"; return
fi
nrefs=$(wc -l < "$revfile" | tr -d ' ')
if [ "$nrefs" -eq 0 ]; then ... EMPTY ...

grep_revs already models this correctly — rc 1 is "no match", rc>1 is "did not run, NOT proven clean". The enumeration should use the same distinction.

---

Also, not blocking the merge

3. Nothing tests nightly_sweep.sh. 250 new lines, the component that decides whether a human hears about a finding, and no suite touches it. The PR's own closing note is that this is the fourth time a check has reassured without checking; the driver deserves the same fixture treatment. A stub org_sweep.sh that aborts, asserting exit 2 and no heartbeat, would have caught Blocking 1. Please also add an assertion that a corrupt repo is UNKNOWN, not EMPTY — the suite is at 39 and none of them cover this.

4. The sizing note contradicts the schedule this driver implements. Measured here: dotCMS/core is 21,289 refs and 4+ hours. The driver mints one token per batch of 30 (:179), and installation tokens live an hour — a batch containing core outlives its own credential by 4x, and every repo after core in that batch fails to clone and lands as UNKNOWN. The comment at :176 anticipates this ("a long run starts failing halfway through — as UNKNOWN, correctly") but the per-batch refresh does not cover the case you measured. Either refresh per repo, put oversized repos in a batch of one, or split core onto its own schedule. As shipped, the nightly job does not complete a nightly cycle.

Happy to re-review as soon as 1 and 2 are addressed — the rest of this is good work and I'd like it in.

[fabrizzio-dotCMS]

Both blocking findings are fixed in 1eb53b8, along with items 3 and 4.

1 — the driver discarded the exit code. Each batch now reconciles the RESULT lines it produced against the repositories it was sent, as you suggested, rather than reading $? — for the reason you gave, that exit 2 is overloaded. A shortfall dies with exit 2 before any heartbeat is written. The summary and the heartbeat now carry the reconciled count instead of $N_SCAN; you were right that nothing reconciled the two, and they were never the same number.

2 — EMPTY was inferred without checking enumeration ran. git for-each-ref / git rev-list now keep their exit status and return UNKNOWN when it is non-zero, the distinction grep_revs already draws. A genuinely empty repository still reports EMPTY at exit 0. I also corrected the comment above that block: it claimed the two cases stayed distinguished, which was only true for a failed clone.

3 — nothing tested the driver. test_nightly_sweep.sh, 21 assertions. It stubs the GitHub API on localhost so the real token-minting and repository-listing paths execute, and every case asserts on the heartbeat as well as the exit code. 12 of the 21 fail against this commit's parent, including your exact repro: 4 scanned, exit 0, heartbeat written, zero repositories examined. test_org_sweep.sh gains the corrupt-mirror, non-repository and genuinely-empty cases; 5 fail against the parent. All six suites green.

One of my assertions was initially vacuous — it grepped for clean run, which with no webhook configured is absent either way, so it passed against the bug. Replaced with an assertion on the summary, which does distinguish. Worth naming given the history in this repo.

4 — the token outlives nothing; the batch outlives the token. Oversized repositories now run one per batch, last, via SOLO_REPOS (default dotCMS/core).

Your three suggested remedies are not equivalent, and the reason changes which one is correct: the token is needed for exactly one step, the mirror clone. After that the scan is git grep on local disk and needs no credential. So a batch of one does not protect core from its own four and a half hours — it does not need protecting, its clone completes in the first minutes while the token is fresh. What breaks is whatever clones behind it on an hour-old credential. Isolating it addresses precisely that, and per-repo token refresh would spend 217 engine self-tests to solve a problem only one repository has. A normal batch is also truncated so it cannot spill into the isolated tail — my first attempt did exactly that and re-paired core with the repository after it. The test caught it.

---

One thing on #10, which I read while checking for interaction: when I trial-merged it before pushing this, it was clean. It is not any more. We both append a case to the end of test_org_sweep.sh, so they now conflict, and we both numbered our new case == 10.. Trivial to resolve — keep both, yours becomes 13 — but whoever merges second inherits it, and the duplicate number survives a careless resolution.

No interaction beyond that: the --orgs guard permits --repos, which is all the driver ever passes.

Also worth saying: that guard is the same defect as both of your blocking findings — a run that examines nothing and exits green. Three instances, three places, found independently.

[sfreudenthaler]

thanks and sorry for the delay in getting the re-review in.