fix(platform): fall back to USERPROFILE when HOME is unset

[bingh0]

Summary

Fixes #77 (Windows: codebase-memory-mcp install fails with "error: HOME not set")

• Add cbm_get_home_dir() helper in platform.{h,c} that tries HOME first, then USERPROFILE
• Replace all 17 raw getenv("HOME") callsites across 9 files with the new helper
• Improve error messages to mention USERPROFILE as the Windows alternative

Context

On Windows, HOME is not set by default — the system uses USERPROFILE instead. This caused all CLI commands (install, uninstall, update, config) to fail immediately. Confirmed by @indy-singh in #77.

Note: The v0.5.4 release includes dual MCP config writes (~/.claude/.mcp.json + ~/.claude.json for Claude Code >=2.1.80, fixes #69), but this change doesn't appear to be on main yet. This PR does not include that fix to avoid duplication.

Test plan

• scripts/build.sh — clean compile, no warnings
• scripts/test.sh — all tests pass (3 pre-existing snippet_fuzzy failures unrelated to this change)
• Manual: HOME= USERPROFILE=/tmp ./build/c/codebase-memory-mcp install --dry-run -y — verifies fallback works
• Manual: HOME= USERPROFILE= ./build/c/codebase-memory-mcp install --dry-run -y — verifies improved error message

🤖 Generated with Claude Code

[bingh0]

QA Round 1 Report

Verdict: PASS — No issues found.

What was reviewed

• All 9 changed files, the new cbm_get_home_dir() helper, and include correctness
• Build, test suite, and manual edge-case verification

Findings

Correctness

• cbm_get_home_dir() correctly tries HOME first, then USERPROFILE, returns NULL if neither is set. The h[0] check correctly rejects empty strings (e.g. HOME=).
• All 17 former getenv("HOME") callsites are replaced — confirmed via grep, only the one inside cbm_get_home_dir() itself remains.
• /tmp fallbacks consistently replaced with cbm_tmpdir() which is cross-platform (uses TEMP/TMP on Windows).
• Error messages consistently updated to mention USERPROFILE.

Includes

• All files that call cbm_get_home_dir() include platform.h.
• All files that call cbm_tmpdir() include compat.h.

Edge cases

• HOME= USERPROFILE=/tmp ./build/c/codebase-memory-mcp install --dry-run -y — correctly uses /tmp as home.
• HOME= USERPROFILE= ./build/c/codebase-memory-mcp install --dry-run -y — correctly prints improved error message.

Build & tests

• Clean compile with no warnings.
• All tests pass except 3 pre-existing snippet_fuzzy failures (unrelated, documented in PR).

No issues to fix. The PR is clean and ready to merge.

🤖 Generated with Claude Code

[bingh0]

QA Round 2 Report

Build & Tests

• Build: Clean compile, zero warnings with -Wall -Wextra -Werror
• Tests: All pass. The 3 snippet_fuzzy failures are pre-existing and unrelated to this PR
• Manual test HOME= USERPROFILE=/tmp ./build/c/codebase-memory-mcp install --dry-run -y — correctly falls back to /tmp as home
• Manual test HOME= USERPROFILE= ./build/c/codebase-memory-mcp install --dry-run -y — correctly prints error: HOME not set (use USERPROFILE on Windows)

Findings

F1 — Test files still use raw getenv("HOME") (low severity)

tests/test_ui.c has 5 instances and tests/test_integration.c has 1 instance of raw getenv("HOME") without USERPROFILE fallback. These tests will break on a Windows CI runner where HOME is unset. They should use cbm_get_home_dir() for consistency.

Files:

• tests/test_ui.c:34,56,83,112,148 — saves/restores HOME for config tests
• tests/test_integration.c:108 — builds DB path with /tmp fallback

F2 — Missing <stdlib.h> in platform.c (nit)

cbm_get_home_dir() calls getenv() which is declared in <stdlib.h>, but platform.c doesn't directly include it. It compiles because getenv is pulled in transitively through other headers. Adding an explicit #include <stdlib.h> would make the dependency clear and guard against future header refactors.

F3 — No issues found in core logic

• The cbm_get_home_dir() function correctly checks h[0] to reject empty strings (not just NULL)
• All 17 callsites in src/ were migrated — grep confirms only platform.c itself still uses raw getenv("HOME")
• Fallback to cbm_tmpdir() instead of hardcoded "/tmp" is correct for Windows
• The function is placed outside the #ifdef _WIN32 guard, so it compiles on all platforms
• NOLINTNEXTLINE annotations for concurrency-mt-unsafe are correctly placed

Verdict

F1 should be fixed (consistency + future Windows CI). F2 is a nit, up to you. Core logic is solid.

[bingh0]

QA Round 2 Report - Deep Review

Deep review focusing on thread safety, pointer lifetime, path separators, and edge cases.

1. Thread Safety — SAFE

All cbm_get_home_dir() callsites are serialized:

• CLI commands run on the main thread
• MCP server processes requests in a single event loop
• HTTP server handlers are serialized by Mongoose's mg_mgr_poll()

2. Pointer Lifetime — SAFE

All callsites consume the returned const char * immediately in snprintf() or strcmp(). No pointer is stored across function boundaries or delayed dereferences. No risk of invalidation by setenv().

3. Path Separators — SAFE (cosmetic inconsistency)

On Windows, USERPROFILE returns backslash paths (e.g., C:\Users\Alice). The code concatenates with forward slashes: "%s/.cache/codebase-memory-mcp". This produces mixed-separator paths like C:\Users\Alice/.cache/codebase-memory-mcp. Windows APIs accept both separators, so this is functionally safe — a future PR could normalize if desired.

4. Edge Cases — PROPERLY HANDLED

• h[0] check correctly rejects empty-string env vars
• All callsites guard against NULL return with cbm_tmpdir() or error message

5. Completeness — VERIFIED

• All production getenv("HOME") calls converted (only platform.c itself remains, inside the helper)
• test_ui.c raw getenv("HOME") calls are correct — they save/restore the env var itself for test isolation, not resolve the home directory
• test_integration.c was fixed in QA round 1

Verdict

No issues found. PR is ready to merge.

[bingh0]

QA Round 3 Report

Scope

Full review of all 10 changed files + security audit of the new cbm_get_home_dir() helper and all callsites.

---

Correctness — PASS

• cbm_get_home_dir() logic is correct: checks HOME first, falls back to USERPROFILE, rejects empty strings via h[0], returns NULL when both are absent.
• All 17 production getenv("HOME") callsites migrated — confirmed only platform.c:225 (inside the helper itself) remains.
• All /tmp fallbacks replaced with cbm_tmpdir() which is cross-platform (TEMP/TMP on Windows, /tmp on Unix).
• Error messages consistently updated to mention USERPROFILE.
• All files that call cbm_get_home_dir() include platform.h. All files that call cbm_tmpdir() include compat.h.
• Function is placed outside the #ifdef _WIN32 / #endif guard, so it compiles on all platforms. ✓

Thread Safety — PASS

• getenv() is marked concurrency-mt-unsafe but all callsites are serialized: CLI commands run on main thread, MCP server uses single event loop, HTTP handlers are serialized by Mongoose's mg_mgr_poll().
• No pointer is stored across function boundaries — all callsites consume the return immediately in snprintf() or strcmp(). No risk of invalidation by setenv().

Pointer Lifetime — PASS

• cbm_get_home_dir() returns a pointer to the environment's internal storage (same as raw getenv()). No callsite stores this pointer persistently or passes it to async callbacks. All usage is immediate snprintf() into stack buffers. No lifetime regression.

---

Security Review

S1 — Pre-existing: No sanitization of name/project query params used in file paths (informational, not introduced by this PR)

Multiple HTTP handlers (handle_adr_get, handle_adr_save, handle_delete_project, handle_project_health, handle_layout) take a user-supplied name or project query parameter and interpolate it directly into a file path:

snprintf(db_path, sizeof(db_path), "%s/.cache/codebase-memory-mcp/%s.db", home, name);

A name value like ../../etc/passwd would construct a path outside the intended directory. While cbm_store_open_path() would likely fail on a non-SQLite file, handle_delete_project calls unlink(db_path) which could delete arbitrary files. The .db suffix provides some mitigation (path becomes ../../etc/passwd.db), but it's worth noting.

This is pre-existing and NOT introduced by this PR, but the PR does change the home portion of these paths, so flagging for awareness. A future PR could add a check that name contains no / or \ characters.

S2 — Pre-existing: handle_browse allows arbitrary directory listing (informational)

The /api/browse?path=X endpoint lists any directory on the filesystem. The server binds to 127.0.0.1 only (line 11), which limits exposure, but any local process can enumerate the filesystem. This is pre-existing and by design (it's a file picker).

S3 — Pre-existing: Unescaped JSON string interpolation in handle_browse (informational)

pos += snprintf(buf + pos, ..., "\"%s\"", ent->d_name);

Directory names containing " or \ would produce malformed JSON. Minor, pre-existing, not introduced by this PR.

S4 — cbm_get_home_dir() trusts environment variables — ACCEPTABLE

The function trusts HOME and USERPROFILE without validation. An attacker who can set environment variables already has process-level access, so this is standard practice and matches the behavior of getenv("HOME") it replaces. No regression.

S5 — No new attack surface introduced — PASS

This PR is a pure mechanical refactor: it centralizes existing getenv("HOME") calls into a helper and adds a USERPROFILE fallback. No new inputs, no new endpoints, no new file operations. The security posture is unchanged or marginally improved (consistent fallback behavior).

---

Minor Observations

M1 — test_ui.c still uses raw getenv("HOME") (5 instances)

These save/restore HOME for test isolation via setenv()/unsetenv(). This is intentional — they're manipulating the env var itself, not resolving the home directory. No action needed (correcting QA round 2's F1 which flagged these).

M2 — Mixed path separators on Windows (cosmetic)

USERPROFILE returns backslash paths (e.g., C:\Users\Alice), but paths are concatenated with forward slashes (%s/.cache/...). Windows APIs accept mixed separators, so this is functionally safe. Could normalize in a future PR if desired.

---

Verdict

PASS — No issues introduced by this PR. The change is a clean, mechanical refactor that correctly centralizes home directory resolution with proper cross-platform fallback. No new security surface. Pre-existing security observations (S1–S3) are noted for future hardening but are outside the scope of this PR.

🤖 Generated with Claude Code