fix(server): guard project_root writes against mock/relative coercion (TAP-4573)#197
Conversation
… (TAP-4573) Two production write paths mkdir/write under Path(settings.project_root) with no guard: agent_identity._write_uuid (via get_stable_agent_id) and server_helpers.write_session_start_marker. ~70 test sites pass a bare MagicMock(); os.fspath(MagicMock().project_root) coerces to the relative path 'MagicMock/mock.project_root/<id>', so full-suite runs created real agent.id / session-start-marker trees in the pytest CWD (the repo root), invisible to git status because .tapps-mcp/ is gitignored at any depth. Add a shared guard is_real_writable_root() in tapps_core.agent_identity that accepts only absolute filesystem paths. Both write paths short-circuit when it fails: get_stable_agent_id returns a valid in-memory id (no persistence), write_session_start_marker returns early. No-op for real deployments (roots are always absolute); no edits to the 70 test sites. Verified: full package unit run reports 'no MagicMock leak' and 7368 passed (the single ordering-flake failure in test_cli_memory is pre-existing and unrelated — passes in isolation and has zero linkage to the changed code). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57ff94c44e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| raw = os.fspath(project_root) # type: ignore[arg-type] | ||
| except (TypeError, ValueError): | ||
| return False | ||
| return Path(raw).is_absolute() |
There was a problem hiding this comment.
In the Claude Code configuration path the project root is intentionally emitted as TAPPS_MCP_PROJECT_ROOT="." (packages/tapps-mcp/src/tapps_mcp/distribution/setup_generator.py:380-383), and load_settings() preserves that env value as Path('.') without resolving it (packages/tapps-core/src/tapps_core/config/settings.py:1517-1518). With this new absolute-only predicate, real Claude Code sessions take the non-persistent fallback in get_stable_agent_id(), so repeated calls generate different UUIDs instead of reading/writing .tapps-mcp/agent.id; that changes X-Agent-Id/Hive registration on every request. The guard should resolve legitimate relative roots (or normalize settings earlier) while still rejecting mock paths.
Useful? React with 👍 / 👎.
1 participant
What
Harden the two production write paths that mkdir/write under
Path(settings.project_root)so a non-real (mock- or relative-coerced)project_rootcan never create a real filesystem tree.Root cause
agent_identity._write_uuid(viaget_stable_agent_id) andserver_helpers.write_session_start_markerbothmkdir(parents=True)underproject_rootwith no guard. ~70 test call sites pass a bareMagicMock();os.fspath(MagicMock().project_root)coerces to the relative pathMagicMock/mock.project_root/<id>(verified empirically), so every full-suite run created realagent.id/.session-start-markertrees in the pytest CWD (the repo root) — 1,848 dirs accumulated, invisible togit statusbecause.tapps-mcp/is gitignored at any depth.Fix
Add a shared guard
is_real_writable_root(project_root)intapps_core.agent_identityaccepting only absolute filesystem paths (os.fspath→Path.is_absolute()). Both write paths short-circuit when it fails:get_stable_agent_idreturns a valid in-memory id (no persistence).write_session_start_markerreturns early.No-op for real deployments (roots are always absolute). No edits to the 70 test sites — the guard stops the leak at the single production write path.
Tests
no MagicMock leak. The one ordering-flake failure (test_cli_memory::test_search_json_output) is pre-existing and unrelated — passes in isolation and in its own file, and has zero linkage to the changed code.Closes TAP-4573.
🤖 Generated with Claude Code