Jens-G commented on PR #3594:
URL: https://github.com/apache/thrift/pull/3594#issuecomment-4698154211

   ### Code review
   
   Found 2 issues:
   
   1. The second commit message contains "so extreme size_t values cannot wrap 
the position-plus-length check and **bypass EOF enforcement**." [AGENTS.md 
ยง6](https://github.com/apache/thrift/blob/35c1a53dd6de205a4a7c23fc356f1fba490116c4/AGENTS.md#L87-L96)
 says to use neutral functional language for serialization bounds changes and 
never describe what an attacker can gain. A neutral rewrite would be: "Use an 
overflow-safe remaining-bytes comparison in the direct bytes decode path to 
prevent size_t wrapping."
   
   
https://github.com/apache/thrift/commit/6af2e54e8655ab3c59f26cdfd418c3621a8a439d
   
   2. AI tool use is acknowledged in the PR body but neither commit contains 
the required `Co-Authored-By:` or `Generated-by:` label ([AGENTS.md 
ยง4](https://github.com/apache/thrift/blob/35c1a53dd6de205a4a7c23fc356f1fba490116c4/AGENTS.md#L57-L71)
 says "**Always** label AI-assisted commits and PRs โ€ฆ Apply this label even 
when AI only generated a portion of the change").
   
   
https://github.com/apache/thrift/blob/35c1a53dd6de205a4a7c23fc356f1fba490116c4/AGENTS.md#L57-L71
   
   ๐Ÿค– Generated with [Claude Code](https://claude.ai/code)
   
   <sub>- If this code review was useful, please react with ๐Ÿ‘. Otherwise, react 
with ๐Ÿ‘Ž.</sub>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to