fix: honesty sweep (fail-closed attestation, honest docs, implement straightforward stubs)#3133
Draft
liamcrumm wants to merge 4 commits into
Draft
fix: honesty sweep (fail-closed attestation, honest docs, implement straightforward stubs)#3133liamcrumm wants to merge 4 commits into
liamcrumm wants to merge 4 commits into
Conversation
…tub docs Honesty sweep across the Python packages: align overclaiming docs with the Public Preview stub reality, and fix one real fail-open security bug. Security fix (agent-os/iatp): - AttestationValidator._verify_signature returned True (signature valid) when the cryptography library was missing, while the README claims Ed25519 cryptographic proof. That is fail-open: any unsigned or forged attestation would be accepted. Verify now fails closed (returns False) without crypto, and a regression test (test_verification_fails_closed_without_crypto_library) covers it. The iatp README now states cryptography>=42.0.0 is required. Doc honesty (no behavior change): - agent-hypervisor README + docs/api-reference: enable_blockchain_commitment is a reserved no-op in Public Preview (blockchain anchoring not implemented, stored in memory only), not 'commit audit hashes to blockchain'. Added a Public Preview status note listing the documented stubs (liability ledger always admits, slashing/quarantine record but do not enforce, vouching no bonding, no saga replay). Kept the field because conformance spec S17 asserts it. - Root README: 'commitment anchoring' -> 'in-memory commitment tracking'. - mcp-kernel-server README: the MCP message adapter is functional but the standalone server transport (MCPServer.start) is a Public Preview placeholder. - a2a_adapter docstring: drop 'All A2A communications are now governed!' next to a placeholder start(); note the transport is a preview placeholder. Verified honest and left as-is: agent-sre has a real replay engine (the feature matrix claim is accurate), nexus README already discloses placeholder crypto, and the hypervisor liability stubs are honestly labeled with no contrary doc. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Liam Crumm <liamcrumm@gmail.com>
liamcrumm
requested review from
MohammadHaroonAbuomar and
imran-siddique
as code owners
github-actions
Bot
added
documentation
🤖 AI Agent: test-generator — `agent-os/modules/iatp/iatp/attestation.py`
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
🤖 AI Agent: security-scanner — Security Review
Security Review
No other security issues found. |
🤖 AI Agent: breaking-change-detector — API Compatibility
API Compatibility
|
🤖 AI Agent: docs-sync-checker — Docs Sync
Docs Sync
|
🤖 AI Agent: code-reviewer — Action items:
TL;DR: 1 blocker, 0 warnings. The PR addresses a critical security issue but requires further clarification on the fail-closed behavior.
Action items:
No warnings. |
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. |
cspell flags the alpha hex literal 'aabbccdd' on the changed line; use digit-only hex (test value is arbitrary). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Liam Crumm <liamcrumm@gmail.com>
github-actions
Bot
added
dependencies
liamcrumm
changed the title
📦 Dependency diff (SBOM)Comparing main → liamcrumm/honesty-sweep-placeholders. ✅ No dependency changes detected. |
liamcrumm
force-pushed
the
liamcrumm/honesty-sweep-placeholders
branch
from
8c63d08 to
0214688
Compare
liamcrumm
changed the title
github-actions
Bot
added
size/S
The standalone _shadow_impl fallback (used by agentmesh.governance.shadow when agent_sre is not installed) left 12 ShadowMode and CanaryRollout methods raising NotImplementedError, so any deployment without agent_sre lost shadow comparison and canary rollout entirely. Port the real implementations from the canonical agent_sre.delivery.rollout (set_similarity_function, compare, is_passing, finish; start, advance, check_rollback, analyze_step, rollback, pause, resume, promote) plus the missing _record_event helper. The two files already share identical data models, so the bodies port verbatim. Verified the fallback end to end: match detection, confidence scoring, rollback to rolled_back, and promote to complete. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Liam Crumm <liamcrumm@gmail.com>
ChromaDBAdapter inherited the VectorStoreAdapter base defaults for update and delete, which raise NotImplementedError, so the ChromaDB-backed episodic store silently could not update or remove episodes (FileAdapter already implements both). Implement both using the ChromaDB collection API, returning False when the id is absent to match FileAdapter semantics. update writes metadata only (get_by_id and retrieve reconstruct episodes from metadata, not the document), which avoids forcing a re-embed when no embedding is supplied. Add hermetic regression tests that skip when chromadb is absent and pass explicit embeddings so no model download is triggered. Also drop a pre-existing unused json import flagged by ruff F401 on the touched file. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Liam Crumm <liamcrumm@gmail.com>
liamcrumm
changed the title
github-actions
Bot
added
size/L
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| adapter = _adapter(tmp_path) | ||
| eid = adapter.store(_episode(), embedding=np.zeros(8, dtype=np.float32)) | ||
|
|
||
| assert adapter.delete(eid) is True |
|
|
||
| def test_delete_missing_returns_false(tmp_path): | ||
| adapter = _adapter(tmp_path) | ||
| assert adapter.delete("does-not-exist") is False |
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
1 participant
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
Honesty sweep. Closes one fail-open security bug, aligns overclaimed stub
docs with reality, and implements the previously-stubbed capabilities that
were straightforward to complete.
Problem
iatp/attestation.py._verify_signaturereturned
True(valid) whencryptographywas absent, accepting anysignature.
reserved or stubbed capabilities as shipped features.
NotImplementedErroreven though the real logic existed nearby or wastrivial to add.
Changes
agent-os/modules/iatp/iatp/attestation.py_verify_signaturefails closed (returnsFalse) whencryptographyis unavailable; regression test addedagent-hypervisor/README.md, rootREADME.md, api-reference, mcp-kernel-server README, a2a/iatp docsagent-mesh/.../governance/_shadow_impl.pyShadowMode/CanaryRolloutfallback methods +_record_event, ported from canonicalagent_sre.delivery.rollout(identical data models)agent-os/modules/emk/emk/store.pyChromaDBAdapter.update/delete(metadata-only update to avoid a re-embed); dropped a pre-existing unusedjsonimportagent-os/modules/emk/tests/test_chroma_adapter.pyenable_blockchain_commitmentis left in place; it is a reservedSessionConfig field asserted by the hypervisor conformance spec.
Deliberately NOT implemented (still raise, by design)
GovernedRetriever/GovernedQueryEngineasync/batch/stream methods(agent-rag-governance): raising is a security guard since the
governance pipeline is synchronous and a passthrough would silently
bypass ACL/rate-limit/scan/audit.
work, left unimplemented and labelled as such (no lifecycle wording).
Testing
_shadow_implfallback: direct functional check (match/confidence/rollback/promote)failures (version banner, semantic-rule immutability) confirmed unrelated
E,F,Wclean on all touched files;no-stubsgate passes