Skip to content

fix: add input validation to critical API routes to prevent 500 on mi…#79

Open
pigeio wants to merge 2 commits intoEAPD-DRB:mainfrom
pigeio:feature/add-route-input-validation
Open

fix: add input validation to critical API routes to prevent 500 on mi…#79
pigeio wants to merge 2 commits intoEAPD-DRB:mainfrom
pigeio:feature/add-route-input-validation

Conversation

@pigeio
Copy link
Contributor

@pigeio pigeio commented Feb 27, 2026

…ssing fields

Summary

  • What changed: Added a [validate_json_fields()] helper to [CaseRoute.py] and [DataFileRoute.py]. Applied it to the 10 most critical POST routes (/deleteCase, /copyCase, /saveCase, /saveParamFile, /saveScOrder, /run, /batchRun, /deleteCaseRun, /createCaseRun, /updateCaseRun). Missing or malformed JSON now returns 400 Bad Request with a descriptive error instead of crashing with an unhandled 500 KeyError.

  • Why: All POST routes accessed request.json['key'] directly. If any field was missing, the server crashed with KeyError — the except blocks only caught IOError/OSError, never KeyError.

Related issues

Validation

  • Tests added/updated (or not applicable)
  • Validation steps documented
  • Evidence attached (logs/screenshots/output as relevant)

Both files pass py_compile syntax check. 13/13 unit tests passed covering:

  • No JSON body → 400
  • Empty JSON body → 400 with missing field name
  • Partial fields → 400 naming the first missing field
  • All fields present → passes through to existing logic unchanged

Documentation

  • Docs updated in this PR (or not applicable)
  • No doc changes needed — this is a bug fix with no new API behavior

Scope check

  • No unrelated refactors
  • Implemented from a feature branch
  • Change is deliverable without upstream OSeMOSYS/MUIO dependency
  • Base repo/branch is EAPD-DRB/MUIOGO:main (not upstream)
  • Base repo/branch is EAPD-DRB/MUIOGO:mac-port

@NamanmeetSingh
Copy link

@pigeio Great catch on fixing these 500 errors! UX-wise, this is a much-needed improvement.

Reviewing the diff, I noticed validate_json_fields is duplicated across multiple route files. To keep the codebase DRY, it would be best to extract this helper into a shared file like API/utils.py before merging.

As a longer-term architectural note: for the Track 1 OG-CLEWS integration (PR #24), we are using Pydantic (API/schemas.py) to solve this exact problem. Pydantic handles both missing keys and type-checking automatically, eliminating the need for manual if/else boilerplate in the routes.

For this PR, extracting to utils.py is the quickest win, but moving these CRUD routes to Pydantic in the future will make the API layer bulletproof!

@pigeio pigeio force-pushed the feature/add-route-input-validation branch from ade6430 to 3e15bbf Compare February 27, 2026 18:16
@pigeio
Copy link
Contributor Author

pigeio commented Feb 27, 2026

@NamanmeetSingh Good point — extracted to a shared API/utils.py to keep it DRY.
The Pydantic approach in PR #24 sounds great for the long term; this fix covers the
immediate crash while the schema layer matures. Updated!

@SeaCelo SeaCelo changed the base branch from mac-port to main March 1, 2026 19:29
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.

2 participants