From 9ebb6cffd42be389cc18a73a5b4547088d90f7d0 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/16] 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 8709f00d7e92bec7dc519233c3309a8a5760073e 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/16] 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 41951a55dfeed65d6af92a055726671e06ac8ad8 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/16] 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 3943987f766aba6c6be54883f56675eea5a95963 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/16] 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 4023ec96c4c66fe3b2f4f48f51d62727e98597c6 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/16] 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 c9702a0ac..fed2a4959 100644 --- a/policyengine_api/services/economy_service.py +++ b/policyengine_api/services/economy_service.py @@ -1794,7 +1794,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 418def7ce..c6918518f 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 8aedb1ae31ec0bf9adafa3d8b278813ca7b5b5ed 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/16] 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 +++- scripts/rules/ci_scripts.py | 8 ++++++-- 6 files changed, 27 insertions(+), 9 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/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 00cd59598903239622909fdeb7ff444132bab6d1 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/16] 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 1cff31161..b1995d42a 100644 --- a/policyengine_api/libs/simulation_entrypoint.py +++ b/policyengine_api/libs/simulation_entrypoint.py @@ -237,7 +237,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 7b116c8cddc91ead927e462c599fb55210a70618 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/16] 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 01e2d0c09..3a4d8bba4 100644 --- a/scripts/export_migration_contracts.py +++ b/scripts/export_migration_contracts.py @@ -163,7 +163,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 3018357a5..0605733af 100644 --- a/scripts/guards/migration_contracts.py +++ b/scripts/guards/migration_contracts.py @@ -152,12 +152,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 5e6f2dc08919771e4d95cb1a6adb75650978d2b2 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/16] 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 ec24ec8a5..0095e480e 100644 --- a/policyengine_api/asgi_factory.py +++ b/policyengine_api/asgi_factory.py @@ -236,13 +236,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: @@ -261,7 +261,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( @@ -273,18 +273,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: @@ -308,13 +308,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 940092ea9..969e3fa8d 100644 --- a/policyengine_api/migration_logging.py +++ b/policyengine_api/migration_logging.py @@ -104,7 +104,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 @@ -146,7 +146,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 fed2a4959..6b4cfa7fe 100644 --- a/policyengine_api/services/economy_service.py +++ b/policyengine_api/services/economy_service.py @@ -1392,7 +1392,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 5e978e5a7..762201ead 100644 --- a/tests/unit/test_alembic_workflows.py +++ b/tests/unit/test_alembic_workflows.py @@ -288,7 +288,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, @@ -355,7 +355,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 ce44fb366..fc4d65f1a 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" ), @@ -110,11 +111,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", @@ -138,13 +137,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, @@ -153,6 +151,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( @@ -398,17 +404,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, ) @@ -471,13 +476,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 @@ -569,13 +568,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 @@ -592,16 +591,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") @@ -750,11 +745,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(), **_public_api_auth_env(), @@ -908,11 +901,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(), **_public_api_auth_env(), @@ -2022,7 +2013,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() @@ -2140,11 +2131,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 ( @@ -2247,7 +2238,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 @@ -2326,9 +2317,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, ) @@ -2382,9 +2371,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( @@ -2420,25 +2409,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 @@ -2471,7 +2448,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 d29b3807f..191da57a0 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 b29fd2eaf..b3f10f2af 100644 --- a/tests/unit/v2/test_alembic_v2.py +++ b/tests/unit/v2/test_alembic_v2.py @@ -185,7 +185,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 056446085..e2be6b91c 100644 --- a/tests/unit/v2/test_import_side_effects.py +++ b/tests/unit/v2/test_import_side_effects.py @@ -76,7 +76,7 @@ def reject_ddl(*args, **kwargs): assert "policyengine_api.data.v2.user_association_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(), @@ -125,7 +125,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 bb99da23d..093ee3bda 100644 --- a/tests/unit/v2/test_metadata_deployment.py +++ b/tests/unit/v2/test_metadata_deployment.py @@ -127,7 +127,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 04737699e32527c99894d0cb1f211ffb43bb7c93 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/16] 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 debc263b0..30508b48f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -96,6 +96,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 84ae5b13cb850951bbbfa7c0eef4564586dd51dd 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/16] 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 891a47e70..e6bea064c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -123,7 +123,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, v2 routes and services, and the user sign-in and account-deletion libraries pass mypy with untyped definitions disallowed. (blocking: mypy) - `api.quality.ruff-format`: All Python files are formatted with ruff format. (blocking: ruff format) -- `api.quality.ruff-lint`: All Python code passes ruff check with every rule family selected in pyproject.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. (blocking: ruff check (ANN, BLE, D, E4, E7, E9, EM, F, FLY, FURB, I, PIE, PYI, RET, RSE, TC, TRY, UP, W)) +- `api.quality.ruff-lint`: All Python code passes ruff check with every rule family selected in pyproject.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. (blocking: ruff check (ANN, BLE, D, DTZ, E4, E7, E9, EM, ERA, EXE, F, FIX, FLY, FURB, G, I, LOG, PIE, PYI, RET, RSE, S, T20, TC, TD, TRY, UP, W)) - `api.quality.runtime-evaluated-models-resolve`: Every package model is one ruff treats as runtime-evaluated (a base resolves, through the import in its own module, to a name in runtime-evaluated-base-classes, or is a same-module class that does), and every Pydantic model and SQLAlchemy mapper resolves its annotations once the package is imported. (blocking: pytest `tests/unit/test_runtime_evaluated_annotations.py`) - `api.quality.tests-pass`: The unit, contract and to-refactor test suites pass. (blocking: CI job `test`) - `api.quality.tests-ruff-exemptions`: Test files may use pytest's bare assert and need no docstrings, annotations or named constants for magic numbers (S101, D1, ANN, PLR2004); every other ruff rule applies to tests. (blocking: ruff per-file ignore of ANN, D1, PLR2004, S101 for `**/tests/**`) 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 fe7b0b4eb..64e106bd6 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -272,7 +272,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, v2 routes and services, and the user sign-in and account-deletion libraries 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.runtime-evaluated-models-resolve` | Every package model is one ruff treats as runtime-evaluated (a base resolves, through the import in its own module, to a name in runtime-evaluated-base-classes, or is a same-module class that does), and every Pydantic model and SQLAlchemy mapper resolves its annotations once the package is imported. | mechanical | enforced | yes | pytest `tests/unit/test_runtime_evaluated_annotations.py` | `pr.yml:test` | **unproven** | | `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` | — | @@ -292,7 +292,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_runtime_evaluated_models_resolve("api.quality.runtime-evaluated-models-resolve (enforced)") e_pytest_tests_unit_test_runtime_evaluated_annotations_py["pytest tests/unit/test_runtime_evaluated_annotations.py"] j_CI_pr_yml_test["CI pr.yml:test"] @@ -308,8 +308,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_runtime_evaluated_models_resolve --> e_pytest_tests_unit_test_runtime_evaluated_annotations_py e_pytest_tests_unit_test_runtime_evaluated_annotations_py --> j_CI_pr_yml_test r_api_quality_tests_pass --> e_CI_job_test diff --git a/rules/registry.toml b/rules/registry.toml index 4c8b8a997..1ad31d9a9 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -700,7 +700,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" From 13792b033fcd81490fb470698757e169bde03733 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Fri, 16 Oct 2026 01:00:48 +0400 Subject: [PATCH 12/16] Replace the type-narrowing asserts #3891 added with explicit checks (S101) The user-household and user-policy patch methods asserted that a supplied legacy id is not None before checking it in v1. They now raise TypeError instead, as the other v2 services do. The update models already reject an explicit null legacy id, so neither check can fail for a validated request; if one did, nothing catches either type and the route returns its generic 500. The checks also hold under python -O. Co-Authored-By: Claude Opus 5.5 --- policyengine_api/services/v2/user_households/services.py | 4 +++- policyengine_api/services/v2/user_policies/services.py | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/policyengine_api/services/v2/user_households/services.py b/policyengine_api/services/v2/user_households/services.py index 65e09c0d0..efe9c874a 100644 --- a/policyengine_api/services/v2/user_households/services.py +++ b/policyengine_api/services/v2/user_households/services.py @@ -265,7 +265,9 @@ def patch_user_household( """ repoint = "legacy_household_id" in association_input.model_fields_set if repoint: - assert association_input.legacy_household_id is not None + if association_input.legacy_household_id is None: + msg = "legacy_household_id must not be null" + raise TypeError(msg) self._require_legacy_household( country_id=country_id, legacy_household_id=association_input.legacy_household_id, diff --git a/policyengine_api/services/v2/user_policies/services.py b/policyengine_api/services/v2/user_policies/services.py index 6a65b90bf..508819a83 100644 --- a/policyengine_api/services/v2/user_policies/services.py +++ b/policyengine_api/services/v2/user_policies/services.py @@ -539,7 +539,9 @@ def patch_user_policy( """ repoint = "legacy_policy_id" in association_input.model_fields_set if repoint: - assert association_input.legacy_policy_id is not None + if association_input.legacy_policy_id is None: + msg = "legacy_policy_id must not be null" + raise TypeError(msg) self._require_legacy_policy( country_id=country_id, legacy_policy_id=association_input.legacy_policy_id, From 4905866ac565270429553078bc6697e63cc53222 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Fri, 16 Oct 2026 01:00:48 +0400 Subject: [PATCH 13/16] Explain or fix the rest of the group 6 findings in the code #3891 added - S105: the Auth0 Management secret environment-variable names and the invalid-token WWW-Authenticate challenge keep their literals with a reason, as do the Auth0 test's Secret Manager path and the route-auth test's leak sentinel. - S106: the Cloud Run deploy-script tests name the Auth0 Management secret resource once, as TEST_AUTH0_SECRET_RESOURCE, instead of repeating the literal three times. - S110: the user routes' failure logging keeps its try/except/pass with the BLE001 reason. - T201: the association qualification command's two prints are its JSON evidence and keep a reason. - EXE001: scripts/qualify_v2_user_association_migration.py has a python3 shebang and a __main__ entry point like its two siblings, so git now records it as executable. These were the other 10 ruff group 6 findings in the code #3891 added. Co-Authored-By: Claude Opus 5.5 --- .../v2/user_association_migration_qualification.py | 4 ++-- policyengine_api/fastapi_routes/v2/errors.py | 2 +- .../fastapi_routes/v2/user_resources.py | 2 +- policyengine_api/libs/auth0_management.py | 4 ++-- scripts/qualify_v2_user_association_migration.py | 0 tests/unit/libs/test_auth0_management.py | 2 +- tests/unit/test_cloud_run_deploy_scripts.py | 13 +++++-------- tests/unit/v2/test_user_route_auth.py | 2 +- 8 files changed, 13 insertions(+), 16 deletions(-) mode change 100644 => 100755 scripts/qualify_v2_user_association_migration.py diff --git a/policyengine_api/data/v2/user_association_migration_qualification.py b/policyengine_api/data/v2/user_association_migration_qualification.py index 9d7aad2a5..9b9e86a96 100644 --- a/policyengine_api/data/v2/user_association_migration_qualification.py +++ b/policyengine_api/data/v2/user_association_migration_qualification.py @@ -231,9 +231,9 @@ def main() -> int: try: evidence = qualify_user_association_migration() 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/fastapi_routes/v2/errors.py b/policyengine_api/fastapi_routes/v2/errors.py index c5ba09cfd..e99ace645 100644 --- a/policyengine_api/fastapi_routes/v2/errors.py +++ b/policyengine_api/fastapi_routes/v2/errors.py @@ -7,7 +7,7 @@ from starlette.responses import JSONResponse BEARER_CHALLENGE = "Bearer" -INVALID_TOKEN_BEARER_CHALLENGE = 'Bearer error="invalid_token"' +INVALID_TOKEN_BEARER_CHALLENGE = 'Bearer error="invalid_token"' # noqa: S105 # a WWW-Authenticate challenge, not a secret class V2RequestTooLargeError(ValueError): diff --git a/policyengine_api/fastapi_routes/v2/user_resources.py b/policyengine_api/fastapi_routes/v2/user_resources.py index 2496e9585..945dc448b 100644 --- a/policyengine_api/fastapi_routes/v2/user_resources.py +++ b/policyengine_api/fastapi_routes/v2/user_resources.py @@ -163,7 +163,7 @@ def _log_failure(event: str, error: Exception) -> None: } }, ) - except Exception: # noqa: BLE001 # observability must not alter the response + except Exception: # noqa: BLE001, S110 # observability must not alter the response pass diff --git a/policyengine_api/libs/auth0_management.py b/policyengine_api/libs/auth0_management.py index 6254b11ce..1db5aea88 100644 --- a/policyengine_api/libs/auth0_management.py +++ b/policyengine_api/libs/auth0_management.py @@ -30,8 +30,8 @@ AUTH0_MANAGEMENT_DOMAIN_ENV = "AUTH0_MANAGEMENT_DOMAIN" AUTH0_MANAGEMENT_CLIENT_ID_ENV = "AUTH0_MANAGEMENT_CLIENT_ID" -AUTH0_MANAGEMENT_CLIENT_SECRET_ENV = "AUTH0_MANAGEMENT_CLIENT_SECRET" -AUTH0_MANAGEMENT_CLIENT_SECRET_RESOURCE_ENV = "AUTH0_MANAGEMENT_CLIENT_SECRET_RESOURCE" +AUTH0_MANAGEMENT_CLIENT_SECRET_ENV = "AUTH0_MANAGEMENT_CLIENT_SECRET" # noqa: S105 # an environment variable name, not a secret +AUTH0_MANAGEMENT_CLIENT_SECRET_RESOURCE_ENV = "AUTH0_MANAGEMENT_CLIENT_SECRET_RESOURCE" # noqa: S105 # an environment variable name, not a secret AUTH0_MANAGEMENT_ENV_VARS = ( AUTH0_MANAGEMENT_DOMAIN_ENV, AUTH0_MANAGEMENT_CLIENT_ID_ENV, diff --git a/scripts/qualify_v2_user_association_migration.py b/scripts/qualify_v2_user_association_migration.py old mode 100644 new mode 100755 diff --git a/tests/unit/libs/test_auth0_management.py b/tests/unit/libs/test_auth0_management.py index 668a79b96..d3dfe8468 100644 --- a/tests/unit/libs/test_auth0_management.py +++ b/tests/unit/libs/test_auth0_management.py @@ -15,7 +15,7 @@ ) DOMAIN = "policyengine.us.auth0.com" -SECRET_RESOURCE = "projects/p/secrets/auth0-management/versions/latest" +SECRET_RESOURCE = "projects/p/secrets/auth0-management/versions/latest" # noqa: S105 # a Secret Manager resource path, not a secret def _client(handler, **settings) -> tuple[Auth0ManagementClient, list[httpx.Request]]: diff --git a/tests/unit/test_cloud_run_deploy_scripts.py b/tests/unit/test_cloud_run_deploy_scripts.py index fc4d65f1a..e3a99e285 100644 --- a/tests/unit/test_cloud_run_deploy_scripts.py +++ b/tests/unit/test_cloud_run_deploy_scripts.py @@ -19,6 +19,7 @@ TEST_V2_RUNTIME_SECRET_RESOURCE = ( "projects/test-project/secrets/v2-runtime-database-url/versions/latest" # noqa: S105 # a Secret Manager resource path, not a secret ) +TEST_AUTH0_SECRET_RESOURCE = "projects/p/secrets/auth0-management/versions/latest" # noqa: S105 # a Secret Manager resource path, not a secret TEST_OTEL_ENDPOINT = "https://collector.example.test" CLOUD_RUN_SERVICE_SCRIPTS = ( "scripts/deploy_cloud_run_candidate.sh", @@ -2535,16 +2536,14 @@ def test_deploy_cloud_run_candidate_passes_user_auth_settings(): CLOUD_RUN_TAG="stage3-test", AUTH0_MANAGEMENT_DOMAIN="policyengine.us.auth0.com", AUTH0_MANAGEMENT_CLIENT_ID="management-client", - AUTH0_MANAGEMENT_CLIENT_SECRET_RESOURCE=( - "projects/p/secrets/auth0-management/versions/latest" - ), + AUTH0_MANAGEMENT_CLIENT_SECRET_RESOURCE=TEST_AUTH0_SECRET_RESOURCE, ), ) assert with_management.returncode == 0, with_management.stderr assert "AUTH0_MANAGEMENT_DOMAIN=policyengine.us.auth0.com" in with_management.stdout assert ( - "AUTH0_MANAGEMENT_CLIENT_SECRET_RESOURCE=" - "projects/p/secrets/auth0-management/versions/latest" in with_management.stdout + f"AUTH0_MANAGEMENT_CLIENT_SECRET_RESOURCE={TEST_AUTH0_SECRET_RESOURCE}" + in with_management.stdout ) @@ -2583,9 +2582,7 @@ def test_validate_cloud_run_deploy_env_keeps_auth0_management_out_of_staging(): management = { "AUTH0_MANAGEMENT_DOMAIN": "policyengine.us.auth0.com", "AUTH0_MANAGEMENT_CLIENT_ID": "management-client", - "AUTH0_MANAGEMENT_CLIENT_SECRET_RESOURCE": ( - "projects/p/secrets/auth0-management/versions/latest" - ), + "AUTH0_MANAGEMENT_CLIENT_SECRET_RESOURCE": TEST_AUTH0_SECRET_RESOURCE, } staging = _script_env(**_staging_runtime_env(), **management) production = _script_env(**_required_runtime_env(), **management) diff --git a/tests/unit/v2/test_user_route_auth.py b/tests/unit/v2/test_user_route_auth.py index 108545fa3..50f390a00 100644 --- a/tests/unit/v2/test_user_route_auth.py +++ b/tests/unit/v2/test_user_route_auth.py @@ -269,7 +269,7 @@ def emit(self, record: logging.LogRecord) -> None: return response, handler.records -SECRET_SUBJECT = "google-oauth2|secret-subject" +SECRET_SUBJECT = "google-oauth2|secret-subject" # noqa: S105 # a sentinel that must not leak into the logs def test_unexpected_failures_are_logged_without_their_message() -> None: From 42cfdd0399b1383b33d2ffbe744f382e25c5ddbe Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Fri, 16 Oct 2026 09:45:47 +0400 Subject: [PATCH 14/16] Exclude Alembic revisions from ruff lint instead of exempting them per file Per-file-ignores are now only for tests, CI/CD scripts and command-line scripts, so the "migrations/*/versions/*.py" = ["ALL"] entry moves to [tool.ruff.lint] exclude. The revisions must stay exactly as Alembic generates them (api.migration.alembic-autogenerated-only), so ruff still doesn't lint them when it walks the repository, and ruff format still checks them as before. api.quality.alembic-revisions-unused-imports now describes the exclusion, with no ruff enforcer. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 2 +- docs/generated/rules.md | 10 ++++------ pyproject.toml | 8 +++++--- rules/registry.toml | 7 ++----- 4 files changed, 12 insertions(+), 15 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e6bea064c..9e7f8b690 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -119,7 +119,7 @@ To add or change a rule, read `docs/engineering/skills/rules-registry.md`. ### Code quality and types -- `api.quality.alembic-revisions-unused-imports`: Alembic revision files are exempt from ruff lint rules, because they must stay exactly as Alembic generates them. (advisory: ruff per-file ignore of ALL for `migrations/*/versions/*.py`, not blocking) +- `api.quality.alembic-revisions-unused-imports`: Ruff doesn't lint Alembic revision files (lint.exclude in pyproject.toml), because they must stay exactly as Alembic generates them. (advisory: nothing, not blocking) - `api.quality.environment-variables-load`: Required environment variables and credentials load in a deployed-like environment. (blocking: CI job `test_env_vars`) - `api.quality.mypy-v2`: Shared query models, FastAPI dependencies, v2 routes and services, and the user sign-in and account-deletion libraries pass mypy with untyped definitions disallowed. (blocking: mypy) - `api.quality.ruff-format`: All Python files are formatted with ruff format. (blocking: ruff format) diff --git a/docs/generated/rules.md b/docs/generated/rules.md index 64e106bd6..3ee5bdcaf 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -33,7 +33,7 @@ Every rule that governs `policyengine-api` is listed here with how it is enforce ### Advisory (checked but not blocking) -- `api.quality.alembic-revisions-unused-imports`: Alembic revision files are exempt from ruff lint rules, because they must stay exactly as Alembic generates them. +- `api.quality.alembic-revisions-unused-imports`: Ruff doesn't lint Alembic revision files (lint.exclude in pyproject.toml), because they must stay exactly as Alembic generates them. ### Unproven (no proof that the checker rejects a bad example) @@ -268,7 +268,7 @@ flowchart LR | Rule | Statement | Tier | Status | Blocking | Enforced by | CI | Proof | | --- | --- | --- | --- | --- | --- | --- | --- | -| `api.quality.alembic-revisions-unused-imports` | Alembic revision files are exempt from ruff lint rules, because they must stay exactly as Alembic generates them. | mechanical | advisory | no | ruff per-file ignore of ALL for `migrations/*/versions/*.py` | — | — | +| `api.quality.alembic-revisions-unused-imports` | Ruff doesn't lint Alembic revision files (lint.exclude in pyproject.toml), because they must stay exactly as Alembic generates them. | mechanical | advisory | no | nothing | — | — | | `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, v2 routes and services, and the user sign-in and account-deletion libraries 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` | — | @@ -280,8 +280,7 @@ flowchart LR ```mermaid flowchart LR r_api_quality_alembic_revisions_unused_imports("api.quality.alembic-revisions-unused-imports (advisory)") - e_ruff_per_file_ignore_of_ALL_for_migrations___versions___py["ruff per-file ignore of ALL for migrations/*/versions/*.py"] - local_only>"not run in CI"] + gap_unenforced>"not enforced"] r_api_quality_environment_variables_load("api.quality.environment-variables-load (enforced)") e_CI_job_test_env_vars["CI job test_env_vars"] j_CI_pr_yml_test_env_vars["CI pr.yml:test_env_vars"] @@ -300,8 +299,7 @@ flowchart LR e_CI_job_test["CI job test"] r_api_quality_tests_ruff_exemptions("api.quality.tests-ruff-exemptions (enforced)") e_ruff_per_file_ignore_of_ANN__D1__PLR2004__S101_for____tests___["ruff per-file ignore of ANN, D1, PLR2004, S101 for **/tests/**"] - r_api_quality_alembic_revisions_unused_imports --> e_ruff_per_file_ignore_of_ALL_for_migrations___versions___py - e_ruff_per_file_ignore_of_ALL_for_migrations___versions___py -.-> local_only + r_api_quality_alembic_revisions_unused_imports -.-> gap_unenforced r_api_quality_environment_variables_load --> e_CI_job_test_env_vars e_CI_job_test_env_vars --> j_CI_pr_yml_test_env_vars r_api_quality_mypy_v2 --> e_mypy diff --git a/pyproject.toml b/pyproject.toml index 30508b48f..f4b958467 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -102,11 +102,13 @@ select = [ # Mutually exclusive pydocstyle pairs: keep D211 (no blank line before a class # docstring) and D212 (summary on the first line). ignore = ["D203", "D213"] +# Alembic revisions must stay exactly as Alembic generates them +# (api.migration.alembic-autogenerated-only), so ruff does not lint them. This +# is an exclusion, not a file-level exception: per-file-ignores are only for +# tests, CI/CD scripts and command-line scripts. +exclude = ["migrations/*/versions/*.py"] [tool.ruff.lint.per-file-ignores] -# Alembic revisions must stay exactly as Alembic generates them -# (api.migration.alembic-autogenerated-only), so ruff does not lint them. -"migrations/*/versions/*.py" = ["ALL"] # Tests may use pytest's bare assert, and need no docstrings, annotations or # named constants for magic numbers. Every other rule applies to tests. "**/tests/**" = ["ANN", "D1", "PLR2004", "S101"] diff --git a/rules/registry.toml b/rules/registry.toml index 1ad31d9a9..56c7f5490 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -732,17 +732,14 @@ invocation = "make test" [[rule]] id = "api.quality.alembic-revisions-unused-imports" category = "quality" -statement = "Alembic revision files are exempt from ruff lint rules, because they must stay exactly as Alembic generates them." +statement = "Ruff doesn't lint Alembic revision files (lint.exclude in pyproject.toml), because they must stay exactly as Alembic generates them." rationale = "docs/engineering/skills/alembic-migrations.md#mandatory-generation-rule" scope = ["migrations/*/versions/*.py"] tier = "mechanical" status = "advisory" autofix = false [rule.enforcer] -kind = "ruff" -mode = "per-file-ignore" -pattern = "migrations/*/versions/*.py" -codes = ["ALL"] +kind = "none" [[rule]] id = "api.quality.tests-ruff-exemptions" From 93369b7c24a4d5974f8ccf5ab57e5a4cec4165c7 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Fri, 16 Oct 2026 09:44:12 +0400 Subject: [PATCH 15/16] Allow file-level ruff exceptions only in tests, CI/CD and command-line scripts A ruff per-file-ignores entry switches rules off for whole files. That is now acceptable where it's the norm: tests, CI/CD scripts under .github, and command-line scripts (under a scripts directory, or modules run through if __name__ == "__main__"). Everywhere else a kept finding still needs an inline suppression with its reason. The rules guard already requires a per-file-ignore rule for every entry. It now also expands each entry's pattern the way ruff does (globset syntax, where * and ? also match "/", and a pattern also matches a file by its name alone) and fails when the pattern covers any other file, is negated, or matches nothing. scripts/rules/file_exceptions.py holds the definitions, and the registry doc describes the policy. Co-Authored-By: Claude Opus 5.5 --- docs/engineering/skills/rules-registry.md | 26 +++- scripts/rules/file_exceptions.py | 160 ++++++++++++++++++++++ scripts/rules/resolve.py | 47 +++++++ scripts/rules/tests/test_guard.py | 88 ++++++++++++ 4 files changed, 319 insertions(+), 2 deletions(-) create mode 100644 scripts/rules/file_exceptions.py diff --git a/docs/engineering/skills/rules-registry.md b/docs/engineering/skills/rules-registry.md index d78d7824b..2ada8972b 100644 --- a/docs/engineering/skills/rules-registry.md +++ b/docs/engineering/skills/rules-registry.md @@ -69,7 +69,7 @@ gap is visible, rather than leaving it out. | --- | --- | --- | | `pytest` | `nodes` (`path::test` or `path::Class::test`), optional `project` (sub-project directory the paths are relative to) | Each test exists, and the CI job's command collects it | | `quality-guard` | `name`, optional `runner` | The name is in `GUARDS` in `scripts/run_quality_guards.py` | -| `ruff` | `mode` = `format`, `lint` (with `codes`) or `per-file-ignore` (with `pattern`, `codes`) | Codes are selected / the ignore entry exists | +| `ruff` | `mode` = `format`, `lint` (with `codes`) or `per-file-ignore` (with `pattern`, `codes`) | Codes are selected / the ignore entry exists and covers only [files allowed file-level exceptions](#file-level-exceptions) | | `mypy` | `paths` | Each path is in `[tool.mypy].files` | | `workflow-job` | optional `step` | The CI job exists (the job itself is the check) | | `pyright`, `import-linter` | `config` (+ `contract` for Import Linter) | The config file exists | @@ -117,13 +117,35 @@ fail the guard yet. - **Ruff is never ratcheted.** Adopting a ruff rule family is one standalone PR that changes the select list and fixes every resulting violation. Don't use - `--add-noqa` or blanket per-file ignores. + `--add-noqa`. A finding that is kept gets an inline `# noqa: CODE # reason`, + except in the files below. - Other enforcers (mypy, Pyright, Import Linter, custom guards) may start `ratcheted` with a baseline file of tolerated violations. Baselines may only shrink: the guard compares counts against the base branch. - Moving a rule from enforced/ratcheted to advisory/doc-only needs an explicit `[rule.downgrade] reason`. +## File-level exceptions + +A ruff `per-file-ignores` entry switches rules off for whole files. It is +allowed only where that's the norm rather than an exception to it: + +- **tests**: files under a `tests` or `authenticated_tests` directory; +- **CI/CD scripts**: files under `.github`; +- **command-line scripts**: files under a `scripts` directory, or modules that + run as a program through `if __name__ == "__main__":`. + +Everywhere else, keep a finding only with an inline suppression and its reason. + +Each entry needs a `per-file-ignore` rule (see below). The guard expands the +entry's pattern like ruff does and fails when the pattern: + +- covers any other file; +- is negated; +- matches nothing. + +`scripts/rules/file_exceptions.py` holds these definitions. + ## Hidden rules The guard fails when something enforces a rule that the registry doesn't diff --git a/scripts/rules/file_exceptions.py b/scripts/rules/file_exceptions.py new file mode 100644 index 000000000..2bca805b4 --- /dev/null +++ b/scripts/rules/file_exceptions.py @@ -0,0 +1,160 @@ +"""Where file-level lint exceptions are allowed. + +A ruff ``per-file-ignores`` entry switches rules off for whole files, so it is +allowed only for files where that is the norm rather than an exception to it: + +- **tests**: files under a ``tests`` or ``authenticated_tests`` directory; +- **CI/CD scripts**: files under ``.github``; +- **command-line scripts**: files under a ``scripts`` directory, or Python + modules that run as a program through ``if __name__ == "__main__":``. + +Everywhere else a kept finding gets an inline suppression with its reason. See +docs/engineering/skills/rules-registry.md#file-level-exceptions. +""" + +from __future__ import annotations + +import ast +import os +import re +from functools import cache +from pathlib import Path + +TEST_DIRECTORIES = frozenset({"tests", "authenticated_tests"}) +CI_DIRECTORY = ".github" +SCRIPT_DIRECTORY = "scripts" +LINTED_SUFFIXES = (".py", ".pyi") +# Directories that hold no source of this repository. +SKIPPED_DIRECTORIES = frozenset( + { + ".git", + ".venv", + "venv", + "node_modules", + "__pycache__", + ".mypy_cache", + ".ruff_cache", + } +) + + +# One piece of a ruff (globset) pattern: a wildcard, a character class, an +# alternation character, or a run of literal characters. +GLOB_PIECE = re.compile(r"\*\*/|\*\*|\*|\?|\[!?[^\]]*\]|[{},\[]|[^*?\[{},]+") +# As in ruff, "*" and "?" also match "/". +WILDCARDS = {"**/": "(?:.*/)?", "**": ".*", "*": ".*", "?": "."} + + +def _character_class(piece: str) -> str: + if piece == "[": + return re.escape(piece) + body = piece[1:-1].replace("\\", "\\\\") + if body.startswith("!"): + body = "^" + body[1:] + return f"[{body}]" + + +def glob_regex(pattern: str) -> re.Pattern[str]: + """Translate a ruff (globset) file pattern into a regex over relative paths. + + Supports ``**``, ``*``, ``?``, ``[...]`` and ``{a,b}`` alternatives. As in + ruff, ``*`` and ``?`` also match ``/``, so ``pkg/*.py`` covers + ``pkg/sub/module.py`` too. + """ + parts = [] + open_alternations = 0 + for piece in GLOB_PIECE.findall(pattern): + if piece in WILDCARDS: + parts.append(WILDCARDS[piece]) + elif piece.startswith("["): + parts.append(_character_class(piece)) + elif piece == "{": + parts.append("(?:") + open_alternations += 1 + elif piece == "}" and open_alternations: + parts.append(")") + open_alternations -= 1 + elif piece == "," and open_alternations: + parts.append("|") + else: + parts.append(re.escape(piece)) + return re.compile("".join(parts) + "$") + + +@cache +def linted_files(root: Path) -> tuple[str, ...]: + """Return the repository's Python files as sorted root-relative POSIX paths.""" + found = [] + for directory, subdirectories, files in os.walk(root): + subdirectories[:] = [ + name + for name in subdirectories + if name not in SKIPPED_DIRECTORIES + and (not name.startswith(".") or name == CI_DIRECTORY) + ] + found.extend( + Path(directory, name).relative_to(root).as_posix() + for name in files + if name.endswith(LINTED_SUFFIXES) + ) + return tuple(sorted(found)) + + +def matching_files(root: Path, pattern: str) -> list[str]: + """Return the files a ruff ``per-file-ignores`` pattern applies to. + + Like ruff, a pattern matches either a file's path or its name. + """ + regex = glob_regex(pattern) + return [ + path + for path in linted_files(root) + if regex.match(path) or regex.match(path.rsplit("/", 1)[-1]) + ] + + +def runs_as_program(root: Path, path: str) -> bool: + """Return whether a module has a top-level ``if __name__ == "__main__":``.""" + try: + tree = ast.parse((root / path).read_text(encoding="utf-8")) + except (OSError, SyntaxError, UnicodeDecodeError): + return False + for node in tree.body: + if ( + isinstance(node, ast.If) + and isinstance(node.test, ast.Compare) + and isinstance(node.test.left, ast.Name) + and node.test.left.id == "__name__" + and any( + isinstance(comparator, ast.Constant) and comparator.value == "__main__" + for comparator in node.test.comparators + ) + ): + return True + return False + + +def allows_file_level_exceptions(root: Path, path: str) -> bool: + """Return whether a file is a test, a CI/CD script or a command-line script.""" + directories = path.split("/")[:-1] + return ( + any(directory in TEST_DIRECTORIES for directory in directories) + or (bool(directories) and directories[0] == CI_DIRECTORY) + or SCRIPT_DIRECTORY in directories + or runs_as_program(root, path) + ) + + +def out_of_scope_files(root: Path, pattern: str) -> list[str] | None: + """Return the files a pattern covers that may not take file-level exceptions. + + ``None`` means the pattern can't be checked: it is negated, so it covers + every file it doesn't name. + """ + if pattern.startswith("!"): + return None + return [ + path + for path in matching_files(root, pattern) + if not allows_file_level_exceptions(root, path) + ] diff --git a/scripts/rules/resolve.py b/scripts/rules/resolve.py index ed2bba559..d00d15e91 100644 --- a/scripts/rules/resolve.py +++ b/scripts/rules/resolve.py @@ -13,6 +13,11 @@ from functools import cache from typing import TYPE_CHECKING, Any +from scripts.rules.file_exceptions import ( + linted_files, + matching_files, + out_of_scope_files, +) from scripts.rules.model import CiRef, Kind, Rule, Violation from scripts.rules.workflows import JobBlock, parse_jobs @@ -23,6 +28,8 @@ DEFAULT_RUNNER = "scripts/run_quality_guards.py" RUFF_DEFAULT_SELECT = ("E4", "E7", "E9", "F") PROOF_CI_FIELDS = ("workflow", "job", "invocation") +# How many offending files a file-level exception violation names. +SHOWN_OUT_OF_SCOPE_FILES = 3 # --- shared readers ------------------------------------------------------- @@ -38,6 +45,7 @@ def clear_caches() -> None: _read.cache_clear() workflow_jobs.cache_clear() test_names.cache_clear() + linted_files.cache_clear() @cache @@ -264,6 +272,45 @@ def _resolve_ruff(rule: Rule, root: Path) -> list[Violation]: "match enforcer.pattern/codes to the ruff config", ) ] + return _file_level_exception_scope(rule, root, pattern) + return [] + + +def _file_level_exception_scope( + rule: Rule, root: Path, pattern: str +) -> list[Violation]: + """Check a per-file ignore covers only tests, CI/CD and command-line scripts.""" + out_of_scope = out_of_scope_files(root, pattern) + if out_of_scope is None: + return [ + _v( + rule, + f"ruff per-file-ignores pattern {pattern!r} is negated", + "name the files the exception covers", + ) + ] + if not matching_files(root, pattern): + return [ + _v( + rule, + f"ruff per-file-ignores pattern {pattern!r} matches no files", + "fix the pattern, or remove the exception", + ) + ] + if out_of_scope: + shown = ", ".join(out_of_scope[:SHOWN_OUT_OF_SCOPE_FILES]) + hidden = len(out_of_scope) - SHOWN_OUT_OF_SCOPE_FILES + more = f" and {hidden} more" if hidden > 0 else "" + return [ + _v( + rule, + f"ruff per-file-ignores {pattern!r} covers files that may not take " + f"file-level exceptions: {shown}{more}", + "file-level exceptions are only for tests, CI/CD scripts and " + "command-line scripts; narrow the pattern and suppress other " + "findings inline with a reason", + ) + ] return [] diff --git a/scripts/rules/tests/test_guard.py b/scripts/rules/tests/test_guard.py index 731f65d49..6c80c0c3a 100644 --- a/scripts/rules/tests/test_guard.py +++ b/scripts/rules/tests/test_guard.py @@ -38,6 +38,34 @@ def with_rule(extra: str) -> dict[str, str]: return {"rules/registry.toml": REGISTRY + extra} +def per_file_ignore_rule(name: str, pattern: str) -> str: + """Return a registry rule describing a ruff per-file ignore of E501.""" + return f""" +[[rule]] +id = "ex.quality.{name}-ignores" +category = "quality" +statement = "Long lines are allowed here." +rationale = "docs/guide.md" +scope = ["rules/registry.toml"] +tier = "mechanical" +status = "advisory" +autofix = false +[rule.enforcer] +kind = "ruff" +mode = "per-file-ignore" +pattern = "{pattern}" +codes = ["E501"] +""" + + +def per_file_ignore_config(pattern: str) -> str: + """Return a pyproject.toml whose ruff config ignores E501 for ``pattern``.""" + return ( + '[tool.mypy]\nfiles = ["pkg"]\n\n' + f'[tool.ruff.lint.per-file-ignores]\n"{pattern}" = ["E501"]\n' + ) + + class ValidRepositoryTest(unittest.TestCase): def test_valid_repository_passes(self) -> None: with Repo() as repo: @@ -442,6 +470,66 @@ def test_unregistered_ruff_ignore(self) -> None: ) self.assertViolation(repo, "ruff per-file-ignores 'pkg/*.py' has no rule") + def test_file_level_exceptions_only_for_tests_ci_and_scripts(self) -> None: + program = 'def main():\n pass\n\n\nif __name__ == "__main__":\n main()\n' + allowed = { + "tests/**": "tests", + ".github/**": "ci", + "scripts/**": "scripts", + "pkg/cli.py": "cli", + } + for pattern, name in allowed.items(): + with ( + self.subTest(pattern=pattern), + Repo( + { + "pkg/cli.py": program, + ".github/scripts/release.py": "print(1)\n", + **with_rule(per_file_ignore_rule(name, pattern)), + } + ) as repo, + ): + repo.write("pyproject.toml", per_file_ignore_config(pattern)) + self.assertEqual([m for m in repo.messages() if "per-file" in m], []) + + def test_file_level_exception_outside_allowed_files_is_rejected(self) -> None: + with Repo(with_rule(per_file_ignore_rule("routes", "pkg/*.py"))) as repo: + repo.write("pyproject.toml", per_file_ignore_config("pkg/*.py")) + self.assertViolation( + repo, + "covers files that may not take file-level exceptions: pkg/routes.py", + ) + + def test_file_level_exception_needs_a_program_outside_scripts(self) -> None: + # A module with a main() that is never run as a program isn't a + # command-line script. + with Repo( + { + "pkg/cli.py": "def main():\n pass\n", + **with_rule(per_file_ignore_rule("cli", "pkg/cli.py")), + } + ) as repo: + repo.write("pyproject.toml", per_file_ignore_config("pkg/cli.py")) + self.assertViolation(repo, "may not take file-level exceptions: pkg/cli.py") + + def test_file_level_exception_star_crosses_directories(self) -> None: + # Ruff's * also matches "/", so "*/routes.py" reaches pkg/routes.py. + with Repo(with_rule(per_file_ignore_rule("star", "*/routes.py"))) as repo: + repo.write("pyproject.toml", per_file_ignore_config("*/routes.py")) + self.assertViolation( + repo, "may not take file-level exceptions: pkg/routes.py" + ) + + def test_negated_file_level_exception_is_rejected(self) -> None: + with Repo(with_rule(per_file_ignore_rule("negated", "!tests/**"))) as repo: + repo.write("pyproject.toml", per_file_ignore_config("!tests/**")) + self.assertViolation(repo, "pattern '!tests/**' is negated") + + def test_file_level_exception_must_match_a_file(self) -> None: + with Repo(with_rule(per_file_ignore_rule("stale", "nowhere/**"))) as repo: + repo.write("pyproject.toml", per_file_ignore_config("nowhere/**")) + self.assertViolation(repo, "pattern 'nowhere/**' matches no files") + def test_unregistered_ci_job(self) -> None: with Repo() as repo: text = repo.read(".github/workflows/pr.yml") From 88bf897ad458c5739ce3bfcd3c49af4283568b37 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Fri, 16 Oct 2026 09:49:09 +0400 Subject: [PATCH 16/16] Move group 6's test, CI and script suppressions to file-level exceptions Group 6's rules flag patterns that are the norm in tests, CI/CD scripts and command-line programs: fake credentials, subprocess calls to repository scripts, prints, naive datetimes, fixed URLs. Those files now take file-level exceptions, each registered as a per-file-ignore rule, and the 122 inline suppressions they cover are removed: - tests (**/tests/**, extending api.quality.tests-ruff-exemptions): S105 30, S603 14, DTZ001 13, S607 11, S106 2, S608 2, DTZ011 1, T201 1; - CI/CD scripts (.github/**): T201 6, S310 2; - command-line scripts (scripts/**): T201 17, S603 4, S607 2, DTZ011 2, S310 2, S110 2 (BLE001 stays on those two lines), S311 1; - modules that also run as programs (catalog initialization, the three migration qualifications and worker_spm_release): T201 10. Each entry lists exactly the codes these files suppressed, not whole families, so other security rules still apply to them. Suppressions in application code are unchanged. Co-Authored-By: Claude Opus 5.5 --- .github/find-api-model-versions.py | 4 +- .github/scripts/check_updates.py | 6 +- .../scripts/verify_observability_collector.py | 6 +- AGENTS.md | 5 +- docs/generated/rules.md | 29 ++++++-- .../data/v2/catalog/initialization.py | 4 +- .../v2/household_migration_qualification.py | 4 +- .../data/v2/policy_migration_qualification.py | 4 +- ...ser_association_migration_qualification.py | 4 +- policyengine_api/worker_spm_release.py | 4 +- pyproject.toml | 21 +++++- rules/registry.toml | 69 ++++++++++++++++++- scripts/capture_migration_baseline.py | 2 +- scripts/compare_migration_baseline.py | 4 +- scripts/export_migration_contracts.py | 2 +- scripts/guards/migration_contracts.py | 6 +- scripts/measure_cloud_run_runtime.py | 4 +- scripts/process_v1_household_mirror_event.py | 8 +-- scripts/rules/baseline.py | 8 +-- scripts/rules/ci_scripts.py | 4 +- scripts/rules/cli.py | 4 +- scripts/rules/guard.py | 2 +- scripts/rules/tests/test_guard.py | 6 +- scripts/run_quality_guards.py | 6 +- scripts/smoke_test_local_boot.py | 8 +-- scripts/v1_database_migration.py | 2 +- .../test_environment_variables.py | 2 +- tests/fixtures/services/economy_service.py | 6 +- tests/integration/test_live_economy.py | 2 +- tests/integration/test_runtime_cache_redis.py | 6 +- .../test_v2_catalog_publication.py | 4 +- ...st_v2_catalog_publication_qualification.py | 2 +- tests/unit/data/test_v1_database_migration.py | 6 +- tests/unit/libs/test_auth0_management.py | 2 +- tests/unit/libs/test_gateway_auth.py | 6 +- .../unit/routes/test_reform_impact_routes.py | 4 +- .../unit/runtime_cache/test_reform_impacts.py | 10 +-- tests/unit/runtime_cache/test_settings.py | 4 +- .../test_direct_orm_local_analysis.py | 2 +- .../services/test_reform_impacts_service.py | 6 +- 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 | 24 +++---- tests/unit/test_constants.py | 2 +- tests/unit/test_country.py | 2 +- tests/unit/test_deploy_pipeline_scripts.py | 4 +- tests/unit/test_install_locked_ruff_script.py | 4 +- .../test_observability_collector_verifier.py | 2 +- .../unit/test_simulation_and_report_routes.py | 6 +- 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 +- tests/unit/v2/test_user_route_auth.py | 2 +- 57 files changed, 233 insertions(+), 135 deletions(-) diff --git a/.github/find-api-model-versions.py b/.github/find-api-model-versions.py index 915dc9d67..9ae2ab7d4 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)}") # noqa: T201 # the shell assignments are this command's output + print(f"{key}={shlex.quote(value)}") else: find_api_model_versions_and_output_to_github() - print("API model versions found and written to GitHub environment.") # noqa: T201 # the command reports its result + print("API model versions found and written to GitHub environment.") sys.exit(0) diff --git a/.github/scripts/check_updates.py b/.github/scripts/check_updates.py index ba4fbea3b..e3c232e9f 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) # noqa: T201 # the command reports a missing changelog on stderr + print(f"Could not fetch changelog for {args.package}.", file=sys.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)) # noqa: T201 # the formatted changelog is this command's output + print(format_changes(changes)) else: - print("No changelog entries found between these versions.") # noqa: T201 # the command reports that nothing changed + print("No changelog entries found between these versions.") return 0 diff --git a/.github/scripts/verify_observability_collector.py b/.github/scripts/verify_observability_collector.py index f3d64eec4..2483183e5 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( # noqa: S310 # callers pass only https Google Cloud API URLs + request = Request( url, headers={"Authorization": f"Bearer {access_token}"}, ) try: - with urlopen(request, timeout=15) as response: # noqa: S310 # callers pass only https Google Cloud API URLs + with urlopen(request, timeout=15) as response: return json.load(response) except HTTPError as error: if error.code == 404: @@ -391,7 +391,7 @@ def main() -> None: metric_location=args.metric_location, access_token=os.environ["GOOGLE_OAUTH_ACCESS_TOKEN"], ) - print( # noqa: T201 # the command reports the verified ingestion + print( "Verified collector ingestion " f"(verification_id={probe.verification_id}, trace_id={probe.trace_id})" ) diff --git a/AGENTS.md b/AGENTS.md index 9e7f8b690..81677f6d6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -120,13 +120,16 @@ To add or change a rule, read `docs/engineering/skills/rules-registry.md`. ### Code quality and types - `api.quality.alembic-revisions-unused-imports`: Ruff doesn't lint Alembic revision files (lint.exclude in pyproject.toml), because they must stay exactly as Alembic generates them. (advisory: nothing, not blocking) +- `api.quality.ci-cd-scripts-ruff-exemptions`: CI/CD scripts under .github may print their results and fetch fixed URLs (T201, S310); every other ruff rule applies to them. (blocking: ruff per-file ignore of S310, T201 for `.github/**`) +- `api.quality.command-line-modules-ruff-exemptions`: Application modules that also run as command-line programs (if __name__ == "__main__") may print their output (T201); every other ruff rule applies to them. (blocking: ruff per-file ignore of T201 for `policyengine_api/{data/v2/catalog/initialization,data/v2/household_migration_qualification,data/v2/policy_migration_qualification,data/v2/user_association_migration_qualification,worker_spm_release}.py`) +- `api.quality.command-line-scripts-ruff-exemptions`: Command-line scripts under scripts/ may print their output, run other programs, fetch fixed URLs, stamp today's date, pick random samples and ignore a failed best-effort step (T201, S603, S607, S310, DTZ011, S311, S110); every other ruff rule applies to them. (blocking: ruff per-file ignore of DTZ011, S110, S310, S311, S603, S607, T201 for `scripts/**`) - `api.quality.environment-variables-load`: Required environment variables and credentials load in a deployed-like environment. (blocking: CI job `test_env_vars`) - `api.quality.mypy-v2`: Shared query models, FastAPI dependencies, v2 routes and services, and the user sign-in and account-deletion libraries pass mypy with untyped definitions disallowed. (blocking: mypy) - `api.quality.ruff-format`: All Python files are formatted with ruff format. (blocking: ruff format) - `api.quality.ruff-lint`: All Python code passes ruff check with every rule family selected in pyproject.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. (blocking: ruff check (ANN, BLE, D, DTZ, E4, E7, E9, EM, ERA, EXE, F, FIX, FLY, FURB, G, I, LOG, PIE, PYI, RET, RSE, S, T20, TC, TD, TRY, UP, W)) - `api.quality.runtime-evaluated-models-resolve`: Every package model is one ruff treats as runtime-evaluated (a base resolves, through the import in its own module, to a name in runtime-evaluated-base-classes, or is a same-module class that does), and every Pydantic model and SQLAlchemy mapper resolves its annotations once the package is imported. (blocking: pytest `tests/unit/test_runtime_evaluated_annotations.py`) - `api.quality.tests-pass`: The unit, contract and to-refactor test suites pass. (blocking: CI job `test`) -- `api.quality.tests-ruff-exemptions`: Test files may use pytest's bare assert and need no docstrings, annotations or named constants for magic numbers (S101, D1, ANN, PLR2004); every other ruff rule applies to tests. (blocking: ruff per-file ignore of ANN, D1, PLR2004, S101 for `**/tests/**`) +- `api.quality.tests-ruff-exemptions`: Test files may use pytest's bare assert and need no docstrings, annotations or named constants for magic numbers (S101, D1, ANN, PLR2004). They may also use fake credentials, naive datetimes, subprocess calls to repository scripts, SQL built from strings and prints (S105, S106, DTZ001, DTZ011, S603, S607, S608, T201). Every other ruff rule applies to tests. (blocking: ruff per-file ignore of ANN, D1, DTZ001, DTZ011, PLR2004, S101, S105, S106, S603, S607, S608, T201 for `**/tests/**`) ### Observability diff --git a/docs/generated/rules.md b/docs/generated/rules.md index 3ee5bdcaf..0e98f9965 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -8,11 +8,11 @@ Every rule that governs `policyengine-api` is listed here with how it is enforce | Tier | enforced | ratcheted | advisory | doc-only | Total | | --- | ---: | ---: | ---: | ---: | ---: | -| mechanical | 44 | 0 | 1 | 12 | 57 | +| mechanical | 47 | 0 | 1 | 12 | 60 | | metric | 0 | 0 | 0 | 0 | 0 | | judgment | 0 | 0 | 0 | 0 | 0 | -**Blocking:** 44 of 57 rules. +**Blocking:** 47 of 60 rules. ## Gaps @@ -269,18 +269,28 @@ flowchart LR | Rule | Statement | Tier | Status | Blocking | Enforced by | CI | Proof | | --- | --- | --- | --- | --- | --- | --- | --- | | `api.quality.alembic-revisions-unused-imports` | Ruff doesn't lint Alembic revision files (lint.exclude in pyproject.toml), because they must stay exactly as Alembic generates them. | mechanical | advisory | no | nothing | — | — | +| `api.quality.ci-cd-scripts-ruff-exemptions` | CI/CD scripts under .github may print their results and fetch fixed URLs (T201, S310); every other ruff rule applies to them. | mechanical | enforced | yes | ruff per-file ignore of S310, T201 for `.github/**` | `pr.yml:lint` | — | +| `api.quality.command-line-modules-ruff-exemptions` | Application modules that also run as command-line programs (if __name__ == "__main__") may print their output (T201); every other ruff rule applies to them. | mechanical | enforced | yes | ruff per-file ignore of T201 for `policyengine_api/{data/v2/catalog/initialization,data/v2/household_migration_qualification,data/v2/policy_migration_qualification,data/v2/user_association_migration_qualification,worker_spm_release}.py` | `pr.yml:lint` | — | +| `api.quality.command-line-scripts-ruff-exemptions` | Command-line scripts under scripts/ may print their output, run other programs, fetch fixed URLs, stamp today's date, pick random samples and ignore a failed best-effort step (T201, S603, S607, S310, DTZ011, S311, S110); every other ruff rule applies to them. | mechanical | enforced | yes | ruff per-file ignore of DTZ011, S110, S310, S311, S603, S607, T201 for `scripts/**` | `pr.yml:lint` | — | | `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, v2 routes and services, and the user sign-in and account-deletion libraries 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, 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.runtime-evaluated-models-resolve` | Every package model is one ruff treats as runtime-evaluated (a base resolves, through the import in its own module, to a name in runtime-evaluated-base-classes, or is a same-module class that does), and every Pydantic model and SQLAlchemy mapper resolves its annotations once the package is imported. | mechanical | enforced | yes | pytest `tests/unit/test_runtime_evaluated_annotations.py` | `pr.yml:test` | **unproven** | | `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` | — | +| `api.quality.tests-ruff-exemptions` | Test files may use pytest's bare assert and need no docstrings, annotations or named constants for magic numbers (S101, D1, ANN, PLR2004). They may also use fake credentials, naive datetimes, subprocess calls to repository scripts, SQL built from strings and prints (S105, S106, DTZ001, DTZ011, S603, S607, S608, T201). Every other ruff rule applies to tests. | mechanical | enforced | yes | ruff per-file ignore of ANN, D1, DTZ001, DTZ011, PLR2004, S101, S105, S106, S603, S607, S608, T201 for `**/tests/**` | `pr.yml:lint` | — | ```mermaid flowchart LR r_api_quality_alembic_revisions_unused_imports("api.quality.alembic-revisions-unused-imports (advisory)") gap_unenforced>"not enforced"] + r_api_quality_ci_cd_scripts_ruff_exemptions("api.quality.ci-cd-scripts-ruff-exemptions (enforced)") + e_ruff_per_file_ignore_of_S310__T201_for__github___["ruff per-file ignore of S310, T201 for .github/**"] + j_CI_pr_yml_lint["CI pr.yml:lint"] + r_api_quality_command_line_modules_ruff_exemptions("api.quality.command-line-modules-ruff-exemptions (enforced)") + e_ruff_per_file_ignore_of_T201_for_policyengine_api__data_v2_catalog_initialization_data_v2_household_migration_qualification_data_v2_policy_migration_qualification_data_v2_user_association_migration_qualification_worker_spm_release__py["ruff per-file ignore of T201 for policyengine_api/{data/v2/catalog/initialization,data/v2/household_migration_qualification,data/v2/policy_migration_qualification,data/v2/user_association_migration_qualification,worker_spm_release}.py"] + r_api_quality_command_line_scripts_ruff_exemptions("api.quality.command-line-scripts-ruff-exemptions (enforced)") + e_ruff_per_file_ignore_of_DTZ011__S110__S310__S311__S603__S607__T201_for_scripts___["ruff per-file ignore of DTZ011, S110, S310, S311, S603, S607, T201 for scripts/**"] r_api_quality_environment_variables_load("api.quality.environment-variables-load (enforced)") e_CI_job_test_env_vars["CI job test_env_vars"] j_CI_pr_yml_test_env_vars["CI pr.yml:test_env_vars"] @@ -289,7 +299,6 @@ flowchart LR j_CI_pr_yml_typecheck_v2["CI pr.yml:typecheck-v2"] r_api_quality_ruff_format("api.quality.ruff-format (enforced)") 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__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_runtime_evaluated_models_resolve("api.quality.runtime-evaluated-models-resolve (enforced)") @@ -298,8 +307,14 @@ flowchart LR r_api_quality_tests_pass("api.quality.tests-pass (enforced)") e_CI_job_test["CI job test"] r_api_quality_tests_ruff_exemptions("api.quality.tests-ruff-exemptions (enforced)") - e_ruff_per_file_ignore_of_ANN__D1__PLR2004__S101_for____tests___["ruff per-file ignore of ANN, D1, PLR2004, S101 for **/tests/**"] + e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ011__PLR2004__S101__S105__S106__S603__S607__S608__T201_for____tests___["ruff per-file ignore of ANN, D1, DTZ001, DTZ011, PLR2004, S101, S105, S106, S603, S607, S608, T201 for **/tests/**"] r_api_quality_alembic_revisions_unused_imports -.-> gap_unenforced + r_api_quality_ci_cd_scripts_ruff_exemptions --> e_ruff_per_file_ignore_of_S310__T201_for__github___ + e_ruff_per_file_ignore_of_S310__T201_for__github___ --> j_CI_pr_yml_lint + r_api_quality_command_line_modules_ruff_exemptions --> e_ruff_per_file_ignore_of_T201_for_policyengine_api__data_v2_catalog_initialization_data_v2_household_migration_qualification_data_v2_policy_migration_qualification_data_v2_user_association_migration_qualification_worker_spm_release__py + e_ruff_per_file_ignore_of_T201_for_policyengine_api__data_v2_catalog_initialization_data_v2_household_migration_qualification_data_v2_policy_migration_qualification_data_v2_user_association_migration_qualification_worker_spm_release__py --> j_CI_pr_yml_lint + r_api_quality_command_line_scripts_ruff_exemptions --> e_ruff_per_file_ignore_of_DTZ011__S110__S310__S311__S603__S607__T201_for_scripts___ + e_ruff_per_file_ignore_of_DTZ011__S110__S310__S311__S603__S607__T201_for_scripts___ --> j_CI_pr_yml_lint r_api_quality_environment_variables_load --> e_CI_job_test_env_vars e_CI_job_test_env_vars --> j_CI_pr_yml_test_env_vars r_api_quality_mypy_v2 --> e_mypy @@ -312,8 +327,8 @@ flowchart LR e_pytest_tests_unit_test_runtime_evaluated_annotations_py --> j_CI_pr_yml_test 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___ - e_ruff_per_file_ignore_of_ANN__D1__PLR2004__S101_for____tests___ --> j_CI_pr_yml_lint + r_api_quality_tests_ruff_exemptions --> e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ011__PLR2004__S101__S105__S106__S603__S607__S608__T201_for____tests___ + e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ011__PLR2004__S101__S105__S106__S603__S607__S608__T201_for____tests___ --> j_CI_pr_yml_lint ``` ## Observability diff --git a/policyengine_api/data/v2/catalog/initialization.py b/policyengine_api/data/v2/catalog/initialization.py index 83df2241e..10efdda6f 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) # noqa: T201 # the command's JSON evidence is its output + print(json.dumps(_error_payload(error), sort_keys=True), file=sys.stderr) return 1 - print(json.dumps(evidence.as_dict(), sort_keys=True)) # noqa: T201 # the command's JSON evidence is its output + print(json.dumps(evidence.as_dict(), sort_keys=True)) return 0 diff --git a/policyengine_api/data/v2/household_migration_qualification.py b/policyengine_api/data/v2/household_migration_qualification.py index 609d740f7..ce679323e 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) # noqa: T201 # the command's JSON evidence is its output + print(json.dumps(_error_payload(error), sort_keys=True), file=sys.stderr) return 1 - print(json.dumps(evidence.as_dict(), sort_keys=True)) # noqa: T201 # the command's JSON evidence is its output + print(json.dumps(evidence.as_dict(), sort_keys=True)) return 0 diff --git a/policyengine_api/data/v2/policy_migration_qualification.py b/policyengine_api/data/v2/policy_migration_qualification.py index 46a3b3486..beb03db24 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) # noqa: T201 # the command's JSON evidence is its output + print(json.dumps(_error_payload(error), sort_keys=True), file=sys.stderr) return 1 - print(json.dumps(evidence.as_dict(), sort_keys=True)) # noqa: T201 # the command's JSON evidence is its output + print(json.dumps(evidence.as_dict(), sort_keys=True)) return 0 diff --git a/policyengine_api/data/v2/user_association_migration_qualification.py b/policyengine_api/data/v2/user_association_migration_qualification.py index 9b9e86a96..9d7aad2a5 100644 --- a/policyengine_api/data/v2/user_association_migration_qualification.py +++ b/policyengine_api/data/v2/user_association_migration_qualification.py @@ -231,9 +231,9 @@ def main() -> int: try: evidence = qualify_user_association_migration() except Exception as error: # noqa: BLE001 # command must emit safe evidence - print(json.dumps(_error_payload(error), sort_keys=True), file=sys.stderr) # noqa: T201 # the command's JSON evidence is its output + print(json.dumps(_error_payload(error), sort_keys=True), file=sys.stderr) return 1 - print(json.dumps(evidence.as_dict(), sort_keys=True)) # noqa: T201 # the command's JSON evidence is its output + print(json.dumps(evidence.as_dict(), sort_keys=True)) return 0 diff --git a/policyengine_api/worker_spm_release.py b/policyengine_api/worker_spm_release.py index 13cdb5d8e..f83c9ebf2 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") # noqa: T201 # the command reports its result on stdout + print("The installed legacy bundle and registered worker agree") else: - print("The installed bundle and registered worker capability agree") # noqa: T201 # the command reports its result on stdout + print("The installed bundle and registered worker capability agree") if __name__ == "__main__": diff --git a/pyproject.toml b/pyproject.toml index f4b958467..cfb6fccbe 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -109,9 +109,26 @@ ignore = ["D203", "D213"] exclude = ["migrations/*/versions/*.py"] [tool.ruff.lint.per-file-ignores] +# File-level exceptions are allowed only for tests, CI/CD scripts and +# command-line scripts; the rules guard checks each pattern covers nothing else +# (docs/engineering/skills/rules-registry.md#file-level-exceptions). # Tests may use pytest's bare assert, and need no docstrings, annotations or -# named constants for magic numbers. Every other rule applies to tests. -"**/tests/**" = ["ANN", "D1", "PLR2004", "S101"] +# named constants for magic numbers. They may also use fake credentials, naive +# datetimes, subprocess calls to repository scripts, SQL built from strings and +# prints. Every other rule applies to tests. +"**/tests/**" = [ + "ANN", "D1", "DTZ001", "DTZ011", "PLR2004", "S101", "S105", "S106", "S603", + "S607", "S608", "T201", +] +# CI/CD scripts print their results and fetch fixed URLs. +".github/**" = ["S310", "T201"] +# Command-line scripts print their output, run other programs, fetch fixed +# URLs, stamp today's date and pick random samples. +"scripts/**" = [ + "DTZ011", "S110", "S310", "S311", "S603", "S607", "T201", +] +# Modules that also run as command-line programs print their output. +"policyengine_api/{data/v2/catalog/initialization,data/v2/household_migration_qualification,data/v2/policy_migration_qualification,data/v2/user_association_migration_qualification,worker_spm_release}.py" = ["T201"] [tool.mypy] python_version = "3.11" diff --git a/rules/registry.toml b/rules/registry.toml index 56c7f5490..68e960285 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -744,8 +744,8 @@ kind = "none" [[rule]] id = "api.quality.tests-ruff-exemptions" category = "quality" -statement = "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." -rationale = "docs/engineering/skills/rules-registry.md#baselines" +statement = "Test files may use pytest's bare assert and need no docstrings, annotations or named constants for magic numbers (S101, D1, ANN, PLR2004). They may also use fake credentials, naive datetimes, subprocess calls to repository scripts, SQL built from strings and prints (S105, S106, DTZ001, DTZ011, S603, S607, S608, T201). Every other ruff rule applies to tests." +rationale = "docs/engineering/skills/rules-registry.md#file-level-exceptions" scope = ["tests/**/*.py", "scripts/rules/tests/**/*.py"] tier = "mechanical" status = "enforced" @@ -754,7 +754,70 @@ autofix = false kind = "ruff" mode = "per-file-ignore" pattern = "**/tests/**" -codes = ["ANN", "D1", "PLR2004", "S101"] +codes = ["ANN", "D1", "DTZ001", "DTZ011", "PLR2004", "S101", "S105", "S106", "S603", "S607", "S608", "T201"] +[[rule.ci]] +workflow = ".github/workflows/pr.yml" +job = "lint" +invocation = "ruff check ." + +[[rule]] +id = "api.quality.ci-cd-scripts-ruff-exemptions" +category = "quality" +statement = "CI/CD scripts under .github may print their results and fetch fixed URLs (T201, S310); every other ruff rule applies to them." +rationale = "docs/engineering/skills/rules-registry.md#file-level-exceptions" +scope = [".github/**/*.py"] +tier = "mechanical" +status = "enforced" +autofix = false +[rule.enforcer] +kind = "ruff" +mode = "per-file-ignore" +pattern = ".github/**" +codes = ["S310", "T201"] +[[rule.ci]] +workflow = ".github/workflows/pr.yml" +job = "lint" +invocation = "ruff check ." + +[[rule]] +id = "api.quality.command-line-scripts-ruff-exemptions" +category = "quality" +statement = "Command-line scripts under scripts/ may print their output, run other programs, fetch fixed URLs, stamp today's date, pick random samples and ignore a failed best-effort step (T201, S603, S607, S310, DTZ011, S311, S110); every other ruff rule applies to them." +rationale = "docs/engineering/skills/rules-registry.md#file-level-exceptions" +scope = ["scripts/**/*.py"] +tier = "mechanical" +status = "enforced" +autofix = false +[rule.enforcer] +kind = "ruff" +mode = "per-file-ignore" +pattern = "scripts/**" +codes = ["DTZ011", "S110", "S310", "S311", "S603", "S607", "T201"] +[[rule.ci]] +workflow = ".github/workflows/pr.yml" +job = "lint" +invocation = "ruff check ." + +[[rule]] +id = "api.quality.command-line-modules-ruff-exemptions" +category = "quality" +statement = "Application modules that also run as command-line programs (if __name__ == \"__main__\") may print their output (T201); every other ruff rule applies to them." +rationale = "docs/engineering/skills/rules-registry.md#file-level-exceptions" +scope = [ + "policyengine_api/data/v2/catalog/initialization.py", + "policyengine_api/data/v2/household_migration_qualification.py", + "policyengine_api/data/v2/policy_migration_qualification.py", + "policyengine_api/data/v2/user_association_migration_qualification.py", + "policyengine_api/worker_spm_release.py", +] +tier = "mechanical" +status = "enforced" +autofix = false +[rule.enforcer] +kind = "ruff" +mode = "per-file-ignore" +pattern = "policyengine_api/{data/v2/catalog/initialization,data/v2/household_migration_qualification,data/v2/policy_migration_qualification,data/v2/user_association_migration_qualification,worker_spm_release}.py" +codes = ["T201"] [[rule.ci]] workflow = ".github/workflows/pr.yml" job = "lint" diff --git a/scripts/capture_migration_baseline.py b/scripts/capture_migration_baseline.py index 76ef4cff0..8cb02d539 100644 --- a/scripts/capture_migration_baseline.py +++ b/scripts/capture_migration_baseline.py @@ -24,7 +24,7 @@ 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 + print(message) @dataclass(frozen=True) diff --git a/scripts/compare_migration_baseline.py b/scripts/compare_migration_baseline.py index 74728e9e3..d3f320526 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) # 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 + print(line) + print("RESULT: PASS" if passed else "RESULT: FAIL") return 0 if passed else 1 diff --git a/scripts/export_migration_contracts.py b/scripts/export_migration_contracts.py index 3a4d8bba4..01e2d0c09 100644 --- a/scripts/export_migration_contracts.py +++ b/scripts/export_migration_contracts.py @@ -163,7 +163,7 @@ def main(argv: list[str] | None = None) -> int: json_path=Path(args.json), markdown_path=Path(args.markdown), ) - print( # noqa: T201 # the command reports what it exported + print( "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 0605733af..3018357a5 100644 --- a/scripts/guards/migration_contracts.py +++ b/scripts/guards/migration_contracts.py @@ -152,12 +152,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") # noqa: T201 # the guard reports its result + print("migration-contracts guard passed") return 0 - print("migration-contracts guard failed:") # noqa: T201 # the guard reports its result + print("migration-contracts guard failed:") for violation in violations: - print(f" - {violation}") # noqa: T201 # the guard reports its result + print(f" - {violation}") return 1 diff --git a/scripts/measure_cloud_run_runtime.py b/scripts/measure_cloud_run_runtime.py index 581f5fd98..9ca1ddd99 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) # noqa: S311 # a seeded generator makes the request mix reproducible; nothing secret depends on it + rng = random.Random(args.seed * 1000 + client_index) seq = 0 while time.monotonic() < deadline: seq += 1 @@ -171,7 +171,7 @@ async def _run(args: argparse.Namespace) -> dict: CALCULATE_PATH, json=throwaway, timeout=args.timeout_calculate ) except httpx.HTTPError as error: - print( # noqa: T201 # the command warns on stderr and keeps measuring + print( 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 c5dad701c..5be2f988e 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, S110 # outcome logging must not change the command result + except Exception: # noqa: BLE001 # outcome logging must not change the command result pass safe_types = (HouseholdEventCommandConfigurationError,) message = ( @@ -230,7 +230,7 @@ def main( if isinstance(error, safe_types) else "household create-event processing failed" ) - print( # noqa: T201 # the command's JSON outcome is its output + print( json.dumps( { "outcome": "error", @@ -254,9 +254,9 @@ def main( outcome="ok", destination_household_id=result.household_id, ) - except Exception: # noqa: BLE001, S110 # outcome logging must not change the command result + except Exception: # noqa: BLE001 # outcome logging must not change the command result pass - print( # noqa: T201 # the command's JSON outcome is its output + print( json.dumps( { "outcome": "ok", diff --git a/scripts/rules/baseline.py b/scripts/rules/baseline.py index 4ebe33fac..5bc3feb90 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( # 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 + verify = subprocess.run( + ["git", "rev-parse", "--verify", "--quiet", f"{ref}^{{commit}}"], cwd=root, capture_output=True, text=True, ) if verify.returncode != 0: raise BaseUnavailable(ref) - 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 + shown = subprocess.run( + ["git", "show", f"{ref}:{REGISTRY_PATH.as_posix()}"], cwd=root, capture_output=True, text=True, diff --git a/scripts/rules/ci_scripts.py b/scripts/rules/ci_scripts.py index e252cea64..571b63ac2 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( # noqa: S603 # runs the resolved shellcheck on this repository's scripts + result = subprocess.run( [shellcheck, "--format=gcc", *scripts], cwd=root, capture_output=True, @@ -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}") # noqa: T201 # the guard adapter prints its notes + print(f"note: {note}") return [str(violation) for violation in violations] diff --git a/scripts/rules/cli.py b/scripts/rules/cli.py index 9df65da56..78ebf6149 100644 --- a/scripts/rules/cli.py +++ b/scripts/rules/cli.py @@ -16,7 +16,7 @@ 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 + print(message) def _print(violations: list, notes: list[str] | None = None) -> int: @@ -73,7 +73,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()) # noqa: DTZ011 # review and expiry dates are calendar days, checked against the local date + violations += validate(registry, root, date.today()) if args.command == "render": if violations: _report("render refused: fix the registry first") diff --git a/scripts/rules/guard.py b/scripts/rules/guard.py index 092425003..3efa3d4f5 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()) # noqa: DTZ011 # review and expiry dates are calendar days, checked against the local date + result.violations += validate(registry, root, today or date.today()) for rule in registry.rules: result.violations += resolve_enforcer(rule, root) result.violations += resolve_ci(rule, root) diff --git a/scripts/rules/tests/test_guard.py b/scripts/rules/tests/test_guard.py index 6c80c0c3a..00151ed23 100644 --- a/scripts/rules/tests/test_guard.py +++ b/scripts/rules/tests/test_guard.py @@ -677,9 +677,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) # 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 + 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) repo.edit("rules/registry.toml", "count = 1", "count = 2") repo.write("rules/baselines/ratcheted.txt", "pkg/routes.py\npkg/other.py\n") repo.render() diff --git a/scripts/run_quality_guards.py b/scripts/run_quality_guards.py index bf01f94e6..a89b7bd3b 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") # noqa: T201 # the guard runner reports each result + print(f"{name}: passed") continue failed = True - print(f"{name}: failed") # noqa: T201 # the guard runner reports each result + print(f"{name}: failed") for violation in violations: - print(f" - {violation}") # noqa: T201 # the guard runner reports each result + print(f" - {violation}") return 1 if failed else 0 diff --git a/scripts/smoke_test_local_boot.py b/scripts/smoke_test_local_boot.py index 4af9b4f89..8aa3cdde8 100755 --- a/scripts/smoke_test_local_boot.py +++ b/scripts/smoke_test_local_boot.py @@ -27,7 +27,7 @@ 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 + print(message) def find_free_port() -> int: @@ -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") # 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 + request = urllib.request.Request(url, method="GET") + with urllib.request.urlopen(request, timeout=HTTP_TIMEOUT_SECONDS) as response: 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( # noqa: S603 # starts this repository's app with the current interpreter + process = subprocess.Popen( [ sys.executable, "-m", diff --git a/scripts/v1_database_migration.py b/scripts/v1_database_migration.py index e51da59a2..679631d8f 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) # noqa: T201 # the command prints the database state for its caller + print(database_state(connection).value) elif args.mode == "verify-head": verify_head_schema(connection) else: diff --git a/tests/env_variables/test_environment_variables.py b/tests/env_variables/test_environment_variables.py index 191c7eb37..de4175d7b 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" # noqa: S105 # the Secret Manager secret's name, not its value +EXPECTED_HUGGING_FACE_TOKEN_NAME = "pe-uk-private-hf-read-token" GITHUB_API_URL = "https://api.github.com/user" diff --git a/tests/fixtures/services/economy_service.py b/tests/fixtures/services/economy_service.py index c6918518f..418def7ce 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) # noqa: DTZ001 # stands in for the naive local time that economy_service stores + mock_now = datetime.datetime(2025, 6, 26, 12, 0, 0) 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), # 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 + 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), ) 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 6e68d8421..5e9d062c4 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 # noqa: DTZ011 # matches the API's default year, which follows the local calendar +CURRENT_YEAR = date.today().year 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 8435b8e85..2b114f3cc 100644 --- a/tests/integration/test_runtime_cache_redis.py +++ b/tests/integration/test_runtime_cache_redis.py @@ -24,8 +24,8 @@ 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 +WRITER_CLAIM_TOKEN = "writer" +CONTENDER_CLAIM_TOKEN = "contender" @pytest.fixture @@ -243,7 +243,7 @@ def test_real_reform_indexes_are_cross_connection_bounded_and_expiring( reform_impact_json={}, status="computing", message=None, - start_time=datetime(2026, 1, day), # noqa: DTZ001 # reform_impact start_time is a naive column + start_time=datetime(2026, 1, day), end_time=None, execution_id=execution_id, ) diff --git a/tests/integration/test_v2_catalog_publication.py b/tests/integration/test_v2_catalog_publication.py index 233b7b19e..06e48f64c 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}") # noqa: S608 # table names come from the fixed tuple below, never from input + text(f"SELECT count(*) FROM {table_name}") ).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") # noqa: S608 # table names come from the fixed tuple below, never from input + text(f"SELECT id FROM {table_name} ORDER BY id") ).scalars() ) for table_name in ( diff --git a/tests/integration/test_v2_catalog_publication_qualification.py b/tests/integration/test_v2_catalog_publication_qualification.py index 98af69ad4..8400c4ac9 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( # noqa: T201 # the qualification evidence is this test's report + print( json.dumps( { **evidence.as_dict(), diff --git a/tests/unit/data/test_v1_database_migration.py b/tests/unit/data/test_v1_database_migration.py index 8447a3087..2b5d37579 100644 --- a/tests/unit/data/test_v1_database_migration.py +++ b/tests/unit/data/test_v1_database_migration.py @@ -16,8 +16,8 @@ ) # 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 +FAKE_PASSWORD_WITH_URL_CHARACTERS = "p@ss:/word" +FAKE_PASSWORD = "p@ssword" def test_database_url_uses_sqlalchemy_url_without_stringifying_credentials(): @@ -161,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" # noqa: S105 # a sentinel that must not leak into the error + secret = "do-not-print-this-value" difference = ("unknown", secret) with pytest.raises(ValueError, match="Unsupported metadata difference") as error: diff --git a/tests/unit/libs/test_auth0_management.py b/tests/unit/libs/test_auth0_management.py index d3dfe8468..668a79b96 100644 --- a/tests/unit/libs/test_auth0_management.py +++ b/tests/unit/libs/test_auth0_management.py @@ -15,7 +15,7 @@ ) DOMAIN = "policyengine.us.auth0.com" -SECRET_RESOURCE = "projects/p/secrets/auth0-management/versions/latest" # noqa: S105 # a Secret Manager resource path, not a secret +SECRET_RESOURCE = "projects/p/secrets/auth0-management/versions/latest" def _client(handler, **settings) -> tuple[Auth0ManagementClient, list[httpx.Request]]: diff --git a/tests/unit/libs/test_gateway_auth.py b/tests/unit/libs/test_gateway_auth.py index 1a704e169..8103c9a69 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" # noqa: S105 # a fake test secret +CLIENT_SECRET = "test-client-secret" CLIENT_SECRET_RESOURCE = ( - "projects/policyengine-api/secrets/gateway-auth-client-secret/versions/1" # noqa: S105 # a Secret Manager resource path, not a secret + "projects/policyengine-api/secrets/gateway-auth-client-secret/versions/1" ) @@ -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" # noqa: S105 # the fake token from the mocked response + assert token == "tok-1" mock_post.assert_called_once() call_kwargs = mock_post.call_args.kwargs assert call_kwargs["json"]["grant_type"] == "client_credentials" diff --git a/tests/unit/routes/test_reform_impact_routes.py b/tests/unit/routes/test_reform_impact_routes.py index 4274317a7..29ffbf65e 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) # noqa: DTZ001 # stands in for the service's naive UTC _now() +COMPLETED_AT = datetime(2026, 1, 1, 1) @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), # noqa: DTZ001 # reform_impact start_time is a naive column + start_time=datetime(2026, 1, 1, 0, i // 60, i % 60), 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 1c48e0ab3..2f47afc6f 100644 --- a/tests/unit/runtime_cache/test_reform_impacts.py +++ b/tests/unit/runtime_cache/test_reform_impacts.py @@ -19,10 +19,10 @@ ) # 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 +OWNER_CLAIM_TOKEN = "owner" +CONTENDER_CLAIM_TOKEN = "contender" +CLIFF_OWNER_CLAIM_TOKEN = "cliff-owner" +NEXT_OWNER_CLAIM_TOKEN = "next-owner" def _namespace() -> CacheNamespace: @@ -44,7 +44,7 @@ def _impact(execution_id: str, options_hash: str, day: int) -> CachedReformImpac reform_impact_json={}, status="computing", message=None, - start_time=datetime(2026, 1, day), # noqa: DTZ001 # reform_impact start_time is a naive column + start_time=datetime(2026, 1, day), end_time=None, execution_id=execution_id, ) diff --git a/tests/unit/runtime_cache/test_settings.py b/tests/unit/runtime_cache/test_settings.py index 6a8aa61e7..5f7c9d699 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" # 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 + secret = "do-not-print-cache-password" + ca_secret = "do-not-print-cache-ca" settings = load_runtime_cache_settings( { RUNTIME_CACHE_MODE: "deployed", diff --git a/tests/unit/services/test_direct_orm_local_analysis.py b/tests/unit/services/test_direct_orm_local_analysis.py index cc5383bf7..a0ef9d91f 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), # noqa: DTZ001 # reform_impact start_time is a naive column + start_time=datetime(2026, 1, 1), execution_id="job", ) diff --git a/tests/unit/services/test_reform_impacts_service.py b/tests/unit/services/test_reform_impacts_service.py index 49e55ecda..84acb781b 100644 --- a/tests/unit/services/test_reform_impacts_service.py +++ b/tests/unit/services/test_reform_impacts_service.py @@ -12,8 +12,8 @@ ) # 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 +OWNER_CLAIM_TOKEN = "owner" +CONTENDER_CLAIM_TOKEN = "contender" @pytest.fixture @@ -45,7 +45,7 @@ def _create_impact( status="computing", api_version="1", reform_impact_json={}, - start_time=datetime(2026, 1, day), # noqa: DTZ001 # reform_impact start_time is a naive column + start_time=datetime(2026, 1, day), execution_id=execution_id, ) diff --git a/tests/unit/test_alembic_workflows.py b/tests/unit/test_alembic_workflows.py index 762201ead..5e978e5a7 100644 --- a/tests/unit/test_alembic_workflows.py +++ b/tests/unit/test_alembic_workflows.py @@ -288,7 +288,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"], # noqa: S607 # bash comes from the test environment's PATH + ["bash", ".github/scripts/validate_database_environment.sh", "cloud-sql"], cwd=REPO, env={ **os.environ, @@ -355,7 +355,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"], # noqa: S607 # bash comes from the test environment's PATH + ["bash", ".github/scripts/migrate_v1_cloud_sql.sh"], cwd=REPO, env={ **os.environ, diff --git a/tests/unit/test_bundle_update_pins.py b/tests/unit/test_bundle_update_pins.py index f4aff5516..2ccfb19e6 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( # 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 + result = subprocess.run( + ["bash", str(SCRIPT), *arguments], cwd=root, env={ **os.environ, diff --git a/tests/unit/test_ci_complete_gate.py b/tests/unit/test_ci_complete_gate.py index 07aefdb86..618d05720 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( # noqa: S603 # runs this repository's script with test-chosen arguments - ["bash", GATE_SCRIPT], # noqa: S607 # bash comes from the test environment's PATH + return subprocess.run( + ["bash", GATE_SCRIPT], 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( # noqa: S603 # runs this repository's script with test-chosen arguments - ["bash", GATE_SCRIPT], # noqa: S607 # bash comes from the test environment's PATH + result = subprocess.run( + ["bash", GATE_SCRIPT], 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 e3a99e285..e5cc55354 100644 --- a/tests/unit/test_cloud_run_deploy_scripts.py +++ b/tests/unit/test_cloud_run_deploy_scripts.py @@ -17,9 +17,9 @@ 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" # noqa: S105 # a Secret Manager resource path, not a secret + "projects/test-project/secrets/v2-runtime-database-url/versions/latest" ) -TEST_AUTH0_SECRET_RESOURCE = "projects/p/secrets/auth0-management/versions/latest" # noqa: S105 # a Secret Manager resource path, not a secret +TEST_AUTH0_SECRET_RESOURCE = "projects/p/secrets/auth0-management/versions/latest" TEST_OTEL_ENDPOINT = "https://collector.example.test" CLOUD_RUN_SERVICE_SCRIPTS = ( "scripts/deploy_cloud_run_candidate.sh", @@ -27,7 +27,7 @@ "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 +PRODUCTION_DB_PASSWORD_SECRET = "policyengine-api-prod-db-password:latest" DEDICATED_CLOUD_RUN_RUNTIME_SERVICE_ACCOUNT = ( "policyengine-api-cr-runtime@policyengine-api.iam.gserviceaccount.com" ) @@ -142,8 +142,8 @@ def _run_bash( *arguments: str, env: dict[str, str] | None = None, ) -> subprocess.CompletedProcess[str]: - 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 + return subprocess.run( + ["bash", *arguments], cwd=REPO, env=env, text=True, @@ -2014,7 +2014,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 }}" # noqa: S105 # a workflow line that references a secret by name + secret_env = "SIMULATION_ENTRYPOINT_URL: ${{ secrets.SIMULATION_ENTRYPOINT_URL }}" pr_workflow = _pr_workflow() push_workflow = _push_workflow() @@ -2132,11 +2132,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: " # noqa: S105 # a workflow line that references a secret by name + "CLOUD_RUN_RUNTIME_SERVICE_ACCOUNT: " "${{ secrets.GCP_CLOUD_RUN_RUNTIME_SERVICE_ACCOUNT }}" ) deploy_account_secret = ( - "CLOUD_RUN_RUNTIME_SERVICE_ACCOUNT: ${{ secrets.GCP_DEPLOY_SERVICE_ACCOUNT }}" # noqa: S105 # a workflow line that references a secret by name + "CLOUD_RUN_RUNTIME_SERVICE_ACCOUNT: ${{ secrets.GCP_DEPLOY_SERVICE_ACCOUNT }}" ) assert ( @@ -2239,7 +2239,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 }}" # noqa: S105 # a workflow line that references a secret by name + "POLICYENGINE_DB_PASSWORD: ${{ secrets.POLICYENGINE_DB_PASSWORD }}" ) assert database_secret_env not in staging_sync_step assert production_sync_step.count(database_secret_env) == 1 @@ -2372,9 +2372,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", # noqa: S106 # a fake secret value for the sync script + POLICYENGINE_GITHUB_MICRODATA_AUTH_TOKEN="microdata-value", OPENAI_API_KEY="openai-value", - PE_UK_PRIVATE_HF_READ_TOKEN="hf-value", # noqa: S106 # a fake secret value for the sync script + PE_UK_PRIVATE_HF_READ_TOKEN="hf-value", ) result = _run_script( @@ -2449,7 +2449,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( # noqa: S603 # runs a fixed in-repository check with the current interpreter + result = subprocess.run( [sys.executable, "-c", script], cwd=REPO, text=True, diff --git a/tests/unit/test_constants.py b/tests/unit/test_constants.py index 017ff8382..33a4040c8 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( # noqa: S603 # runs fixed test code with the current interpreter + result = subprocess.run( [sys.executable, "-c", code], text=True, capture_output=True, diff --git a/tests/unit/test_country.py b/tests/unit/test_country.py index fc4be035d..ea2c753fe 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)], # noqa: DTZ001 # checks the offset-free isoformat of a naive datetime + "nested": [datetime(2026, 5, 12, 8, 30, 45)], } ) diff --git a/tests/unit/test_deploy_pipeline_scripts.py b/tests/unit/test_deploy_pipeline_scripts.py index 191da57a0..d29b3807f 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( # 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 + return subprocess.run( + ["bash", str(SCRIPTS / script), *args], 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 a3dc1173e..78ba3aae6 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( # 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 + result = subprocess.run( + ["bash", str(SCRIPT), *args], 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 573e50ba2..06d7669d1 100644 --- a/tests/unit/test_observability_collector_verifier.py +++ b/tests/unit/test_observability_collector_verifier.py @@ -11,7 +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 +ACCESS_TOKEN = "access-token" def _load_verifier(): diff --git a/tests/unit/test_simulation_and_report_routes.py b/tests/unit/test_simulation_and_report_routes.py index 38e1a70fb..559b260d8 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) # 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 + 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) result = create_test_client().get(f"/us/report/{report_id}").get_json()["result"] diff --git a/tests/unit/test_workflow_guard_scripts.py b/tests/unit/test_workflow_guard_scripts.py index da028d3eb..0763e7f15 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( # noqa: S603 # runs this repository's script with test-chosen arguments - ["bash", script, *args], # noqa: S607 # bash comes from the test environment's PATH + return subprocess.run( + ["bash", script, *args], 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 26ae0eb7e..0b75db3ad 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( # noqa: S603 # runs fixed test code with the current interpreter + result = subprocess.run( 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 b3f10f2af..b29fd2eaf 100644 --- a/tests/unit/v2/test_alembic_v2.py +++ b/tests/unit/v2/test_alembic_v2.py @@ -185,7 +185,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" # noqa: S105 # a sentinel that must not be echoed in the error + secret = "do-not-echo-this-password" 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 e2be6b91c..056446085 100644 --- a/tests/unit/v2/test_import_side_effects.py +++ b/tests/unit/v2/test_import_side_effects.py @@ -76,7 +76,7 @@ def reject_ddl(*args, **kwargs): assert "policyengine_api.data.v2.user_association_migration_qualification" not in sys.modules """ - result = subprocess.run( # noqa: S603 # runs fixed test code with the current interpreter + result = subprocess.run( [sys.executable, "-c", script], cwd=tmp_path, env=_environment_without_v2(), @@ -125,7 +125,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( # noqa: S603 # runs fixed test code with the current interpreter + result = subprocess.run( [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 093ee3bda..bb99da23d 100644 --- a/tests/unit/v2/test_metadata_deployment.py +++ b/tests/unit/v2/test_metadata_deployment.py @@ -127,7 +127,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"], # noqa: S607 # bash comes from the test environment's PATH + ["bash", "-n", ".github/scripts/migrate_v2_metadata_schema.sh"], cwd=REPO, capture_output=True, text=True, diff --git a/tests/unit/v2/test_settings.py b/tests/unit/v2/test_settings.py index 85712b4d2..38085e680 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" # noqa: S105 # a Secret Manager resource path, not a secret + "projects/test-project/secrets/v2-runtime-database-url/versions/latest" ) @@ -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" # noqa: S105 # a fake URL whose password must not be echoed + secret_url = "postgresql+psycopg://user:do-not-echo@localhost/postgres" with pytest.raises(V2ConfigurationError) as raised: load_v2_runtime_database_settings( diff --git a/tests/unit/v2/test_user_route_auth.py b/tests/unit/v2/test_user_route_auth.py index 50f390a00..108545fa3 100644 --- a/tests/unit/v2/test_user_route_auth.py +++ b/tests/unit/v2/test_user_route_auth.py @@ -269,7 +269,7 @@ def emit(self, record: logging.LogRecord) -> None: return response, handler.records -SECRET_SUBJECT = "google-oauth2|secret-subject" # noqa: S105 # a sentinel that must not leak into the logs +SECRET_SUBJECT = "google-oauth2|secret-subject" def test_unexpected_failures_are_logged_without_their_message() -> None: