Skip to content

Commit 01f0868

Browse files
Add Security and IP related contributing guide and enforce coderabbit review
Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
1 parent e589ac8 commit 01f0868

File tree

14 files changed

+244
-129
lines changed

14 files changed

+244
-129
lines changed

.coderabbit.yaml

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,35 @@ reviews:
44
profile: chill
55
collapse_walkthrough: true
66
poem: false
7+
path_instructions:
8+
- path: "modelopt/**/*.py"
9+
instructions: &security_instructions |
10+
Review all modelopt package and examples Python changes against the security coding practices in
11+
SECURITY.md. Flag any of the following as CRITICAL security issues,
12+
request changes, and fail the check if ANY are present:
13+
1. torch.load(..., weights_only=False) with no inline comment justifying why it is safe
14+
(e.g. confirming the file is internally-generated and not user-supplied).
15+
2. numpy.load(..., allow_pickle=True) with no inline comment justifying why it is safe.
16+
Should expose allow_pickle as a caller-configurable parameter defaulting to False, not hardcode True.
17+
3. trust_remote_code=True hardcoded for transformers model or tokenizer loading.
18+
Code should expose it as a caller-configurable parameter defaulting to False, not hardcode True.
19+
4. eval() or exec() on any input that could originate from outside the process.
20+
5. Any use of "# nosec" comments to bypass Bandit security checks is not allowed.
21+
If a security-sensitive pattern is genuinely necessary, the PR must be reviewed and approved
22+
by @NVIDIA/modelopt-setup-codeowners with an explicit justification in the PR description.
23+
- path: "examples/**/*.py"
24+
instructions: *security_instructions
725
auto_review:
826
auto_incremental_review: false
927
drafts: false
1028
base_branches: ["main", "release/.*", "feature/.*"]
29+
pre_merge_checks:
30+
custom_checks:
31+
- name: "Security anti-patterns"
32+
mode: "error"
33+
instructions: *security_instructions
34+
knowledge_base:
35+
code_guidelines:
36+
filePatterns:
37+
- "CONTRIBUTING.md"
38+
- "SECURITY.md"

.github/PULL_REQUEST_TEMPLATE.md

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,28 @@
1-
## What does this PR do?
1+
### What does this PR do?
22

3-
**Type of change:** ? <!-- Use one of the following: Bug fix, new feature, new example, new tests, documentation. -->
3+
Type of change: ? <!-- Use one of the following: Bug fix, new feature, new example, new tests, documentation. -->
44

5-
**Overview:** ?
5+
<!-- More details about the change. -->
66

7-
## Usage
8-
<!-- You can potentially add a usage example below. -->
7+
### Usage
98

109
```python
1110
# Add a code snippet demonstrating how to use this
1211
```
1312

14-
## Testing
13+
### Testing
1514
<!-- Mention how have you tested your change if applicable. -->
1615

17-
## Before your PR is "*Ready for review*"
18-
<!-- If you haven't finished some of the above items you can still open `Draft` PR. -->
16+
### Before your PR is "*Ready for review*"
1917

20-
- **Make sure you read and follow [Contributor guidelines](https://github.com/NVIDIA/Model-Optimizer/blob/main/CONTRIBUTING.md)** and your commits are signed.
21-
- **Is this change backward compatible?**: Yes/No <!--- If No, explain why. -->
22-
- **Did you write any new necessary tests?**: Yes/No
23-
- **Did you add or update any necessary documentation?**: Yes/No
24-
- **Did you update [Changelog](https://github.com/NVIDIA/Model-Optimizer/blob/main/CHANGELOG.rst)?**: Yes/No <!--- Only for new features, API changes, critical bug fixes or bw breaking changes. -->
18+
Make sure you read and follow [Contributor guidelines](https://github.com/NVIDIA/Model-Optimizer/blob/main/CONTRIBUTING.md) and your commits are signed (`git commit -s -S`).
2519

26-
## Additional Information
20+
Make sure you read and follow the [Security Best Practices](https://github.com/NVIDIA/Model-Optimizer/blob/main/SECURITY.md#security-coding-practices-for-contributors) (e.g. hardcoded `trust_remote_code=True`, `torch.load(..., weights_only=True)`, `pickle`, etc.).
21+
22+
- Is this change backward compatible?: ✅❌ <!--- If ❌, explain why. -->
23+
- If you copied code from any other source, did you follow IP policy in [CONTRIBUTING.md](https://github.com/NVIDIA/Model-Optimizer/blob/main/CONTRIBUTING.md#-copying-code-from-other-sources)?: ✅❌ <!--- Mandatory -->
24+
- Did you write any new necessary tests?: ✅❌ <!--- Mandatory for new features or examples. -->
25+
- Did you add or update any necessary documentation and update [Changelog](https://github.com/NVIDIA/Model-Optimizer/blob/main/CHANGELOG.rst)?: ✅❌ <!--- Only for new features, API changes, critical bug fixes or backward incompatible changes. -->
26+
27+
### Additional Information
2728
<!-- E.g. related issue. -->

CONTRIBUTING.md

Lines changed: 36 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -39,36 +39,45 @@ To run the pre-commit hooks without committing, use:
3939
pre-commit run --all-files
4040
```
4141

42-
## 📝 Writing tests
42+
## 🔒 Security coding practices
4343

44-
We use [pytest](https://docs.pytest.org/) for all tests. The tests are organized into the following directories:
44+
All contributors must follow the security coding practices documented in our
45+
[SECURITY.md](./SECURITY.md#security-coding-practices-for-contributors) page (see the *Security Coding Practices for
46+
Contributors* section). These rules cover a few key security considerations like:
4547

46-
- `tests/unit`: Fast cpu-based unit tests for the core ModelOpt library. They should not take more than a few seconds to run.
47-
- `tests/gpu`: Fast GPU-based unit tests for the core ModelOpt library. In most cases, they should not take more than a few seconds to run.
48-
- `tests/examples`: Integration tests for ModelOpt examples. They should not take more than a few minutes to run. Please refer to [example test README](./tests/examples/README.md) for more details.
48+
- Safe use of `torch.load`, `numpy.load`, and `yaml.load`
49+
- Hardcoded `trust_remote_code=True` flag for transformers model loading
50+
- Subprocess and shell command safety
4951

50-
Please refer to [tox.ini](./tox.ini) for more details on how to run the tests and their dependencies.
52+
Any security-sensitive exception requires review and approval from `@NVIDIA/modelopt-setup-codeowners`.
5153

52-
### Code Coverage
54+
## 📋 Copying code from other sources
5355

54-
For any new features / examples, make sure to they are covered by the tests and that the Codecov coverage check in your PR passes.
56+
The utilization of third-party code requires authorization via the Open Source Review Board (OSRB) team and needs to follow proper guidance on contributing code.
5557

56-
## Submitting your code
58+
If you are an external contributor, seek guidance from `@NVIDIA/modelopt-setup-codeowners` for next steps. For internal contributors, follow the steps below:
5759

58-
- If you are an external contributor, create a fork of the repository.
59-
- Rebase (not merge) your code to the most recent commit of the `main` branch. We want to ensure a linear history;
60-
see [Merge vs Rebase](https://www.atlassian.com/git/tutorials/merging-vs-rebasing). Remember to test again locally after rebasing to catch any new issues before pushing to your PR.
60+
- **File NVBug for use of open source code:**
61+
Clone NVBug 2885977 and add your use case. Copying code from permissive licensed repositories (e.g. MIT, Apache 2) is generally self-checkout but for other licenses, it is necessary to get expert guidance before merging your PR.
62+
- **License header format:** The file which has code copied from another third-party GitHub repository should have the following in order:
63+
1. A reference link (with commit hash) to the source from which the code was copied.
64+
1. The original repository's Copyright / License.
65+
1. The NVIDIA Apache 2.0 Copyright / License header.
6166

62-
```bash
63-
git pull
64-
git rebase origin/main
65-
git push origin <branch> --force-with-lease
66-
```
67+
See [`modelopt/torch/speculative/eagle/utils.py`](./modelopt/torch/speculative/eagle/utils.py)
68+
for an example of the correct license header format.
69+
- **Exclude from license pre-commit hook:** Exclude copied files from the license pre-commit hook so it doesn't auto-add the NVIDIA Apache 2.0 license on top of the file. Add the file path to the `exclude` list in the `insert-license` hook in [`.pre-commit-config.yaml`](./.pre-commit-config.yaml).
6770

68-
- When pushing the rebased (or any) branch, use `git push --force-with-lease` instead of `git push --force`.
69-
- Submit a pull request and let auto-assigned reviewers (based on [CODEOWNERS](./.github/CODEOWNERS)) review your PR.
70-
- If any CI/CD checks fail, fix the issues and push again.
71-
- Once your PR is approved and all checks pass, one of the reviewers will merge the PR.
71+
## 📝 Writing tests
72+
73+
We use [pytest](https://docs.pytest.org/) for all tests. For any new features / examples, make sure to add tests and that the coverage check in your PR passes. The tests are organized into the following directories:
74+
75+
- `tests/unit`: Fast cpu-based unit tests for the core ModelOpt library. They should not take more than a few seconds to run.
76+
- `tests/gpu`: Fast GPU-based unit tests for the core ModelOpt library. In most cases, they should not take more than a few seconds to run.
77+
- `tests/gpu_megatron`: Fast GPU-based unit tests for the core ModelOpt library for Megatron-Core features. In most cases, they should not take more than a few seconds to run.
78+
- `tests/examples`: Integration tests for ModelOpt examples. They should not take more than a few minutes to run. Please refer to [example test README](./tests/examples/README.md) for more details.
79+
80+
Please refer to [tox.ini](./tox.ini) for more details on how to run the tests and their dependencies.
7281

7382
## ✍️ Signing your work
7483

@@ -135,3 +144,9 @@ git push origin <branch> --force-with-lease
135144
136145
(d) I understand and agree that this project and the contribution are public and that a record of the contribution (including all personal information I submit with it, including my sign-off) is maintained indefinitely and may be redistributed consistent with this project or the open source license(s) involved.
137146
```
147+
148+
## Submitting your code
149+
150+
- Submit a pull request and let auto-assigned reviewers (based on [CODEOWNERS](./.github/CODEOWNERS)) review your PR.
151+
- If any CI/CD checks fail, fix the issues and push again.
152+
- Once your PR is approved and all checks pass, one of the reviewers will merge the PR.

SECURITY.md

Lines changed: 147 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,150 @@ While NVIDIA currently does not have a bug bounty program, we do offer acknowled
2222

2323
## NVIDIA Product Security
2424

25-
For all security-related concerns, please visit NVIDIA's [Product Security portal](https://www.nvidia.com/en-us/security)
25+
For all security-related concerns, please visit NVIDIA's [Product Security portal](https://www.nvidia.com/en-us/security).
26+
27+
---
28+
29+
## Security Considerations
30+
31+
### Overview
32+
33+
NVIDIA Model Optimizer (ModelOpt) is a library used to optimize ML models and may load and process user-provided artifacts (models, weights, configs, calibration data) and their dependencies. Secure deployment depends on how you source artifacts, validate inputs, and harden the environment where ModelOpt runs.
34+
35+
### What to Be Aware Of
36+
37+
#### Untrusted model and data inputs
38+
39+
- Models, weights, configs and data may be malicious or corrupted.
40+
41+
#### Deserialization and code-execution risks
42+
43+
- Unsafe deserialization can lead to arbitrary code execution if fed untrusted inputs.
44+
- Avoid using serialization formats/settings that can deserialize arbitrary objects.
45+
46+
#### Input validation and resource exhaustion
47+
48+
- Large or malformed inputs can trigger crashes or excessive CPU/GPU/memory use.
49+
- Missing size/type checks can increase DoS risk.
50+
51+
#### Data in transit and at rest
52+
53+
- If fetching models or dependencies over the network, insecure transport can enable tampering.
54+
- Stored artifacts, logs, and caches may contain sensitive data.
55+
56+
#### Logging and observability
57+
58+
- Logs may inadvertently contain sensitive inputs, paths, tokens, or proprietary model details.
59+
- Overly verbose logs can leak operational and security-relevant information.
60+
61+
#### Supply chain and third-party components
62+
63+
- Dependencies may include known vulnerabilities or be compromised.
64+
- Third-party plugins/components loaded at runtime may not have the same security assurances.
65+
66+
### Example Security Approaches
67+
68+
#### Artifact integrity
69+
70+
- Only load artifacts from trusted sources.
71+
- Prefer signed artifacts; verify signatures before loading.
72+
73+
#### Safe parsing and deserialization
74+
75+
- Prefer safer storage formats (avoid object deserialization for untrusted inputs).
76+
- Avoid `pickle`, `torch.load()` with untrusted weights, or YAML `unsafe_load`.
77+
- Treat any unverified artifact as untrusted and block/guard its loading.
78+
79+
#### Hardening and least privilege
80+
81+
- Run with least privilege and isolate workloads.
82+
83+
#### Data protection
84+
85+
- Encrypt sensitive data at rest; use TLS 1.3 for data in transit.
86+
- Never hardcode or log credentials.
87+
88+
#### Resilience
89+
90+
- Validate inputs and enforce limits (file size, timeouts, quotas, etc.).
91+
- Keep OS, containers, and dependencies patched; scan for known vulnerabilities.
92+
93+
---
94+
95+
## Security Coding Practices for Contributors
96+
97+
ModelOpt processes model checkpoints and weights from various sources. Contributors must avoid patterns that can introduce security vulnerabilities. These rules apply to all code except tests. These rules cover a few key security considerations as follows:
98+
99+
### Deserializing untrusted data
100+
101+
**`torch.load` should not be used with `weights_only=False`** unless there is a documented reason why it cannot. It uses pickle under the hood and can execute arbitrary code from a malicious checkpoint.
102+
103+
```python
104+
# Bad — allows arbitrary code execution from the checkpoint file
105+
state = torch.load(path, weights_only=False)
106+
107+
# Good
108+
state = torch.load(path, weights_only=True, map_location="cpu")
109+
110+
# Acceptable only with an inline comment explaining why weights_only=False
111+
# is required and confirming the file is internally-generated / trusted.
112+
state = torch.load(
113+
path,
114+
weights_only=False, # loaded file is generated internally by ModelOpt and not supplied by the user
115+
map_location="cpu",
116+
)
117+
```
118+
119+
**`numpy.load` should not be used with `allow_pickle=True`** unless there is a documented reason why it cannot. It uses pickle under the hood and can execute arbitrary code from a malicious checkpoint.
120+
121+
```python
122+
# Bad — allows arbitrary code execution from the checkpoint file
123+
state = numpy.load(path, allow_pickle=True)
124+
125+
# Good - let the caller decide; default to False
126+
def load_data(path: str, trust_data: bool = False):
127+
return numpy.load(path, allow_pickle=trust_data)
128+
```
129+
130+
**Do not use `yaml.load()`** — always use `yaml.safe_load()`. The default loader can execute arbitrary Python objects embedded in YAML.
131+
132+
### Loading transformers models with `trust_remote_code`
133+
134+
**Do not hardcode `trust_remote_code=True`.** This flag tells Transformers to execute arbitrary Python shipped with a checkpoint, which is an RCE vector if the model source is untrusted.
135+
136+
```python
137+
# Bad — silently opts every user into remote code execution
138+
model = AutoModel.from_pretrained(name, trust_remote_code=True)
139+
140+
# Good — let the caller decide; default to False
141+
def load_model(name: str, trust_remote_code: bool = False):
142+
return AutoModel.from_pretrained(name, trust_remote_code=trust_remote_code)
143+
```
144+
145+
### Subprocess and shell commands
146+
147+
**Never use `shell=True` with string interpolation or user-supplied input.** This is a command-injection vector.
148+
149+
```python
150+
# Bad — command injection if model_name contains shell metacharacters
151+
subprocess.run(f"python convert.py --model {model_name}", shell=True)
152+
153+
# Good — pass arguments as a list
154+
subprocess.run(["python", "convert.py", "--model", model_name])
155+
```
156+
157+
### Other patterns to avoid
158+
159+
- **`eval()` / `exec()`** on strings derived from external input. If you must generate and execute code dynamically, validate the input against an allowlist of safe patterns.
160+
- **Hardcoded secrets or credentials** — never commit tokens, passwords, or API keys. Use environment variables or config files listed in `.gitignore`.
161+
162+
### Bandit security checks
163+
164+
Bandit is used as a pre-commit hook to check for security-sensitive patterns in the code. **`# nosec` comments are not allowed** as a bypass for security checks.
165+
166+
### Creating a security exception
167+
168+
If a security-sensitive pattern (e.g. `pickle`, `subprocess`) is genuinely required, the contributor must:
169+
170+
1. **Add an inline comment** explaining *why* the pattern is necessary and *why* it is safe in this specific context (e.g. "loaded file is generated internally by ModelOpt").
171+
1. **Request review from [@NVIDIA/modelopt-setup-codeowners](https://github.com/orgs/NVIDIA/teams/modelopt-setup-codeowners)** and include a clear justification in the PR description.

docs/source/reference/2_security.rst

Lines changed: 0 additions & 78 deletions
This file was deleted.
Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
11
cuda-python<13
22
nvtx
33
opencv-python>=4.8.1.78,<4.12.0.88
4-
sentencepiece

examples/gpt-oss/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ Performing finetuning with Quantization Aware Training solves these issues. The
2020
Install the necessary dependencies:
2121

2222
```bash
23+
pip install -U nvidia-modelopt[hf]
2324
pip install -r requirements.txt
2425
```
2526

0 commit comments

Comments
 (0)