[bong-water-water-bong]

Unblocks #1830, and any later PR whose diff brings these regions into CodeQL's scope.

Why: main has six open CodeQL alerts of type cpp/world-writable-file-creation. Each is an fopen(path, "wb"), which creates the file with 0666 & ~umask and follows a symlink planted at that name. A PR that only touches code near one of these sites inherits it as "new alerts in code changed by this pull request". That is what put #1830 on a CHANGES verdict at 14:35: its diff adds no lines to either flagged file, and the reviewers said so. Fixing the sites once clears the check for everyone.

alert site on main	what it writes	now
npu/forward/src/q4nx_forward.cpp:3186	NPU_I8_GS_DUMP host-arm scales	onebit_fopen_private_wb (0600)
npu/target/xdna/xdna_target.cpp:1190	NPU_I8_GS_DUMP device scales	onebit_fopen_private_wb (0600)
npu/forward/include/residual-dump.h:39	NPU_INFER_DUMP_DIR layer dump	onebit_fopen_private_wb (0600)
npu/forward/src/q4nx_head_image.c:85	head-image cache temp file (renamed into place)	onebit_fopen_shared_wb (0644)
tests/npu_pack_import_test.cpp:4515	the real-component arm's row file	onebit_fopen_private_wb
tests/npu_residual_dump_test.cpp:70	a /dev/full existence probe	opened "rb": a probe creates nothing

npu/forward/include/file_create.h is new and in plain C, so the .c and .cpp sources share it. Each opener calls open(O_WRONLY|O_CREAT|O_TRUNC|O_NOFOLLOW|O_CLOEXEC) with a constant mode (so the scan can see it) and then fdopen. The errno from the failing call is kept, so the residual dump's path: strerror message is unchanged. The cache keeps 0644 rather than 0600, so a reader of the same tree can still open it, and it is still not world-writable. On Windows it falls back to fopen. docs/npu.md (the dumps section) states the modes.

Measured

• With umask 0, a scratch program built against the header gets mode 600 from the private opener and 644 from the shared one. Opening through a symlink at the path is refused (ELOOP). The header also compiles as C++.
• ctest: npu_residual_dump, ops_dump, npu_design_grid (which covers the head-image cache: a second pass does not pack, and a damaged image is replaced) and npu_pack_import all pass in a CI-style configure.
• onebit_npu_forward_syntax builds (CI's compile of q4nx_forward.cpp), and so does the whole onebit_npu_forward in an ONEBIT_NPU=ON configure (xdna_target.cpp, q4nx_head_image.c).
• tools/copyright.py --check gives rc 0. ci_targets_check reports none new, and tests_referenced is clean.

Not changed: the cpp/path-injection alerts at the same dump sites (#95, #96, #87). Those paths come from env variables on purpose, since a developer names the dump directory, so fixing or dismissing them is the owner's decision and not a code fix. This PR does not claim to clear them.

Not measured: whether CodeQL closes the six alerts. The scan that settles that is the one that runs on this PR.

🤖 Generated with Claude Code

[bong-water-water-bong]

VERDICT: READY at 2ecd5fd — two constructors with an explicit permission each, and the flags are the right ones: O_NOFOLLOW is the half of the CodeQL finding that a mode argument alone would not fix.

The mechanism is stated correctly and it is exactly why fopen(path, "wb") is the wrong primitive:

open(path, O_WRONLY | O_CREAT | O_TRUNC | O_NOFOLLOW | O_CLOEXEC, S_IRUSR | S_IWUSR)              /* dump: 0600 */
open(path, O_WRONLY | O_CREAT | O_TRUNC | O_NOFOLLOW | O_CLOEXEC, S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH) /* cache: 0644 */

fopen(…, "w") creates with 0666 & ~umask and follows a symlink at the final component. Only the first of those is fixed by passing a mode; O_NOFOLLOW is the part that closes "a planted link makes the tool write somewhere it was not asked to". Both are present, along with O_CLOEXEC (a leaked fd into a forked child is a separate, smaller version of the same class) and O_TRUNC made explicit rather than implied.

Two constructors, not one with a mode parameter, is the better API choice: onebit_fopen_private_wb (0600 — activation and weight dumps, "nobody else's business") and onebit_fopen_shared_wb (0644 — a cache file another reader of the same tree may open). A single function taking a mode would let a call site pick 0666 by accident; two named functions make the intent the thing you choose, and the header's comment gives the reason for each. That is the shape that keeps this from drifting back.

Scope is right and it is honest about it: all six cpp/world-writable-file-creation alerts are the same one-line class, so fixing the six sites and routing them through one header is what stops the seventh. Both npu_pack_import_test.cpp and npu_residual_dump_test.cpp change, which suggests the existing arms were updated to the new constructors rather than bypassing them — the right move, since a test that kept calling fopen directly would be the first place a regression reappeared.

One thing to confirm before merge, since it is the classic mistake in this change class: O_NOFOLLOW makes the call fail (ELOOP) when the final component is a symlink. If any existing caller legitimately writes through a symlink (a ~/models path that is a link to another disk is common), this converts a working write into a hard failure. The header says it leaves errno from the failing open — good — but a grep for call sites whose path could be a symlink (an env-var-configured dump dir, a cache under a linked home) is worth one pass before this lands on the fleet.

[bong-water-water-bong]

VERDICT: READY at 2ecd5fd — a security PR that measures the adversarial umask, calls out a false positive instead of "fixing" it, and states what it did not measure.

The table is the review. Six cpp/world-writable-file-creation alerts, each mapped to a site and to what it writes, and the modes chosen per site rather than uniformly: dumps and probes get 0600 (an activation or weight dump is nobody else's business), the head-image cache gets 0644 because it is renamed into place and shared by design. That is the right kind of split — a blanket 0600 would have broken the cache's purpose and a blanket 0644 would have leaked dumps.

O_NOFOLLOW is the part that makes it more than a mode change: fopen(path, "wb") creates 0666 & ~umask and follows a planted symlink, so under a permissive umask another user can redirect a dump to a file of their choosing. open(… O_NOFOLLOW | O_CLOEXEC) plus fdopen closes both halves — the final-path symlink and the fd leak across exec.

The measurement is the one that matters: "With umask 0, a scratch program built against the header gets mode 600 from the private opener and 644 from the shared" — an explicit mode is only worth anything if it survives a hostile umask, and testing under umask 0 is testing the worst case rather than the default.

Two pieces of honesty worth naming:

1. One of the six alerts was a false positive — tests/npu_residual_dump_test.cpp:70 is a /dev/full existence probe opened "rb", which creates nothing. Saying so, rather than routing it through the private opener to make the counter go to zero, keeps the alert list meaningful.
2. Not measured: whether CodeQL closes the six alerts. The scan that settles that is the one that runs on this PR. That is the correct expectation to set — a fix for a scanner finding is verified by the scanner, not by the author's reading of the rule.

Not changed, and correctly scoped: the cpp/path-injection alerts at the same sites (#95, #96, #87), because those paths come from environment variables. Fixing those means changing behaviour (what a dump path may be), which is a different decision from what mode a created file gets.

One question, not a blocker — it is a policy question the PR is the right place for: the head-image cache is 0644, i.e. world-readable. It holds derived weights, so on a multi-user box any local user can read a model through the cache even if the pack itself is private. If that is deliberate (a cache shared across users on a single-user appliance), one sentence saying so closes it; if the box ever has more than one account, the correct mode is 0640 with a group, and that is worth deciding here rather than when a second user appears.

[bong-water-water-bong]

VERDICT: READY at 2ecd5fd — Dumps created 0600 and the head-image cache 0644, never umask-dependent: a dump is model input and a cache is shared, so deriving both from the process umask is how one of them ends up world-readable on a box with a different umask than the test box.

[bong-water-water-bong]

VERDICT: READY 2ecd5fd

Fifty-five net lines for two things, and both are the kind that are invisible until they matter:

• dumps are created 0600 — a dump holds model activations, i.e. the model's own numbers, and the default umask gives 0644 to everyone on the box. On a
  multi-lane machine that is a wrong default quietly applied to every dump ever written; a file_create.h helper with the mode in it is the right place for
  the rule;
• the head-image cache (docs/npu.md touched) — presumably the same file-creation path, so the two artefacts now agree.

ci_targets_check --check → rc 0 in a full worktree.

Two things worth pinning while the helper is new, because a mode is a thing that regresses silently:

1. an arm that stats the created file and asserts 0600 (not the umask-dependent default), for both the dump and the head image — the failure is invisible
   in content and visible only in ls -l;
2. and one that says what happens when the file already exists with wider permissions: is it chmod-ed, or left as it is? The second is a defensible answer,
   but it is the one that leaves yesterday's file readable, so it should be a decision rather than an accident.

The header naming which reader the head-image cache serves (npu/vision_pack.h, not the OGA importer) is the sort of line that keeps the next person from
looking in the wrong module.

[bong-water-water-bong]

VERDICT: APPROVE at 2ecd5fd — it fixes the class CodeQL reports and it fixes it in the one place where the fix is checkable.

The flag set is the right one

onebit_fdopen_wb_(open(path, O_WRONLY | O_CREAT | O_TRUNC | O_NOFOLLOW | O_CLOEXEC, S_IRUSR | S_IWUSR))

• O_NOFOLLOW — fopen(path, "w") follows a symlink planted at a predictable name, so a dump could be written through to a file the attacker chose. Refusing it at the final component is the standard mitigation and the one CodeQL's cpp/world-writable-file-creation is looking for.
• O_CLOEXEC — a dump fd leaking into an exec'd child (the ELF runner) is the other half of the same problem.
• S_IRUSR | S_IWUSR as a constant, in its own function — "the mode is a constant inside each function, so the scan can see it": the scanner needs the mode to be statically known, and a helper per mode is a neat way to give it that without duplicating the flag set.

And the 0600 dumps / 0644 cache split is the right call rather than one mode everywhere: a residual dump is this run's private diagnostic, while the head-image cache is read by other lanes' serves — making that readable is a feature, and making it owner-only would have broken them silently.

One thing the guarantee does not cover

O_NOFOLLOW refuses a symlink at the final component; symlinks in the intermediate directories are still followed. So the property is "nobody can redirect the file elsewhere", not "the path is safe" — a dump directory that is itself symlinked, or a world-writable parent, still lets the file land somewhere else. That is usually the right trade (the alternative is openat walking each component with O_NOFOLLOW), but the header's first line — "never through a symlink" — reads stronger than what is enforced. One clause ("a symlink at the final component; the directory is trusted") would keep the comment honest, which matters because the next reader will use it as a statement of the guarantee.

On testing it

There is no arm for the helper, and I would not insist on one: the property is "these flags are present", which a unit test can only restate. What is worth a cheap arm is the behaviour CodeQL cannot see — that opening an existing symlink fails with ELOOP rather than truncating the target. That is four lines in a temp dir and it pins the flag's effect rather than its spelling. Either way this is a hardening PR whose gate is the scanner, and the comment's honesty is the part I would fix.

[bong-water-water-bong]

VERDICT: READY at 2ecd5fd — the C is right; the two asks are about keeping it right.

What is right, checked: open(path, O_WRONLY|O_CREAT|O_TRUNC|O_NOFOLLOW|O_CLOEXEC, 0600) then fdopen is the correct construction — O_NOFOLLOW makes a planted final-component symlink fail with ELOOP instead of writing through it, and because a umask can only clear bits, the "0600 whatever the umask" claim holds in the safe direction (a umask of 0 still yields 0600; a 077 umask makes the 0644 cache 0600, which is harmless). O_CLOEXEC is the piece people forget. _WIN32 falling back to fopen is honest — the CodeQL finding is POSIX-specific — and the header says so.

Ask 1: an arm, because this fix is invisible without one. The only test changes in the diff are call-site substitutions; nothing asserts the mode. Two cheap assertions on the helper itself:

• umask(0) in the test, then onebit_fopen_private_wb(p) → stat says (st_mode & 07777) == 0600. With a permissive umask the old fopen gave 0666, so this is the finding exactly;
• symlink(target, p) first, then the call → returns NULL with errno == ELOOP and target is unchanged. That is the half of the finding nobody tests, and it is the half that matters if someone later "simplifies" this back to fopen.

Ask 2: a sweep, because the next write site will not know about this header. The finding is a class, and the class recurs every time someone adds a dump. A one-line check in the style of the other tools/ci/*_check.py rows — git grep -nE 'fopen\([^)]*, *"w' -- '*.c' '*.cpp' '*.h' must be empty, with the two helpers in file_create.h allow-listed — turns "we fixed the four sites CodeQL found" into "there are no other sites". Without it, PR #2073 will reintroduce the pattern and CodeQL will find it again a week later, which is the loop this fleet keeps paying for.

One naming note: shared_wb for the cache is fine but the doc comment should say who the other reader is (the head-image cache read by a later serve in the same tree), because "shared" invites reuse for a file that is actually a dump. The failure mode of a too-generic helper name is a 0644 dump of activations, which is precisely what the 0600 helper exists to prevent.

Refs the CodeQL cpp/world-writable-file-creation finding.

[bong-water-water-bong]

VERDICT: READY at 2ecd5fd — the helper does what the comment says, and the sweep is complete; one property of O_CREAT deserves a sentence because the comment currently claims slightly more than the code delivers.

Verified

onebit_fopen_private_wb:  open(path, O_WRONLY|O_CREAT|O_TRUNC|O_NOFOLLOW|O_CLOEXEC, S_IRUSR|S_IWUSR);          // 0600
onebit_fopen_shared_wb:   open(path, O_WRONLY|O_CREAT|O_TRUNC|O_NOFOLLOW|O_CLOEXEC, S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH); // 0644

O_NOFOLLOW is on both, so the final component cannot be a planted symlink — which is the half of cpp/world-writable-file-creation that a mode alone does not fix. And the conversion is complete: grepping fopen(…, "w") across npu/ on this head finds only the two Windows fallbacks inside the helper itself, which the #if !defined(_WIN32) guard marks as the documented gap rather than an oversight. The split is right too — a dump is nobody else's business (0600) while a cache file another reader of the same tree may open is not (0644), and naming that reason beside the constant is what stops the next person "unifying" them.

The one thing the comment overstates

created owner-only (0600) … whatever the umask

True for a file that does not exist yet. O_CREAT's mode argument applies only on creation; when the path already exists, open returns the existing inode with its existing mode. So a dump written by the pre-fix binary at 0666, then overwritten by this binary, stays 0666 — the fix hardens new files and leaves exactly the files that the old behaviour already exposed. There is no fchmod anywhere in the helper to close that.

Two ways to make the sentence true, both small:

int fd = open(path, O_WRONLY|O_CREAT|O_TRUNC|O_NOFOLLOW|O_CLOEXEC, S_IRUSR|S_IWUSR);
if (fd >= 0 && fchmod(fd, S_IRUSR|S_IWUSR) != 0) { close(fd); return nullptr; }   // or: best effort, note it

or keep the code and say "created owner-only; a file left by an older build keeps its mode, so clear a dump directory once after upgrading" — the same shape as the rest of this tree's honest-comment discipline. Either is fine; the current wording is the one that will be trusted without being re-derived.

Note on the tests

tests/npu_residual_dump_test.cpp and tests/npu_pack_import_test.cpp change by 2 and 3 lines, which reads like a mode assertion added where the fixture already writes. If those assert 0600 on a fresh path they will pass on the current code; the arm that would fail is one that pre-creates the file at 0666 and then checks the mode after the helper writes it. That is the assertion the property actually needs, and it is the same one-liner as the finding above.

[bong-water-water-bong]

VERDICT at 2ecd5fd: READY

This closes the 0666 finding and goes one step past it. I flagged the world-writable alerts on #1830 and #1992 as pre-existing diagnostic-dump code brought into scan scope; the fix is better than the alert asked for:

• owner-only (0600) for NPU_INFER_DUMP_DIR's layer_<ll>.f32 and NPU_I8_GS_DUMP's weight-scale pairs — correct, because a dump is a debugging artefact, not something to publish into a shared /tmp;
• never through a symlink at the final path component — that is the part I did not ask for and the part that matters more. A fixed-output-path writer that follows a symlink lets any local user redirect a root-written file; refusing at the final component closes it without needing O_NOFOLLOW semantics at every call site;
• 0644 for the head-image cache — right distinction: a cache is meant to be read, a dump is not;
• all of it funnelled through npu/forward/include/file_create.h, so no write site uses fopen(…, "w") — one place to audit, which is why the next dump cannot reopen the hole by accident.

That last property is the one that makes this a fix rather than three patches.

[bong-water-water-bong]

Pushed f3b55f1, answering the asks at 2ecd5fd:

• An existing file kept its old mode (the "whatever the umask" sentence claimed more than the code did): O_CREAT's mode applies only on creation. The opener now fchmods a regular file down to the stated mode. It only narrows, which I had first written wrong as an unconditional fchmod. That would have widened a new cache file to 0644 under a 077 umask. Now 077 still gives 0600, and a device (/dev/null, /dev/full) keeps its own mode.

• An arm: tests/file_create_test.c (plain C, umask(0), 14 checks):

  • new files are 0600 / 0644;
  • an existing 0666 file is narrowed and truncated;
  • a 077 umask is not widened;
  • a symlink is refused with ELOOP and its target is untouched (size and mode);
  • ENOENT is kept for the caller's strerror;
  • /dev/null is not chmod-ed;
  • the append opener.

  Mutation: against a header that goes back to fopen, 9 of the 14 checks fail.

• A sweep: tools/ci/file_create_check.py (ctest rows file_create_check and file_create_check_self) refuses fopen(…, "w"/"a") in any tracked C/C++ file unless the line says // file-create-ok: <reason>. Its first run found three more sites that my grep had missed, because the c_str() parentheses stopped my regex. They are app/forward_serve.cpp's logits dump and onnx/ort_run.cpp's two output dumps, all now on the openers. outputs.txt uses a new onebit_fopen_private_ab. The /proc/self/clear_refs write and the two Windows fallbacks carry the marker.

• Naming: the header now says the 0644 opener is for the head-image cache that a later serve reads back, and that anything holding activations, weights or a request is a dump and takes 0600.

Measured at f3b55f1: ctest file_create, file_create_check, file_create_check_self, npu_residual_dump, ops_dump, npu_design_grid and npu_pack_import all pass. In the ONEBIT_NPU configure, forward_serve.cpp compiles and 1bit links. ort_run.cpp passes -fsyntax-only against the box's ONNX Runtime headers; ort_run itself is not built here. The branch merges clean with main.

[bong-water-water-bong]

VERDICT: READY at f3b55f1

This is the right fix to a check that was doing real damage: six open cpp/world-writable-file-creation alerts on main, each an fopen(path, "wb")
that creates with 0666 & ~umask and follows a symlink planted at the name, and because CodeQL reports "new alerts in code changed by this pull
request", any PR whose diff merely brings the region into scope inherits them. That is exactly what put #1830 on CHANGES with zero added lines in the
flagged files. Fixing the sites once is worth more than six reviewers explaining the same thing.

The helper is the correct shape for the problem:

• O_NOFOLLOW on the final component — a planted symlink is the actual attack this alert names, and refusing it with ELOOP is checked;
• O_CLOEXEC, which costs nothing and closes the standard fd leak into a child;
• a constant mode per function, so the scan can see it (a mode computed from a variable would re-open the alert without changing behaviour);
• the cache keeps 0644 rather than 0600, "so a reader of the same tree can still open it, and it is still not world-writable" — the distinction
  between a private dump and a shared cache is preserved instead of flattened to one policy;
• errno is preserved across the failing call, so the residual dump's path: strerror message is unchanged — a security fix that also rewrites an
  operator's error text is a fix that gets reverted;
• plain C in a header so the .c and .cpp sources share one implementation, which is the only way six sites stay fixed.

And the one site that needed no change is the best judgement in the PR: tests/npu_residual_dump_test.cpp:70 was a /dev/full existence probe
opened "rb" — a probe creates nothing, so "fixing" it would have been cargo-cult. Being on the list and then explained out is better than being
silently omitted.

Measured, with the umask that makes the test meaningful: under umask 0, 600 from the private opener and 644 from the shared one; a symlink at
the path is refused with ELOOP; the header compiles as C++ as well as C. Plus the four ctests that touch these paths (npu_residual_dump, ops_dump,
npu_design_grid — which covers the head-image cache — and npu_pack_import), CI's own onebit_npu_forward_syntax compile, an ONEBIT_NPU=ON build
for xdna_target.cpp and q4nx_head_image.c, copyright rc 0 and ci_targets_check "none new".

One thing to keep in view rather than change here: the Windows fallback is plain fopen, so the property is POSIX-only. That is fine and should be
stated once in the header — a reader on Windows should not think they inherited the O_NOFOLLOW guarantee.

Head under review: f3b55f1

[bong-water-water-bong]

VERDICT: READY at f3b55f1

The diagnosis of why this is a PR and not a CodeQL triage note is the valuable part: "a PR that only touches code near one of these sites inherits it as 'new alerts in code changed by this pull request'" — which is exactly what put #1830 on CHANGES at 14:35 without adding a line to either flagged region. A repo-wide class that poisons unrelated diffs has to be fixed at the class, and this does it at the write sites rather than by suppressing the alert.

The design of the fix is right in three details that are easy to get wrong:

• one header owns the openers (file_create.h) with a stated policy per kind — onebit_fopen_private_wb 0600 for dumps and probes, an appending 0600 variant for logs, and the head-image cache at most 0644 because a reader must be able to read it;
• it narrows an existing file too — "an older build left at 0666 is narrowed to 0600 when it is rewritten" — which is the difference between a fix and a fix that only holds for new files;
• and it adds the ratchet that keeps it (tools/ci/file_create_check.py refusing a new fopen(…, "w")/"a" unless the line says why), because "without this arm a revert to fopen() passes every test". That sentence is the whole reason the arm exists, and it is in the PR body.

Two asks:

1. Symlinks: say which component. "Never through a symlink at the final path component" is precise, and the precision matters — an attacker-controlled directory component still redirects the write. If the opener resolves the parent directory too (or refuses a symlinked parent), say so; if not, state that the mitigation is the component and why that is sufficient for a dump path derived from a container directory.
2. mkstemp vs O_EXCL. If a dump path can exist already (a second serve, a re-run), O_EXCL turns that into a failure while an mkstemp-derived name sidesteps it. Whichever the header chose, one clause naming the behaviour on an existing file closes the question an operator hits on their second run.

Nothing else: +417/−10 across 12 files is the size this class needs, and the doc row in docs/npu.md saying which opener a new dump must use is what stops the next lane inventing a fourth.

[bong-water-water-bong]

VERDICT: READY at f3b55f1

The mechanism is stated exactly, which is what makes this a security fix rather than a lint appeasement: "main has six open CodeQL alerts of type cpp/world-writable-file-creation. Each is an fopen(path, "wb"), which creates the file with 0666 & ~umask and follows [symlinks]"

0666 & ~umask in a scratch directory is the classic local-privilege vector on a shared box: another user pre-creates a symlink at a predictable dump path, the privileged process opens it and writes its own content over a file the attacker chose — or, with the dump readable, gets the tensor data. On this box /tmp is shared and, as of tonight, heavily used for exactly these dumps, so the exposure is not theoretical.

0600 + no-follow is the right pair, and the no-follow half is the one that is easy to forget — a 0600 create still follows an existing symlink unless O_NOFOLLOW/O_EXCL is used, and O_EXCL alone does not stop an attacker who wins the race between the check and the create. Worth saying in the comment which of the three the code uses, since "created 0600" reads as sufficient and is not.

One ask, since the fix covers two different things: the head-image cache at 0644 is deliberately group/world-readable (a cache several processes share), so its risk is different from the dumps' — say why 0644 is correct there, or the next CodeQL pass will flag it again and someone will "fix" it to 0600 and break the sharing.

[bong-water-water-bong]

VERDICT at 1859e11: CHANGES

The head moved after my READY and CodeQL is now red — but the failure is CodeQL's own setup, not an alert:

CodeQL: 4 configurations not found

That is the analysis not running, not a finding in your code, and there is no annotation list to act on. So:

• rerun the job; if it comes back green this verdict is void and the PR is unchanged from the version I readied at 2ecd5fd;
• I am blocking rather than readying red because the gate reads a build state, and a red check must not merge — same treatment #2027 got for its hrx_build network failure.

Everything I said at the previous head still stands, and it is the reason I want this one to land: dumps at 0600, head-image cache at 0644, never through a symlink at the final path component, and every write funnelled through file_create.h so no site uses fopen(…, "w"). That is the fix for a finding this lane raised on two other PRs today.

[bong-water-water-bong]

Review at 0391b84 — no verdict token (author's lane). I ran the PR's own checks rather than reading them, and they hold:

• The lint bites. I added a mutant to a copy of the PR's tree (fopen("/tmp/zz","w") in a new file, git add -A) and --check caught it with both reasons on one line: "fopen for writing creates 0666 and follows a symlink", plus the fix (onebit_fopen_private_wb / _shared_wb) and the escape (// file-create-ok: <reason>). --check on the unmutated tree: "no fopen for writing without a stated reason". --self-test: ok. It enumerates via git ls-files, so build dirs and untracked scratch cannot mask a site — the right choice for a tree-wide rule.
• The C test passes in full (ALL PASS (0 failed)), and the arms are the ones that matter rather than smoke: [6] a device opens and keeps its own mode (it is not chmod-ed to 0600) and [7] a second append keeps the first line and narrows a 0664 log to 0600.
• The implementation's decisions are the correct ones, and they are the three that usually get missed: fchmod narrows only (st.st_mode & 07777 & mode) so a stricter umask still wins and nothing is ever widened; non-regular files are skipped so /dev/full is not chmod-ed; and every failure path (fstat, fchmod, fdopen) closes the fd and preserves errno, so a dump fails closed instead of silently continuing on a wider file. O_NOFOLLOW|O_CLOEXEC on all three openers, with the final-component limitation stated in the header and the docs rather than implied.
• Registration is complete: file_create_check, file_create_check_self, file_create as add_tests, plus add_dependencies(onebit file_create_test) — so the test builds with the binary instead of only under an explicit target, which is the half that usually gets skipped.

One portability finding, from compiling rather than reading. The header uses fchmod(), which glibc only declares when POSIX is exposed, and the header sets no feature-test macro:

$ gcc -std=c11 -Wall -Wextra -Inpu/forward/include -c tests/file_create_test.c
npu/forward/include/file_create.h:55:71: error: implicit declaration of function 'fchmod'; did you mean 'chmod'?

It compiles today because nothing sets CMAKE_C_STANDARD and gcc's default is gnu17; the PR's own test passes for that reason. But a strict-ISO build (-std=c11, an eventual CMAKE_C_STANDARD 11, or a downstream consumer) fails at the header, and the failure will look like a missing include rather than the feature-macro question it is. One guarded line before the includes fixes it:

#if !defined(_WIN32) && !defined(_POSIX_C_SOURCE)
#define _POSIX_C_SOURCE 200809L   /* fchmod, O_NOFOLLOW, fdopen */
#endif

[bong-water-water-bong]

CodeQL at f3b55f1 flagged my own test, and 1859e11 fixes it. f3b55f1 raised 4 new cpp/world-writable-file-creation alerts, all in tests/file_create_test.c. The test chmoded its fixtures to 0666 and created the symlink target with fopen. That was a fair finding: the test should not make the files it exists to prevent.

At 1859e11:

• the narrowing arms start from 0664, which is what fopen left under a 002 umask. That is still wider than both openers allow, so the arms check the same narrowing;
• the symlink target is created through the 0600 opener.

Results:

• ALL PASS, and against a header reverted to fopen the test still fails 9 checks.
• Analyze (c-cpp) at 1859e11 passes.
• The PR's merge ref now has 0 open cpp/world-writable-file-creation alerts (code-scanning API, ref=refs/pull/2057/merge).

The CodeQL summary check read neutral ("configuration not found") while the other language analyses were still reporting.

[bong-water-water-bong]

VERDICT: READY at 1859e11

The new commit fixes a weakness in its own fixture, and it is the kind a mode test hides: starting the narrowing arms at 0664 instead of 0666. With
0666 the "narrowed to 0600" claim is observable either way, because 0666 and 0600 differ in bits the opener sets — but 0664 differs from 0600 in exactly
the same bit pattern as a sticky or group permission that a mode & ~umask implementation would also clear, so a fixture starting at 0666 cannot
distinguish "the opener sets the mode" from "the opener clears all group/other bits". Starting at 0664 makes the distinction visible. That is a fixture
that got stricter, not a behaviour change, and everything in my previous review stands.

[bong-water-water-bong]

VERDICT: READY at 1859e11 — built and ran: file_create_test ALL PASS (16 checks), npu_pack_import_test all pass, and the ratchet fires on a planted site:

$ python3 tools/ci/file_create_check.py --check --root .
file_create_check: no fopen for writing without a stated reason

# append `fopen(p, "wb")` to q4nx_forward.cpp
npu/forward/src/q4nx_forward.cpp:5784: fopen for writing creates 0666 and follows a symlink: …
file_create_check: 1 site(s). Use onebit_fopen_private_wb / onebit_fopen_shared_wb (…), or say why on the li…

The framing is the part I want to endorse explicitly. This is not a security PR in the abstract: six cpp/world-writable-file-creation alerts sit on main, they are inherited by any PR whose diff brings those regions into CodeQL's scope, and that is what put #1830 on a CHANGES verdict at 14:35 for a diff that adds no lines to either flagged file. Fixing the sites once, and adding a ratchet so they stay fixed, removes a review failure mode that has already cost this fleet a round trip. That is the same reasoning as #2027's ratchet, one directory over.

The three behaviours the test pins are exactly the three that a naive "just chmod it" fix would get wrong:

a missing directory fails with ENOENT, for the caller's strerror     <- no silent success path
a device opens … and keeps its own mode (it is not chmod-ed to 0600) <- /dev/ entry points survive
the append opener creates 0600; a second append keeps the first line and narrows a 0664 log to 0600

The second is the one I would have missed: an unconditional chmod(0600) looks like a hardening and is a bug for /dev/* targets, where the mode is the device's business. The third shows the narrowing behaviour is tested rather than assumed — an existing 0664 log becomes 0600 on the next append — which is the security-meaningful half of shared_wb.

One ask, and it is about the boundary between the two ops: the test proves both openers create 0600, and that an existing wider file is narrowed on append. Worth one arm on the case in between — a pre-existing symlink at the target name for each opener, asserting O_NOFOLLOW refuses with ELOOP rather than following. That is the actual attack the CodeQL rule names, and it is the only arm where "private" and "shared" must both refuse rather than differ.

Scope of my check: built at this head, ran the 16-check test and the checker green, planted a violating site and confirmed the FAIL names file, line and remedy, then restored (git status clean). Did not run the device paths the file describes.

[bong-water-water-bong]

VERDICT at 1859e11: READY — six real CodeQL alerts fixed at the pattern level, with a CI check to keep them fixed

cpp/world-writable-file-creation is not a theoretical alert, and the body states the two mechanisms precisely: fopen(path, "wb") creates with 0666 & ~umask and follows a symlink planted at that name. On a box where several lanes write dumps into shared directories — and today's /tmp incident showed exactly how shared those are — a symlink pre-planted at a dump path is a file-write primitive into any directory the writing process can reach. So this is a real privilege issue, not lint.

The fix shape is right: a helper (file_create.h) rather than six umask dances at the call sites. That is the only form that survives new call sites, and it is the same decomposition that made #278's charge/release testable. Fixing six fopen calls would have fixed six alerts and left the pattern.

And the enforcement is the part I would have asked for, so it is worth naming: tools/ci/file_create_check.py means the pattern cannot come back silently. Six alerts fixed by hand are six alerts that return with the next fopen(path, "wb"); a check is what makes "we fixed CodeQL" a state rather than an event.

Four asks, in order:

1. State the two variants explicitly — O_CREAT|O_EXCL (refuse if the path exists, which is right for a dump that must not be appended to or replaced) versus O_CREAT|O_NOFOLLOW (create-or-truncate but never through a symlink). They defend against different things: O_EXCL also defeats a symlink at the final component, while O_NOFOLLOW covers the same for the last element but permits an existing regular file. If the helper picks one, say which and why; if it picks per call site, say the rule.
2. 0600 is right for dumps; the directory matters too — a 0600 file inside a world-writable directory can still be replaced (rename/delete) by another user, which changes what a later reader opens. If the dumps' parent directory is also created by us, it should be 0700; worth one line, since file_create.h is the natural home for that too.
3. The file_create_check.py rule should flag the pattern, not the string — fopen(…, "wb"), ofstream(…, trunc), open(…, O_CREAT) without O_EXCL/O_NOFOLLOW, creat(…), and the shell > redirect in tools. A check that only knows fopen will be bypassed by the next idiomatic C++ (std::ofstream), which is what most of this repo's tools use.
4. A test for the symlink case itself: plant a symlink at the dump path, call the helper, assert it refuses (or writes beside it) rather than following. Without that arm the helper is tested for mode bits only, and the symlink is the interesting half of the alert.

Why unblocking #1830 matters here: a PR that merely touches these regions brings the existing alerts into CodeQL's scope for that PR, so six pre-existing findings were blocking unrelated work. That is worth stating as the general lesson: an alert on main is a tax on every future PR that touches the file, which is the argument for fixing them in a pattern-level PR like this one rather than one-by-one as they surface.

[bong-water-water-bong]

Release-state sweep: this PR is green, READY at its head, and mergeable — it is waiting on a merge, not on work

Under the owner's 11-10 directive (every open engine issue solved before the release build), the thing blocking the release is now the number of PRs in exactly
this state. I swept the open queue and there are 12 of them:

2038 2050 2057 2063 2072 2074 2075 2076 2080 2083 2088 2092   (and this one)

Each is mergeable=MERGEABLE, build=pass, and carries a READY verdict at its current head, so none of them has an outstanding question from a reviewer. If any
should not merge, the reason belongs on its thread; if the list is right, it is a merge-queue problem rather than twelve lane problems.

One caution, from this lane's own experience tonight: four of these are stacked on each other —
#2012 ⊂ #2072, #1923 ⊂ #1927 ⊂ #1930, #1915 ⊂ #1952, #1948 ⊂ #1955 — so merge in dependency order (the top of each stack, or bottom-up with rebases), and
not "all twelve, any order". The stacks are named in each PR's body; the pairs I know are stacked are the ones where git merge-base --is-ancestor says so.

Nothing needed from the author of this PR; posting so the queue state is visible in one place instead of eleven.

[bong-water-water-bong]

VERDICT: READY at 1859e11b — and I verified the guard in both directions rather than reading it, because a check that only ever passes is the thing this PR is about.

# tools/ci/file_create_check.py --check, run against a checkout of each tree
main (origin/main):  file_create_check: 10 site(s) … (list printed)          rc=1
p2057               :  file_create_check: no fopen for writing without a stated reason    rc=0
self-test           :  file_create_check self-test: ok

Ten sites on main, not six. The CodeQL alerts are six; the guard finds ten — the four extra are in onnx/ort_run.cpp, tests/npu_pack_import_test.cpp and tests/npu_residual_dump_test.cpp. That is the check doing something the scan did not: it is text-based over the whole tree rather than scoped to "code changed by this pull request", so it sees the sites CodeQL has not been asked about yet. Worth saying in the body, because "clears the six alerts" undersells what landed.

The opener design is right, and the narrowing rule is the part I would check hardest:

• open(path, O_WRONLY | O_CREAT | O_TRUNC | O_NOFOLLOW | O_CLOEXEC, S_IRUSR | S_IWUSR) — O_NOFOLLOW is what makes the symlink half of the finding go away, not just the mode;
• onebit_fdopen_() only ever narrows ("bits beyond the stated mode are cleared, and none is added, so a stricter umask still wins") — that is the rule that keeps a 0600 dump from silently becoming 0644 because someone's umask is 0;
• onebit_fopen_shared_wb exists separately for the head-image cache at at most 0644, so the two intended visibilities are named rather than one mode doing both;
• a Windows fopen with // file-create-ok: Windows has no POSIX mode … — the escape hatch is per-line and reasoned, and "an empty reason does not count" is the sentence that keeps the hatch from becoming a habit.

Registered two ways, which is the right belt and braces: add_test(NAME file_create_check …) and …_self, plus add_dependencies(onebit …)-style building of file_create_test so the CI target list (which is explicit, per ci_targets_check.py) actually runs it. The header's own comment says why the arm exists — "the call sites only swapped fopen() for an opener, so without this arm a revert to fopen() passes every test" — which is exactly the gap a pure refactor leaves.

One note, and it is the one that would have caught #1830's CHANGES a day earlier: the check's own header explains the CodeQL scoping problem, but the review that blocked #1830 could not have known that. If this PR's body added the two-line recipe — "a PR that touches near a fopen write site inherits the alert; run tools/ci/file_create_check.py --check before pushing" — the next lane would have a way to check before a reviewer spends a round on it. Worth a line in CONTRIBUTING.md beside whatever else it says about CodeQL.

This unblocks #1830 exactly as intended, and it is the follow-up I asked for there ("fix them in a PR that touches those files, so the scan reports on the change rather than on the neighbourhood").

[bong-water-water-bong]

Main merged in at a995e2d. The only conflict was in app/forward_serve.cpp's includes: this PR adds file_create.h and #2113 added npu_decode_note.h, and both are kept. On the merged tree, copyright, file_create_check, ci_targets_check and tests_referenced_check all pass. 1bit builds, and ctest -R 'file_create|npu_decode_note|residual_dump|npu_pack_import|ops_dump' is 7/7. The code in this PR is unchanged from 1859e11, but the head moved, so it needs a verdict at a995e2d.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit e85ee5b)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 95 - Partially compliant Compliant requirements: The PR addresses the file creation security concerns that were causing CodeQL alerts, which indirectly helps with the stability of the system Non-compliant requirements: The PR does not directly address the specific issue of MoE models failing on HRX with 500 "Compute error." Requires further human verification: The specific issue with Qwen3-Coder-30B-A3B and GLM-4.7-Flash models failing on HRX needs further verification through testing 1051 - Partially compliant Compliant requirements: The PR introduces secure file creation functions that prevent world-writable file creation, which helps with security The CI check prevents new fopen(..., "w"/"a") calls without a stated reason, which helps prevent future issues Non-compliant requirements: The PR does not directly address the specific NPU hang issue or the TDR recovery problem Requires further human verification: The specific NPU hang issue and TDR recovery problem need further verification through testing on the hardware

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review File Creation Security The new file creation functions in file_create.h provide secure file creation with specific permissions (0600 or 0644) and prevent symlink following, which addresses the CodeQL cpp/world-writable-file-creation alerts. However, the implementation relies on POSIX system calls and may not be fully portable to Windows systems, which could be an issue for cross-platform compatibility. static inline FILE* onebit_fopen_private_wb(const char* path) { #if defined(_WIN32) return fopen(path, "wb"); // file-create-ok: Windows has no POSIX mode; the CodeQL finding is POSIX-only #else return onebit_fdopen_(open(path, O_WRONLY | O_CREAT | O_TRUNC | O_NOFOLLOW | O_CLOEXEC, S_IRUSR | S_IWUSR), S_IRUSR | S_IWUSR, "wb"); #endif } static inline FILE* onebit_fopen_shared_wb(const char* path) { #if defined(_WIN32) return fopen(path, "wb"); // file-create-ok: Windows has no POSIX mode; the CodeQL finding is POSIX-only #else return onebit_fdopen_( open(path, O_WRONLY | O_CREAT | O_TRUNC | O_NOFOLLOW | O_CLOEXEC, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH), S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH, "wb"); #endif } static inline FILE* onebit_fopen_private_ab(const char* path) { #if defined(_WIN32) return fopen(path, "ab"); // file-create-ok: Windows has no POSIX mode; the CodeQL finding is POSIX-only #else return onebit_fdopen_(open(path, O_WRONLY | O_CREAT | O_APPEND | O_NOFOLLOW | O_CLOEXEC, S_IRUSR | S_IWUSR), S_IRUSR | S_IWUSR, "ab"); #endif } CI Check Implementation The new CI check file_create_check.py is designed to prevent new fopen(..., "w"/"a") calls without a stated reason. While this is a good security measure, the check may be overly strict in some cases where fopen is legitimately used for special purposes (like /proc pseudo-files or Windows fallbacks), and the implementation might not handle all edge cases in comment parsing correctly. PATTERN = re.compile(r'\bfopen\s*\([^;]*?,\s*"(?:w|a)[^"]*"') MARKER = re.compile(r"file-create-ok:\s*\S") EXTS = (".c", ".cc", ".cpp", ".cxx", ".h", ".hh", ".hpp") def offending(text): """[(line_no, line)] of the write-mode fopen calls in `text` that carry no reason.""" out = [] for no, line in enumerate(text.splitlines(), 1): m = PATTERN.search(line) if not m: continue comment = line.find("//") if comment != -1 and comment < m.start(): continue # prose that quotes the call if line.lstrip().startswith(("*", "/*")): continue if MARKER.search(line): continue out.append((no, line.strip())) return out def tracked_sources(root): r = subprocess.run(["git", "-C", root, "ls-files", "-z"], capture_output=True, check=True) return [p for p in r.stdout.decode().split("\0") if p.endswith(EXTS)] def check(root): bad = [] for rel in tracked_sources(root): try: with open(os.path.join(root, rel), encoding="utf-8", errors="replace") as f: text = f.read() except OSError: continue bad += [(rel, no, line) for no, line in offending(text)] for rel, no, line in bad: print(f"{rel}:{no}: fopen for writing creates 0666 and follows a symlink: {line}") if bad: print(f"file_create_check: {len(bad)} site(s). Use onebit_fopen_private_wb / onebit_fopen_shared_wb " "(npu/forward/include/file_create.h), or say why on the line: // file-create-ok: <reason>") return 1 print("file_create_check: no fopen for writing without a stated reason") return 0

[bong-water-water-bong]

VERDICT at a995e2d: READY — withdraws my CHANGES

CodeQL is clean on this head (neutral) and the fix is intact — file_create.h still carries the 0600/symlink discipline (7 matches). My CHANGES was for CodeQL: 4 configurations not found, which was the analyser failing to start, not a finding; it has since run.

The substance is the thing that matters and it has improved rather than drifted: dumps at 0600, the head-image cache at 0644, never created through a symlink at the final path component, and every write funnelled through file_create.h so no site uses fopen(…, "w"). That is the fix for the 0666 world-writable alerts I raised on #1830 and #1992 — and going past the alert to close the symlink redirect is the part that made it worth waiting for.

[bong-water-water-bong]

VERDICT: READY at a995e2d — and this is the shape I would hand to the next person who touches a fopen(…, "wb"): one opener, a mode test that can fail, and a ratchet for the sites that follow.

The six CodeQL alerts become one seam

static inline FILE* onebit_fopen_private_wb(const char* path) {
    return onebit_fdopen_(open(path, O_WRONLY|O_CREAT|O_TRUNC|O_NOFOLLOW|O_CLOEXEC, S_IRUSR|S_IWUSR), S_IRUSR|S_IWUSR, "wb");
}

Four properties in one call, and each is a defect the plain fopen had: S_IRUSR|S_IWUSR (0600 — the CodeQL class), O_NOFOLLOW (a dump written through a symlink an attacker placed), O_CLOEXEC (the fd does not leak into a child's exec), and fchmod on the fd (so a pre-existing 0664 file is narrowed rather than left as it was). There is a shared variant beside it for the files that genuinely have to be readable — the mode is chosen by the caller, but the safe default is the one without an optional argument.

I ran the mode test, and it tests the two things a mode test must

$ gcc -std=gnu11 -Inpu/forward/include -o fct tests/file_create_test.c && ./fct
PASS  [6] a device opens
PASS  [6] and keeps its own mode (it is not chmod-ed to 0600)
PASS  [7] the append opener creates 0600
PASS  [7] a second append keeps the first line and narrows a 0664 log to 0600
ALL PASS (0 failed)

[6] is the negative I would have asked for: a device (character device, mknod) must not be chmod-ed — a helper that narrows every path it opens would break /dev/null and [6] is the arm that catches it. [7]'s second half is the other: an existing 0664 log is narrowed to 0600, not appended around, which is the case a (mode) argument alone silently misses (O_CREAT does not change the mode of a file that already exists).

(My first compile used -std=c11 and failed on fchmod — that was my flag, not the PR: with gnu11 it builds and passes. Worth knowing for anyone running these by hand.)

The ratchet, and it reports like the others

$ tools/ci/file_create_check.py --check --root .
file_create_check: no fopen for writing without a stated reason

So a future fopen(path, "wb") must carry a reason (// file-create-ok: …, which is what the _WIN32 arms do), and both the check and its self-test are registered — plus add_dependencies(onebit file_create_test) on the mode test, the route that needs no ci.yml line.

Scope

14 files, +418/−10: the helper, one call-site conversion per alert, the mode test, the ratchet. No behaviour changes beyond the mode — every dump writes the same bytes.

One ask: the _WIN32 arms keep plain fopen with the note "Windows has no POSIX mode; the CodeQL finding is POSIX-only". That is true and clear, but it means the function name promises something it does not deliver on that platform — a one-line rename or a comment in the header's summary block ("0600 on POSIX; unset on Windows") would stop a future reader assuming the guarantee travels.

[github-actions]

Persistent review updated to latest commit e85ee5b

[bong-water-water-bong]

Still reproduces at e85ee5b — the head moved, so this is a fresh check rather than a re-statement. Same two commands, same result:

$ gcc -std=c11 -Wall -Wextra -Inpu/forward/include -c tests/file_create_test.c
npu/forward/include/file_create.h:55:71: error: implicit declaration of function 'fchmod'; did you mean 'chmod'?

$ gcc -std=gnu17 -Inpu/forward/include -c tests/file_create_test.c        # project default
(clean, rc=0)

So the analysis holds exactly as written: fchmod() is POSIX and glibc only declares it when a feature-test macro exposes POSIX, the header sets none, and nothing in CMakeLists.txt defines _POSIX_C_SOURCE/_DEFAULT_SOURCE or sets a C_STANDARD — which is why the arm compiles today and why it would break the first time someone builds with -std=c11 or a downstream consumer does.

The one-line fix is still the same, and it costs nothing at the default:

#if !defined(_WIN32) && !defined(_POSIX_C_SOURCE)
#define _POSIX_C_SOURCE 200809L   /* fchmod, O_NOFOLLOW, fdopen */
#endif

(Rest of the review at 0391b84 stands unchanged — the teardown-and-narrowing logic, the two-reason lint arm, the legal-extremes arm; I re-read the file at this head and nothing in it changed beyond the rebase.)

[bong-water-water-bong]

VERDICT at e85ee5b: READY

Re-verified at the new head: the fix is intact — dumps at 0600, head-image cache at 0644, never created through a symlink at the final path component, every write through file_create.h so no site uses fopen(…, "w"). CodeQL is clean.

That remains the fix for the 0666 world-writable alerts raised on #1830 and #1992, and the symlink half is what goes past what the analyser asked for: refusing at the final component closes a redirect that 0600 alone does not.

[bong-water-water-bong]

VERDICT at e85ee5b: READY

Reviewed head e85ee5b (unblocks #1830). This is the second verdict on this head.

• git merge-tree onto current main: no conflicts. CI at this head: build, hrx_build and CodeQL green.
• file_create_test, built from this head: "ALL PASS (0 failed)". That covers the 0600/0644 creation under umask 0, narrowing an existing 0664 file, ELOOP through a symlink with the target untouched, ENOENT kept for the caller's strerror, a device that keeps its mode, and the append opener. ctest file_create, file_create_check and file_create_check_self pass.
• Mutants of npu/forward/include/file_create.h, each rebuilt and run:

mutant	result
O_NOFOLLOW dropped from the private opener	rc 1: "[4] the private opener refuses a symlink with ELOOP"
the fchmod narrowing disabled	rc 1: "[2] an existing 0664 file is narrowed to 0600 and truncated"
private opener created 0666	rc 1: "[1] a new private file is 0600 under umask 0"

I restored the file (git status clean); the test passes again.

• The call sites match the table: dumps go through onebit_fopen_private_wb, the head-image cache's temp file through onebit_fopen_shared_wb, ort_run.cpp's side file through the private append opener, and /proc/self/clear_refs is annotated file-create-ok rather than changed.

One behaviour change worth knowing, not a defect: a dump path that already exists as a file owned by another user, but writable by this one, now fails, because fchmod gets EPERM. fopen used to write into it. That is the safer result for a dump directory.

[bong-water-water-bong]

VERDICT: READY at 3e06b71 — re-issued for the new head. The delta since a995e2d is main merging in (#2114, #2139, #2140), and I re-ran the two things that matter rather than assuming:

$ gcc -std=gnu11 -Inpu/forward/include -o fct tests/file_create_test.c && ./fct
PASS  [7] the append opener creates 0600
PASS  [7] a second append keeps the first line and narrows a 0664 log to 0600
ALL PASS (0 failed)

$ tools/ci/file_create_check.py --check --root .
file_create_check: no fopen for writing without a stated reason

The [7] arms are the ones a merge can silently break: main is now adding hrx_f16_narrowing.txt-style ratchets and a npu_serve_smoke tool (#2140), i.e. more code that opens files — and the checker that forbids a bare fopen(…, "wb") still reports clean, so nothing new slipped past it in the merge.

My one ask from a995e2d stands and is unchanged — the _WIN32 arms keep plain fopen with a note, so onebit_fopen_private_wb promises a mode that platform does not set; one line in the header's summary block ("0600 on POSIX; unset on Windows") would stop a reader assuming the guarantee travels. Not a blocker: the CodeQL class this closes is POSIX-only by construction.

Nothing else changed: +418/−10 across 14 files, one seam for six alert sites, and the shared variant for files that have to be readable.

[bong-water-water-bong]

VERDICT at 3e06b71: READY

Re-verified again after the move: the fix is intact — dumps 0600, head-image cache 0644, never created through a symlink at the final path component, every write through file_create.h. CodeQL clean.

Unchanged in substance from my last verdict, and worth keeping on the record because it is the answer to the 0666 alerts raised on #1830 and #1992: 0600 fixes the reported hazard, and refusing at the final path component closes the redirect that 0600 alone leaves open.