fix(session): hold the session boundary a credentialed reset draws - #628
Closed
imran-siddique wants to merge 3 commits into
Closed
fix(session): hold the session boundary a credentialed reset draws#628imran-siddique wants to merge 3 commits into
imran-siddique wants to merge 3 commits into
Conversation
Mirrors agentrust-io/agent-manifest#405, which is merged and where the same approach found a real bug in the COSE decoders on its first run. Scorecard's Fuzzing check does not recognise Hypothesis or Atheris on their own, so Python property tests would not have moved it. ClusterFuzzLite is what it detects, and it runs real coverage-guided fuzzing rather than only satisfying the check. Two targets, on what arrives before anything about it has been verified: fuzz_attestation_parsers.py parse_event_log walks a TCG event log, whose Spec ID header declares digest algorithms and lengths and whose every event declares its own digest count and data length. parse_nv_certify reads a size-prefixed TPM NV certification. fuzz_canonical_json.py the bytes a catalog approval signature covers. The parser target asserts each function fails closed: it returns, or raises the ValueError its module documents. The canonicalizer target asserts a round trip rather than absence of a crash, because the three RFC 8785 bugs found in the sibling agent-manifest canonicalizer were all silent. Standing when added: both parsers already fail closed under a local probe of 12,000 mutated inputs, and the canonicalizer round-tripped 17,918 generated documents while refusing 12,080 as declared, with no invariant violation. These are regression guards, not holes being closed. Carries both fixes agent-manifest paid for: --collect-submodules=email, without which PyInstaller misses the lazy import and libFuzzer reports the runtime ModuleNotFoundError as a crash in the target; and a 45 minute budget, since the oss-fuzz base image build dominates. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XbDBXDWWvMFa7c2jGgyq9t
The build check failed fuzz_attestation_parsers with FileNotFoundError: agentrust_trace/schema/trace-v0.2.json PyInstaller bundles code, not package data. cmcp_verify's import chain reaches agentrust_trace, which loads its JSON schema from inside the package at import time, so the bundled target had no schema to open and the target was reported broken rather than merely failing at runtime. --collect-data for the three packages involved. Same class as the email.mime fix above it: PyInstaller sees only what is statically reachable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XbDBXDWWvMFa7c2jGgyq9t
Found while reviewing the Monotonic Session Sensitivity State draft against
the runtime. A reset is supposed to close one session and open a successor at
the minimum level. Four things let the boundary leak.
A response arriving during a reset raised the successor. The mutation lock
serialised the reset and the response elevation but did not order them, so
whichever coroutine acquired it second won: a response in flight when the reset
landed was applied to the successor and recorded its pre-reset call_id as the
raiser. update_from_inspection() now takes the reset_count observed at call
entry and drops a response whose generation no longer matches. The
discriminator is reset_count and not session_id because upgrade_attestation()
rotates the identifier while deliberately continuing the same session, so a
call in flight across an attestation upgrade must still apply. The previous
concurrency test asserted only that max_sensitivity stayed a member of
SENSITIVITY_ORDER, which every value satisfies.
The reset route accepted the tool-invocation token. POST
/sessions/{id}/reset is not reachable as an MCP tool, but it sat behind the
same CMCP_BEARER_TOKEN as POST /mcp, so an agent host holding its own
credential could clear accumulated sensitivity. The operator interface now
takes CMCP_OPERATOR_TOKEN, required outside dev mode and refused if it equals
the bearer token. Unset, those routes still fall back, so existing deployments
keep working until they set it.
The reset audit entry recorded the sensitivity transition and nothing that
identified the boundary. detail now carries the closed session id, the
successor id, the resulting reset counter and which credential was verified.
detail is inside the canonical body, so those fields are hash-covered.
The chain attributed every entry after a reset to the closed session, and the
successor's identifier appeared nowhere in it. rotate_session_id() moves
attribution after the boundary entry is written.
The closed session's final value is now preserved as a distinct
ClosedSessionRecord rather than overwritten, and the reset response returns it.
Nine tests added. Two suite failures are pre-existing on the base:
test_release_distribution_smoke (version 0.5.0 vs metadata 0.4.0) and
test_tpm_chained_verify (assertion text drift).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CDzMWcn12dszchgrKfXzZY
imran-siddique
requested review from
a team,
carloshvp,
qubeena07 and
zohebk8s
as code owners
September 10, 2026 23:14
Member
Author
|
Superseded by #629, which cherry-picks this onto current main and adds the shared, persistent session-state store. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Found while reviewing the Monotonic Session Sensitivity State patent draft against the runtime. A credentialed reset is supposed to close one session and open a successor at the minimum sensitivity level. Four things let that boundary leak.
A response arriving during a reset raised the successor
The per-session mutation lock serialised the reset and the response elevation but never ordered them, so whichever coroutine acquired it second won. A response in flight when the reset landed was applied to the successor, which had just been initialised to
public, and recorded its pre-resetcall_idas the call that raised it.update_from_inspection()now takes thereset_countobserved at call entry and drops a response whose generation no longer matches, loggingSESSION_RESET_RACE.The discriminator is
reset_countand notsession_idbecauseupgrade_attestation()rotates the identifier while deliberately continuing the same session at its current sensitivity, so a call in flight across an attestation upgrade must still apply. Both cases are tested.The previous concurrency test asserted only that
max_sensitivityremained a member ofSENSITIVITY_ORDER, which every value satisfies.The reset route accepted the tool-invocation token
POST /sessions/{id}/resetis not reachable as an MCP tool, but it sat behind the same singleCMCP_BEARER_TOKENasPOST /mcp, so an agent host holding only its own tool-invocation credential could clear accumulated session sensitivity.The operator interface, session reset and catalog exception, now takes
CMCP_OPERATOR_TOKEN. It must differ fromCMCP_BEARER_TOKENand is required outsideCMCP_DEV_MODE=1(OPERATOR_TOKEN_REQUIRED). Where it is unset those routes still fall back to the bearer token, so an existing single-token deployment keeps working until it sets the new variable.Deployment note: production deployments need
CMCP_OPERATOR_TOKENset before this ships, or startup aborts. Documented indocs/configuration.md.The reset audit entry did not identify the boundary
It recorded the sensitivity transition and nothing else.
detailnow carries the closed session identifier, the successor identifier, the resulting reset counter, and which credential was verified.detailis inside_canonical_body(), so those fields are covered by the entry hash.The chain attributed post-reset entries to the closed session
AuditChainstamps its construction-timesession_id, so every entry written after a reset carried the closed session's identifier and the successor's identifier appeared nowhere in the chain.rotate_session_id()moves attribution to the successor after the boundary entry is written, so the reset entry belongs to the session that reached the recorded value.The closed session's final value is also preserved now as a distinct
ClosedSessionRecordrather than overwritten, and the reset response returnsclosed_session_max_sensitivityandreset_count.Testing
1625 passed, 9 new. Ruff and mypy clean on
src/.Two failures are pre-existing on the base branch, confirmed by stashing:
test_release_distribution_smoke(runtime 0.5.0 against metadata 0.4.0) andtest_tpm_chained_verify(assertion text drift).🤖 Generated with Claude Code
https://claude.ai/code/session_01CDzMWcn12dszchgrKfXzZY