Close the split-token and reflection bypasses, and self-test the guard

.github/workflows/ci.yml

@@ -49,6 +49,12 @@ jobs:
       - name: Verify release changelogs exist for this versionCode
         run: ./scripts/check-release-changelogs.sh
 
+      # Assert the guard's rules before trusting them. A member access split
+      # after a trailing dot, and a class name assembled by concatenation, both
+      # walked past the scanner and nothing would have noticed.
+      - name: Self-test the RNG hygiene guard
+        run: ./scripts/test-rng-hygiene.sh
+
       - name: Check for silent RNG fallbacks and GCM IV reuse
         run: ./scripts/check-rng-hygiene.sh
 

scripts/check-rng-hygiene.sh

@@ -124,21 +124,28 @@ preprocess() {
       }
       function emit() {
         if (buf != "" && !bufopt) printf "%s:%d:%s\n", fname, bufline, buf
-        buf = ""; bufopt = 0; open = 0
+        buf = ""; bufopt = 0; open = 0; dotcont = 0
       }
 
-      BEGIN { inblock = 0; instr = 0; blockopt = 0; buf = ""; open = 0; bufopt = 0 }
+      BEGIN { inblock = 0; instr = 0; blockopt = 0; buf = ""; open = 0; bufopt = 0; dotcont = 0 }
       {
         code = trim(strip($0))
         marked = index(cmt, optout); cmt = ""
 
         if (code == "") { if (marked) blockopt = 1; next }
 
-        if (open > 0) {
-          buf = buf " " code
+        # A line ending in `.` continues a member access: Kotlin accepts
+        # `Math.` on one line and `random()` on the next as the same call, and a
+        # per-physical-line matcher sees neither half. Buffered like an
+        # unbalanced paren so the rules match the joined expression.
+        if (open > 0 || dotcont) {
+          # Join directly after a trailing dot: `Math.` + ` ` + `random()` would
+          # reassemble as `Math. random()`, which no longer matches `Math[.]random`.
+          buf = buf (dotcont ? "" : " ") code
           if (marked && !keepoptout) bufopt = 1
           open += count(code, "(") - count(code, ")")
-          if (open <= 0) emit()
+          dotcont = (code ~ /\.[ \t]*$/)
+          if (open <= 0 && !dotcont) emit()
           next
         }
 
@@ -147,7 +154,8 @@ preprocess() {
 
         buf = code; bufline = FNR; bufopt = 0
         open = count(code, "(") - count(code, ")")
-        if (open <= 0) emit()
+        dotcont = (code ~ /\.[ \t]*$/)
+        if (open <= 0 && !dotcont) emit()
       }
       END { if (open > 0) emit() }
     ' "$f") || rc=2
@@ -191,9 +199,16 @@ scan() { # $1 = ERE, matched against code with string literals blanked
 }
 
 scan_with_strings() { # $1 = ERE, matched against code with literals intact
+  # Concatenation is collapsed before matching: `"java.util." + "Random"` builds
+  # the same class name a literal match would catch, and Class.forName reaches
+  # the same PRNG. Reported against the raw line so output shows what was
+  # written, not the normalised form.
   printf '%s\n' "$CODE_STR" | awk -v pat="$1" '{
+    raw = $0
     line = $0; sub(/^[^:]*:[0-9]+:/, "", line)
-    if (line ~ pat) print $0
+    norm = line
+    gsub(/"[ \t]*\+[ \t]*"/, "", norm)
+    if (line ~ pat || norm ~ pat) print raw
   }' || scanner_died
 }
 
@@ -216,6 +231,16 @@ report() { # $1 = findings, $2 = headline, $3.. = hints
 # refactor away.
 # The `(^|[^a-zA-Z0-9_.])` guard deliberately does NOT apply to the fully
 # qualified names: `java.util.concurrent.ThreadLocalRandom.current()` has a `.`
+# ------------------------------ reflection reaches a PRNG by name, not by token ----
+# `Class.forName("java.util." + "Random")` builds the banned name at runtime, so
+# no literal match can see it, and reflection can reach any generator this file
+# forbids by spelling. There is no legitimate use in this codebase today, so the
+# rule is: don't. A marker documents it if that ever changes.
+reflect_bad=$(scan '(Class[.]forName|[.]loadClass)[ \t]*[(]')
+report "$reflect_bad" "loads a class by name, which can reach a banned generator without naming it:" \
+  'construct the type directly so the rules above can see which one it is.' \
+  "Mark a deliberate use: // $OPT_OUT - <reason>"
+
 # in front of the token, so anchoring it would make qualified use invisible.
 weak_rng=$(scan 'kotlin[.]random|java[.]util[.]Random|Math[.]random|ThreadLocalRandom|SplittableRandom|(^|[^a-zA-Z0-9_.])(Random[(]|Random[.]next)|[.]random[(][)]|SecureRandom[(][^)]')
 report "$weak_rng" "non-cryptographic randomness in production code:" \

scripts/test-rng-hygiene.sh

@@ -0,0 +1,130 @@
+#!/usr/bin/env bash
+# Self-test for check-rng-hygiene.sh.
+#
+# This guard keeps non-CSPRNG generators, weakened SecureRandom and
+# caller-supplied GCM IVs out of production Kotlin and Java. A scanner that
+# quietly stops scanning reports a clean tree exactly like a clean tree does, so
+# the rules are asserted rather than trusted.
+#
+# Probes go under app/src/main/kotlin: the guard deliberately skips test,
+# androidTest and testFixtures, so a probe placed there passes and looks like a
+# bypass when it is only a misplaced probe. The positive control catches that.
+#
+# Each reject case requires the guard to NAME the probe file. Without that, a
+# guard aborting for an unrelated reason would be credited as a detection and
+# every case below would pass while detecting nothing.
+#
+# Probes are staged into a throwaway GIT_INDEX_FILE, so the real index is never
+# touched. The file itself must exist on disk while the guard runs, because the
+# scanner reads bytes; it is removed on every path including the EXIT trap.
+set -uo pipefail
+
+cd "$(dirname "$0")/.." || { echo "FAIL: cannot cd to the repo root"; exit 1; }
+GUARD=scripts/check-rng-hygiene.sh
+[ -x "$GUARD" ] || { echo "FAIL: $GUARD not found or not executable"; exit 1; }
+
+TMPD=$(mktemp -d)
+PROBE=""
+cleanup() { rm -rf "$TMPD"; [ -n "$PROBE" ] && rm -f "$PROBE"; }
+trap cleanup EXIT
+
+fails=0
+
+# run_probe <path> <content> <pass|fail> <description>
+run_probe() {
+    local name="$1" content="$2" expect="$3" desc="$4"
+
+    if [ -e "$name" ]; then
+        echo "  HARNESS BROKEN: $name exists; refusing to overwrite a real file"
+        fails=$((fails + 1)); return
+    fi
+    PROBE="$name"
+    printf '%s' "$content" > "$name"
+
+    rm -f "$TMPD/index"
+    GIT_INDEX_FILE="$TMPD/index" git read-tree HEAD 2>/dev/null
+    GIT_INDEX_FILE="$TMPD/index" git add -f "$name" 2>/dev/null
+
+    # Both that the tree is populated AND that this probe is in it. Discarding
+    # the git errors above means a failed `git add` would otherwise leave the
+    # guard scanning a probe-free tree, and the case would be judged on a file
+    # the guard never saw.
+    local staged
+    staged=$(GIT_INDEX_FILE="$TMPD/index" git ls-files | wc -l)
+    if [ "$staged" -lt 10 ]; then
+        echo "  HARNESS BROKEN: only $staged file(s) staged; the guard would scan almost nothing"
+        fails=$((fails + 1)); rm -f "$name"; PROBE=""; return
+    fi
+    if ! GIT_INDEX_FILE="$TMPD/index" git ls-files --error-unmatch "$name" >/dev/null 2>&1; then
+        echo "  HARNESS BROKEN: $name was not staged; the guard would never see it"
+        fails=$((fails + 1)); rm -f "$name"; PROBE=""; return
+    fi
+
+    local rc=0 out
+    out=$(GIT_INDEX_FILE="$TMPD/index" "$GUARD" 2>&1) || rc=$?
+    rm -f "$name"; PROBE=""
+
+    if [ "$expect" = fail ]; then
+        if [ "$rc" -eq 0 ]; then
+            echo "  BYPASS: $desc"; fails=$((fails + 1))
+        elif ! printf '%s' "$out" | grep -qF "$name"; then
+            echo "  WRONG REASON: $desc (guard failed without naming $name)"; fails=$((fails + 1))
+        else
+            echo "  ok: $desc"
+        fi
+    else
+        if [ "$rc" -ne 0 ]; then
+            echo "  FALSE POSITIVE: $desc"
+            printf '%s\n' "$out" | sed 's/^/      /' | head -4
+            fails=$((fails + 1))
+        else
+            echo "  ok: $desc"
+        fi
+    fi
+}
+
+echo "== rejects what it must reject =="
+
+SRC=app/src/main/kotlin/io/privkey/keep
+
+run_probe $SRC/ProbeCtl.kt 'val x = Math.random()
+' fail "Math.random() (positive control: if this passes, nothing below means anything)"
+
+run_probe $SRC/ProbeSplit.kt 'val x = Math.
+random()
+' fail "member access split after a trailing dot"
+
+run_probe $SRC/ProbeSplit2.kt 'val r = java.util.
+Random()
+' fail "qualified name split after the package dot"
+
+run_probe $SRC/ProbeReflect.kt 'val c = Class.forName("java.util." + "Random")
+' fail "reflection with a concatenated class name"
+
+run_probe $SRC/ProbeLoad.kt 'val c = javaClass.classLoader.loadClass("java.util.Random")
+' fail "loadClass reaching a generator by name"
+
+run_probe $SRC/ProbeSeed.kt 'val r = java.security.SecureRandom(); r.setSeed(1L)
+' fail "setSeed weakening SecureRandom"
+
+run_probe $SRC/ProbeTlr.kt 'val x = ThreadLocalRandom.current().nextInt()
+' fail "ThreadLocalRandom"
+
+echo "== accepts what it must accept =="
+
+run_probe $SRC/ProbeClean.kt 'val x = 1
+' pass "ordinary code"
+
+run_probe $SRC/ProbeComment.kt '// Math.random() is named here in prose only
+val x = 1
+' pass "a banned token inside a comment is not code"
+
+run_probe $SRC/ProbeTest.kt 'val x = 1
+' pass "ordinary code in the production tree"
+
+echo
+if [ "$fails" -ne 0 ]; then
+    echo "FAIL: $fails case(s) did not behave as required"
+    exit 1
+fi
+echo "OK: check-rng-hygiene.sh rejects every known bypass and accepts sanctioned use"