Repository navigation
Use what the handshake negotiated - #481
bar-capsule wants to merge 1 commit into
Conversation
The ServerHello is checked against the ClientHello. Each step waits at most its method's negotiated timeout for the whole exchange and follows no redirect. ACS_REQUIRE_PROFILES denies steps when a required profile is not accepted. Signed-off-by: Bar Kaduri <bar@capsule.security>
drk-ram
left a comment
There was a problem hiding this comment.
Reviewed commit c15888fe5621650eb95473f57e677f76c43ea64f against its stated stacked base.
No blocking finding in this slice.
acs/core/_handshake.py:34-63 checks the answer against the actual offer, rather than treating a signed object as sufficient negotiation. The per-method timeout reaches each integration, and the abortable transport bounds slow TLS headers without following redirects. _required_profiles also reapplies the current hook's requirement to a stored negotiation, which is the right separation between session facts and hook policy.
The focused checks below passed on Windows/Python 3.11. The broader selection was not green: 92 passed, 8 failures involving POSIX permission/pipe assumptions, localhost timeout classification, and a timing-sensitive session test. I am not treating those as a clean full-suite result. The PR's test and package checks are green.
Local validation (from adapters, Python 3.11):
python -m pytest tests/core/test_handshake.py tests/client/test_config.py tests/client/test_session.py::Negotiation tests/client/test_exchange.py::TlsDeadline -q --tb=short
Result: 27 passed, 76 subtests passed in 2.64s
|
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 #480. Next: #482. The full stack is listed in #22.
What changed
methods_evaluated,selected_transport, timeouts,on_decision_failureandprofiles_accepted. One that fails a check is not a negotiation.ACS_REQUIRE_PROFILESnames profiles the Guardian must accept; a claimed profile the Guardian leaves out is audited once asprofile_not_accepted.Ariel's findings this answers
profiles_acceptedhalf. The first-MCP-use audit is in Lock session state, refuse a second install, and audit MCP use #483.Where this differs from Ariel's proposal
Negotiatedkeeps the verified ServerHello, checked once bycore.server_hello_problem, rather than a typed value. The timeouts live onFailPosture(timeout_for),method_evaluatedreads the evaluated methods, andNegotiatedlives inacs.client.profiles_acceptedfollows Ariel's suggestion as written: missing continues, an unclaimed profile invalidates the ServerHello, a profile inACS_REQUIRE_PROFILESleft out refuses, andacs-coreleft out continues withprofile_not_accepted.Which issue does this implement
Part of #134
Base branch
Stacked on #480, not on
integration. It lands through #22; see the stack in #22.Type of change
I tested this
At this slice's top (
c15888fe), withclauderemoved from PATH andACS_LIVE_CLAUDEandACS_SPEC_DIRunset: 400 passed and 3 expected skips with the NAT extra, 351 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
Each step waits at most its negotiated timeout, follows no redirect, and a ServerHello that fails its checks negotiates nothing.