mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-30 19:09:28 +00:00
fix(file_ops): apply umask-default permissions in _atomic_write for new files (#70856)
This commit is contained in:
parent
9ceac1896e
commit
fbfee8e405
2 changed files with 73 additions and 0 deletions
|
|
@ -552,3 +552,73 @@ class _DeletedTestGitBaselineCheck:
|
|||
helper is restored or replaced.
|
||||
"""
|
||||
pass
|
||||
|
||||
|
||||
# =========================================================================
|
||||
# Atomic write: umask-default permissions for new files
|
||||
# =========================================================================
|
||||
|
||||
class TestAtomicWriteNewFilePermissions:
|
||||
"""_atomic_write should apply umask-default perms to new files (not 0600)."""
|
||||
|
||||
def test_generated_script_contains_umask_else_branch(self):
|
||||
"""The shell script should contain the umask-computed chmod for new files."""
|
||||
mock = MagicMock()
|
||||
mock.cwd = "/tmp"
|
||||
ops = ShellFileOperations(mock)
|
||||
# _atomic_write is private; exercise it via write_file which delegates
|
||||
result = ops.write_file("/tmp/nonexistent/new_file.txt", "hello")
|
||||
# The mock always returns success, but we captured the generated script
|
||||
script = mock.execute.call_args[0][0]
|
||||
assert "else" in script, (
|
||||
"Expected 'else' branch for new-file mode, got:\n" + script
|
||||
)
|
||||
assert "umask" in script, (
|
||||
"Expected 'umask' call in else branch, got:\n" + script
|
||||
)
|
||||
assert "(0666 & ~0" in script, (
|
||||
"Expected umask-computed mode in else branch, got:\n" + script
|
||||
)
|
||||
assert "chmod" in script, (
|
||||
"Expected chmod of computed mode in else branch, got:\n" + script
|
||||
)
|
||||
|
||||
def test_new_file_gets_umask_default_permissions(self, tmp_path):
|
||||
"""Newly created file should get umask-computed perms, not mktemp's 0600.
|
||||
|
||||
Uses a real subprocess so the shell script actually runs.
|
||||
"""
|
||||
env = MagicMock()
|
||||
env.cwd = str(tmp_path)
|
||||
|
||||
def execute(command, **kwargs):
|
||||
completed = subprocess.run(
|
||||
command,
|
||||
shell=True,
|
||||
text=True,
|
||||
capture_output=True,
|
||||
input=kwargs.get("stdin_data"),
|
||||
)
|
||||
return {
|
||||
"output": completed.stdout + completed.stderr,
|
||||
"returncode": completed.returncode,
|
||||
}
|
||||
|
||||
env.execute = execute
|
||||
ops = ShellFileOperations(env)
|
||||
dest = tmp_path / "new_file.txt"
|
||||
assert not dest.exists()
|
||||
|
||||
result = ops.write_file(str(dest), "test content\n")
|
||||
assert result.error is None, f"write failed: {result.error}"
|
||||
assert dest.exists()
|
||||
|
||||
# Compute expected mode: 0666 & ~umask
|
||||
current_umask = os.umask(0)
|
||||
os.umask(current_umask) # restore
|
||||
expected_mode = 0o666 & ~current_umask
|
||||
actual_mode = dest.stat().st_mode & 0o777
|
||||
assert actual_mode == expected_mode, (
|
||||
f"Expected mode {expected_mode:04o} (umask {current_umask:04o}), "
|
||||
f"got {actual_mode:04o}"
|
||||
)
|
||||
|
|
|
|||
|
|
@ -1022,6 +1022,9 @@ class ShellFileOperations(FileOperations):
|
|||
'if [ -e "$t" ]; then '
|
||||
'm="$(stat -c%a "$t" 2>/dev/null || stat -f%Lp "$t" 2>/dev/null || true)"; '
|
||||
'[ -n "$m" ] && chmod "$m" "$tmp" 2>/dev/null || true; '
|
||||
# new file: apply umask-computed default instead of mktemp's 0600
|
||||
'else '
|
||||
'u="$(umask)"; chmod $(printf '"'"'%04o'"'"' $((0666 & ~0$u))) "$tmp" 2>/dev/null || true; '
|
||||
"fi; "
|
||||
'cat > "$tmp"; '
|
||||
'mv -f "$tmp" "$t"; '
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue