Skip to content

moe_trace: create the trace file 0644 and fail if it cannot be written - #153

Merged
bong-water-water-bong merged 1 commit into
mainfrom
fix/codeql-extended
Sep 26, 2026
Merged

bong-water-water-bong merged 1 commit into
mainfrom
fix/codeql-extended

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

Fixes the high-severity alert #5 (cpp/world-writable-file-creation) from the extended CodeQL suite, which is now on for this repo.

  • fopen(path, "w") creates the file with mode 0666 and leaves the rest to the umask. The trace is now opened with an explicit 0644.
  • If the path can't be opened, the tool stops with a message instead of writing through a null FILE*.

Checked with g++ -std=c++17 -fsyntax-only against the pinned llama.cpp headers.

Alert #4 (cpp/path-injection, 1bit comfy running ONEBIT_COMFYUI or a comfyui_cpp found on PATH) is dismissed as a false positive. 1bit isn't setuid, so whoever sets that environment is the user running it. The code already refuses world-writable binaries and fexecves the exact file it checked.

🤖 Generated with Claude Code

CodeQL cpp/world-writable-file-creation (alert 5): fopen("w") creates with
0666 and leaves the rest to the umask. Open with an explicit 0644, and stop
with a message instead of writing through a null FILE* when the path is bad.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@context7

context7 Bot commented Sep 26, 2026

Copy link
Copy Markdown

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 2e1b7bf

@github-actions

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

5 - Partially compliant

Compliant requirements:

  • Remove the CPU reference and tokenizer
  • Port the existing engine instead
  • Remove engine/core, engine/backends/cpu, the tests and golden tools, and their docs
  • Rewrite the README, the porting map and contributing rule 1
  • Keep the license, NOTICE, the copyright check and CI

Non-compliant requirements:

  • None

Requires further human verification:

  • None

4 - Partially compliant

Compliant requirements:

  • 1bit-server speaks Lemonade's backend protocol
  • Takes Lemonade's launch flags
  • Has /health, /v1/models, /v1/chat/completions and /v1/completions endpoints
  • Supports streaming (SSE, [DONE])
  • Has OpenAI usage and llama-server timings telemetry
  • Uses GGUF's own Jinja template with chat_template_kwargs
  • Routes <tool_call> to reasoning_content
  • Supports stop strings and end-of-turn detection
  • Implements prompt cache with cache_n
  • Uses pinned deps: nlohmann/json 3.12.0, cpp-httplib 0.57.1 and minja 021c2293
  • Has docs/server.md covering the contract, verification and what's not done yet
  • Includes server_e2e.py for verification
  • Includes golden_template_qwen3 for CI
  • Has unit tests: test_text_stream, test_openai, test_sampler

Non-compliant requirements:

  • None

Requires further human verification:

  • None
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

File permissions and error handling for trace file

The change modifies the trace file creation to use open() with explicit permissions 0644 instead of fopen(), which previously used the default 0666 mode left to the umask. While this improves security by avoiding world-writable files, it introduces a potential failure point if the file cannot be opened. The code now checks for tfd < 0 and returns an error with strerror(errno) if the file cannot be opened, which is a good improvement. However, the error message could be more informative by including the specific path that failed to open, and the handling of the file descriptor and FILE* could be made more robust to prevent potential resource leaks if fdopen() fails.

// the trace is data for the owner: 0644, not fopen's 0666 left to the umask
const int tfd = open(a[4], O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC, 0644);
w.f = tfd < 0 ? nullptr : fdopen(tfd, "w");
if (!w.f) { fprintf(stderr, "cannot write %s: %s\n", a[4], strerror(errno)); return 1; }

@bong-water-water-bong
bong-water-water-bong merged commit 068afb7 into main Sep 26, 2026
9 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the fix/codeql-extended branch September 26, 2026 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant