Skip to content

Maintainability: production safety gate, API response models, security rationale, PR template#12

Open
amar-python wants to merge 1 commit into
feature/schema-validationfrom
fix/maintainability-improvements
Open

Maintainability: production safety gate, API response models, security rationale, PR template#12
amar-python wants to merge 1 commit into
feature/schema-validationfrom
fix/maintainability-improvements

Conversation

@amar-python

Copy link
Copy Markdown
Owner

Summary

Third of three code-quality PRs (see #10 CI stability, #11 data integrity). This one focuses on maintainability and operational safety.

Why this targets feature/schema-validation: the restructured layout (backend/, frontend/, docs/) only exists on the unmerged #7 branch. Targeting main would drag all 251 files of #7 into this diff. Merge #7 first, then these PRs.

7. Production safety gate

  • backend/config.py: new ALLOW_SCHEMA_AUTO_CREATE flag (default true for dev) with a pydantic model_validator that fails startup when APP_ENV=production and the flag is enabled — production schema changes must go through explicit migrations, never create_all. Gate is case/whitespace-insensitive.
  • backend/main.py: Base.metadata.create_all on startup is now gated behind the flag.
  • Tests: backend/tests/test_config_safety.py (7 tests) — production+enabled raises, production+disabled OK, dev/staging unaffected, env-var coercion honoured.

8. Pydantic response_model on all remaining endpoints

  • / → new RootResponse; /api/health → new HealthResponse
  • /api/dashboard → existing DashboardStats
  • /api/migrations/{id}/executeMigrationExecuteResponse; /{id}/evaluateEvaluationResponse
  • /api/reports/{id}/generateReportResponse — this also stops leaking the server filesystem path to clients (frontend only consumes download_url; verified via smoke test).
  • Removed an unused execution_service import in the reports route (flake8 F401).

9. docs/security/Rationale.md

Documents every # nosec / # noqa suppression with the rule suppressed, technical justification, and accepted residual risk:

  • backend/database/connection.py:54 (noqa: BLE001 — health check must never raise)
  • backend/migration/evals/runner.py:309 (nosec B608 — table name from hardcoded _DEV_SEED_TABLES allowlist)
  • backend/migration/evals/runner.py:642 (noqa: BLE001 — eval loop isolation)

Includes a mandatory template for future suppressions.

10. .github/pull_request_template.md

Checklist-driven PR template including Tests Added and Verified Workflow Paths sections, plus safety/security and API/data checks. Note: it references tools/verify_workflow_paths.py (added in #10) and the RunStatus lifecycle (added in #11) — merge those first for the links to resolve.

Verification

  • pytest backend/tests/45 passed (38 baseline + 7 new)
  • flake8 clean on all touched files (max-line-length=120)
  • Manual smoke via TestClient: /, /api/health, /api/dashboard, /api/reports/{id}/generate all 200 with the declared shapes; report response contains no path key.

Checklist

  • Tests Added — test_config_safety.py; full suite green
  • Verified Workflow Paths — no workflow-referenced files moved in this PR
  • New suppressions: none; existing ones now documented in docs/security/Rationale.md

⚠️ Please review before merging — do not auto-merge.

…ity rationale, PR template

- config.py: add ALLOW_SCHEMA_AUTO_CREATE flag with a fail-fast validator
  that refuses to start when APP_ENV=production and auto-create is enabled;
  main.py gates Base.metadata.create_all behind the flag
- Add Pydantic response_model to all remaining endpoints: /, /api/health,
  /api/dashboard, /api/migrations/{id}/execute, /api/migrations/{id}/evaluate,
  /api/reports/{id}/generate (also stops leaking the internal report path)
- New schemas: RootResponse, HealthResponse; remove unused import in reports route
- docs/security/Rationale.md: document all nosec/noqa suppressions with
  technical justification + template for future entries
- .github/pull_request_template.md: checklist incl. Tests Added and
  Verified Workflow Paths
- tests: backend/tests/test_config_safety.py (7 tests) covering the
  production safety gate
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant