Skip to content

fix(mcpinit): atomic settings write, allowlist sanitizer, hook consistency#111

Merged
wcatz merged 1 commit intomainfrom
fix/mcpinit-security-hardening
Mar 23, 2026
Merged

fix(mcpinit): atomic settings write, allowlist sanitizer, hook consistency#111
wcatz merged 1 commit intomainfrom
fix/mcpinit-security-hardening

Conversation

@wcatz
Copy link
Copy Markdown
Owner

@wcatz wcatz commented Mar 23, 2026 •
edited by coderabbitai Bot
Loading

Summary

  • Atomic settings write: Replace os.WriteFile with temp file + os.Rename for POSIX atomicity on ~/.claude/settings.json. Backup failure is now a hard error.
  • Allowlist sanitizer: Switch sanitizeName from denylist (strip backticks/newlines) to allowlist [a-zA-Z0-9 -_.] — prevents prompt injection via project names interpolated into MEMORY.md files that Claude Code auto-loads.
  • Hook consistency: Add ghost_list_projects as step 1 in SessionStart hook output to match MEMORY.md redirect instructions.

Test plan

  • go vet ./... clean
  • go test ./internal/mcpinit/... passes
  • Updated test for new hook output

Summary by CodeRabbit

  • Bug Fixes

    • Improved project name validation to handle special characters and edge cases more reliably
    • Enhanced settings file handling with automatic backups and safer persistence

  • Documentation

    • Updated session start instructions to prioritize project discovery as the first recommended step

@wcatz
fix(mcpinit): atomic settings write, allowlist sanitizer, hook consis…
9e3b426 
…tency

- settings.go: replace os.WriteFile with temp file + os.Rename for
  POSIX atomicity; make backup failure a hard error instead of silent
- init.go: switch sanitizeName from denylist (strip backticks/newlines)
  to allowlist [a-zA-Z0-9 -_.] to prevent prompt injection via project
  names interpolated into MEMORY.md
- hook.go: add ghost_list_projects as step 1 in SessionStart hook
  output, matching the MEMORY.md redirect instructions
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 23, 2026 •
edited
Loading

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ac3458f5-d73d-462c-bce2-2d47ec018ecc

📥 Commits

Reviewing files that changed from the base of the PR and between c993e40 and 9e3b426.

📒 Files selected for processing (4)
  • internal/mcpinit/hook.go
  • internal/mcpinit/init.go
  • internal/mcpinit/init_test.go
  • internal/mcpinit/settings.go
📝 Walkthrough

Walkthrough

The PR updates session-start instructions to prioritize ghost_list_projects, refactors the sanitizeName function to use an allowlist instead of blacklist for safety, upgrades settings persistence with atomic writes and backup creation, and updates related tests to reflect the new behavior.

Changes

Cohort / File(s) Summary
Session-start Instructions
internal/mcpinit/hook.go, internal/mcpinit/init_test.go		 Reordered session-start guidance to call ghost_list_projects first (step 1), followed by ghost_project_context (step 2) and ghost_memory_save (step 3). Test assertion updated to verify new instruction ordering.
Input Sanitization
internal/mcpinit/init.go		 Changed sanitizeName from blacklist (filtering specific chars) to allowlist (only ASCII letters, digits, -, _, space, .). Returns "unknown" for empty results after 64-char truncation. Prevents prompt injection in MEMORY.md.
Settings Persistence
internal/mcpinit/settings.go		 Implemented atomic file writes with upfront directory existence check, .bak backup creation, temp-file intermediate, explicit 0600 permissions, and atomic rename. Enhanced error handling for all I/O operations with proper cleanup on failures.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • PR #110: Introduces the mcpinit codepaths (hook.go, init.go, init_test.go, settings.go) that this PR directly modifies and refines.
  • PR #107: Introduces the ghost_list_projects MCP tool that is now prioritized in the session-start instructions updated by this PR.

Poem

🐰 A rabbit hops through safer code today,
With backups tucked and allowlists in play,
Instructions reordered, each tool in its place,
Atomic writes dancing at a swifter pace! ✨

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/mcpinit-security-hardening

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@wcatz wcatz merged commit bb7ac39 into main Mar 23, 2026
2 of 4 checks passed
@wcatz wcatz deleted the fix/mcpinit-security-hardening branch March 23, 2026 19:17
This was referenced Mar 25, 2026
Merged
Merged
@coderabbitai coderabbitai Bot mentioned this pull request Apr 25, 2026
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@wcatz