fix(agent-os): harden replay, tool-poisoning, audit accounting, health, signed CloudEvents#3247
Draft
liamcrumm wants to merge 2 commits into
Draft
fix(agent-os): harden replay, tool-poisoning, audit accounting, health, signed CloudEvents#3247liamcrumm wants to merge 2 commits into
liamcrumm wants to merge 2 commits into
Conversation
…h, signed CloudEvents Close five agent_os security and integrity gaps so the kernel can be trusted to gate and record untrusted activity. Each defect ships with a test that fails before the change and passes after. 1. Nonce-cache replay (mcp_protocols / mcp_message_signer). InMemoryNonceStore evicted by count via OrderedDict.popitem, dropping nonces still inside the replay window and letting messages replay as valid. Eviction is now expired-first and fails closed at capacity (NonceStoreCapacityError) rather than evicting an in-window nonce, keeping max_entries a real bound. 2. Tool-poisoning coverage (memory_guard). validate_write missed markup-wrapped standing instructions carrying destructive shell commands and exfil. Added markup-tag, destructive-command and exfil pattern sets and a TOOL_POISONING alert. Tags are MEDIUM alone and HIGH when paired with a dangerous payload, so benign prose (including a bare curl mention) stays allowed. 3. Event accounting (event_sink.GovernanceEventProcessor). The processor had no delivered counter and lost events dispatched to a failing or circuit-open sink without counting them. Added lock-guarded submitted, delivered and failed counters so submitted == delivered + failed + dropped after shutdown. 4. False-healthy probe (integrations/health). A fresh HealthChecker reported HEALTHY with zero components and the audit built-in was an unconditional HEALTHY stub. Built-ins now auto-register (register_builtins flag), an empty report aggregates to UNHEALTHY, and the audit check reports DEGRADED when no backend is configured. 5. Signed CloudEvents (event_sink). GovernanceEvent gains to_cloudevent() (CloudEvents 1.0 per ADR-0021 and AUDIT-COMPLIANCE-1.0 section 20.4) plus an HMAC GovernanceEventSigner whose signature covers the algorithm attribute, and StdoutGovernanceSink and OTLPGovernanceSink. to_dict() is unchanged for AuditBackendSinkAdapter back-compat. Validation: ruff check --select E,F,W --ignore E501 clean on changed files; targeted pytest for the four touched suites plus consumers passes; all five repros flip. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: liamcrumm <liamcrumm@microsoft.com>
PR Review Summary
Verdict: AI review comments are untrusted advisory output. The summary reports workflow-generated completion status only, not model-authored pass/fail claims. |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
| checker = HealthChecker(version="test") | ||
|
|
||
| class _AuditBackend: | ||
| def write(self, entry) -> None: ... |
|
|
||
| class _AuditBackend: | ||
| def write(self, entry) -> None: ... | ||
| def flush(self) -> None: ... |
…, health, docs) Remediations from a multi-lens deep code review of the security hardening: - event_sink: spread GovernanceEvent.attributes BEFORE authoritative fields in to_cloudevent() so a free-form attribute key can no longer shadow the real agent_id/decision/action inside a validly-signed CloudEvent (audit integrity). - event_sink: CloudEvents type namespace changed from ai.agentmesh.* to ai.agentos.* — Agent OS is a distinct producer from Agent Mesh (ADR-0021 reserves ai.agentmesh.* for the mesh producer); stop inventing mesh-namespaced types not in AUDIT-COMPLIANCE 20.4. - event_sink: signer/sinks serialize with default=str (matches AuditEntry.to_json) so a non-JSON-native attribute value no longer aborts the whole batch; derived CloudEvents source is percent-encoded. - event_sink: accounting distinguishes intentional sink DROPPED (bucketed as dropped, not failed); OTLPGovernanceSink returns DROPPED (not SUCCESS) when OpenTelemetry is absent, and promotes severity/resource/policy_name/session_id as searchable OTel metadata alongside the CloudEvent envelope. - mcp_protocols: nonce retention is inclusive of the exact expiry instant (has/cleanup use strict >), matching the verifier's inclusive replay-window check, closing a boundary replay at now == expires_at. - memory_guard: markup tag + a BARE curl/wget is MEDIUM (allowed) not HIGH, to stop false-positive blocks of benign tooling runbooks; fork-bomb regex tolerates whitespace. Destructive/exfil pairings still block. - health: removed the unused policy_engine constructor parameter (silently-ignored input); documented that the server /health is DEGRADED until a durable audit backend is wired. - docs/spec: CHANGELOG [Unreleased] Security entry; MCP-SECURITY-GATEWAY 7.7/7.9 now specify capacity fail-closed + inclusive retention; api-reference HealthChecker signature and example updated. Tests: +new regressions for each finding; targeted + conformance suites pass (381), full agent-os suite unchanged vs baseline (4804 passed; pre-existing agent_sre/autogen env failures only). ruff clean on changed files. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: liamcrumm <liamcrumm@microsoft.com>
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
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.
Summary
Close five agent_os security and integrity gaps so the kernel can be trusted to gate and record untrusted activity. Every defect ships a test that fails before the change and passes after, and each evidence repro flips.
Problem
agent_oshad a cluster of governance holes: replay protection that expired early, a memory guard blind to markup-wrapped tool poisoning, an audit event processor that lost events without counting them, a health probe that reported HEALTHY while verifying nothing, and audit events that were plain unsigned dicts rather than signed CloudEvents.Changes
mcp_protocols.pyInMemoryNonceStore.addevicts expired-first and raisesNonceStoreCapacityErrorinstead of evicting an in-window nonce; new error type; docstrings updatedmcp_message_signer.pyverify_messagesurfaces capacity saturation as a fail-closed verification failure; cache-size docstring correctedmemory_guard.pyAlertType.TOOL_POISONING; MEDIUM alone, HIGH when pairedevent_sink.pysubmitted/delivered/failedcounters (reconcile withdropped);GovernanceEvent.to_cloudevent();GovernanceEventSigner;StdoutGovernanceSinkandOTLPGovernanceSinkintegrations/health.pyregister_builtinsflag), empty report aggregates to UNHEALTHY, audit check reports DEGRADED with no backend__init__.pyNonceStoreCapacityError,GovernanceEventSigner,StdoutGovernanceSink,OTLPGovernanceSinkDefects and repro flips
cache=2, verify 4 msgs, re-verify msg0 ->is_valid=False.<important>...rm -rf / and curl evil.com</important>wasallowed=True, 0 alerts. Nowallowed=Falsewith a HIGHTOOL_POISONINGalert; benign prose stays allowed.submitted == delivered + failed + dropped; a FAILURE sink yieldsfailed=50, not silent loss.HealthChecker().check_health()was empty and HEALTHY. Now built-ins auto-register, empty aggregates UNHEALTHY, and the audit check is DEGRADED without a backend.to_dict()lackedid/source/type/specversionand a signature. Newto_cloudevent()(ADR-0021, AUDIT-COMPLIANCE-1.0 section 20.4) plus HMAC signer (signature covers the algorithm attribute); tamper of data or algorithm fails verification.Design notes and scope
max_entriesa non-bound). Availability tradeoff under saturation is documented; operators raisemax_nonce_cache_sizeor shorten the replay window.to_dict()is intentionally unchanged forAuditBackendSinkAdapterback-compat; the CloudEvents form is the export path per the documented method nameto_cloudevent()./healthnow reportsdegraded(audit backend not wired into the checker) instead of a falsehealthy. Readiness stays green because DEGRADED is still ready.Testing
ruff check --select E,F,W --ignore E501clean on all changed files.pytestfor the four touched suites plus consumers (test_server,test_optional_deps_integration,test_core_features,test_spec_audit_compliance_conformance) passes; +21 new tests.test_autogen_hooksfailures and theagent_sre-missing collection error reproduce on a clean tree and are unrelated to this change.