Skip to content

fix(checkpoint): reject weight_map shard names that leave the checkpoint directory - #1054

Open
li-lizhe wants to merge 1 commit into
sgl-project:mainfrom
li-lizhe:fix/weight-map-shard-containment
Open

li-lizhe wants to merge 1 commit into
sgl-project:mainfrom
li-lizhe:fix/weight-map-shard-containment

Conversation

@li-lizhe

Copy link
Copy Markdown
Contributor

Motivation

read_weight_map / load_checkpoint_tensors read the weight_map of a sharded checkpoint index as
shard file names, then join them to the checkpoint directory:

shard_path = os.path.join(checkpoint_dir, shard)

Nothing checks where that join lands. An index that maps a tensor to
../outside/weights.safetensors, or to an absolute path, makes the loader read a weight file outside
the directory the caller pointed at. Eagle3DraftModel.load_embedding and export_to_hf use exactly
this path for --embedding-source, so the tensor ends up in embed_tokens.weight of the exported
draft: a package that contains only config.json and model.safetensors.index.json still exports,
carrying weights that came from somewhere else.

Reported in #1041 with a reproduction I confirmed independently on CPU: with the index pointing at
../private/licensed.safetensors, load_checkpoint_tensors(pkg, keys=["model.embed_tokens.weight"])
returns the outside tensor (7.3125 marker), while the in-package control returns its own shard's
value.

Modifications

_read_weight_map_file now rejects a weight_map value that does not stay inside the checkpoint
directory, next to the existing weight_map validation:

  • an absolute value is rejected outright — an index maps to shard file names, not paths;
  • a relative value must resolve inside the directory (os.path.realpath + os.path.commonpath),
    which also covers .. hidden in the middle of a value and symlinks that point out.

The check sits in the one place every sharded load passes through, so load_checkpoint_tensors,
read_weight_map and list_checkpoint_keys all get it. Single-file checkpoints are unaffected: their
path comes from the fixed layout table, not from the index.

The error names the index file, the offending value, and the directory, so a legitimate package is not
left guessing.

Related Issues

Fixes #1041

Accuracy Test

No model-side or kernel code is touched. The new test covers the three escape forms (parent-relative,
.. in the middle, absolute) plus the in-package control that must keep loading:

python -m pytest tests/test_modeling/test_target/test_checkpoint_loading.py -q
  • before the change, with the new test added: 3 failed (ValueError not raised for each escape
    form), 8 passed, 16 subtests passed
  • after the change: 8 passed, 19 subtests passed

Environment for that run: Windows 11, Python 3.11, torch 2.8.0+cpu, safetensors 0.8.0.
tests/test_utils/test_mtp.py (the other caller of read_weight_map) could not be collected here
because this machine has no transformers; it only exercises in-package shard names, which the
in-package control case reproduces, and it is left to CI.

Checklist

  • black / isort clean on both files (repo pre-commit config)
  • Unit test added
  • No user-facing documentation change needed — the loader now rejects indexes that were never
    valid input

…int directory

The weight_map of a sharded checkpoint index is read as shard file names inside
the checkpoint directory, but the loader joined the value to the directory
without checking where it lands. An index that maps a tensor to
../outside/file.safetensors, or to an absolute path, made the loader read a file
outside the package; the exported draft then carried those weights, and a
package holding no weight file at all still exported successfully.

Refuse such an index instead of following it: an absolute value is rejected
outright, and a relative one must resolve inside the checkpoint directory.
Every sharded load goes through load_checkpoint_tensors, so the check lives in
_read_weight_map_file next to the existing weight_map validation.

Test added in test_checkpoint_loading.py covers both escape forms, the absolute
form, and the in-package control that must keep loading.

Signed-off-by: li-lizhe <147392333@qq.com>

This branch has not been deployed

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

Labels

None yet

Projects

None yet

1 participant