Harden subprocess usage in sync_service.py and expand SECURITY.md

[groksrc]

Summary

Recent MCP ecosystem reporting (CSO Online, OX Security) has drawn attention to unsafe subprocess patterns in MCP-adjacent code. The article's primary concern is client-side (MCP clients spawning arbitrary commands from config), which doesn't affect us directly — Basic Memory is a server. The recommended install (uvx basic-memory mcp) is also clean.

However, a pass through our subprocess usage turned up two small hardening items worth addressing proactively. Neither is known to be exploitable today, but both are exactly the kind of pattern the OX Security writeup flags as an anti-pattern, and fixing them now keeps us clean as the threat model around MCP evolves.

Items

1. Replace create_subprocess_shell with create_subprocess_exec in sync_service.py

Two call sites interpolate a Path into a shell string:

• src/basic_memory/sync/sync_service.py:1533 — _quick_count_files:

  await asyncio.create_subprocess_shell(
      f'find "{directory}" -type f | wc -l', ...
  )

• src/basic_memory/sync/sync_service.py:1586 — _scan_directory_modified_since:

  await asyncio.create_subprocess_shell(
      f'find "{directory}" -type f -newermt "{since_date}"', ...
  )

directory is a project path from config, not from MCP tool input, so this isn't reachable via prompt injection today. But a project path containing a double-quote or $(...) would break out of the quoting, and "project paths come from trusted config" is the kind of assumption that ages poorly.

Proposed fix: switch both to create_subprocess_exec with arg lists, and do the counting in Python (len(stdout.splitlines())) instead of piping through the shell:

process = await asyncio.create_subprocess_exec(
    "find", str(directory), "-type", "f",
    stdout=asyncio.subprocess.PIPE,
    stderr=asyncio.subprocess.PIPE,
)
stdout, _ = await process.communicate()
return len(stdout.splitlines())

Same pattern for the -newermt variant. No behavior change expected; just removes the shell from the loop.

2. Expand SECURITY.md

SECURITY.md is currently a placeholder. Given Basic Memory runs locally with filesystem access and sits in the MCP ecosystem that's now getting security scrutiny, it would be useful to document:

• The recommended install pattern (uvx basic-memory mcp) and why config values shouldn't be arbitrary shell commands — this is a client concern, not ours, but worth pointing users at
• That the server enforces project boundaries via validate_project_path() (resolves + is_relative_to() check) on all filesystem-touching tools
• Link to the OX Security / CSO writeups so users and enterprise evaluators understand the client-side threat model

This is more about having a written answer ready for enterprise conversations than fixing anything broken.

Out of scope

• Nothing on the MCP tool input side needs changing — path traversal is already defended via validate_project_path() across read_content, edit_note, write_note, delete_note, etc.
• auto_update.py already does the right thing (list-form args, stdin=DEVNULL, hardcoded commands).
• file_utils.py formatter invocation already uses shlex.split + create_subprocess_exec.

Context

Surfaced while reviewing the CSO Online article on MCP STDIO RCE patterns in light of our codebase. Filing for tracking rather than as a response to an active incident.

[claude]

Claude finished @groksrc's task in 2m 58s —— View job • claude/issue-760-20260420-1327 • Create PR ➔

---

Security Hardening Implementation

Triage: Enhancement | Priority: Medium | Component: sync, docs | Complexity: Simple
Labels added: enhancement, documentation

• Analyze issue and gather context
• Read sync_service.py to understand the subprocess call sites
• Read SECURITY.md current state
• Replace create_subprocess_shell with create_subprocess_exec in both call sites
• Expand SECURITY.md with security documentation
• Commit and push changes (1ffb232)
• Add labels to issue

Changes Made

src/basic_memory/sync/sync_service.py

Both call sites now use create_subprocess_exec with explicit arg lists:

• _quick_count_files (line 1533): find "{directory}" -type f | wc -l → create_subprocess_exec("find", str(directory), "-type", "f", ...) + len(stdout.splitlines())
• _scan_directory_modified_since (line 1588): find "{directory}" -type f -newermt "{since_date}" → create_subprocess_exec("find", str(directory), "-type", "f", "-newermt", since_date, ...)

No behavior change; shell removed from the execution path.

SECURITY.md

Expanded from placeholder to a full threat model document covering:

• Path validation via validate_project_path() / is_relative_to()
• Subprocess hardening (exec vs shell)
• The MCP client-side RCE pattern (with links to the OX Security / CSO writeups)
• Secure configuration checklist

|