Validate timeoutMs on POST /computers/:botId/exec - #427
Merged
davidmckayv merged 2 commits intoSep 8, 2026
Conversation
A NaN, Infinity, negative, fractional, or ten-hour timeout travelled to the computer, where AbortSignal.timeout threw a RangeError 500 or the run outlasted the 615s transport backstop the gateway documents. The shell's real bounds are 1s to 600s. Reject anything outside a whole number of milliseconds in 1000..600000 with 400 before the gateway decides and records, so the refusal is a caller error rather than a failed action on the trail.
Ayush7614
requested review from
MikeRyanDev,
davidmckayv,
guidovizoso and
tylerslaton
as code owners
September 7, 2026 15:41
davidmckayv
approved these changes
Sep 8, 2026
davidmckayv
left a comment
davidmckayv
left a comment
Contributor
There was a problem hiding this comment.
Deep-reviewed against live code (correctness, governance, no vendor/secret/scale issues). Composed build+tests green. CHANGELOG/format rebase on CI-validated substance.
davidmckayv
added a commit
that referenced
this pull request
Sep 9, 2026
`POST /:botId/scroll` and `POST /:botId/human/scroll` guarded `deltaY` with `typeof value === "number"`, which is true of `Infinity`. `1e999` is valid JSON and parses to exactly that, so the value passed the check, travelled to the gateway, and was written by `JSON.stringify` as `null` on the hop to the Bot's computer -- where `typeof body.deltaY === "number"` is now false and the computer scrolls its own default instead (600 pixels for the tool path, 400 for a person's). The caller got 200 and a scroll it did not ask for. Every other number on this surface is already checked at the edge: `timeoutMs` on `exec` is rejected unless it is a whole number in range (#427), and the coordinates behind `human/click` are rejected unless `Number.isFinite` accepts them. `deltaY` was the one that was not, so it is checked the same way, at the same place, with the same 400. Wrong types are refused rather than silently dropped, which is what `exec` already does with a `timeoutMs` of `"3000"`. Measured: with the routes unchanged and only the new test applied, 10 of 14 cases fail -- every rejection case on both paths answers 200 and reaches the gateway. With the change, 14 pass. Co-authored-by: David McKay <david@copilotkit.ai>
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
2 participants
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.
What
POST /:botId/execinserver/src/computer/routes.tspassed any numerictimeoutMsstraight togateway.runCommand, whose 615s transport backstop exists to outlast the shell rather than enforce the limit. A NaN, Infinity, negative, fractional, or ten-hour value travelled to the computer, whereAbortSignal.timeoutthrew a RangeError 500 or the run outlasted the backstop the gateway documents.This validates
timeoutMsagainst the shell's own bounds (whole milliseconds, 1000..600000) and answers400 { error }before the gateway decides and records — so the refusal is a caller error, not a failed action on the audit trail. Omitted timeouts still run with the computer default.Why it matters
Reliability and cost: unbounded shell runs are the most expensive thing a Bot can start, and a malformed timeout previously surfaced as a 500 or a run that outlived its transport. Twelve route-level cases now pin the contract.
Verification
server/tests/computer-exec-timeout.test.ts: valid 5000 reaches the gateway; absent timeout runs with default; NaN, Infinity, negatives, zero, 999, 600001, 36M, fractions, strings, and null each answer 400 without reaching the gateway.bun test server/tests/computer-exec-timeout.test.ts— 12 pass.bun run --filter server typecheck— clean. Biome format + lint — clean.