G
YouTube TikTok Games
Skip to content

Keep a Cloudflare challenge's long address within the saved limit - #1013

Open
rudycelekli wants to merge 1 commit into
CopilotKit:mainfrom
rudycelekli:fix/cf-challenge-url-length
Open

rudycelekli wants to merge 1 commit into
CopilotKit:mainfrom
rudycelekli:fix/cf-challenge-url-length

Conversation

@rudycelekli

Copy link
Copy Markdown
Contributor

What this changes

A Cloudflare challenge on a long address can switch off a Bot's computer.

When the computer sees a challenge, detectChallenge (agent-computer/src/challenge.ts) raises a handoff with page.url(). pageWorthKeeping (agent-computer/src/control.ts) then decides what address to keep. It checks the length of the address it was given and drops the __cf_chl_* parameters with parsed.searchParams.delete(). Deleting through searchParams re-serialises the whole query with form encoding, so every :, /, ,, @ and ; in the other parameters comes back as %XX. An address under MAX_REQUEST_URL (2,048) can come back longer than it.

The control store (control-store.ts, validRequest) refuses a request whose url is over 2,048 characters, so save throws "Corrupt computer control state.". persist turns that into the sticky "The handoff state could not be saved. Computer actions are disabled until storage is repaired." and closes the action barrier. Every later persist for that Bot throws it again.

Repro (the same case as the new test): https://shop.example.com/login?next=https://shop.example.com/a/a/…(500 × "a/")&__cf_chl_rt_tk=abc is 1,083 characters. pageWorthKeeping returns 2,069 characters, because next=https://shop.example.com/a/a/… becomes next=https%3A%2F%2Fshop.example.com%2Fa%2Fa%2F…. requestHelp then throws. A second, ordinary requestHelp(..., "https://example.com/") throws the same error.

How a deployment reaches it: Cloudflare adds __cf_chl_rt_tk to the address of the page it challenges and keeps the original query. A sign-in or checkout page with a return address in its query is enough. The store is on by default (PROFILES_DIR, default /profiles). The same rewrite also turns ?flag into ?flag=.

The fix drops the __cf_chl_* pairs from the query as it was written and puts what is left back on the URL, so nothing else is re-encoded. The length check now applies to the address that is kept. One that is still too long is not kept, as already happens when the input itself is too long.

Where it runs

OpenBot is deployed as several server processes behind a load balancer, serving a whole company.
Consecutive requests from the same person reach different processes, and the process that answered a
WebSocket upgrade is rarely the one that answers the next call on that conversation.

State that outlives a single request therefore has to be shared, or the change works on one machine
and stops working the moment there are two, without saying so. That failure is worse than not
shipping the feature: it passes review, passes CI, passes a local demo, and only surfaces as a Bot
that forgets, a question nobody can answer, or a boundary that never fires.

Answer these even when the answer is "none":

  • New state that outlives a request? None. pageWorthKeeping is a pure function. The request it feeds is saved in the existing per-Bot control file.
  • What happens on the second replica? No change. This runs inside a Bot's computer, which owns its own control file.
  • Anything serialised? No.
  • Anything fanned out to a browser? No.
  • New listener, port, or schedule? No.

Postgres is already there and is the default answer to all of the above: a table, a unique index, a
conditional update, LISTEN/NOTIFY for fan-out.

Boundary and audit

  • Every acting call still goes through the gateway: resolve, decide, audit, then act. Unchanged. This is the computer's handoff record.
  • New refusals and new failures each write a row. There is no new refusal or failure. A failure that used to happen is removed.
  • Nothing new is trusted from the client that the server can resolve itself. Unchanged.

Changelog

  • A line in CHANGELOG.md under Unreleased:

A Cloudflare challenge on a long address no longer stops a Bot's computer

When a Bot met a Cloudflare challenge, the computer kept the page's address without the challenge's one-time __cf_chl_* tokens. Removing them rewrote the rest of the query, so characters such as : and / came back as %3A and %2F, and an address that fit the 2,048-character limit could grow past it. The computer then could not save the request, and from then on refused every action from that Bot with "The handoff state could not be saved". The tokens are now removed from the query as it was written, and an address still too long is not kept.

Proof

New tests:

  • agent-computer/tests/control-handoff-page.test.ts, dropping a challenge token leaves the rest of the query as it was written: an address with next=https://…, a bare flag and a __cf_chl_rt_tk keeps everything except the token, byte for byte.
  • agent-computer/tests/control-store.test.ts, a challenge on a long address is recorded, and later requests still save: with the real file store, the challenge request is saved with the cleaned address, and the next request saves too.

Before the fix (agent-computer/src/control.ts reverted to origin/main, tests kept):

$ cd agent-computer && bun test tests/control-handoff-page.test.ts tests/control-store.test.ts
error: The handoff state could not be saved. Computer actions are disabled until storage is repaired.
(fail) a challenge on a long address is recorded, and later requests still save
error: expect(received).toBe(expected)
(fail) only a real page is worth keeping > dropping a challenge token leaves the rest of the query as it was written
 21 pass
 2 fail

After the fix:

$ cd agent-computer && bun test tests/control-handoff-page.test.ts tests/control-store.test.ts
 23 pass
 0 fail

The existing a spent Cloudflare challenge token is not kept, the rest of the address is cases pass unchanged.

Gates: biome format and biome lint --error-on-warnings (2.5.10) report nothing on the three touched files. tsc --noEmit -p agent-computer reports no errors in the touched files. It ran against symlinked dependencies, and the only error it reports is that it cannot resolve spiffe (src/identity.ts, a file this change doesn't touch).

pageWorthKeeping dropped __cf_chl_* parameters through searchParams, which
re-encodes the whole query. An address under MAX_REQUEST_URL could come back
longer than it, the control store refused to save the request, and the Bot's
computer closed action admission. Filter the query as written instead, and
check the length of what is kept.

This branch has not been deployed

No deployments
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