Skip to content

fix(projection): validate canonical decimal timestamps in TypeScript hrTime and skip non-finite floats in spanAttributes (#114) - #119

Open
drk-ram wants to merge 1 commit into
agentrust-io:mainfrom
drk-ram:drk-ram-fix-projection-114
Open

drk-ram wants to merge 1 commit into
agentrust-io:mainfrom
drk-ram:drk-ram-fix-projection-114

Conversation

@drk-ram

@drk-ram drk-ram commented Oct 25, 2026

Copy link
Copy Markdown

Closes #114

Summary

  • Validate canonical non-negative decimal strings (/^(0|[1-9][0-9]{0,19})$/) and safe-integer seconds in TypeScript hrTime (packages/typescript/src/projection.ts) before calling BigInt(value).
  • Skip non-finite float/number values (NaN, Infinity) in span_attributes (src/agentrust_telemetry/projection.py) and spanAttributes (packages/typescript/src/projection.ts).
  • Add Python and TypeScript unit tests in tests/test_otel_projectors.py and packages/typescript/test/metrics.test.ts.

Validation

  • pytest tests/test_otel_projectors.py -k "not real and not action_metrics" -q — passed (4 passed)

…hrTime and skip non-finite floats in spanAttributes (agentrust-io#114)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Raghuvar Durga <177662415+drk-ram@users.noreply.github.com>
@drk-ram
drk-ram requested review from a team and imran-siddique as code owners October 25, 2026 06:48
@github-actions

github-actions Bot commented Oct 25, 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 Oct 25, 2026

@dinakarjs dinakarjs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bounded review of exact head 49da7a9. I fetched packages/typescript/src/projection.ts at this SHA and executed that module directly using Node 24.22.1 with type stripping. Canonical controls 0, 1 and 1000000001 return the expected tuples; trailing LF, CR, CRLF and Unicode line/paragraph separators are rejected. The spanAttributes control retains run_id while omitting NaN/Infinity values. No new blocker found in those checked paths.

The added 20-digit ceiling already bounds seconds to at most 99,999,999,999, so the subsequent MAX_SAFE_INTEGER-seconds check cannot be reached by a value that passed that regex. Treat it as defensive/redundant, not an independently exercised boundary. A useful nonblocking addition is a committed positive control at 99999999999999999999 ns with the exact [99999999999, 999999999] tuple, alongside the 21-digit refusal.

This is direct TypeScript-module validation and source review; I have not run the repository's complete Python/TypeScript suites or schema consumers in this environment. Reviewed with OpenAI Codex.

@dinakarjs dinakarjs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completing review at unchanged exact head 49da7a9 after the earlier direct-module controls. Fresh hash-locked requirements/test.txt environment and editable local installation: complete Python unittest discovery passes 159 tests with OTEL_TRACES_SAMPLER=always_on. The TypeScript build passes; all 45 TypeScript tests pass via node --import tsx --test test/*.test.ts. npm test's tsx launcher cannot create its local IPC pipe in this environment; invoking the same test files through Node's loader avoids that launcher limitation without changing tests or source. No source changes. Approving the bounded projection/canonical-decimal fix; the suggested 20-digit positive boundary remains nonblocking. Hosted gates and maintainer disposition remain separate. Reviewed with OpenAI Codex.

Copy link
Copy Markdown
Contributor

The shared boundary-test follow-up is now a signed test-only adoption companion: dinakarjs#5 at 4d5dcb2, based on your exact 49da7a9 head. Six positive timestamp tuples, thirteen refusals and five projection cases run from the same fixture in both SDKs. Python full suite 162 passes; TS full suite 69 passes and build passes. Removing each decimal/nonfinite projection guard independently fails the new cases. No runtime/schema change or reapproval of our own tests; author adoption and independent review remain separate.

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.

projection: TypeScript hrTime accepts empty/hex/negative strings and spanAttributes projects non-finite floats

2 participants