Skip to content

feat(proxy): record advertised but unadmitted tools as audit observat… - #723

Merged
imran-siddique merged 2 commits into
agentrust-io:mainfrom
solloek369-arch:feat/566-o3-unadmitted-observation
Oct 5, 2026
Merged

imran-siddique merged 2 commits into
agentrust-io:mainfrom
solloek369-arch:feat/566-o3-unadmitted-observation

Conversation

@solloek369-arch

@solloek369-arch solloek369-arch commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

What

At successful first-contact comparison, _check_upstream_drift records at most 64 sorted advertised names outside the same server's active catalog entries as individual tool_observed_unadmitted AuditChain entries. Recorded tool_name is capped at 256 Python characters. Any remainder is one existing tool_observed_unadmitted entry with tool_name null, detail.status observed_unadmitted_summary, and integer detail.omitted_name_count. Extra names stay outside the drift comparison and do not themselves deny approved calls; an unadmitted call remains subject to catalog_miss, and definition_changed / withdrawn behavior is preserved. Part of #566 (Obligation 3), per the ruling.

Carrier. A separate observation at comparison time, alongside the existing drift audit path, so the evidence describes the advertisement versus the active admission basis rather than the triggering approved call's terminal result. Attaching evidence to a genuine call terminal remains a plausible alternative; the ruling establishes the semantics, not this carrier choice.

Fields. server_identity follows the existing server-URL convention and does not encode the full provenance key; for a stdio server it is empty. Scalar detail records status: observed_unadmitted, source: upstream, measured_catalog_hash, admission_basis: active_catalog_entries, active_admitted_count and active_exception_count. A truncated recorded name also stores recorded_name_truncated, tool_name_original_length, and tool_name_sha256. Names past the cap are not stored individually: the summary entry uses tool_name: null, status: observed_unadmitted_summary, and integer omitted_name_count, with the same context fields. Active admission includes runtime break-glass entries, so an exception-admitted tool is not recorded as unadmitted; the measured catalog hash stays unchanged.

Boundaries. Appended after complete discovery/classification and before the no-drift return, including when approved tools also drift. Incomplete discovery produces no observation. A successfully appended observation survives a later denial, fault or cancellation. Completed comparisons use the existing provenance-key cache and post-await guard, including concurrent first contact; this is not continuous monitoring or a global exactly-once guarantee after resets or partial write failures. The append is synchronous and uses the same persistence and exception-propagation mechanism as the sibling catalog_drift append. TRACE schema and tool-call transcript entries are unchanged; the original audit root is preserved, while audit length/tip and tool_transcript.hash change as expected.

Schema. schemas/audit-entry.schema.json is already stale on main: it omits five runtime EntryType values (egress_denied, suspicious_call_sequence, attestation_stale, catalog_drift, break_glass_used), disallows detail, and requires a string call_id. An existing catalog_drift entry already fails strict validation with the same errors. Aligning the schema seems like a separate change. This commit does not include that cleanup.

Why

Advertised names outside the approved catalog previously left no evidence that a server offered them. The ruling asks for them to be recorded as observed-and-unadmitted, without turning them into drift or a denial.

Security impact

No change to admission or denial behavior. The new entry is evidence only; it never admits a tool or suppresses real drift.

Test plan

Local only, not GitHub CI. tests/unit/test_unadmitted_observation_bounds.py together with tests/unit/test_upstream_catalog_drift.py: 107 focused passed. Local prescribed population (unit/conformance/integration/confinement): 2421 passed, 29 skipped. The persisted 65-name case records 64 individual observations plus one summary with 1 omitted; the 1,041-name case records 64 plus one summary with 977 omitted. Mutants that drop the recording, record only without drift or only on a successful terminal, count extras as drift, write the entry as catalog_drift or tool_call, ignore exceptions, or remove the post-await dedup guard each fail the tests. Local Python 3.12 only.

  • pytest passes
  • ruff check passes
  • mypy passes
  • Manual test performed (describe steps below if applicable)

DCO sign-off

…ions

Signed-off-by: Loek <solloek369@gmail.com>
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@imran-siddique imran-siddique left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@solloek369-arch this records what the #566 ruling asked for, and a separate entry at comparison time is the right carrier: it keeps the observation off the triggering call's result.

One change before merge. Every unadmitted name becomes its own synchronous audit append, and discovery allows MAX_DISCOVERY_PAGES = 1000 pages with no cap on names per page or on name length. Before this PR those names cost nothing. Now a hostile upstream decides how many entries we write at first contact.

Bound it: record at most a fixed number of names per comparison, sorted as now, carry the remainder as a count, and cap tool_name length. Add a test that a listing over the bound writes exactly the cap plus the count. Can you have it up by 9 October?

Agreed that aligning audit-entry.schema.json is a separate change.

Record at most 64 sorted unadmitted names per completed comparison and
retain the omitted count in one existing observation entry. Limit recorded
name prefixes to 256 characters with explicit truncation and digest
evidence while preserving full-name admission and drift comparisons.

Signed-off-by: Loek <solloek369@gmail.com>

Copy link
Copy Markdown
Contributor

Independent review at head 2035d5759b288fcfd29e420b96a176d89bf7793d: all 84 upstream catalog-drift tests passed locally on Python 3.12.14 with the hash-pinned development dependencies.

A focused harness using the exact drift method and real AuditChain independently confirms the existing bounds concern: 1,024 extras produce 1,024 observations, and an 8,192-character name is retained intact. The harness stubs catalog/digest/transport dependencies; this is focused reproduction, not a full integration test. The real discovery function also accepts that long name.

For the bounded revision, I suggest checking:

  • K−1, K and K+1 names, with exact omitted counts and deterministic selection.
  • Long Unicode names and distinct names sharing a truncated prefix; document whether the length limit measures characters or encoded bytes.
  • Approved-call positive twins, unknown-tool refusal without forwarding, and preserved drift outcomes.
  • Cached/concurrent deduplication and persistence of the remainder evidence.

This supports Imran’s requested change; it is not merge approval. I can review the revised pinned head once available.

@solloek369-arch

Copy link
Copy Markdown
Contributor Author

@imran-siddique Yeah, you're right — sorry. But it gave some extra fuel for a proper check :D.

Pushed dc82afe, addressing the requested bound: each completed comparison now records at most 64 individually sorted unadmitted-name observations, with recorded tool_name capped at 256 Python characters. Any remainder is represented by one summary observation with tool_name=null, status=observed_unadmitted_summary, and integer detail.omitted_name_count.

The persisted 65-name test records exactly 64 individual observations plus one summary with 1 omitted. The 1,041-name test records 64 plus one summary with 977 omitted. No audit-schema cleanup is included.

I also ran a broader local boundary and regression pass around this bounded-audit change. Nothing in that pass changed the design above, so I've kept those checks as supporting evidence rather than turning additional observations into new acceptance criteria.

@imran-siddique
imran-siddique merged commit 962e54e into agentrust-io:main Oct 5, 2026
15 of 18 checks passed
@imran-siddique

Copy link
Copy Markdown
Member

@rajnisht7 this merged with one maintainer approval where maintainers.json requires two, because it touches src/cmcp_runtime/audit/chain.py. That was my mistake at merge time, not the author's. Can you review the merged change (squash of dc82afe) by 8 October? If you find a problem, it gets a follow-up PR or a revert.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants