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

[anth-volk]

Fixes #784

Same-Repository Draft PR Check

• This PR is a draft.
• This PR is from a branch in PolicyEngine/policyengine-sim-api, not a personal fork.

Summary

Phase 1 step 1.8, group 6: security, logging and debugging leftovers. Bottom of the stack since #783 merged.

• Rules: ruff.toml gains S, T20, LOG, G, DTZ, ERA, TD, FIX and EXE. Every finding is fixed (one commit per family), covered by a file-level exception in tests, CI/CD scripts or command-line scripts, or kept inline with a reason in application code. ERA, TD, FIX and EXE had no findings.
• File-level exceptions (new policy): a ruff per-file-ignores entry is now allowed only for tests (tests or authenticated_tests directories), CI/CD scripts (.github) and command-line scripts (a scripts directory, or a module run through if __name__ == "__main__":).

  • Enforcement: the rules guard already required a registered rule for every entry. Now it also expands each entry's pattern the way ruff does and fails when the pattern covers any other file, is negated, or matches nothing. Ruff's * also matches /, and a pattern also matches a bare file name; the check follows both.

  • Where it lives: scripts/rules/file_exceptions.py. docs/engineering/skills/rules-registry.md has a new "File-level exceptions" section.

  • Codes, from the suppressions those files already carried:

    Files	Codes added
    Tests, and the deployed-service tests	S102, S105, S106, S108, S301, S603, S607, DTZ001, DTZ007 (added to their existing exemptions)
    .github/**	T201
    **/scripts/**	DTZ011, S603, S607, T201
    Programs under src: the Modal deployment utilities, and the entry and gateway OpenAPI and Stage 12 infrastructure commands	S310, T201

  • Effect: this removes 75 inline codes from 44 files. Each of the five branches up to Register weighted-microdata rules, weight the builder tests, and check change weights and household counts #807 differs from its previous version in project Python only by noqa comments; a script checked each file line by line.

  • Registry: three new rules: sim.quality.ci-cd-scripts-ruff-exemptions, sim.quality.command-line-scripts-ruff-exemptions and sim.quality.src-programs-ruff-exemptions.

• Behaviour changes (all deliberate):
  • Gateway auth misconfiguration log (G201): it now uses logger.exception("Gateway auth misconfigured"). Before, it logged "Gateway auth misconfigured: Gateway auth misconfigured: set GATEWAY_AUTH_ISSUER …", repeating the message; the traceback still ends with the full RuntimeError message. The 503 response is unchanged.
  • Logger name (LOG002): the executor's simulation.py uses getLogger(__name__) instead of getLogger(__file__).
  • Gateway image smoke: its local entrypoint raises RuntimeError with a message instead of failing a bare assert. The smoke script and the integration test only check the exit code.
  • Six f-string log calls become %s arguments with identical text (G004).
• Asserts outside tests (S101):
  • Contract spm.py (4): the asserts couldn't fail, because _require_complete_selection already raises ValueError when either value is missing. It now returns them, so nothing new is raised and Pydantic validators still see ValueError.
  • Executor stage12_qualification.py: the closure captures a non-optional runtime instead of asserting it.
  • Test double in fixtures/: raises AssertionError explicitly, as the repo's other fakes do.
  • authenticated_tests: this is the deployed-service pytest suite (run by simulation-deploy.reusable.yml), which the **/tests/** glob didn't match. It gets the same exemptions as **/tests/**, registered as sim.quality.deployed-tests-ruff-exemptions.
  • Nothing in the repo runs Python with -O.
• Prints in commands (T201): command-line scripts and Modal local entrypoints keep their output. Scripts, .github and the programs under src are covered by file-level exceptions. The Modal local entrypoints (smoke_app.py, v2_static_runtime_files_smoke.py) and the executor's import-time generate_openapi.py don't run through __main__, so they keep an inline reason. update_version_registry.py (22 prints) and .github/scripts/update_policyengine_package.py route output through one helper. No application code had a debug print.
• Subprocess calls (S603, S607): each test module that runs repository scripts does it through one helper, for example _run_bash in test_modal_scripts.py (32 calls). Tests and scripts are covered by file-level exceptions; the one call in application code keeps an inline reason. Executables still come from the child's PATH, since several tests pass their own env.
• Datetimes (DTZ): every value stays naive, matching .py's ParameterValue dates and its own period parsing. The 11 naive test dates go through one _naive_policy_date helper.
• Other security findings: no vulnerability found.
  • Fixed: the GCP metadata probe builds its Request inline, with identical URL, headers and method.
  • Covered by the test exceptions: fake tokens (now module constants), image tests that exec repository-owned app source, and tests that pickle objects they built.
  • Kept inline with a reason, in application code:
    • Modal secret and display names;
    • the Modal container's fixed /tmp/policyengine-data paths;
    • the Hugging Face Hub URL built from HF_ENDPOINT and quoted path parts.
  • S110: each try/except/pass already carried a reasoned BLE001 suppression from group 5, which now lists S110 too.
• Modal images: code changes in the executor image's simulation.py, simulation_runtime.py, stage12_qualification.py and update_version_registry.py, and in the shared contract, observability and fastapi libraries. src/modal/app.py changes only in comments.
• Registry: sim.quality.ruff-lint now lists the nine families. The rules tool is synced from the Public API's group 6 (Enable ruff security, logging and debugging rules (S, T20, LOG, G, DTZ, ERA, TD, FIX, EXE) policyengine-api#3915), and both copies gain the same file-level exception check (file_exceptions.py is identical in both).

Testing

• Suites: at this branch's tip after today's restack onto main, all seven passed:

  Project	Passed
  executor	1,190 (22 skipped)
  entry	529
  gateway	243
  contract	192
  observability	31
  stage12	17
  fastapi	14

  The file-level exceptions change touches project code only in noqa comments, so the suites weren't re-run for it.

• For the file-level exceptions change, at each tip from Enable ruff security, logging and debugging rules (S, T20, LOG, G, DTZ, ERA, TD, FIX, EXE) #785 to Register weighted-microdata rules, weight the builder tests, and check change weights and household counts #807:

  • ruff check and ruff format --check (0.15.22) are clean repo-wide;
  • the rules guard, the ci-scripts guard and the rules tool unittests pass, including six new tests for the check;
  • rendered rules are fresh at all 132 commits from main to Register weighted-microdata rules, weight the builder tests, and check change weights and household counts #807.

• Negative check: pointing an exception at projects/*/src/** fails the guard and names the application files it would cover.

• Lint at the top of the stack: the new rules-tool code also passes the stack top's select = ["ALL"].

• Integration smoke tests: these need real Modal, so they were only collected.

• CI/CD: no workflow changed, and no environment variable was added or rewired.

• CI: this is now the lowest unmerged PR in the stack, so its full CI runs.

🤖 Generated with Claude Code