On 23. 5. 26 10:04, orbisai0security (via GitHub) wrote:
orbisai0security commented on PR #36: URL:https://github.com/apache/subversion/pull/36#issuecomment-4524721722Thanks, both points make sense.I agree the patch should be split. The `assert` → explicit exception changes are independent from the file-mode change, and I’m happy to keep those as a separate cleanup if you think they’re worth committing. On the file-permission hardening: fair point about `~/.subversion/auth` already being created as `0700` by `ensure_authdirs()`. Given that, I agree this should not be presented as a security bug in the normal/default threat model. At most, creating the temp file as `0600` would be defence-in-depth for unusual/manual configurations where the directory permissions have been loosened, but that does not seem like something Subversion needs to treat as a vulnerability. I’ll rework this accordingly: separate the `assert` → `raise` cleanup from the file-mode change, and I’m fine dropping the file-mode part if maintainers don’t think it is useful. Is that okay?
Such a serious conversation with an AI agent that, having **completely** missed the point the first time – that it did not in fact find a vulnerability – now simulates nodding wisely and agreeing to redo the patch. Even though what remains of it is reduced to using exceptions instead of asserts, which hardly makes any semantic difference.
Blech. Tell me again how this helps, when two people had to spend time reviewing and pointing out beginners' mistakes?
Oh, by the way, we require Python 3.6 to run tools and tests. The usedforsecurity keyword was introduced in 3.9.
-- Brane
