Skip to content

fix: carry #330 forward with Conduit observation checks - #374

Open
dinakarjs wants to merge 23 commits into
agentrust-io:mainfrom
dinakarjs:fix/330-conduit-observation
Open

dinakarjs wants to merge 23 commits into
agentrust-io:mainfrom
dinakarjs:fix/330-conduit-observation

Conversation

@dinakarjs

@dinakarjs dinakarjs commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Problem and change

Conduit finish responses could be accepted when running was absent or a falsy non-boolean. Completion now requires running is False; missing, null, zero, empty string/list/dict, true and string values have refusal controls. Separate cases keep start valid and mutate each finish commit field independently, covering missing, malformed and mismatched values.

The released API probe's emitted records also failed TRACE Level 0 because timestamps were 1/2 and subjects were not URI identities. It now emits current Unix timestamps and synthetic did:example: subjects. A mandatory pinned agentrust-trace-tests==0.6.2 test loads and checks both actual emitted records at Level 0, with default freshness, and verifies subject-tampered twins fail signature checks. The manifest records TRACE 0.11.0 and tests 0.6.2. cA2A runtime 0.4.0 remains documented/pinned outside the schema-limited manifest. Existing transaction, repository, receipt-chain and commit binding remain.

Successor to #330 at Imran's request, preserving Val's 17 original commits and all subsequent history. This remains a synthetic released-package/software harness; stricter Conduit transaction echo still requires operator adoption and a fresh live capture. Example DIDs are not independently provisioned identities. Historical captures remain historical. No new remote execution, live peer, independent custody, hardware/TEE or production acceptance is claimed; receipt hashes are unsigned. #199 remains open.

Validation — 1 November 2026

Head: 8d3533057a0eb43286c7796bacf824b78bf224f7; tree: 9f04e8cdc4f0ac71a485ab75594f06db0891da2e.

  • Hash-locked Linux/Python 3.12 probe environment: 95 harness tests pass normally and under optimized Python; pip check passes.
  • Both emitted records pass TR-ENV/TR-SIG/TR-POL/TR-APR at Level 0. Optional findings remain explicitly skipped by the released suite; no Level 1/2 claim.
  • Restoring the old running check causes 6 assertion failures; removing either finish-commit comparison causes 6 failures each. Restoring the old probe fails the emitted-record conformance test on timestamp/subject findings. Source restored and final suite passes.
  • Separate hash-locked Python 3.11 validation environment: all 60 root tests, 49 manifests, compatibility and generated index/catalog checks pass. CI Ruff subset and whitespace pass.
  • The lock adds the tests package without upgrading existing dependency versions/hashes.

Independent current-head review and hosted checks remain required. Prior approvals apply to their earlier heads. Prepared with OpenAI Codex; DCO-signed. No merge or release performed.

altrudev and others added 18 commits October 15, 2026 21:38
Apply the two pre-merge checks proposed by Noah and the bounded remediation supplied by Val. Preserve all 17 original commits. Update valid fixtures and add wrong/missing repository and rehashed FAIL receipt controls.

Prepared with OpenAI Codex.
Signed-off-by: Srinivasa Consultants <78673153+dinakarjs@users.noreply.github.com>
@github-actions

github-actions Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

🟡 Contributor Check: MEDIUM

Check Result
Profile MEDIUM
Credential LOW
Overall MEDIUM

Automated check by AgenTrust Contributor Check.

Copy link
Copy Markdown
Contributor Author

@carloshvp @rajnisht7 requesting independent review of current head 0516135, the requested #330 successor. The reviewer-assignment API returns 403, so no formal assignment is claimed. Latest CI's substantive blockers are the unchanged Nobulex links returning HTTP 451 (listing removal already in #357) and the WCM example's missing OpenSSL import. Its model-signing extra installs successfully but does not provide that import. The offline harness results in the description are separate from those repository jobs and do not establish the remaining live/hardware acceptance criteria. No gate bypass or merge requested.

@noah-ing noah-ing 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 exact head 0516135e34c4cbd236c0ef84a087389dcca9e43d. In a fresh Python 3.12 environment installed from the hash-locked requirements, the 87 harness tests and 28 repository tests passed. Ruff passed, and the committed packet verified with its published key pins as a historical replay. Paired synthetic controls show that the new repository-identity and final-receipt-PASS guards reject the cases raised on #330; those two corrections are effective. This does not repeat the historical two-host run or establish independent key custody, protected inference, or hardware assurance.

I request two changes before this successor merges:

  1. requirements/confidential-workflow-probe.txt pins certifi==2026.7.22. That release is in the affected range of the public certifi advisory, which identifies 2026.10.19 as patched. The fresh locked install reproduced the affected pin. Please regenerate and verify the hashes for a patched release. I have not shown a TLS exploit in this harness's SSH path.
  2. The PR description uses Closes #199, so merging into main would auto-close the issue. #199 still has unchecked acceptance for a bound transaction, independent live peers, protected inference and other controls that this bounded software pilot explicitly does not establish. I understand the successor routing came from Imran; please coordinate with him on Refs #199 or a named follow-on tracker that preserves every outstanding criterion before this PR closes #199.

One metadata follow-up: integration.yaml still names @altrudev, who has stepped away, and lists cmcp-runtime: 0.5.0 while the new bounded path pins and exercises 0.7.0. Please confirm the active integration owner and clarify whether tested_against describes the older deterministic path or this new one; update it if it is meant to describe the latter.

The hosted harness validation, lint and CodeQL checks pass. The links and Weight Custody Manifest example failures are being addressed separately in #357 and #362; the required maintainer gate remains outstanding. Please request re-review after these points are resolved on a new exact head.

@carloshvp carloshvp 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.

Reviewed exact head 0516135e34c4cbd236c0ef84a087389dcca9e43d in an isolated checkout.

The successor retains effective repository-identity/final-receipt checks and passes my offline validation (87 harness tests and 28 repository tests, pip check and the repository CI Ruff subset). I have not repeated the historical two-host run or established independent operator/key custody, protected inference or hardware assurance.

Two current blockers remain, consistent with the existing review:

  1. requirements/confidential-workflow-probe.txt still pins certifi 2026.7.22, the affected release reported by the current dependency audit. Please regenerate and verify hashes for a patched release. This is a vulnerable dependency finding, not a demonstrated TLS exploit in the harness's SSH path.
  2. The PR description still uses Closes #199, while that open issue retains unchecked acceptance for a bound transaction, independently observed boundaries and independently operated live peers. The PR explicitly says those broader claims are not established. Please coordinate the successor routing with Imran and retain the acceptance tracker via Refs #199 or a concrete linked follow-on before automatic closure.

The integration manifest also still names the original owner and records cmcp-runtime 0.5.0 while this added path pins 0.7.0. Please confirm active ownership and which exact run tested_against describes. Please also clean the reported diff whitespace. Requesting the dependency/scope corrections above; passing offline tests do not complete #199's broader acceptance.

Reviewed with OpenAI Codex.

Keep the broader acceptance tracker open and preserve original capture attribution.

Signed-off-by: Srinivasa Dinakar Jainti <dinakarjs@gmail.com>

Copy link
Copy Markdown
Contributor Author

Addressed the two current change requests at bf04e7d: the probe lock now installs patched certifi with verified release hashes, and the PR uses Refs #199 so outstanding acceptance stays open. I took over the manifest maintenance contact as requested for this successor; tested_against describes the bounded cMCP 0.7.0 run, and the README distinguishes historical captures. Reported whitespace is cleaned. Val's 17 commits remain unchanged.

@noah-ing @carloshvp please re-review this head. @imran-siddique please confirm keeping the existing #199 tracker open for the unfulfilled acceptance criteria under the successor route. No source gate bypass or merge requested.

@noah-ing noah-ing 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 current head bf04e7dce5a0dff82c07676dfecadcdc6b58aa2a in a fresh Python 3.12 checkout. The new commit changes only the probe lock, metadata and documentation. A hash-required install selected certifi==2026.10.19; both lock hashes match the official PyPI release artifacts, and the installed packages passed the dependency check. All 87 harness tests passed normally and under -O. The 28 repository tests, 47 integration validations, compatibility and generated index/catalog checks, and diff whitespace check passed. The repository/final-receipt guards verified in my previous review are unchanged. The dependency and ownership/version concerns from that review are addressed.

One issue remains in the PR description. It now says Refs #199, but later says must not automatically close #199. GitHub interprets close #199 as a closing keyword even in that negative sentence. On this exact PR, GitHub's closingIssuesReferences still contains #199. Because this PR targets main, merging it would close the still-open acceptance tracker despite the stated intent. GitHub's documentation confirms this behavior.

Please reword that sentence, for example, Issue #199 remains open after this PR merges; ..., and verify #199 is absent from the PR's closing-issues list. This is a description-only correction; no source change is needed for this point. The remaining hosted checks and maintainer gate are separate merge requirements. The offline results do not establish the unfulfilled live or hardware acceptance criteria in #199.

carloshvp
carloshvp previously approved these changes Oct 27, 2026

@carloshvp carloshvp 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.

Approving the bounded software-harness successor at exact head bf04e7dce5a0dff82c07676dfecadcdc6b58aa2a after re-review. The requested certifi, ownership, tested-version, and whitespace corrections are addressed. The runtime guards remain unchanged from the previously reviewed source.

Independent Python 3.12 validation: a fresh hash-required probe install and pip check passed; the certifi 2026.10.19 hashes match the official PyPI artifacts. All 87 harness tests pass normally and under -O. The 28 repository tests, 47 manifest validations, compatibility, generated index/catalog parity, CI Ruff subset, and diff whitespace checks pass.

I also corrected the PR description's accidental closing-keyword sentence. GitHub now returns an empty closingIssuesReferences list; issue #199 remains the acceptance tracker. This resolves the scope concern in my previous changes request. Noah's separate review and the repository checks still need their normal disposition.

These are offline tests with synthetic transport responses. The earlier two-host captures retain their original authorship/revisions; I did not repeat them or establish hardware, protected inference, independent custody, or full #199 acceptance.

Reviewed with OpenAI Codex.

@altrudev

Copy link
Copy Markdown
Contributor

DDC Radial independent audit (exact head bf04e7d): fresh hash-locked installation and 87/87 harness tests passed; reviewer approval and closingIssuesReferences=[] confirmed. The two hosted failures are still links and weight-custody-manifest. WCM job 121463158327 fails in the offline demo/test step, not install; the follow-up OpenSSL import correction was merged to main via #362 after this head was tested. Please synchronize this successor branch with current main and rerun its exact-head checks. Nobulex HTTP 451 is tracked by open PR #357; do not bypass links or assert it fixed until that change merges and hosted crawl passes. No change to Issue #199 scope, hardware claims or Frequency ownership is requested. I cannot mark #374 merge-ready while either check fails.

Preserve the original contributor commits and successor corrections while bringing in the merged WCM OpenSSL fix. Harness and probe dependency bytes are unchanged.

Prepared with OpenAI Codex.

Signed-off-by: Srinivasa Consultants <78673153+dinakarjs@users.noreply.github.com>

@noah-ing noah-ing 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.

Thanks for the follow-up. I re-reviewed current head a358be70dba5755febe13d385e87654b04937980. The revised description no longer creates a GitHub closing reference to #199 (closingIssuesReferences is empty), resolving my remaining request. The confidential-workflow harness and both probe locks are byte-identical to the previously reviewed head; the merge from main adds the workflow OIDC baseline test.

I reran the harness in a fresh, hash-locked Python 3.12 environment: 87/87 tests passed normally and 87/87 passed under -O. The root validation tests passed 60/60 in a separate fresh environment. The current hosted WCM example and maintainer gate also pass. I approve this head on those reviewed points.

The documentation-links check is still red on the separate Nobulex HTTP 451 issue. Please wait for that fix and a green links rerun before merging this PR.

Preserve the successor history and unchanged harness/probe locks while incorporating merged agentrust-io#357 and current validation baseline.

Prepared with OpenAI Codex.

Signed-off-by: Srinivasa Consultants <78673153+dinakarjs@users.noreply.github.com>

Copy link
Copy Markdown
Contributor Author

Closure refresh: current head a3a0a83 retains Carlos and Noah approvals on that exact commit, has no failing returned checks, and GitHub currently reports mergeability clean. The former Nobulex/WCM baseline blockers are not current failures on this head. @rajnisht7 please disposition the normal maintainer merge step when appropriate. Issue #199 remains the broader acceptance tracker; this bounded software pilot does not establish its live-peer, protected-inference, independent-custody or hardware requirements. No merge was performed by this follow-up. Prepared with OpenAI Codex.

@noah-ing noah-ing 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 exact head a3a0a83cf6448d4ceac44d5510e547aea1a163ee after its merge from main. I compared it with the head of my previous approval: every confidential-workflow harness and probe-requirements file is byte-identical, and the effective 32-file PR patch is unchanged. The PR's workflow edit is also identical on the updated base. All 28 current-head hosted checks pass, including links, harness validation, the WCM example, lint, and the maintainer gate.

This resolves the exact-head review gap noted in the updated description, so I approve the bounded software-harness change on this head. The earlier scope limits remain: offline synthetic validation and the historical two-host packet do not establish the outstanding live-peer, independent-custody, protected-inference, or hardware acceptance criteria tracked in #199. The final merge decision remains with the maintainers.

@rajnisht7 rajnisht7 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.

Reviewed the current head a3a0a83.A few points remain in the Conduit binding path which this PR is strengthening:

  • conduit_live.py still does not bind transaction_id to the Conduit job/receipt. cMCP admission uses it, but job_start only sends intent + repo. The returned receipts are never checked against the requested transaction. Wrong-repo is rejected; wrong-transaction is not, also no regression test for this.
  • Missing commit_sha is accepted, when both start and finish omit commit_sha / commit_sha_end, None == None lets the check pass, docs claim a pinned commit is verified; presence is never required.
  • Malformed nested responses can raise uncaught AttributeError, if result or detail is a list (or similar), .get() crashes. The handler catches TypeError/KeyError but not AttributeError, so the fail-closed path is incomplete.
  • HarnessRunner still hardcodes package_versions as cmcp 0.5.0 / ca2a 0.2.0 while the released path and docs use 0.7.0 / 0.4.0. Machine-readable evidence ends up with stale versions.
  • TRACE is produced and verified (Level 0, agentrust-trace 0.11.0) but integration.yaml only declares cmcp + ca2a. No trace in integrates_with, no trace_roles / trace_conformance_level, no agentrust-trace in tested_against. Catalog under-reports what the integration actually does.

Prepared with OpenAI Codex.

Signed-off-by: Srinivasa Dinakar Jainti <dinakarjs@gmail.com>

Copy link
Copy Markdown
Contributor Author

@rajnisht7 @noah-ing please re-review e00c530. Transaction IDs now bind both dispatches, results and receipt details; commits must be present/full SHA; malformed nested responses return unknown. Deterministic evidence no longer claims stale package versions; released probes retain measured versions. TRACE Level 0 producer/consumer metadata and catalog are updated. Fresh hash-locked environments: 92 harness tests normally/under -O, 60 root tests, 49 manifests, compatibility, generated checks and Ruff pass. Old-source controls expose the new defects. The stricter transaction echo contract needs Conduit operator adoption and a fresh capture before live acceptance; historical evidence is unchanged. #199 and independent-custody/protected-inference/hardware holds remain. Prior approval is not asserted for this new head. Prepared with OpenAI Codex.

Copy link
Copy Markdown
Contributor Author

@rajnisht7 @noah-ing requesting re-review and disposition on current head e00c530. The published correction binds transaction IDs across dispatches/results/receipt details, requires present full commit SHAs, refuses malformed nested responses, removes unmeasured deterministic package versions, and updates TRACE Level 0 producer/consumer metadata and the generated catalog.

I have just rechecked this head: all 13 returned PR-triggered technical workflows succeeded. The commit check inventory still contains failing maintainer gate entries; earlier approval at a3a0a83 is not approval of this changed source. Please review the repair on this head, disposition the reported findings and identify any remaining changes. Prior validation in the description records 92 harness tests normally/under -O, 60 root tests and behavioral old-source controls; no fresh local suite run is claimed here.

The stricter transaction echo contract requires Conduit operator adoption and fresh capture before live acceptance. Historical captures retain their original scope, and #199 remains open for broader live-peer, independent-custody, protected-inference and hardware acceptance. Prepared with OpenAI Codex.

@noah-ing noah-ing 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 exact head e00c5300f76b61e6a79d8f79d7e001dc9f38cbca in fresh, hash-locked Python 3.12 environments. The follow-up sends transaction_id with both Conduit calls and checks it in both returned results and receipt details; requires a full lowercase 40-hex start commit and matching finish commits; handles malformed nested responses as unknown; removes invented package versions from the deterministic runner; and aligns the TRACE Level 0 manifest and generated catalog. These address the five points Rajnish raised.

Independent validation: 92 harness tests passed normally and under -O; 60 repository tests, all 49 manifest checks, compatibility and generated index/catalog checks passed. The new binding and metadata regressions fail against the prior source and pass with this head. Both locked environments passed pip check, and the changed-file whitespace check passed. The current rollup has 34 successful technical checks; the two failing entries are maintainer-approval gates, which still require normal disposition.

I approve this bounded software-harness repair on this head. The stricter transaction echo still needs Conduit operator adoption and a new live capture before live acceptance. The historical packet, unsigned receipt hashes, and these synthetic tests do not establish the broader live-peer, independent-custody, protected-inference, or hardware criteria tracked in #199.

@rajnisht7 rajnisht7 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.

Reviewed the head, a few points still needs tightening:

  • TRACE Level 0 : The declaration is correct and matches what the integration does (and what we asked for earlier). Several other producer/consumer integrations pin agentrust-trace-tests when they claim a level. This one currently only records agentrust-trace: "0.11.0", kindly run the Level 0 suite against an emitted record and add the exact tests version to tested_against.
  • running field - the new finish checks still only do finished.get("running"). Missing/None/0/""/[] are falsy and can still return observed-live-conduit-inspect. kindly require it to be a bool that is False, and add tests for those cases.
  • Commit validation tests - test_missing_and_invalid_commit_is_rejected mutates both start and finish, so it only exercises the start check (untrusted-start), kindly add separate cases that keep start valid and mutate finish commit_sha / commit_sha_end (missing, invalid, mismatch).
  • ca2a-runtime-the probe and README exercise ca2a-runtime==0.4.0, but the schema does not allow that key in tested_against. Other integrations (e.g. horizonshield-ca2a-companion) document the pin outside the schema block.

Require explicit false running state, isolate both finish-commit checks, and test emitted probe records with pinned TRACE Level 0 suite. Emit current timestamps and synthetic example DID subjects; preserve software-only and live-adoption limits.

Prepared with OpenAI Codex.

Signed-off-by: Srinivasa Dinakar Jainti <dinakarjs@gmail.com>

Copy link
Copy Markdown
Contributor Author

@rajnisht7 @noah-ing requesting current-head re-review at 8d3533057a0eb43286c7796bacf824b78bf224f7.

Completion now requires running is False. Separate finish-commit controls preserve a valid start and exercise each missing/invalid/mismatched finish field. Both records actually emitted by the released probe now pass pinned agentrust-trace-tests 0.6.2 at Level 0, using current timestamps and synthetic example DIDs; tampered twins fail. The manifest records the tests version; the schema-excluded cA2A runtime pin remains in the README/lock. Local evidence: 95 harness tests normally/under -O and 60 root tests, with targeted guard-removal failures. Please disposition the remaining findings on this head. Operator adoption/fresh live capture and broader #199 acceptance remain separate.

All returned current-head technical workflows pass. Validation details and limitations are in the PR description; this follow-up does not claim another local suite run or independent approval. Prepared with OpenAI Codex.

@noah-ing noah-ing 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 exact head 8d3533057a0eb43286c7796bacf824b78bf224f7. The four follow-ups raised on the previous head are addressed: completion requires running is False; finish-commit tests preserve a valid start; both records emitted by the released probe are tested against pinned TRACE Level 0; and the cA2A runtime pin is documented outside the schema-limited manifest. In fresh hash-locked Python 3.12 environments, I reran 95 harness tests normally and under -O, 60 repository tests, 49 manifest validations, compatibility and generated-file checks. Both emitted records pass the mandatory Level 0 findings; the suite reports optional findings as skipped. The current head has 34 successful hosted checks, with only the maintainer-approval gate outstanding.

One result-acceptance issue remains in harness/conduit_live.py:

  • Lines 65 and 88 use truthiness for the job_start and job_poll ok fields. With otherwise valid synthetic receipts, setting either field to the JSON string "false" still returns observed-live-conduit-inspect.
  • Line 103 compares exit_code to integer zero. Setting the finish field to JSON false also returns observed-live-conduit-inspect, because Python considers False == 0.

These are malformed-response controls against this software harness, not a claim of a live exploit. Please require ok is True at both boundaries and a genuine integer zero exit code (excluding booleans), with negative regression cases for all three inputs. I request changes for this remaining fail-closed gap. Conduit operator adoption, a fresh live capture, and the broader criteria in #199 remain separate from this review.

@noah-ing

noah-ing commented Nov 1, 2026

Copy link
Copy Markdown
Collaborator

Follow-up to my current-head review of 8d3533057a0eb43286c7796bacf824b78bf224f7: the separate remote cA2A peer path needs the same fail-closed attention. The stricter conduit_live.run checks do not cover remote_peer_service.execute_job.

I exercised the actual service, signed cA2A response, signed TRACE bundle and client verification with synthetic Conduit responses. The valid control and cases with commit_sha: "abc", session_id: null, or exit_code: false each returned authenticated-remote-ca2a-software with trace_verified=True and both receipt hashes. The service checks equality of commit fields without a full-SHA format check, accepts a null session when the hash-covered receipt agrees, and compares exit code with == 0; the client also accepts boolean false as zero. Separate direct controls show that string "false" in either Conduit ok field and null running also produce an accepted execution result.

There is also no request-to-worker transaction binding in this path: execute_job has no transaction_id parameter, the job_start/job_poll calls omit it, and _receipt_ok checks the session but no transaction echo. Two synthetic requests with different transaction IDs accepted the same hashed PASS receipt pair and produced positive software-only results. This shows the path cannot detect that synthetic misassociation; it is not evidence of a real Conduit replay.

Please validate strict booleans and a genuine integer zero, require a nonempty session and full lowercase 40-hex commit, and carry/check the requested transaction ID in both Conduit calls, results and receipt details. Add negative regressions through the signed service/client path for these cases. The worker responses were mocked; I did not run a live VPS or establish broader #199 acceptance.

This branch has not been deployed

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

Labels

needs-review:MEDIUM Contributor check flagged MEDIUM risk

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants