Skip to content

fix(trace): reject noncanonical approval timestamps - #127

Open
dinakarjs wants to merge 1 commit into
agentrust-io:mainfrom
dinakarjs:fix/trace-canonical-timestamps-126
Open

dinakarjs wants to merge 1 commit into
agentrust-io:mainfrom
dinakarjs:fix/trace-canonical-timestamps-126

Conversation

@dinakarjs

@dinakarjs dinakarjs commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

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

  • No wire-contract change: enforces common.schema.json's existing timestamp representation.
  • Valid streams retain their appraisal and issuance time.
  • Malformed approval/action times cannot establish binding; public finalization raises TraceFinalizationError.

Evidence

Python 3.12, fresh environment installed with --require-hashes -r requirements/test.txt:

  • Full Python suite: 170 tests passed. This workspace injects OTEL_TRACES_SAMPLER=parentbased_traceidratio and OTEL_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.
  • The new six-test selection fails on unmodified main (negative subtests expose the defect); all six methods pass with the repair. Final focused TRACE suite after import/style cleanup: 31 passed.
  • sync_schemas.py, schema/version/OTel compatibility/TypeScript schema/OIDC checks, conformance runner, compileall, pip check, touched-file Ruff, and whitespace checks pass.
  • Wheel and sdist build and installed-artifact smoke checks pass.

Hosted checks on 658eca654f4f05e8904fca19f193d6ae871b0556 are 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

  • Synthetic fixtures only; no production data or credentials added.
  • Security-sensitive change received maintainer review.

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.

Signed-off-by: Srinivasa Dinakar Jainti <dinakarjs@gmail.com>
@dinakarjs
dinakarjs requested review from a team and imran-siddique as code owners November 3, 2026 14:28
@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

🟡 Contributor Check: MEDIUM

Check Result
Profile MEDIUM
Credential LOW
Overall MEDIUM

Automated check by AgenTrust Contributor Check.

@github-actions github-actions Bot added the needs-review:MEDIUM Contributor check flagged MEDIUM risk label Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

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.

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

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

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.

bug: trace_adapter binds an approval whose timestamps are booleans

2 participants