Threatcrush triage

.github/workflows/security-gate.yml

@@ -0,0 +1,105 @@
+# Blocking security gate.
+#
+# Separate from threatcrush-scan.yml on purpose: that file is managed by the
+# sh1pt Actions Fleet and carries a content hash, so local edits are liable to
+# be overwritten on the next pack update. It reports findings; this decides
+# whether they stop the merge.
+#
+# This started out as a reviewed-baseline gate, because ThreatCrush 0.3.0
+# reported 56 findings here and all 56 were false positives — six of them
+# high-severity, so `--fail-on high` would have blocked every pull request.
+# Those were rule bugs rather than anything in this repository, and they are
+# fixed in 0.4.0 (profullstack/threatcrush#76): static `innerHTML` assignments,
+# short escaper aliases like `esc()`, `searchParams.set` counted as untrusted
+# input, and test fixtures read as live credentials. This repository now has
+# zero high-severity findings, so the plain gate does the job and ~200 lines of
+# baseline machinery went away with it.
+#
+# The remaining findings are medium and deliberately do not block: they are
+# `innerHTML` sinks in multi-line templates whose interpolations are escaped on
+# a different line than the assignment, which a line-oriented scanner cannot
+# see.
+#
+# Know what this does not catch. Severity depends on whether the scanner can
+# see the taint source near the sink, so the same XSS is rated differently
+# depending on how the code is arranged:
+#
+#     innerHTML = '<b>' + new URL(location).searchParams.get('q') + '</b>'  high, blocks
+#     innerHTML = '<b>' + q + '</b>'   // q is a parameter                  medium, does not
+#
+# Both were measured. The gate therefore catches a vulnerability written in
+# one place and misses one whose source sits in another function. Raising it to
+# `--fail-on medium` would close that gap and currently costs 31 false
+# positives; the honest position is that this gate is a floor, not a proof.
+name: security gate
+
+on:
+  push:
+    branches: [main]
+  pull_request:
+
+permissions:
+  contents: read
+
+jobs:
+  gate:
+    name: no high-severity findings
+    runs-on: ubuntu-latest
+    timeout-minutes: 15
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: actions/setup-node@v4
+        with:
+          node-version: "20"
+
+      # Same retry shape as the scan workflow: a registry blip is not a
+      # security signal and must not read like one.
+      - name: Install ThreatCrush
+        run: |
+          for attempt in 1 2 3; do
+            if npm install -g "@profullstack/threatcrush@latest"; then
+              exit 0
+            fi
+            delay=$((attempt * 10))
+            echo "::warning::ThreatCrush install attempt ${attempt}/3 failed; retrying in ${delay}s"
+            sleep "${delay}"
+          done
+          echo "::error::ThreatCrush install failed after 3 attempts"
+          exit 1
+
+      - name: Record the version that ran
+        run: threatcrush --version
+
+      # Fails closed in both directions. `--fail-on high` exits 1 when a
+      # high-or-critical finding exists; any other non-zero exit is a scan that
+      # did not complete, and an unexamined diff is not a clean one — so the
+      # step fails either way rather than treating "no output" as "no findings".
+      - name: Gate on high-severity findings
+        run: |
+          set -o pipefail
+          code=0
+          threatcrush scan . --fail-on high --format json --output threatcrush.json || code=$?
+
+          if [ ! -s threatcrush.json ]; then
+            echo "::error::ThreatCrush produced no findings file (exit ${code}) — this diff was NOT scanned"
+            exit 1
+          fi
+
+          total=$(python3 -c "import json;d=json.load(open('threatcrush.json'));print(len(d['findings'] if isinstance(d,dict) else d))")
+          echo "ThreatCrush: ${total} finding(s), exit ${code}." >> "$GITHUB_STEP_SUMMARY"
+
+          if [ "$code" -ne 0 ]; then
+            echo "::error::high-severity finding(s) present — see the Security tab or the artifact below"
+            exit "$code"
+          fi
+
+      - name: Upload findings
+        if: always()
+        uses: actions/upload-artifact@v4
+        with:
+          name: threatcrush-findings
+          path: threatcrush.json
+          retention-days: 30
+          if-no-files-found: warn

README.md

@@ -479,6 +479,70 @@ bun test          # deterministic unit tests (indicators, parsing)
 bun run typecheck # tsc --noEmit
 ```
 
+## Security scanning
+
+Every pull request is scanned by ThreatCrush. Two workflows are involved and
+they do different jobs:
+
+- **`threatcrush-scan.yml`** reports findings and uploads SARIF to the Security
+  tab. It is managed by the sh1pt Actions Fleet and carries a content hash, so
+  do not edit it locally — a pack update will overwrite it.
+- **`security-gate.yml`** decides whether findings stop the merge. It runs
+  `threatcrush scan . --fail-on high`, so a high-or-critical finding fails the
+  build.
+
+It fails closed in both directions: a scan that produces no findings file is
+reported as *not scanned* rather than as clean, because an unexamined diff is
+not a clean one.
+
+### Why the gate is `high` and not `medium`
+
+The scan reported **56 findings here, all 56 false positives**, six of them
+high-severity — which made any `--fail-on` setting unusable, since it would
+have blocked every pull request. Triaging them showed the fault was in the
+rules rather than in this repository, and the fixes shipped in ThreatCrush
+0.4.0 ([threatcrush#76](https://github.com/profullstack/threatcrush/pull/76)):
+
+- static `innerHTML` assignments reported as XSS
+- the escaper guard matching `escapeHtml(` but not `esc(`, so the code that
+  escapes most rigorously was reported most often
+- `searchParams` counted as untrusted input even when *writing* an outbound
+  URL, which fired the SSRF rule on constant hosts
+- credentials in test fixtures treated as live
+
+That took this repository to **zero high-severity findings**, so the plain gate
+now works and the reviewed-baseline machinery it replaced (~200 lines) is gone.
+
+The findings that remain are medium and deliberately do not block. They are
+`innerHTML` sinks inside multi-line templates whose interpolations *are*
+escaped, just on a different line from the assignment — which a line-oriented
+scanner cannot see.
+
+### What this gate does not catch
+
+Severity depends on whether the scanner can see the taint source near the sink,
+so the *same* vulnerability is rated differently depending on how the code is
+arranged. Both of these were measured against this repository:
+
+| shape | severity | blocks? |
+| --- | --- | --- |
+| `innerHTML = '<b>' + new URL(location).searchParams.get('q') + '</b>'` | high | yes |
+| `innerHTML = '<b>' + q + '</b>'`, where `q` is a parameter | medium | **no** |
+
+So the gate stops a vulnerability written in one place and misses one whose
+source sits in another function. `--fail-on medium` would close the gap and
+today costs 31 false positives, which is why it is not set. Treat this as a
+floor, not a proof — it is not a substitute for review.
+
+To silence a finding you have established is safe, use the scanner's own
+directive on the line above it, with the rule named so a *different* rule
+firing there still surfaces:
+
+```js
+// threatcrush-disable-next-line js-unescaped-html-sink
+el.innerHTML = template;
+```
+
 ## Compliance
 
 Every ranking includes: