diff --git a/.github/codeql/codeql-config.yml b/.github/codeql/codeql-config.yml new file mode 100644 index 00000000..d10c2d4a --- /dev/null +++ b/.github/codeql/codeql-config.yml @@ -0,0 +1,35 @@ +# CodeQL configuration -- which of `security-and-quality`'s queries this repository acts on. +# +# The workflow's first run produced 205 findings. Reading them was the point, and this file +# records what the reading concluded. Two quality queries account for 112 of them, and in every +# case sampled the finding was the query meeting a Python idiom rather than a defect. They are +# excluded here, at the suite, rather than dismissed one alert at a time: a dismissal is bound to +# an alert, so the next Protocol method written would raise a fresh one and the judgment would +# have to be made again by whoever happened to see it. +# +# Everything else was acted on rather than silenced. `py/repeated-import`'s 19 findings were +# correct and the redundant imports were deleted; `py/bad-tag-filter` found a real case- +# sensitivity bug in a test helper and it was fixed. Nothing security-relevant is excluded here: +# every `error`-severity and every `security-severity` query in the suite still runs, and the +# five findings they produced were triaged individually and dismissed with a reason recorded in +# the code-scanning tab. + +name: "CTRLRun Python" + +query-filters: + # 79 findings, every one of them `...`. The Ellipsis literal is a statement with no effect, and + # it is also how Python spells an empty body. `state.py`, `approval.py`, `identity.py`, + # `receipt.py`, `adapter.py` and `conformance/store/backends.py` are largely Protocol + # definitions -- ARCHITECTURE.md §6 puts the protocols at the module boundaries -- and the + # tests use the same form for functions that exist only to be decorated by `@protect`. The + # query cannot distinguish either from a statement whose result someone forgot to use. + - exclude: + id: py/ineffectual-statement + + # 33 findings, all in `tests/` and `examples/cookbook/`, and all the same shape: `import + # ctrlrun` beside `from ctrlrun import protect`. In an example that is deliberate, because the + # reader is being shown both the package and the name; in a test it is how one file refers to + # a module for a patch and to a name for a `pytest.raises`. Style, and not the style this + # repository has chosen. + - exclude: + id: py/import-and-import-from diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index f06250f6..5d0d049a 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -40,6 +40,8 @@ jobs: # there is nothing to build and nothing to install for the analyser to see the code. build-mode: none queries: security-and-quality + # Which of that suite's queries this repository acts on, and why two of them are not. + config-file: ./.github/codeql/codeql-config.yml - uses: github/codeql-action/analyze@cdf488f595d80d6e07e03d4674febd5ab45fa938 # v4.37.9 with: diff --git a/src/ctrlrun/acs.py b/src/ctrlrun/acs.py index d5c73c44..95dd24dc 100644 --- a/src/ctrlrun/acs.py +++ b/src/ctrlrun/acs.py @@ -331,6 +331,11 @@ def report_what_happened() -> Any: try: self._control.resume(held, report_what_happened) except (NotExecuted, _Unknown): + # Not a failure to handle -- these two are how `report_what_happened` states the + # outcome, and `Control.resume` has already written the `failed` or `ambiguous` + # receipt before re-raising (control.py §5.5). There is nothing left to do, and + # turning either into an error response would tell ACS the hook broke when what + # actually happened is that the tool call did. pass except InvalidArgument: # No held suspension matches. A restarted Guardian, a result fired twice, or one diff --git a/tests/test_adapters_openai_agents.py b/tests/test_adapters_openai_agents.py index 80789190..b8587857 100644 --- a/tests/test_adapters_openai_agents.py +++ b/tests/test_adapters_openai_agents.py @@ -368,8 +368,6 @@ def test_T135b_the_adapter_reuses_the_sdks_primitive_and_reimplements_nothing(): import ast import inspect - import ctrlrun_openai_agents - source = inspect.getsource(ctrlrun_openai_agents) tree = ast.parse(source) called = { diff --git a/tests/test_docs_reference.py b/tests/test_docs_reference.py index 2b5648f0..9993103e 100644 --- a/tests/test_docs_reference.py +++ b/tests/test_docs_reference.py @@ -88,7 +88,6 @@ def test_every_rendered_api_signature_is_the_one_python_would_accept(): is the check that would have caught it. """ import inspect - import re import ctrlrun @@ -283,7 +282,6 @@ def test_every_quoted_verify_transcript_is_one_verify_actually_prints(): except the `policy` line, whose absolute path is machine-specific and is labelled as such on both pages. """ - import re import subprocess import sys import tempfile @@ -341,7 +339,6 @@ def test_every_api_page_says_how_to_import_the_thing_it_documents(): `JWTIdentityProvider`, `AcsControlHook` and `serve` all raise `MissingDependency` without their extra, and no page said which. """ - import re pages = sorted((DOCS / "reference" / "api").glob("*.mdx")) assert len(pages) > 50, len(pages) diff --git a/tests/test_docs_site.py b/tests/test_docs_site.py index 8c456b9c..cf85c50a 100644 --- a/tests/test_docs_site.py +++ b/tests/test_docs_site.py @@ -23,7 +23,6 @@ def _published() -> set[str]: """Every page path `docs.json` lists, so a Markdown document that is a site page is tested like one and a Markdown document that is not is left alone.""" - import json found: set[str] = set() @@ -89,7 +88,16 @@ def _body(page: Path) -> str: return _FRONTMATTER.sub("", page.read_text(encoding="utf-8"), count=1) -_SCRIPT = re.compile(r"", re.S) +# The end tag is matched the way a browser's parser reads one, not the way it is usually typed. +# Tag names are case-insensitive; whitespace is allowed around the slash and the name; and an +# end tag carrying attribute-like text -- `` -- still closes the element, because +# the parser ignores what it finds there rather than refusing the tag. A filter that misses any +# of those spellings leaves the block in the text, where a structured-data payload counts +# against the page's prose budget, which is the one thing this helper exists to prevent. +# +# `\bscript\b` on both ends so that a tag merely beginning with those letters -- `` +# -- neither opens nor closes a block. +_SCRIPT = re.compile(r"<\s*script\b.*?<\s*/\s*script\b[^>]*>", re.S | re.I) def _prose(page: Path) -> str: @@ -349,3 +357,47 @@ def nope() -> str: if found and "reaches a decision" not in line: wrong.append(f"{page.name}: {line.strip()[:90]!r}") assert wrong == [], wrong + + +@pytest.mark.parametrize( + ("opening", "closing"), + [ + (""), + (""), + (""), + (""), + (""), + (""), + ("< script", ""), + ], + ids=[ + "plain", + "upper-case", + "space-before-gt", + "space-after-slash", + "newline", + "attribute-like-text", + "space-in-start-tag-and-junk-in-end-tag", + ], +) +def test_the_prose_filter_strips_a_script_block_however_its_tags_are_written( + tmp_path: Path, opening: str, closing: str +): + """Every spelling of the tags closes the same block, and the word budget must see none of it. + + HTML tag names are case-insensitive and an end tag may carry whitespace before its `>`. A + filter that misses a spelling leaves the block in the text, where a structured-data payload + -- markup for a search engine, not words a reader reads -- is counted against the page's + budget and can push a page over it for a reason no author could see. + """ + page = tmp_path / "page.mdx" + page.write_text( + f'---\ntitle: t\n---\n\nvisible prose\n\n{opening} type="application/ld+json">\n' + f'{{"@type": "SoftwareApplication", "hidden": "wordone wordtwo"}}\n{closing}\n', + encoding="utf-8", + ) + + prose = _prose(page) + + assert "visible prose" in prose + assert "wordone" not in prose, f"{opening} ... {closing} reached the word budget" diff --git a/tests/test_framework_probe.py b/tests/test_framework_probe.py index e5efab97..9890ff66 100644 --- a/tests/test_framework_probe.py +++ b/tests/test_framework_probe.py @@ -153,7 +153,6 @@ def test_T123_the_version_is_read_from_the_installed_distribution(adapter): @pytest.mark.parametrize("adapter", all_adapters(), ids=lambda a: a.name) def test_T123_no_adapter_hard_codes_a_version(adapter): import inspect - import re source = inspect.getsource(type(adapter)) @@ -561,7 +560,6 @@ def test_the_results_directory_holds_a_run_and_the_table_rendered_from_it(): """`results/` was empty on purpose until there was a run somebody had read. There is one now, and the Markdown beside it is rendered from the JSON and never written by hand -- so the assertion is that the two agree, not that a file exists.""" - import json from framework_probe.results import to_markdown @@ -580,7 +578,6 @@ def test_the_results_directory_holds_a_run_and_the_table_rendered_from_it(): def test_the_published_run_is_what_the_readme_says_it_is(): """The README's numbers and the results file must not drift apart. A README quoting a run that is no longer the one checked in is the shape of every stale finding.""" - import json document = json.loads((PROBE_ROOT / "results" / "2026-09-05.json").read_text()) by_cell = {(row["framework"], row["scenario"]): row for row in document["results"]} diff --git a/tests/test_migrations.py b/tests/test_migrations.py index 4d01ce3a..c3d1739c 100644 --- a/tests/test_migrations.py +++ b/tests/test_migrations.py @@ -441,7 +441,6 @@ def test_T149b_the_recording_is_inside_the_migration_transaction(tmp_path, monke `_apply`: a first attempt at this test built its own copy of the logic and therefore proved nothing about the code -- the mutation stayed green. """ - import sqlite3 import ctrlrun.migrations as migrations @@ -480,7 +479,6 @@ def test_T149c_a_failed_migration_leaves_no_transaction_open(): residual is real -- the next writer meets `database is locked` -- so this opens that window on purpose rather than reading past it. """ - import sqlite3 import ctrlrun.migrations as migrations @@ -529,7 +527,6 @@ def test_T149d_concurrent_opens_all_succeed(tmp_path, shape): handler does not cover -- which `_enable_wal` fixes by treating another process winning that race as the success it is. Measured before: 38 of 60 and 5-of-6-per-trial. After: 120 of 120. """ - import sqlite3 import ctrlrun.migrations as migrations diff --git a/tests/test_packaging.py b/tests/test_packaging.py index 4f88e312..8e610eb2 100644 --- a/tests/test_packaging.py +++ b/tests/test_packaging.py @@ -452,7 +452,6 @@ def test_every_test_docs_claims_cites_exists(): renamed, the row keeps the old one, and the table reads as evidence while pointing at nothing. This is cheap and it is the only thing standing between the two. """ - import re claims = REPO_ROOT / "docs" / "CLAIMS.md" if not claims.exists(): # pragma: no cover - not a checkout @@ -556,7 +555,6 @@ def test_T136_an_adapter_depends_on_ctrlrun_and_never_the_reverse(): file, so there is no configuration in which both halves go unchecked: in a checkout this loop runs, and in an sdist the other test is what proves the directory should be missing. """ - import tomllib from pathlib import Path root = Path(__file__).resolve().parents[1] @@ -670,7 +668,6 @@ def test_T139_the_adapter_section_names_prevention_and_attribution(): def _claims(text: str) -> list[str]: """Sentences using a claim word **affirmatively**.""" - import re offending = [] for sentence in re.split(r"(?<=[.!?])\s+|\n\n", text): @@ -730,7 +727,6 @@ def test_the_claims_table_line_numbers_point_at_what_they_name(): be resolved mechanically. A drifted line for a reference with no symbol is not caught here, which is stated rather than left to be assumed. """ - import re claims = REPO_ROOT / "docs" / "CLAIMS.md" if not claims.exists(): # pragma: no cover - not a checkout @@ -800,7 +796,6 @@ def test_every_documented_install_names_a_distribution_this_repository_builds(): fact and `.github/workflows/publish.yml` is what makes it true; saying otherwise would be the kind of loosely-true assertion this suite keeps refusing. """ - import tomllib root = REPO_ROOT with (root / "pyproject.toml").open("rb") as handle: diff --git a/tests/test_verify_action.py b/tests/test_verify_action.py index abf17675..128e175e 100644 --- a/tests/test_verify_action.py +++ b/tests/test_verify_action.py @@ -413,7 +413,6 @@ def test_the_readme_quotes_the_real_verify_output(): that lies. The version line is normalised: it moves at every release, and the README is not the place that number is kept honest — `pyproject.toml` is. """ - import re report = run(AUTHORITY_PAYMENTS) printed = { @@ -552,7 +551,6 @@ def test_the_publish_script_fast_forwards_on_the_second_run(tmp_path): broken script passes — which it did, until the line was added. A negative test proves nothing unless the thing it forbids would otherwise happen. """ - import json import subprocess script = _publish_script() @@ -650,7 +648,6 @@ def publish(message: str) -> subprocess.CompletedProcess[str]: def test_the_publish_script_is_a_no_op_when_the_badge_has_not_changed(tmp_path): """A push per green build, for a file nobody edited, is noise in the history.""" - import json import subprocess script = _publish_script()