Repository navigation
fix(platform_measurement): reject non-string or empty runtime.measurement across Python and TypeScript (#628) - #633
Conversation
…ment across Python and TypeScript (agentrust-io#628) Closes agentrust-io#628 Signed-off-by: Raghuvar Durga <177662415+drk-ram@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
🟡 Contributor Check: MEDIUM
Automated check by AgenTrust Contributor Check. |
lywinged
left a comment
There was a problem hiding this comment.
REVIEW on #633, head 5599617: Approve
Measured on 5599617 with the hash-pinned requirements, on 3.11 and on 3.12, the steps ci.yml runs: ruff, the dash check and mypy clean; 4787 passed and 48 skipped on both; the verifier context replay experiment 116 passed and 4 skipped; the portability step's first command 4786 passed and 49 skipped on both, its conformance run 958 passed, 2 skipped and 5 xpassed, its review-bar tests under -OO 52 passed and its two --help commands clean. The same steps on the merge commit the checks run against, a901f7a with this head, on 3.12: 5150 passed and 98 skipped, then 116 and 4, then 5149 and 99, with the same conformance and -OO results. The package's own suite, npm test under node 22, 161 passed on the head and on that merge commit. The two Python files give 86 passed, as the description says, where the merge base collects 85, the TypeScript check being the one new test. Against the merge base da69b31 the review bar sorts the two tests the description names as failing on behaviour, which is the description's count, and the other 84 as pass or skip. The change does what #628 asks and nothing else.
-
Confirmed, and where it reaches. Through
verify_recordin either language a signed record whoseruntime.measurementis absent,null, the empty string, 123,trueor a list is refused by the schema before the platform check runs, on the merge base as on this head, measured on a re-signed vector: Python'sverify_recordraises on the schema and the TypeScript verifier fails withschema_invalid. So the change is reachable only through a direct call to the exported helper, which is the scope the description gives, "on direct calls", and what thefield_absentrow ofexamples/platform-measurement/README.mdalready says. On a direct call the merge base returnedappraised, with the bad value carried in the evidence, for the empty string, 123,Trueand the list in Python, and for the first three in TypeScript, where the array already fell tomeasurement_mismatchbecause an array never equals its copy. This head returnsnot_attemptedwith causefield_absentfor all six in both, measured. On the whole suite on a copy, one mutation at a time, 4787 passing before any: the Python check reverted to the merge base's failstest_P9_a_non_object_record_is_refused_and_a_missing_measurement_is_field_absentand nothing else, and the TypeScript check reverted failstest_ts_non_string_or_empty_runtime_measurement_is_field_absentand nothing else. Two narrower fixes a reviewer might have written: a string required with the empty string still allowed fails the same one test in each language, on the empty-string case alone; a falsy check,if not measurement, refusesNone, the empty string,0andFalse, and their TypeScript counterparts, and passes 123,Trueand the list, probed in both languages, so it fails the same one test in each language, on 123. So of the six cases,{}andNonewere alreadyfield_absenton the merge base, the empty string is what holds the emptiness half of the check, and 123,Trueand the list are what hold the type half. A non-empty string that is no digest,not-a-digest, still reaches the appraiser on a direct call and comes backappraisedin both languages, measured. The digest form is the schema's andverify_recordapplies it, which is the right place for it. -
Two sentences the change leaves behind. The docstring of
check_platform_measurementstill says "A record without runtime.measurement reports not_attempted with cause field_absent", and thefield_absentrow of the vectors' README still says "the record carries noruntime.measurement". Both now also cover ameasurementthat is present and not a non-empty string. One clause each. The cause name itself,field_absentfor a field that is present, is the issue's choice and the description's;Causehas no closer member, and adding one would reach the TypeScript type and that table, so it is fine as it is once the two sentences say so. -
Small things. The entry under Changelog entry is one bullet in the file's style, names #628 and the author, and says "on direct
check_platform_measurement/checkPlatformMeasurementcalls", which is the scope;CHANGELOG.mditself is untouched, as CONTRIBUTING step 5 on main and the Changelog guard now ask, the merge base predating both. The spec section named, 3.1, names no JSON type for the field; its 3.1.5 calls it a digest, the schema makes it a string in the digest form, and the docstring's "measurement, the string it appraised" is about the report member, so the non-empty string is the issue's proposal and the schema's form. Nothing to change in any of them.
Approve. Item 2 is two clauses; nothing here waits on them.
What this changes
Closes #628.
src/agentrust_trace/platform_measurement.py(check_platform_measurement) &trust-record-verify/src/platform.ts(checkPlatformMeasurement):runtime.measurementto be a non-empty string (isinstance(measurement, str) and bool(measurement)in Python;typeof measurement === "string" && measurement !== ""in TypeScript), returningnot_attempted(cause="field_absent") whenruntime.measurementis absent, empty, or not a string on direct calls.Type of change
Spec section
spec/trace-v0.2.mdsection 3.1 (runtime.measurement).Tests for a fix
tests.test_platform_measurement::test_P9_a_non_object_record_is_refused_and_a_missing_measurement_is_field_absenttests.test_trust_record_verify_citations_and_platform::test_ts_non_string_or_empty_runtime_measurement_is_field_absentpython -m pytest tests/test_platform_measurement.py tests/test_trust_record_verify_citations_and_platform.py -q->86 passedpython tools/review_bar.py --base origin/main --head HEAD->2 fail on behaviour, 0 fail on a missing name or fileChangelog entry
agentrust_trace.platform_measurement,@agentrust-io/trust-record-verify): treat non-string or emptyruntime.measurementvalues asnot_attempted(cause='field_absent') on directcheck_platform_measurement/checkPlatformMeasurementcalls (fix(platform_measurement): reject non-string or empty runtime.measurement across Python and TypeScript #628, @drk-ram).Checklist
git commit -s)CHANGELOG.mdupdated (for any normative change)<!-- CHANGED: #NNN: description -->in spec text