Repository navigation
Conversation
Signed-off-by: Srinivasa Dinakar Jainti <dinakarjs@gmail.com>
|
🟡 Contributor Check: MEDIUM
Automated check by AgenTrust Contributor Check. |
|
Consolidation recommendation for #126 (author comparison; not independent approval). Compared #127 at 658eca6 with #128 at 47216c2. Both reject noncanonical event timestamps used for issuance and reject the Boolean approval-window shape. They differ on the finalizer's treatment of malformed copied approval metadata and on private helper outcomes. Fresh hash-locked Linux/Python 3.12 environment: our 31 TRACE tests pass. An independently rehashed snapshot with requested_at_unix_nano=True in both request and resolution is refused before signing by #127. With only #128's adapter substituted, the same snapshot produces a signed warning appraisal. The six-method timestamp control set also distinguishes the helper/outcome contracts; those cross-contract failures are not counts of independent defects. Sources were restored exactly. No affirming appraisal or approval-authority bypass was demonstrated by this comparison. My recommendation is for #127 to carry the issue because the public finalizer consistently refuses malformed timestamp representations before signing, with each-position, canonical-boundary, authentic rehashed-snapshot and no-signing controls. Please explicitly choose strict finalization refusal versus signed warning for malformed copied metadata, then select one implementation. If #128 is preferred, I will transfer the applicable controls and any owner-selected early validation into it, then close #127 after adoption. #128's unrelated None-policy-event guard should be assessed on its own contract rather than silently folded into this timestamp repair. @imran-siddique is already requested on both PRs. This comment makes the comparison/transfer path concrete; it does not authorize us to choose the maintainer outcome or approve our own code. No new competing PR, push or merge. Prepared with OpenAI Codex. |
There was a problem hiding this comment.
the core fix looks good, _timestamp() matches the existing canonical decimal-string schema and validating timestamps before appraisal/signing properly closes the int() coercion issue, the rehashed-snapshot tests that assert signing is never invoked are especially useful, also verified the behavioral difference with #128: this PR refuses finalization on malformed approval-window timestamps (e.g. expires_at_unix_nano = True) whereas #128 can still produce a signed affirming record in the same case. for a signed evidence finalizer, would prefer the strict refusal behavior here.
two points:
- kindly consolidate with #128 so that we have a single implementation
- also kindly add a few public-finalizer cases for missing required approval timestamps and a multi-candidate scenario (malformed + valid) as they would make the fail-closed guarantee clearer.
also kindly resolve conflicts and rebase to main
Reject noncanonical decimal-string timestamps before TRACE appraisal/signing, including self-consistent imported snapshots. Rehashed negative controls prevent chain-integrity failure from hiding timestamp validation.
Current head:
658eca654f4f05e8904fca19f193d6ae871b0556. Hosted Python 3.10–3.13/TypeScript, fuzzing and CodeQL checks pass. Independent review remains pending. Fresh Linux/Python 3.12 TRACE selection: 31 tests pass; the consolidation follow-up records comparison with #128.Consolidation decision pending with #128: maintain one implementation for #126. This PR refuses malformed timestamps before signing; #128 can sign a warning appraisal for a malformed copied request timestamp, while its helper verdicts differ. That is an outcome-contract choice, not proof of approval-authority bypass. Retain canonical boundary, each-position, authentic rehashed-snapshot and no-signing controls in the selected implementation.
The existing wire representation is enforced; valid streams retain behavior. This is not full event-schema revalidation or proof of approval authority. Imran's existing request remains; no competing implementation is merged by this description.
Prepared with OpenAI Codex; source head and existing review history retained.
Historical description and validation snapshots (superseded by current status above)
Summary
Fixes #126. A self-consistent imported evidence snapshot could contain boolean or otherwise noncanonical timestamps that Python's
int()coerced into an approval window or TRACE issuance time. The adapter now accepts only the existing ASCII decimal-string timestamp format before appraisal/signing, and applies the same parser in request/resolution and executed-action binding.The regression tests rehash malformed snapshots so chain-integrity validation cannot mask the timestamp defect. They exercise each timestamp position, booleans/numbers/noncanonical strings, canonical boundaries, expiry, a valid signed appraisal, and refusal before signing.
Contract impact
common.schema.json's existing timestamp representation.TraceFinalizationError.Evidence
Python 3.12, fresh environment installed with
--require-hashes -r requirements/test.txt:OTEL_TRACES_SAMPLER=parentbased_traceidratioandOTEL_TRACES_SAMPLER_ARG=0.01; removing those two process-local overrides restores the default sampler. The first run with those overrides had three unrelated tracing failures; they are not concealed as a passing run.sync_schemas.py, schema/version/OTel compatibility/TypeScript schema/OIDC checks, conformance runner, compileall,pip check, touched-file Ruff, and whitespace checks pass.Hosted checks on
658eca654f4f05e8904fca19f193d6ae871b0556are now complete: Python 3.10–3.13, TypeScript, fuzzing, CodeQL analysis and contributor check all pass; the inapplicable auto-merge job is skipped. Imran is already a requested reviewer. Independent maintainer review remains pending.Privacy and security
Limits
This is a Python TRACE-adapter timestamp repair, not a full event-schema revalidation or proof of approval authority. TypeScript, dependency locks, wire schemas and policy semantics are unchanged. Hosted CI and independent maintainer review remain required.
Prepared and validated with OpenAI Codex; DCO signed.