Skip to content

fix(resources): verify NLTK mirror archives with pinned digests - #1663

Merged
Yunnglin merged 1 commit into
modelscope:mainfrom
git-jxj:codex/fix-nltk-mirror-digests
Aug 28, 2026
Merged

fix(resources): verify NLTK mirror archives with pinned digests#1663
Yunnglin merged 1 commit into
modelscope:mainfrom
git-jxj:codex/fix-nltk-mirror-digests

Conversation

@git-jxj

@git-jxj git-jxj commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR makes fallback NLTK resource downloads fail closed unless the downloaded archive matches a pinned SHA-256 digest.

  • Pin a digest for every enabled punkt_tab, stopwords, and averaged_perceptron_tagger_eng mirror URL.
  • Pass the URL-specific digest to the existing download helper before extraction.
  • Remove the Gitee punkt_tab fallback because it has no trusted digest.
  • Add offline regression coverage for digest configuration, digest forwarding, checksum failure, and cleanup.

Root Cause

When a required NLTK resource was absent, check_nltk_data downloaded a ZIP from a mutable third-party mirror and extracted it directly. Although download_url already supports checksum verification, the call did not provide sha256. A valid but replaced or corrupted archive could therefore be installed into the NLTK data directory.

Changes

  • Move the NLTK mirror configuration to a module-level MIRROR_MAP with per-URL SHA-256 values.
  • Use download_url(url, zip_path, sha256=...) for every enabled mirror.
  • Keep extraction after the verified download only; a checksum failure raises and does not invoke ZipFile.
  • Verify the three pinned archives again through the configured proxy before this commit:
    • ModelScope punkt_tab: c2b16c23d738effbdc5789d7aa601397c13ba2819bf922fb904687f3f16657ed
    • Gitee stopwords: 15c94179887425ca1bedc265608cab9f27d650211f709bb929e320990a4b01d1
    • NLTK GitHub averaged_perceptron_tagger_eng: 6025f530624335c67d6547d44757b357b4e79bae030a0383e9887a92c1718f0b

Reproduction

On the unmodified base, monkeypatching download_url to record its keyword arguments during check_nltk_data(averaged_perceptron_tagger_eng) records sha256 as not passed. The downloader then skips the checksum comparison and resource_utils extracts the returned archive.

Validation

Targeted resource tests:

python -m pytest tests/test_resource_utils.py -q
6 passed

Adjacent NLTK consumer tests:

python -m pytest tests/benchmark/test_ifeval_language_checkers.py -q
2 passed

CI smoke test:

python -m pytest tests/cli/test_all.py::TestRun::test_ci_lite -q -s -p no:warnings
1 passed

Lint:

PATH=/root/code/evalscope/.venv/bin:$PATH make lint
Passed

All added tests use mocked NLTK and download behavior; no test accesses the network.

Scope

The change is limited to NLTK mirror integrity and its tests. It intentionally reuses the existing checksum-capable download helper and does not include HTTP resume or other download behavior changes.

@Yunnglin Yunnglin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM!

@Yunnglin
Yunnglin merged commit fe127d0 into modelscope:main Aug 28, 2026
3 checks passed
@git-jxj
git-jxj deleted the codex/fix-nltk-mirror-digests branch August 29, 2026 14:29
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.

2 participants