fix: encode order_id to bytes before hmac.compare_digest - #29
Conversation
hmac.compare_digest raises TypeError on non-ASCII str input, so a crafted non-ASCII order_id on /keys/mine crashed with a 500 instead of the intended 404. Flagged by CodeRabbit on PR #26.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
hrco
left a comment
There was a problem hiding this comment.
🔤 ROAST: fix/order-id-ascii-compare (#29)
The good: 3-line fix. Prevents a 500. Correct.
The roast:
This is the PR equivalent of finding one cockroach and calling the whole family to witness it with a full write-up. Three lines of actual fix, 6 lines of test, and a PR description that reads like you defused a bomb. You did not defuse a bomb. You called .encode("utf-8").
"Flagged by CodeRabbit review on PR #26" — so a bot found this for you. In a 3-character diff. You are outsourcing your QA to automation and then publishing the receipt. I appreciate the honesty but maybe keep the "a robot did my code review" part as an internal note.
The real question: who is sending non-ASCII order IDs? Did someone try to order a license key in UTF-16? You built an entire paywall bypass security system (/keys/mine with order_id second factor, HMAC comparison, constant-time comparison) and it folds to a French accent. hmac.compare_digest does not like é — join the club, mate.
Verdict: Correct fix. But the fact that CodeRabbit had to teach you about encoding gives me pause. Merge it quietly.
Summary
hmac.compare_digestraisesTypeErrorwhen given non-ASCIIstrinput. A craftedorder_idon/keys/minecontaining non-ASCII characters crashed the request with a 500 instead of the intended 404.main→releasepromotion of the paywall fix).Test plan
test_non_ascii_order_id_returns_404_not_500— reproduces the crash pre-fix, passes post-fix.test_key_provisioning.pysuite passes (12/12).test_tool_chains.py(pytest-asyncio plugin registration) are unchanged by this diff.