[investigate] Baseline tests for #14198 (remote-bindings Access auth) - #14234
[investigate] Baseline tests for #14198 (remote-bindings Access auth)#14234petebacondarwin wants to merge 2 commits into
Conversation
…s auth) Adds three test files that characterise — entirely on `main`, without the PR applied — the existing behaviour of the Cloudflare Access auth path for remote bindings. The goal is to settle whether PR #14198's miniflare-side credential plumbing is necessary, and to surface a real but separate gap (WS/RPC path lacks Access-block detection) that is worth addressing in any case. Test 1 — wrangler RemoteRuntimeController.test.ts Asserts that the proxy session's RemoteRuntimeController, on every reload (including `remote: "minimal"` mode), calls `getAccessHeaders(token.host)` and spreads the result into `proxyData.headers`. Covers both auth modes the helper returns: service-token pair and `Cookie: CF_Authorization=...`. Test 2 — miniflare proxy-websocket-header-forwarding.spec.ts Replicates the exact forward line from ProxyWorker.processQueue in isolation (`fetch(url, new Request(request, { headers }))` with an `Upgrade: websocket` header) and asserts that workerd carries BOTH the proxyData-merged sentinel and the incoming-request sentinel to the fake edge server's WebSocket-upgrade handshake. This is the linchpin: it proves the ProxyWorker→edge hop already authenticates WS binding traffic. Test 3 — miniflare remote-bindings-access-warning.spec.ts Mirrors the existing HTTP-path Access-block warning test but for the WS/RPC path (`makeRemoteProxyStub`'s capnweb session). Asserts that when the proxy returns a 403 Cloudflare Access block on the WS upgrade, the RPC call fails AND no actionable warning is logged — characterising the gap that PR #14011's detection added for the HTTP path but never extended to the WS/RPC path. All three tests pass on `main` without PR #14198 applied.
|
|
This confirms Test 3's premise: I've completed a thorough static review. Let me summarize my findings: The tests are well-constructed and accurately mirror the production code:
No logic bugs, security issues, backward-compat violations, or incorrect API behavior. The only observations are:
These are characteristics of an investigation PR, not actionable defects. The author clearly states this PR is not intended to merge as-is. There are no logic bugs, security issues, or incorrect API behaviors to flag, and I was instructed not to flag formatting or subjective preferences. LGTM |
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
@cloudflare/wrangler-bundler
commit: |
…forward-header instrumentation Adds a credentialed E2E repro for PR #14198 (companion to the baseline tests in the rest of this branch). One `wrangler dev` worker exercises both remote-binding proxy code paths and reports each independently in a single JSON response, so a single request tells us which path works/fails and why: - `ai` → HTTP `makeFetch` (wrapped fetcher — the AI `invalid_token` case in #14198) - `serviceFetch` → HTTP `makeFetch` (service binding `.fetch`) - `rpc` → WebSocket `makeRemoteProxyStub` (capnweb / RPC — the Artifacts case in #14198) The repro lives at `investigation/remote-access-repro/`, outside the pnpm-workspace globs (so CI, lint, type-check, fixture validation, and turbo all skip it). It is **manual, account-dependent**: it requires Workers AI, a deployed target worker, Access on the account's workers.dev subdomain with a Service Auth policy, and Service Token creds. The README documents prereqs, the run protocol, scenario matrix (A: main no creds, B: main + creds [the decisive run], C/D: PR #14198 +/- creds), and the interpretation table. Also adds a THROWAWAY debug log in `packages/wrangler/templates/startDevWorker/ProxyWorker.ts` — grep for `mf-access-debug` — that logs, for every request the local ProxyWorker forwards to the workers.dev edge (HTTP and WebSocket upgrades alike), whether the merged headers carry the Access service-token / cookie / preview token. This is the smoking gun for the central question: do the Access headers reach the edge on the WS-upgrade hop? **Must be reverted before any non-investigation merge** — it is a `console.log` in a template, gated only by being in this debug branch.
|
Just added a minimal manual E2E repro at It's deliberately tiny: one deployed target worker ( {
"ai": { "path": "http/makeFetch (AI)", "ok": …, "result|error": … },
"serviceFetch": { "path": "http/makeFetch (service.fetch)", "ok": …, "result|error": … },
"rpc": { "path": "ws/makeRemoteProxyStub (service RPC)", "ok": …, "result|error": … }
}So one Also commits a throwaway debug log in The decisive run is scenario B (main + creds set) — full protocol + interpretation table in the README. It runs against the locally built wrangler so switching The repro lives outside the pnpm workspace globs (CI, lint, type-check, fixture-validation, turbo all skip it). |
|
Cross-linking from #14198 (closing that one). End-to-end verification confirms your baseline conclusion: on current released versions ( I reproduced it in a minimal vite-plugin app (passes with a service token in |
|
Thanks for folllowing up @krys-cf - I'll close this out. Glad you are unblocked! |
Discussion / investigation PR — companion to #14198. Not intended to merge as-is. The goal is to share three baseline-characterisation tests that run on
main(without #14198 applied), and use their empirical results to discuss whether #14198's miniflare-side credential plumbing is necessary, and what the genuine outstanding gap is.cc @krys-cf @petebacondarwin
TL;DR
mainwithout [miniflare] Authenticate remote bindings with Cloudflare Access service tokens #14198 applied.RemoteRuntimeControlleralways injectsgetAccessHeaders(token.host)intoproxyData.headers, this means the edge hop already authenticates remote-binding traffic — HTTP and WS alike — with the existing service-token env vars.If these results hold up under closer scrutiny / a debug repro from @krys-cf, then #14198's two mechanisms (HTTP creds in
makeFetchand thefetch()-based WS upgrade inmakeRemoteProxyStub) appear to be redundant for the auth they target, and the AIInferenceUpstreamError: invalid_tokenreported in #14198 may be a separate AI-upstream concern rather than a Cloudflare Access failure.Why this matters / topology recap
remoteProxyConnectionStringis not the workers.dev URL — it resolves to the local ProxyController Miniflare (http://127.0.0.1:<port>,start-remote-proxy-session.ts:160-161→DevEnv.ts:215-217→ProxyController.ts:188with no host override, defaulting to127.0.0.1). The Access-protected host istoken.host(*.workers.dev), one hop further in.The only Access-gated hop is B→edge.
RemoteRuntimeController.ts:325already callsgetAccessHeaders(token.host)and spreads it intoproxyData.headers(:333-337).ProxyWorker.processQueueunconditionally merges those into every forwarded request (ProxyWorker.ts:146-158), including WS upgrades.The remaining question — and the entire reason I wrote Test 2 — is whether workerd actually carries those merged headers on a WS upgrade, or whether something subtle (e.g.
new Response(res.body, res)atProxyWorker.ts:160dropping thewebSocket) causes the WS path to silently lose them. The workerd C++ source (Response::constructorinsrc/workerd/api/http.c++) preserves bothwebSocketandstatusCodewhen constructed from another Response, but a runtime check is more decisive than reading C++.The three tests + their empirical results
Test 1 —
RemoteRuntimeControllerputs Access headers inproxyDataFile:
packages/wrangler/src/__tests__/api/startDevWorker/RemoteRuntimeController.test.ts(+91 lines)Run:
pnpm test:ci -F wrangler -- RemoteRuntimeControllerResult: ✅ passes on
main. The controller callsgetAccessHeaders(token.host)on every reload (includingremote: "minimal") and spreads the result intoproxyData.headersalongside the preview token. Verified for both auth modes (service-token pair andCookie: CF_Authorization=...fromcloudflared).Test 2 — workerd forwards headers on a WS upgrade (the linchpin)
File:
packages/miniflare/test/plugins/shared/proxy-websocket-header-forwarding.spec.ts(new)Run:
pnpm --filter miniflare exec vitest run test/plugins/shared/proxy-websocket-header-forwarding.spec.tsResult: ✅ passes on
main— "both sentinels" outcome.The test replicates
ProxyWorker.processQueue's exact forward line:…sends an
Upgrade: websocketrequest withx-incoming-sentinel: from-client, and a fake WS server (viauseServer(_, wsListener)) captures the upgradereq.headers. Both sentinels arrive at the edge:invalid_tokenis a separate issue.Test 3 — WS/RPC path has no Access-block detection
File:
packages/miniflare/test/plugins/shared/remote-bindings-access-warning.spec.ts(+87 lines)Run:
pnpm --filter miniflare exec vitest run test/plugins/shared/remote-bindings-access-warning.spec.tsResult: ✅ passes on
main— confirms the gap.A fake proxy server returns a 403 Cloudflare Access block (the same HTML body the existing HTTP-path tests use) for every request, including WS upgrades. A worker script invokes a non-
.fetchRPC method on the service binding (so the proxy stub takes the capnweb / WebSocket path, notmakeFetch). The RPC call fails (response.status === 500, bodyerr:…) and no Access warning is logged — in contrast to the HTTP path which produces a single, actionable warning + readable error body courtesy of #14011'smaybeReportCloudflareAccessBlock. The capnweb path inmakeRemoteProxyStubhas no equivalent.What this implies for #14198
If the empirical results above hold up under @krys-cf's repro:
proxyData.headersalready carriesCF-Access-Client-Id/Secret(orCookie) to the edge. The PR's HTTP-side credential injection inmakeFetchand its WS-sidefetch()-based upgrade inmakeRemoteProxyStubwould, in this reading, be attaching Access headers on the localhost loopback hop where Access is not enforced.InferenceUpstreamError: invalid_tokenreported in [miniflare] Authenticate remote bindings with Cloudflare Access service tokens #14198 may not be a Cloudflare Access failure — it's an AI-upstream / Inference Gateway error code. Worth confirming separately.maybeReportCloudflareAccessBlock(or equivalent) into the capnweb path so RPC bindings surface the same single, actionable warning the HTTP path already does.If, on the other hand, @krys-cf can reproduce the failure with the PR reverted, env creds set, and
WRANGLER_LOG=debugconfirmingUsing Access Service Token headers for domain: <token.host>is logged for the proxy session — then there's a real bug somewhere that my static + isolated tests have missed, and we should hunt for it before plumbing more credentials.Specific things I'd love a debug capture of
If @krys-cf has time, the most useful data point would be: with the PR fully reverted, env creds set, against the failing repro,
WRANGLER_LOG=debugoutput capturing:getAccessHeaderslogsUsing Access Service Token headers for domain: <token.host>for the proxy session (access.ts:111). If yes,proxyDatashould carry them, and the question is what happens after the merge.InferenceUpstreamError: invalid_tokentext? These are different failure modes.That single capture would distinguish "real bug in B→edge for some bindings" from "AI-upstream auth issue conflated with Access" from "Access is the culprit for some other reason I haven't traced".
Fixes #N/A — discussion PR; not intended to merge as-is.