Skip to content

Change temporary file permissions to 666 #13429

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mahan201 wants to merge 1 commit into
base: master
Choose a base branch
from
+1 −0
Open

Change temporary file permissions to 666 #13429

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/user_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,7 @@ async def post_userdata(request):
try:
with os.fdopen(fd, "wb") as f:
f.write(body)
os.chmod(tmp_path, 0o666)
os.replace(tmp_path, path)
Comment on lines +386 to 387
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid making uploaded userdata world-writable (0o666)

Line 386 broadens permissions so the final file (after Line 387) is writable by any local user. That weakens isolation and enables cross-user tampering in shared hosts. Keep the temp file private until replace, then apply a safer target mode (or preserve existing mode).

Suggested fix 
                 try:
                     with os.fdopen(fd, "wb") as f:
                         f.write(body)
-                    os.chmod(tmp_path, 0o666)
-                    os.replace(tmp_path, path)
+                    try:
+                        target_mode = os.stat(path).st_mode & 0o777
+                    except FileNotFoundError:
+                        target_mode = 0o664
+                    os.replace(tmp_path, path)
+                    os.chmod(path, target_mode)
                 except:
                     os.unlink(tmp_path)
                     raise

As per coding guidelines: “Only comment on issues directly introduced by this PR's code changes.”

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
os.chmod(tmp_path, 0o666)
os.replace(tmp_path, path)
try:
with os.fdopen(fd, "wb") as f:
f.write(body)
try:
target_mode = os.stat(path).st_mode & 0o777
except FileNotFoundError:
target_mode = 0o664
os.replace(tmp_path, path)
os.chmod(path, target_mode)
except:
os.unlink(tmp_path)
raise
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@app/user_manager.py` around lines 386 - 387, The code currently makes the
temp file world-writable with os.chmod(tmp_path, 0o666) before
os.replace(tmp_path, path); change this to keep the temp file private and apply
a safer final mode: either set the temp file to 0o600 (os.chmod(tmp_path,
0o600)) before os.replace, or preserve the existing target mode by reading the
target's mode (st = os.stat(path) if exists) and applying os.chmod(path,
st.st_mode) after os.replace; in short, remove 0o666 and ensure the final file
is created with 0o600 or the preserved existing mode using the os.chmod/os.stat
calls around os.replace.

except:
os.unlink(tmp_path)
Expand Down