security: harden model-file parsing, server, and build (untrusted-mirror threat model)

[KingIcyCreamProjects]

What

A security-hardening pass against the model-file trust boundary (colibri loads model dirs + safetensors from untrusted mirrors), plus the server and build. Every item was adversarially confirmed against the tree, with no regression vs. legitimate GLM-5.2 loading or normal server flows.

Findings fixed

Model-file trust boundary

• glm.c: qt_check_fmt() rejects (rather than silently infers) any quantized tensor whose weight/scale byte counts disagree with the config shape, at all three format sites (qt_from_disk + both expert_load arms) — closes a matmul heap OOB read and a per-row scale OOB write (SEC-1). embed_row zero-fills out-of-range token ids (SEC-5).
• st.h: cross-check numel == nbytes/esz for float dtypes + shape-product overflow guard + st_read_f32_cap at config-sized sinks (SEC-2 / B1); st_read_slice_f32 NULL + bounds guard (B6).
• glm.c: SEC-9 — bound the config.json / ref_glm.json slurps (256 MB cap + NULL-checked alloc + full-read check); the old unbounded ftell → malloc(n+1) gave a hostile file a load-time OOM or, on malloc failure, a NULL-deref via b[got]=0.
• tok.h: negative-id + json_get NULL/type guards (SEC-3); tk_read_file fseek/ftell/malloc/fread error handling + size cap (B3 / B4).
• json.h: bound \u reads against the NUL + full-keyword strncmp for true/false/null (SEC-4); growable string buffer replacing the silent 64 KB truncation (B5).

Server (openai_server.py)

• Fail-closed on non-loopback bind without a key (SEC-6); Host-header allowlist incl. OPTIONS preflight (SEC-7); auth-gate /health & /experts telemetry (SEC-8); scheduler head-of-line fix (B2); [DONE] under ka_lock (B9).

Independent corroboration

An unrelated adversarial audit re-derived SEC-1 (matmul OOB read) and SEC-2 (scale OOB write) from scratch against a tree without these fixes — same two bugs, same locations. They're real, not theoretical.

Status / ask

This branch was cut from an older dev and is behind, so it needs a rebase onto current dev before merge. The conflicts are nearby churn — dev contains none of these fixes (verified: no qt_check_fmt / st_read_f32_cap upstream), so nothing here is superseded. Before investing in the rebase: do you want this as one PR, or split into (a) model-file C trust-boundary fixes, (b) server fixes, (c) build/docs? I'll do either. Threat model throughout is the untrusted-mirror model directory.

[KingIcyCreamProjects]

Rebased onto current dev — now 0 behind, and re-verified end-to-end.

• One rebase conflict (st.h st_read_slice_f32): resolved to keep the audit's NULL-check on the malloc'd raw while adopting dev's st_pread_full (st: chunked pread with EINTR retry and honest short-read errors #331 chunked + EINTR read) — a strict superset of the audit's pread(...)!=nb, so no protection is lost and short reads / >2 GB tensors are now handled honestly.
• Ran an adversarial re-verify of the rebased tree: every fix is present, correct, and still effective against current dev (SEC-1 qt_check_fmt at all 3 format sites, SEC-2/B1 numel-vs-nbytes + overflow + st_read_f32_cap, SEC-3/4/5, SEC-6/7/8, SEC-9 cfg_slurp, B2/B5/B6/B9). glm.c compiles clean; openai_server.py py_compile clean.

One thing the re-verify caught and I fixed here: dev's newer GET /profile route shipped unauthenticated, unlike its SEC-8 siblings /health and /experts — leaking per-turn timing/token telemetry. Added a commit gating it behind _is_authed() (mirrors /experts), so SEC-8's "telemetry behind auth" holds for the new route too.

Out-of-scope follow-up (pre-existing on dev, not a rebase artifact): the Linux io_uring load path (uring_finalize_load) infers fmt with the same ternary but never calls qt_check_fmt and has no fmt=4 handling — a 4th quant-format site the original audit didn't cover. I'd do that as its own PR since it needs Linux+io_uring testing plus real fmt=4 detection, not just a guard.

[KingIcyCreamProjects]

Correction to my previous comment — I reverted the /profile auth gate, and here's why (leaving the finding on the table for you to decide).

My re-verify flagged that dev's newer GET /profile returns per-turn timing/token telemetry with no auth gate, unlike its SEC-8 siblings /health and /experts. I gated it to match — but that broke test_profile_reports_recent_turns_without_auth, and reading it, the test constructs the server with an api key ("secret") and still expects /profile to answer without the key. So /profile being public even when a key is set is a deliberate design choice (presumably so the dashboard/monitors can poll live metrics without embedding the key) — not an oversight. That's your call, not something I should force in a rebase, so I backed it out. The branch is green again (71/71 python tests pass).

Flagging it so it's a conscious decision: when the server is exposed with COLI_API_KEY set, /profile still leaks per-turn wall_s / prompt+completion token counts / expert disk+wait+matmul / attention / lm_head / forwards to any host-allowlisted caller. _check_host covers DNS-rebinding, so it's low severity, but it's inconsistent with SEC-8's "telemetry behind auth". If you want it gated for parity, it's a 4-line change plus flipping that test to send the key — happy to do it, or leave /profile public by design. Your call.

Net: the PR is exactly the original audit (SEC-1…B9 + SEC-9), rebased clean onto dev, no behavior changes beyond the documented hardening.

[KingIcyCreamProjects]

Extended the audit after a research + code pass (corroborated against the llama.cpp GGUF integer-overflow CVEs — 33298/53630/27940/25664-68 — and the safetensors "two-authorities" TOCTOU class). The original audit hardened glm.c thoroughly but left sibling surfaces. Four fixes, all Windows-built + tested:

• CRITICAL — heap OOB write in the OLMoE loader (olmoe.c load_expert_w). The dtype-3 (U8/I8) container path wrote header-controlled byte counts into config-sized buffers (q=malloc(O*I), scale=falloc(O)). st_init deliberately skips its numel↔nbytes cross-check for dtype-3, so a crafted .safetensors with an over-declared U8 expert weight (or oversized .qs) was a controlled heap overflow — the exact SEC-1 primitive glm.c's qt_check_fmt already rejects, just never applied to olmoe. Added the matching size guard (weight == O*I bytes, scale == O elems) in the shared loader.
• olmoe.c load_cfg — unbounded config.json slurp (SEC-9 class) + json_get(...)->num NULL-deref on any missing key. Added the 256 MB cap + NULL/short-read guards, a req_num() that rejects missing/non-numeric keys, and CKR-style range checks.
• tok.h (hostile tokenizer.json) — type-check vocab ids / merge-pair entries before deref (was NULL-deref / silent id=0), and cap maxid so n_ids=maxid+1 can't signed-overflow into a multi-GB calloc. Verified the real 151K-vocab GLM tokenizer still loads + encodes unchanged.
• openai_server.py — APIHandler.timeout = 30: a slowloris client could otherwise pin a worker thread + KV slot forever, before the B2 queue bound applies.

Validation: glm.c/olmoe.c compile clean, 71/71 python tests pass, real GLM tokenizer loads, tiny oracle 32/32.

Two things deferred (not in this PR):

1. The Linux io_uring finalize path (uring_finalize_load) is a 5th fmt-inference site with the same missing qt_check_fmt + no fmt=4 handling — but it's #ifdef __linux__, uncompilable/untestable on my Windows box, so it needs its own Linux-gated PR (the fix is the proven pread-arm pattern verbatim). Flagging rather than shipping a memory-safety change I can't build.
2. /profile stays public by design (a test asserts it even with a key set) — your call whether to gate it.

[JustVugg]

Heads-up on the rebase you flagged: I did an independent audit of the model-file trust boundary and confirmed your SEC-1/SEC-2 (the safetensors OOB read/write) and SEC-9 (config slurp) against current dev — they're real and present in v1.0.0. Given the severity I've landed a minimal standalone fix for just the C memory-corruption items in #413 (numel↔nbytes cross-check + shape-overflow guard in st.h; a validating qt_resolve_fmt replacing the silent int2 fallback at all three sites; bounded config slurp). Verified no regression (TF token-exact on every quant format, fmt=4 preserved, ASan/UBSan clean, hostile file refused).

This is meant to unblock your rebase, not replace your PR: #413 is deliberately scoped to the C corruption bugs so v1.0.1 can ship them fast. The server-side hardening (SEC-6 fail-closed bind, SEC-7 Host allowlist, SEC-8 telemetry auth) and the build/docs items are all still yours and don't overlap #413. So to answer your "one PR or split?": please split — rebase #368 down to the server + build items and it lands clean on top of #413. Credited you in the #413 description; thank you for the careful writeup, it made the independent verification fast.

[JustVugg]

Heads-up: c/glm.c was just split into c/colibri.c + header modules (#391, now merged into dev). This PR touches the old glm.c, so it will need a rebase onto current dev with its changes moved into colibri.c (or the relevant extracted header: quant.h, sample.h, kv_persist.h, telemetry.h, grammar.h). The make glm target still works (alias of make colibri), so no build scripts break. Apologies for the churn — ping me if the rebase gets thorny and I can help place the hunks.

[JustVugg]

Rebase needed: #298 (CUDA per-group scale fix) and #192 (per-request grammars, touches openai_server.py) just merged into dev and this now conflicts. This PR is a priority for v1.0.1 (it's one of the two remaining items), so if you're short on time say the word and I'll handle the rebase — otherwise all yours. The security-hardening semantics are yours to keep; nothing in the new dev changes your threat model.

[JustVugg]

This is a v1.0.1 priority and I want it in. Two blockers:

1. It conflicts now (dev moved a lot post-#391). The conflicts are in security-sensitive parsing/server code, so I won't resolve them blind — a wrong merge could silently weaken a check. Could you rebase on current dev (CI is green)? If a specific conflict is purely contextual (code moved by the refactor, logic unchanged), point me at it and I'll take that hunk.

2. Tests per hardened path. For each check, a one-line note on the threat it closes + ideally a test that fails-before / passes-after (a malformed-header fixture, an oversized data_offset, a path-traversal attempt in a tensor name). With the rebase green and those tests, this merges — the threat model is real and I want it landed correctly, not fast. Thanks @KingIcyCreamProjects.

[JustVugg]

I went through the conflicts hunk by hunk to see exactly what still needs your hands. Good news: your rebase just got much smaller, and one of your findings is already fixed on dev with credit to you.

Already covered on dev — you can drop these hunks:

• c/glm.c → qt_check_fmt: dev has qt_resolve_fmt (security: reject malformed model tensors at the untrusted-mirror boundary (C half of #368) #413), which is a superset — it resolves the fmt from the byte counts and refuses unknown layouts instead of falling through to int2, and also covers fmt=5. Same threat, stricter handling.
• c/st.h → the shape-product overflow guard: dev has an equivalent guard from the same fix.
• c/olmoe.c → this one was a real find, and it was still open on dev. Your patch hardened load_expert_w, but the olmoe refactor replaced that function with slot_ensure_allocated + load_expert_merged, so the dtype-3 hole survived in the new code. I've just fixed it on dev (9b1d87d): exact nbytes == ng+ng+nd and .qs numel == inter+inter+hidden before the raw read, reject-never-repair. Credited to your review in the commit message — thank you, that hole was live.

Still unique to this PR, still wanted:

• c/openai_server.py (+94/-27) — the server-side hardening. This is the biggest remaining piece and nothing on dev covers it.
• c/json.h, c/tok.h — parser/tokenizer hardening.
• c/Makefile (+18) — build hardening flags.
• download_fp8.py / tools/download_glm52.py / requirements.txt — downloader + dependency pinning.

So the ask is now: rebase on current dev keeping only the server/json/tok/build/downloader parts, and drop the container-parsing hunks that dev already has. That should cut the conflicts to almost nothing (the web/* ones were only i18n churn). With that and a short note per check on the threat it closes — plus a fails-before/passes-after test where practical — this lands quickly. It's the last thing gating the next release, and I want it in.

[JustVugg]

Merged. I did the rebase for you rather than making you carry it — the conflicts were a mix of "dev already has an equal-or-stronger guard" and i18n churn, so no security check was lost:

• colibri.c / st.h → dev's side. qt_resolve_fmt (security: reject malformed model tensors at the untrusted-mirror boundary (C half of #368) #413) is a superset of qt_check_fmt: it resolves the layout from the byte counts and refuses unknown ones rather than falling through to int2, and it covers the newer grouped formats. The shape-product overflow guard is equivalent.
• olmoe.c → dev's side, but your finding was the valuable one: your patch hardened load_expert_w, which the olmoe refactor had replaced, so the dtype-3 hole was still live on dev in the new load_expert_merged. That's now fixed (9b1d87d) with your review credited.
• web/* → i18n only.

What landed is your unique work: openai_server.py (+121), json.h, tok.h, the Makefile flags, and the downloader/requirements pinning.

One bug I found while validating, worth knowing about: st_read_f32_cap is a good primitive, but its call site bounded the scale read with the per-row cardinality O. That's right for int8/int4/int2 and wrong for grouped formats — int3-g64 (fmt=5) legitimately carries O*i3_groups(I) scales, so a well-formed container was rejected and tests/test_int3_load failed. The cap now matches the cardinality qt_resolve_fmt already validates and falloc actually reserved. Worth remembering for future hardening: a bound derived from an assumed layout becomes a correctness bug the moment a new layout ships.

Verified before merge: clean build, token-exact unchanged (fp 32/32, int4 21/32), test_int3_load ok, full make check OK. Thank you — this was the last thing gating the release.