fix(file_ops): replace umask arithmetic with chmod "=rw" — zsh silently corrupts new-file perms - #74918
Merged
kshitijk4poor merged 1 commit intoJul 31, 2026
Conversation
Follow-ups on top of NousResearch#70888's cherry-picked fix: - Replace the $((0666 & ~0$u)) shell arithmetic with POSIX who-less 'chmod "=rw"'. zsh (reachable via _find_bash's $SHELL fallback on bash-less hosts) parses leading-zero constants as decimal and silently chmods a garbage mode (e.g. 0210); the symbolic form is spec-identical across bash/dash/busybox-ash/zsh and degrades to mktemp's 0600 (pre-fix behavior) rather than corrupting perms if chmod rejects it. - Move the new-file chmod after the content stream so the temp file stays owner-writable while cat runs. - Run the chmod on a '[ ! -e "$t" ]' check after cat instead of the stat/else branch, keeping the overwrite path untouched. - Update the stale perms comment NousResearch#70856 called out (new files did NOT land with default umask perms pre-fix). - Tests: select the atomic-write script by content instead of call order (the previous last-call capture only worked because the bare MagicMock's falsy-exit early return suppressed later execs), assert behavior at explicit umasks 0022/0002/0077 via parametrize, add an overwrite mode-preservation regression guard, and dedupe the real-subprocess env fake into make_real_subprocess_env() shared with TestSearchFilesFallbackHiddenPaths. (webtecnica's email mapping already exists in contributors/emails/ on current main; the PR's check-attribution red was stale-base only.) # Conflicts: # tests/tools/test_file_operations.py
alt-glitch
added
type/bug
kshitijk4poor
enabled auto-merge (rebase)
July 31, 2026 08:52
This was referenced Aug 3, 2026
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
2 participants
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.
Summary
Hardens the new-file umask fix that just merged in #74897: the
$((0666 & ~0$u))shell arithmetic silently corrupts permissions under zsh, which is a reachable backend shell via_find_bash()'s$SHELLfallback on bash-less hosts.Root cause
zsh parses leading-zero constants as decimal (no
octal_zeroes), so the arithmetic computes a garbage mode andchmodapplies it without error. Reproduced against current main: new file chmodded to 0210 under umask 022 (expected 0644), 0230 under 002.Changes
tools/file_operations.py: replace the arithmetic with POSIX who-lesschmod "=rw"— spec-mandated to apply the umask (verified empirically on BSD, GNU coreutils 9.7, and busybox 1.37 chmod × bash/dash/ash/zsh; correct mode in every cell). Quoted so zsh doesn't=word-expand it;|| true-guarded so an exotic chmod degrades to mktemp's 0600 (pre-fix behavior) instead of corrupting. Moved aftercatso a write-masking umask can't EACCES the content stream. Also corrects the stale perms comment tools/file_operations.py::_atomic_write leaves newly-created files at mktemp's 0600, umask never applied (third instance of #14181/#23613's bug class) #70856 called out.tests/tools/test_file_operations.py: parametrize the behavioral test over umasks 0022/0002/0077, add an overwrite mode-preservation regression guard (0755 script stays 0755), dedupe the real-subprocess env fake intomake_real_subprocess_env()shared withTestSearchFilesFallbackHiddenPaths.Validation
chmod "=rw")tests/tools/test_file_operations.py: 42 passed. E2E real-subprocess matrix (4 umasks × new/overwrite): 8/8. ruff clean.
Surfaced during the final-diff review of #74897 / #70888 (salvage of @webtecnica's fix).