Skip to content

fix(updates): reject GitHub HTTP error payloads - #3167

Open
tang-vu wants to merge 1 commit into
Osmantic:mainfrom
tang-vu:fix/release-api-http-status
Open

fix(updates): reject GitHub HTTP error payloads#3167
tang-vu wants to merge 1 commit into
Osmantic:mainfrom
tang-vu:fix/release-api-http-status

Conversation

@tang-vu

@tang-vu tang-vu commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Why this matters

All three dashboard consumers of the GitHub Releases API parse JSON without first checking the HTTP status. A rate-limit response (403) is therefore treated as a valid latest-release payload and replaces useful stale cache data with an empty version for five minutes. The update dry-run reports no update and no error. The manifest path also accepts any list-shaped JSON body from a failing proxy as real releases.

This calls raise_for_status() before parsing the latest-release cache, release manifest, and dry-run responses. Existing narrow httpx.HTTPError boundaries then preserve stale cache data or return the documented local-version fallback/error field instead of manufacturing a successful no-update result.

Behavioral invariant: only a successful GitHub HTTP response may update release metadata; non-2xx responses must preserve last-known-good state or be surfaced through the endpoint fallback contract.

Overlap check

Searched open and closed PRs for GitHub release API raise_for_status, release rate limit dashboard update, and the changed production file ods/extensions/services/dashboard-api/routers/updates.py. No PR covers HTTP-status validation at these call sites.

Existing tests covered connection exceptions and non-list manifest bodies but not valid JSON delivered with a non-success status. This PR extends that existing router suite rather than adding another release client.

Regression test

tests/test_updates.py adds three concrete contracts:

  • a 403 rate-limit response cannot overwrite an expired but useful release cache;
  • /api/update/dry-run reports the 403 instead of silently returning a clean no-update result; and
  • /api/releases/manifest rejects a list-shaped JSON body delivered with HTTP 503 and falls back to the installed version.

Before the fix, the cache test was replaced by an empty payload and the dry-run error field was null; the 503 list was published as version 999.0.0.

Validation

  • python -m pytest tests/test_updates.py -q ? 32 passed
  • python -m compileall -q routers/updates.py tests/test_updates.py ? passed
  • make lint ? passed
  • git diff --check ? passed

The suite uses real httpx.Response status behavior behind deterministic mocked transport and exercises the public FastAPI endpoints for dry-run and manifest. No live GitHub rate limit or outage was induced. Rollback only restores the permissive parser and requires no persisted-data migration.

Batch compatibility

This PR was validated on synthetic integration head 9af795fc, which applies #3158 through #3167 in numeric order on upstream/main (6ff9b4fc). Combined make lint, make test, make smoke, make simulate, and all 418 BATS cases passed (one root-specific permission assertion skipped by design).

Recommended merge order: #3158 ? #3159 ? #3160 ? #3161 ? #3162 ? #3163 ? #3164 ? #3165 ? #3166 ? #3167. The only manual reconciliation observed was the adjacent Makefile test insertion shared by #3164 and #3166; retain both test-unix-restart-recreate-env.sh and test-chat-error-exit-parity.sh lines. Production code merged automatically across the full batch.

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