Skip to content

Lock session state, refuse a second install, and audit MCP use - #483

Closed
bar-capsule wants to merge 1 commit into
refimpl/adapters-07-round-tripfrom
refimpl/adapters-08-session-state
Closed

bar-capsule wants to merge 1 commit into
refimpl/adapters-07-round-tripfrom
refimpl/adapters-08-session-state

Conversation

@bar-capsule

@bar-capsule bar-capsule commented Nov 3, 2026 •

Copy link
Copy Markdown
Collaborator

Stacked on #482. Next: #484. The full stack is listed in #22.

What changed

  • Locking. Every session-state change is one read-modify-write under an exclusive lock on a sidecar file (flock; msvcrt on 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.
  • Duplicate installs. acs-claude-code-wire and acs-cursor-wire refuse 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 same request_id, which §10.3 replay protection rejects.
    • Claude Code: user, project, local and managed settings, and the hooks of each installed plugin that enabledPlugins turns on for the project (highest scope wins, else the plugin's defaultEnabled): its hooks/hooks.json and every hook file or inline group its manifest declares.
    • Cursor: enterprise, project and user hooks.json.
    • Cursor also runs the Claude Code hooks in user, project and local settings by default, so each tool checks the other's files for those. --cursor-third-party-imports-off skips that check when Cursor's import setting is off.
    • The check covers the project whose sessions load the file being wired (the project holding a .claude or .cursor file, or the working directory for a user-level one), and each project resolves enabledPlugins from its own settings, so the result is the same from any working directory.
  • MCP auditing. A signed session claims acs-core, which needs protocols/MCP/* for MCP. The first MCP tool call such a session makes records profile_claim_degraded once: Claude Code's mcp__* tools, and Cursor's MCP hooks or its tool hooks for MCP:<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's defaultEnabled. NAT has no MCP detection. Each session leaves one .lock file.

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

  • Reference implementation or adapter

I tested this

At this slice's top (81841cc4), with claude removed from PATH and ACS_LIVE_CLAUDE and ACS_SPEC_DIR unset: 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

  • Commits are signed off with git commit -s (required by the DCO)
  • Prose follows STYLE.md
  • No secrets, tokens, or internal URLs in the diff

Security

Parallel hooks keep each other's session-state updates, and a second ACS install is refused.

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 drk-ram left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@rocklambros

Copy link
Copy Markdown
Collaborator

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.

@rocklambros rocklambros closed this Nov 5, 2026
@github-project-automation github-project-automation Bot moved this from In progress to Done in ACS Project Tracker Nov 5, 2026
@bar-capsule
bar-capsule deleted the refimpl/adapters-08-session-state branch November 8, 2026 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants