ci: install jsonschema for manifest contract tests - #277
Conversation
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8d145201b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if timezone in {"Factory", "posixrules"}: | ||
| _invalid() |
There was a problem hiding this comment.
When a manifest uses calendar.timezone: "localtime", ZoneInfo resolves it successfully on common systems, but the zone is determined by that machine's local configuration. The current deny-list therefore accepts evidence whose meaning can change across hosts while its canonical bytes and digest remain identical, undermining research reproducibility; reject localtime as well or restrict this field to stable IANA zone identifiers.
Useful? React with 👍 / 👎.
| if not isinstance(parsed, dict): | ||
| _invalid() | ||
| return validate_research_input_manifest(parsed) | ||
| except (InvalidResearchInputEvidence, UnicodeDecodeError, TypeError, ValueError, json.JSONDecodeError): |
There was a problem hiding this comment.
When an untrusted payload contains roughly 1,000 nested arrays or objects, json.loads raises RecursionError, which is not covered by this exception handler. read_research_input_manifest_json therefore leaks an unexpected exception instead of the documented InvalidResearchInputEvidence, so callers that rely on the public error type can be terminated by malformed input; reject excessive nesting or translate RecursionError here.
Useful? React with 👍 / 👎.
| numerator = int(fraction[1:]) if fraction else 0 | ||
| denominator = 10 ** (len(fraction) - 1) if fraction else 1 |
There was a problem hiding this comment.
On supported Python 3.10 runtimes, or newer runtimes with the integer digit limit disabled, an untrusted timestamp can contain an arbitrarily long fractional part because the regex and schema impose no maximum. Converting that fraction and constructing 10 ** digits consumes superlinear CPU and large memory before the manifest can be rejected or compared, allowing a moderately large payload to stall validation; cap fractional precision consistently in the schema and validator or compare bounded strings without constructing huge integers.
Useful? React with 👍 / 👎.
1 participant
Adds jsonschema only to the CI test install list. The retained F0 product files are unchanged.