Enable ruff security, logging and debugging rules (S, T20, LOG, G, DTZ, ERA, TD, FIX, EXE)

.github/bump_version.py

@@ -3,14 +3,20 @@
 import re
 import sys
 from pathlib import Path
+from typing import TextIO
+
+
+def _report(message: str, *, file: TextIO | None = None) -> None:
+    """Print one line of this command's output, to stdout unless ``file`` is given."""
+    print(message, file=file)
 
 
 def get_current_version(pyproject_path: Path) -> str:
     """Return the version in pyproject.toml, exiting with status 1 if it is absent."""
     text = pyproject_path.read_text()
     match = re.search(r'^version\s*=\s*"(\d+\.\d+\.\d+)"', text, re.MULTILINE)
     if not match:
-        print(
+        _report(
             "Could not find version in pyproject.toml",
             file=sys.stderr,
         )
@@ -28,7 +34,7 @@ def infer_bump(changelog_dir: Path) -> str:
         f for f in changelog_dir.iterdir() if f.is_file() and f.name != ".gitkeep"
     ]
     if not fragments:
-        print("No changelog fragments found", file=sys.stderr)
+        _report("No changelog fragments found", file=sys.stderr)
         sys.exit(1)
 
     categories = {f.suffix.lstrip(".") for f in fragments}
@@ -63,7 +69,7 @@ def update_file(path: Path, old_version: str, new_version: str) -> None:
     )
     if updated != text:
         path.write_text(updated)
-        print(f"  Updated {path}")
+        _report(f"  Updated {path}")
 
 
 def main() -> None:
@@ -76,7 +82,7 @@ def main() -> None:
     bump = infer_bump(changelog_dir)
     new = bump_version(current, bump)
 
-    print(f"Version: {current} -> {new} ({bump})")
+    _report(f"Version: {current} -> {new} ({bump})")
 
     update_file(pyproject, current, new)
 

AGENTS.md

@@ -119,14 +119,17 @@ To add or change a rule, read `docs/engineering/skills/rules-registry.md`.
 
 ### Code quality and types
 
-- `api.quality.alembic-revisions-unused-imports`: Alembic revision files are exempt from ruff lint rules, because they must stay exactly as Alembic generates them. (advisory: ruff per-file ignore of ALL for `migrations/*/versions/*.py`, not blocking)
+- `api.quality.alembic-revisions-unused-imports`: Ruff doesn't lint Alembic revision files (lint.exclude in pyproject.toml), because they must stay exactly as Alembic generates them. (advisory: nothing, not blocking)
+- `api.quality.ci-cd-scripts-ruff-exemptions`: CI/CD scripts under .github may print their results and fetch fixed URLs (T201, S310); every other ruff rule applies to them. (blocking: ruff per-file ignore of S310, T201 for `.github/**`)
+- `api.quality.command-line-modules-ruff-exemptions`: Application modules that also run as command-line programs (if __name__ == "__main__") may print their output (T201); every other ruff rule applies to them. (blocking: ruff per-file ignore of T201 for `policyengine_api/{data/v2/catalog/initialization,data/v2/household_migration_qualification,data/v2/policy_migration_qualification,data/v2/user_association_migration_qualification,worker_spm_release}.py`)
+- `api.quality.command-line-scripts-ruff-exemptions`: Command-line scripts under scripts/ may print their output, run other programs, fetch fixed URLs, stamp today's date, pick random samples and ignore a failed best-effort step (T201, S603, S607, S310, DTZ011, S311, S110); every other ruff rule applies to them. (blocking: ruff per-file ignore of DTZ011, S110, S310, S311, S603, S607, T201 for `scripts/**`)
 - `api.quality.environment-variables-load`: Required environment variables and credentials load in a deployed-like environment. (blocking: CI job `test_env_vars`)
 - `api.quality.mypy-v2`: Shared query models, FastAPI dependencies, v2 routes and services, and the user sign-in and account-deletion libraries pass mypy with untyped definitions disallowed. (blocking: mypy)
 - `api.quality.ruff-format`: All Python files are formatted with ruff format. (blocking: ruff format)
-- `api.quality.ruff-lint`: All Python code passes ruff check with every rule family selected in pyproject.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. (blocking: ruff check (ANN, BLE, D, E4, E7, E9, EM, F, FLY, FURB, I, PIE, PYI, RET, RSE, TC, TRY, UP, W))
+- `api.quality.ruff-lint`: All Python code passes ruff check with every rule family selected in pyproject.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. (blocking: ruff check (ANN, BLE, D, DTZ, E4, E7, E9, EM, ERA, EXE, F, FIX, FLY, FURB, G, I, LOG, PIE, PYI, RET, RSE, S, T20, TC, TD, TRY, UP, W))
 - `api.quality.runtime-evaluated-models-resolve`: Every package model is one ruff treats as runtime-evaluated (a base resolves, through the import in its own module, to a name in runtime-evaluated-base-classes, or is a same-module class that does), and every Pydantic model and SQLAlchemy mapper resolves its annotations once the package is imported. (blocking: pytest `tests/unit/test_runtime_evaluated_annotations.py`)
 - `api.quality.tests-pass`: The unit, contract and to-refactor test suites pass. (blocking: CI job `test`)
-- `api.quality.tests-ruff-exemptions`: Test files may use pytest's bare assert and need no docstrings, annotations or named constants for magic numbers (S101, D1, ANN, PLR2004); every other ruff rule applies to tests. (blocking: ruff per-file ignore of ANN, D1, PLR2004, S101 for `**/tests/**`)
+- `api.quality.tests-ruff-exemptions`: Test files may use pytest's bare assert and need no docstrings, annotations or named constants for magic numbers (S101, D1, ANN, PLR2004). They may also use fake credentials, naive datetimes, subprocess calls to repository scripts, SQL built from strings and prints (S105, S106, DTZ001, DTZ011, S603, S607, S608, T201). Every other ruff rule applies to tests. (blocking: ruff per-file ignore of ANN, D1, DTZ001, DTZ011, PLR2004, S101, S105, S106, S603, S607, S608, T201 for `**/tests/**`)
 
 ### Observability
 

changelog.d/ruff-security-logging.changed.md

@@ -0,0 +1 @@
+Enabled ruff's security, logging and debugging rules (S, T20, LOG, G, DTZ, ERA, TD, FIX, EXE): type-narrowing asserts became explicit checks, command-line scripts keep their printed output with reasons, commented-out code and TODO markers are gone, and every deliberate exception carries a reasoned suppression; error responses and published schemas are unchanged.

changelog.d/ruff-security-logging.fixed.md

@@ -0,0 +1 @@
+Removed the debug prints from the country validator and the report output and simulation routes, so requests no longer write context-free lines to stdout.

dashboard/experiments/gpt4_api_user.py

@@ -5,6 +5,7 @@
 from the resulting economic impact.
 """
 
+import ast
 import json
 import os
 import time
@@ -21,10 +22,10 @@
 
 @st.cache_data
 def get_embeddings_df() -> pd.DataFrame:
-    """Load parameter_embeddings.csv.gz, evaluating each embedding string to a list."""
+    """Load parameter_embeddings.csv.gz, parsing each embedding string to a list."""
     embeddings_df = pd.read_csv("parameter_embeddings.csv.gz", compression="gzip")
     embeddings_df.parameter_embedding = embeddings_df.parameter_embedding.apply(
-        lambda x: eval(x)
+        ast.literal_eval
     )
     return embeddings_df
 
@@ -46,8 +47,9 @@ def embed(prompt: str, engine: str = "text-embedding-ada-002") -> list[float]:
     "Revenue impact of changing the basic rate of income tax to 25%",
 )
 
-# First task: convert to:
-# { 'hmrc.income_tax.rates.uk[0].rate': { '2023-01-01.2024-01-01': 0.25 } }
+# First task: convert the question to a reform that maps each parameter path to
+# its values by period, for example hmrc.income_tax.rates.uk[0].rate set to 0.25
+# for the period 2023-01-01.2024-01-01.
 
 # We're going to do this with GPT4- we'll pass in the question, the related parameter metadata, and the task description.
 
@@ -112,7 +114,7 @@ def get_policyengine_impact(reform: object) -> object:
     payload = {"data": reform}
     url = "https://api.policyengine.org/uk/policy"
 
-    response = requests.post(url, json=payload)
+    response = requests.post(url, json=payload)  # noqa: S113  # a timeout would change this experiment's behaviour; it waits as long as the API takes
 
     # json.result.policy_id
 
@@ -122,13 +124,13 @@ def get_policyengine_impact(reform: object) -> object:
 
     url = f"https://api.policyengine.org/uk/economy/{policy_id}/over/1?time_period=2023&region=uk"
 
-    response = requests.get(url).json()
+    response = requests.get(url).json()  # noqa: S113  # a timeout would change this experiment's behaviour; it waits as long as the API takes
 
     # if 'status' = 'computing', wait 5 seconds and try again until 'status' = 'ok'
 
     while response["status"] == "computing":
         time.sleep(5)
-        response = requests.get(url).json()
+        response = requests.get(url).json()  # noqa: S113  # a timeout would change this experiment's behaviour; it waits as long as the API takes
 
     return response["result"]
 

docs/engineering/skills/rules-registry.md

@@ -69,7 +69,7 @@ gap is visible, rather than leaving it out.
 | --- | --- | --- |
 | `pytest` | `nodes` (`path::test` or `path::Class::test`), optional `project` (sub-project directory the paths are relative to) | Each test exists, and the CI job's command collects it |
 | `quality-guard` | `name`, optional `runner` | The name is in `GUARDS` in `scripts/run_quality_guards.py` |
-| `ruff` | `mode` = `format`, `lint` (with `codes`) or `per-file-ignore` (with `pattern`, `codes`) | Codes are selected / the ignore entry exists |
+| `ruff` | `mode` = `format`, `lint` (with `codes`) or `per-file-ignore` (with `pattern`, `codes`) | Codes are selected / the ignore entry exists and covers only [files allowed file-level exceptions](#file-level-exceptions) |
 | `mypy` | `paths` | Each path is in `[tool.mypy].files` |
 | `workflow-job` | optional `step` | The CI job exists (the job itself is the check) |
 | `pyright`, `import-linter` | `config` (+ `contract` for Import Linter) | The config file exists |
@@ -117,13 +117,35 @@ fail the guard yet.
 
 - **Ruff is never ratcheted.** Adopting a ruff rule family is one standalone PR
   that changes the select list and fixes every resulting violation. Don't use
-  `--add-noqa` or blanket per-file ignores.
+  `--add-noqa`. A finding that is kept gets an inline `# noqa: CODE  # reason`,
+  except in the files below.
 - Other enforcers (mypy, Pyright, Import Linter, custom guards) may start
   `ratcheted` with a baseline file of tolerated violations. Baselines may only
   shrink: the guard compares counts against the base branch.
 - Moving a rule from enforced/ratcheted to advisory/doc-only needs an explicit
   `[rule.downgrade] reason`.
 
+## File-level exceptions
+
+A ruff `per-file-ignores` entry switches rules off for whole files. It is
+allowed only where that's the norm rather than an exception to it:
+
+- **tests**: files under a `tests` or `authenticated_tests` directory;
+- **CI/CD scripts**: files under `.github`;
+- **command-line scripts**: files under a `scripts` directory, or modules that
+  run as a program through `if __name__ == "__main__":`.
+
+Everywhere else, keep a finding only with an inline suppression and its reason.
+
+Each entry needs a `per-file-ignore` rule (see below). The guard expands the
+entry's pattern like ruff does and fails when the pattern:
+
+- covers any other file;
+- is negated;
+- matches nothing.
+
+`scripts/rules/file_exceptions.py` holds these definitions.
+
 ## Hidden rules
 
 The guard fails when something enforces a rule that the registry doesn't