1bit comfy: run the checked binary; fetch the submodule when missing

[bong-water-water-bong]

Follow-up to #65: fixes both things PR-Agent flagged there (#65 was merged before this commit went up).

Running the binary safely:

• 1bit comfy opens the binary once and checks it through that open file: a regular file, executable, and not writable by other users.
• It then runs that same file with fexecve, so nothing can swap it between the check and the run.
• ONEBIT_COMFYUI must be an absolute path, and relative PATH entries are skipped.

Tested on strixhalo:

• a normal run gives the same image (50.6 dB against ComfyUI);
• a relative path, a world-writable binary, a missing binary and a non-executable file each fail with a clear message (exit 127).

Submodule: CMake (-DONEBIT_COMFYUI=ON) and scripts/build-comfyui.sh fetch third_party/comfyui.cpp themselves when it's missing. In a fresh clone with no submodules, configuring fetched it at the pinned 4a08238.

🤖 Generated with Claude Code

[context7]

Docs7 for 1bit-monster/engine

Result	Status	Action
Deployment	➖ Not used	—
Content review	➖ Did not run. This site has no agent runs available this month. Wait for the monthly reset or check your Docs7 plan.	—

Commit 9a5ad41

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 9a5ad41)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 65 - Partially compliant Compliant requirements: Brings ComfyUI.cpp into the engine Pinned submodule third_party/comfyui.cpp at 4a08238 Build with -DONEBIT_COMFYUI=ON runs scripts/build-comfyui.sh Run with 1bit comfy <workflow.json> looks for binary in $ONEBIT_COMFYUI, then build's, then PATH License compliance: ComfyUI.cpp is GPL-3.0, engine is Apache-2.0, runs as separate program Docs updated in docs/comfyui.md, README, and NOTICE Non-compliant requirements: None Requires further human verification: Verification that the binary runs correctly on strixhalo hardware with the specified PSNR measurements Confirmation that the submodule fetching works correctly in a fresh clone without submodules

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Security: Potential race condition in binary execution The code uses fexecve to execute the binary after checking its properties, which is good for preventing file swapping attacks. However, the check for world-writable permissions (st.st_mode & S_IWOTH) is done after opening the file descriptor, but before fexecve. If another process modifies the file between the check and execution, it could lead to a security issue. While fexecve ensures the same file is executed, the check for world-writable permissions should be atomic with the execution to prevent any race condition. if (st.st_mode & S_IWOTH) { ::close(fd); throw std::runtime_error(bin + " is writable by other users; refusing to run it"); } Submodule fetching logic The script fetches the submodule only if CMakeLists.txt is missing and if the repository is a git repository. This logic might not cover all edge cases, such as when the directory exists but is not a valid git repository or when the submodule is corrupted. It's important to ensure that the submodule is properly initialized and at the correct commit. # Fetch the pinned submodule when it is missing (not its own third_party/ComfyUI: # that pin is only for the parity check). if [ ! -f "$src/CMakeLists.txt" ] && git -C "$root" rev-parse --git-dir > /dev/null 2>&1; then echo "fetching third_party/comfyui.cpp" git -C "$root" submodule update --init third_party/comfyui.cpp fi [ -f "$src/CMakeLists.txt" ] || { echo "third_party/comfyui.cpp is empty and could not be fetched: git submodule update --init third_party/comfyui.cpp"; exit 1; }

[github-actions]

Persistent review updated to latest commit 9a5ad41