Secrets in the environment leak into checkpoint config.yaml — env_vars dict is mutated in place with os.environ
- 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
Assessment
This issue has not been assessed yet.