Croway opened a new pull request, #28681: URL: https://github.com/apache/camel/pull/28681
_Claude Code on behalf of Croway_ [CAMEL-26398](https://issues.apache.org/jira/browse/CAMEL-26398) ## Problem `camel_validate_source`, `camel_get_files`, `camel_edit_file` and `camel_write_file` take a `directory` and a `file` relative to it. In the eval (200 trials), 21 calls failed because the model named the file another way: an absolute `file` (with or without `directory`), no `directory` at all, a file name relative to the wrong directory, the YAML passed as `file`, or a `camel_write_file` into a directory that did not exist yet. The answers did not say what to pass instead. The JIRA first proposed taking an absolute file's parent as the directory, and the working directory of a stdio server as a default. As davsclaus pointed out in his review on the JIRA, that would let a call read any file on the disk. This PR follows the revised Fix section: the resolved file always stays inside the resolved project directory, and an absolute path is never a way around that. ## Fix One resolver (`AuthoringTools.fileTarget`) for the four tools, in camel-jbang-core: - **The directory** is the `directory` argument, else the default directory (the TUI's selected integration, `camel ask`'s project, as today), else the client's root: when the MCP client declares exactly one `file:` root. Without any of them the call is refused, and the answer says what to pass. An absolute `file` never stands for the directory. - **The file** is a path relative to the directory, or an absolute path inside it, which is made relative. **Containment** uses `toRealPath()` on both, for a path that does not exist yet the real path of its nearest existing parent. So a `..` that leaves the directory, an absolute path outside it, or a symbolic link in the project that leads out of it (also a link to nothing, which a write would follow) is refused. This applies to every path, also to the TUI's own file tools, which use the same `resolveFile`. - **`camel_write_file` only** creates a missing `directory`, and only when it lies inside the client's root (or the default directory). Otherwise the answer names the existing directory to pass, with the subdirectory in `file`, which the write already creates: `{"directory":"/workspace","file":"camel/application.properties"}`. - **"No such file"** (validate, read and edit) names a file of that name in a subdirectory of the directory: `camel/order-intake.yaml exists: pass file camel/order-intake.yaml`. The walk is the listing's: bounded, without `target`, `.git`, `node_modules` and hidden directories, and it does not follow links. For an edit this replaces "write the whole file with camel_write_file", which would have made a second copy. - **A `file` of several lines** (the source passed as the file name) is answered with "file has several lines, so it is not a file name ... the source goes in content" (for `camel_edit_file` there is no `content` hint). MCP roots (camel-jbang-mcp): the four wrappers get the `io.quarkiverse.mcp.server.Roots` parameter. The root is a lazy supplier in `ToolContext` (which stays free of MCP dependencies). It is asked for only when a call needs it (no `directory`, or a write into a missing one), at most once per call, only of a client that declares the `roots` capability and is on a stateful protocol version. `quarkus.mcp.server.roots.default-timeout=2s`, with a 3 s cap in the code, so a client that does not answer gets the "directory is required" answer instead of a hanging call. A failure counts as no root. An explicit `directory` is still any existing directory the call names, as before. The MCP client asks the user before a tool that is not read-only runs. | Call | origin/main | This PR | |---|---|---| | `validate {"file":"/w/camel/routes.yaml"}` | `directory is required: the project directory with the source files` | one client root `/w`: validated as `camel/routes.yaml`. No root: `directory is required: pass directory, the absolute project directory with the source files, and file relative to it (an absolute file must be inside it)` | | `validate {"file":"/w/camel/main-route.yaml","directory":"/w"}` | `file must be a path relative to the directory /w, not /w/camel/main-route.yaml` | validated as `camel/main-route.yaml` (same for get, edit, write) | | `validate {"file":"camel/book-orders.yaml"}` | `directory is required: ...` | one client root: validated there. No root: `directory is required: pass directory, ...` | | `validate {"file":"order-intake.yaml","directory":"/w"}` (the file is `camel/order-intake.yaml`) | `No such file in the directory: order-intake.yaml` | `No such file in the directory /w: order-intake.yaml; camel/order-intake.yaml exists: pass file camel/order-intake.yaml` | | YAML passed as `file` | `No such file in the directory: - route: ...` (the YAML echoed) | `file has several lines, so it is not a file name: file names the file, e.g. routes.camel.yaml, and the source goes in content` | | `write {"directory":"/w/camel-new","file":"application.properties"}` | `No such directory: /w/camel-new` | client root `/w`: created. No root: `No such directory: /w/camel-new; pass the existing project directory as directory and the subdirectory in file, which is created: {"directory":"/w","file":"camel-new/application.properties"}` | | `get {"directory":"/w","file":"/etc/hosts"}` | `file must be a path relative to the directory /w, ...` | `file /etc/hosts is not inside the directory /w: pass file relative to the directory, or the absolute path of a file inside it` | | `get {"directory":"/w","file":"shared/secret.yaml"}`, `/w/shared` a link to `/elsewhere` | the file outside `/w` is read | `file must stay inside the directory /w: shared/secret.yaml (a symbolic link leads out of it)` | | `get {"directory":"/w","file":"../x.yaml"}` | refused | refused (unchanged) | For `camel_validate_source`, an absolute `file` inside the directory is validated with the directory's other files, as the same file named relative to it is. With `content`, the directory is used only when the call passes one, as before, so that call stays deterministic. The working directory of a stdio server is not used. `connection.initialRequest().transport()` tells stdio from HTTP per connection, so it could be told apart, but the revised Fix section drops that fallback. Docs: `camel-jbang-mcp.adoc` describes the path rules next to the file tools, and `camel-jbang-tui-ai-agents.adoc` says that an absolute path inside the source directory is taken. Upgrade guide: a 4.24 entry for the one thing that worked before and is refused now, a file reached through a symbolic link out of the project directory (or a link to nothing). ## Testing - `AuthoringToolsFilePathsTest` (camel-jbang-core, new, 18 tests): an absolute file inside the directory (also only through a link, `/var` vs `/private/var` on macOS), outside it, and with no directory. The root as the default, asked once and not when the call names the directory. The default directory before the root. A write creating a missing directory inside the root and refused outside one, with the suggested call. An invalid write creates no directory. Only the write creates one, never the root itself. The subdirectory hint (and no hint for a copy under `target/`), the multi-line `file`, traversal, symbolic links out of the project (directory link, file link, link to nothing; disabled on Windows), and `sameAsFile` for an absolute file. - `AuthoringToolsRootsTest` (camel-jbang-mcp, new, 7 tests): the root mapping (one `file:` root, non-file roots ignored, two roots = none, bad URIs), the four wrappers reading and writing in the root, containment in the root, not asking when the call names a directory, not asking a client without roots, and a client that fails or never answers. The client's answer is faked through the `Roots` interface: a real MCP round trip needs a client, which this module's tests do not start. - `TuiFileToolsTest`: an absolute file inside the integration's source directory is written after the user confirms; one outside is not written, also without an integration named. - Updated: `AuthoringToolsTest` (core: the Maven-layout read names the file in `src/main/resources/camel`; mcp: new wrapper signatures, `Roots` is not a tool argument), `McpSecurityInterceptorTest` (the `camel_write_file` signature), `McpFacadeWriteFileTest` (an absolute path is refused when it is outside the source directory). - On origin/main, 13 of the 15 core path tests that compile there fail (the 3 that count root lookups need the new API, and the 2 traversal and missing-directory guards pass on main too). The Maven-layout hint test and both new `TuiFileToolsTest` tests fail too. The symbolic-link test shows that origin/main reads a file outside the directory through a link in it. - Full test suites: camel-jbang-core 2531 + 244, camel-jbang-plugin-tui 2285 (with `AiPanelPromptBudgetTest`), camel-jbang-mcp 512, no failures. Unrelated timing tests (`LaunchManagerOutcomeTest`, `RuntimeServiceTest`, `MonitorStartupTest`) failed once under load and passed when run again. - End to end over stdio with the runner jar and a Python MCP client: one declared root gets `roots/list` and is the default, `/etc/hosts` and a link out of the root are refused, a write creates `camel-new/` inside the root. Without roots, a call with no directory, or with an absolute file and no directory, is refused, and a write into a missing directory gets the suggested call. `Roots` does not show up in the tools' input schemas. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
