fix(cli): give an edge mitigation its own code instead of FORBIDDEN - #33
Merged
Conversation
A Cloudflare bot or WAF product intercepting a request produced `FORBIDDEN`, whose hint reads "An administrator must grant your identity a role on this scope. `prk whoami` shows the identity the server sees." So a managed challenge sent the reader off to audit the grant table -- a system that was working, in a different product, with nothing to find. The message named the mitigation correctly the whole time; only the next step was wrong. The two are the same status. Both arrive as a bare 403, which is why the mitigation is read off `cf-mitigated` rather than the status, and why sharing a kind with an Access denial was tempting. Their fixes have nothing in common: FORBIDDEN wants a grant, and no grant will ever clear a bot rule. `MITIGATED` exits 7 rather than taking a new number. The request never reached the origin, which is what 7 already means for UNREACHABLE, TLS_FAILURE, TIMEOUT and NOT_A_PRICK_SERVER, and the exit table is part of the CLI's contract. It is deliberately not retryable, unlike Unreachable and Timeout: the identical request is challenged identically until something changes at the edge. Two tests had pinned the old behaviour, one of them end to end. The unit test asserted only that the message named the mitigation, which held just as well while the kind was wrong -- the assertion that would have caught this is the one nobody wrote, so it is written now. Docs: a new Cloudflare protections guide, because nothing in docs/ mentioned the bot layer at all. It covers why this hits servers and CI but not laptops, and two things worth stating outright -- that Bot Fight Mode cannot be skipped by a custom rule at all (it does not run on the Ruleset Engine, so the obvious fix silently does nothing on a free plan), and that the exception should skip the bot product rather than the whole WAF, since the Worker implements no rate limiting and Cloudflare's edge is the entire denial-of-service story. The GitHub Actions guide gets a note where it tells the reader to expect a 403: runners are datacenter IPs, so the confusable failure is documented next to the expected one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The previous commit stopped an edge mitigation being reported as FORBIDDEN, but
only on the response path. `prk login` fails earlier than that and never reached
the new code.
Discovery walks a list of candidate URLs because deployments answer
protected-resource metadata at different spellings, and a candidate that 404s is
expected input rather than a failure -- so the loops dropped every error. That is
right for a spelling nobody serves and wrong for a challenge: the request never
reached the server at all, and filing it under "this spelling does not exist"
left the reader with
error: could not discover the authorization server: no protected resource
metadata naming an authorization server at any of: ...
help: The URL answered but is not a prick server. Point --api-url at the
Worker's hostname, not at the web UI or a proxy.
while --api-url was already correct and Cloudflare was the whole story.
Both loops now keep the FIRST mitigation and carry on walking, then report it in
preference to their own summary. Carrying on matters: an edge exception can cover
one candidate and not another, and a document that answers is still better than
any error. That asymmetry is exactly how this was found -- a WAF rule scoped to
`/api` let the health probe through while every `/.well-known/` candidate stayed
challenged, because RFC 9728 appends the resource path to the well-known prefix
and `/.well-known/oauth-protected-resource/api/v1/health` contains `/api` without
starting with it.
The regression test mounts that same shape, and was checked against the
unfixed code rather than assumed to bite.
Docs: the guide led with the `/api` expression that causes this, so it now leads
with matching the hostname and carries the path-scoping trap as a caution. Also
states that zone-level custom rules are on every plan including Free, because the
dashboard offers an account-level WAF add-on that is Enterprise-only, unrelated,
and the obvious thing to click when a rule is what you came for.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Found while running
prk loginfrom an Ubuntu server. Cloudflare challenged the request, andprkreported it accurately — then told the reader to go and fix the wrong system:The message was right all along; only the next step was wrong.
cf_mitigatedresponses were classified asErrorKind::Forbidden, so they inherited its hint and sent you to audit a grant table that was working fine, in a different product, with nothing to find.Sharing a kind was tempting because the two are genuinely the same status — both arrive as a bare
403, which is why the mitigation is read offcf-mitigatedrather than the status in the first place. But their fixes have nothing in common:FORBIDDENwants a grant, and no grant will ever clear a bot rule.Same shape as
2ed6b02, which gave a guard refusal its own code instead of reusingLAUNCH_FAILED.The new kind
FORBIDDENMITIGATEDExit 7, not a new number: the request never reached the origin, which is already what 7 means for
UNREACHABLE,TLS_FAILURE,TIMEOUTandNOT_A_PRICK_SERVER— and the exit table is part of the CLI's contract. Deliberately not retryable, unlikeUnreachableandTimeout, because the identical request gets challenged identically until something changes at the edge.Two tests had pinned the old behaviour
One of them end to end through the real client. The unit test asserted only that the message named the mitigation, which held just as well while the kind was wrong — the assertion that would have caught this is the one nobody wrote. Both now assert the kind.
Docs
Nothing in
docs/mentioned the bot layer at all before this. The new Cloudflare protections guide covers why this hits servers and CI runners but not laptops, and states two things outright:Skip/Bypass/Allowhave no effect — the obvious fix silently does nothing on a free plan. Super Bot Fight Mode is the one that supports a skip rule.It also answers the question a reader will have — no, an exception doesn't weaken the authorization model, because Access still runs and the bot layer was never the gate.
The Actions guide gets a note in "The first run will 403, and that is the flow": GitHub-hosted runners are datacenter IPs, so the confusable failure is now documented right next to the expected one.
Verification
lint:rust(clippy-D warnings) green — it caught the first version of the exit-code arm undermatch_same_armsastro checkon the docs: 0 errors, 0 warnings, 0 hintsfmt:check,lint:typos,lint:locgreenNot verified against a live challenge: the fix was derived from the
cf-mitigatedheader contract and exercised through the mock intests/responses.rs, not by provoking a real Cloudflare challenge.🤖 Generated with Claude Code