Fix export-time trust_remote_code bypass in FP8/INT8/GGUF-LoRA export#6869
Conversation
The torchao, compressed-tensors, and LoRA GGUF export paths re-read the merged checkpoint and used to set trust_remote_code from the checkpoint config's static auto_map (the torchao path also scanned the staged tokenizer/processor configs). A model that loads with built-in Transformers classes can carry an auto_map entry, which skips the load-time remote-code consent scan (that only runs when the load already requested trust_remote_code) yet flips trust_remote_code on at export, running unvetted custom code. Derive the reload trust_remote_code from the approved load decision instead: a new _loaded_via_remote_code() checks whether the in-memory model / tokenizer was itself loaded from custom code (its class lives in the transformers_modules package), walking PEFT / wrapper layers. Built-in-loaded models no longer gain trust from config metadata; genuine custom-code models (loaded with consent) still reload correctly. Add CPU-only regression tests.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
for more information, see https://pre-commit.ci
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses a security bypass where export paths derived trust_remote_code from a checkpoint's static auto_map config rather than the actual load decision. It introduces a helper _loaded_via_remote_code to verify if the model or tokenizer was loaded via custom code by inspecting its module path, and updates export paths to use this helper. Feedback suggests making the module name check in _loaded_via_remote_code more robust by using getattr and checking if the module is a string to prevent potential AttributeErrors when module is None or missing, along with adding a corresponding test case.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if type(node).__module__.startswith("transformers_modules"): | ||
| return True |
There was a problem hiding this comment.
Directly accessing type(node).__module__ and calling .startswith() can raise an AttributeError if __module__ is None or missing (which can happen for certain dynamically created classes, built-in types, or mock objects). To make this check more robust and prevent potential crashes during export, defensively retrieve the module name using getattr and verify it is a string before calling .startswith().
| if type(node).__module__.startswith("transformers_modules"): | |
| return True | |
| module = getattr(type(node), "__module__", None) | |
| if isinstance(module, str) and module.startswith("transformers_modules"): | |
| return True |
|
|
||
| def test_none_is_not_remote_code(): |
There was a problem hiding this comment.
Add a test case to verify that _loaded_via_remote_code handles objects with a None module gracefully without raising an AttributeError.
| def test_none_is_not_remote_code(): | |
| def test_none_is_not_remote_code(): | |
| assert _loaded_via_remote_code(None) is False | |
| def test_none_module_is_not_remote_code(): | |
| assert _loaded_via_remote_code(_obj(None)) is False |
Read type(node).__module__ via getattr and require a string before startswith, so a dynamically created or C-extension class with a None module does not raise during export. Add a regression test.
|
Applied both in 04c2d17. |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04c2d17f61
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # trust_remote_code must reflect the approved load decision (whether the model / tokenizer | ||
| # was actually loaded from custom code), not the config's static auto_map, so a | ||
| # built-in-loadable model carrying auto_map cannot run unvetted code in the subprocess. | ||
| trust_remote_code = _loaded_via_remote_code(model) or _loaded_via_remote_code(tokenizer) |
There was a problem hiding this comment.
When the compressed export sees an approved custom tokenizer/processor but a built-in-loadable model that still carries a model auto_map, this or sets the single subprocess flag and unsloth/_compressed_quantize.py passes that same args.trust_remote_code to both auto_model.from_pretrained and auto_proc.from_pretrained. In that scenario the tokenizer's consent enables the staged model's unapproved custom code during compressed quantization, preserving the export-time consent bypass that the torchao path avoids by tracking model_trust and tok_trust separately.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 99ce1b8: the subprocess now takes a separate --trust-remote-code-tokenizer flag for the processor load while --trust-remote-code stays model-only, and save.py passes model_trust / tok_trust individually like the torchao path. Call-site test added.
| except Exception: | ||
| nxt = None | ||
| if nxt is None or nxt is node: | ||
| nxt = getattr(node, "base_model", None) or getattr(node, "model", None) |
There was a problem hiding this comment.
When the tokenizer argument is a built-in ProcessorMixin or wrapper that contains an approved remote-code component under .tokenizer, .image_processor, or .feature_extractor, this walker never descends into those processor attributes and returns False. The previous staged-config scan covered tokenizer/processor/preprocessor auto_map entries, but now torchao and compressed exports can pass trust_remote_code=False to the processor reload for an already-approved custom tokenizer/processor, causing those exports to fail or reload the wrong component.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 99ce1b8: the walker is now a bounded BFS that also descends into tokenizer, image_processor, feature_extractor, and video_processor attributes, so an approved custom component inside a built-in processor keeps its trust on reload. Tests added for custom and built-in components plus cycle termination.
…rocessor components The compressed-tensors export collapsed model and tokenizer trust into one --trust-remote-code flag, so an approved custom tokenizer would have let an unapproved model's custom code run inside the quantization subprocess. The subprocess now takes --trust-remote-code-tokenizer for the processor load and keeps --trust-remote-code for the model loads, matching the torchao path's separate model_trust / tok_trust. _loaded_via_remote_code now also walks processor components (tokenizer, image_processor, feature_extractor, video_processor), so an approved custom tokenizer held inside a built-in ProcessorMixin keeps its trust on the export reload instead of failing with trust_remote_code=False. The walk is a bounded BFS with a seen set so wrapper cycles terminate.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
for more information, see https://pre-commit.ci
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
The FP8/INT8 (portable torchao), compressed-tensors, and LoRA GGUF export paths re-read the just-merged checkpoint to apply quantization. They derived
trust_remote_codefor that reload from the checkpoint config's staticauto_map(the torchao path additionally scanned the stagedtokenizer_config.json/processor_config.json/preprocessor_config.json).That is the wrong source. A model can load with built-in Transformers classes while still carrying an
auto_mapentry. Loading it does not requiretrust_remote_code, so it never triggers the load-time remote-code consent scan (which only runs when the load itself requestedtrust_remote_code). At export the same model then hadtrust_remote_codeflipped on from thatauto_mapmetadata, so its custom code ran during the reload without ever being scanned or approved. In Studio this bypasses the CRITICAL/HIGH remote-code consent gate.Fix
Derive the reload
trust_remote_codefrom the previously approved load decision rather than from untrusted config metadata. A new helper_loaded_via_remote_code(obj)reports whether the in-memory model / tokenizer was itself loaded from custom code, by checking whether its class lives in Transformers'transformers_modulespackage (whereauto_mapcode is imported). It walks PEFT / wrapper layers so a LoRA over a custom-code base is still detected.trust_remote_codefrom configauto_map, so no unvetted remote code runs at export.trust_remote_code, and in Studio was consent-scanned at load) still reports as remote-code-loaded, so it reloads and exports correctly. No regression for Nemotron and similar.Applied to all three reload sites:
_unsloth_save_torchao,_unsloth_save_compressed_tensors, and the LoRA GGUF converter invocation. The staged-configauto_mapscan in the torchao path is removed.Testing
tests/saving/test_torchao_remote_code_consent.py(new, CPU-only): AST-extracts and exercises_loaded_via_remote_code(built-in vstransformers_modules, PEFT unwrap, wrapper walk, and the key case that anauto_mapin config alone does not grant trust), plus asserts every export path dropped the config-auto_maptrust derivation.tests/saving/test_export_api_surface.pyandtests/saving/test_export_dispatch.pypass.python -m py_compile unsloth/save.pyis clean.