From a9b29fab3380cb617e8c8ea3416429fa6b1a9787 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 21:29:27 +0400 Subject: [PATCH 01/11] Remove commented-out code (ERA001) Delete the commented-out TestPolicySearch class in the to-refactor policy tests, along with the note that introduced it, the commented-out profiler import in the Flask app and the commented-out service imports in the services package. The GPT-4 experiment's example reform is reworded as prose. Co-Authored-By: Claude Opus 5.5 --- dashboard/experiments/gpt4_api_user.py | 5 ++- policyengine_api/api.py | 2 - policyengine_api/services/__init__.py | 2 - tests/to_refactor/python/test_policy.py | 58 ------------------------- 4 files changed, 3 insertions(+), 64 deletions(-) diff --git a/dashboard/experiments/gpt4_api_user.py b/dashboard/experiments/gpt4_api_user.py index 1fd37047b..5c73b7f46 100644 --- a/dashboard/experiments/gpt4_api_user.py +++ b/dashboard/experiments/gpt4_api_user.py @@ -46,8 +46,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. diff --git a/policyengine_api/api.py b/policyengine_api/api.py index df4fea71c..91709ad51 100644 --- a/policyengine_api/api.py +++ b/policyengine_api/api.py @@ -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/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/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), -# ) -# From 0341051c6259563e1e690513eb4e4ae99de3c8e4 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 21:29:50 +0400 Subject: [PATCH 02/11] Make the shebang Python scripts executable (EXE001) Each of these six scripts has a python3 shebang and a __main__ entry point, so it is meant to be run directly. Git now records them as executable; running them through python is unchanged. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/check_updates.py | 0 scripts/process_v1_household_mirror_event.py | 0 scripts/publish_v2_metadata_catalog.py | 0 scripts/qualify_v2_household_migration.py | 0 scripts/qualify_v2_policy_migration.py | 0 scripts/smoke_test_local_boot.py | 0 6 files changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 .github/scripts/check_updates.py mode change 100644 => 100755 scripts/process_v1_household_mirror_event.py mode change 100644 => 100755 scripts/publish_v2_metadata_catalog.py mode change 100644 => 100755 scripts/qualify_v2_household_migration.py mode change 100644 => 100755 scripts/qualify_v2_policy_migration.py mode change 100644 => 100755 scripts/smoke_test_local_boot.py diff --git a/.github/scripts/check_updates.py b/.github/scripts/check_updates.py old mode 100644 new mode 100755 diff --git a/scripts/process_v1_household_mirror_event.py b/scripts/process_v1_household_mirror_event.py old mode 100644 new mode 100755 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/smoke_test_local_boot.py b/scripts/smoke_test_local_boot.py old mode 100644 new mode 100755 From 79d6690d6174c14ef4aadc0ba3a9fb13d0e97af4 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 21:30:12 +0400 Subject: [PATCH 03/11] Rewrite the two TODO comments as plain notes (TD002, TD003, FIX002) The user-profile update note keeps its link to issue #2054, which was closed as not planned, and now says so. The simulation serializer note explains that its JSON-in-a-string projection serves only the v1 contract. Co-Authored-By: Claude Opus 5.5 --- policyengine_api/routes/simulation_routes.py | 7 ++++--- policyengine_api/routes/user_profile_routes.py | 5 +++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/policyengine_api/routes/simulation_routes.py b/policyengine_api/routes/simulation_routes.py index bffb45680..a6372c1fa 100644 --- a/policyengine_api/routes/simulation_routes.py +++ b/policyengine_api/routes/simulation_routes.py @@ -39,9 +39,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 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) From c7b00107e076d9118c162b6d335de30c1a9f50e3 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 21:31:00 +0400 Subject: [PATCH 04/11] Log household calculation errors on a module logger (LOG015) The three logging.exception calls in the household routes used the root logger. They now use a module logger, so the records are named after policyengine_api.routes.household_routes; the message text and traceback are unchanged. The module keeps its existing structured logger for log_struct. Co-Authored-By: Claude Opus 5.5 --- policyengine_api/routes/household_routes.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/policyengine_api/routes/household_routes.py b/policyengine_api/routes/household_routes.py index f49aabf17..10025e505 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() @@ -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, From 5cbed232f60280b17601eb1b8d8b1763f6ec9945 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 21:32:16 +0400 Subject: [PATCH 05/11] Decide each naive datetime (DTZ) The catalog extractor builds a date's midnight with tzinfo=UTC directly instead of attaching UTC afterwards; the result is the same aware value. Every other naive value stays naive, with an inline reason: reform impact start_time and report run timestamps are naive columns (and the runtime cache's datetime.min fallback must compare with them), economy start_time has always held local time, the default year and household period follow the host's local calendar, and the rules registry checks review and expiry dates against the local date. Co-Authored-By: Claude Opus 5.5 --- policyengine_api/constants.py | 2 +- policyengine_api/data/v2/catalog/extraction.py | 2 +- policyengine_api/runtime_cache/reform_impacts.py | 2 +- policyengine_api/services/economy_service.py | 2 +- policyengine_api/services/household_calculation_service.py | 2 +- scripts/rules/cli.py | 2 +- scripts/rules/guard.py | 2 +- tests/fixtures/services/economy_service.py | 6 +++--- tests/integration/test_live_economy.py | 2 +- tests/integration/test_runtime_cache_redis.py | 2 +- tests/unit/routes/test_reform_impact_routes.py | 4 ++-- tests/unit/runtime_cache/test_reform_impacts.py | 2 +- tests/unit/services/test_direct_orm_local_analysis.py | 2 +- tests/unit/services/test_reform_impacts_service.py | 2 +- tests/unit/test_country.py | 2 +- tests/unit/test_simulation_and_report_routes.py | 6 +++--- 16 files changed, 21 insertions(+), 21 deletions(-) diff --git a/policyengine_api/constants.py b/policyengine_api/constants.py index e9c97ff61..88f241cdd 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", 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/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/economy_service.py b/policyengine_api/services/economy_service.py index a91ffa945..a60f2dc3a 100644 --- a/policyengine_api/services/economy_service.py +++ b/policyengine_api/services/economy_service.py @@ -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/scripts/rules/cli.py b/scripts/rules/cli.py index a4134cacb..e0b4393c0 100644 --- a/scripts/rules/cli.py +++ b/scripts/rules/cli.py @@ -68,7 +68,7 @@ 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") 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/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..f14bbfc53 100644 --- a/tests/integration/test_runtime_cache_redis.py +++ b/tests/integration/test_runtime_cache_redis.py @@ -240,7 +240,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/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..0fedaf57c 100644 --- a/tests/unit/runtime_cache/test_reform_impacts.py +++ b/tests/unit/runtime_cache/test_reform_impacts.py @@ -38,7 +38,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, ) 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..4c1dd228d 100644 --- a/tests/unit/services/test_reform_impacts_service.py +++ b/tests/unit/services/test_reform_impacts_service.py @@ -41,7 +41,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, ) 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_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"] From 411f10bed72e3ea08c98cc6bb2a953a071812106 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 21:36:04 +0400 Subject: [PATCH 06/11] Replace type-narrowing asserts with explicit checks (S101) The eight asserts in the v2 household, policy and user-household services and the two in the rules ci-scripts guard now raise TypeError when the narrowed value has the wrong type. None runs inside a Pydantic validator and nothing catches AssertionError: the v2 routes return their generic 500 and the mirror paths report an "unexpected" failure for either type, and the rules callers check for a missing [ci_scripts] table first. The checks also hold under python -O, which strips asserts. Co-Authored-By: Claude Opus 5.5 --- policyengine_api/services/v2/households/services.py | 4 +++- .../services/v2/households/transformations.py | 12 +++++++++--- .../services/v2/households/validators.py | 4 +++- policyengine_api/services/v2/policies/services.py | 4 +++- .../services/v2/policies/transformations.py | 4 +++- .../services/v2/user_households/services.py | 4 +++- scripts/rules/ci_scripts.py | 8 ++++++-- 7 files changed, 30 insertions(+), 10 deletions(-) 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/scripts/rules/ci_scripts.py b/scripts/rules/ci_scripts.py index 0a0edecd2..571b63ac2 100644 --- a/scripts/rules/ci_scripts.py +++ b/scripts/rules/ci_scripts.py @@ -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), From 2919fc5cbeb4c68ec5d57e82b165b2ef34f2e8be Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 21:37:17 +0400 Subject: [PATCH 07/11] Remove debug prints from the Flask routes (T201) validate_country no longer prints "Validating country" on every request (#3909). The simulation and report routes no longer print on create and get, where the URL already carries the country and id; their update messages, whose id comes from the request body, become debug records on a module logger with the same text. The remaining prints are output by design and carry a reason: the CLI entry points' JSON evidence and results, the startup timings and the unauthenticated simulation client warning on stderr. Co-Authored-By: Claude Opus 5.5 --- policyengine_api/api.py | 2 +- policyengine_api/data/v2/catalog/initialization.py | 4 ++-- .../data/v2/household_migration_qualification.py | 4 ++-- .../data/v2/policy_migration_qualification.py | 4 ++-- policyengine_api/libs/simulation_entrypoint.py | 2 +- policyengine_api/routes/report_output_routes.py | 8 ++++---- policyengine_api/routes/simulation_routes.py | 9 ++++----- .../utils/payload_validators/validate_country.py | 1 - policyengine_api/worker_spm_release.py | 4 ++-- 9 files changed, 18 insertions(+), 20 deletions(-) diff --git a/policyengine_api/api.py b/policyengine_api/api.py index 91709ad51..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") 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/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/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 a6372c1fa..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() @@ -68,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" @@ -153,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) @@ -210,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/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__": From 4480544dbc2d2a7a9fa91f6a1dc18fb345580802 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 21:38:48 +0400 Subject: [PATCH 08/11] Keep the command-line scripts' prints, with reasons (T201) Printing is these scripts' job. The four scripts with more than three prints (the version bump, the migration baseline capture, the local boot smoke test and the rules CLI) route their output through one _report helper. Three of those helpers' prints carry the suppression; the version bump's helper passes its file argument through, which T201 does not flag, and its stderr messages still go to stderr. Every other print carries an inline reason, as does the catalog publication qualification test, whose printed evidence is its report. Co-Authored-By: Claude Opus 5.5 --- .github/bump_version.py | 14 ++++++--- .github/find-api-model-versions.py | 4 +-- .github/scripts/check_updates.py | 6 ++-- .../scripts/verify_observability_collector.py | 2 +- scripts/capture_migration_baseline.py | 13 +++++--- scripts/compare_migration_baseline.py | 4 +-- scripts/export_migration_contracts.py | 2 +- scripts/guards/migration_contracts.py | 6 ++-- scripts/measure_cloud_run_runtime.py | 2 +- scripts/process_v1_household_mirror_event.py | 4 +-- scripts/rules/ci_scripts.py | 2 +- scripts/rules/cli.py | 17 ++++++---- scripts/run_quality_guards.py | 6 ++-- scripts/smoke_test_local_boot.py | 31 +++++++++++-------- scripts/v1_database_migration.py | 2 +- ...st_v2_catalog_publication_qualification.py | 2 +- 16 files changed, 69 insertions(+), 48 deletions(-) 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 index e3c232e9f..ba4fbea3b 100755 --- 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..c5dec4bb5 100644 --- a/.github/scripts/verify_observability_collector.py +++ b/.github/scripts/verify_observability_collector.py @@ -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/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..056edd9a3 100644 --- a/scripts/measure_cloud_run_runtime.py +++ b/scripts/measure_cloud_run_runtime.py @@ -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 index 5be2f988e..75d15af06 100755 --- a/scripts/process_v1_household_mirror_event.py +++ b/scripts/process_v1_household_mirror_event.py @@ -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", @@ -256,7 +256,7 @@ def main( ) except Exception: # noqa: BLE001 # 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/rules/ci_scripts.py b/scripts/rules/ci_scripts.py index 571b63ac2..7ec9a6ea3 100644 --- a/scripts/rules/ci_scripts.py +++ b/scripts/rules/ci_scripts.py @@ -247,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 e0b4393c0..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) @@ -71,9 +76,9 @@ def main(argv: list[str] | None = None) -> int: 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/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 index eca2fd272..ee058aab8 100755 --- 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: @@ -49,8 +54,8 @@ def fetch(url: str) -> tuple[int, str]: 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,12 +92,12 @@ 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, @@ -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/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(), From 41905649810b4baa8a9d722869eaa4f4615075ac Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 21:45:23 +0400 Subject: [PATCH 09/11] Fix or explain the remaining security findings (S) Behaviour-identical fixes: the runtime cache version's sha1 is marked usedforsecurity=False (the digest is unchanged), and the GPT-4 experiment parses its embedding strings with ast.literal_eval. Test modules reuse one constant per fake credential or claim token instead of repeating the literal, and the Cloud Run deploy-script tests run bash through one helper. Every other finding keeps its code with an inline reason: try/except/pass on the deliberate telemetry and logging catch-alls (S110, alongside their BLE001 reason), environment-variable and Secret Manager names and test sentinels (S105, S106), fixed repository commands (S603, S607), fixed https and loopback URLs (S310), the experiment's untimed requests (S113), the load test's seeded random mix (S311) and fixed table names in a test query (S608). Co-Authored-By: Claude Opus 5.5 --- .../scripts/verify_observability_collector.py | 4 +- dashboard/experiments/gpt4_api_user.py | 11 +- policyengine_api/asgi_factory.py | 16 +-- policyengine_api/constants.py | 4 +- policyengine_api/data/v2/settings.py | 2 +- .../fastapi_routes/v2/metadata/common.py | 2 +- policyengine_api/gcp_logging.py | 2 +- policyengine_api/libs/gateway_auth.py | 4 +- policyengine_api/migration_logging.py | 4 +- policyengine_api/observability/runtime.py | 2 +- policyengine_api/request_context.py | 2 +- policyengine_api/routes/household_routes.py | 2 +- policyengine_api/routes/policy_routes.py | 2 +- policyengine_api/services/economy_service.py | 2 +- .../services/household_mirroring.py | 2 +- policyengine_api/services/policy_mirroring.py | 2 +- .../services/user_policy_mirroring.py | 2 +- scripts/measure_cloud_run_runtime.py | 2 +- scripts/process_v1_household_mirror_event.py | 4 +- scripts/rules/baseline.py | 8 +- scripts/rules/ci_scripts.py | 2 +- scripts/rules/tests/test_guard.py | 6 +- scripts/smoke_test_local_boot.py | 6 +- .../test_environment_variables.py | 2 +- tests/integration/test_runtime_cache_redis.py | 15 ++- .../test_v2_catalog_publication.py | 4 +- tests/unit/data/test_v1_database_migration.py | 14 ++- tests/unit/libs/test_gateway_auth.py | 6 +- .../unit/runtime_cache/test_reform_impacts.py | 22 ++-- tests/unit/runtime_cache/test_settings.py | 4 +- .../services/test_reform_impacts_service.py | 14 ++- tests/unit/test_alembic_workflows.py | 4 +- tests/unit/test_bundle_update_pins.py | 4 +- tests/unit/test_ci_complete_gate.py | 8 +- tests/unit/test_cloud_run_deploy_scripts.py | 115 +++++++----------- tests/unit/test_constants.py | 2 +- tests/unit/test_deploy_pipeline_scripts.py | 4 +- tests/unit/test_install_locked_ruff_script.py | 4 +- .../test_observability_collector_verifier.py | 9 +- tests/unit/test_workflow_guard_scripts.py | 4 +- tests/unit/utils/test_cache_utils.py | 2 +- tests/unit/v2/test_alembic_v2.py | 2 +- tests/unit/v2/test_import_side_effects.py | 4 +- tests/unit/v2/test_metadata_deployment.py | 2 +- tests/unit/v2/test_settings.py | 4 +- 45 files changed, 169 insertions(+), 173 deletions(-) diff --git a/.github/scripts/verify_observability_collector.py b/.github/scripts/verify_observability_collector.py index c5dec4bb5..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: diff --git a/dashboard/experiments/gpt4_api_user.py b/dashboard/experiments/gpt4_api_user.py index 5c73b7f46..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 @@ -113,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 @@ -123,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/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 88f241cdd..4ff7c2ed1 100644 --- a/policyengine_api/constants.py +++ b/policyengine_api/constants.py @@ -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/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/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 10025e505..26e7b6df0 100644 --- a/policyengine_api/routes/household_routes.py +++ b/policyengine_api/routes/household_routes.py @@ -109,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." 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/services/economy_service.py b/policyengine_api/services/economy_service.py index a60f2dc3a..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)) 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/scripts/measure_cloud_run_runtime.py b/scripts/measure_cloud_run_runtime.py index 056edd9a3..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 diff --git a/scripts/process_v1_household_mirror_event.py b/scripts/process_v1_household_mirror_event.py index 75d15af06..c5dad701c 100755 --- 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 = ( @@ -254,7 +254,7 @@ 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( # noqa: T201 # the command's JSON outcome is its output json.dumps( 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 7ec9a6ea3..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, 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/smoke_test_local_boot.py b/scripts/smoke_test_local_boot.py index ee058aab8..4af9b4f89 100755 --- a/scripts/smoke_test_local_boot.py +++ b/scripts/smoke_test_local_boot.py @@ -46,8 +46,8 @@ 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 @@ -103,7 +103,7 @@ def main() -> int: 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", 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/integration/test_runtime_cache_redis.py b/tests/integration/test_runtime_cache_redis.py index f14bbfc53..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, ) 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/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/runtime_cache/test_reform_impacts.py b/tests/unit/runtime_cache/test_reform_impacts.py index 0fedaf57c..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") @@ -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_reform_impacts_service.py b/tests/unit/services/test_reform_impacts_service.py index 4c1dd228d..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(): @@ -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_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_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( From 3f82610b84193a221cd83f675ac640835a720b4c Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 21:46:16 +0400 Subject: [PATCH 10/11] Enable ruff group 6 (security, logging and debugging leftovers) Select flake8-datetimez (DTZ), eradicate (ERA), flake8-executable (EXE), flake8-fixme (FIX), flake8-logging-format (G), flake8-logging (LOG), flake8-bandit (S), flake8-print (T20) and flake8-todos (TD). Tests follow all nine: their exemptions stay limited to S101, D1, ANN and PLR2004. Co-Authored-By: Claude Opus 5.5 --- pyproject.toml | 2 ++ 1 file changed, 2 insertions(+) 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). From 509c02c19d82447f2dc4665222ad0d7a747ae7a4 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 13 Oct 2026 21:51:01 +0400 Subject: [PATCH 11/11] Register group 6 in the ruff-lint rule api.quality.ruff-lint now lists DTZ, ERA, EXE, FIX, G, LOG, S, T20 and TD. The rendered rules page also picks up the rules tool's new hash. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 2 +- changelog.d/ruff-security-logging.changed.md | 1 + changelog.d/ruff-security-logging.fixed.md | 1 + docs/generated/rules.md | 8 ++++---- rules/registry.toml | 2 +- 5 files changed, 8 insertions(+), 6 deletions(-) create mode 100644 changelog.d/ruff-security-logging.changed.md create mode 100644 changelog.d/ruff-security-logging.fixed.md 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/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/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"