Skip to content

fix: centralize file path validation for tools#2150

Closed
msnandhis wants to merge 1 commit into
ChromeDevTools:mainfrom
msnandhis:fix/centralize-file-path-validation
Closed

fix: centralize file path validation for tools#2150
msnandhis wants to merge 1 commit into
ChromeDevTools:mainfrom
msnandhis:fix/centralize-file-path-validation

Conversation

@msnandhis
Copy link
Copy Markdown

Fixes #2138.

Summary

This centralizes workspace-root validation for request file path fields instead of having individual tool handlers call context.validatePath(...) directly.

Changes included:

  • add required annotations.filePathFields metadata to tool definitions
  • validate annotated request fields centrally in ToolHandler before handler execution
  • annotate existing tools that accept request path fields such as filePath, requestFilePath, responseFilePath, outputDirPath, and extension path
  • remove duplicated per-tool validation calls for direct request path fields
  • keep non-request validation in reload_extension, because that path is resolved from existing extension state rather than from request params
  • add ToolHandler coverage to ensure annotated file path fields are validated before the handler runs

Testing

  • npm run format
  • npm run check-format
  • npm run test tests/ToolHandler.test.ts

I also started a full npm run test; it built and ran many tests locally, then remained quiet in a daemon/browser section, so I stopped the local run and cleaned up the spawned daemon processes rather than leaving them running.

@google-cla
Copy link
Copy Markdown

google-cla Bot commented May 28, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Comment thread src/tools/slim/tools.ts
category: ToolCategory.DEBUGGING,
// Not read-only due to filePath param.
readOnlyHint: false,
filePathFields: [],
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, we discussed it offline and we actually want to move filePathFields to the same level as blockedByDialog (i.e., outside of annotations). cc @Lightning00Blade

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.

[Task]: Refactor path validation

2 participants