diff --git a/docs/SECURITY.md b/docs/SECURITY.md index b9e43ee3..96091f32 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -1674,7 +1674,7 @@ multi-host deployment must additionally front the API with a proxy/WAF limiter a | Request body | `[store].max_upload_bytes` (the `/uploads` routes only) | 1 MiB elsewhere | per request | no | no | no | **stateless** — every route, in ASGI middleware | **413** over the cap, **400** on ambiguous CL+TE framing or an invalid `Content-Length`, **411** on a chunked body | | OIDC pending flows | `[auth].oidc_flow_cache_max` (global), `DEFAULT_PER_IP_CAP` (per-IP, no knob), `oidc_flow_ttl_seconds` | 512 / 16 / 300 s | 300 s TTL | no | **yes** (512) | **yes** (16) | **in-process** — `GET /ui/oidc/start` — reject-when-full, never evict | 303 → `/ui/login?e=rate_limited`, WARNING-logged, **never** audited | | WebAuthn pending ceremonies | `GLOBAL_PENDING_CAP`, `PER_USER_PENDING_CAP`, `CHALLENGE_TTL_SECONDS` (module constants, no knobs) | 4096 / 16 / 120 s | 120 s TTL | **yes** (16) | **yes** (4096) | no | **in-process** — every passkey registration + assertion ceremony | per-user: evicts that user's **own** oldest pending ceremony (silent); global: `ChallengeCacheFullError` naming the cause + the `admin_reset_mfa` recovery path | -| **Ingest plane** | `max_messages_per_second`, `message_burst` (MLLP inbound) | **off** | per message | no | no | no | **in-process** — one bucket per MLLP connection, so it neither coordinates across engine shards nor aggregates per peer | **exists but ships OFF, so unset there is still no volume bound.** When set, the listener **pauses reading** over budget so TCP back-pressures the sender: no message is dropped, refused, NAK'd or reordered (the count-and-log invariant forbids accept-and-drop, so a discarding limiter was never available). Bounded by the bucket deficit. The off default is **ruled, not accidental** — a rate on a clinical interface is only safe at a number from a real feed profile. **Not covered:** the raw-TCP inbound, and any per-peer bound (MLLP peers are unauthenticated, so the only key would be source IP, which NAT collapses). Other inbound caps are resource-only — `max_connections` (256), `receive_timeout` (60.0 s), `max_frame_bytes` (16 MiB), per-connection `max_message_bytes`, `source_ip_allowlist` | +| **Ingest plane** | `max_messages_per_second`, `message_burst` (MLLP inbound) | **NOT REACHABLE — read the note** | per message | no | no | no | **in-process** — one bucket per MLLP connection, so it neither coordinates across engine shards nor aggregates per peer | **THE PACER IS BUILT AND NO DOCUMENTED CONFIGURATION CAN TURN IT ON.** Read that before the rest of this cell. Neither key is a parameter of the `MLLP()` factory, and `connections.toml` routes through that same factory, so **neither the code-first nor the TOML surface can express them**. This row previously read state **"off"**, which every reader takes to mean "set it to on". **So there is currently NO message-RATE bound anywhere on the ingest plane.** — **This is NOT "unbounded intake", and that correction runs in the engine's favour:** several bounds ship **ON** and are listed at the end of this cell. They bound **SIZE and CONCURRENCY, not RATE**. — *What the pacer would do if it were reachable:* the listener **pauses reading** over budget so TCP back-pressures the sender; no message is dropped, refused, NAK'd or reordered (the count-and-log invariant forbids accept-and-drop, so a discarding limiter was never available). The off default was **ruled, not accidental** — a rate on a clinical interface is only safe at a number from a real feed profile — but **a ruled default and an unreachable setting are different things, and only the first was intended.** **Not covered even if reachable:** the raw-TCP inbound, and any per-peer bound (MLLP peers are unauthenticated, so the only key would be source IP, which NAT collapses). **Resource bounds that DO ship on** — `max_connections` (256), `receive_timeout` (60.0 s), `max_frame_bytes` (16 MiB), per-connection `max_message_bytes`, `source_ip_allowlist` | **What these limits defend, and what they do not.** The full inventory of resource-demanding functionality — including the surfaces that remain **unbounded** at this release — is diff --git a/scripts/docs/dangling_citation_check.py b/scripts/docs/dangling_citation_check.py new file mode 100644 index 00000000..067bd888 --- /dev/null +++ b/scripts/docs/dangling_citation_check.py @@ -0,0 +1,270 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Report a `#N` backlog citation that names NO ITEM AT ALL (BACKLOG #1235). + +NOT `backlog_citation_check.py`, WHICH SITS BESIDE THIS FILE AND ANSWERS A DIFFERENT QUESTION. +That gate (BACKLOG #1095) resolves a citation against the ledger FILE its item lives in, so a +reference naming the live `docs/BACKLOG.md` for an item that has been archived is caught -- the +number is real, the path is stale. This one asks whether the number names anything whatsoever. +An item that exists in either ledger is invisible here and is that gate's business; a number that +exists in neither is invisible there and is this one's. Neither subsumes the other, and the near- +identical names are the reason this paragraph is the first thing in the file. + +WHAT THE DEFECT IS. While a number names nothing, a citation to it resolves to NOTHING -- honest and +harmless, and it advertises its own brokenness. If that number is later issued, the citation starts +resolving to unrelated work and NOTHING anywhere reports a problem. The ledger's own erratum names +that as the worse outcome: a wrongly-resolving reference "reads as a working cross-reference +forever". This tool catches the citation while it is still in the honest state. + +THE DISTINCTION THAT DECIDES WHETHER A CITATION IS A TRAP, and it is not the obvious one. Two very +different states both look like "resolves to nothing", and only one of them can ever arm: + + * BELOW THE HIGH-WATER MARK. `scripts/coord/alloc.ps1` starts its search at ``$observed + 1`` and + scans UPWARD, never filling a hole -- "numbers are never reclaimed ... holes are free, + collisions are not". A number at or below the mark is therefore unreachable FOREVER, and the + citation is harmless permanently rather than by luck. Measured here: 26 of 32 hits. + * ABOVE IT. That number will be issued in the normal course, and on the day it is, the citation + silently starts naming unrelated work. This is the only shape that can arm. + +A tool reporting only "resolves in the ledger or not" rates those identically and raises 26 false +alarms on this repository alone. + +WHY THE FLOOR AND NOT THE ALLOCATION REGISTRY. An earlier version of this tool classified by looking +for an allocation RECORD under the git common dir. It produced the same answers and rested on the +wrong thing: those records are machine-local, uncommittable and losable, so garbage-collecting a +directory would appear to change a conclusion that never depended on it. The unreachability is a +structural property of how the allocator picks a number. Reasoning from the registry makes a sound +property look fragile and invites a guard nothing needs. + +WHY A TOOL AND NOT VIGILANCE. There is no gate on either side. `alloc.ps1` answers "is this number +free"; nothing asks "is anything already pointing at it". The two halves are each individually +correct, and the gap between them is the defect. + +WHAT THIS DELIBERATELY DOES NOT DO. + + * It does NOT claim a defect count. Its output is an UPPER BOUND ON CITATIONS -- some hits are + near-certainly not backlog references at all (a four-digit token in a research evaluation, one + inside a diagram). Reporting the raw number as a defect count would be a completeness claim the + scan cannot support, so the summary says so in those words rather than leaving it to be + inferred. Every hit is printed with its file, line and full source line so a human judges it. + * It does NOT sweep or edit. Fixing existing instances is a separate, owner-present task. + * It does NOT see the private companion repository, where the known instances live. A citation in + another repository is invisible to every check this one runs -- that limitation is inherent, is + the reason the item exists, and is printed rather than left implicit. + +The allocated set is read through `backlog_status_check.parse_items`, never a hand-rolled scan: +that module DEFINES what an item is, and a second definition here would drift from it silently. +""" + +from __future__ import annotations + +import argparse +import importlib.util +import re +import sys +from pathlib import Path +from typing import NamedTuple + +_HERE = Path(__file__).resolve().parent + +# The window the item pinned. Numbers below 1000 are the pre-partition internal sequence and the +# repository's PR/issue numbers, which share the `#N` spelling and are NOT backlog citations. +_LOW = 1000 +_HIGH = 9000 + +#: A citation is `#` immediately followed by digits AND ENDING THERE. A Markdown heading +#: (`## 1235.`) puts a SPACE between, so it cannot match -- which matters, because every item +#: heading in the ledger would otherwise report as a citation of itself. +#: +#: The trailing boundary is not cosmetic. Without it, a CSS/Mermaid hex colour matches its own digit +#: prefix: `#1565c0` reads as a citation of #1565 and `#06302b` as one of #6302. Measured on the +#: first real run over docs/ -- those two alone produced 8 of 40 hits, all spurious, across +#: ARCHITECTURE.md and architecture-diagram.md. An inflated count in a tool whose entire output is a +#: bound is the one failure that makes it useless. +_CITATION = re.compile(r"#(\d+)(?![0-9A-Za-z])") + +#: A `#N` introduced by one of these is a PULL REQUEST, issue or forum reference, not a backlog +#: citation. This repository's PR numbers ALREADY exceed 1000 (`PR #995`, `PR #1001` are both cited +#: in docs/adr/), so the [1000,9000) window does not separate the two namespaces on its own -- +#: measured, not assumed. Matching hits are still REPORTED and still COUNTED; they are only +#: annotated, because this item's discipline is to DISCLOSE a false positive rather than trim it. +_PR_CONTEXT = re.compile(r"(?i)\b(?:PR|pull request|commit|issue|discussion)\s+$") + +#: `pyodbc#1459`, `coder/code-server#6256` -- a `#N` glued to an identifier is ANOTHER PROJECT's +#: issue number. Both shapes are live in docs/ today. +_FOREIGN_REPO = re.compile(r"[A-Za-z0-9_./-]$") + + +class Hit(NamedTuple): + path: Path + lineno: int + number: int + line: str + pr_shaped: bool + + +def _load_backlog_module() -> object: + """Import backlog_status_check by path -- it is a script directory, not an installed package.""" + spec = importlib.util.spec_from_file_location( + "_backlog_status_check", _HERE / "backlog_status_check.py" + ) + if spec is None or spec.loader is None: # pragma: no cover - a packaging accident, not a state + raise RuntimeError("cannot load backlog_status_check.py") + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def allocation_floor(sources: list[Path] | None = None) -> int: + """The highest item number the ledgers know about. Nothing at or below it can ever be issued. + + THIS IS A STRUCTURAL PROPERTY OF THE ALLOCATOR, NOT A FACT ABOUT ANY REGISTRY FILE. + `scripts/coord/alloc.ps1` starts its search at ``$observed + 1`` (or + ``[Math]::Max($observed, $PublicBacklogFloor - 1) + 1`` under the public clamp) and scans + UPWARD. It never fills a hole: "numbers are never reclaimed ... holes are free, collisions are + not". So a number at or below the high-water mark is unreachable **forever**, and a number above + it will be issued in the normal course. That is the whole classification. + + An earlier version of this tool read the allocation records under the git common dir instead. + That gave the same answers here and was the wrong basis: those records are machine-local, + uncommittable and losable, so garbage-collecting a directory would have "changed" a conclusion + that does not actually depend on it. Reasoning from the registry makes a sound property look + fragile and invites a guard nothing needs. + + Deliberately conservative: the allocator's observed set is a SUPERSET of the ledger headings -- + it also reads refs, the working tree, claim files and a persisted high-water mark (``:97``) -- so + this ledger-only figure can only ever sit at or BELOW the true floor. A number between the two is + reported as reachable when it is not: an over-warning, never a missed trap. + + THAT GAP IS THE POINT, NOT A ROUNDING ERROR, AND THE FLAGS IT PRODUCES ARE NOT FALSE POSITIVES. + A number that has been ALLOCATED but whose heading is not yet committed sits above this floor and + is reported live -- correctly, because the allocator's ratchet persists the max of the observed + set rather than the number just issued, so until that heading lands the only durable record of it + is an untracked, never-pushed registry file. Lose that file and the number is re-issued. + + The flag then clears ITSELF: once the heading is committed the floor rises past it and it stops + being reported, which is exactly when it becomes permanently reserved. Do not "fix" a live report + on a freshly-allocated number, and do not add a near-the-floor rule to catch this case -- this + floor already covers it, and a second rule aimed at an edge the first one covers is how a tool + acquires a guard nothing needs. + """ + numbers = allocated_numbers(sources) + return max(numbers) if numbers else 0 + + +def allocated_numbers(sources: list[Path] | None = None) -> set[int]: + """Every item number that EXISTS in the ledgers, open or closed. + + Closed items are included deliberately: a citation to a closed item resolves correctly and is + not this defect. Only a number that names nothing at all is the trap. + """ + module = _load_backlog_module() + paths = sources if sources is not None else [Path(p) for p in module.DEFAULT_SOURCES] # type: ignore[attr-defined] + numbers: set[int] = set() + for path in paths: + if not path.exists(): + continue + for item in module.parse_items(path.read_text(encoding="utf-8")): # type: ignore[attr-defined] + numbers.add(item.num) + return numbers + + +def citations_in(text: str) -> list[tuple[int, int, str, bool]]: + """(lineno, number, line, pr_shaped) for every in-window `#N` token. 1-indexed lines.""" + found: list[tuple[int, int, str, bool]] = [] + for lineno, line in enumerate(text.splitlines(), start=1): + for match in _CITATION.finditer(line): + number = int(match.group(1)) + if _LOW <= number < _HIGH: + before = line[: match.start()] + pr_shaped = ( + _PR_CONTEXT.search(before) is not None + or _FOREIGN_REPO.search(before) is not None + ) + found.append((lineno, number, line.strip(), pr_shaped)) + return found + + +def unresolved_citations(paths: list[Path], allocated: set[int]) -> list[Hit]: + hits: list[Hit] = [] + for path in paths: + try: + text = path.read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError): + continue + for lineno, number, line, pr_shaped in citations_in(text): + if number not in allocated: + hits.append(Hit(path, lineno, number, line, pr_shaped)) + return hits + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter + ) + parser.add_argument("paths", nargs="*", type=Path, help="files to scan (default: docs/**/*.md)") + parser.add_argument( + "--fail", action="store_true", help="exit non-zero when any citation is unresolved" + ) + args = parser.parse_args(argv) + + # A scanner that CRASHES partway prints a partial list that reads as a complete one. Source lines + # legitimately carry characters a stock Windows cp1252 console cannot encode, and this tool hit + # exactly that on its first real run (U+2194, inside an ADR). Force UTF-8 and KEEP errors=replace: + # the defect is the wrong codec, and replacement is what stops one exotic glyph truncating a + # measurement. + if hasattr(sys.stdout, "reconfigure"): + sys.stdout.reconfigure(encoding="utf-8", errors="replace") + + paths = args.paths or sorted(Path("docs").rglob("*.md")) + allocated = allocated_numbers() + hits = unresolved_citations(list(paths), allocated) + + if not hits: + print(f"No unresolved backlog citation in {len(paths)} file(s).") + print(f"Resolved against {len(allocated)} allocated item numbers (open and closed).") + return 0 + + floor = allocation_floor() + + for hit in hits: + note = ( + " [PR/issue/foreign-repo shaped -- very likely NOT a backlog citation]" + if hit.pr_shaped + else "" + ) + print(f"{hit.path}:{hit.lineno}: #{hit.number} resolves to no filed item{note}") + print(f" {hit.line}") + state = ( + f"BELOW THE FLOOR ({floor}) -- the allocator only ever issues above its high-water " + "mark, so this number can NEVER be issued and the citation is permanently harmless." + if hit.number <= floor + else f"ABOVE THE FLOOR ({floor}) -- this number CAN still be issued to unrelated work. " + "This is the live shape." + ) + print(f" -> {state}") + + distinct = sorted({hit.number for hit in hits}) + pr_shaped = sum(1 for hit in hits if hit.pr_shaped) + print() + print( + f"{len(hits)} token(s) across {len({h.path for h in hits})} file(s); " + f"{len(distinct)} distinct number(s): {', '.join(f'#{n}' for n in distinct)}" + ) + if pr_shaped: + print( + f"{pr_shaped} of those {len(hits)} TOKENS (not of the {len(distinct)} numbers) are " + f"PR/issue/foreign-repo shaped, annotated above." + ) + print( + "This is an UPPER BOUND ON CITATIONS, NOT A COUNT OF DEFECTS -- some hits are very likely" + ) + print("not backlog references at all. Each is printed above with its source line; judge them.") + print( + "Not scanned: the private companion repository, where a citation is invisible to this repo." + ) + return 1 if args.fail else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/_extras_probe.py b/tests/_extras_probe.py new file mode 100644 index 00000000..7bffd20a --- /dev/null +++ b/tests/_extras_probe.py @@ -0,0 +1,91 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Which optional extras are absent from this interpreter (BACKLOG #1230, loud-omission half). + +WHY THIS IS A SIBLING MODULE AND NOT PART OF ``conftest.py``, which is where it started and where it +does not belong. The tests reached it with a bare ``import conftest``. That passes when only +``tests/`` is collected and FAILS IN A FULL RUN, because ``pyproject.toml`` has two testpaths and +``packaging/messagefoundry-webconsole/tests/conftest.py`` claims the same top-level module name: + + AttributeError: + has no attribute '_OPTIONAL_EXTRAS' + +Seven tests passed alone and failed together, which is the worst way for a test to be wrong -- the +name resolved to the neighbouring module and nothing said so. Importing ``conftest`` BY PATH instead +would re-execute it, and its module body claims a per-process test slot and registers an ``atexit`` +unlink, so a second execution burns a slot for nothing. + +So the probe lives here: no module-level side effects, imported package-qualified as +``tests._extras_probe`` (the same idiom as ``tests._workflow_contexts``), and ``conftest`` keeps only +the thin pytest hooks that render what these functions compute. +""" + +from __future__ import annotations + +import importlib.util +from typing import Protocol + +#: extra name -> the import sentinels its tests need. Mirrors pyproject.toml's +#: [project.optional-dependencies]; every distribution an extra pulls in is listed, so a +#: half-installed extra reads as absent rather than present. +OPTIONAL_EXTRAS: dict[str, tuple[str, ...]] = { + "fhir": ("fhir.resources", "fhirpathpy"), + "dicom": ("pydicom", "pynetdicom"), + "x12": ("pyx12",), + "xml": ("lxml", "xmlschema", "signxml"), + "webauthn": ("webauthn",), +} + + +class SummaryWriter(Protocol): + """The two ``TerminalReporter`` methods the summary uses, and nothing else.""" + + def write_line(self, line: str) -> None: ... + + def write_sep(self, sep: str, title: str = "", **kwargs: object) -> None: ... + + +def extra_is_installed(sentinels: tuple[str, ...]) -> bool: + """True only when EVERY sentinel resolves. + + ``find_spec`` rather than an import: it answers the question without paying the import cost and + without leaving a partially-imported module behind on failure. A sentinel whose PARENT package is + absent (``fhir.resources`` with no ``fhir``) RAISES instead of returning ``None`` -- measured, not + assumed -- so both arms have to mean "absent" or the fhir extra would read as present. + """ + for name in sentinels: + try: + if importlib.util.find_spec(name) is None: + return False + except (ImportError, ValueError): + return False + return True + + +def missing_extras() -> list[str]: + """Extras whose tests cannot be collected in this interpreter, in declaration order.""" + return [name for name, probes in OPTIONAL_EXTRAS.items() if not extra_is_installed(probes)] + + +def report_header_lines() -> list[str]: + """State the omission up front -- a summary line is easy to scroll past on a long run.""" + missing = missing_extras() + if not missing: + return [] + return [f"INCOMPLETE RUN: optional extras absent -- {', '.join(missing)}"] + + +def write_incomplete_run_summary(reporter: SummaryWriter) -> None: + """Print the omission where the verdict is rendered, so green cannot be read as complete.""" + missing = missing_extras() + if not missing: + return + write = reporter.write_line + reporter.write_sep("=", "INCOMPLETE RUN -- coverage was NOT collected", red=True, bold=True) + write(f"Optional extras absent from this interpreter: {', '.join(missing)}") + write("Every test module gated on them removed itself at COLLECTION time, so its tests are not") + write("in the counts above -- passed, failed and skipped alike. This result does NOT establish") + write("that the full suite is green, and must not be reported as if it did.") + write("") + write("To collect them, install what CI installs (.github/workflows/ci.yml):") + write(f' pip install --constraint constraints.lock -e ".[dev,harness,{",".join(missing)}]"') diff --git a/tests/conftest.py b/tests/conftest.py index 6bb7a79e..f5a27c1a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -25,6 +25,7 @@ import pytest from messagefoundry.config.settings import INSECURE_CONFIG_SOURCE_ESCAPE_ENV +from tests._extras_probe import report_header_lines, write_incomplete_run_summary # --------------------------------------------------------------------------------------------------- # Per-PROCESS test slot. @@ -327,3 +328,44 @@ def _tolerate_logging_on_closed_capture_streams() -> Iterator[None]: yield finally: logging.raiseExceptions = prior_raise + + +# --------------------------------------------------------------------------------------------------- +# LOUD OMISSION: an incomplete run must SAY SO (BACKLOG #1230 — the loud-omission half only, per the +# owner's 2026-08-12 scope ruling; the venv is deliberately NOT changed here). +# +# scripts/worktree/new.ps1:232 creates a worktree venv with `.[dev,harness]`. CI installs +# `.[dev,harness,fhir,dicom,x12,xml,webauthn]` (.github/workflows/ci.yml:273). Every module gated on +# that five-extra gap removes ITSELF at collection time via a module-level `pytest.importorskip`, so +# those tests never become test items at all: a large block of coverage collapses into a short skip +# tally and the run still prints as green. +# +# THE DEFECT IS THE SILENCE, NOT THE ABSENCE. Skipping an uninstalled optional extra is correct and +# intended. Rendering an incomplete run as a complete one is not — "the full suite is green" is the +# sentence the next session bases its own scope on, and a local run cannot currently earn it. +# +# Deliberately NOT a pinned test count. A hard-coded figure would be right the day it was written and +# silently wrong after, and a stale number in a measurement surface is worse than none: re-running +# reproduces it, so it reads as verified. What prints is what is re-derived every run — which extras +# are absent, and the command that installs them. +# +# This never fails a run and never skips anything itself. It only makes an already-incomplete run +# legible, so the omission has to be read rather than inferred from a skip count. +# +# SCOPE, STATED RATHER THAN IMPLIED. These hooks live in tests/conftest.py, so they load for any run +# that collects tests/ — which includes every full-suite run, the only kind that can earn the phrase +# above. A run scoped ENTIRELY to the other testpath (packaging/messagefoundry-webconsole/tests) +# does not load this file and prints no banner. Measured both ways, with a control to rule out the +# hooks simply being inert: webconsole-only collected 365 tests silently; the same flags over a +# tests/ path printed the banner. That gap is left rather than fixed with a repo-root conftest.py, +# which would change collection globally and is outside this item's ruled scope — a deliberately +# scoped run already reads as partial; a full-suite run is the one that must not. +# --------------------------------------------------------------------------------------------------- + + +def pytest_report_header() -> list[str]: + return report_header_lines() + + +def pytest_terminal_summary(terminalreporter: pytest.TerminalReporter) -> None: + write_incomplete_run_summary(terminalreporter) diff --git a/tests/test_dangling_citation_check.py b/tests/test_dangling_citation_check.py new file mode 100644 index 00000000..06022e16 --- /dev/null +++ b/tests/test_dangling_citation_check.py @@ -0,0 +1,176 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""The unresolved-backlog-citation detector (BACKLOG #1235). + +Every false-positive shape asserted here was found by RUNNING the tool over docs/, not predicted +before it. Two of them were defects in the detector itself on its first real run: a hex colour +matching its own digit prefix, and a crash on a character a cp1252 console cannot encode. They are +pinned as tests because both re-appear the moment the pattern is loosened. +""" + +from __future__ import annotations + +import importlib.util +import pathlib +from pathlib import Path + +import pytest + +_SPEC = importlib.util.spec_from_file_location( + "_dangling_citation_check", + Path(__file__).resolve().parents[1] / "scripts" / "docs" / "dangling_citation_check.py", +) +assert _SPEC is not None and _SPEC.loader is not None +cc = importlib.util.module_from_spec(_SPEC) +_SPEC.loader.exec_module(cc) + + +def _numbers(text: str) -> list[int]: + return [number for _lineno, number, _line, _pr in cc.citations_in(text)] + + +# --- what a citation IS --------------------------------------------------------------------------- + + +def test_a_plain_citation_is_found() -> None: + assert _numbers("see BACKLOG #1235 for the rule") == [1235] + + +def test_line_numbers_are_one_indexed() -> None: + lineno, number, _line, _pr = cc.citations_in("first\nsecond #1235\n")[0] + assert (lineno, number) == (2, 1235) + + +@pytest.mark.parametrize("trailing", [".", ",", ")", "'s", " and", "]", ";"]) +def test_ordinary_punctuation_still_closes_a_citation(trailing: str) -> None: + assert _numbers(f"see #1235{trailing}") == [1235] + + +# --- what a citation IS NOT ----------------------------------------------------------------------- + + +def test_a_markdown_heading_is_not_a_citation_of_itself() -> None: + """Every item heading in the ledger would otherwise report as citing its own number.""" + assert _numbers("## 1235. a citation to an unallocated number") == [] + + +@pytest.mark.parametrize("colour", ["#1565c0", "#06302b", "#1234ab"]) +def test_a_hex_colour_does_not_match_its_own_digit_prefix(colour: str) -> None: + """Measured, not predicted: on the first real run over docs/, `#1565c0` and `#06302b` alone + produced 8 of 40 hits. An inflated count in a tool whose whole output is a bound is fatal.""" + assert _numbers(f"classDef x fill:#e3f2fd,stroke:{colour};") == [] + + +def test_numbers_outside_the_window_are_ignored() -> None: + assert _numbers("PR #995 and issue #42 and #9000 and #12345") == [] + + +def test_the_window_is_half_open_at_both_ends() -> None: + assert _numbers("#999") == [] + assert _numbers("#1000") == [1000] + assert _numbers("#8999") == [8999] + assert _numbers("#9000") == [] + + +# --- annotation, which discloses rather than trims ------------------------------------------------- + + +@pytest.mark.parametrize( + "line", + [ + "shipped in PR #1001", + "see pull request #1001", + "fixed by commit #1001", + "upstream issue #1001", + "code-server discussion #6256", + ], +) +def test_a_pr_or_forum_reference_is_annotated(line: str) -> None: + hits = cc.citations_in(line) + assert len(hits) == 1 + assert hits[0][3] is True, "should be flagged as very likely not a backlog citation" + + +@pytest.mark.parametrize("line", ["upstream pyodbc#1459", "coder/code-server#6256"]) +def test_a_foreign_repo_reference_is_annotated(line: str) -> None: + hits = cc.citations_in(line) + assert len(hits) == 1 + assert hits[0][3] is True + + +def test_a_genuine_citation_is_not_annotated() -> None: + hits = cc.citations_in("this supersedes #1084") + assert len(hits) == 1 + assert hits[0][3] is False + + +def test_an_annotated_hit_is_still_reported_and_still_counted() -> None: + """The item's discipline is to DISCLOSE a false positive, never to trim it -- a trimmed scan + silently understates, which is the failure this whole item is about.""" + assert len(cc.citations_in("upstream pyodbc#1459")) == 1 + + +# --- resolution against the ledger ------------------------------------------------------------------ + + +def test_only_unallocated_numbers_are_reported(tmp_path: Path) -> None: + doc = tmp_path / "d.md" + doc.write_text("real #1230, unreal #8999\n", encoding="utf-8") + hits = cc.unresolved_citations([doc], allocated={1230}) + assert [h.number for h in hits] == [8999] + + +def test_a_closed_item_still_resolves(tmp_path: Path) -> None: + """A citation to a CLOSED item points at something real. Only a number naming nothing is the trap.""" + doc = tmp_path / "d.md" + doc.write_text("see #1230\n", encoding="utf-8") + assert cc.unresolved_citations([doc], allocated={1230}) == [] + + +def test_an_unreadable_file_is_skipped_not_fatal(tmp_path: Path) -> None: + missing = tmp_path / "gone.md" + assert cc.unresolved_citations([missing], allocated=set()) == [] + + +def test_the_real_ledger_yields_a_plausible_allocated_set() -> None: + """Positive control on the ledger read: if parse_items ever returns nothing, every citation in + the repository would report as unresolved and the tool would look catastrophically alarming.""" + allocated = cc.allocated_numbers() + assert len(allocated) > 100 + assert 1235 in allocated, "the item that defines this tool must resolve" + + +# --- the floor, which decides whether a citation can ever arm --------------------------------------- + + +def test_the_floor_is_found_and_is_plausible() -> None: + """Positive control. If the ledger read ever silently returned nothing, the floor would be 0 and + EVERY citation would classify as live -- 26 manufactured alarms on this repository.""" + assert cc.allocation_floor() > 1000 + + +def test_a_number_below_the_floor_can_never_be_issued() -> None: + """#1203 and #1231 are the two instances BACKLOG #1235 names as live traps. Both sit BELOW the + high-water mark, and alloc.ps1 starts at `$observed + 1` and never fills a hole, so neither can + ever be issued. The citations are inert BY CONSTRUCTION, not by luck. + + Note what this does NOT depend on: any allocation record under .git. Those are machine-local and + losable; the unreachability is structural and survives losing them.""" + floor = cc.allocation_floor() + assert floor >= 1203 + assert floor >= 1231 + filed = cc.allocated_numbers() + assert 1203 not in filed, "if #1203 gets filed this test has served its purpose -- update it" + assert 1231 not in filed + + +def test_a_number_above_the_floor_is_the_live_shape() -> None: + assert cc.allocation_floor() < 8999 + + +def test_the_floor_is_conservative_never_optimistic(tmp_path: pathlib.Path) -> None: + """Built from the ledgers only, so it can only ever UNDERSTATE the allocator's true floor (which + also spans refs and allocations). Understating means over-warning, never a missed trap.""" + ledger = tmp_path / "L.md" + ledger.write_text("## 1500. an item\n\n> open\n", encoding="utf-8") + assert cc.allocation_floor([ledger]) == 1500 diff --git a/tests/test_incomplete_run_banner.py b/tests/test_incomplete_run_banner.py new file mode 100644 index 00000000..9d1de192 --- /dev/null +++ b/tests/test_incomplete_run_banner.py @@ -0,0 +1,88 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""The incomplete-run banner (BACKLOG #1230, loud-omission half). + +Proved in BOTH directions, because a banner that always prints and a banner that never prints are +both indistinguishable from a working one if you only ever observe the state you happen to be in. +This worktree's venv is missing all five extras, so the "fires" direction is the ambient case and +the SILENT direction is the one that needs staging. +""" + +from __future__ import annotations + +from typing import Any + +from tests import _extras_probe as probe + + +class _FakeReporter: + """Captures what the summary hook writes. Mirrors only the two methods the hook uses.""" + + def __init__(self) -> None: + self.lines: list[str] = [] + self.seps: list[str] = [] + + def write_line(self, line: str, **_: Any) -> None: + self.lines.append(line) + + def write_sep(self, _sep: str, title: str = "", **_kw: Any) -> None: + self.seps.append(title) + + +def test_a_resolvable_sentinel_reads_as_installed() -> None: + """Positive control: the probe must be capable of returning True at all, or every other + assertion here would pass against a predicate that can only ever say 'absent'.""" + assert probe.extra_is_installed(("json",)) is True + assert probe.extra_is_installed(("json", "pathlib")) is True + + +def test_an_absent_sentinel_reads_as_missing() -> None: + assert probe.extra_is_installed(("mefor_no_such_module_1230",)) is False + + +def test_one_absent_sentinel_condemns_the_whole_extra() -> None: + """A half-installed extra must read as absent -- its tests still cannot be collected.""" + assert probe.extra_is_installed(("json", "mefor_no_such_module_1230")) is False + + +def test_a_missing_parent_package_reads_as_missing_not_as_present() -> None: + """``find_spec`` RAISES rather than returning None when the parent package is absent. If that + arm were not caught, the fhir extra (``fhir.resources``) would report as installed.""" + assert probe.extra_is_installed(("mefor_no_such_parent_1230.child",)) is False + + +def test_silent_when_every_extra_resolves(monkeypatch: Any) -> None: + """The direction this venv cannot show on its own: nothing missing means nothing printed.""" + monkeypatch.setattr(probe, "OPTIONAL_EXTRAS", {"stdlib": ("json",)}) + assert probe.missing_extras() == [] + assert probe.report_header_lines() == [] + reporter = _FakeReporter() + probe.write_incomplete_run_summary(reporter) + assert reporter.lines == [] + assert reporter.seps == [] + + +def test_loud_when_an_extra_is_absent(monkeypatch: Any) -> None: + monkeypatch.setattr( + probe, "OPTIONAL_EXTRAS", {"stdlib": ("json",), "ghost": ("mefor_no_such_module_1230",)} + ) + assert probe.missing_extras() == ["ghost"] + header = probe.report_header_lines() + assert len(header) == 1 + assert "ghost" in header[0] + + reporter = _FakeReporter() + probe.write_incomplete_run_summary(reporter) + body = "\n".join(reporter.lines) + assert "ghost" in body + assert "stdlib" not in body # only what is actually absent is named + assert any("INCOMPLETE RUN" in title for title in reporter.seps) + # The remedy has to be present and has to name the absent extra, or the banner states a problem + # without a way out and gets ignored. + assert 'pip install --constraint constraints.lock -e ".[dev,harness,ghost]"' in body + + +def test_the_real_extra_table_matches_pyproject() -> None: + """The table is hand-maintained beside pyproject.toml; assert it still names the five extras CI + installs, so a renamed or dropped extra fails here instead of silently never being reported.""" + assert set(probe.OPTIONAL_EXTRAS) == {"fhir", "dicom", "x12", "xml", "webauthn"} diff --git a/tests/test_security_doc_rate_limits.py b/tests/test_security_doc_rate_limits.py index 572f428b..08f4f486 100644 --- a/tests/test_security_doc_rate_limits.py +++ b/tests/test_security_doc_rate_limits.py @@ -532,23 +532,50 @@ def test_scope_guard_detects_a_planted_rescoping() -> None: ) -def test_ingest_plane_rate_limit_is_documented_as_existing_but_off() -> None: - """A silent omission reads as coverage, and so does an overstatement in the other direction. - - Until 2026-08-11 this guard asserted the doc said "no message-rate or volume limit exists", which - was true and load-bearing. The MLLP pacing build (ASVS 2.4.1 / 15.2.2) falsified it, so the guard - had to move with the code and the doc rather than be deleted -- the row must now state BOTH that - a control exists and that it ships OFF, because either half alone misleads: "exists" implies the - shipped default is bounded, and "none" is now simply false. +def test_ingest_plane_rate_limit_row_matches_the_code() -> None: + """The ingest row must describe the pacer's REACHABILITY as the code actually has it. + + This guard has moved twice. Until 2026-08-11 it asserted the doc said "no message-rate or volume + limit exists"; the MLLP pacing build (ASVS 2.4.1 / 15.2.2) falsified that, so it moved to + requiring both that a control exists and that it "ships OFF". BACKLOG #1249 falsified that + wording in turn: the pacer is built, but neither key is a parameter of the ``MLLP()`` factory and + ``connections.toml`` desugars through that SAME factory, so no documented surface can set either. + "Off" is the more dangerous half-truth, because a reader takes it to mean "then set it to on". + + WHAT THIS ASSERTS IS CONSISTENCY, NOT A PREFERENCE, AND THE DISTINCTION IS THE WHOLE POINT. + **#1249 is OPEN.** The owner has not chosen between exposing the two keys and rewording the row, + and a guard that pinned "NOT REACHABLE" as the correct state would settle that product question + BY BUILD -- the same shape as settling one by omission. So reachability is READ FROM THE + SIGNATURE and the doc is required to agree with whatever it says. Expose the keys and this test + demands the row stop saying unreachable; leave them out and it demands the row say so. Either + ruling stays cheap, and neither is pre-empted by a green run here. """ + import inspect + + from messagefoundry.config import wiring + + pacing_keys = {"max_messages_per_second", "message_burst"} + accepted = pacing_keys & set(inspect.signature(wiring.MLLP).parameters) + block = _section(_H_LIMITS) assert "Ingest plane" in block assert "max_messages_per_second" in block, ( "the ingest row must name the control that now exists" ) - assert "ships OFF" in block, "and must say it is not on by default, or 'exists' overstates it" - # The honest gaps stay named: an off default bounds nothing, and the raw-TCP intake never got it. - assert "still no volume bound" in block + if accepted: + assert "NOT REACHABLE" not in block, ( + f"MLLP() now accepts {sorted(accepted)}, so the row may no longer call the pacer " + "unreachable -- it must state the shipped default instead" + ) + else: + assert "NOT REACHABLE" in block, ( + "MLLP() accepts neither pacing key, so the row must say the pacer cannot be turned on " + "rather than that it is merely off by default" + ) + assert "NO message-RATE bound anywhere on the ingest plane" in block, ( + "and must name the resulting gap outright, not leave it inferred" + ) + # True under either ruling: the raw-TCP intake never got the pacer at all. assert "raw-TCP inbound" in block for cap in ("max_connections", "receive_timeout", "max_frame_bytes", "max_message_bytes"): assert cap in block, f"the ingest row must name the {cap} resource cap it DOES have"