fix(mcp): stop search_project leaking $HOME and credentials; clamp goto_line

.github/workflows/build.yml

@@ -1212,6 +1212,13 @@ jobs:
             echo no tracker file — no transport suite reached its first test
           )
 
+      # Read the PE version resource back off the built .exe.
+      #
+      # resources/notepatra.rc carried a hardcoded FILEVERSION 0,1,0,0 for 124
+      # releases, so winget / SCCM / Intune and File Properties reported 0.1.0
+      # on every machine ever. It is now generated from project(Notepatra
+      # VERSION ...) — and this gate is what keeps it that way, because nothing
+      # else on any platform can observe an embedded Windows resource.
       # Separate step because the assertion needs pwsh string matching, and cmd
       # has no clean equivalent. `serve` on a std-only build exits 2 after
       # printing this, so we match on the text rather than the exit code.
@@ -1471,6 +1478,32 @@ jobs:
             Copy-Item "installers\unregister-associations.bat" ".\notepatra-win\" -Force
           }
 
+      # MUST stay AFTER the bundle step. This asserts on a built artifact;
+      # sitting above "Build C++ with CMake" it failed every run with
+      # "not found" on a binary that did not exist yet.
+      - name: Assert PE FileVersion matches the project version
+        shell: pwsh
+        run: |
+          $cmakeVer = (Select-String -Path CMakeLists.txt `
+              -Pattern 'project\(Notepatra VERSION ([0-9]+\.[0-9]+\.[0-9]+)' `
+              | Select-Object -First 1).Matches.Groups[1].Value
+          if (-not $cmakeVer) { Write-Host "::error::could not read version from CMakeLists.txt"; exit 1 }
+          $expected = "$cmakeVer.0"
+          # The SHIPPED exe, not the one still in the build tree: the bundle
+          # step copies it out and embeds the icon, so this is the file a
+          # user right-clicks -> Properties -> Details on.
+          $exe = Get-ChildItem -Path notepatra-win -Filter notepatra.exe -ErrorAction SilentlyContinue | Select-Object -First 1
+          if (-not $exe) { Write-Host "::error::notepatra.exe not found in notepatra-win/ - did the bundle step run?"; exit 1 }
+          $actual = (Get-Item $exe.FullName).VersionInfo.FileVersion
+          Write-Host "CMakeLists version : $cmakeVer"
+          Write-Host "expected FileVersion: $expected"
+          Write-Host "actual   FileVersion: $actual"
+          if ($actual -ne $expected) {
+            Write-Host "::error::PE FileVersion '$actual' != expected '$expected' — the .rc template did not substitute"
+            exit 1
+          }
+          Write-Host "OK: PE version resource matches the project version"
+
       - name: Diagnose notepatra.exe imports + Qt plugin loading
         # Launches notepatra.exe: a Qt plugin-load stall or a modal error box would otherwise hang here forever.
         timeout-minutes: 5

CMakeLists.txt

@@ -298,9 +298,29 @@ set(HEADERS
 
 # ── Platform-specific resources ──
 if(WIN32)
-    # Windows: embed icon via .rc resource file
-    if(EXISTS "${CMAKE_SOURCE_DIR}/resources/notepatra.rc")
-        list(APPEND SOURCES resources/notepatra.rc)
+    # Windows: embed the icon AND the version resource via a GENERATED .rc.
+    #
+    # The .rc used to be a static file with FILEVERSION hardcoded to 0,1,0,0,
+    # so every Windows build reported 0.1.0 to winget / SCCM / Intune and to
+    # File Properties > Details, regardless of the real version. Generating it
+    # from project(Notepatra VERSION ...) makes that impossible to drift: there
+    # is exactly one place the version is written.
+    if(EXISTS "${CMAKE_SOURCE_DIR}/resources/notepatra.rc.in")
+        # VERSIONINFO needs a 4-part comma form (a,b,c,d); PROJECT_VERSION is
+        # 3-part dotted, so pad the build field with 0.
+        set(NOTEPATRA_RC_VERSION_COMMA
+            "${PROJECT_VERSION_MAJOR},${PROJECT_VERSION_MINOR},${PROJECT_VERSION_PATCH},0")
+        set(NOTEPATRA_RC_VERSION_DOT
+            "${PROJECT_VERSION_MAJOR}.${PROJECT_VERSION_MINOR}.${PROJECT_VERSION_PATCH}.0")
+        # The generated .rc lives in the build tree, where a bare
+        # "notepatra.ico" would not resolve — point at the source copy. RC
+        # wants forward or escaped slashes, so hand it a forward-slash path.
+        set(NOTEPATRA_ICON_PATH "${CMAKE_SOURCE_DIR}/resources/notepatra.ico")
+        configure_file(
+            "${CMAKE_SOURCE_DIR}/resources/notepatra.rc.in"
+            "${CMAKE_BINARY_DIR}/notepatra.rc"
+            @ONLY)
+        list(APPEND SOURCES "${CMAKE_BINARY_DIR}/notepatra.rc")
     endif()
 endif()
 
@@ -586,6 +606,7 @@ if(NOTEPATRA_BUILD_TESTS AND EXISTS "${CMAKE_SOURCE_DIR}/test_indent_guide_theme
         ${CMAKE_SOURCE_DIR}/src/rustbridge.cpp
         ${CMAKE_SOURCE_DIR}/src/gitgutter.cpp
         ${CMAKE_SOURCE_DIR}/src/git_hunk_apply.cpp
+        ${CMAKE_SOURCE_DIR}/src/path_denylist.cpp
         ${CMAKE_SOURCE_DIR}/src/gutter_hunk_popup.cpp
         ${CMAKE_SOURCE_DIR}/src/diff_view.cpp)
     add_dependencies(test_indent_guide_theme rust_core)
@@ -1096,6 +1117,7 @@ if(NOTEPATRA_BUILD_TESTS AND EXISTS "${CMAKE_SOURCE_DIR}/test_ai_tools.cpp")
     add_executable(test_ai_tools
         test_ai_tools.cpp
         src/ai_tools.cpp
+        src/path_denylist.cpp
         src/git_tools.cpp
         src/csvanalyst.cpp
         src/dbconnections.cpp
@@ -1118,6 +1140,7 @@ if(NOTEPATRA_BUILD_TESTS AND EXISTS "${CMAKE_SOURCE_DIR}/test_git_tools_e2e.cpp"
     add_executable(test_git_tools_e2e
         test_git_tools_e2e.cpp
         src/ai_tools.cpp
+        src/path_denylist.cpp
         src/git_tools.cpp
         src/csvanalyst.cpp
         src/dbconnections.cpp
@@ -1140,6 +1163,7 @@ if(NOTEPATRA_BUILD_TESTS AND EXISTS "${CMAKE_SOURCE_DIR}/test_ai_dataanalyst.cpp
         test_ai_dataanalyst.cpp
         src/ai_systemprompt.cpp
         src/ai_tools.cpp
+        src/path_denylist.cpp
         src/git_tools.cpp
         src/chartrender.cpp
         src/chart_modal.cpp
@@ -1839,6 +1863,19 @@ if(NOTEPATRA_BUILD_TESTS AND EXISTS "${CMAKE_SOURCE_DIR}/test_single_instance.cp
     message(STATUS "Regression test enabled — target: test_single_instance")
 endif()
 
+# Workspace-root scoping. FileExplorer::workspaceRoot() must stay empty until
+# the user opens a folder — conflating it with the tree's display root (which
+# defaults to $HOME) made search_project walk the whole user profile and
+# disarmed the AI CSV sandbox guard.
+if(NOTEPATRA_BUILD_TESTS AND EXISTS "${CMAKE_SOURCE_DIR}/test_workspace_root.cpp")
+    add_executable(test_workspace_root test_workspace_root.cpp src/fileexplorer.cpp)
+    target_include_directories(test_workspace_root PRIVATE ${CMAKE_SOURCE_DIR}/src)
+    target_link_libraries(test_workspace_root PRIVATE Qt5::Widgets Qt5::Test)
+    notepatra_add_qt_test(test_workspace_root)
+    set_tests_properties(test_workspace_root PROPERTIES LABELS "smoke;privacy;mcp")
+    message(STATUS "Regression test enabled — target: test_workspace_root")
+endif()
+
 # D7 (win-open-ghost) — CLI arg parse contract (the Windows wide-argv fix
 # feeds QCoreApplication::arguments() into this parser).
 if(NOTEPATRA_BUILD_TESTS AND EXISTS "${CMAKE_SOURCE_DIR}/test_cli_args.cpp")
@@ -2610,6 +2647,7 @@ if(NOTEPATRA_BUILD_TESTS AND EXISTS "${CMAKE_SOURCE_DIR}/test_mcp_bridge.cpp")
     add_executable(test_mcp_bridge
         test_mcp_bridge.cpp
         src/mcp_bridge.cpp
+        src/path_denylist.cpp
         src/singleinstance.cpp
         # v0.1.118 — list_notes/read_note go through the real NotesStorage
         # layer (QtCore-only; notes_template.cpp resolves shellHtml).
@@ -2884,6 +2922,7 @@ if(NOTEPATRA_BUILD_TESTS AND EXISTS "${CMAKE_SOURCE_DIR}/test_options_actually_w
         src/editor_symbols.cpp   # Editor::applySymbolSettings() links against it
         src/gitgutter.cpp
         src/git_hunk_apply.cpp
+        src/path_denylist.cpp
         src/gutter_hunk_popup.cpp
         src/diff_view.cpp
         src/lexerutils.cpp
@@ -3034,6 +3073,7 @@ if(NOTEPATRA_BUILD_TESTS AND EXISTS "${CMAKE_SOURCE_DIR}/test_vega_chart.cpp")
     add_executable(test_vega_chart
         test_vega_chart.cpp
         src/ai_tools.cpp
+        src/path_denylist.cpp
         src/git_tools.cpp
         src/csvanalyst.cpp
         src/dbconnections.cpp

notepatra-mcp/mcpb/build-mcpb.py

@@ -104,6 +104,22 @@ def main():
     dcmd = cfg.get("platform_overrides", {}).get("darwin", {}).get("command", "")
     if not dcmd.endswith("server/darwin/notepatra-mcp"):
         fail(f"manifest darwin override '{dcmd}' does not point at server/darwin/notepatra-mcp")
+    # win32 was the ONLY platform whose override was never checked — the same
+    # blind spot that let the Windows build go unverified everywhere else.
+    wcmd = cfg.get("platform_overrides", {}).get("win32", {}).get("command", "")
+    if not wcmd.endswith("server/win32-x64/notepatra-mcp.exe"):
+        fail(f"manifest win32 override '{wcmd}' does not point at server/win32-x64/notepatra-mcp.exe")
+    # Every platform must end up launching with --socket. Overrides that specify
+    # only `command` inherit the base args, so the effective args are the base
+    # ones unless a platform sets its own; assert on the resolved value rather
+    # than trusting that inheritance, because losing this flag silently swaps the
+    # real editor for the in-memory MOCK and every tool returns fabricated data.
+    base_args = cfg.get("args", [])
+    for plat, ov in list(cfg.get("platform_overrides", {}).items()) + [("<base>", {})]:
+        eff = ov.get("args", base_args)
+        if "--socket" not in eff:
+            fail(f"platform '{plat}' resolves to args {eff} — missing --socket, "
+                 f"so it would run the in-memory mock instead of the editor")
     if a.require_all:
         for plat in ALL_PLATFORMS:
             if not any(n.startswith(f"server/{plat}/") for n in names):

notepatra-mcp/src/main.rs

@@ -2,15 +2,80 @@
 use notepatra_mcp::server::Server;
 use notepatra_mcp::transport::{mock::MockEditor, socket::SocketEditor};
 
+const USAGE: &str = "\
+notepatra-mcp — Model Context Protocol server for the Notepatra editor
+
+USAGE:
+    notepatra-mcp [--socket]
+    notepatra-mcp <serve|pair|connect>          (requires the `remote` feature)
+
+OPTIONS:
+    --socket        Drive the RUNNING editor over its local MCP bridge. Without
+                    this the server answers from an in-memory MOCK whose data is
+                    fabricated — useful for exercising the protocol, never for
+                    real work.
+    -h, --help      Print this help and exit.
+    -V, --version   Print the version and exit.
+
+This is a stdio server: with no flags it speaks JSON-RPC on stdin/stdout and
+will appear to hang if you run it by hand. That is expected — it is meant to be
+launched by an MCP client, not a terminal.
+
+Docs: https://notepatra.org/mcp.html";
+
 fn main() -> std::io::Result<()> {
     // Phase 3a subcommands (serve/pair/connect) are dispatched FIRST and only
     // when they are the first argument, so every existing invocation — no args,
-    // `--socket`, anything else — reaches the unchanged stdio path below,
-    // byte-for-byte identical to HEAD.
+    // `--socket`, anything else — reaches the unchanged stdio path below.
     if let Some(mode @ ("serve" | "pair" | "connect")) = std::env::args().nth(1).as_deref() {
         return run_remote_mode(mode);
     }
 
+    // A MISTYPED subcommand must fail loudly too.
+    //
+    // Rejecting only bad flags left half the hang in place: USAGE advertises
+    // bare-word subcommands, so `notepatra-mcp sevre` fell straight through to
+    // the stdio loop and blocked on stdin forever — the same "looks crashed"
+    // symptom, reached by the other spelling mistake.
+    if let Some(first) = std::env::args().nth(1) {
+        if !first.starts_with('-') && !matches!(first.as_str(), "serve" | "pair" | "connect") {
+            eprintln!("notepatra-mcp: unknown subcommand '{first}'");
+            eprintln!("Expected one of: serve, pair, connect");
+            eprintln!("Try 'notepatra-mcp --help' for usage.");
+            std::process::exit(2);
+        }
+    }
+
+    // Discovery flags, and rejection of anything else that LOOKS like a flag.
+    //
+    // Before v0.1.125 none of this existed: every unrecognised argument fell
+    // through to the stdio loop below, which blocks reading stdin. So
+    // `--version` printed nothing and hung, and a typo like `--sokcet` silently
+    // started a MOCK server instead of failing. Both read as a crashed program.
+    // An unknown flag must be a loud error, never a hang.
+    for arg in std::env::args().skip(1) {
+        match arg.as_str() {
+            "-h" | "--help" => {
+                println!("{USAGE}");
+                return Ok(());
+            }
+            "-V" | "--version" => {
+                println!("notepatra-mcp {}", env!("CARGO_PKG_VERSION"));
+                return Ok(());
+            }
+            "--socket" => {}
+            // Bare words are left alone: a client may pass a path or an
+            // extra positional we do not own. Only flag-shaped typos are
+            // fatal, because those are ours to get wrong.
+            other if other.starts_with('-') => {
+                eprintln!("notepatra-mcp: unrecognised option '{other}'");
+                eprintln!("Try 'notepatra-mcp --help' for usage.");
+                std::process::exit(2);
+            }
+            _ => {}
+        }
+    }
+
     // `--socket` targets the running editor over its dedicated MCP bridge
     // socket; default is the in-memory mock so any MCP client can exercise
     // the protocol without a running editor.

notepatra-mcp/src/server.rs

@@ -31,8 +31,12 @@ const NOTE_URI_PREFIX: &str = "notepatra://note/";
 pub struct Server<T: EditorTransport> {
     transport: T,
     /// serverInfo.version, resolved once at startup: NOTEPATRA_MCP_VERSION
-    /// (set by the editor when it spawns the sidecar) wins over this crate's
-    /// own version.
+    /// wins over this crate's own version if it is set.
+    ///
+    /// NOTE: nothing in the repo currently SETS that variable — the editor does
+    /// not spawn the sidecar (MCP clients launch it), so in every shipped
+    /// configuration this is CARGO_PKG_VERSION. The override is kept as an
+    /// escape hatch for a packager who ships a differently-versioned binary.
     version: String,
 }
 

notepatra-mcp/src/tools.rs

@@ -210,7 +210,7 @@ pub fn definitions() -> Value {
         },
         {
             "name": "goto_line",
-            "description": "Move the editor cursor to a 1-based line number in a tab (defaults to the active tab). Use to point the user at a specific location, e.g. after finding a match.",
+            "description": "Move the editor cursor to a 1-based line number in a tab (defaults to the active tab). Use to point the user at a specific location, e.g. after finding a match. Lines past end-of-file are CLAMPED to the last line: the response's `line` is where the cursor actually landed and `clamped` is true, so check it before any cursor-relative write.",
             "inputSchema": {
                 "type": "object",
                 "properties": {
@@ -779,23 +779,53 @@ fn optional_index(args: &Map<String, Value>, key: &str) -> Result<Option<usize>,
     }
 }
 
+/// The ONE message every 1-based-integer rejection uses.
+///
+/// These helpers used to delegate to `optional_index`/`required_index`, whose
+/// `as_u64()` rejected negatives with "must be a non-negative integer" before
+/// the `>= 1` check was ever reached. So `line: 0` and `line: -5` — the same
+/// mistake — came back with two different and mutually contradictory rules, one
+/// of which explicitly permits 0. Validating here keeps it to one sentence.
+fn one_based_error(key: &str) -> CallOutcome {
+    CallOutcome::InvalidParams(format!("{key} must be an integer >= 1"))
+}
+
+/// The C++ bridge reads these through QJsonValue::toInt(0), which yields 0 —
+/// and therefore the misleading "must be >= 1" — for anything above i32::MAX.
+/// Reject out of range here instead, with a message that names the real limit.
+const MAX_ONE_BASED: u64 = i32::MAX as u64;
+
+fn check_one_based(key: &str, n: u64) -> Result<usize, CallOutcome> {
+    if n == 0 {
+        return Err(one_based_error(key));
+    }
+    if n > MAX_ONE_BASED {
+        return Err(CallOutcome::InvalidParams(format!(
+            "{key} must be an integer >= 1 and <= {MAX_ONE_BASED}"
+        )));
+    }
+    Ok(n as usize)
+}
+
 /// Optional 1-based integer (line/column numbers).
 fn optional_one_based(args: &Map<String, Value>, key: &str) -> Result<Option<usize>, CallOutcome> {
-    match optional_index(args, key)? {
-        Some(0) => Err(CallOutcome::InvalidParams(format!(
-            "{key} must be an integer >= 1"
-        ))),
-        v => Ok(v),
+    match args.get(key) {
+        None | Some(Value::Null) => Ok(None),
+        Some(v) => match v.as_u64() {
+            None => Err(one_based_error(key)),
+            Some(n) => check_one_based(key, n).map(Some),
+        },
     }
 }
 
 /// Required 1-based integer (line/column numbers).
 fn required_one_based(args: &Map<String, Value>, key: &str) -> Result<usize, CallOutcome> {
-    match required_index(args, key)? {
-        0 => Err(CallOutcome::InvalidParams(format!(
-            "{key} must be an integer >= 1"
-        ))),
-        n => Ok(n),
+    match args.get(key) {
+        None => Err(CallOutcome::InvalidParams(format!("{key} is required"))),
+        Some(v) => match v.as_u64() {
+            None => Err(one_based_error(key)),
+            Some(n) => check_one_based(key, n),
+        },
     }
 }
 
@@ -965,9 +995,12 @@ fn goto_line(transport: &mut dyn EditorTransport, args: &Map<String, Value>) ->
     if let Err(e) = reject_extras(args, &["line", "tab_index"]) {
         return e;
     }
-    let line = match required_index(args, "line") {
-        Ok(n) if n >= 1 => n,
-        Ok(_) => return CallOutcome::InvalidParams("line must be an integer >= 1".into()),
+    // required_one_based, NOT a hand-rolled required_index + `>= 1` check:
+    // select_range and insert_text already route every 1-based argument through
+    // the shared helper, and goto_line being the one exception is what produced
+    // two contradictory rules for the same mistake.
+    let line = match required_one_based(args, "line") {
+        Ok(n) => n,
         Err(e) => return e,
     };
     let tab_index = match optional_index(args, "tab_index") {

notepatra-mcp/src/transport/mock.rs

@@ -485,8 +485,23 @@ impl EditorTransport for MockEditor {
         if line == 0 {
             return Err(TransportError("line numbers are 1-based".into()));
         }
+        // Clamp exactly as the C++ bridge does, and report the line LANDED on.
+        //
+        // This mock is the default transport of the shipped binary (no
+        // `--socket`) and every protocol test runs through it — so while it
+        // echoed the requested line it reproduced the NP-01 false report
+        // verbatim (`ok:true, line:99999` on a 4-line tab) and no Rust test
+        // could ever have observed the fixed response shape.
+        let total = self.tabs[i].content.lines().count().max(1);
+        let landed = line.min(total);
         self.selection.0 = i;
-        Ok(json!({ "ok": true, "tab_index": i, "line": line }))
+        Ok(json!({
+            "ok": true,
+            "tab_index": i,
+            "line": landed,
+            "requested_line": line,
+            "clamped": landed != line,
+        }))
     }
 
     fn select_range(