fix(security): the embedding model loads only safetensors; advisory paths stay unreachable by cdeust · Pull Request #938 · cdeust/Cortex

cdeust · 2026-11-07T08:15:21Z

Problem

main has 20 open Dependabot advisories against torch, transformers, cryptography and setuptools. On Linux, Windows and Apple silicon macOS 14+, uv.lock already resolves patched versions. The vulnerable versions are locked only for the platforms the project keeps (owner decision 2026-11-07, #740: x86_64 under Rosetta), and PyPI offers no patched wheel there. Probed with uv pip compile --only-binary :all: on 2026-11-07:

Platform	Patched version	Installable
macOS Intel	torch ≥ 2.6	no wheel (stops at 2.2.2)
macOS Intel	cryptography ≥ 49	no wheel (stops at 48.0.1)
Apple silicon, macOS < 14	torch ≥ 2.12	no wheel (stops at 2.11)

Version bumps cannot close these advisories there, so this PR closes their paths instead.

Change

use_safetensors=True. The embedding model is built with model_kwargs={"use_safetensors": True}, both at load (embedding_model_lifecycle._construct_model) and in the background download child. transformers then loads only *.safetensors and raises when the revision has none. A pickled pytorch_model.bin, the torch.load vector of the critical GHSA-53q9-r3pm-6pq6 and of GHSA-63cw-57p8-fm3p, is never read.

Contract test. tests_py/infrastructure/test_vulnerable_dependency_paths.py:

names, per advisory, the function that reaches it (from each GHSA's description) and fails when mcp_server/ calls it: torch.load, torch.jit, torch.ops.profiler, lstm_cell, pad_packed_sequence, unpack_sequence, quantized modules, mkldnn_max_pool2d, ctc_loss, save_pretrained, load_custom_generate, trust_remote_code=True, Trainer(, LightGlue, x509 verification / PolicyBuilder, pkcs7;
pins the model to one 40-hex Hub commit, so its config.json cannot be attacker-chosen (GHSA-29pf-2h5f-8g72, via _attn_implementation_internal);
checks that load and download pass use_safetensors=True.

cryptography is not imported by Cortex. It comes in through pyjwt[crypto] (MCP SDK auth), which uses neither the x509 verifier nor PKCS#7.

Evidence

On the real pinned model (local macOS, torch 2.13, transformers 5.21, sentence-transformers 6.0.1):

SentenceTransformer(..., revision=<pin>, model_kwargs={"use_safetensors": True}) loads and encodes ((1, 384)).
A copy of the snapshot with model.safetensors removed and a planted pytorch_model.bin (a pickle whose __reduce__ runs os.system) is refused: OSError: Error no file named model.safetensors. torch.load is called 0 times, and the payload's marker file is never created.

Checks

19 contract cases pass; the 2 loading tests fail without the change.
118 embedding and infrastructure tests pass.
ruff check and ruff format --check pass on the touched files.
scripts/check_craftsmanship.py: OK.

Not in this PR

The x86_64 run itself is not reproduced: Rosetta is not installed on this host. The 20 alerts stay open; dismissing them as vulnerable_code_not_used with this PR as the reason is the owner's call.

…aths stay unreachable macOS Intel has no wheel past torch 2.2.2 or cryptography 48.0.1, and Apple silicon before macOS 14 none past torch 2.11, so their 20 open Dependabot advisories cannot be closed by version on those platforms (uv --only-binary probe, 2026-11-07). The project keeps those platforms (owner, #740). The critical ones reach torch.load through a pickled checkpoint. The model is now built with model_kwargs use_safetensors=True, at load and in the background download: on the real pinned model it loads as before; a snapshot holding only a planted pytorch_model.bin is refused with OSError, torch.load is called 0 times and the payload never runs. A contract test names, per advisory, the function that reaches it (torch.jit, lstm_cell, save_pretrained, load_custom_generate, x509 verification, pkcs7, ...) and fails when mcp_server calls one; it also pins the model to one Hub commit, which keeps its config.json out of an attacker's hands (GHSA-29pf-2h5f-8g72).

chatgpt-codex-connector · 2026-11-07T08:15:34Z

Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-07T08:44:25.602125Z	`8642571`	New commits
🔒 Security Review	✅ Completed 2026-11-07T08:46:06.844046Z	`8642571`	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

claude · 2026-11-07T08:15:56Z

Claude finished @cdeust's task in 2m 51s —— View job

PR #938 review (1 of 3)

Read the diff and the code around it
Ran five reviews: security, code quality, test coverage, documentation accuracy, performance
Posted the findings: fix(security): the embedding model loads only safetensors; advisory paths stay unreachable #938 (comment)

No Blockers. Both places that load the model, _construct_model and the background download, pass use_safetensors=True with the pinned revision. Nothing else in mcp_server/ calls torch.load or from_pretrained. I read the code but did not run the tests.

Three Notes, none needed before merge:

The install scripts still preload the model with no pinned revision and no safetensors setting, and the contract test does not check them.
A model revision without safetensors would be treated as "not cached" and start a download that cannot succeed. The current pinned revision can't hit this.
The background download hardcodes the safetensors setting instead of reusing SAFETENSORS_ONLY, so the two copies can drift.

Verdict: approve

claude · 2026-11-07T08:18:46Z

Review 1 of 3: PR #938

I ran five reviews (security, code quality, test coverage, documentation accuracy, performance) and found no Blockers.

There are only two places in mcp_server/ that load weights: embedding_model_lifecycle.py:154 and the download child at embedding_downloads.py:54-55. Both pass use_safetensors=True and the pinned revision.
The only production constructor, embedding_factory.py:61, cannot override the model name or the revision.
There is no other torch.load, from_pretrained or CrossEncoder call, and the reranker uses flashrank, which runs on ONNX.
The locked versions (transformers 4.57.6 and 5.21.0, sentence-transformers 5.7.0 and 6.0.1) all accept model_kwargs and use_safetensors.
The tests are sound in CI: sentence-transformers is a core dependency, and spawned[0][3] is the -c program.

Notes, no action before merge

The install-time precache calls SentenceTransformer('all-MiniLM-L6-v2') without a pinned revision or use_safetensors, and the contract test does not check it because it only scans mcp_server/ (scripts/setup.py:288, scripts/lib/precache_embedding_model.sh:46). The unpinned-config concern (GHSA-29pf-2h5f-8g72) still applies there.
_cache_miss_types() treats every OSError as "not cached" (embedding_model_lifecycle.py:125-137, 183). A revision with no safetensors would therefore show as MODEL_FILES_ABSENT and start a background download that cannot succeed, every session. The current pin can't reach this.
The download child hardcodes {'use_safetensors': True}, and the test checks that literal instead of SAFETENSORS_ONLY, so the two copies can drift (embedding_downloads.py:53-55).

Verdict: approve

…isory paths stay unreachable Mutation gate (ADR-1122), run on #938: 4 survivors in trigger_background_model_download, on the lines of the child program. The test only matched a substring of that program; it now executes the program against a fake sentence_transformers and checks the exact call: name, revision, cache_folder and model_kwargs use_safetensors.

claude · 2026-11-07T08:43:50Z

Claude finished @cdeust's task in 1m 15s —— View job

PR #938 review (2 of 3)

Checked the earlier Blockers: review 1 had none
Reviewed the one commit pushed since 872dce4: 8642571, which changes one test
Ran five reviews: security, code quality, test coverage, documentation accuracy, performance
Posted the findings: fix(security): the embedding model loads only safetensors; advisory paths stay unreachable #938 (comment)

No Blockers. The reworked test now runs the download child's own program against a fake sentence_transformers and checks the exact call: model name, revision, cache folder and use_safetensors=True. That catches the 4 survivors the mutation gate reported. I read the code but did not run the tests or lint, because those commands need approval in this job.

Two Notes, none needed before merge:

The test never passes an empty revision or cache folder, so the code that turns them into None is still not exercised.
At line 93, spawned[0][4:] would read more clearly than argv[1:].

Verdict: approve

claude · 2026-11-07T08:45:12Z

Review 2 of 3: PR #938

Review 1 found no Blockers. I reviewed only the commit pushed after 872dce4: 8642571, which changes one test. Each of the five reviews (security, code quality, test coverage, documentation accuracy, performance) was read-only. I did not run the tests or lint because those commands need approval in this job. No Blockers.

The argv slicing is correct. The download command is [exe, "-s", "-c", prog, name, rev, cf] (embedding_downloads.py:62-70), so the test gives the program the same sys.argv that a real python -c child would get.
The exact-match assertion fails if model_kwargs or use_safetensors is dropped or changed, if revision or cache_folder is lost, or if the arguments are swapped. This closes the 4 survivors named in the commit message.
Nothing leaks into other tests. monkeypatch restores sys.modules and sys.argv, exec runs in an empty namespace, and the fake module means torch is never imported.

Notes, no action before merge

The empty-to-None paths (sys.argv[2] or None at embedding_downloads.py:52 and revision or \"\" at :68-69) still survive mutation. The test only passes \"r\" and \"/cache\", never None.
At test_vulnerable_dependency_paths.py:93, spawned[0][4:] would be clearer than argv[1:] on a list whose first item is the program.

Verdict: approve

github-actions Bot merged commit 1c325dc into main Nov 7, 2026
18 checks passed

Navigation Menu

Repository navigation

fix(security): the embedding model loads only safetensors; advisory paths stay unreachable - #938

fix(security): the embedding model loads only safetensors; advisory paths stay unreachable#938
github-actions[bot] merged 2 commits into
mainfrom
fix/vulnerable-deps-unreachable

cdeust commented Nov 7, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 7, 2026 •

edited

Loading

Uh oh!

claude Bot commented Nov 7, 2026 •

edited

Loading

Uh oh!

claude Bot commented Nov 7, 2026

Uh oh!

claude Bot commented Nov 7, 2026 •

edited

Loading

Uh oh!

claude Bot commented Nov 7, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

cdeust commented Nov 7, 2026

Problem

Change

Evidence

Checks

Not in this PR

Uh oh!

chatgpt-codex-connector Bot commented Nov 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot commented Nov 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR #938 review (1 of 3)

Uh oh!

claude Bot commented Nov 7, 2026

Review 1 of 3: PR #938

Uh oh!

claude Bot commented Nov 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR #938 review (2 of 3)

Uh oh!

claude Bot commented Nov 7, 2026

Review 2 of 3: PR #938

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

chatgpt-codex-connector Bot commented Nov 7, 2026 •

edited

Loading

claude Bot commented Nov 7, 2026 •

edited

Loading

claude Bot commented Nov 7, 2026 •

edited

Loading