Skip to content

Post to Bar over http.client to keep Access header names intact - #63

Merged
gowtham0992 merged 1 commit into
mainfrom
bar-header-transport
Aug 15, 2026
Merged

Post to Bar over http.client to keep Access header names intact#63
gowtham0992 merged 1 commit into
mainfrom
bar-header-transport

Conversation

@gowtham0992

@gowtham0992 gowtham0992 commented Aug 15, 2026

Copy link
Copy Markdown
Owner

Replaces the Bar POST transport so Cloudflare Access sees the header
names it requires.

Root cause

urllib.request title-cases header names in
AbstractHTTPHandler.do_open, so CF-Access-Client-Id reached the edge
as Cf-Access-Client-Id and Access answered 403. Captured on the wire:

old (urllib)      Cf-Access-Client-Id: ID
new (http.client) CF-Access-Client-Id: ID

http.client.putheader writes names verbatim, so the spelling survives.

The request Bar now receives

POST /api/v1/github/investigations HTTP/1.1
Host: bar-private.gowthamsarveswaran.com
Accept-Encoding: identity
Content-Type: application/json
Content-Length: <n>
User-Agent: link-bar-action/v1
Idempotency-Key: <sha256>
CF-Access-Client-Id: <id>
CF-Access-Client-Secret: <secret>

Transport properties

  • Origin pinning. The endpoint must equal the allowlisted constant,
    and is independently re-parsed to require HTTPS, the exact hostname,
    port 443, and no userinfo or fragment. A non-HTTPS endpoint is
    rejected before the connection factory is called.
  • TLS unchanged. The default HTTPSConnection context verifies
    certificates and hostnames with a TLS 1.2 floor.
  • No redirects. http.client does not follow them, so a 3xx raises
    immediately. A test stands up a second server as a credential sink and
    asserts it receives nothing.
  • Bounded reads. The response is read to 64 KiB + 1 and the size is
    checked before the status is interpreted.
  • Connection cleanup. close() runs in a finally. After three
    failed attempts, no sockets remain open.
  • Idempotency. The body is serialized once outside the retry loop, so
    every attempt sends identical bytes under the same Idempotency-Key.
  • Retries. 408, 429, 500, 502, 503, and 504 retry; everything else
    fails on the first attempt, so a 403 no longer burns three requests.
    Retry-After is matched case-insensitively and capped at 60 seconds.

Also in this change

  • User-Agent. http.client sends none, and an unnamed agent is a
    plausible trigger for edge bot rules. The collector now names itself
    here exactly as it already does to GitHub.
  • TLS failures are permanent. ssl.SSLCertVerificationError
    subclasses OSError, so it was being retried three times and reported
    as a generic network error, which would bury an intercepted
    certificate. It now fails immediately with a distinct message. A
    companion test proves ordinary socket errors still retry.
  • Release-hygiene allowlist for the transport tests. They bind only
    loopback servers to assert wire-level header spelling, and tests/ is
    not in the wheel, which packages only link_mcp and link_core.

Verification

Collector tests 37 passed, full suite 1047 passed, release hygiene
passed, hygiene tests 11 passed, ruff clean, actionlint clean.

Header spelling, cleanup, idempotent retry bodies, and Retry-After
casing were each confirmed against real loopback socket servers reading
raw request bytes, not mocks.

urllib.request title-cases header names in AbstractHTTPHandler.do_open,
so CF-Access-Client-Id went out as Cf-Access-Client-Id and Cloudflare
Access answered 403. http.client's putheader writes names verbatim.

The transport pins the HTTPS origin before connecting, never follows a
redirect, reads a bounded response, and closes the connection in a
finally block. Retries reuse the same body and Idempotency-Key, and
Retry-After is matched case-insensitively and capped.

Also names the collector in a User-Agent, since http.client sends none
and an unnamed agent invites edge bot rules to answer instead of Bar,
and treats ssl.SSLCertVerificationError as a permanent trust failure
rather than retrying it as a transient network error.

Allowlists the transport tests in the release-hygiene network check.
They bind only loopback servers to assert wire-level header spelling,
and tests/ is not in the wheel.
@gowtham0992
gowtham0992 merged commit 5a7181b into main Aug 15, 2026
10 checks passed
@gowtham0992
gowtham0992 deleted the bar-header-transport branch August 15, 2026 21:00
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