Repository navigation
conformance: harden URI, explicit-null, and string-digest checks in TR-RTE, TR-ANC, TR-TXN, TR-SCA, and TR-POL (#570) - #576
Conversation
dinakarjs
left a comment
There was a problem hiding this comment.
Reviewed exact head e28e8f3. Independent Linux/Python 3.12.14 with matching hash-locked dependencies and exact source paths: 12 new boundary tests and 800 conformance tests pass, with 5 existing XPASS outcomes. Initial conformance execution lacked installed distribution metadata; the complete rerun after installing local package metadata passes. The digest-type and explicit-null changes behave as intended.
One residual URI-shape gap: _valid_https_uri and TR-ANC-001 treat a nonempty netloc as a nonempty host. Those are different. Direct local checks still accept https://:443/rim and https://user@/rim, whose parsed hostname is None, alongside the valid https://example.invalid/rim control. The current new helper and success messages call these well-formed/valid HTTPS URIs. This is a conformance pointer-shape finding, not a demonstrated network request, signature or anchor-inclusion bypass.
Please validate the parsed host and safely handle invalid port/authority forms in the same shared URI-shape contract, with positive twins and missing-host/invalid-authority regressions. Preserve optional-field omission behavior and do not add unrelated network/reachability policy. DCO/whitespace checked; all returned technical workflows succeed, Dependabot skipped. Reviewed with OpenAI Codex assistance.
…ntrust-io#570) - Require string type before regex matching on digest fields in TR-RTE-002, TR-TXN-001, TR-SCA-002, and TR-POL-001/TR-POL-003. - Skip TR-RTE-003 and TR-TXN-002 only when rim_uri / call_count are absent from the parent object, rejecting explicit JSON null. - Require a non-empty host and reject whitespace or control characters in TR-RTE-003 (runtime.rim_uri) and TR-ANC-001 (transparency). Signed-off-by: Raghuvar Durga <177662415+drk-ram@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Author-adoption companion for agentrust-io#576. Reference: https://docs.python.org/3.12/library/urllib.parse.html Signed-off-by: Srinivasa Consultants <78673153+dinakarjs@users.noreply.github.com>
|
Prepared a focused, DCO-signed author-adoption companion: d5c6627c, directly atop reviewed head Uses one shared HTTPS pointer-shape helper for TR-RTE-003 and TR-ANC-001: a real parsed hostname is required and the lazy port accessor is validated. Existing parse-error reporting and omission behavior remain. Twenty paired cases include ordinary hosts, IPv6 and userinfo positives plus missing-host and malformed-port negatives. Full conformance suite: 820 pass / 5 existing XPASS; Ruff, targeted mypy, dash and whitespace checks pass. Removing hostname or port validation separately causes 4 assertion failures while 16 cases pass. This checks pointer shape, not endpoint trust or anchor inclusion. Reference: primary technical source. It informs the implementation/packaging boundary; it is not independent approval or a general security guarantee. The commit message retains the reference too. For adoption: fetch I have not changed this PR's head or converted the existing changes request into approval. Please adopt or adjust the companion, run the original final-head checks and obtain independent review. Prepared with Codex; original ownership is retained. |
e28e8f3 to
f28c903
Compare
dinakarjs
left a comment
There was a problem hiding this comment.
Checked new head f28c903 against the previously reviewed e28e8f3. The relevant tr_rte.py, tr_anc.py and boundary test file are byte-unchanged; this head does not yet incorporate the URI-authority correction. Direct local checks still accept https://:443/rim, https://user@/rim and https://example.invalid:bad/rim in both pointer-shape checks; the valid https://example.invalid/rim control also passes. This is the existing shape-validation finding, not a new network or inclusion claim.
The previously supplied DCO-signed companion d5c6627 remains available in comment 6273501181. Please adopt or adjust it before re-requesting review. The current returned technical workflows pass (Dependabot skipped), but those checks do not close this demonstrated gap. My existing changes request remains. Primary reference and paired regression/mutation evidence are retained in the companion handoff. Reviewed with OpenAI Codex assistance.
|
@drk-ram f28c903 is a rebase. d5c6627 cherry-picks cleanly onto f28c903. With it, all three fail, |
Author-adoption companion for agentrust-io#576. Reference: https://docs.python.org/3.12/library/urllib.parse.html Signed-off-by: Srinivasa Consultants <78673153+dinakarjs@users.noreply.github.com> Signed-off-by: Raghuvar Durga <177662415+drk-ram@users.noreply.github.com> Co-authored-by: Srinivasa Consultants <78673153+dinakarjs@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Thanks @imran-siddique and @dinakarjs! Cherry-picked |
dinakarjs
left a comment
There was a problem hiding this comment.
Incorporation verification at 3497fd3: The shared HTTPS-pointer helper now validates hostname and lazy port parsing in both consumers. The additional bracket-tail check explicitly rejects a non-port suffix after an IPv6 closing bracket. All 18 focused URI/null boundary tests pass on Linux/Python 3.12.14. I inspected the Python 3.11 compatibility correction but did not execute that interpreter locally.
This closes my correction-incorporation action. Because this PR incorporates my contributed correction, this COMMENT is not independent approval of that correction. Required independent review and current-head hosted gates remain. Validation covers the stated controls, not endpoint trust or a wider production assurance claim. Reviewed with Codex.
There was a problem hiding this comment.
The overall changes looks good, a few things to address:
- kindly reject full C1 range (or switch to unicodedata.category(ch) == "Cc") and add at least one C1 regression on both surfaces.
- also remove the dead urlparse path in tr_anc so the shared helper is the single authority.
What this changes
Hardens URI, explicit-null, and string-digest checks across five conformance modules (
TR-RTE,TR-ANC,TR-TXN,TR-SCA, andTR-POL):TR-RTE-002,TR-TXN-001,TR-SCA-002,TR-POL-001,TR-POL-003):Requires
isinstance(value, str)before calling_DIGEST_RE.fullmatch(...)rather than coercing arbitrary Python objects viastr(...), so non-string objects whose__str__returns a digest string are rejected by programmatic callers oftrace_tests.modules.*.check(...).nullrejection on optional fields (TR-RTE-003,TR-TXN-002):Skips
TR-RTE-003(runtime.rim_uri) andTR-TXN-002(tool_transcript.call_count) only when the key is absent from the parent dictionary ("rim_uri" not in runtimeand"call_count" not in txn). When the key is present withNone(explicit JSONnull), the check emitsStatus.FAIL, matchingTR-APR-003,TR-APR-004, andschemas/trace-claim.json.TR-RTE-003,TR-ANC-001):TR-RTE-003validatesruntime.rim_urivia_valid_https_uri, requiringscheme == "https", a non-emptynetloc, and no whitespace or control characters (ch.isspace() or ord(ch) < 0x20 or ord(ch) == 0x7F).TR-ANC-001rejectstransparencyURIs containing whitespace or control characters alongside the existinghttpsscheme and non-emptynetlocchecks.Closes #570
Contributor: Raghuvar Durga (@drk-ram)
Independent review verified that
conformance/measurement/scripts/mutate_modules.pymutates everyStatus.FAILsite inconformance/src/trace_tests/modules/tr_*.pyand requires every site to fail at least one test inconformance/tests/. KeptTR-RTE-003andTR-ANC-001to their single URI-shapeStatus.FAILsites and added unit coverage inconformance/tests/unit/test_tr_rte.pyfor missing/emptyruntime.nonceat Level 1 so all 27 measured checks reportunverified : 0.Type of change
Spec section
No normative spec text changes; aligns conformance module checks in
conformance/src/trace_tests/modules/withspec/trace-v0.2.mdsection 3.1 andconformance/schemas/trace-claim.json.Tests for a fix
Added
tests/test_conformance_module_uri_and_null_boundaries.pyand unit tests inconformance/tests/unit/test_tr_{rte,anc,txn,sca,pol}.py.Validation commands and results:
pytest conformance/tests: passed (798 passed, 5 xpassed)pytest tests/test_conformance_module_uri_and_null_boundaries.py: passed (12 passed)python conformance/measurement/scripts/mutate_modules.py conformance: passed (checks measured : 27, verified : 27, unverified : 0)python tools/review_bar.py --base origin/main --head HEAD: passed (12 fail on behaviour, 0 fail on a missing name or file, 0 pass or skipagainst merge base)python tools/check_dashes.py: passed (no dashes outside examples/, spec/trace-v0.1.md)ruff check: passed (All checks passed!)mypy --strict tests/test_conformance_module_uri_and_null_boundaries.py: passed (Success: no issues found in 1 source file)Each of the 12 test cases in
tests/test_conformance_module_uri_and_null_boundaries.pyfails onbehaviourwhen run againstorigin/main:test_tr_rte_003_rejects_null_and_malformed_https_uris[explicit-null]test_tr_rte_003_rejects_null_and_malformed_https_uris[empty-host]test_tr_rte_003_rejects_null_and_malformed_https_uris[whitespace]test_tr_rte_003_rejects_null_and_malformed_https_uris[control-char]test_tr_anc_001_rejects_https_uri_with_whitespace_or_control_chars[space]test_tr_anc_001_rejects_https_uri_with_whitespace_or_control_chars[newline]test_tr_anc_001_rejects_https_uri_with_whitespace_or_control_chars[nul]test_tr_txn_002_rejects_explicit_null_call_counttest_tr_rte_002_rejects_non_string_digest_objecttest_tr_sca_002_rejects_non_string_digest_objecttest_tr_txn_001_rejects_non_string_digest_objecttest_tr_pol_001_and_003_reject_non_string_digest_objectChecklist
git commit -s)CHANGELOG.mdupdated (for any normative change)<!-- CHANGED: #NNN: description -->in spec text