Skip to content

Explicitly restrict checkpoint loading - #4

Merged
jjomier merged 2 commits into
mainfrom
fix/restricted-checkpoint-loading
Sep 22, 2026
Merged

jjomier merged 2 commits into
mainfrom
fix/restricted-checkpoint-loading

Conversation

@waltsims

Copy link
Copy Markdown
Contributor

Explicitly set weights_only=True in load_checkpoint and add regression tests. Both tests pass on PyTorch 2.9.1.

@waltsims
waltsims requested review from jjomier and a lite review from Copilot September 22, 2026 21:06
@jjomier
jjomier merged commit cc8d598 into main Sep 22, 2026
2 checks passed
@jjomier
jjomier deleted the fix/restricted-checkpoint-loading branch September 22, 2026 21:07
@greptile-apps

greptile-apps Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

The implementation appears safe to merge, with a non-blocking request to retain regression coverage for the security-sensitive checkpoint-loading contract.

Findings

  1. P2 Checkpoint Tests Are Missing ▶

Summary

This PR explicitly restricts training checkpoint deserialization to tensors and primitive state by passing weights_only=True to torch.load.

  • Reduces exposure to arbitrary pickle object construction.
  • Remains compatible with the repository’s supported PyTorch versions and generated checkpoints.
  • Does not include the regression tests described in the PR description.

Reviews (1) · Last reviewed commit: "Remove checkpoint regression tests"

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The change is straightforward; the only noted gap is a minor request for regression coverage.

Review effort: Lite
Findings: None

What changed in this PR

Restricts checkpoint loading to safe tensor/primitive deserialization and documents the supported format.

Changes:

  • Sets weights_only=True in load_checkpoint.
  • Documents restricted checkpoint behavior.
File Summary Review note
utils/​training.py Restricts checkpoint deserialization. Regression tests referenced in the PR description are not included.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread utils/training.py
def load_checkpoint(path: Path | str) -> dict[str, Any]:
return torch.load(Path(path), map_location="cpu")
"""Load tensor and primitive checkpoint state without arbitrary pickle objects."""
return torch.load(Path(path), map_location="cpu", weights_only=True)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Checkpoint Tests Are Missing

The new weights_only=True restriction has no committed regression coverage, despite the PR description stating that tests were added. Please add tests showing that tensor and primitive checkpoints still round-trip and that unsupported pickle objects are rejected; otherwise, this security-sensitive behavior could silently regress.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants