ci(semgrep): the project SAST gate scanned two directories, not the project (BACKLOG #334)

.github/workflows/security.yml

@@ -408,9 +408,49 @@ jobs:
           # (1.172.0 is what `~=1.90` resolved to on 2026-07-29, so this pin is a no-op today.)
           python -m pip install --upgrade pip "semgrep==1.172.0"
           # --error: any finding fails the step — and the job, now that this gate is blocking.
-          # Includes `tee/` (in-tree vendored SOUP) so the standalone relay is held to the same
-          # dangerous-sink rules as the engine.
-          semgrep --config .semgrep --error --metrics off messagefoundry tee
+          #
+          # SCOPE = the whole repo minus the excludes below, which is the bandit exclude set in this
+          # same file EXACTLY. It was the allow-list `messagefoundry tee`, which never covered
+          # scripts/ (the security tooling itself), messagefoundry_webconsole/ (the operator UI) or
+          # docker/ — 59 tracked .py files that the sibling bandit gate already scans, held to none of
+          # this project's own dangerous-sink rules. An allow-list cannot be kept in step with "the
+          # project" by hand; `.` minus explicit excludes cannot go stale when a package is added.
+          # `tee/` (in-tree vendored SOUP) stays in scope for the reason it always was: the standalone
+          # relay is held to the SAME bar as the engine. Widening cost nothing — clean at this bar.
+          #   tests/harness/samples — intentional non-production idioms. This exclude is here for
+          #                           PARITY WITH BANDIT — which is what the parity test enforces —
+          #                           NOT as a prediction about findings. semgrep's own default
+          #                           .semgrepignore already excludes tests/: that is the recorded
+          #                           fact scripts/ci/assert_semgrep_handler_taint.py:41 depends on,
+          #                           and the reason that step copies its fixture to a temp dir to
+          #                           get it analysed at all. So this flag is explicit and probably
+          #                           redundant, stated that way on purpose — a gate's scope must be
+          #                           readable from its own argv, never inherited from a tool default
+          #                           a version bump can change. (The only two sinks in the whole
+          #                           tree that match these rules do sit under tests/: an `eval`
+          #                           taint fixture and a `pickle.loads` sandbox test.)
+          #   packaging/messagefoundry-webconsole/tests — the same, one directory deeper
+          #   ide/                  — TypeScript; no Python to scan
+          #   docs/benchmarks/results — archived measurement artifacts, not maintained source
+          # The `./` prefix bandit uses is DROPPED on purpose: semgrep matches --exclude as a GLOB,
+          # not a path, so `./tests` matches nothing and the flag would be inert — copying bandit's
+          # string byte-for-byte yields an exclude list that excludes nothing. (Whether tests/ would
+          # then actually be SCANNED is the separate question the default .semgrepignore above
+          # answers; the point is that this gate's scope must not rest on that answer.)
+          # Whether those globs are root-anchored or match a bare directory NAME at any depth is not
+          # settled here, and is not load-bearing — measured over the WHOLE tracked tree, which is
+          # the population this scan now covers: the only nested directory matching any of these
+          # names is packaging/messagefoundry-webconsole/tests, excluded explicitly on both sides
+          # anyway (2026-08-04).
+          # tests/test_lint_scope_parity.py fails if this scope and bandit's drift apart again. It
+          # also asserts, separately, that no --exclude here carries the `./` prefix (its normalised
+          # set comparison would otherwise call `./tests` and `tests` identical), that no `--include`
+          # re-narrows the scan behind the positional `.`, and that `--error` survives.
+          semgrep --config .semgrep --error --metrics off \
+            --exclude tests --exclude harness --exclude samples --exclude ide \
+            --exclude docs/benchmarks/results --exclude packaging/messagefoundry-webconsole/tests \
+            --exclude .venv --exclude node_modules \
+            .
       - name: Handler-config taint rules — validate, regression gate, scan samples (ADR 0144 Inc 3)
         run: |
           # semgrep is already installed by the step above (same job). This is why the packaged rules

.semgrep/messagefoundry.yml

@@ -1,7 +1,14 @@
 # MessageFoundry SAST rules — high-signal patterns for a PHI-handling HL7 engine. These complement
-# bandit; each targets a sink that untrusted HL7/config data must never reach unguarded. All rules
-# have ZERO matches in the current tree (verified at authoring), so the gate starts clean; the
-# security.yml job runs them advisory for one cycle, then flips to blocking (CI-1).
+# bandit; each targets a sink that untrusted HL7/config data must never reach unguarded.
+#
+# These rules are BLOCKING. The security.yml `semgrep` job runs them with --error, and that job is a
+# required context (.github/required-contexts.txt:78, asserted by tests/test_security_posture.py's
+# _BLOCKING_SECURITY_JOBS), so a single match fails the build. Stated from those two checkable facts
+# rather than dated: this file's history was squashed, so "advisory until <date>" is not something a
+# reader can verify here, and the previous header's "runs them advisory for one cycle" outlived the
+# flip it described. Scope is the WHOLE REPO minus that job's excludes — deliberately not an
+# allow-list. This header deliberately makes no claim that the tree is currently clean: the gate is
+# what keeps it clean, and the gate is the only thing that can still say so at a later date.
 rules:
   - id: mf-no-shell-true
     languages: [python]

docs/BACKLOG.md

@@ -3146,7 +3146,7 @@ Honestly bounded: **this is build-time only.** No PHI path, no running-engine su
 
 ## 334. semgrep, a required blocking gate, scans a two-directory allow-list
 
-> 🔢 **Filed 2026-08-01 — not started.** Value **5/10** · Difficulty **2/10** · _fill-in_. `security.yml:413` is still `semgrep --config .semgrep --error --metrics off messagefoundry tee` while bandit next door scans `-r .` at `:359`, and `tests/test_lint_scope_parity.py` — the control cited as stopping exactly this drift — mentions semgrep nowhere, so the project-specific rules directory silently skips the separately-versioned console wheel; bandit and CodeQL cover the sinks today, so this is breadth parity with a live compensating control, and the fix is one argument list mirroring bandit's `--exclude` plus one parity arm modelled on `:119-125`.
+> ✅ **Status CLOSED (built 2026-08-04).** `semgrep --config .semgrep --error --metrics off messagefoundry tee` is now `semgrep --config .semgrep --error --metrics off --exclude … .` carrying bandit's exclude set from the same file, name-for-name, so the project's own dangerous-sink rules now cover **59** tracked `.py` files they never saw: `messagefoundry_webconsole/` (33), `scripts/` (24), `docker/` (2). **The body's "56 / 32 / 22" is a stale measurement, not a different scope** — re-measured 2026-08-04 at **339** in-scope files, up from 280. Clean at that bar (0 findings, AST emulation of all five rules); **not run with real semgrep, which has no supported Windows install** — the first CI run on the PR is the real check. `tests/test_lint_scope_parity.py` carries the parity arm the item asked for, plus three assertions it did not: no `--include` (it re-narrows the scan behind a positional `.`, so a targets-only check reads green on this very regression), no `./` prefix on a semgrep `--exclude` (a glob, not a path — and the set comparison normalises `./` off both sides), and `--error` still present (without it the widened gate prints every finding and exits 0). **Every `security.yml:NNN` anchor in the body below has moved** — the command is now at `:449`; the job still starts at `:393`. Two claims elsewhere rested on this item's old state and were corrected in the same commit: ADR 0034's residual row mitigated an unpinned `pip` bootstrap with *"semgrep is **not** a required context"* (it is — `.github/required-contexts.txt:78`), and `docs/Secure_Build_Scorecard_MEFOR.md:56` carried a now-resolved nit about the `.semgrep` header still calling the rules "advisory".
 
 **Cluster:** Security / CI gates. **Priority:** P2. **Verdict:** build. **Severity:** low.
 

docs/Secure_Build_Scorecard_MEFOR.md

@@ -53,7 +53,7 @@ Status tags use the built / designed-but-deferred / aspirational taxonomy (SDS 
 |---|--------|--------|-------------------------------------|
 | 1 | Per-interface threat model & secure design | **Built — Strong** | `docs/security/THREAT-MODEL.md` (STRIDE-lite over 6 trust boundaries L16-30; 6 interface tables; risky-third-party 15.1.4 L90 + dangerous-functionality 15.1.5 L104). The ReDoS/single-parser guard is a real blocking pytest gate, not a doc claim: `tests/test_security_static.py` (`test_no_catastrophic_regex_in_source` / `test_single_json_parser` / `test_single_url_parser` / `test_xml_parsers_are_confined_to_the_allowlist` + planted-`(a+)+` meta-tests for the inline **and** the constant-passed shape). Scope corrected 2026-07-22: it is **not** repo-wide — the clauses walk `messagefoundry/`, `messagefoundry_webconsole/` and `harness/` (`_SOURCE_ROOTS`), deliberately excluding `tests/` and `scripts/`, whose own corpora and release tooling would trip the scanners on false positives; the per-clause root registry records why. Caveat resolved 2026-07-14: the STRIDE-table TLS "deferred" (L40, L74) intra-doc contradiction and the stale `console/service_control.py` path were corrected. The STRIDE rows now say built (matching the residual section + `api/tls.py`/`transports/mllp.py`) and the subprocess row points at `messagefoundry/service.py`. |
 | 2 | Secure coding practices enforced | **Built — Strong** *(grade holds; asserted evidence overstated)* | `parsing/peek.py` + `parsing/validate.py` (ingress validation), `store/store.py` (bound-parameter SQL; f-strings interpolate only literal column names per STORE-4), `api/security.py` (deny-by-default 401/403), `store/crypto.py` (AES-GCM) + `auth/passwords.py` (argon2id). Overstated: "PW.5 all Pass" is false. The cited `SDS-CONFORMANCE-REVIEW-2026-06-12.md` marks REST/SOAP (:74), file-handler (:75) and TLS-in-transit (:76) Partial, and that review is an explicit self-review, not independent. The four specific items credited (ingress / param-SQL / fail-closed authz / vetted crypto) are Pass. |
-| 3 | Blocking security gates in CI, red-on-regression | **Built — Strong** | `.github/workflows/security.yml`: 7 jobs, no `continue-on-error`, none `if:`-gated, so all run on `pull_request`: pip-audit, npm-audit, bandit, gitleaks (`fetch-depth: 0`), `semgrep --error` (→ `.semgrep/messagefoundry.yml`, 5 dangerous-sink ERROR rules), crypto-inventory, forbidden-content. Honest residual verified: `continue-on-error: true` appears only on `sbom` (L104) + `trivy` (L137), both additionally cron/dispatch-only, never PR-gating. Nit: `.semgrep` header comment is stale, still calls semgrep "advisory". |
+| 3 | Blocking security gates in CI, red-on-regression | **Built — Strong** | `.github/workflows/security.yml`: 7 jobs, no `continue-on-error`, none `if:`-gated, so all run on `pull_request`: pip-audit, npm-audit, bandit, gitleaks (`fetch-depth: 0`), `semgrep --error` (→ `.semgrep/messagefoundry.yml`, 5 dangerous-sink ERROR rules), crypto-inventory, forbidden-content. Honest residual verified: `continue-on-error: true` appears only on `sbom` (L104) + `trivy` (L137), both additionally cron/dispatch-only, never PR-gating. Nit resolved 2026-08-04 (BACKLOG #334): the `.semgrep` header no longer calls semgrep "advisory" — it now states the rules are blocking and names the required context. Scope corrected in the same change: `semgrep --error` scanned the two-directory allow-list `messagefoundry tee`, and now scans the whole repo minus bandit's exact `--exclude` set (59 more tracked `.py` files: `messagefoundry_webconsole/`, `scripts/`, `docker/`), pinned by `tests/test_lint_scope_parity.py`. |
 | 4 | Dependency & supply-chain integrity | **Built — Strong** (3 sub-items open) | `security.yml` pip-audit runs DEP-1 (`uv lock --check` + byte-diff + `pip install --require-hashes` + `pip-audit -r requirements.lock`), daily SCA cron; `dependabot.yml` (uv/actions/npm) + fail-closed published-GHSA auto-merge gate (`dependabot-auto-merge.yml`). `release.yml` tag-time blocking CycloneDX SBOM + Sigstore keyless + SLSA `attest-build-provenance` + PyPI Trusted Publishing/PEP 740. Open (`SDS-REMEDIATION-PLAN.md`): CI SBOM blocking (4.1/:63), signed-commit/DCO (4.3/:65), PS.3 build-input archival (:64). |
 | 5 | Secrets hygiene | **Built — Strong** | `security.yml` gitleaks full-history + forbidden-content, both blocking; `scripts/security/scan_forbidden.py` fail-closed (exit 1 on hit, exit 2 on a vacuous or under-loaded scan; customer-name / site-code / routable-IPv4 / worktree-slug / home-path detectors) shared by `.pre-commit-config.yaml` and the CI job, both scanning the WHOLE tracked tree. The token list is externalized -- never committed -- and CI asserts a per-section detector floor so a partial or mangled token source fails the build rather than passing green; `.claude/settings.json` deny-list on `.env*`/`secrets/**`/`*.key`/`*.pem`/`*.pfx`/`*.db`. Secrets are `MEFOR_*`-env-only (`config/settings.py`). |
 | 6 | Secure-by-default, fail-closed configuration | **Built — Strong** (at-rest fail-closed for every PHI posture) | `messagefoundry/__main__.py` loopback-only no-auth refusal (:914-920), `--allow-insecure-bind` that cannot relax the no-auth or Posture-B refusals (:104-111), off-loopback TLS-revocation refusal (:1285-1338), keyless-PHI refusal in every env (:989-1004); `config/settings.py` `require_mfa=True` (:1391). At-rest mechanism (verified 2026-07-14): `serve` requires an env (`__main__.py:938`) and `require_posture()` (`settings.py:1625`) refuses a keyless start for any unresolved/custom-env posture; `_KNOWN_ENV_POSTURE` (`settings.py:1560`) is `dev→SYNTHETIC`, `staging→PHI`, `prod→PHI`, so the keyless-PHI gate refuses every PHI posture. No PHI posture runs cleartext. Keyless `IdentityCipher` cleartext (`store/crypto.py:454`) is reachable only on a synthetic posture or the audited `allow_unencrypted_phi` opt-out; `require_encryption=False` (:301) is a stricter-still guard, not the switch. Residual (risk-accepted, register standing note): an operator could misuse a synthetic-declared env for real PHI. Like the `allow_unencrypted_phi` opt-out, that is an operator responsibility, not an engine gap. |