mirror of
https://github.com/vllm-project/vllm.git
synced 2026-10-28 09:45:26 +08:00
[Security][CI/Build] Add pre-commit check for hardcoded trust_remote_code=True (#62547)
Signed-off-by: Mritunjay Sharma <mritunjaysharma394@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
3ff10ddeb0
commit
04631c4915
4 files changed
+135
No files matched your search
@@ -654,6 +654,8 @@ steps:
|
||||
- tests/tools/test_check_test_tethering.py
|
||||
- tools/pre_commit/check_agent_files.py
|
||||
- tests/tools/test_check_agent_files.py
|
||||
- tools/pre_commit/check_trust_remote_code.py
|
||||
- tests/tools/test_check_trust_remote_code.py
|
||||
- tests/tool_parsers
|
||||
- tests/tokenizers_
|
||||
- tests/parser
|
||||
@@ -705,6 +707,7 @@ steps:
|
||||
- pytest -v -s config
|
||||
- pytest -v -s tools/test_check_test_tethering.py
|
||||
- pytest -v -s tools/test_check_agent_files.py
|
||||
- pytest -v -s tools/test_check_trust_remote_code.py
|
||||
|
||||
- label: ":computer: (CPU) Reasoning + Renderers"
|
||||
key: cpu-reasoning-renderers
|
||||
|
||||
@@ -288,6 +288,12 @@ repos:
|
||||
entry: python tools/pre_commit/check_boolean_context_manager.py
|
||||
language: python
|
||||
types: [python]
|
||||
- id: check-trust-remote-code
|
||||
name: Check for hardcoded trust_remote_code=True
|
||||
entry: python tools/pre_commit/check_trust_remote_code.py
|
||||
language: python
|
||||
types: [python]
|
||||
files: ^vllm/
|
||||
# Rust hooks. These shell out to `cargo`; tools/pre_commit/rust-check.sh
|
||||
# skips with a warning when cargo is not installed.
|
||||
- id: rust-cargo-autoinherit
|
||||
|
||||
@@ -0,0 +1,53 @@
|
||||
# SPDX-License-Identifier: Apache-2.0
|
||||
# SPDX-FileCopyrightText: Copyright contributors to the vLLM project
|
||||
"""Unit tests for the hardcoded `trust_remote_code=True` pre-commit check.
|
||||
|
||||
The hook passes silently on a healthy tree, so CI would not notice a rule that
|
||||
stopped firing. Each case below asserts the check reports (or ignores) one
|
||||
pattern.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
|
||||
from tools.pre_commit.check_trust_remote_code import check_source
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"source",
|
||||
[
|
||||
"AutoModel.from_config(cfg, trust_remote_code=True)\n",
|
||||
"get_processor(name, revision=r, trust_remote_code=True)\n",
|
||||
"x = f(g(trust_remote_code=True))\n",
|
||||
"f(\n name,\n trust_remote_code=True,\n)\n",
|
||||
],
|
||||
)
|
||||
def test_hardcoded_true_is_reported(source):
|
||||
assert len(check_source(source)) == 1
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"source",
|
||||
[
|
||||
"AutoModel.from_config(cfg, trust_remote_code=cfg.trust_remote_code)\n",
|
||||
"AutoModel.from_config(cfg, trust_remote_code=False)\n",
|
||||
"f(trust_remote_code=trust_remote_code)\n",
|
||||
# Default values in signatures are not calls.
|
||||
"def f(trust_remote_code=True):\n pass\n",
|
||||
# Docstrings, strings and comments are not code.
|
||||
'"""Example: f(trust_remote_code=True)"""\n',
|
||||
"msg = 'set trust_remote_code=True to load this model'\n",
|
||||
"# f(trust_remote_code=True)\n",
|
||||
# Explicit suppression with a reason.
|
||||
"f(trust_remote_code=True) # trust-remote-code: ok (vetted repo)\n",
|
||||
# Unparsable files are skipped, not crashed on.
|
||||
"def broken(:\n",
|
||||
],
|
||||
)
|
||||
def test_safe_code_is_not_reported(source):
|
||||
assert check_source(source) == []
|
||||
|
||||
|
||||
def test_violation_points_at_the_keyword_line():
|
||||
source = "f(\n name,\n trust_remote_code=True,\n)\n"
|
||||
[violation] = check_source(source, "a.py")
|
||||
assert violation.startswith("a.py:3:")
|
||||
@@ -0,0 +1,73 @@
|
||||
# SPDX-License-Identifier: Apache-2.0
|
||||
# SPDX-FileCopyrightText: Copyright contributors to the vLLM project
|
||||
"""Lint: detect hardcoded `trust_remote_code=True` inside vLLM.
|
||||
|
||||
Passing a literal `True` silently overrides the user's `--trust-remote-code`
|
||||
setting, so a malicious model repository can execute code even when the user
|
||||
opted out (CVE-2026-27893):
|
||||
|
||||
AutoModel.from_config(cfg, trust_remote_code=True) # BUG
|
||||
|
||||
Pass the user's setting through instead:
|
||||
|
||||
AutoModel.from_config(cfg, trust_remote_code=model_config.trust_remote_code)
|
||||
|
||||
Being AST-based, this check ignores docstrings and comments. If a call is
|
||||
genuinely safe, suppress it with a reason on the same line:
|
||||
|
||||
load(path, trust_remote_code=True) # trust-remote-code: ok (reason)
|
||||
"""
|
||||
|
||||
import ast
|
||||
import sys
|
||||
|
||||
SUPPRESS_MARKER = "# trust-remote-code: ok"
|
||||
|
||||
|
||||
def check_source(source: str, filepath: str = "<string>") -> list[str]:
|
||||
try:
|
||||
tree = ast.parse(source, filename=filepath)
|
||||
except SyntaxError:
|
||||
return []
|
||||
|
||||
lines = source.splitlines()
|
||||
violations = []
|
||||
for node in ast.walk(tree):
|
||||
if not isinstance(node, ast.Call):
|
||||
continue
|
||||
for kw in node.keywords:
|
||||
if (
|
||||
kw.arg == "trust_remote_code"
|
||||
and isinstance(kw.value, ast.Constant)
|
||||
and kw.value.value is True
|
||||
and SUPPRESS_MARKER not in lines[kw.value.lineno - 1]
|
||||
):
|
||||
violations.append(
|
||||
f"{filepath}:{kw.value.lineno}: hardcoded "
|
||||
"`trust_remote_code=True` overrides the user's setting"
|
||||
)
|
||||
return violations
|
||||
|
||||
|
||||
def check_file(filepath: str) -> list[str]:
|
||||
try:
|
||||
with open(filepath, encoding="utf-8") as f:
|
||||
source = f.read()
|
||||
except (OSError, UnicodeDecodeError):
|
||||
return []
|
||||
return check_source(source, filepath)
|
||||
|
||||
|
||||
def main() -> int:
|
||||
if len(sys.argv) < 2:
|
||||
print("Usage: check_trust_remote_code.py <file> ...", file=sys.stderr)
|
||||
return 1
|
||||
|
||||
violations = [v for filepath in sys.argv[1:] for v in check_file(filepath)]
|
||||
for v in violations:
|
||||
print(v)
|
||||
return 1 if violations else 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main())
|
||||
Reference in new issue
Block a user