Repository navigation
Fix what running the built wheel found - #487
bar-capsule wants to merge 8 commits into
Conversation
A blocked beforeSubmitPrompt exits 0 with continue false and user_message, so Cursor shows the message. Signed-off-by: Bar Kaduri <bar@capsule.security>
The hook that opens a session holds a startup lock while it handshakes and sends sessionStart; the session's other hooks wait and reuse the stored decision. Signed-off-by: Bar Kaduri <bar@capsule.security>
The NAT README and mapping page state that only a run that returns sends agentResponse and sessionEnd. Signed-off-by: Bar Kaduri <bar@capsule.security>
acs-nat-wire adds acs_guardian to every function's middleware and --check reports any function without it, so nested calls reach the Guardian. Signed-off-by: Bar Kaduri <bar@capsule.security>
A denied or refused sessionStart refuses the session. The outcome is kept in a signed startup record written under the startup lock. Signed-off-by: Bar Kaduri <bar@capsule.security>
A hook whose ACS_REQUIRE_PROFILES is not met denies its own step before sessionStart; a later hook of the session starts it. Signed-off-by: Bar Kaduri <bar@capsule.security>
Both tests store their session records in a temporary directory. Signed-off-by: Bar Kaduri <bar@capsule.security>
One network mock covers both threads, so no mock outlives the test. Signed-off-by: Bar Kaduri <bar@capsule.security>
drk-ram
left a comment
There was a problem hiding this comment.
Reviewed at head e42f2771dc74915e00cb36b9b849fe8558a2507d (stacked on #486, base refimpl/adapters-11-examples-and-imports, which is c59c00e). I read the full diff of the 28 files and ran the adapters suite. I am approving. Three notes follow, none of them blocking.
What I checked in the startup protocol (client/_handshake.py).
open_session(start=…)takes the per-session lock, re-reads the signed session record inside the lock, then writes a signed.startedclaim withos.link().link()fails if any startup record exists, so exactly one process sendssessionStart. The outcome then replaces the claim throughos.replace(). Every other hook either finds the outcome on its unlocked fast path or waits on the lock and then follows what was recorded. That is the right shape, and it removes the race behind the handshake andsessionStartthat two Cursor hooks used to send.- The two record kinds cannot stand in for each other.
_verified()checks a distinct marker (acs_session_recordoracs_session_startup) plus the session id and Guardian URL under the HMAC, and_read_record()refuses a session record that carriesstart_failure. Config.gate()turns a recordedstart_failureinto a fail-closed resolution only when the negotiated posture is deny. A deny, a refusal or a non-allowsessionStartdisposition refuses the session for every later hook, and a hook whose ownACS_REQUIRE_PROFILESrefuses leaves the start to the next hook. The new tests cover each of those, includingtest_the_session_start_decision_governs_every_hook_of_the_sessionas a host × case matrix.- Claude Code emits the opening hook's
sourceintosessionStart, and a laterSessionStartwithresumeorcompactsends nothing.test_a_resumed_or_compacted_claude_session_keeps_its_one_session_startpins that. - Cursor's
beforeSubmitPromptblock now exits 0 with{"continue": false, "user_message": …}. Cursor's hooks page says exit 2 is the block, and it documents the{continue, user_message}output for this hook, so the new shape is the documented one. See the first note below. acs-nat-wire --checknow flags every function with noacs_guardian, and--writeadds it. A call to a function runs only that function's own middleware, so a function that omits it bypasses the Guardian, andtest_wire.pycovers both directions.
Verification (Windows, Python 3.11.9, uv sync --frozen --extra nat --group test).
The harness does not run on Windows as it stands: launch_guardian() calls select.select() on a pipe, and example_guardian.py imports fcntl. I used a thread-based line read and an fcntl stub in my working copy only, and committed nothing. With that, pytest -q on e42f277 gives 452 passed, 37 failed and 3 skipped. I ran the same suite with the same two edits on the base c59c00e: 432 passed, 38 failed, 3 skipped. Every failure on e42f277 is also in the base's failure list. They are POSIX-only assertions: 0600/0700 modes, symlink rejection, /-style paths in test_wire_duplicates.py, and connect-refused timing on 127.0.0.1:1. None is in the code this PR touches. All of the new startup tests pass: the parallel-startup, one-refusal and unanswered-handshake tests, the resume/compact test, and the decision matrix for both hosts. I did not run the Linux-only lint or wheel checks.
Notes (non-blocking).
- Cursor no longer exits 2 on a blocked prompt, so a Cursor release that stopped honouring
continue: falseonbeforeSubmitPromptwould let the prompt through with nothing to catch it. Documented output and the live run are good evidence today. The live check indocs/cursor/live_verification.mdis the right guard, and it would be worth keeping it in the release checklist. _locked()waits differently by platform. POSIXflock(LOCK_EX)waits indefinitely. On Windowsmsvcrt.locking(LK_LOCK)retries about ten times at one-second intervals and then raises, whichopen_sessionturns intosession_startup_lock_unavailable, a refused session. A handshake (5 s default) followed by asessionStart(5 s default) can run past ten seconds when the Guardian is slow, so the hooks waiting behind the opener would refuse on Windows only. A deadline derived from the negotiated timeouts, applied on both platforms, would make the two behave the same.- On this PR alone, a hook that the host kills between the claim and the outcome leaves the record
pending, and every later hook of that session getssession_start_incompleteas a refusal, even underon_decision_failure: proceed. I see #489 settles that case under the session's posture, so this is fine given the stack lands through #22. It is worth saying in #22 that this slice must not land alone.
Approving e42f2771.
|
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 #486. Next: #488. The full stack is listed in #22.
What changed
From running the wheel in Claude Code, Cursor and NAT:
user_message).sessionStartonce, before its other steps: the first hook process holds a per-session startup lock while it starts the session, and the others wait.sessionStartdecision is enforced: a deny or a refusal refuses the session.acs-nat-wirewires every NAT function, and--checkrejects one without the middleware.Eight commits.
Which issue does this implement
Part of #134
Base branch
Stacked on #486, not on
integration. It lands through #22; see the stack in #22.Type of change
I tested this
At this slice's top (
e42f2771), withclauderemoved from PATH andACS_LIVE_CLAUDEandACS_SPEC_DIRunset: 475 passed and 3 expected skips with the NAT extra, 408 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, includinglint-importsand the installed-wheel check on Python 3.11 and 3.12.Checklist
git commit -s(required by the DCO)Security
A denied
sessionStartrefuses the session,sessionStartgoes out once before other steps, and every NAT function is wired.