Repository navigation
Lock session state, refuse a second install, and audit MCP use - #483
bar-capsule wants to merge 1 commit into
Conversation
Session state changes are one locked read-modify-write, so parallel hooks keep each other's updates. The wire tools refuse a second ACS install, and a session's first MCP call records profile_claim_degraded. Signed-off-by: Bar Kaduri <bar@capsule.security>
drk-ram
left a comment
There was a problem hiding this comment.
Reviewed commit 81841cc44e1d72c879793bc84fda7e70e364d562 against its stated stacked base.
P2: include readers in the Windows state-publication contract.
acs/client/_store.py:81-100 serializes writers, but load_session_state still opens the JSON file without the sidecar lock. On Windows, that ordinary read handle does not share delete access. A concurrent _write therefore fails at os.replace, and the except OSError: return None silently discards the step or turn update.
I reproduced this against this exact commit with a fresh state directory:
record_step("session", "first")
with _store._session_state_path("session").open():
record_step("session", "second")The replace raised PermissionError / WinError 5 internally; the final state contained only first. This is the same handle type used by load_session_state, so concurrent hook reads can trigger it. Please coordinate readers with publication, or use an appropriate sharing/retry strategy, and add a reader/writer regression test rather than only writer/writer tests.
The focused checks below passed, but do not cover that interleaving. Nonblocking test-isolation note: the duplicate-wire fixtures patch only HOME; Windows Path.home() uses USERPROFILE, so patch Path.home directly to avoid inspecting the real profile. The broader selection had 41 passed and 23 failures across those fixtures and platform-specific command/timeout assumptions.
Local validation (from adapters, Python 3.11):
python -m pytest tests/client/test_store.py tests/common/test_wire_duplicates.py::CopyScanner tests/integrations/nat/test_emission.py -q --tb=short
Result: 11 passed, 40 warnings, 4 subtests passed in 29.12s
|
Closing this slice because PR #22 (the reference adapters for Claude Code, Cursor and NAT) merged into integration as 2630ec2, and that merge carries every slice in this stack. The top of the stack had the same files as PR #22's final commit, so nothing in this slice was left behind. Thanks, Bar, for cutting the adapters into pieces this size, because it made the review possible. |
Stacked on #482. Next: #484. The full stack is listed in #22.
What changed
flock;msvcrton Windows), published through a temp file of its own. No lock is held during a Guardian round trip. Before, 3 to 11 of 200 parallel steps survived.acs-claude-code-wireandacs-cursor-wirerefuse to wire when another hook configuration the host loads already runs an ACS adapter, and list where. Two copies send each event twice, and both requests for one tool call carry the samerequest_id, which §10.3 replay protection rejects.enabledPluginsturns on for the project (highest scope wins, else the plugin'sdefaultEnabled): itshooks/hooks.jsonand every hook file or inline group its manifest declares.hooks.json.--cursor-third-party-imports-offskips that check when Cursor's import setting is off..claudeor.cursorfile, or the working directory for a user-level one), and each project resolvesenabledPluginsfrom its own settings, so the result is the same from any working directory.acs-core, which needsprotocols/MCP/*for MCP. The first MCP tool call such a session makes recordsprofile_claim_degradedonce: Claude Code'smcp__*tools, and Cursor's MCP hooks or its tool hooks forMCP:<tool>.Ariel's findings this answers
Known limits. The Windows lock path and Windows system paths are untested. The wire tools leave out Cursor's team and plugin hooks, Claude Code plugins loaded with
--plugin-dir, and a marketplace entry'sdefaultEnabled. NAT has no MCP detection. Each session leaves one.lockfile.Which issue does this implement
Part of #134
Base branch
Stacked on #482, not on
integration. It lands through #22; see the stack in #22.Type of change
I tested this
At this slice's top (
81841cc4), withclauderemoved from PATH andACS_LIVE_CLAUDEandACS_SPEC_DIRunset: 442 passed and 3 expected skips with the NAT extra, 392 passed and 3 expected skips without it. The skips are the two opt-in live Claude Code tests and the manual Cursor procedure. CI passes on this PR, including the installed-wheel check on Python 3.11 and 3.12.Checklist
git commit -s(required by the DCO)Security
Parallel hooks keep each other's session-state updates, and a second ACS install is refused.