fix(mcp): stop search_project leaking $HOME and credentials; clamp goto_line - #33
Merged
Conversation
…to_line
Four defects reported from Windows testing, plus everything an adversarial
review found in the fixes themselves.
NP-01 goto_line past EOF lied. Scintilla rejects an out-of-range position and
leaves the cursor at the TOP, while the response still read {"line":99999,
"ok":true}. Since insert_text defaults to the cursor, an assistant aiming at
the end of a file wrote at the beginning, behind an approval card that looked
correct. Editor::gotoLine now clamps and RETURNS where it landed; the verb
reports line/requested_line/clamped.
NP-02 search_project walked the user's whole profile. FileExplorer's ctor set
m_rootPath = QDir::homePath() as a display placeholder, and four separate
guards read that as "the workspace" — including the AI CSV sandbox, a security
guard that had therefore never once closed, and the Coding Mode prompt, which
was dead code. Display root and workspace root are now different questions:
workspaceRoot() stays empty until the user opens a folder.
MainWindow grew four accessors in place of one, because "what folder is the
user working on" has four different right answers: workspaceFolder() (scoping,
empty means refuse), firstOpenFileDir(), suggestedDialogFolder() (may fall back
broadly — the user sees and edits it), aiWorkspaceRoot() (sticky per session).
The first fix collapsed them again as "folder, else current file's directory",
which resolves to $HOME the moment one file in $HOME is open, and made the AI
root flap on every Ctrl+Tab — swapping the chat history and cancelling pending
write approvals. Chat history migrates from the old sha1($HOME) key by COPY.
NP-03 the sidecar hung on every bad argument. --version printed nothing and
blocked on stdin; --sokcet silently started the MOCK against fabricated tabs;
a mistyped subcommand fell through the same way. All now exit 2 loudly.
NP-04 the Windows PE carried FileVersion 0,1,0,0 for 124 releases. The .rc is
generated from CMake now, with a CI step reading VersionInfo back. UNVERIFIED
on any platform — the gate has never run.
search_project also honours the credential deny-list. That list lived in TWO
places and had drifted in BOTH directions: ai_tools had *.tfvars/*.tfstate/
.pypirc/dotenv/backslash variants, git_hunk_apply had *.jks and an unanchored
id_rsa, neither was a superset, and search_project consulted neither. So
read_file refused ~/.ssh/id_rsa while a one-word search returned its lines.
src/path_denylist.h is now the only list — the union — with three callers.
insert_text with col but no line silently dropped col and wrote at the cursor
instead. Silently relocating a write is the one thing an approval gate cannot
protect against, so it is an error now.
Tests: 76/76 C++, 91 Rust. Every new test red-state verified by restoring the
pre-fix code rather than negating a condition — a hand-written sabotage of
search_project still returned the right answer and made a sound test look
weak. Signatures matched the original bug reports: {"line":99999,
"clamped":false}, workspace_searched:true with no workspace, and
"leaked contents of keystore.jks" — the extension only the git_hunk_apply
copy of the deny-list had.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W6BGY2dFUSujZeS3vTDikq
…remote The vacuity guard I added in a0ae9ba spawns each real subcommand and waits for it with .output(). That waits for EOF on the child's pipes, and `serve` built --features remote binds a port and never returns — so the call never came back. CI runs the suite twice, `cargo test --release` then `cargo test --release --features remote`, and the second run wedged the 25-minute step on all four platforms. The C++ suite and the other 91 Rust tests were green everywhere; this one test burned the jobs. It passed locally because I only ever ran the default-feature build, where `serve` exits 2 immediately. Verified the difference directly: default serve -> exit 2 --features remote serve -> still running at 5s Nothing in this file waits unbounded any more. run_bounded() spawns with a null stdin, drains both pipes on their own threads so a full pipe cannot masquerade as a hang, polls try_wait() to a deadline, and kills the child when the budget expires. run() wraps it at 30s and asserts the child exited, so a future regression that makes --help hang fails the test by name instead of timing out the job. For the vacuity guard specifically, a child still running at the 3s budget is now a PASS: the typo guard exits before it can bind anything, so a live process proves the subcommand was understood. If it does exit, the only clean exit is the built-without-remote refusal (2). Red-state verified: moved the dispatch below the guard and dropped its serve|pair|connect exclusion, reproducing the over-broad guard this test exists to catch. Exactly one test went red — the_real_subcommands_are_not_swallowed_by_the_typo_guard, on "unknown subcommand 'serve'" — in 0.10s, not a hang. Both feature configs now pass locally: 4/4 cli tests, cli.rs finishing in 0.10s default and 3.07s with remote (the killed `serve`). clippy 0 in both configs, fmt clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W6BGY2dFUSujZeS3vTDikq
The NP-04 gate ran for the first time on 3e839fc and failed instantly: ##[error]notepatra.exe not found under build/ Not a version mismatch — I had inserted it at step 11 of build-windows, between "Transport test trackers" and "Build C++ with CMake (MSVC)". It asserted on a binary that did not exist yet, so it could only ever fail, and it would have kept failing no matter what the .rc contained. Moved to immediately after "Verify exe, embed icon, bundle Qt + QScintilla DLLs", and retargeted from the build tree to notepatra-win/ — the copy that actually ships, which is the file a user right-clicks -> Properties -> Details on. The bundle step does not touch the resource table (windeployqt and a manual DLL copy only), so this is the same resource the compiler emitted, just checked on the artifact rather than an intermediate. Verified the generation half locally, which is as far as Linux reaches: reconstructed the real WIN32 block from CMakeLists.txt in a throwaway project and ran cmake on it. FILEVERSION 0,1,124,0 PRODUCTVERSION 0,1,124,0 VALUE "FileVersion", "0.1.124.0" VALUE "ProductVersion", "0.1.124" The gate computes "$cmakeVer.0" = "0.1.124.0" and compares against VersionInfo.FileVersion, which reads that StringFileInfo value — exact match, no unsubstituted @var@ left, icon path resolves. What remains unverified anywhere: rc.exe actually compiling the generated .rc, and the version surviving into the PE. Only the Windows runner can answer that, and it has still never gotten far enough to try. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W6BGY2dFUSujZeS3vTDikq
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Four defects reported from Windows testing, plus everything an adversarial review found in the fixes themselves.
goto_linepast EOF reported{"line":99999,"ok":true}while the cursor sat at line 1insert_textdefaults to the cursor, so an assistant aiming at end-of-file wrote at the beginning, behind an approval card that looked correct.search_projectwalked the user's entire profileFileExplorer's ctor setm_rootPath = QDir::homePath()as a display placeholder. Four separate guards read that as "the workspace" — including the AI CSV sandbox, a security guard that had therefore never once closed, and the Coding Mode prompt, which was dead code.--versionprinted nothing and blocked on stdin;--sokcetsilently started the mock against fabricated tabs; a mistyped subcommand fell through the same way. All exit 2 loudly now.FileVersion 0,1,0,0for 124 releases.rcwas hand-written and never updated. Now generated from CMake, with a CI step readingVersionInfoback.Beyond the reported four
MainWindowgrew four accessors in place of one, because "what folder is the user working on" has four different right answers:workspaceFolder()(scoping — empty means refuse),firstOpenFileDir(),suggestedDialogFolder()(may fall back broadly; the user sees and edits it),aiWorkspaceRoot()(sticky per session). The first NP-02 fix collapsed them again as "folder, else current file's directory" — which resolves to$HOMEthe moment one file in$HOMEis open, and made the AI root flap on every Ctrl+Tab, swapping chat history and cancelling pending write approvals. Chat history migrates from the oldsha1($HOME)key by copy, never move.The credential deny-list existed twice and had drifted in both directions.
ai_tools.cpphad*.tfvars/*.tfstate/.pypirc/dotenv/backslash variants;git_hunk_apply.cpphad*.jksand an unanchoredid_rsa. Neither was a superset, andsearch_projectconsulted neither — soread_filerefused~/.ssh/id_rsawhile a one-word search returned the matching lines out of it.src/path_denylist.his now the only list (the union), with three callers.insert_textwithcolbut nolinesilently droppedcoland wrote at the cursor. Silently relocating a write is the one thing an approval gate cannot protect against.Verification
76/76 C++, 91 Rust, clippy 0, fmt clean.
Every new test was red-state verified by restoring the pre-fix code rather than negating a condition — a hand-written sabotage of
search_projectstill returned the right answer and made a sound test look weak. The red signatures matched the original bug reports:{"line":99999,"clamped":false},workspace_searched:truewith no workspace, andleaked contents of keystore.jks— the extension only thegit_hunk_applycopy of the deny-list had.NP-04 is unverified on any platform. Nothing has run
rc.exeagainst the generated.rcor read the embedded version back. This PR exists to make that gate run for the first time.🤖 Generated with Claude Code
https://claude.ai/code/session_01W6BGY2dFUSujZeS3vTDikq