Skip to content

Use what the handshake negotiated - #481

Closed
bar-capsule wants to merge 1 commit into
refimpl/adapters-05-session-gatefrom
refimpl/adapters-06-negotiated
Closed

bar-capsule wants to merge 1 commit into
refimpl/adapters-05-session-gatefrom
refimpl/adapters-06-negotiated

Conversation

@bar-capsule

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

Copy link
Copy Markdown
Collaborator

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

What changed

  • A ServerHello is checked once against the ClientHello it answers: major version, methods_evaluated, selected_transport, timeouts, on_decision_failure and profiles_accepted. One that fails a check is not a negotiation.
  • The ClientHello offers one transport, the Guardian URL's scheme. Each step waits for its method's negotiated timeout, and that timeout bounds the whole exchange, including a response that arrives a few bytes at a time. Redirects are not followed.
  • ACS_REQUIRE_PROFILES names profiles the Guardian must accept; a claimed profile the Guardian leaves out is audited once as profile_not_accepted.

Ariel's findings this answers

Where this differs from Ariel's proposal

  • Negotiated keeps the verified ServerHello, checked once by core.server_hello_problem, rather than a typed value. The timeouts live on FailPosture (timeout_for), method_evaluated reads the evaluated methods, and Negotiated lives in acs.client.
  • profiles_accepted follows Ariel's suggestion as written: missing continues, an unclaimed profile invalidates the ServerHello, a profile in ACS_REQUIRE_PROFILES left out refuses, and acs-core left out continues with profile_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

  • Reference implementation or adapter

I tested this

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

  • 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

Each step waits at most its negotiated timeout, follows no redirect, and a ServerHello that fails its checks negotiates nothing.

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 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 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

@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-06-negotiated 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