Skip to content

MCP suggest_improvement: target directory writability not validated before lock #1921

Description

@Widthdom

Summary

ExecuteSuggestImprovement (McpToolHandlers.cs:1900-1903) calls Directory.CreateDirectory(cdidxDir) and then proceeds straight into SuggestionStore.TryAddAndSubmit(...), which acquires a file lock inside that directory. If .cdidx/ already exists but is owned by a different user, has restrictive permissions, or sits on a read-only mount, the directory creation succeeds (it's a no-op when the dir exists) but the subsequent file-lock attempt fails deep inside SuggestionStore.WithFileLock with a generic "failed to acquire lock" / "access denied" message that gives the AI / user no actionable signal. Also, when Path.GetDirectoryName(_dbPath) returns null/empty (e.g. for a bare-filename DB path), cdidxDir falls back to .cdidx in the current process directory, which may not be the directory the user expects.

Where

Suggested approach

(1) After Directory.CreateDirectory, write-and-delete a small probe file (e.g. .cdidx/.write_probe) to verify writability before entering the lock path. On failure, return a structured tool error such as "cannot write to .cdidx directory <resolved-path>; check permissions". (2) If _dbPath lacks a directory component, resolve it via Path.GetFullPath and log the resolved location so the user can confirm which .cdidx is being targeted. (3) Surface the resolved cdidxDir path in both success and failure responses for diagnostic clarity. (4) Document the write-permission requirement in the suggest_improvement tool annotation. (5) Add a regression test where .cdidx/ exists but chmod 0500 denies writes; assert the tool fails with the structured permission error. (6) Cross-link with #1614, #1460, #1431.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions