fix: check fs access in builtin commands - #1
Conversation
hi-ogawa
commented
Jun 30, 2026
hi-ogawa
commented
- ported from fix(browser): check fs access in builtin commands vitest-dev/vitest#10674
The `takeScreenshot` (write) and `upload` (read) commands resolved client-controlled paths without checking the `allowWrite` permission gate or Vite's `server.fs` access rules. Add vendored `assertBrowserApiWrite`/`assertBrowserFileAccess` helpers and apply them so these commands respect the same protections as the built-in file commands. Co-authored-by: OpenCode (claude-opus-4-8) <noreply@opencode.ai>
hi-ogawa
changed the title
There was a problem hiding this comment.
Dumped the copy of packages/browser/src/node/utils.ts from vitest-dev/vitest#10674
|
I feel like we need a better commands API so we can just hook into something like |
| } | ||
|
|
||
| export function assertBrowserApiWrite(project: TestProject, path: string): void { | ||
| if (!project.config.browser.api.allowWrite || !project.vitest.config.api.allowWrite) { |
There was a problem hiding this comment.
My config PR removes config.browser.api - if we don't want to open a new PR here after vitest-dev/vitest#10554 is merged, we can at least do browser.api?.allowWrite
There was a problem hiding this comment.
Good point.
| if (!project.config.browser.api.allowWrite || !project.vitest.config.api.allowWrite) { | |
| // `browser.api` is gone since https://github.com/vitest-dev/vitest/pull/10554 | |
| if (!(project.config.browser.api && project.config.browser.api.allowWrite) || !project.vitest.config.api.allowWrite) { |
There was a problem hiding this comment.
Oh wait, this is actually wrong !(project.config.browser.api && project.config.browser.api.allowWrite).
There was a problem hiding this comment.
I think it is correct now, no?
There was a problem hiding this comment.
Ah, wait, you should also check for project.config.api, not just project.vitest.config.api. Maybe we should merge the config PR first 😄
There was a problem hiding this comment.
Updated to this. So many configs lol
const browserApiAllowWrite = project.config.browser.api
? project.config.browser.api.allowWrite
: project.config.api.allowWrite
if (!browserApiAllowWrite || !project.vitest.config.api.allowWrite) {
`browser.api` is being unified into the main `api` config in vitest-dev/vitest#10554. Fall back to `api.allowWrite` when `browser.api` is absent so the gate works before and after that change. Co-authored-by: OpenCode (claude-opus-4-8) <noreply@opencode.ai>
…/command-write-gate # Conflicts: # src/commands/permissions.ts
2 participants
