Skip to content

fix: address red flags from code review#18

Closed
mortenoh wants to merge 10 commits intomainfrom
fix/code-review-red-flags
Closed

fix: address red flags from code review#18
mortenoh wants to merge 10 commits intomainfrom
fix/code-review-red-flags

Conversation

@mortenoh
Copy link
Member

@mortenoh mortenoh commented Feb 26, 2026

Summary

Addresses red flags found during code review of src/eo_api. Security issues, reliability problems, and correctness fixes.

Security hardening

  • SSRF + path traversal (zonal_statistics.py): replaced urlopen() with httpx.get() (30s timeout), removed local file path support entirely
  • CORS (main.py): dropped allow_credentials=True (not needed for public data API)

Reliability

  • Exception chain (chirps3_dhis2_pipeline.py): changed from None to from e to preserve tracebacks
  • YAML error handling (registry.py): narrowed bare except Exception to (yaml.YAMLError, OSError)

Correctness + cleanup

  • Hardcoded URL (tasks.py): OGCAPI_BASE_URL now read from env var with localhost fallback
  • assert -> ValueError (serialize.py): replaced asserts (stripped with -O) with proper exceptions
  • Registry caching (registry.py): added @lru_cache to list_datasets() to avoid re-parsing YAML on every request
  • Temp file leak (serialize.py): clean up temp file if to_netcdf() fails

- Replace urllib urlopen() with httpx.get() with a 30s timeout
- Remove local file path support entirely (was an unrestricted read)
- Reject any string input that is not an HTTP(S) URL
allow_origins=["*"] + allow_credentials=True is a dangerous CORS
combination. Credentials are not needed for this public data API.
Change `from None` to `from e` so the original traceback is retained
when ProcessorExecuteError is raised.
Read OGCAPI_BASE_URL from env so Prefect workers outside the API
container can reach the correct endpoint.
Asserts are stripped with python -O. These invariants guard real logic
so they need to be proper exceptions.
YAML files don't change at runtime so there's no need to re-read and
re-parse them on every request.
Catch (yaml.YAMLError, OSError) instead of bare Exception so
unexpected errors like TypeError from malformed data surface
immediately instead of silently dropping datasets.
If to_netcdf() raises, remove the orphaned temp file before
re-raising the exception.
@mortenoh mortenoh closed this Feb 26, 2026
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