security(claude): sandbox UI-bridge tool paths to jupyter_root

[pjdoland]

Summary

Follow-up to #290. That PR closed the working_directory traversal on the two built-in shell tools (run_command_in_embedded_terminal and built_in_toolsets.run_command_in_jupyter_terminal) but left the two Claude-mode siblings open with the same shape of bug. This PR closes them.

• run-command-in-jupyter-terminal at claude.py:1071 forwarded working_directory straight to the JupyterLab UI command notebook-intelligence:run-command-in-terminal, which passes it to terminal:create-new as cwd. An LLM tool call with working_directory='/etc', '../../..', or a workspace symlink pointing outside opened a terminal there and ran the command.
• open-file-in-jupyter-ui at claude.py:1089 forwarded file_path to docmanager:open. JupyterLab's contents service today rejects out-of-root absolute paths, but relying on framework behavior left the same shape of bug latent.

Solution

Three layers, one commit each, plus a remediation commit for the six-agent review findings.

• Lifted _get_safe_path from built_in_toolsets.py to util.safe_jupyter_path (public, next to get_jupyter_root_dir). util.py has no NBI imports, so it's a clean chokepoint with no risk of import cycles. The original module keeps a backwards-compat alias so the 8 existing call sites read identically.
• Applied the gate at both Claude-mode tools. run_command_in_jupyter_terminal validates working_directory and forwards the sandboxed absolute path to the UI bridge (terminal:create-new honors absolute cwd via PTY spawn). open_file_in_jupyter_ui validates file_path and forwards the relative-to-root form to docmanager:open, since JupyterLab's contents service strips leading slashes and rejoins under root, so an absolute path would 404.
• safe_jupyter_path raises RuntimeError (not ValueError) for the "workspace root not set" case so callers' except ValueError blocks can't swallow a server-side misconfig as if the LLM had picked a bad path.
• tool_text_response gains an is_error keyword that maps to MCP's isError flag. Every rejection path now sets it so the model treats sandbox violations as faults to recover from rather than as authoritative output.

Testing

tests/test_claude_tool_path_sandbox.py ships 21 cases:

• TestRunCommandInJupyterTerminalSandbox (10): absolute escape, relative traversal, workspace symlink pointing outside, traversal via a valid-subdir prefix (valid/../../.., pins that resolve() collapses .. before the containment check), embedded NUL, nonexistent dir, file-not-dir, plus positive cases for relative subdir, ., and "" meaning the root. Every rejection asserts the UI bridge spy was never invoked.
• TestOpenFileInJupyterUiSandbox (7): absolute escape, relative traversal, workspace symlink, embedded NUL, plus three positive cases including a deeper nested path and a not-yet-existing file (docmanager:open is used for paths the user is about to create, so the gate deliberately doesn't require existence).
• TestMCPErrorSignalling (4): pins is_error: True on rejection paths and its absence on happy-path returns.
• TestRootNotSet (1): pins that the unset-root case surfaces as a distinct error message ("not set") rather than getting reflected as a sandbox violation.

Mirrors the structure of tests/test_builtin_toolset_cwd_sandbox.py from PR #290 so the two test files read side-by-side.

Verified the new tests fail without the fix by stashing claude.py and re-running the absolute-escape cases; both flunk, then pass once the fix is reapplied.

Full suite: pytest tests/ --ignore=tests/test_claude_client.py → 1029 passed. jlpm tsc --noEmit, jlpm lint:check, jlpm jest all clean.

Six-agent review

Six reviewers (code reuse, code quality, security, frontend bridge, test architecture, API contract) ran in parallel against the first three commits. The remediation commit lands every convergent fix-before-merge finding:

• Three reviewers converged on the docmanager:open regression (security, code quality, frontend bridge): the first revision forwarded str(target), an absolute path, which JupyterLab's contents service would 404 every time. Fixed by forwarding target.relative_to(root_dir).as_posix().
• Two reviewers (code quality, API contract) flagged missing is_error on MCP rejections. Fixed.
• Test architecture flagged a fixture leak (yield/teardown can leave state across tests). Migrated to monkeypatch.setattr.
• Code quality flagged ValueError for the unset-root case. Reclassified as RuntimeError.

Documented follow-ups (none security-relevant, deferred to keep this PR scoped): rename the 8 _get_safe_path call sites to drop the alias; align execute_command's rejection wording with the two terminal tools; relativize jupyter_root_dir out of error text for hosted-deployment safety; document Windows hardlink semantics; add URI-encoded-traversal / whitespace-path / very-long-path test cases.

Risks

• Behavior change: an LLM call with an absolute working_directory or file_path outside the workspace now gets a is_error: True response with the documented "outside allowed directory" wording. Matches the existing built-in tools' UX so the model's existing self-correction patterns apply.
• open-file-in-jupyter-ui path shape: the forwarded payload changes from "whatever the LLM sent" to "the relative-to-root POSIX form." For a relative path the LLM already supplied, this is a normalize-and-pass-through (same path). For an absolute path inside the workspace, the result is a working open instead of a 404. No regression on legitimate calls.