NVIDIA-NeMo / NVIDIA-NeMo/RL

Secrets in the environment leak into checkpoint config.yaml — env_vars dict is mutated in place with os.environ

Open
#3,384 1 comment 0 reactions 1 assignee Claimed by @terrykong View on GitHub
bug community-request waiting-on-customer
Dominant language
Python
Stars
2k
Forks
561
Avg merge
4d 5h
Merged PRs (30d)
145

Description

**Describe the bug**

When `policy.dtensor_cfg.env_vars` is non-empty, the **entire environment of the launching process** is merged into the run config and written to every checkpoint's `config.yaml`. If the environment holds secrets — `OPENAI_API_KEY`, `HF_TOKEN`, `WANDB_API_KEY`, cloud credentials — they are persisted into the checkpoint in plaintext.

`RayWorkerGroup._create_workers_from_bundle_indices` merges `os.environ` into the `env_vars` dict **in place**, and that dict is a live reference into the master config, so the pollution reaches `CheckpointManager`, which dumps the config verbatim.

Checkpoints are routinely copied, uploaded to object storage and shared, so this silently distributes live credentials.

**Steps/Code to reproduce bug**

```bash
export MY_FAKE_SECRET=sk-this-should-never-be-persisted

uv run python examples/run_sft.py \
policy.model_name=Qwen/Qwen2.5-0.5B \
+policy.dtensor_cfg.env_vars.FOO=bar \
sft.max_num_steps=1 \
checkpointing.enabled=true \
checkpointing.save_period=1

grep MY_FAKE_SECRET results/sft/step_1/config.yaml # -> found
```

`+policy.dtensor_cfg.env_vars.FOO=bar` is required: it makes `env_vars` non-empty. With an empty block, `env_vars or {}` (`lm_policy.py:291`) substitutes a fresh dict and the bug is masked.

No GPU is needed to see the mechanism — calling `_create_workers_from_bundle_indices` with a dict taken from a config, then passing that config to `CheckpointManager.init_tmp_checkpoint`, is enough to get the secret into the written `config.yaml`.

**Expected behavior**

- `config.yaml` in a checkpoint should contain the run configuration as loaded, not the process environment.
- Passing `env_vars` into a worker group should not mutate the caller's dict.
- Secrets present in the environment should never be persisted to disk by checkpointing.

**Additional context**

Verified on `main` @ `80555d3` (2026-07-28).

Root cause, three pieces:

1. `nemo_rl/distributed/worker_groups.py:452-454` — in-place merge into the caller's dict:

```python
# Update env_vars with the current environment variables
for k, v in os.environ.items():
if k not in env_vars:
env_vars[k] = v
```

2. `nemo_rl/models/policy/lm_policy.py:187` — the dict passed in is a live reference into the master config: `config["dtensor_cfg"].get("env_vars", {})`, no copy. The megatron path had the same issue until #2355 added a `dict(...)` copy at `lm_policy.py:141`; the dtensor path was not covered.

3. The training loops (`sft.py`, `dpo.py`, `grpo.py`, …) pass `master_config` to `CheckpointManager.init_tmp_checkpoint`, which writes it as `step_N/config.yaml` (`nemo_rl/utils/checkpoint.py:284`).

The pydantic migration does not protect against this: `PolicyConfig` is a `TypedDict`, so the aliased `env_vars` dict lives inside the pydantic model and `run_config.model_dump()` serializes it intact.

Easy to miss, because logged hyperparameters look clean — `logger.log_hyperparams(master_config)` runs during setup, *before* the worker group is created, while checkpoints are written after the mutation. Tracking-backend params therefore show only the declared `env_vars` while every `step_N/config.yaml` carries the full environment.

Exported models are affected too. The HF `config.json` itself is clean (built from `AutoConfig.from_pretrained(model_name, **hf_overrides)`), but `hf_checkpoint/` is written *inside* the leaking checkpoint directory:

```
step_N/
config.yaml <-- contains the full launcher environment
training_info.json
policy/
hf_checkpoint/ <-- the exported HF model
```

Since directory copies usually take `step_N/` as a unit, the secret-bearing `config.yaml` travels with the published model.

Suggested fix — copy at the mutation site, so every current and future caller is covered:

```python
# nemo_rl/distributed/worker_groups.py, _create_workers_from_bundle_indices
env_vars = dict(env_vars)
for k, v in os.environ.items():
if k not in env_vars:
env_vars[k] = v
```

Defense in depth worth considering: redact values whose keys match `(KEY|TOKEN|SECRET|PASSWORD|CREDENTIAL)` in `CheckpointManager.init_tmp_checkpoint` before `yaml.safe_dump`, since checkpoint configs travel widely.

Happy to send a PR for either or both.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.