## Summary
validatePath() in McpContext returned immediately, with no restriction
at all, whenever roots() returned undefined. roots() only returns
undefined when the connecting MCP client never negotiates the optional
roots capability during initialize, which any minimal client can trigger
simply by omitting it from its declared capabilities.
Since roots() already always appends the OS temp directory to whatever
explicit roots are configured, this change makes it return that same
default (temp directory only) instead of undefined when no roots have
been set. This removes the early return in validatePath() entirely, so
path validation now runs unconditionally rather than being conditional
on whether the connecting client happened to negotiate a capability it
was never required to declare per the MCP spec.
Any filePath-accepting tool (take_screenshot, saveFile, and the
performance/Lighthouse export tools that route through the same check)
had its only path-traversal guard silently disabled for the lifetime of
a connection whenever the client omitted the optional roots capability.
Since this server is designed to let an LLM drive a browser, and browsed
page content is not trusted input, this meant a client that simply
doesn't implement roots (a plausible, non-adversarial default for
lightweight or custom MCP clients) removed the only boundary preventing
the connected agent from writing to any path the process can reach.
Added a test that exercises the actual default state of roots (never
calling setRoots()) directly, since the existing tests always call
setRoots(), even with an empty array, before validating. Verified
locally with a minimal MCP client that declares no capabilities: before
this change, take_screenshot with a filePath outside any root wrote a
real file to an arbitrary path with no error; after this change, the
same call is rejected with the existing Access denied error. Also
verified that a client that does declare roots is unaffected, and that
writes to the OS temp directory continue to succeed with no roots
negotiated, matching prior behavior for that path.
Fixes#2206
### Problem
`screencast_start` matched the requested file extension with a
**case-sensitive** `endsWith()` against `['.webm', '.mp4']` and
**silently fell back to `.mp4`** when nothing matched. Combined with
`ensureExtension()` (which replaces the extension), a request for
`demo.WEBM` was recorded as **MP4** to **`demo.mp4`** — a different
format *and* path than requested — and any unsupported extension (e.g.
`recording.avi`) silently became `.mp4`.
Separately, when `screencast_start` is called without a `filePath`, it
creates a temp directory via `mkdtemp()`. If `page.screencast()` then
throws (e.g. ffmpeg missing), that directory was leaked.
### Changes
Two commits:
1. **`fix: match screencast extension case-insensitively and reject
unsupported ones`** — match via `path.extname().toLowerCase()`; reject
an explicitly requested but unsupported extension with an explicit error
listing the supported formats; a missing extension still defaults to
`.mp4`.
2. **`fix: clean up screencast temp directory when recording fails to
start`** — remove the generated temp dir in the `catch` handler, but
only when we own the generated path (never when the caller supplied
`filePath`).
| requested | before | after |
| --------------- | --------------- | -------------- |
| `demo.WEBM` | mp4 → `demo.mp4`| webm → `demo.webm` |
| `recording.avi` | mp4 → `recording.mp4` | error (rejected) |
| `demo.webm` | webm → `demo.webm` | unchanged |
| *(no filePath)* | mp4 temp | unchanged |
The matched extension is normalized to lower case (`demo.WEBM` →
`demo.webm`).
### Testing
Added three regression tests to `tests/tools/screencast.test.ts` using
the existing `sinon`/`withMcpContext` harness. Verified locally against
Chrome for Testing 149 (`PUPPETEER_EXECUTABLE_PATH`):
- With the fix reverted, the two extension tests fail (uppercase `.WEBM`
→ mp4, `.avi` not rejected) and the cleanup test fails (temp dir left
behind) — i.e. they fail for the right reason.
- With the fix applied, the full `screencast.test.ts` suite passes
(11/11).
- `tsc --noEmit` and `npm run check-format` (eslint + prettier) are
clean.
> Note: I ran the `screencast` test file (which stubs `page.screencast`)
plus typecheck/lint locally; the rest of the browser-based suite I left
to CI.
### Notes for reviewers
- I chose to **`throw`** for an unsupported explicit extension
(consistent with the ffmpeg-missing `throw` in the same handler and with
the issue's "reject with an explicit error"). Happy to switch to the
softer `appendResponseLine(...) + return` style used by the in-progress
guard if you'd prefer.
- The two commits are independent and can be split if you'd rather take
them separately.
- I left the pre-existing `as \`${string}.webm\`` assertion on
`resolvedPath` untouched to keep the diff focused, though it's slightly
misleading now that the default is `.mp4`.
---------
Co-authored-by: Nicholas Roscino <nroscino@google.com>
## Summary
`screencast_stop` returns an empty response when no recording is active,
making it impossible for the calling agent to distinguish "stopped
successfully" from "nothing was recording."
`screencast_start` already handles its inverse case with an explicit
error (`"a screencast recording is already in progress"`), so this makes
`stop` consistent.
## Change
Added an error message when `screencast_stop` is called without an
active recording:
```ts
if (!data) {
response.appendResponseLine(
'Error: no active screencast recording to stop.',
);
return;
}
```
## Before
Empty tool response — agent cannot tell what happened.
## After
`Error: no active screencast recording to stop.`
---------
Co-authored-by: Nicholas Roscino <nroscino@google.com>
- renamed getSelectedPage to getSelectedPptrPage
- removes getSelectedPptrPage from tool interfaces
- moves dialog handling to McpPage
- makes responses to be optionally McpPage-scoped
## Summary
Adds `screencast_start` and `screencast_stop` MCP tools that allow
agents to record a video of a page using Puppeteer's `page.screencast()`
API.
Only enabled if the command line option `--experimental-screencast` is
provided
Closes#878.
## New Tools
### `screencast_start`
Starts recording a screencast (video) of the selected page in mp4
format.
| Parameter | Type | Default | Description |
|-----------|------|---------|-------------|
| `path` | string (optional) | temp file | Output file path |
### `screencast_stop`
Stops the active recording and reports the saved file path.
## Design
- **Start/stop pair** following the `performance_start_trace` /
`performance_stop_trace` pattern
- **State management** via `getScreenRecorder()` / `setScreenRecorder()`
on the Context interface
- **Category**: `DEBUGGING` (alongside `take_screenshot`)
- **ffmpeg dependency**: Clear error message when ffmpeg is not
installed
- **Temp file fallback**: When no `path` is provided, creates a temp
file with the correct extension
---------
Co-authored-by: Alex Rudenko <alexrudenko@chromium.org>
Alex Rudenko