fix(github): fail closed on every non-success response and malformed payload

[LuanTrindade95]

Closes remediation item C2 (audit finding F-006).

Problem

HttpGitHubClient::request() only mapped 403 to rate limit and 404 to repository not found. Every other failure — 401, 409, 422, 429, 5xx, connection errors — came back as an ordinary Response, and markdownFiles() consumed it with json(''tree'', []). A server error, a non-JSON body or a body without tree became an empty list, so ingestion finished success with zero documents, indistinguishable from a repository that legitimately has no docs. Invalid base64 was persisted as an empty document, and a truncated: true tree was treated as complete.

A second fail-open path, not described in F-006, is closed here too: IngestionService::run() only caught the two existing exceptions, so any other one escaped the method and left the IngestionRun stuck in running, with no finished_at and no log.

Change

App\Exceptions\GitHubClientException is the abstract base for every GitHub failure and exposes failureCode(). The client now fails closed on every non-success status and validates the payload; run() catches the whole hierarchy and ends the run through fail().

Category	Code	Covers
Rate limit	git_hub_rate_limit_exception	403 with rate-limit header, 429
Auth / permission	github_authentication_failed	401, 403 without that header
Repo or ref missing	git_hub_repository_not_found_exception	404
Conflict / validation	github_validation_failed	409, 422
Transient	github_transient_error	any status >= 500, connection, timeout
Malformed payload	github_malformed_response	non-JSON, no tree, invalid base64, unsupported encoding
Truncated tree	github_tree_truncated	truncated: true

A 403 is classified by the rate-limit header rather than the status alone, so a permission denial is no longer misreported as rate limiting. Transient matches a range, not a list, so unseen 5xx stay covered.

Compatibility

App\Contracts\GitHubClient, FixtureGitHubClient, the IngestionService::fail() signature and the IngestionRun.log entry shape are all unchanged — downstream consumers read the taxonomy through the existing code key with no migration. The two legacy codes keep their historical shape because a test asserts one literally. The 304/ETag path of ADR-11 returns before any error check and is covered by a regression test. No retry or backoff: that remains Priority 8.

Validation

• Pest 50 passed (233 assertions), Pint 122 files, run against a rebuilt image.
• Independent adversarial audit: APPROVED. All 16 scenarios rebuilt with an own Http::fake harness driving IngestionService::run() end to end, reading status and log from the database. No error scenario finished success; no two distinct categories shared a code.
• 502, missing from the implementation report, is covered — confirmed empirically.
• Token leak check with a fictitious token across IngestionRun.log, storage/logs, exception messages and the diff: no occurrence of the token, Bearer or Authorization.
• Test count 33 to 50, no test removed, no skip, no weakened assertion.

Worth knowing for future work: the backend service has no bind mount, so a gate run straight after an edit can execute stale image code. Rebuild before trusting it.

Taxonomy and scope trade-offs recorded in ADR-27; the general rule in BRAIN-007.

🤖 Generated with Claude Code