diff --git a/.github/bump_version.py b/.github/bump_version.py index e29c6bf8c..dee431c03 100644 --- a/.github/bump_version.py +++ b/.github/bump_version.py @@ -3,6 +3,12 @@ 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: @@ -10,7 +16,7 @@ def get_current_version(pyproject_path: Path) -> str: 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) diff --git a/.github/find-api-model-versions.py b/.github/find-api-model-versions.py index 9ae2ab7d4..915dc9d67 100644 --- a/.github/find-api-model-versions.py +++ b/.github/find-api-model-versions.py @@ -113,8 +113,8 @@ def find_api_model_versions_and_output_to_github() -> None: if args.shell: for key, value in find_api_model_versions().items(): - print(f"{key}={shlex.quote(value)}") + print(f"{key}={shlex.quote(value)}") # noqa: T201 # the shell assignments are this command's output else: find_api_model_versions_and_output_to_github() - print("API model versions found and written to GitHub environment.") + print("API model versions found and written to GitHub environment.") # noqa: T201 # the command reports its result sys.exit(0) diff --git a/.github/scripts/check_updates.py b/.github/scripts/check_updates.py old mode 100644 new mode 100755 index e3c232e9f..ba4fbea3b --- a/.github/scripts/check_updates.py +++ b/.github/scripts/check_updates.py @@ -138,16 +138,16 @@ def main() -> int: changelog_text = fetch_changelog(args.package) if not changelog_text: - print(f"Could not fetch changelog for {args.package}.", file=sys.stderr) + print(f"Could not fetch changelog for {args.package}.", file=sys.stderr) # noqa: T201 # the command reports a missing changelog on stderr return 0 entries = parse_changelog(changelog_text) changes = get_changes_between(entries, args.old_version, args.new_version) if changes: - print(format_changes(changes)) + print(format_changes(changes)) # noqa: T201 # the formatted changelog is this command's output else: - print("No changelog entries found between these versions.") + print("No changelog entries found between these versions.") # noqa: T201 # the command reports that nothing changed return 0 diff --git a/.github/scripts/verify_observability_collector.py b/.github/scripts/verify_observability_collector.py index 2483183e5..f3d64eec4 100644 --- a/.github/scripts/verify_observability_collector.py +++ b/.github/scripts/verify_observability_collector.py @@ -199,12 +199,12 @@ def _raise_for_partial_rejection( def _get_json(url: str, access_token: str) -> dict[str, object] | None: - request = Request( + request = Request( # noqa: S310 # callers pass only https Google Cloud API URLs url, headers={"Authorization": f"Bearer {access_token}"}, ) try: - with urlopen(request, timeout=15) as response: + with urlopen(request, timeout=15) as response: # noqa: S310 # callers pass only https Google Cloud API URLs return json.load(response) except HTTPError as error: if error.code == 404: @@ -391,7 +391,7 @@ def main() -> None: metric_location=args.metric_location, access_token=os.environ["GOOGLE_OAUTH_ACCESS_TOKEN"], ) - print( + print( # noqa: T201 # the command reports the verified ingestion "Verified collector ingestion " f"(verification_id={probe.verification_id}, trace_id={probe.trace_id})" ) diff --git a/AGENTS.md b/AGENTS.md index 12fb57d2b..d656fc999 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -122,7 +122,7 @@ To add or change a rule, read `docs/engineering/skills/rules-registry.md`. - `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, and v2 routes and services 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.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/**`) diff --git a/changelog.d/ruff-security-logging.changed.md b/changelog.d/ruff-security-logging.changed.md new file mode 100644 index 000000000..b3a9247bf --- /dev/null +++ b/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. diff --git a/changelog.d/ruff-security-logging.fixed.md b/changelog.d/ruff-security-logging.fixed.md new file mode 100644 index 000000000..a1c1c0ea9 --- /dev/null +++ b/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. diff --git a/dashboard/experiments/gpt4_api_user.py b/dashboard/experiments/gpt4_api_user.py index 1fd37047b..cf3dfbe61 100644 --- a/dashboard/experiments/gpt4_api_user.py +++ b/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®ion=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"] diff --git a/docs/generated/rules.md b/docs/generated/rules.md index 4817dae11..b97f92f7f 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -268,7 +268,7 @@ flowchart LR | `api.quality.environment-variables-load` | Required environment variables and credentials load in a deployed-like environment. | mechanical | enforced | yes | CI job `test_env_vars` | `pr.yml:test_env_vars` | — | | `api.quality.mypy-v2` | Shared query models, FastAPI dependencies, and v2 routes and services pass mypy with untyped definitions disallowed. | mechanical | enforced | yes | mypy | `pr.yml:typecheck-v2` | — | | `api.quality.ruff-format` | All Python files are formatted with ruff format. | mechanical | enforced | yes | ruff format | `pr.yml:lint` | — | -| `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. | mechanical | enforced | yes | ruff check (ANN, BLE, D, E4, E7, E9, EM, F, FLY, FURB, I, PIE, PYI, RET, RSE, TC, TRY, UP, W) | `pr.yml:lint` | — | +| `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. | mechanical | enforced | yes | 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) | `pr.yml:lint` | — | | `api.quality.tests-pass` | The unit, contract and to-refactor test suites pass. | mechanical | enforced | yes | CI job `test` | `pr.yml: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. | mechanical | enforced | yes | ruff per-file ignore of ANN, D1, PLR2004, S101 for `**/tests/**` | `pr.yml:lint` | — | @@ -287,7 +287,7 @@ flowchart LR e_ruff_format["ruff format"] j_CI_pr_yml_lint["CI pr.yml:lint"] r_api_quality_ruff_lint("api.quality.ruff-lint (enforced)") - e_ruff_check__ANN__BLE__D__E4__E7__E9__EM__F__FLY__FURB__I__PIE__PYI__RET__RSE__TC__TRY__UP__W_["ruff check (ANN, BLE, D, E4, E7, E9, EM, F, FLY, FURB, I, PIE, PYI, RET, RSE, TC, TRY, UP, W)"] + e_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_["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)"] r_api_quality_tests_pass("api.quality.tests-pass (enforced)") e_CI_job_test["CI job test"] j_CI_pr_yml_test["CI pr.yml:test"] @@ -301,8 +301,8 @@ flowchart LR e_mypy --> j_CI_pr_yml_typecheck_v2 r_api_quality_ruff_format --> e_ruff_format e_ruff_format --> j_CI_pr_yml_lint - r_api_quality_ruff_lint --> e_ruff_check__ANN__BLE__D__E4__E7__E9__EM__F__FLY__FURB__I__PIE__PYI__RET__RSE__TC__TRY__UP__W_ - e_ruff_check__ANN__BLE__D__E4__E7__E9__EM__F__FLY__FURB__I__PIE__PYI__RET__RSE__TC__TRY__UP__W_ --> j_CI_pr_yml_lint + r_api_quality_ruff_lint --> e_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_ + e_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_ --> j_CI_pr_yml_lint r_api_quality_tests_pass --> e_CI_job_test e_CI_job_test --> j_CI_pr_yml_test r_api_quality_tests_ruff_exemptions --> e_ruff_per_file_ignore_of_ANN__D1__PLR2004__S101_for____tests___ diff --git a/policyengine_api/api.py b/policyengine_api/api.py index df4fea71c..f2beba2fb 100644 --- a/policyengine_api/api.py +++ b/policyengine_api/api.py @@ -10,7 +10,7 @@ def log_timing(message: str) -> None: """Print a startup milestone to stderr with the seconds since import began.""" elapsed = time.time() - start_time - print(f"[{elapsed:.2f}s] {message}", file=sys.stderr, flush=True) + print(f"[{elapsed:.2f}s] {message}", file=sys.stderr, flush=True) # noqa: T201 # startup timings go straight to stderr while the app imports log_timing("Basic imports completed") @@ -31,8 +31,6 @@ def log_timing(message: str) -> None: log_timing("Caching utilities import completed") -# from werkzeug.middleware.profiler import ProfilerMiddleware - # Endpoints from policyengine_api.routes.error_routes import error_bp diff --git a/policyengine_api/asgi_factory.py b/policyengine_api/asgi_factory.py index 925ffb5e6..88e1166c9 100644 --- a/policyengine_api/asgi_factory.py +++ b/policyengine_api/asgi_factory.py @@ -208,13 +208,13 @@ async def add_request_context_and_migration_logging( request.state.policyengine_request_id = request_id _asgi_request_id.reset(context_token) context_token = _asgi_request_id.set(request_id) - except Exception: # noqa: BLE001 # telemetry must never fail the request + except Exception: # noqa: BLE001, S110 # telemetry must never fail the request pass try: request_runtime.set_context( request_id=request_id, ) - except Exception: # noqa: BLE001 # telemetry must never fail the request + except Exception: # noqa: BLE001, S110 # telemetry must never fail the request pass def log_native_route(status_code: int) -> None: @@ -233,7 +233,7 @@ def log_native_route(status_code: int) -> None: ), route_impl=RouteImplementation.FASTAPI_NATIVE, ) - except Exception: # noqa: BLE001 # migration logging must never fail the request + except Exception: # noqa: BLE001, S110 # migration logging must never fail the request pass def finish_native_route( @@ -245,18 +245,18 @@ def finish_native_route( resolved_route = getattr(request.scope.get("route"), "path", initial_route) try: request_runtime.update_request_route(resolved_route) - except Exception: # noqa: BLE001 # telemetry must never fail the request + except Exception: # noqa: BLE001, S110 # telemetry must never fail the request pass try: request_runtime.update_request_status(status_code) - except Exception: # noqa: BLE001 # telemetry must never fail the request + except Exception: # noqa: BLE001, S110 # telemetry must never fail the request pass try: request_runtime.end_request( status_code=status_code, error=error, ) - except Exception: # noqa: BLE001 # telemetry must never fail the request + except Exception: # noqa: BLE001, S110 # telemetry must never fail the request pass try: @@ -280,13 +280,13 @@ def finish_native_route( request_runtime.set_context( observability_id=response_observability_id ) - except Exception: # noqa: BLE001 # telemetry must never fail the request + except Exception: # noqa: BLE001, S110 # telemetry must never fail the request pass if native_request: try: for name, value in request_runtime.response_headers().items(): response.headers[name] = value - except Exception: # noqa: BLE001 # telemetry must never fail the request + except Exception: # noqa: BLE001, S110 # telemetry must never fail the request pass _apply_request_id_header(response, request_id) if response_observability_id is not None: diff --git a/policyengine_api/constants.py b/policyengine_api/constants.py index e9c97ff61..4ff7c2ed1 100644 --- a/policyengine_api/constants.py +++ b/policyengine_api/constants.py @@ -18,7 +18,7 @@ UPDATE = "UPDATE" LIST = "LIST" VERSION = "3.38.1" -CURRENT_YEAR = str(datetime.now().year) +CURRENT_YEAR = str(datetime.now().year) # noqa: DTZ005 # the default year is the host's local calendar year COUNTRIES = ("uk", "us", "ca", "ng", "il") COUNTRY_PACKAGE_NAMES = ( "policyengine_uk", @@ -220,8 +220,8 @@ def _build_runtime_cache_version( schema_version, ) ) - digest = hashlib.sha1(raw.encode("utf-8")).hexdigest()[:digest_length] - return f"{prefix}{schema_version}{digest}" + digest = hashlib.sha1(raw.encode("utf-8"), usedforsecurity=False).hexdigest() + return f"{prefix}{schema_version}{digest[:digest_length]}" def get_economy_impact_cache_version( diff --git a/policyengine_api/data/v2/catalog/extraction.py b/policyengine_api/data/v2/catalog/extraction.py index 7a760a385..4111c966a 100644 --- a/policyengine_api/data/v2/catalog/extraction.py +++ b/policyengine_api/data/v2/catalog/extraction.py @@ -179,7 +179,7 @@ def normalize_json_value(value: object) -> JSONValue: def _aware_datetime(value: object, *, field_name: str) -> datetime: if isinstance(value, date) and not isinstance(value, datetime): - value = datetime(value.year, value.month, value.day) + value = datetime(value.year, value.month, value.day, tzinfo=UTC) if not isinstance(value, datetime): msg = f"{field_name} must be a date or datetime" raise CatalogExtractionError(msg) diff --git a/policyengine_api/data/v2/catalog/initialization.py b/policyengine_api/data/v2/catalog/initialization.py index 10efdda6f..83df2241e 100644 --- a/policyengine_api/data/v2/catalog/initialization.py +++ b/policyengine_api/data/v2/catalog/initialization.py @@ -79,9 +79,9 @@ def main() -> int: try: evidence = initialize_catalog() except Exception as error: # noqa: BLE001 # command must return safe evidence - print(json.dumps(_error_payload(error), sort_keys=True), file=sys.stderr) + print(json.dumps(_error_payload(error), sort_keys=True), file=sys.stderr) # noqa: T201 # the command's JSON evidence is its output return 1 - print(json.dumps(evidence.as_dict(), sort_keys=True)) + print(json.dumps(evidence.as_dict(), sort_keys=True)) # noqa: T201 # the command's JSON evidence is its output return 0 diff --git a/policyengine_api/data/v2/household_migration_qualification.py b/policyengine_api/data/v2/household_migration_qualification.py index ce679323e..609d740f7 100644 --- a/policyengine_api/data/v2/household_migration_qualification.py +++ b/policyengine_api/data/v2/household_migration_qualification.py @@ -262,9 +262,9 @@ def main() -> int: try: evidence = qualify_household_migration_if_pending() except Exception as error: # noqa: BLE001 # command must emit safe evidence - print(json.dumps(_error_payload(error), sort_keys=True), file=sys.stderr) + print(json.dumps(_error_payload(error), sort_keys=True), file=sys.stderr) # noqa: T201 # the command's JSON evidence is its output return 1 - print(json.dumps(evidence.as_dict(), sort_keys=True)) + print(json.dumps(evidence.as_dict(), sort_keys=True)) # noqa: T201 # the command's JSON evidence is its output return 0 diff --git a/policyengine_api/data/v2/policy_migration_qualification.py b/policyengine_api/data/v2/policy_migration_qualification.py index beb03db24..46a3b3486 100644 --- a/policyengine_api/data/v2/policy_migration_qualification.py +++ b/policyengine_api/data/v2/policy_migration_qualification.py @@ -247,9 +247,9 @@ def main() -> int: try: evidence = qualify_policy_migration_if_pending() except Exception as error: # noqa: BLE001 # command must emit safe evidence - print(json.dumps(_error_payload(error), sort_keys=True), file=sys.stderr) + print(json.dumps(_error_payload(error), sort_keys=True), file=sys.stderr) # noqa: T201 # the command's JSON evidence is its output return 1 - print(json.dumps(evidence.as_dict(), sort_keys=True)) + print(json.dumps(evidence.as_dict(), sort_keys=True)) # noqa: T201 # the command's JSON evidence is its output return 0 diff --git a/policyengine_api/data/v2/settings.py b/policyengine_api/data/v2/settings.py index 2b951e28f..33d8fdfea 100644 --- a/policyengine_api/data/v2/settings.py +++ b/policyengine_api/data/v2/settings.py @@ -20,7 +20,7 @@ from collections.abc import Callable, Mapping V2_RUNTIME_DATABASE_URL = "V2_RUNTIME_DATABASE_URL" -V2_RUNTIME_DATABASE_URL_SECRET_RESOURCE = "V2_RUNTIME_DATABASE_URL_SECRET_RESOURCE" +V2_RUNTIME_DATABASE_URL_SECRET_RESOURCE = "V2_RUNTIME_DATABASE_URL_SECRET_RESOURCE" # noqa: S105 # an environment variable name, not a secret V2_MIGRATION_DATABASE_URL = "V2_MIGRATION_DATABASE_URL" V2_DATA_WRITE_DATABASE_URL = "V2_DATA_WRITE_DATABASE_URL" V2_SUPABASE_PROJECT_REF = "V2_SUPABASE_PROJECT_REF" diff --git a/policyengine_api/fastapi_routes/v2/metadata/common.py b/policyengine_api/fastapi_routes/v2/metadata/common.py index b46f759d4..366860544 100644 --- a/policyengine_api/fastapi_routes/v2/metadata/common.py +++ b/policyengine_api/fastapi_routes/v2/metadata/common.py @@ -102,5 +102,5 @@ def read_resource( if reader is not None: try: reader.close() - except Exception: # noqa: BLE001 # best-effort close must not replace the typed response + except Exception: # noqa: BLE001, S110 # best-effort close must not replace the typed response pass diff --git a/policyengine_api/gcp_logging.py b/policyengine_api/gcp_logging.py index f03cdfb27..e8fb2704d 100644 --- a/policyengine_api/gcp_logging.py +++ b/policyengine_api/gcp_logging.py @@ -42,7 +42,7 @@ def log_struct( severity=severity, attributes=attributes, ) - except Exception: # noqa: BLE001 # logging must never affect the caller + except Exception: # noqa: BLE001, S110 # logging must never affect the caller pass diff --git a/policyengine_api/libs/gateway_auth.py b/policyengine_api/libs/gateway_auth.py index 1388b91e7..c940f61db 100644 --- a/policyengine_api/libs/gateway_auth.py +++ b/policyengine_api/libs/gateway_auth.py @@ -23,8 +23,8 @@ GATEWAY_AUTH_ISSUER_ENV = "GATEWAY_AUTH_ISSUER" GATEWAY_AUTH_AUDIENCE_ENV = "GATEWAY_AUTH_AUDIENCE" GATEWAY_AUTH_CLIENT_ID_ENV = "GATEWAY_AUTH_CLIENT_ID" -GATEWAY_AUTH_CLIENT_SECRET_ENV = "GATEWAY_AUTH_CLIENT_SECRET" -GATEWAY_AUTH_CLIENT_SECRET_RESOURCE_ENV = "GATEWAY_AUTH_CLIENT_SECRET_RESOURCE" +GATEWAY_AUTH_CLIENT_SECRET_ENV = "GATEWAY_AUTH_CLIENT_SECRET" # noqa: S105 # an environment variable name, not a secret +GATEWAY_AUTH_CLIENT_SECRET_RESOURCE_ENV = "GATEWAY_AUTH_CLIENT_SECRET_RESOURCE" # noqa: S105 # an environment variable name, not a secret GATEWAY_AUTH_REQUIRED_ENV = "GATEWAY_AUTH_REQUIRED" GATEWAY_AUTH_CORE_ENV_VARS = ( diff --git a/policyengine_api/libs/simulation_entrypoint.py b/policyengine_api/libs/simulation_entrypoint.py index 1472dc35d..09c580ebb 100644 --- a/policyengine_api/libs/simulation_entrypoint.py +++ b/policyengine_api/libs/simulation_entrypoint.py @@ -193,7 +193,7 @@ def __init__(self, entrypoint: str | None = None) -> None: "GATEWAY_AUTH_CLIENT_SECRET." ) raise GatewayAuthError(msg) - print( + print( # noqa: T201 # the unauthenticated-client warning goes to stderr whatever the logging setup "SimulationEntrypointClient initialized without gateway auth; " "all GATEWAY_AUTH_* env vars are unset and " "GATEWAY_AUTH_REQUIRED is not enabled.", diff --git a/policyengine_api/migration_logging.py b/policyengine_api/migration_logging.py index 4de643c09..0f9d8408d 100644 --- a/policyengine_api/migration_logging.py +++ b/policyengine_api/migration_logging.py @@ -98,7 +98,7 @@ def set_request_migration_context() -> None: if runtime is not None: try: runtime.set_context(request_id=flask.g.request_id) - except Exception: # noqa: BLE001 # telemetry must never fail the request + except Exception: # noqa: BLE001, S110 # telemetry must never fail the request pass @app.after_request @@ -140,7 +140,7 @@ def log_request_migration_context(response: flask.Response) -> flask.Response: except Exception: # noqa: BLE001 # logs the traceback below, guarded so logging cannot fail the response try: app.logger.exception("Failed to log migration request context") - except Exception: # noqa: BLE001 # a logging failure must not replace the response + except Exception: # noqa: BLE001, S110 # a logging failure must not replace the response pass return response diff --git a/policyengine_api/observability/runtime.py b/policyengine_api/observability/runtime.py index fe4af6731..8662567f2 100644 --- a/policyengine_api/observability/runtime.py +++ b/policyengine_api/observability/runtime.py @@ -76,5 +76,5 @@ def set_runtime_context(**attributes: object) -> None: """Bind local telemetry attributes without affecting application behavior.""" try: runtime.set_context(**attributes) - except Exception: # noqa: BLE001 # telemetry must never affect application behavior + except Exception: # noqa: BLE001, S110 # telemetry must never affect application behavior pass diff --git a/policyengine_api/request_context.py b/policyengine_api/request_context.py index 832a75f05..e8eb7c270 100644 --- a/policyengine_api/request_context.py +++ b/policyengine_api/request_context.py @@ -63,7 +63,7 @@ def _bind_observability_id(observability_id: str) -> str: from policyengine_api.observability import get_runtime get_runtime().set_context(observability_id=observability_id) - except Exception: # noqa: BLE001 # telemetry must never fail the request + except Exception: # noqa: BLE001, S110 # telemetry must never fail the request pass return observability_id diff --git a/policyengine_api/routes/household_routes.py b/policyengine_api/routes/household_routes.py index f49aabf17..26e7b6df0 100644 --- a/policyengine_api/routes/household_routes.py +++ b/policyengine_api/routes/household_routes.py @@ -54,6 +54,10 @@ _ViewResult = TypeVar("_ViewResult") +# Standard-library logger for tracebacks; ``logger`` is the structured +# log_struct facade. +module_logger = logging.getLogger(__name__) + household_bp = Blueprint("household", __name__) household_service = HouseholdService() household_calculation_service = HouseholdCalculationService() @@ -105,7 +109,7 @@ def _household_persistence_failure( }, severity="ERROR", ) - except Exception: # noqa: BLE001 # metric logging must not replace the error response + except Exception: # noqa: BLE001, S110 # metric logging must not replace the error response pass message = ( "Household database is temporarily unavailable; please try again later." @@ -412,7 +416,7 @@ def get_household_under_policy( response = _spm_error_response(error) if response is not None: return response - logging.exception( + module_logger.exception( "Error calculating household #%s under policy #%s", household_id, policy_id, @@ -446,7 +450,7 @@ def _calculate() -> dict | Response: if response is not None: return response start_observability_id() - logging.exception("Error calculating household under policy") + module_logger.exception("Error calculating household under policy") return _make_error_response( f"Error calculating household under policy: {error}", 500, @@ -476,7 +480,7 @@ def _calculate() -> dict | Response: response = _spm_error_response(error) if response is not None: return response - logging.exception("Error calculating household under policy") + module_logger.exception("Error calculating household under policy") return _make_error_response( f"Error calculating household under policy: {error}", 500, diff --git a/policyengine_api/routes/policy_routes.py b/policyengine_api/routes/policy_routes.py index dc1433e28..524506bda 100644 --- a/policyengine_api/routes/policy_routes.py +++ b/policyengine_api/routes/policy_routes.py @@ -114,7 +114,7 @@ def _user_policy_persistence_failure( }, severity="ERROR", ) - except Exception: # noqa: BLE001 # a logging failure must not replace the persistence response + except Exception: # noqa: BLE001, S110 # a logging failure must not replace the persistence response pass message = ( diff --git a/policyengine_api/routes/report_output_routes.py b/policyengine_api/routes/report_output_routes.py index 3048358e3..0975a853f 100644 --- a/policyengine_api/routes/report_output_routes.py +++ b/policyengine_api/routes/report_output_routes.py @@ -1,6 +1,7 @@ """Flask routes for creating, reading and updating v1 report outputs.""" import json +import logging import jsonschema import pydantic @@ -15,6 +16,8 @@ ) from policyengine_api.utils.payload_validators import validate_country +logger = logging.getLogger(__name__) + report_output_bp = Blueprint("report_output", __name__) report_output_service = ReportOutputService() @@ -62,7 +65,6 @@ def create_report_output(country_id: str) -> Response: - year (str | None): The year for the report (optional) """ - print(f"Creating report output for country {country_id}") payload = request.json if payload is None: msg = "Payload missing from request" @@ -146,8 +148,6 @@ def get_report_output(country_id: str, report_id: int) -> Response: report_id (int): The report output ID. """ - print(f"Getting report output {report_id} for country {country_id}") - view = report_output_service.resolve_report_output(country_id, report_id) if view is None: msg = f"Report #{report_id} not found." @@ -193,7 +193,7 @@ def update_report_output(country_id: str) -> Response: report_id = payload.get("id") output = payload.get("output") error_message = payload.get("error_message") - print(f"Updating report #{report_id} for country {country_id}") + logger.debug("Updating report #%s for country %s", report_id, country_id) # Validate status if provided if status is not None and status not in [ diff --git a/policyengine_api/routes/simulation_routes.py b/policyengine_api/routes/simulation_routes.py index bffb45680..d3d127d9e 100644 --- a/policyengine_api/routes/simulation_routes.py +++ b/policyengine_api/routes/simulation_routes.py @@ -1,6 +1,7 @@ """Flask routes for creating, reading and updating v1 simulation records.""" import json +import logging import jsonschema import pydantic @@ -13,6 +14,8 @@ from policyengine_api.spm import SPMValidationError from policyengine_api.utils.payload_validators import validate_country +logger = logging.getLogger(__name__) + simulation_bp = Blueprint("simulation", __name__) simulation_service = SimulationService() @@ -39,9 +42,10 @@ def _serialize_v1_simulation(simulation: Simulation) -> dict: if value is not None and not isinstance(value, str): response[field] = json.dumps(value) - # TODO: Remove this compatibility projection when the v1 contract is - # retired. New v2 response models should expose these fields as native JSON - # objects instead of carrying the historical JSON-in-a-string shape forward. + # This JSON-in-a-string projection exists only for the v1 contract and can + # go when that contract is retired. v2 response models should expose these + # fields as native JSON objects rather than carry the historical shape + # forward. return response @@ -67,8 +71,6 @@ def create_simulation(country_id: str) -> Response: - policy_id (int): The policy ID """ - print(f"Creating simulation for country {country_id}") - payload = request.json if payload is None: msg = "Payload missing from request" @@ -152,8 +154,6 @@ def get_simulation(country_id: str, simulation_id: int) -> Response: simulation_id (int): The simulation ID. """ - print(f"Getting simulation {simulation_id} for country {country_id}") - if not isinstance(simulation_id, int): msg = "simulation_id must be an integer" raise BadRequest(msg) @@ -209,7 +209,7 @@ def update_simulation(country_id: str) -> Response: simulation_id = payload.get("id") output = payload.get("output") error_message = payload.get("error_message") - print(f"Updating simulation #{simulation_id} for country {country_id}") + logger.debug("Updating simulation #%s for country %s", simulation_id, country_id) # Validate status if provided if status is not None and status not in ["pending", "complete", "error"]: diff --git a/policyengine_api/routes/user_profile_routes.py b/policyengine_api/routes/user_profile_routes.py index 1ab09dc6d..3cc175b5b 100644 --- a/policyengine_api/routes/user_profile_routes.py +++ b/policyengine_api/routes/user_profile_routes.py @@ -126,8 +126,9 @@ def update_user_profile(country_id: str) -> Response: msg = "No user data provided in request" raise BadRequest(msg) - # TODO: we should validate the payload - # to ensure type safety https://github.com/PolicyEngine/policyengine-api/issues/2054 + # The payload's field types are not validated here. Request validation was + # proposed in https://github.com/PolicyEngine/policyengine-api/issues/2054, + # which was closed as not planned. user_id = payload.pop("user_id") username = payload.pop("username", None) primary_country = payload.pop("primary_country", None) diff --git a/policyengine_api/runtime_cache/reform_impacts.py b/policyengine_api/runtime_cache/reform_impacts.py index 4d204fe66..decb7ba25 100644 --- a/policyengine_api/runtime_cache/reform_impacts.py +++ b/policyengine_api/runtime_cache/reform_impacts.py @@ -537,7 +537,7 @@ def matching( selected, key=lambda impact: ( impact.options_hash == options_hash, - impact.start_time or datetime.min, + impact.start_time or datetime.min, # noqa: DTZ901 # start_time is a naive column, so the fallback must be naive to compare with it impact.reform_impact_id, ), reverse=True, diff --git a/policyengine_api/services/__init__.py b/policyengine_api/services/__init__.py index 5bd08f8f8..0f71c72b0 100644 --- a/policyengine_api/services/__init__.py +++ b/policyengine_api/services/__init__.py @@ -1,3 +1 @@ """Business logic for the v1 API resources; ``v2`` holds the v2 resource services.""" - -# from .economy_service import EconomyService, JobService, PolicyService diff --git a/policyengine_api/services/economy_service.py b/policyengine_api/services/economy_service.py index a91ffa945..1fbfd2c7e 100644 --- a/policyengine_api/services/economy_service.py +++ b/policyengine_api/services/economy_service.py @@ -1384,7 +1384,7 @@ def _record_uncertifiable_stored_impact( error_code=error.code, execution_id=most_recent_impact.execution_id, ) - except Exception: # noqa: BLE001 # best effort: the typed SPM error is the answer, and the failure is already logged + except Exception: # noqa: BLE001, S110 # best effort: the typed SPM error is the answer, and the failure is already logged pass @observability_runtime.span(ECONOMY_ANNUAL_STAGES.name(Stage.ECONOMY_READ_FAILED)) @@ -1786,7 +1786,7 @@ def _set_reform_impact_computing( status=ImpactStatus.COMPUTING.value, api_version=setup_options.api_version, reform_impact_json={}, - start_time=datetime.datetime.now(), + start_time=datetime.datetime.now(), # noqa: DTZ005 # start_time is a naive column that has always held local time execution_id=execution_id, observability_id=setup_options.observability_id, ) diff --git a/policyengine_api/services/household_calculation_service.py b/policyengine_api/services/household_calculation_service.py index 8ceb14170..3f54eeabc 100644 --- a/policyengine_api/services/household_calculation_service.py +++ b/policyengine_api/services/household_calculation_service.py @@ -110,7 +110,7 @@ def get_household_year(household: dict) -> int | str: This is the period used when filling in missing yearly variables. """ - household_year: int | str = date.today().year + household_year: int | str = date.today().year # noqa: DTZ011 # the default period is the host's local calendar year household_age_list = list( household.get("people", {}).get("you", {}).get("age", {}).keys() ) diff --git a/policyengine_api/services/household_mirroring.py b/policyengine_api/services/household_mirroring.py index 2e7feb5d5..7ee534d87 100644 --- a/policyengine_api/services/household_mirroring.py +++ b/policyengine_api/services/household_mirroring.py @@ -113,7 +113,7 @@ def _log_copy_operation( }, severity="INFO" if outcome == "ok" else "ERROR", ) - except Exception: # noqa: BLE001 # metric logging must not change the mirror outcome + except Exception: # noqa: BLE001, S110 # metric logging must not change the mirror outcome pass diff --git a/policyengine_api/services/policy_mirroring.py b/policyengine_api/services/policy_mirroring.py index 22ba5f023..64209c9a6 100644 --- a/policyengine_api/services/policy_mirroring.py +++ b/policyengine_api/services/policy_mirroring.py @@ -117,7 +117,7 @@ def _log_mirror_event( }, severity="INFO" if outcome == "ok" else "ERROR", ) - except Exception: # noqa: BLE001 # metric logging must not change the mirror outcome + except Exception: # noqa: BLE001, S110 # metric logging must not change the mirror outcome pass diff --git a/policyengine_api/services/user_policy_mirroring.py b/policyengine_api/services/user_policy_mirroring.py index bd18019e8..c14fe6c60 100644 --- a/policyengine_api/services/user_policy_mirroring.py +++ b/policyengine_api/services/user_policy_mirroring.py @@ -144,7 +144,7 @@ def _log_mirror_operation( }, severity="INFO" if outcome == "ok" else "ERROR", ) - except Exception: # noqa: BLE001 # metric logging must not change the mirror outcome + except Exception: # noqa: BLE001, S110 # metric logging must not change the mirror outcome pass diff --git a/policyengine_api/services/v2/households/services.py b/policyengine_api/services/v2/households/services.py index f4444ee3b..0ad26681b 100644 --- a/policyengine_api/services/v2/households/services.py +++ b/policyengine_api/services/v2/households/services.py @@ -60,7 +60,9 @@ def normalize_creation_input( ) -> HouseholdCreationInput: """Return the input with its household document validated and normalized.""" document = household_input.household_data - assert isinstance(document, dict) + if not isinstance(document, dict): + msg = "household_data must be an object" + raise TypeError(msg) return HouseholdCreationInput( country_id=household_input.country_id, default_year=household_input.default_year, diff --git a/policyengine_api/services/v2/households/transformations.py b/policyengine_api/services/v2/households/transformations.py index 40670c871..6edf56876 100644 --- a/policyengine_api/services/v2/households/transformations.py +++ b/policyengine_api/services/v2/households/transformations.py @@ -84,7 +84,9 @@ def canonical_household_document(household_input: HouseholdCreationInput) -> byt "people", *SUPPORTED_ENTITY_COLLECTIONS[household_input.country_id], }: - assert isinstance(records, list) + if not isinstance(records, list): + msg = f"household {collection} must be a list" + raise TypeError(msg) typed_records = cast("list[dict[str, Any]]", records) content[collection] = sorted( typed_records, @@ -198,7 +200,9 @@ def translate_legacy_household( translated exactly. """ source = snapshot.household_json - assert isinstance(source, dict) + if not isinstance(source, dict): + msg = "legacy household_json must be an object" + raise TypeError(msg) collections = SUPPORTED_ENTITY_COLLECTIONS[snapshot.country_id] allowed_source = { "people", @@ -244,7 +248,9 @@ def translate_legacy_household( msg = f"legacy {source_name} members must reference named people" raise LegacyHouseholdTranslationError(msg) for raw_person_name in value: - assert isinstance(raw_person_name, str) + if not isinstance(raw_person_name, str): + msg = f"legacy {source_name} member names must be strings" + raise TypeError(msg) person_name = raw_person_name memberships = person_memberships[person_name] previous = memberships.get(collection) diff --git a/policyengine_api/services/v2/households/validators.py b/policyengine_api/services/v2/households/validators.py index 83298c844..e46520cd6 100644 --- a/policyengine_api/services/v2/households/validators.py +++ b/policyengine_api/services/v2/households/validators.py @@ -190,7 +190,9 @@ def normalize_household_document( required_memberships = set(collections) for person in normalized["people"]: memberships = person["memberships"] - assert isinstance(memberships, dict) + if not isinstance(memberships, dict): + msg = "people record memberships must be an object" + raise TypeError(msg) if set(memberships) != required_memberships: msg = ( "people record memberships must identify every country-supported " diff --git a/policyengine_api/services/v2/policies/services.py b/policyengine_api/services/v2/policies/services.py index cb4c53a3a..5c21c124d 100644 --- a/policyengine_api/services/v2/policies/services.py +++ b/policyengine_api/services/v2/policies/services.py @@ -215,7 +215,9 @@ def mirror_legacy_policy_in_session( running_policyengine_version=running_policyengine_version, ) policy_json = snapshot.policy_json - assert isinstance(policy_json, dict) + if not isinstance(policy_json, dict): + msg = "legacy policy_json must be an object" + raise TypeError(msg) parameters = read_parameters_by_name( session, model_version_id=selected.model_version.id, diff --git a/policyengine_api/services/v2/policies/transformations.py b/policyengine_api/services/v2/policies/transformations.py index 8bb05633f..e20790ee1 100644 --- a/policyengine_api/services/v2/policies/transformations.py +++ b/policyengine_api/services/v2/policies/transformations.py @@ -270,7 +270,9 @@ def translate_legacy_policy( msg = "legacy policy api_version does not match the running country package" raise LegacyPolicyTranslationError(msg) policy_json = snapshot.policy_json - assert isinstance(policy_json, dict) + if not isinstance(policy_json, dict): + msg = "legacy policy_json must be an object" + raise TypeError(msg) parameter_names = set(policy_json) if set(parameters) != parameter_names: msg = "every legacy parameter path must exist in the running catalog" diff --git a/policyengine_api/services/v2/user_households/services.py b/policyengine_api/services/v2/user_households/services.py index 5cb2d7fd9..5077d530c 100644 --- a/policyengine_api/services/v2/user_households/services.py +++ b/policyengine_api/services/v2/user_households/services.py @@ -170,7 +170,9 @@ def patch_user_household( association_id=association_id, ) if "household_id" in association_input.model_fields_set: - assert association_input.household_id is not None + if association_input.household_id is None: + msg = "household_id must not be null" + raise TypeError(msg) replacement = read_household_for_association( session, association_input.household_id, diff --git a/policyengine_api/utils/payload_validators/validate_country.py b/policyengine_api/utils/payload_validators/validate_country.py index 33f2e7a15..d8e6a06c3 100644 --- a/policyengine_api/utils/payload_validators/validate_country.py +++ b/policyengine_api/utils/payload_validators/validate_country.py @@ -41,7 +41,6 @@ def validate_country( def validate_country_wrapper( country_id: str, *args: object, **kwargs: object ) -> _ViewResult | Response: - print("Validating country") try: ensure_supported_country(country_id) except InvalidCountryError as error: diff --git a/policyengine_api/worker_spm_release.py b/policyengine_api/worker_spm_release.py index f83c9ebf2..13cdb5d8e 100644 --- a/policyengine_api/worker_spm_release.py +++ b/policyengine_api/worker_spm_release.py @@ -118,9 +118,9 @@ def main() -> None: raise SystemExit(msg) selection = validate_installed_worker(json.load(sys.stdin), sys.argv[1]) if selection is None: - print("The installed legacy bundle and registered worker agree") + print("The installed legacy bundle and registered worker agree") # noqa: T201 # the command reports its result on stdout else: - print("The installed bundle and registered worker capability agree") + print("The installed bundle and registered worker capability agree") # noqa: T201 # the command reports its result on stdout if __name__ == "__main__": diff --git a/pyproject.toml b/pyproject.toml index 329d43a06..280a4bf5c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -95,6 +95,8 @@ select = [ "D", # Group 5: exceptions and messages. "BLE", "EM", "TRY", + # Group 6: security, logging and debugging leftovers. + "DTZ", "ERA", "EXE", "FIX", "G", "LOG", "S", "T20", "TD", ] # Mutually exclusive pydocstyle pairs: keep D211 (no blank line before a class # docstring) and D212 (summary on the first line). diff --git a/rules/registry.toml b/rules/registry.toml index d1e450584..a0643548c 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -679,7 +679,7 @@ autofix = true [rule.enforcer] kind = "ruff" mode = "lint" -codes = ["ANN", "BLE", "D", "E4", "E7", "E9", "EM", "F", "FLY", "FURB", "I", "PIE", "PYI", "RET", "RSE", "TC", "TRY", "UP", "W"] +codes = ["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"] [[rule.ci]] workflow = ".github/workflows/pr.yml" job = "lint" diff --git a/scripts/capture_migration_baseline.py b/scripts/capture_migration_baseline.py index 674871fcf..76ef4cff0 100644 --- a/scripts/capture_migration_baseline.py +++ b/scripts/capture_migration_baseline.py @@ -22,6 +22,11 @@ from collections.abc import Iterable +def _report(message: str) -> None: + """Print one line of the capture's output.""" + print(message) # noqa: T201 # reporting the baseline is this command's job + + @dataclass(frozen=True) class RouteProbe: """One API request to time, and the status codes that count as success.""" @@ -289,7 +294,7 @@ def main(argv: list[str] | None = None) -> int: results: list[ProbeResult] = [] if not args.base_url: - print("API_BASE_URL is not set; skipping API route baseline capture.") + _report("API_BASE_URL is not set; skipping API route baseline capture.") else: results.extend(run_probes(args.base_url, args.repetitions)) @@ -304,16 +309,16 @@ def main(argv: list[str] | None = None) -> int: ) ) elif args.simulation_gateway_url or args.simulation_payload_file: - print( + _report( "SIMULATION_ENTRYPOINT_URL and SIMULATION_PAYLOAD_FILE are both required; " "skipping simulation gateway baseline capture." ) if not results: - print("No baseline probes ran.") + _report("No baseline probes ran.") return 0 - print(json.dumps(summarize(results), indent=2, sort_keys=True)) + _report(json.dumps(summarize(results), indent=2, sort_keys=True)) return 0 diff --git a/scripts/compare_migration_baseline.py b/scripts/compare_migration_baseline.py index d3f320526..74728e9e3 100644 --- a/scripts/compare_migration_baseline.py +++ b/scripts/compare_migration_baseline.py @@ -173,8 +173,8 @@ def main(argv: list[str] | None = None) -> int: p95_ratio=args.p95_ratio, ) for line in lines: - print(line) - print("RESULT: PASS" if passed else "RESULT: FAIL") + print(line) # noqa: T201 # the comparison report is this command's output + print("RESULT: PASS" if passed else "RESULT: FAIL") # noqa: T201 # the comparison report is this command's output return 0 if passed else 1 diff --git a/scripts/export_migration_contracts.py b/scripts/export_migration_contracts.py index 1a716dc65..b5cacc633 100644 --- a/scripts/export_migration_contracts.py +++ b/scripts/export_migration_contracts.py @@ -158,7 +158,7 @@ def main(argv: list[str] | None = None) -> int: json_path=Path(args.json), markdown_path=Path(args.markdown), ) - print( + print( # noqa: T201 # the command reports what it exported "Exported " f"{payload['metadata']['workflow_count']} workflows and " f"{payload['metadata']['request_count']} requests to " diff --git a/scripts/guards/migration_contracts.py b/scripts/guards/migration_contracts.py index a7760ea43..d058c5169 100644 --- a/scripts/guards/migration_contracts.py +++ b/scripts/guards/migration_contracts.py @@ -134,12 +134,12 @@ def main() -> int: """Print the guard's violations and return 1 if there are any.""" violations = check() if not violations: - print("migration-contracts guard passed") + print("migration-contracts guard passed") # noqa: T201 # the guard reports its result return 0 - print("migration-contracts guard failed:") + print("migration-contracts guard failed:") # noqa: T201 # the guard reports its result for violation in violations: - print(f" - {violation}") + print(f" - {violation}") # noqa: T201 # the guard reports its result return 1 diff --git a/scripts/measure_cloud_run_runtime.py b/scripts/measure_cloud_run_runtime.py index 9ca1ddd99..581f5fd98 100644 --- a/scripts/measure_cloud_run_runtime.py +++ b/scripts/measure_cloud_run_runtime.py @@ -91,7 +91,7 @@ async def _client_loop( deadline: float, records: list[dict], ) -> None: - rng = random.Random(args.seed * 1000 + client_index) + rng = random.Random(args.seed * 1000 + client_index) # noqa: S311 # a seeded generator makes the request mix reproducible; nothing secret depends on it seq = 0 while time.monotonic() < deadline: seq += 1 @@ -171,7 +171,7 @@ async def _run(args: argparse.Namespace) -> dict: CALCULATE_PATH, json=throwaway, timeout=args.timeout_calculate ) except httpx.HTTPError as error: - print( + print( # noqa: T201 # the command warns on stderr and keeps measuring f"warning: warm-up calculate failed ({type(error).__name__}); " "continuing", file=sys.stderr, diff --git a/scripts/process_v1_household_mirror_event.py b/scripts/process_v1_household_mirror_event.py old mode 100644 new mode 100755 index 5be2f988e..c5dad701c --- a/scripts/process_v1_household_mirror_event.py +++ b/scripts/process_v1_household_mirror_event.py @@ -222,7 +222,7 @@ def main( outcome="error", error_type=type(error).__name__, ) - except Exception: # noqa: BLE001 # outcome logging must not change the command result + except Exception: # noqa: BLE001, S110 # outcome logging must not change the command result pass safe_types = (HouseholdEventCommandConfigurationError,) message = ( @@ -230,7 +230,7 @@ def main( if isinstance(error, safe_types) else "household create-event processing failed" ) - print( + print( # noqa: T201 # the command's JSON outcome is its output json.dumps( { "outcome": "error", @@ -254,9 +254,9 @@ def main( outcome="ok", destination_household_id=result.household_id, ) - except Exception: # noqa: BLE001 # outcome logging must not change the command result + except Exception: # noqa: BLE001, S110 # outcome logging must not change the command result pass - print( + print( # noqa: T201 # the command's JSON outcome is its output json.dumps( { "outcome": "ok", diff --git a/scripts/publish_v2_metadata_catalog.py b/scripts/publish_v2_metadata_catalog.py old mode 100644 new mode 100755 diff --git a/scripts/qualify_v2_household_migration.py b/scripts/qualify_v2_household_migration.py old mode 100644 new mode 100755 diff --git a/scripts/qualify_v2_policy_migration.py b/scripts/qualify_v2_policy_migration.py old mode 100644 new mode 100755 diff --git a/scripts/rules/baseline.py b/scripts/rules/baseline.py index 5bc3feb90..4ebe33fac 100644 --- a/scripts/rules/baseline.py +++ b/scripts/rules/baseline.py @@ -28,16 +28,16 @@ def load_base_registry(root: Path, ref: str) -> Registry | None: Raise BaseUnavailable when ``ref`` does not name a commit. """ - verify = subprocess.run( - ["git", "rev-parse", "--verify", "--quiet", f"{ref}^{{commit}}"], + verify = subprocess.run( # noqa: S603 # runs git with a fixed command and no shell + ["git", "rev-parse", "--verify", "--quiet", f"{ref}^{{commit}}"], # noqa: S607 # git comes from PATH, as in any checkout the tool runs in cwd=root, capture_output=True, text=True, ) if verify.returncode != 0: raise BaseUnavailable(ref) - shown = subprocess.run( - ["git", "show", f"{ref}:{REGISTRY_PATH.as_posix()}"], + shown = subprocess.run( # noqa: S603 # runs git with a fixed command and no shell + ["git", "show", f"{ref}:{REGISTRY_PATH.as_posix()}"], # noqa: S607 # git comes from PATH, as in any checkout the tool runs in cwd=root, capture_output=True, text=True, diff --git a/scripts/rules/ci_scripts.py b/scripts/rules/ci_scripts.py index 0a0edecd2..e252cea64 100644 --- a/scripts/rules/ci_scripts.py +++ b/scripts/rules/ci_scripts.py @@ -96,7 +96,7 @@ def shellcheck_failures(config: CiScripts, root: Path) -> set[str] | None: ) if not scripts: return set() - result = subprocess.run( + result = subprocess.run( # noqa: S603 # runs the resolved shellcheck on this repository's scripts [shellcheck, "--format=gcc", *scripts], cwd=root, capture_output=True, @@ -149,7 +149,9 @@ def compare(registry: Registry, findings: Findings, root: Path) -> list[Violatio A check that could not run is skipped rather than reported. """ config = registry.ci_scripts - assert config is not None + if config is None: + msg = "the registry has no [ci_scripts] table" + raise TypeError(msg) checks = ( ( "multiline", @@ -197,7 +199,9 @@ def compare(registry: Registry, findings: Findings, root: Path) -> list[Violatio def write_baselines(registry: Registry, findings: Findings, root: Path) -> None: """Record today's findings as the baselines (used once, when adopting the rule).""" config = registry.ci_scripts - assert config is not None + if config is None: + msg = "the registry has no [ci_scripts] table" + raise TypeError(msg) for found, path in ( (findings.multiline, config.multiline_baseline), (findings.untested, config.untested_baseline), @@ -243,5 +247,5 @@ def check() -> list[str]: """Adapter for scripts/run_quality_guards.py.""" violations, notes = run_check(REPO_ROOT) for note in notes: - print(f"note: {note}") + print(f"note: {note}") # noqa: T201 # the guard adapter prints its notes return [str(violation) for violation in violations] diff --git a/scripts/rules/cli.py b/scripts/rules/cli.py index a4134cacb..9df65da56 100644 --- a/scripts/rules/cli.py +++ b/scripts/rules/cli.py @@ -14,12 +14,17 @@ from scripts.rules.validate import validate +def _report(message: str) -> None: + """Print one line of the command's output.""" + print(message) # noqa: T201 # reporting the result is this command's job + + def _print(violations: list, notes: list[str] | None = None) -> int: for note in notes or []: - print(f"note: {note}") + _report(f"note: {note}") for violation in violations: - print(f" - {violation}") - print("rules: failed" if violations else "rules: passed") + _report(f" - {violation}") + _report("rules: failed" if violations else "rules: passed") return 1 if violations else 0 @@ -55,7 +60,7 @@ def main(argv: list[str] | None = None) -> int: ci_scripts.write_baselines( registry, ci_scripts.find(registry.ci_scripts, root), root ) - print("wrote ci_scripts baselines; update the rules' baseline counts") + _report("wrote ci_scripts baselines; update the rules' baseline counts") return 0 violations, notes = ci_scripts.run_check(root) return _print(violations, notes) @@ -68,12 +73,12 @@ def main(argv: list[str] | None = None) -> int: registry, violations = load_registry(root) if registry is None: return _print(violations) - violations += validate(registry, root, date.today()) + violations += validate(registry, root, date.today()) # noqa: DTZ011 # review and expiry dates are calendar days, checked against the local date if args.command == "render": if violations: - print("render refused: fix the registry first") + _report("render refused: fix the registry first") return _print(violations) for path in write_outputs(registry, root): - print(f"wrote {path.relative_to(root)}") + _report(f"wrote {path.relative_to(root)}") return 0 return _print(violations) diff --git a/scripts/rules/guard.py b/scripts/rules/guard.py index 3efa3d4f5..092425003 100644 --- a/scripts/rules/guard.py +++ b/scripts/rules/guard.py @@ -89,7 +89,7 @@ def run_guard( if registry is None: return result - result.violations += validate(registry, root, today or date.today()) + result.violations += validate(registry, root, today or date.today()) # noqa: DTZ011 # review and expiry dates are calendar days, checked against the local date for rule in registry.rules: result.violations += resolve_enforcer(rule, root) result.violations += resolve_ci(rule, root) diff --git a/scripts/rules/tests/test_guard.py b/scripts/rules/tests/test_guard.py index 400a68b7e..731f65d49 100644 --- a/scripts/rules/tests/test_guard.py +++ b/scripts/rules/tests/test_guard.py @@ -589,9 +589,9 @@ def test_compares_against_git_base_ref(self) -> None: repo.write("rules/baselines/ratcheted.txt", "pkg/routes.py\n") repo.render() git = ["git", "-c", "user.email=t@example.com", "-c", "user.name=t"] - subprocess.run(["git", "init", "-q"], cwd=repo.root, check=True) - subprocess.run([*git, "add", "-A"], cwd=repo.root, check=True) - subprocess.run([*git, "commit", "-qm", "base"], cwd=repo.root, check=True) + subprocess.run(["git", "init", "-q"], cwd=repo.root, check=True) # noqa: S607 # git comes from PATH in the temporary test repository + subprocess.run([*git, "add", "-A"], cwd=repo.root, check=True) # noqa: S603 # a fixed git command in the temporary test repository + subprocess.run([*git, "commit", "-qm", "base"], cwd=repo.root, check=True) # noqa: S603 # a fixed git command in the temporary test repository repo.edit("rules/registry.toml", "count = 1", "count = 2") repo.write("rules/baselines/ratcheted.txt", "pkg/routes.py\npkg/other.py\n") repo.render() diff --git a/scripts/run_quality_guards.py b/scripts/run_quality_guards.py index a89b7bd3b..bf01f94e6 100644 --- a/scripts/run_quality_guards.py +++ b/scripts/run_quality_guards.py @@ -29,13 +29,13 @@ def main() -> int: for name, check in GUARDS: violations = check() if not violations: - print(f"{name}: passed") + print(f"{name}: passed") # noqa: T201 # the guard runner reports each result continue failed = True - print(f"{name}: failed") + print(f"{name}: failed") # noqa: T201 # the guard runner reports each result for violation in violations: - print(f" - {violation}") + print(f" - {violation}") # noqa: T201 # the guard runner reports each result return 1 if failed else 0 diff --git a/scripts/smoke_test_local_boot.py b/scripts/smoke_test_local_boot.py old mode 100644 new mode 100755 index eca2fd272..4af9b4f89 --- a/scripts/smoke_test_local_boot.py +++ b/scripts/smoke_test_local_boot.py @@ -25,6 +25,11 @@ HTTP_TIMEOUT_SECONDS = 2 +def _report(message: str) -> None: + """Print one line of the smoke test's output.""" + print(message) # noqa: T201 # reporting progress and results is this command's job + + def find_free_port() -> int: """Return a localhost TCP port that was free when checked (nothing reserves it).""" with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: @@ -41,16 +46,16 @@ def read_text(path: Path) -> str: def fetch(url: str) -> tuple[int, str]: """Return the status and body of a GET; error statuses raise urllib's HTTPError.""" - request = urllib.request.Request(url, method="GET") - with urllib.request.urlopen(request, timeout=HTTP_TIMEOUT_SECONDS) as response: + request = urllib.request.Request(url, method="GET") # noqa: S310 # callers pass only the local app's http://127.0.0.1 URLs + with urllib.request.urlopen(request, timeout=HTTP_TIMEOUT_SECONDS) as response: # noqa: S310 # callers pass only the local app's http://127.0.0.1 URLs body = response.read().decode("utf-8", errors="replace") return response.getcode(), body def print_section(title: str, body: str) -> None: """Print a titled block of captured output, or "(empty)" when there is none.""" - print(f"\n=== {title} ===") - print(body.rstrip() if body else "(empty)") + _report(f"\n=== {title} ===") + _report(body.rstrip() if body else "(empty)") def main() -> int: @@ -87,18 +92,18 @@ def main() -> int: except importlib.metadata.PackageNotFoundError: policyengine_version = "not installed" - print(f"repo: {REPO_ROOT}") - print(f"python: {sys.executable}") - print(f"policyengine: {policyengine_version}") - print(f"port: {port}") - print(f"stdout log: {stdout_path}") - print(f"stderr log: {stderr_path}") + _report(f"repo: {REPO_ROOT}") + _report(f"python: {sys.executable}") + _report(f"policyengine: {policyengine_version}") + _report(f"port: {port}") + _report(f"stdout log: {stdout_path}") + _report(f"stderr log: {stderr_path}") with ( stdout_path.open("w", encoding="utf-8") as stdout_handle, stderr_path.open("w", encoding="utf-8") as stderr_handle, ): - process = subprocess.Popen( + process = subprocess.Popen( # noqa: S603 # starts this repository's app with the current interpreter [ sys.executable, "-m", @@ -142,22 +147,22 @@ def main() -> int: ) if process.poll() is not None else process.terminate() if process.poll() is None: process.wait(timeout=5) - print("\nBoot smoke test failed: app did not become ready.") + _report("\nBoot smoke test failed: app did not become ready.") print_section("stdout", read_text(stdout_path)) print_section("stderr", read_text(stderr_path)) return 1 - print("\nBoot smoke test reached a live app. Probing health endpoints:") + _report("\nBoot smoke test reached a live app. Probing health endpoints:") for path in HEALTH_PATHS: status, body = fetch(f"http://127.0.0.1:{port}{path}") - print(f"{path}: {status} {body.strip()}") + _report(f"{path}: {status} {body.strip()}") if status != 200 or body.strip() != "OK": - print("\nHealth endpoint probe failed.") + _report("\nHealth endpoint probe failed.") print_section("stdout", read_text(stdout_path)) print_section("stderr", read_text(stderr_path)) return 1 - print("\nBoot smoke test passed.") + _report("\nBoot smoke test passed.") return 0 finally: if "process" in locals() and process.poll() is None: diff --git a/scripts/v1_database_migration.py b/scripts/v1_database_migration.py index 679631d8f..e51da59a2 100644 --- a/scripts/v1_database_migration.py +++ b/scripts/v1_database_migration.py @@ -249,7 +249,7 @@ def main(argv: list[str] | None = None) -> int: ) with connection_context as connection: if args.mode == "state": - print(database_state(connection).value) + print(database_state(connection).value) # noqa: T201 # the command prints the database state for its caller elif args.mode == "verify-head": verify_head_schema(connection) else: diff --git a/tests/env_variables/test_environment_variables.py b/tests/env_variables/test_environment_variables.py index de4175d7b..191c7eb37 100644 --- a/tests/env_variables/test_environment_variables.py +++ b/tests/env_variables/test_environment_variables.py @@ -10,7 +10,7 @@ HUGGING_FACE_UK_POPULATION_DATASET_URL = ( "https://huggingface.co/api/datasets/policyengine/populace-uk-private" ) -EXPECTED_HUGGING_FACE_TOKEN_NAME = "pe-uk-private-hf-read-token" +EXPECTED_HUGGING_FACE_TOKEN_NAME = "pe-uk-private-hf-read-token" # noqa: S105 # the Secret Manager secret's name, not its value GITHUB_API_URL = "https://api.github.com/user" diff --git a/tests/fixtures/services/economy_service.py b/tests/fixtures/services/economy_service.py index 5330cc1ca..a2c1e2299 100644 --- a/tests/fixtures/services/economy_service.py +++ b/tests/fixtures/services/economy_service.py @@ -198,7 +198,7 @@ def mock_logger(): @pytest.fixture def mock_datetime(): """Mock datetime.datetime.now().""" - mock_now = datetime.datetime(2025, 6, 26, 12, 0, 0) + mock_now = datetime.datetime(2025, 6, 26, 12, 0, 0) # noqa: DTZ001 # stands in for the naive local time that economy_service stores with patch("policyengine_api.services.economy_service.datetime.datetime") as mock: mock.now.return_value = mock_now yield mock @@ -251,8 +251,8 @@ def create_mock_reform_impact( reform_impact_json=reform_impact_json or default_reform_impact_json, execution_id=execution_id, message=message, - start_time=start_time or datetime.datetime(2025, 6, 26, 12, 0, 0), - end_time=(datetime.datetime(2025, 6, 26, 12, 5, 0) if status == "ok" else None), + start_time=start_time or datetime.datetime(2025, 6, 26, 12, 0, 0), # noqa: DTZ001 # reform_impact start_time is a naive column + end_time=(datetime.datetime(2025, 6, 26, 12, 5, 0) if status == "ok" else None), # noqa: DTZ001 # reform_impact end_time is a naive column ) impact.observability_id = MOCK_OBSERVABILITY_ID return impact diff --git a/tests/integration/test_live_economy.py b/tests/integration/test_live_economy.py index 5e9d062c4..6e68d8421 100644 --- a/tests/integration/test_live_economy.py +++ b/tests/integration/test_live_economy.py @@ -3,7 +3,7 @@ from datetime import date from pathlib import Path -CURRENT_YEAR = date.today().year +CURRENT_YEAR = date.today().year # noqa: DTZ011 # matches the API's default year, which follows the local calendar def _load_reform_payload(filename: str) -> dict: diff --git a/tests/integration/test_runtime_cache_redis.py b/tests/integration/test_runtime_cache_redis.py index 941b4953d..8435b8e85 100644 --- a/tests/integration/test_runtime_cache_redis.py +++ b/tests/integration/test_runtime_cache_redis.py @@ -23,6 +23,9 @@ ) RUNTIME_CACHE_TEST_URL = "RUNTIME_CACHE_TEST_URL" +# Claim tokens are opaque ids that tell competing workers apart. +WRITER_CLAIM_TOKEN = "writer" # noqa: S105 # an opaque claim id, not a secret +CONTENDER_CLAIM_TOKEN = "contender" # noqa: S105 # an opaque claim id, not a secret @pytest.fixture @@ -184,30 +187,30 @@ def test_reform_submission_claim_is_shared_across_connections(redis_pair) -> Non assert writer.claim_start( **arguments, - claim_token="writer", + claim_token=WRITER_CLAIM_TOKEN, observability_id=writer_observability_id, ) assert not contender.claim_start( **arguments, - claim_token="contender", + claim_token=CONTENDER_CLAIM_TOKEN, observability_id=contender_observability_id, ) winning_claim = contender.get_start_claim(**arguments) - assert winning_claim.submission_claim_id == "writer" + assert winning_claim.submission_claim_id == WRITER_CLAIM_TOKEN assert winning_claim.observability_id == writer_observability_id assert not contender.release_start( **arguments, - claim_token="contender", + claim_token=CONTENDER_CLAIM_TOKEN, observability_id=contender_observability_id, ) assert writer.release_start( **arguments, - claim_token="writer", + claim_token=WRITER_CLAIM_TOKEN, observability_id=writer_observability_id, ) assert contender.claim_start( **arguments, - claim_token="contender", + claim_token=CONTENDER_CLAIM_TOKEN, observability_id=contender_observability_id, ) @@ -240,7 +243,7 @@ def test_real_reform_indexes_are_cross_connection_bounded_and_expiring( reform_impact_json={}, status="computing", message=None, - start_time=datetime(2026, 1, day), + start_time=datetime(2026, 1, day), # noqa: DTZ001 # reform_impact start_time is a naive column end_time=None, execution_id=execution_id, ) diff --git a/tests/integration/test_v2_catalog_publication.py b/tests/integration/test_v2_catalog_publication.py index 06e48f64c..233b7b19e 100644 --- a/tests/integration/test_v2_catalog_publication.py +++ b/tests/integration/test_v2_catalog_publication.py @@ -69,7 +69,7 @@ def _counts(engine: Engine) -> dict[str, int]: with engine.connect() as connection: return { table_name: connection.execute( - text(f"SELECT count(*) FROM {table_name}") + text(f"SELECT count(*) FROM {table_name}") # noqa: S608 # table names come from the fixed tuple below, never from input ).scalar_one() for table_name in ( "tax_benefit_models", @@ -94,7 +94,7 @@ def _identifiers(engine: Engine) -> dict[str, tuple[str, ...]]: table_name: tuple( str(value) for value in connection.execute( - text(f"SELECT id FROM {table_name} ORDER BY id") + text(f"SELECT id FROM {table_name} ORDER BY id") # noqa: S608 # table names come from the fixed tuple below, never from input ).scalars() ) for table_name in ( diff --git a/tests/integration/test_v2_catalog_publication_qualification.py b/tests/integration/test_v2_catalog_publication_qualification.py index 8400c4ac9..98af69ad4 100644 --- a/tests/integration/test_v2_catalog_publication_qualification.py +++ b/tests/integration/test_v2_catalog_publication_qualification.py @@ -105,7 +105,7 @@ def test_complete_installed_catalog_bulk_publication() -> None: evidence.entity_counts["regions"], ) assert all(representative) - print( + print( # noqa: T201 # the qualification evidence is this test's report json.dumps( { **evidence.as_dict(), diff --git a/tests/to_refactor/python/test_policy.py b/tests/to_refactor/python/test_policy.py index b6adf8397..1857bee9a 100644 --- a/tests/to_refactor/python/test_policy.py +++ b/tests/to_refactor/python/test_policy.py @@ -57,61 +57,3 @@ def test_create_nonunique_policy(self, api_client): def test_create_policy_invalid_country(self, api_client): res = api_client.post("/au/policy", json=self.test_policy) assert res.status_code == 400 - - -# The below test is very prone to race conditions due to emission against live db; -# should be refactored to use mock/test db and reinstated - -# class TestPolicySearch: -# country_id = "us" -# policy_json = {"sample_input": {"2023-01-01.2024-12-31": True}} -# label = "maxwell" -# policy_hash = hash_object(policy_json) -# api_version = COUNTRY_PACKAGE_VERSIONS.get(country_id) -# -# # Pre-seed database with duplicate policies -# for i in range(2): -# database.query( -# f"INSERT INTO policy (country_id, label, policy_json, policy_hash, api_version) VALUES (?, ?, ?, ?, ?)", -# ( -# country_id, -# label, -# json.dumps(policy_json), -# policy_hash, -# api_version, -# ), -# ) -# -# db_output = database.query( -# f"SELECT * FROM policy WHERE label = ?", -# (label,), -# ).fetchall() -# -# def test_search_all_policies(self, api_client): -# res = api_client.get("/us/policies") -# return_object = json.loads(res.text) -# -# filtered_return = list( -# filter(lambda x: x["label"] == self.label, return_object["result"]) -# ) -# -# assert return_object["status"] == "ok" -# assert len(filtered_return) == len(self.db_output) -# -# def test_search_unique_policies(self, api_client): -# res = api_client.get("/us/policies?unique_only=true") -# return_object = json.loads(res.text) -# -# filtered_return = list( -# filter(lambda x: x["label"] == self.label, return_object["result"]) -# ) -# -# assert return_object["status"] == "ok" -# assert len(filtered_return) == 1 -# -# # Clean up duplicate policies created -# database.query( -# f"DELETE FROM policy WHERE policy_hash = ? AND label = ? AND country_id = ?", -# (self.policy_hash, self.label, self.country_id), -# ) -# diff --git a/tests/unit/data/test_v1_database_migration.py b/tests/unit/data/test_v1_database_migration.py index 29d172aa7..8447a3087 100644 --- a/tests/unit/data/test_v1_database_migration.py +++ b/tests/unit/data/test_v1_database_migration.py @@ -15,11 +15,15 @@ upgrade_database, ) +# Fake credentials; the first holds characters that URLs must escape. +FAKE_PASSWORD_WITH_URL_CHARACTERS = "p@ss:/word" # noqa: S105 # a fake test password +FAKE_PASSWORD = "p@ssword" # noqa: S105 # a fake test password + def test_database_url_uses_sqlalchemy_url_without_stringifying_credentials(): url = build_database_url( username="schema reader", - password="p@ss:/word", + password=FAKE_PASSWORD_WITH_URL_CHARACTERS, host="127.0.0.1", port=3307, database="policyengine", @@ -27,7 +31,7 @@ def test_database_url_uses_sqlalchemy_url_without_stringifying_credentials(): assert isinstance(url, URL) assert url.username == "schema reader" - assert url.password == "p@ss:/word" + assert url.password == FAKE_PASSWORD_WITH_URL_CHARACTERS assert url.host == "127.0.0.1" assert url.port == 3307 assert url.database == "policyengine" @@ -57,13 +61,13 @@ def test_database_target_builds_in_memory_url_for_each_database_role( ): monkeypatch.delenv("STAGE7_EXISTING_DATABASE_URL", raising=False) monkeypatch.delenv("ALEMBIC_DATABASE_URL", raising=False) - monkeypatch.setenv(password_name, "p@ssword") + monkeypatch.setenv(password_name, FAKE_PASSWORD) target = migration._database_target(mode) assert isinstance(target, URL) assert target.username == expected_user - assert target.password == "p@ssword" + assert target.password == FAKE_PASSWORD assert target.host == "127.0.0.1" assert target.port == 3307 assert target.database == "policyengine" @@ -157,7 +161,7 @@ def test_metadata_difference_descriptions_are_stable_and_do_not_include_data(): def test_metadata_difference_rejects_unknown_shapes_without_repr_leakage(): - secret = "do-not-print-this-value" + secret = "do-not-print-this-value" # noqa: S105 # a sentinel that must not leak into the error difference = ("unknown", secret) with pytest.raises(ValueError, match="Unsupported metadata difference") as error: diff --git a/tests/unit/libs/test_gateway_auth.py b/tests/unit/libs/test_gateway_auth.py index 8103c9a69..1a704e169 100644 --- a/tests/unit/libs/test_gateway_auth.py +++ b/tests/unit/libs/test_gateway_auth.py @@ -25,9 +25,9 @@ ISSUER = "https://policyengine.uk.auth0.com" AUDIENCE = "https://sim-gateway.policyengine.org" CLIENT_ID = "test-client-id" -CLIENT_SECRET = "test-client-secret" +CLIENT_SECRET = "test-client-secret" # noqa: S105 # a fake test secret CLIENT_SECRET_RESOURCE = ( - "projects/policyengine-api/secrets/gateway-auth-client-secret/versions/1" + "projects/policyengine-api/secrets/gateway-auth-client-secret/versions/1" # noqa: S105 # a Secret Manager resource path, not a secret ) @@ -105,7 +105,7 @@ def test__given_first_call__then_fetches_and_returns_token(self): ) as mock_post: token = provider.get_token() - assert token == "tok-1" + assert token == "tok-1" # noqa: S105 # the fake token from the mocked response mock_post.assert_called_once() call_kwargs = mock_post.call_args.kwargs assert call_kwargs["json"]["grant_type"] == "client_credentials" diff --git a/tests/unit/routes/test_reform_impact_routes.py b/tests/unit/routes/test_reform_impact_routes.py index 29ffbf65e..4274317a7 100644 --- a/tests/unit/routes/test_reform_impact_routes.py +++ b/tests/unit/routes/test_reform_impact_routes.py @@ -23,7 +23,7 @@ ) from policyengine_api.services.reform_impacts_service import ReformImpactsService -COMPLETED_AT = datetime(2026, 1, 1, 1) +COMPLETED_AT = datetime(2026, 1, 1, 1) # noqa: DTZ001 # stands in for the service's naive UTC _now() @pytest.fixture @@ -67,7 +67,7 @@ def _seed_reform_impacts(service: ReformImpactsService, n: int) -> None: api_version="1.0.0", reform_impact_json={}, status="computing", - start_time=datetime(2026, 1, 1, 0, i // 60, i % 60), + start_time=datetime(2026, 1, 1, 0, i // 60, i % 60), # noqa: DTZ001 # reform_impact start_time is a naive column execution_id=execution_id, ) service.set_complete_reform_impact( diff --git a/tests/unit/runtime_cache/test_reform_impacts.py b/tests/unit/runtime_cache/test_reform_impacts.py index 26a02cca7..1c48e0ab3 100644 --- a/tests/unit/runtime_cache/test_reform_impacts.py +++ b/tests/unit/runtime_cache/test_reform_impacts.py @@ -18,6 +18,12 @@ reform_impact_id, ) +# Claim tokens are opaque ids that tell competing workers apart. +OWNER_CLAIM_TOKEN = "owner" # noqa: S105 # an opaque claim id, not a secret +CONTENDER_CLAIM_TOKEN = "contender" # noqa: S105 # an opaque claim id, not a secret +CLIFF_OWNER_CLAIM_TOKEN = "cliff-owner" # noqa: S105 # an opaque claim id, not a secret +NEXT_OWNER_CLAIM_TOKEN = "next-owner" # noqa: S105 # an opaque claim id, not a secret + def _namespace() -> CacheNamespace: return CacheNamespace("test", "api") @@ -38,7 +44,7 @@ def _impact(execution_id: str, options_hash: str, day: int) -> CachedReformImpac reform_impact_json={}, status="computing", message=None, - start_time=datetime(2026, 1, day), + start_time=datetime(2026, 1, day), # noqa: DTZ001 # reform_impact start_time is a naive column end_time=None, execution_id=execution_id, ) @@ -70,7 +76,7 @@ def test_reform_impact_start_claim_is_atomic_exact_ttl_and_token_safe() -> None: assert ( cache.claim_start( **arguments, - claim_token="owner", + claim_token=OWNER_CLAIM_TOKEN, observability_id=owner_observability_id, ) is True @@ -78,7 +84,7 @@ def test_reform_impact_start_claim_is_atomic_exact_ttl_and_token_safe() -> None: assert ( cache.claim_start( **arguments, - claim_token="contender", + claim_token=CONTENDER_CLAIM_TOKEN, observability_id=contender_observability_id, ) is False @@ -86,7 +92,7 @@ def test_reform_impact_start_claim_is_atomic_exact_ttl_and_token_safe() -> None: assert ( cache.claim_start( **_claim_arguments(target="cliff"), - claim_token="cliff-owner", + claim_token=CLIFF_OWNER_CLAIM_TOKEN, observability_id=owner_observability_id, ) is True @@ -95,12 +101,12 @@ def test_reform_impact_start_claim_is_atomic_exact_ttl_and_token_safe() -> None: claim = cache.get_start_claim(**arguments) assert claim is not None - assert claim.submission_claim_id == "owner" + assert claim.submission_claim_id == OWNER_CLAIM_TOKEN assert claim.observability_id == owner_observability_id assert ( cache.release_start( **arguments, - claim_token="contender", + claim_token=CONTENDER_CLAIM_TOKEN, observability_id=contender_observability_id, ) is False @@ -108,7 +114,7 @@ def test_reform_impact_start_claim_is_atomic_exact_ttl_and_token_safe() -> None: assert ( cache.release_start( **arguments, - claim_token="owner", + claim_token=OWNER_CLAIM_TOKEN, observability_id=contender_observability_id, ) is False @@ -116,7 +122,7 @@ def test_reform_impact_start_claim_is_atomic_exact_ttl_and_token_safe() -> None: assert ( cache.release_start( **arguments, - claim_token="owner", + claim_token=OWNER_CLAIM_TOKEN, observability_id=owner_observability_id, ) is True @@ -124,7 +130,7 @@ def test_reform_impact_start_claim_is_atomic_exact_ttl_and_token_safe() -> None: assert ( cache.claim_start( **arguments, - claim_token="next-owner", + claim_token=NEXT_OWNER_CLAIM_TOKEN, observability_id=contender_observability_id, ) is True diff --git a/tests/unit/runtime_cache/test_settings.py b/tests/unit/runtime_cache/test_settings.py index 5f7c9d699..6a8aa61e7 100644 --- a/tests/unit/runtime_cache/test_settings.py +++ b/tests/unit/runtime_cache/test_settings.py @@ -73,8 +73,8 @@ def test_deployed_mode_requires_tls_auth_nonlocal_host_port_and_database_zero( def test_valid_deployed_configuration_is_secret_safe() -> None: - secret = "do-not-print-cache-password" - ca_secret = "do-not-print-cache-ca" + secret = "do-not-print-cache-password" # noqa: S105 # a sentinel that must not leak into the settings repr + ca_secret = "do-not-print-cache-ca" # noqa: S105 # a sentinel that must not leak into the settings repr settings = load_runtime_cache_settings( { RUNTIME_CACHE_MODE: "deployed", diff --git a/tests/unit/services/test_direct_orm_local_analysis.py b/tests/unit/services/test_direct_orm_local_analysis.py index a0ef9d91f..cc5383bf7 100644 --- a/tests/unit/services/test_direct_orm_local_analysis.py +++ b/tests/unit/services/test_direct_orm_local_analysis.py @@ -31,7 +31,7 @@ def test_reform_impact_service_writes_typed_expiring_cache_entity() -> None: status="computing", api_version="1", reform_impact_json={}, - start_time=datetime(2026, 1, 1), + start_time=datetime(2026, 1, 1), # noqa: DTZ001 # reform_impact start_time is a naive column execution_id="job", ) diff --git a/tests/unit/services/test_reform_impacts_service.py b/tests/unit/services/test_reform_impacts_service.py index c4371bffa..49e55ecda 100644 --- a/tests/unit/services/test_reform_impacts_service.py +++ b/tests/unit/services/test_reform_impacts_service.py @@ -11,6 +11,10 @@ ReformImpactsService, ) +# Claim tokens are opaque ids that tell competing workers apart. +OWNER_CLAIM_TOKEN = "owner" # noqa: S105 # an opaque claim id, not a secret +CONTENDER_CLAIM_TOKEN = "contender" # noqa: S105 # an opaque claim id, not a secret + @pytest.fixture def service(): @@ -41,7 +45,7 @@ def _create_impact( status="computing", api_version="1", reform_impact_json={}, - start_time=datetime(2026, 1, day), + start_time=datetime(2026, 1, day), # noqa: DTZ001 # reform_impact start_time is a naive column execution_id=execution_id, ) @@ -100,25 +104,25 @@ def test_reform_impact_start_claim_is_exclusive_and_releasable(service): assert service.claim_reform_impact_start( **arguments, - claim_token="owner", + claim_token=OWNER_CLAIM_TOKEN, observability_id=owner_observability_id, ) assert not service.claim_reform_impact_start( **arguments, - claim_token="contender", + claim_token=CONTENDER_CLAIM_TOKEN, observability_id=contender_observability_id, ) claim = service.get_reform_impact_start_claim(**arguments) - assert claim.submission_claim_id == "owner" + assert claim.submission_claim_id == OWNER_CLAIM_TOKEN assert claim.observability_id == owner_observability_id service.release_reform_impact_start( **arguments, - claim_token="owner", + claim_token=OWNER_CLAIM_TOKEN, observability_id=owner_observability_id, ) assert service.claim_reform_impact_start( **arguments, - claim_token="contender", + claim_token=CONTENDER_CLAIM_TOKEN, observability_id=contender_observability_id, ) diff --git a/tests/unit/test_alembic_workflows.py b/tests/unit/test_alembic_workflows.py index f08c309f4..e52fa7e9b 100644 --- a/tests/unit/test_alembic_workflows.py +++ b/tests/unit/test_alembic_workflows.py @@ -287,7 +287,7 @@ def test_cloud_sql_workflow_uses_oidc_and_separate_database_credentials(): def test_database_environment_validation_rejects_production_migration_credentials_in_staging(): result = subprocess.run( - ["bash", ".github/scripts/validate_database_environment.sh", "cloud-sql"], + ["bash", ".github/scripts/validate_database_environment.sh", "cloud-sql"], # noqa: S607 # bash comes from the test environment's PATH cwd=REPO, env={ **os.environ, @@ -354,7 +354,7 @@ def _run_migration_orchestrator(tmp_path: Path, database_state: str): python_calls = tmp_path / "python-calls" gcloud_calls = tmp_path / "gcloud-calls" result = subprocess.run( - ["bash", ".github/scripts/migrate_v1_cloud_sql.sh"], + ["bash", ".github/scripts/migrate_v1_cloud_sql.sh"], # noqa: S607 # bash comes from the test environment's PATH cwd=REPO, env={ **os.environ, diff --git a/tests/unit/test_bundle_update_pins.py b/tests/unit/test_bundle_update_pins.py index 2ccfb19e6..f4aff5516 100644 --- a/tests/unit/test_bundle_update_pins.py +++ b/tests/unit/test_bundle_update_pins.py @@ -66,8 +66,8 @@ def update_checkout(tmp_path): def run_update(root, *arguments, **environment): log = root / "calls.jsonl" - result = subprocess.run( - ["bash", str(SCRIPT), *arguments], + result = subprocess.run( # noqa: S603 # runs this repository's script with test-chosen arguments + ["bash", str(SCRIPT), *arguments], # noqa: S607 # bash comes from the test environment's PATH cwd=root, env={ **os.environ, diff --git a/tests/unit/test_ci_complete_gate.py b/tests/unit/test_ci_complete_gate.py index 618d05720..07aefdb86 100644 --- a/tests/unit/test_ci_complete_gate.py +++ b/tests/unit/test_ci_complete_gate.py @@ -96,8 +96,8 @@ def _run_gate( pull_request_base_ref: str = "master", stack_base_ref: str = "", ) -> subprocess.CompletedProcess[str]: - return subprocess.run( - ["bash", GATE_SCRIPT], + return subprocess.run( # noqa: S603 # runs this repository's script with test-chosen arguments + ["bash", GATE_SCRIPT], # noqa: S607 # bash comes from the test environment's PATH cwd=REPO, env={ "HOME": os.environ.get("HOME", ""), @@ -166,8 +166,8 @@ def test_gate_fails_when_a_job_was_skipped_on_full_ci() -> None: def test_gate_requires_the_job_results() -> None: - result = subprocess.run( - ["bash", GATE_SCRIPT], + result = subprocess.run( # noqa: S603 # runs this repository's script with test-chosen arguments + ["bash", GATE_SCRIPT], # noqa: S607 # bash comes from the test environment's PATH cwd=REPO, env={"PATH": os.environ["PATH"], "PULL_REQUEST_BASE_REF": "master"}, text=True, diff --git a/tests/unit/test_cloud_run_deploy_scripts.py b/tests/unit/test_cloud_run_deploy_scripts.py index 6f81fffa7..c84dffa75 100644 --- a/tests/unit/test_cloud_run_deploy_scripts.py +++ b/tests/unit/test_cloud_run_deploy_scripts.py @@ -17,7 +17,7 @@ TEST_V2_PROJECT_REF = "abcdefghijklmnopqrst" TEST_V2_ENVIRONMENT = "test-foundation" TEST_V2_RUNTIME_SECRET_RESOURCE = ( - "projects/test-project/secrets/v2-runtime-database-url/versions/latest" + "projects/test-project/secrets/v2-runtime-database-url/versions/latest" # noqa: S105 # a Secret Manager resource path, not a secret ) TEST_OTEL_ENDPOINT = "https://collector.example.test" CLOUD_RUN_SERVICE_SCRIPTS = ( @@ -26,11 +26,12 @@ "scripts/resolve_cloud_run_candidate_state.sh", "scripts/set_cloud_run_revision.sh", ) +PRODUCTION_DB_PASSWORD_SECRET = "policyengine-api-prod-db-password:latest" # noqa: S105 # a Secret Manager secret name, not its value DEDICATED_CLOUD_RUN_RUNTIME_SERVICE_ACCOUNT = ( "policyengine-api-cr-runtime@policyengine-api.iam.gserviceaccount.com" ) CLOUD_RUN_SECRET_MAPPINGS = { - "POLICYENGINE_DB_PASSWORD": "policyengine-api-prod-db-password:latest", + "POLICYENGINE_DB_PASSWORD": PRODUCTION_DB_PASSWORD_SECRET, "POLICYENGINE_GITHUB_MICRODATA_AUTH_TOKEN": ( "policyengine-api-prod-github-microdata-token:latest" ), @@ -102,11 +103,9 @@ def _required_runtime_env() -> dict[str, str]: TEST_V2_RUNTIME_SECRET_RESOURCE ), "PRODUCTION_CLOUD_RUN_POLICYENGINE_DB_PASSWORD_SECRET": ( - "policyengine-api-prod-db-password:latest" - ), - "CLOUD_RUN_POLICYENGINE_DB_PASSWORD_SECRET": ( - "policyengine-api-prod-db-password:latest" + PRODUCTION_DB_PASSWORD_SECRET ), + "CLOUD_RUN_POLICYENGINE_DB_PASSWORD_SECRET": PRODUCTION_DB_PASSWORD_SECRET, "POLICYENGINE_DB_PASSWORD": "raw-db-secret-value", "POLICYENGINE_GITHUB_MICRODATA_AUTH_TOKEN": ("raw-github-secret-value"), "OPENAI_API_KEY": "raw-openai-secret-value", @@ -129,13 +128,12 @@ def _required_runtime_env() -> dict[str, str]: } -def _run_script( - path: str, - env: dict[str, str], - *args: str, +def _run_bash( + *arguments: str, + env: dict[str, str] | None = None, ) -> subprocess.CompletedProcess[str]: - return subprocess.run( - ["bash", path, *args], + return subprocess.run( # noqa: S603 # runs this repository's shell scripts with test-chosen arguments + ["bash", *arguments], # noqa: S607 # bash comes from the test environment's PATH cwd=REPO, env=env, text=True, @@ -144,6 +142,14 @@ def _run_script( ) +def _run_script( + path: str, + env: dict[str, str], + *args: str, +) -> subprocess.CompletedProcess[str]: + return _run_bash(path, *args, env=env) + + def _fake_gcloud(tmp_path: Path) -> tuple[Path, Path]: state_path = tmp_path / "cloud-run-state.json" state_path.write_text( @@ -389,17 +395,16 @@ def _run_simulation_version_guard( f"{shlex.quote(versions_json)}" '; }; . .github/request-simulation-model-versions.sh "$@"' ) - return subprocess.run( - ["bash", "-c", command, "request-simulation-model-versions.sh", *args], - cwd=REPO, + return _run_bash( + "-c", + command, + "request-simulation-model-versions.sh", + *args, env=_script_env( SIM_ENTRYPOINT=entrypoint, SIMULATION_ENTRYPOINT_URL="https://simulation.example.test", OLD_SIMULATION_GATEWAY_URL="https://old-gateway.example.test", ), - text=True, - capture_output=True, - check=False, ) @@ -462,13 +467,7 @@ def test_cloud_run_startup_uses_asgi_entrypoint(): def test_cloud_run_startup_script_is_shell_syntax_valid(): - result = subprocess.run( - ["bash", "-n", "gcp/cloud_run/start.sh"], - cwd=REPO, - text=True, - capture_output=True, - check=False, - ) + result = _run_bash("-n", "gcp/cloud_run/start.sh") assert result.returncode == 0, result.stderr @@ -560,13 +559,13 @@ def test_simulation_version_guard_rejects_missing_selected_endpoint( command = ( "curl() { printf '{}'; }; . .github/request-simulation-model-versions.sh \"$@\"" ) - result = subprocess.run( - ["bash", "-c", command, "request-simulation-model-versions.sh", "-py", "1.0"], - cwd=REPO, + result = _run_bash( + "-c", + command, + "request-simulation-model-versions.sh", + "-py", + "1.0", env=_script_env(SIM_ENTRYPOINT=entrypoint), - text=True, - capture_output=True, - check=False, ) assert result.returncode == 1 @@ -583,16 +582,12 @@ def test_policyengine_bundle_support_check_passes_pyproject_pin_to_guard(tmp_pat encoding="utf-8", ) - result = subprocess.run( - ["bash", ".github/check-policyengine-bundle-supported.sh"], - cwd=REPO, + result = _run_bash( + ".github/check-policyengine-bundle-supported.sh", env=_script_env( SIMULATION_VERSION_GUARD_SCRIPT=str(guard_script), CAPTURE_PATH=str(capture_path), ), - text=True, - capture_output=True, - check=False, ) pyproject = (REPO / "pyproject.toml").read_text(encoding="utf-8") @@ -741,11 +736,9 @@ def test_validate_cloud_run_deploy_env_accepts_direct_mode_from_environment(): TEST_V2_RUNTIME_SECRET_RESOURCE ), PRODUCTION_CLOUD_RUN_POLICYENGINE_DB_PASSWORD_SECRET=( - "policyengine-api-prod-db-password:latest" - ), - CLOUD_RUN_POLICYENGINE_DB_PASSWORD_SECRET=( - "policyengine-api-prod-db-password:latest" + PRODUCTION_DB_PASSWORD_SECRET ), + CLOUD_RUN_POLICYENGINE_DB_PASSWORD_SECRET=PRODUCTION_DB_PASSWORD_SECRET, **_v2_target_env(), **_gateway_auth_env(), **_observability_env(), @@ -898,11 +891,9 @@ def test_validate_cloud_run_deploy_env_requires_only_selected_url( TEST_V2_RUNTIME_SECRET_RESOURCE ), PRODUCTION_CLOUD_RUN_POLICYENGINE_DB_PASSWORD_SECRET=( - "policyengine-api-prod-db-password:latest" - ), - CLOUD_RUN_POLICYENGINE_DB_PASSWORD_SECRET=( - "policyengine-api-prod-db-password:latest" + PRODUCTION_DB_PASSWORD_SECRET ), + CLOUD_RUN_POLICYENGINE_DB_PASSWORD_SECRET=PRODUCTION_DB_PASSWORD_SECRET, **_v2_target_env(), **_gateway_auth_env(), **_observability_env(), @@ -2011,7 +2002,7 @@ def test_workflows_do_not_store_simulation_selector_in_git(): def test_workflows_scope_simulation_routing_config_to_github_environments(): selector_env = "SIM_ENTRYPOINT: ${{ vars.SIM_ENTRYPOINT }}" - secret_env = "SIMULATION_ENTRYPOINT_URL: ${{ secrets.SIMULATION_ENTRYPOINT_URL }}" + secret_env = "SIMULATION_ENTRYPOINT_URL: ${{ secrets.SIMULATION_ENTRYPOINT_URL }}" # noqa: S105 # a workflow line that references a secret by name pr_workflow = _pr_workflow() push_workflow = _push_workflow() @@ -2129,11 +2120,11 @@ def test_push_workflow_uses_dedicated_cloud_run_runtime_service_account(): cloud_run_production = _workflow_job_block(workflow, "deploy-cloud-run-candidate") runtime_account_secret = ( - "CLOUD_RUN_RUNTIME_SERVICE_ACCOUNT: " + "CLOUD_RUN_RUNTIME_SERVICE_ACCOUNT: " # noqa: S105 # a workflow line that references a secret by name "${{ secrets.GCP_CLOUD_RUN_RUNTIME_SERVICE_ACCOUNT }}" ) deploy_account_secret = ( - "CLOUD_RUN_RUNTIME_SERVICE_ACCOUNT: ${{ secrets.GCP_DEPLOY_SERVICE_ACCOUNT }}" + "CLOUD_RUN_RUNTIME_SERVICE_ACCOUNT: ${{ secrets.GCP_DEPLOY_SERVICE_ACCOUNT }}" # noqa: S105 # a workflow line that references a secret by name ) assert ( @@ -2236,7 +2227,7 @@ def test_push_workflow_syncs_environment_specific_secrets_before_deployment(): assert staging_sync_step.count(raw_secret_env) == 1 assert production_sync_step.count(raw_secret_env) == 1 database_secret_env = ( - "POLICYENGINE_DB_PASSWORD: ${{ secrets.POLICYENGINE_DB_PASSWORD }}" + "POLICYENGINE_DB_PASSWORD: ${{ secrets.POLICYENGINE_DB_PASSWORD }}" # noqa: S105 # a workflow line that references a secret by name ) assert database_secret_env not in staging_sync_step assert production_sync_step.count(database_secret_env) == 1 @@ -2315,9 +2306,7 @@ def test_sync_cloud_run_secrets_is_idempotent_and_grants_complete_batch(tmp_path CLOUD_RUN_PROJECT="policyengine-api", CLOUD_RUN_REGION="us-central1", CLOUD_RUN_SERVICE=PRODUCTION_CLOUD_RUN_SERVICE, - CLOUD_RUN_POLICYENGINE_DB_PASSWORD_SECRET=( - "policyengine-api-prod-db-password:latest" - ), + CLOUD_RUN_POLICYENGINE_DB_PASSWORD_SECRET=PRODUCTION_DB_PASSWORD_SECRET, **secret_values, ) @@ -2371,9 +2360,9 @@ def test_sync_cloud_run_staging_batch_preserves_distinct_database_secret(tmp_pat CLOUD_RUN_PROJECT="policyengine-api", CLOUD_RUN_REGION="us-central1", CLOUD_RUN_SERVICE=STAGING_CLOUD_RUN_SERVICE, - POLICYENGINE_GITHUB_MICRODATA_AUTH_TOKEN="microdata-value", + POLICYENGINE_GITHUB_MICRODATA_AUTH_TOKEN="microdata-value", # noqa: S106 # a fake secret value for the sync script OPENAI_API_KEY="openai-value", - PE_UK_PRIVATE_HF_READ_TOKEN="hf-value", + PE_UK_PRIVATE_HF_READ_TOKEN="hf-value", # noqa: S106 # a fake secret value for the sync script ) result = _run_script( @@ -2409,25 +2398,13 @@ def test_hugging_face_credential_uses_purpose_specific_managed_names(): def test_sync_cloud_run_secrets_script_is_shell_syntax_valid(): - result = subprocess.run( - ["bash", "-n", ".github/scripts/sync_cloud_run_secrets.sh"], - cwd=REPO, - text=True, - capture_output=True, - check=False, - ) + result = _run_bash("-n", ".github/scripts/sync_cloud_run_secrets.sh") assert result.returncode == 0, result.stderr def test_changelog_fragment_script_is_shell_syntax_valid(): - result = subprocess.run( - ["bash", "-n", ".github/scripts/check_changelog_fragment.sh"], - cwd=REPO, - text=True, - capture_output=True, - check=False, - ) + result = _run_bash("-n", ".github/scripts/check_changelog_fragment.sh") assert result.returncode == 0, result.stderr @@ -2460,7 +2437,7 @@ def reject_sqlalchemy(name, *args, **kwargs): builtins.__import__ = reject_sqlalchemy runpy.run_path("tests/integration/conftest.py", run_name="integration_conftest") """ - result = subprocess.run( + result = subprocess.run( # noqa: S603 # runs a fixed in-repository check with the current interpreter [sys.executable, "-c", script], cwd=REPO, text=True, diff --git a/tests/unit/test_constants.py b/tests/unit/test_constants.py index 33a4040c8..017ff8382 100644 --- a/tests/unit/test_constants.py +++ b/tests/unit/test_constants.py @@ -197,7 +197,7 @@ def find_spec(self, fullname, path=None, target=None): ) ) """ - result = subprocess.run( + result = subprocess.run( # noqa: S603 # runs fixed test code with the current interpreter [sys.executable, "-c", code], text=True, capture_output=True, diff --git a/tests/unit/test_country.py b/tests/unit/test_country.py index ea2c753fe..fc4be035d 100644 --- a/tests/unit/test_country.py +++ b/tests/unit/test_country.py @@ -28,7 +28,7 @@ def test__json_safe_serializes_dates(self): result = country._json_safe( { "release_date": date(2026, 5, 12), - "nested": [datetime(2026, 5, 12, 8, 30, 45)], + "nested": [datetime(2026, 5, 12, 8, 30, 45)], # noqa: DTZ001 # checks the offset-free isoformat of a naive datetime } ) diff --git a/tests/unit/test_deploy_pipeline_scripts.py b/tests/unit/test_deploy_pipeline_scripts.py index 01980f177..e6c60351d 100644 --- a/tests/unit/test_deploy_pipeline_scripts.py +++ b/tests/unit/test_deploy_pipeline_scripts.py @@ -27,8 +27,8 @@ def _recording_command(bin_dir: Path, name: str, log: Path, exit_code: int = 0) def _run( script: str, *args: str, cwd: Path = REPO, env: dict[str, str] | None = None ) -> subprocess.CompletedProcess[str]: - return subprocess.run( - ["bash", str(SCRIPTS / script), *args], + return subprocess.run( # noqa: S603 # runs this repository's script with test-chosen arguments + ["bash", str(SCRIPTS / script), *args], # noqa: S607 # bash comes from the test environment's PATH cwd=cwd, env={ "HOME": os.environ.get("HOME", ""), diff --git a/tests/unit/test_install_locked_ruff_script.py b/tests/unit/test_install_locked_ruff_script.py index 78ba3aae6..a3dc1173e 100644 --- a/tests/unit/test_install_locked_ruff_script.py +++ b/tests/unit/test_install_locked_ruff_script.py @@ -30,8 +30,8 @@ def _run( tmp_path: Path, *args: str, pip_exit: int = 0 ) -> tuple[subprocess.CompletedProcess[str], list[str]]: bin_dir, calls = _fake_commands(tmp_path, pip_exit=pip_exit) - result = subprocess.run( - ["bash", str(SCRIPT), *args], + result = subprocess.run( # noqa: S603 # runs this repository's script with test-chosen arguments + ["bash", str(SCRIPT), *args], # noqa: S607 # bash comes from the test environment's PATH cwd=REPO, env={ "PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}", diff --git a/tests/unit/test_observability_collector_verifier.py b/tests/unit/test_observability_collector_verifier.py index fce1d8c49..573e50ba2 100644 --- a/tests/unit/test_observability_collector_verifier.py +++ b/tests/unit/test_observability_collector_verifier.py @@ -11,6 +11,7 @@ ROOT = Path(__file__).parents[2] VERIFIER_PATH = ROOT / ".github" / "scripts" / "verify_observability_collector.py" +ACCESS_TOKEN = "access-token" # noqa: S105 # a fake test token def _load_verifier(): @@ -80,7 +81,7 @@ def Export(self, request, *, metadata, timeout): "identity-token", project_id="observability-project", metric_location="us-central1", - access_token="access-token", + access_token=ACCESS_TOKEN, timeout_seconds=3.0, delivery_timeout_seconds=9.0, ) @@ -117,7 +118,7 @@ def Export(self, request, *, metadata, timeout): "trace_id": probe.trace_id, "verification_id": probe.verification_id, "metric_location": "us-central1", - "access_token": "access-token", + "access_token": ACCESS_TOKEN, "started_at": probe.started_at, "timeout_seconds": 9.0, } @@ -150,7 +151,7 @@ def Export(self, *_args, **_kwargs): "identity-token", project_id="observability-project", metric_location="us-central1", - access_token="access-token", + access_token=ACCESS_TOKEN, ) @@ -173,7 +174,7 @@ def get_json(url: str, _access_token: str): "observability-project", "collector-verify-123", "us-central1", - "access-token", + ACCESS_TOKEN, 1_700_000_000.0, ) assert len(urls) == 2 diff --git a/tests/unit/test_simulation_and_report_routes.py b/tests/unit/test_simulation_and_report_routes.py index 559b260d8..38e1a70fb 100644 --- a/tests/unit/test_simulation_and_report_routes.py +++ b/tests/unit/test_simulation_and_report_routes.py @@ -220,9 +220,9 @@ def test_report_get_serializes_display_run_timestamps(orm_session_factory): with orm_session_factory.begin() as session: report = session.get(ReportOutput, report_id) run = session.get(ReportOutputRun, report.latest_successful_run_id) - run.requested_at = datetime(2026, 5, 4, 12, 0) - run.started_at = datetime(2026, 5, 4, 12, 1) - run.finished_at = datetime(2026, 5, 4, 12, 2) + run.requested_at = datetime(2026, 5, 4, 12, 0) # noqa: DTZ001 # report run timestamps are naive columns + run.started_at = datetime(2026, 5, 4, 12, 1) # noqa: DTZ001 # report run timestamps are naive columns + run.finished_at = datetime(2026, 5, 4, 12, 2) # noqa: DTZ001 # report run timestamps are naive columns result = create_test_client().get(f"/us/report/{report_id}").get_json()["result"] diff --git a/tests/unit/test_workflow_guard_scripts.py b/tests/unit/test_workflow_guard_scripts.py index 0763e7f15..da028d3eb 100644 --- a/tests/unit/test_workflow_guard_scripts.py +++ b/tests/unit/test_workflow_guard_scripts.py @@ -14,8 +14,8 @@ def _run( script: str, env: dict[str, str], *args: str ) -> subprocess.CompletedProcess[str]: - return subprocess.run( - ["bash", script, *args], + return subprocess.run( # noqa: S603 # runs this repository's script with test-chosen arguments + ["bash", script, *args], # noqa: S607 # bash comes from the test environment's PATH cwd=REPO, env={"HOME": os.environ.get("HOME", ""), "PATH": os.environ["PATH"], **env}, text=True, diff --git a/tests/unit/utils/test_cache_utils.py b/tests/unit/utils/test_cache_utils.py index 0b75db3ad..26ae0eb7e 100644 --- a/tests/unit/utils/test_cache_utils.py +++ b/tests/unit/utils/test_cache_utils.py @@ -85,7 +85,7 @@ def test_make_cache_key_stable_across_processes(): def run_with_seed(seed: str) -> str: env_cmd = [sys.executable, "-c", script] - result = subprocess.run( + result = subprocess.run( # noqa: S603 # runs fixed test code with the current interpreter env_cmd, capture_output=True, text=True, diff --git a/tests/unit/v2/test_alembic_v2.py b/tests/unit/v2/test_alembic_v2.py index 95a578b55..208b8743c 100644 --- a/tests/unit/v2/test_alembic_v2.py +++ b/tests/unit/v2/test_alembic_v2.py @@ -184,7 +184,7 @@ def test_persistent_mode_rejects_an_ambiguous_non_supabase_host() -> None: def test_target_errors_never_echo_url_passwords() -> None: - secret = "do-not-echo-this-password" + secret = "do-not-echo-this-password" # noqa: S105 # a sentinel that must not be echoed in the error with pytest.raises(V2MigrationTargetError) as raised: load_v2_alembic_settings( { diff --git a/tests/unit/v2/test_import_side_effects.py b/tests/unit/v2/test_import_side_effects.py index 768cf1843..0036b6b7f 100644 --- a/tests/unit/v2/test_import_side_effects.py +++ b/tests/unit/v2/test_import_side_effects.py @@ -75,7 +75,7 @@ def reject_ddl(*args, **kwargs): assert "policyengine_api.data.v2.household_migration_qualification" not in sys.modules """ - result = subprocess.run( + result = subprocess.run( # noqa: S603 # runs fixed test code with the current interpreter [sys.executable, "-c", script], cwd=tmp_path, env=_environment_without_v2(), @@ -124,7 +124,7 @@ def test_import_startup_and_request_never_create_runtime_sqlite( assert not list(Path.cwd().glob('*.db')) assert not list(Path.cwd().glob('*.init.lock')) """ - result = subprocess.run( + result = subprocess.run( # noqa: S603 # runs fixed test code with the current interpreter [sys.executable, "-c", script], cwd=tmp_path, env=environment, diff --git a/tests/unit/v2/test_metadata_deployment.py b/tests/unit/v2/test_metadata_deployment.py index c636c347d..7715fa397 100644 --- a/tests/unit/v2/test_metadata_deployment.py +++ b/tests/unit/v2/test_metadata_deployment.py @@ -113,7 +113,7 @@ def test_seeding_success_is_required_before_candidate_creation() -> None: def test_stage_9_deployment_shell_script_is_syntax_valid() -> None: result = subprocess.run( - ["bash", "-n", ".github/scripts/migrate_v2_metadata_schema.sh"], + ["bash", "-n", ".github/scripts/migrate_v2_metadata_schema.sh"], # noqa: S607 # bash comes from the test environment's PATH cwd=REPO, capture_output=True, text=True, diff --git a/tests/unit/v2/test_settings.py b/tests/unit/v2/test_settings.py index 38085e680..85712b4d2 100644 --- a/tests/unit/v2/test_settings.py +++ b/tests/unit/v2/test_settings.py @@ -33,7 +33,7 @@ "supabase.co:5432/postgres?sslmode=verify-ca" ) RUNTIME_SECRET_RESOURCE = ( - "projects/test-project/secrets/v2-runtime-database-url/versions/latest" + "projects/test-project/secrets/v2-runtime-database-url/versions/latest" # noqa: S105 # a Secret Manager resource path, not a secret ) @@ -250,7 +250,7 @@ def test_v1_and_debug_settings_never_supply_missing_v2_configuration() -> None: def test_configuration_errors_do_not_echo_secret_values() -> None: - secret_url = "postgresql+psycopg://user:do-not-echo@localhost/postgres" + secret_url = "postgresql+psycopg://user:do-not-echo@localhost/postgres" # noqa: S105 # a fake URL whose password must not be echoed with pytest.raises(V2ConfigurationError) as raised: load_v2_runtime_database_settings(