Repository navigation
fix: Bump cryptography and parse token cache salts safely - #116
Merged
Merged
Conversation
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
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.
Requested by @drafty
🔴 Review tier: HIGH — human review required
What
Supersedes #96 without modifying or closing it. Permit
cryptography>=40.0.0,<49.0.1and regenerateuv.lockusinguv --no-config lock --upgrade-package cryptography --default-index https://pypi.org/simple(uv 0.12.9). Python 3.8 resolves cryptography 47.0.0; tested Python 3.9–3.13 resolve 49.0.0. Only cryptography and its CFFI dependency change package versions; regeneration also adds upstream artifact upload timestamps.Why the original CI failed
The original run logs have expired (GitHub HTTP 410). Surviving job metadata shows successful dependency installation/lint followed by pytest exit status 4 on Python 3.8/3.9, with 3.11/3.12 passing. Reproducing PR #96 at
0297d428on Python 3.8 confirms import/collection fails because Pydantic cannot evaluateusage: Usage | None; the same annotation exists in its parent, so this was pre-existing, not a cryptography incompatibility. Main already fixes it in699d53b(Optional[Usage]), which this branch inherits. The original PR changed only pyproject.toml and still installed locked cryptography 43.0.3.Current main also has an independent intermittent
test_load_save_token_diskfailure (run): splitting random salt bytes on::corrupts salts containing::or ending in:. Parse the existing fixed 16-byte salt and validate the separator instead, in both sync and async loaders. Stored bytes remain compatible. Deterministic tests exercise all sync/async writer-reader combinations and malformed headers; eight collision cases fail before the fix.How tested
ruff check asknews_sdk, wheel build, andgit diff --check.Risk / rollback
Major cryptography upgrade and token-cache parsing require human review. Python support is not dropped; the resolver selects compatible versions. The dependency floor remains 40.0.0 as in the original proposal (this is not a mandatory minimum-security-version policy change). No workflows, release configuration, token format, KDF, or encryption algorithms change. Revert this commit and release a follow-up version if needed. No merge, tag, or publication performed by this task.