fix: reject unsafe integers in proposal and reservation parsers - #11
Merged
Conversation
Duration fields were admitted with Number.isInteger, so values such as MAX_VALUE and 2^53 passed and overflowed millisecond conversion. Require safe integers whose millisecond product stays exact, and fail closed before action capacity or connector reservation is consumed.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Devin Review found 1 potential issue.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
Comment on lines
+1265
to
+1272
| assertSafeInteger( | ||
| input.evidence_plan.max_age_seconds, | ||
| "ActionProposal.evidence_plan.max_age_seconds", | ||
| { | ||
| min: 1, | ||
| max: MAX_DURATION_SECONDS, | ||
| message: "A positive evidence max age is required.", | ||
| }, |
There was a problem hiding this comment.
🟡 Published schemas accept rejected values
Oversized durations and attempt counts pass the published schemas, but assertSafeInteger rejects them. Schema-valid clients therefore receive unexpected validation errors.
Prompt for agents
Align every public contract with the new runtime numeric bounds. Add the duration maximum of 9007199254740 to max_age_seconds and remedy_window_seconds, and Number.MAX_SAFE_INTEGER-equivalent maxima to max_attempts and relevant money fields in spec/schemas/action-proposal.schema.json, spec/schemas/recourse-reservation.schema.json, spec/schemas/connector-recourse-commitment.schema.json, and api/openapi.json. Add repository checks or contract tests that prove the published schemas reject the same boundary values as ConsequenceRail and bundle validation.
Was this helpful? React with 👍 or 👎 to provide feedback.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Proposal and reservation parsers now reject non-integral, non-finite, and unsafe integer values for duration and money fields. Evidence
max_age_secondsandremedy_window_secondsmust also convert exactly to milliseconds (value * 1000remains a safe integer). Settlement-bundle validation uses the same duration bound.Why
Number.isIntegeradmitsNumber.MAX_VALUEand2 ** 53. Those values overflow when converted to milliseconds, so an evidence freshness window could becomeInfinityand never expire. Amount fields already usedNumber.isSafeInteger; duration and attempt fields did not.How tested
node ./scripts/check.jsnode --testmax_age_seconds,max_attempts,remedy_window_seconds, and amount fields fail closed without consuming action capacity or calling the connector.max_age_seconds: 1e20(422, no stored action).