Repository navigation
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 7 files
This PR changes authentication, authorization, or input validation. Ultrareviews find 2.4x more serious bugs than standard reviews. Comment @cubic-dev-ai ultrareview to run one.
View guided diff | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
|
Hi @gregpr07, could you review whether this opt-in command guard is a good fit for the shared shell execution boundary? It is related to browser-use/browser-use#6119 and uses the local Veltra checker, without sending commands to its hosted API. Checker failures deny execution, and the review findings on cancellation, environment overrides, UTF-8 handling, and input limits have been addressed. The docs also make clear that regex inspection is not a sandbox. Happy to narrow or adjust the integration based on your feedback. |
|
Hi @gregpr07, a scope question following the update from the Veltra author: they confirmed that hosted scanning is inspection-only and supplied an explicit allow/deny response contract. However, a fresh public-endpoint check still returns 404 for scanning while health succeeds; the results and deployment clarification request are in the original issue. Could you review the local execution gate independently of that hosted endpoint? This PR remains opt-in, uses the local checker, sends no commands to a third-party service, and adds no default dependency. I'd keep any hosted adapter as a separate follow-up after its deployment is verifiable and its data-sharing requirements are documented. The main feedback needed is whether the shared process-start/nonempty-stdin boundary and checker contract are the right integration point for Terminal, or whether you'd prefer a narrower scope. If someone else owns that area, could you point me to the appropriate reviewer? Happy to make concrete changes based on that guidance. |
|
Follow-up after the Veltra author's deployment fix: the hosted scan endpoint now passes the documented allow/deny response-contract smoke checks, plus missing/wrong-key and malformed-input checks. The previous 404 report is historical; fresh results and benchmark details are here. This does not change the PR's scope: the adapter remains local, opt-in, and sends no commands to the hosted API. All 48 Python tests passed again with the real Fresh Windows measurements put the four local checks, including audit I/O, at 1.34–3.31 ms median across five cases. The full checker subprocess was 309–653 ms median, with p95 around 942–1198 ms; it includes interpreter startup/imports/JSON I/O/cleanup and excludes the Rust caller. Those numbers are more variable than earlier runs, so this is not a fixed 200 ms or microsecond end-to-end claim. The Rust workspace was not re-run in this pass; the existing baseline failures documented above are not claimed fixed. @gregpr07, could you review whether the shared process-start/nonempty-stdin boundary and fail-closed checker contract are a good fit for Terminal? The updated test and timing evidence is above; happy to narrow the scope or address concrete feedback. Any hosted adapter can remain a separate follow-up rather than adding command-data sharing to this PR. |
3240588 to
875dcac
Compare
|
Hi @gregpr07 @laithrw, a gentle follow up on this. The command guard is still opt in, runs the checker locally, sends nothing to a hosted service and adds no default dependency. The main thing I need is a yes or no on whether the shared process start boundary in |
Agent-generated shell commands currently have no configurable inspection step at the shared process boundary. This adds an opt-in
BUT_COMMAND_GUARDexecutable that receives the proposed argv, working directory, per-command environment overrides, and call ID as JSON before a process starts. It must exit successfully with an explicit{"continue":true}verdict. Denials, malformed output, checker failures, oversized responses, and timeouts prevent execution. Cancellation interrupts a pending check. Nonemptywrite_stdincalls are checked too; output polling is unchanged.Includes a local adapter for
veltra-agent==0.4.0, using its command, filesystem, outbound-port, and environment-leak checks without executing the submitted payload. The adapter ignores task-controlled policy files and removes temporary audit data. It rejects interactive input because checking independent fragments can miss a command split across writes. It also rejects per-command environment overrides, whose execution effects Veltra's text checks cannot validate. Setup, UTF-8 configuration for Windows, the checker contract, and detection limits are documented indocs/command-guard.md; no new default dependency is required.Invalid guard configuration and checker failures report fixed diagnostic categories without forwarding raw checker output or command secrets. The adapter distinguishes operational failures from ordinary policy denials through its exit status.
Related to browser-use/browser-use#6119. The beta Python agent delegates its built-in shell tools to this Rust core, so enforcement belongs here. The hosted scan URL was rechecked on 18 October 2026 after the author reported a route fix: synthetic POSTs still return HTTP 404 while the health endpoint returns HTTP 200, and the published OpenAPI schema has no scan route. This adapter continues to use the published local inspection functions. Details and the request for a corrected deployment are in browser-use/browser-use#6119 (comment).
Validation:
cargo test --workspace --no-fail-fastis not green: the agent crate has six existing prompt/browser-recovery failures, and the browser crate has an existing wait-default assertion failure followed by poisoned-lock failures. The seven root failures reproduce on a clean checkout of upstream16cdd3a; those files are unchanged by this PR.