Change temporary file permissions to 666

[mahan201]

Set permissions of the temporary file to 666 before replacing.

Why?
ComfyUI can be used (as is with our case) in a collaborative environment where files and workflows are shared between users. Previous versions of ComfyUI (last version I checked was v0.11.0) were saving with readable permissions.
This change just changes the permissions of the temp file before the replace call.

[coderabbitai]

📝 Walkthrough

Walkthrough

The change adds a single line to the file upload handling logic in app/user_manager.py. When processing POST /userdata/{file} requests, the code now sets file permissions to 0o666 on the temporary file immediately after writing the uploaded content. This occurs before the temporary file is atomically moved to replace the target file path using os.replace(). No modifications were made to the request handling flow, error handling mechanisms, or public APIs.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly and concisely describes the main change: applying 666 permissions to a temporary file before replacement.
Description check	✅ Passed	The description clearly explains the change (setting temp file permissions to 666) and provides context about collaborative environments where file sharing requires group/other readability.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/user_manager.py`:
- Around line 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.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8212ad04-0cc1-42f2-b21b-23ead397764c

📥 Commits

Reviewing files that changed from the base of the PR and between e9a2d1e and 8717b26.

📒 Files selected for processing (1)

• app/user_manager.py

[coderabbitai]

⚠️ 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.

[asagi4]

I don't think ComfyUI should be changing the permissions like this. It's insecure.

Instead you should give your users membership in the group you run ComfyUI as, or maybe configure umask for the server user if you really want world-writable files. On Windows you can probably add an inheritable ACL to the target directories to give your users access to others' files.