Skip to content

Drop inbound edge headers to avoid Cloudflare Error 1000 - #98

Merged
jerryfane merged 1 commit into
mainfrom
fix/strip-edge-headers
Oct 26, 2026
Merged

jerryfane merged 1 commit into
mainfrom
fix/strip-edge-headers

Conversation

@jerryfane

Copy link
Copy Markdown
Contributor

Assignment

Closes #97. Phobos request 361717 (owner's ImgStudio blocked), accepted 361719. Phobos owns deployment and live GET /api/health acceptance under 361721. Implementation by keyring/OpenAI in-session; no review agent implemented changes.

Fix

Strip exactly the 12 inbound Cloudflare/forwarding headers requested in #97 in outboundHeader. No CF-prefix filter, Worker mutation, service policy, credentials, grants, retries or timeouts changed. CF-Access-Client-Id/Secret are still injected normally; other application headers remain intact. README documents the fixed boundary.

Verification

  • Before fix, TestProxyDropsEdgeHeaders reproduced HTTP403 error code: 1000 through the actual proxy handler and an HTTP upstream.

  • Actual pre-fix binary (PR95) with loopback fixture returned403 and forwarded all12 edge headers. Actual rebuilt fixed binary returned204 and forwarded none. In both, the upstream received the stored CF Access pair, not forged caller values, and retained unrelated CF-Custom-Service and X-Request-Id headers.

  • Durable smoke instrument: /root/fleet-tools/state/keyring-seat/edge-headers-97/smoke.py. Fixture state and processes cleaned up; no production request or real credential access.

  • test-check prove: proven, TestProxyDropsEdgeHeaders red on old code and green on new. Receipt /root/.local/state/test-check/receipts/20261026T143725Z-34d976fb272d322ea21cfb53cdf7f80c/receipt.json.

Rollout

Phobos deploys Worker plus rebuilt container from the merge SHA, then calls the owner's ImgStudio GET /api/health through the keyring. Production acceptance remains unverified here. Keep existing OAuth bindings, templates, service credentials and grants unchanged.

@jerryfane

Copy link
Copy Markdown
Contributor Author

Local full Go suite passed. First CI run failed in unchanged TestWorkspaceAccountQuotasAndSelectorsFailClosed during fixture creation: admin/proxy address collision, before any proxy request. Source freeAddr closes its temporary listener before separately allocating the next address. Tracked in #99 with exact failed run; requested one failed-job retry, with no code or production-policy changes. CI server package including the new header regression passed on the first run. Merge remains gated on green CI and independent exact-head review.

@jerryfane

Copy link
Copy Markdown
Contributor Author

Agent: adspower-fleet-review
Runtime: omp
Job: local-review-review-router-18e21b342771c407-1

Decision: approved
Summary: I'm approving PR #98 at head d370ed5. I read the full diff against base f2959f3: 3 files, +60/-2 (README.md, internal/server/server.go, internal/server/server_test.go).

The change only touches outboundHeader (server.go:824-833). It removes exactly the 12 edge headers named in issue #97: CF-Connecting-IP, CF-Connecting-IPv6, CF-IPCountry, CF-Ray, CF-Visitor, CF-EW-Via, CF-Pseudo-IPv4, CDN-Loop, True-Client-IP, X-Forwarded-For, X-Forwarded-Proto and X-Real-IP. It uses Header.Del, which matches names case-insensitively and removes every value. The strip runs before the stored credentials are added: the primary key in buildOutbound (server.go:553) and the extra headers via header.Set (server.go:548-552).

Security and regression checks:

  • No new input is trusted and nothing is added to what the proxy forwards.
  • Caller-supplied CF-Access-Client-* values are still replaced by the stored values.
  • If an admin names a credential header after one of the 12 edge headers (X-Real-IP as the primary, CF-Ray as an extra; policy allows this), the stored value is still sent.
  • Headers not on the list pass through unchanged: CF-Access-Jwt-Assertion, CF-Worker, X-Forwarded-Host, Forwarded and X-Request-Id. That keeps the change to the requested scope.
  • outboundHeader has one caller (server.go:517). internal/relay/relay.go:215 also copies caller headers, but it forwards to the keyring, which now applies the strip, so it is not an exit path.
  • The README line matches the code.

Observation, not a finding, since widening the list was explicitly out of scope: RFC 7239 Forwarded: for=... and X-Forwarded-Host still reach the upstream, so client-identity headers are not all removed. Issue #97's Error 1000 comes from CF-Connecting-IP, which is now stripped.

Prior verdicts: GITMOOT_PRIOR_VERDICTS (as_of 2026-10-26T14:44:20Z) has 0 entries for pull_request 98, so there are no earlier findings to answer.

I made no edits; the throwaway test trees in /tmp we
[truncated]

Tests Run

  • CGO_ENABLED=0 go build ./... -> BUILD_OK. A cgo build first failed because the sandbox denies /usr/include/stdc-predef.h, so every Go command was run with CGO_ENABLED=0.
  • CGO_ENABLED=0 go vet ./... -> VET_OK
  • CGO_ENABLED=0 go test -count=1 -v -run 'TestProxyDropsEdgeHeaders|TestExtraHeaderProxy|TestProxyAddsKeyAndDropsCallerCredentials' ./internal/server -> PASS (all 3)
  • Red/green: base f2959f3 extracted with git archive into /tmp, with server_test.go replaced by the head version, then CGO_ENABLED=0 go test -count=1 -run TestProxyDropsEdgeHeaders ./internal/server -> FAIL server_test.go:231 'header-auth upstream rejected proxied request: 403 error code: 1000'. The same test at head -> PASS.
  • CGO_ENABLED=0 go test -count=1 ./... at head -> all packages ok (cmd/keyring, admin, consent, dashboard, googlelogin, instance, policy, relay, requests, server 74.4s, store)
  • Throwaway wire probe TestProbeEdgeWire (/tmp copies only, deleted afterwards): a real TCP HTTP/1.1 request through the Handler sent all 12 edge headers in lowercase and uppercase, with repeated CF-Connecting-IP, CDN-Loop and X-Forwarded-For lines, plus forged CF-Access-Client-Id/Secret. At base: all 12 reached the upstream (e.g. Cf-Connecting-Ip=[1.1.1.1 2.2.2.2]). At head: none of the 12 reached the upstream; Cf-Access-Client-Id=stored-id and Cf-Access-Client-Secret=[REDACTED] arrived instead of the forged values; CF-Access-Jwt-Assertion, CF-Worker, X-Forwarded-Host and Forwarded were kept. A second service config with primary header X-Real-IP and extra header CF-Ray -> at head the upstream got X-Real-Ip=stored-id and Cf-Ray=stored-secret, with no caller edge values.

Raw runtime output was retained in local Gitmoot state and is not posted here.

@jerryfane

Copy link
Copy Markdown
Contributor Author

Exact-head merge evidence

Head d370ed5cd3d7919c3177957d75a84f80cd147cc0, level3_required.

Independent Claude Opus 5.5 reviewer adspower-fleet-review, job local-review-review-router-18e21b342771c407-1: approved, no findings, no delegations. Implementer was keyring/OpenAI. Full result saved at /root/fleet-tools/state/keyring-seat/edge-headers-97/review.json.

Reviewer executed build, vet, uncached full Go suite and three focused proxy tests. Independently proved the new regression fails403/Error1000 on base f2959f3 and passes at head. Its raw TCP probe covered uppercase/lowercase and repeated edge header lines: all12 removed at head; stored CF Access pair replaced forged values; unrelated headers retained. Gitmoot labels the result static_only despite the substantive executed checks in tests_run; that metadata is disclosed, not represented as executed mode.

Implementing seat also ran full Go tests and actual pre-fix/fixed CLI binaries: old403 with all12 edge headers, fixed204 with none, correct stored pair and unrelated CF application header retained. test-check prove: proven red/green, receipt /root/.local/state/test-check/receipts/20261026T143725Z-34d976fb272d322ea21cfb53cdf7f80c/receipt.json.

CI is green at this same head after one diagnosed unrelated fixture port-collision retry (tracked #99; initial failure and retry disclosed in #98 (comment)). Format/vet/build/race tests and native macOS pipe-import checks passed. No head change or production exception was used to bypass the failed fixture.

Phobos owns the new-container rollout and live ImgStudio GET /api/health acceptance. Production behavior is not yet claimed verified.

@jerryfane
jerryfane merged commit 31ac165 into main Oct 26, 2026
4 of 5 checks passed
@jerryfane

Copy link
Copy Markdown
Contributor Author

Deployed by phobos-dev from merge31ac165c7609d5bb4c1c2a52bf72a5164116a0ec. Worker dc7c2711-f0ce-4026-af56-054e7ed2dcbb, container v16 image sha256:8db5736ada589dfbbbe577d3b39bfcd13922c4171ae9be340266d2475fffe44c; operator reports7/7 healthy. Saved deployment log confirms Worker/image identities; before/after binding receipts are byte-identical. Live acceptance reported by phobos-dev: owner ImgStudio through keyring GET /api/me200 and GET /api/health200, previously403 Error1000. Message361763 and saved /root/fleet-tools/state/phobos-dev/own-services/keyring-deploy-97/RECEIPT.md; consolidated keyring evidence /root/fleet-tools/state/keyring-seat/edge-headers-97/verification.json. No duplicate production probe by keyring; rollback procedure recorded, not exercised.

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.

Proxy forwards CF-Connecting-IP upstream: Cloudflare-hosted services fail with Error 1000

1 participant