Harden release downloader and finalize releases-first model

[tazarov]

Summary

This PR finalizes the releases-first download architecture and hardens the downloader, verification, and docs/workflow consistency.

What changed

• Uses fixed download sources:
  • Primary: https://releases.amikos.tech/pure-tokenizers
  • Fallback: GitHub releases from amikos-tech/pure-tokenizers
• Removes release-source env overrides (TOKENIZERS_RELEASES_BASE_URL, TOKENIZERS_RELEASES_PROJECT, TOKENIZERS_GITHUB_REPO) from runtime behavior.
• Adds one-time warning when legacy TOKENIZERS_GITHUB_REPO is set (now ignored).
• Enforces strict checksum behavior and keeps fallback warnings consistent across primary/fallback paths.
• Replaces panic-recovery symbol probing with explicit symbol checks:
  • unix: purego.Dlsym
  • windows: windows.GetProcAddress
• Validates ABI compatibility and required symbols for downloaded libraries.
• Moves post-download ABI validation into DownloadAndCacheLibrary() so standalone calls are safe.
• Avoids duplicate cache clear calls in LoadTokenizerLibrary().
• Fixes stale docs/workflow references (rust-release.yml, go-release.yml, make test, checksum manifest mention).
• Fixes examples/example_download.go to compile (removes non-existent WithDownloadLibrary()).
• Adds/extends tests for:
  • request header auth scoping + GH_TOKEN fallback
  • checksum parsing (missing entry, invalid, empty/comment-only manifests)
  • helper URL builders and hash validator

Validation

• go test ./...
• TOKENIZERS_REQUIRE_ONLINE_TESTS=1 go test -run 'TestDownloadFunctionality/GetAvailableVersions|TestDownloadLibraryFromGitHub' -count=1 ./...
• Focused helper tests for checksum/url/header behavior

Notes

• Untracked local file skills-lock.json is intentionally not included in this PR.

[claude]

PR Review: Harden release downloader and finalize releases-first model

Summary

Overall this is a solid improvement — the releases-first architecture is cleaner, auth token scoping is correct, and strict checksum enforcement closes real gaps. A few things worth addressing:

---

Issues / Concerns

1. Double-load inefficiency in LoadTokenizerLibrary

verifyLibraryABICompatibility loads, inspects, then closes the library — and immediately after, loadLibrary is called again on the same path. This pattern repeats for user path, env path, and cached path. It's not a correctness bug, but it doubles the OS library open/close cycles on the hot path.

Consider returning the library handle from a combined verify+load function to avoid the redundant syscalls.

---

2. Wrapper functions for constants add unnecessary indirection

getGitHubRepo(), getReleasesBaseURL(), and getReleasesProject() are trivial one-liners that just return the corresponding constants. These suggest they were kept from the env-override era. Since those overrides are now intentionally removed, the wrappers can be replaced with direct constant references — which is already what the PR enforces at the policy level.

---

3. ChecksumsURL in latest.json allows redirect to arbitrary HTTPS endpoint

In resolveChecksumsURL, an absolute HTTPS URL in the primary endpoint's latest.json is accepted as-is:

if parsed.IsAbs() {
    if !strings.EqualFold(parsed.Scheme, "https") {
        return "", fmt.Errorf("invalid checksums_url scheme...")
    }
    return checksumsURL, nil  // could be any HTTPS host
}

If releases.amikos.tech is compromised, an attacker could serve a latest.json pointing to an attacker-controlled HTTPS checksums file that matches a malicious binary. Unlike the GitHub fallback path (where checksums are assets on the same release), here the checksum source and asset source are decoupled.

Consider pinning the checksums URL to the same host as ReleasesBaseURL, or at minimum logging a warning when it resolves to a different host.

---

4. Module-level sync.Once warning state in tests

warnDownloadFallbackOnce, warnVersionsFallbackOnce, and warnIgnoredLegacyRepoEnv are package-level vars. In tests that run in the same process, these fire only on the first trigger and are permanently consumed. This is intentional for production use, but it means test ordering can affect whether a warning is observable. Not a blocker, just worth being aware of if debugging test flakiness.

---

Positives

• Auth scoping (setRequestHeaders) correctly gates GITHUB_TOKEN/GH_TOKEN and X-GitHub-Api-Version to api.github.com only — this is a meaningful security improvement over the previous code that sent tokens to any URL.
• os.MkdirTemp replaces the old fixed /tmp/tokenizers-download path, avoiding races on concurrent downloads.
• Strict checksum enforcement on the GitHub fallback (requires either SHA256SUMS or .sha256 — no skip-if-missing) closes the silent-skip bug.
• checksumForAsset handles comments, empty manifests, *-prefixed filenames, and invalid format — well covered by the new unit tests.
• verifyLibraryABICompatibility replacing panic-recovery symbol probing with explicit purego.Dlsym/GetProcAddress is strictly better.
• Test coverage for setRequestHeaders, checksumForAsset, buildReleaseURL, normalizeReleaseVersion, and resolveChecksumsURL is thorough.

[github-actions]

Benchmark Comparison

goos: linux
goarch: amd64
pkg: github.com/amikos-tech/pure-tokenizers
cpu: Intel(R) Xeon(R) Platinum 8370C CPU @ 2.80GHz
                                  │ base_bench.txt │            pr_bench.txt            │
                                  │     sec/op     │    sec/op     vs base              │
Encode/Short-4                        9.617µ ± ∞ ¹   9.494µ ± ∞ ¹  -1.28% (p=0.016 n=5)
Encode/Medium-4                       42.02µ ± ∞ ¹   41.66µ ± ∞ ¹       ~ (p=1.000 n=5)
Encode/Long-4                         318.1µ ± ∞ ¹   316.5µ ± ∞ ¹       ~ (p=0.548 n=5)
EncodeWithOptions/Default-4           41.53µ ± ∞ ¹   41.15µ ± ∞ ¹       ~ (p=0.421 n=5)
EncodeWithOptions/WithTypeIDs-4       41.43µ ± ∞ ¹   41.73µ ± ∞ ¹       ~ (p=0.222 n=5)
EncodeWithOptions/WithTokens-4        41.70µ ± ∞ ¹   41.26µ ± ∞ ¹       ~ (p=0.151 n=5)
EncodeWithOptions/WithOffsets-4       42.18µ ± ∞ ¹   41.68µ ± ∞ ¹       ~ (p=0.421 n=5)
EncodeWithOptions/AllOptions-4        43.99µ ± ∞ ¹   44.02µ ± ∞ ¹       ~ (p=0.421 n=5)
Decode/WithSpecialTokens-4            19.08µ ± ∞ ¹   19.12µ ± ∞ ¹       ~ (p=1.000 n=5)
Decode/SkipSpecialTokens-4            19.83µ ± ∞ ¹   19.04µ ± ∞ ¹       ~ (p=0.095 n=5)
BatchEncode-4                         423.2µ ± ∞ ¹   425.4µ ± ∞ ¹       ~ (p=0.690 n=5)
FromHuggingFace/CreationOnly-4        39.09m ± ∞ ¹   40.40m ± ∞ ¹       ~ (p=0.056 n=5)
FromHuggingFace/FullLifecycle-4       42.11m ± ∞ ¹   40.14m ± ∞ ¹  -4.67% (p=0.008 n=5)
VocabSize-4                           3.436m ± ∞ ¹   3.462m ± ∞ ¹       ~ (p=0.690 n=5)
EncodeDecode/Short-4                  14.17µ ± ∞ ¹   13.99µ ± ∞ ¹  -1.24% (p=0.032 n=5)
EncodeDecode/Medium-4                 63.14µ ± ∞ ¹   61.99µ ± ∞ ¹  -1.83% (p=0.008 n=5)
EncodeDecode/Long-4                   462.7µ ± ∞ ¹   457.8µ ± ∞ ¹       ~ (p=0.222 n=5)
Truncation-4                          314.4µ ± ∞ ¹   304.2µ ± ∞ ¹       ~ (p=0.151 n=5)
Padding-4                             113.6µ ± ∞ ¹   115.3µ ± ∞ ¹       ~ (p=0.222 n=5)
ConcurrentCacheRead-4                 3.602µ ± ∞ ¹   3.575µ ± ∞ ¹       ~ (p=0.421 n=5)
ConcurrentCacheValidation-4           4.421µ ± ∞ ¹   4.466µ ± ∞ ¹       ~ (p=0.151 n=5)
ConcurrentHFCacheLookup-4             6.705µ ± ∞ ¹   6.773µ ± ∞ ¹       ~ (p=0.222 n=5)
DownloadWithFailureRecovery-4          1.127 ± ∞ ¹    1.214 ± ∞ ¹       ~ (p=0.421 n=5)
ConcurrentDownloadsWithFailures-4     44.42m ± ∞ ¹   44.50m ± ∞ ¹       ~ (p=0.548 n=5)
FromHuggingFaceWithCache-4            5.478µ ± ∞ ¹   5.645µ ± ∞ ¹  +3.05% (p=0.008 n=5)
FromHuggingFaceWithoutCache-4         117.2µ ± ∞ ¹   118.7µ ± ∞ ¹       ~ (p=0.151 n=5)
geomean                               154.3µ         154.2µ        -0.07%
¹ need >= 6 samples for confidence interval at level 0.95

                                  │ base_bench.txt │             pr_bench.txt              │
                                  │      B/op      │     B/op       vs base                │
Encode/Short-4                         920.0 ± ∞ ¹     920.0 ± ∞ ¹       ~ (p=1.000 n=5) ²
Encode/Medium-4                      1.516Ki ± ∞ ¹   1.516Ki ± ∞ ¹       ~ (p=1.000 n=5) ²
Encode/Long-4                        6.703Ki ± ∞ ¹   6.703Ki ± ∞ ¹       ~ (p=1.000 n=5) ²
EncodeWithOptions/Default-4          1.516Ki ± ∞ ¹   1.516Ki ± ∞ ¹       ~ (p=1.000 n=5) ²
EncodeWithOptions/WithTypeIDs-4      1.609Ki ± ∞ ¹   1.609Ki ± ∞ ¹       ~ (p=1.000 n=5) ²
EncodeWithOptions/WithTokens-4       1.516Ki ± ∞ ¹   1.516Ki ± ∞ ¹       ~ (p=1.000 n=5) ²
EncodeWithOptions/WithOffsets-4      1.703Ki ± ∞ ¹   1.703Ki ± ∞ ¹       ~ (p=1.000 n=5) ²
EncodeWithOptions/AllOptions-4       2.109Ki ± ∞ ¹   2.109Ki ± ∞ ¹       ~ (p=1.000 n=5) ²
Decode/WithSpecialTokens-4             740.0 ± ∞ ¹     740.0 ± ∞ ¹       ~ (p=1.000 n=5) ²
Decode/SkipSpecialTokens-4             740.0 ± ∞ ¹     740.0 ± ∞ ¹       ~ (p=1.000 n=5) ²
BatchEncode-4                        11.30Ki ± ∞ ¹   11.30Ki ± ∞ ¹       ~ (p=1.000 n=5) ²
FromHuggingFace/CreationOnly-4       6.136Mi ± ∞ ¹   6.150Mi ± ∞ ¹       ~ (p=0.095 n=5)
FromHuggingFace/FullLifecycle-4      6.132Mi ± ∞ ¹   6.136Mi ± ∞ ¹       ~ (p=0.690 n=5)
VocabSize-4                            288.0 ± ∞ ¹     288.0 ± ∞ ¹       ~ (p=1.000 n=5) ²
EncodeDecode/Short-4                 1.516Ki ± ∞ ¹   1.516Ki ± ∞ ¹       ~ (p=1.000 n=5) ²
EncodeDecode/Medium-4                2.242Ki ± ∞ ¹   2.242Ki ± ∞ ¹       ~ (p=1.000 n=5) ²
EncodeDecode/Long-4                  8.430Ki ± ∞ ¹   8.430Ki ± ∞ ¹       ~ (p=1.000 n=5) ²
Truncation-4                         5.500Ki ± ∞ ¹   5.500Ki ± ∞ ¹       ~ (p=1.000 n=5) ²
Padding-4                            15.89Ki ± ∞ ¹   15.89Ki ± ∞ ¹       ~ (p=1.000 n=5) ²
ConcurrentCacheRead-4                2.062Ki ± ∞ ¹   2.062Ki ± ∞ ¹       ~ (p=1.000 n=5) ²
ConcurrentCacheValidation-4          3.023Ki ± ∞ ¹   3.023Ki ± ∞ ¹       ~ (p=1.000 n=5) ²
ConcurrentHFCacheLookup-4            3.181Ki ± ∞ ¹   3.181Ki ± ∞ ¹       ~ (p=0.722 n=5)
DownloadWithFailureRecovery-4        58.41Ki ± ∞ ¹   61.03Ki ± ∞ ¹       ~ (p=1.000 n=5)
ConcurrentDownloadsWithFailures-4    18.83Ki ± ∞ ¹   18.79Ki ± ∞ ¹       ~ (p=0.222 n=5)
FromHuggingFaceWithCache-4           1.727Ki ± ∞ ¹   1.727Ki ± ∞ ¹       ~ (p=1.000 n=5)
FromHuggingFaceWithoutCache-4        16.21Ki ± ∞ ¹   16.21Ki ± ∞ ¹       ~ (p=0.881 n=5)
geomean                              5.420Ki         5.429Ki        +0.17%
¹ need >= 6 samples for confidence interval at level 0.95
² all samples are equal

                                  │ base_bench.txt │             pr_bench.txt             │
                                  │   allocs/op    │  allocs/op    vs base                │
Encode/Short-4                         16.00 ± ∞ ¹    16.00 ± ∞ ¹       ~ (p=1.000 n=5) ²
Encode/Medium-4                        35.00 ± ∞ ¹    35.00 ± ∞ ¹       ~ (p=1.000 n=5) ²
Encode/Long-4                          165.0 ± ∞ ¹    165.0 ± ∞ ¹       ~ (p=1.000 n=5) ²
EncodeWithOptions/Default-4            35.00 ± ∞ ¹    35.00 ± ∞ ¹       ~ (p=1.000 n=5) ²
EncodeWithOptions/WithTypeIDs-4        36.00 ± ∞ ¹    36.00 ± ∞ ¹       ~ (p=1.000 n=5) ²
EncodeWithOptions/WithTokens-4         35.00 ± ∞ ¹    35.00 ± ∞ ¹       ~ (p=1.000 n=5) ²
EncodeWithOptions/WithOffsets-4        36.00 ± ∞ ¹    36.00 ± ∞ ¹       ~ (p=1.000 n=5) ²
EncodeWithOptions/AllOptions-4         41.00 ± ∞ ¹    41.00 ± ∞ ¹       ~ (p=1.000 n=5) ²
Decode/WithSpecialTokens-4             10.00 ± ∞ ¹    10.00 ± ∞ ¹       ~ (p=1.000 n=5) ²
Decode/SkipSpecialTokens-4             10.00 ± ∞ ¹    10.00 ± ∞ ¹       ~ (p=1.000 n=5) ²
BatchEncode-4                          261.0 ± ∞ ¹    261.0 ± ∞ ¹       ~ (p=1.000 n=5) ²
FromHuggingFace/CreationOnly-4        92.19k ± ∞ ¹   92.20k ± ∞ ¹  +0.01% (p=0.008 n=5)
FromHuggingFace/FullLifecycle-4       92.20k ± ∞ ¹   92.21k ± ∞ ¹  +0.01% (p=0.008 n=5)
VocabSize-4                            5.000 ± ∞ ¹    5.000 ± ∞ ¹       ~ (p=1.000 n=5) ²
EncodeDecode/Short-4                   26.00 ± ∞ ¹    26.00 ± ∞ ¹       ~ (p=1.000 n=5) ²
EncodeDecode/Medium-4                  45.00 ± ∞ ¹    45.00 ± ∞ ¹       ~ (p=1.000 n=5) ²
EncodeDecode/Long-4                    175.0 ± ∞ ¹    175.0 ± ∞ ¹       ~ (p=1.000 n=5) ²
Truncation-4                           127.0 ± ∞ ¹    127.0 ± ∞ ¹       ~ (p=1.000 n=5) ²
Padding-4                              535.0 ± ∞ ¹    535.0 ± ∞ ¹       ~ (p=1.000 n=5) ²
ConcurrentCacheRead-4                  25.00 ± ∞ ¹    25.00 ± ∞ ¹       ~ (p=1.000 n=5) ²
ConcurrentCacheValidation-4            43.00 ± ∞ ¹    43.00 ± ∞ ¹       ~ (p=1.000 n=5) ²
ConcurrentHFCacheLookup-4              38.00 ± ∞ ¹    38.00 ± ∞ ¹       ~ (p=1.000 n=5) ²
DownloadWithFailureRecovery-4          455.0 ± ∞ ¹    461.0 ± ∞ ¹       ~ (p=0.889 n=5)
ConcurrentDownloadsWithFailures-4      231.0 ± ∞ ¹    231.0 ± ∞ ¹       ~ (p=0.444 n=5)
FromHuggingFaceWithCache-4             7.000 ± ∞ ¹    7.000 ± ∞ ¹       ~ (p=1.000 n=5) ²
FromHuggingFaceWithoutCache-4          217.0 ± ∞ ¹    217.0 ± ∞ ¹       ~ (p=1.000 n=5) ²
geomean                                89.77          89.82        +0.05%
¹ need >= 6 samples for confidence interval at level 0.95
² all samples are equal

[claude]

PR Review: Harden release downloader and finalize releases-first model

Overall, this is a solid hardening PR. Good fixes: scoped auth headers to GitHub API only, os.MkdirTemp replacing shared temp dir (eliminates concurrent-download races), proper checksumForAsset parsing with typed sentinel errors, and post-download ABI validation moved into DownloadAndCacheLibrary. The test additions are valuable.

A few things worth considering:

Security

resolveChecksumsURL accepts arbitrary HTTPS host from latest.json (download.go):

If releases.amikos.tech is compromised, checksums_url in latest.json can redirect checksum fetching to an attacker-controlled server - the scheme-only https check is necessary but not sufficient. Consider validating the host against a known allowlist (e.g. releases.amikos.tech and objects.githubusercontent.com).

Performance

Double library load in the fast path (library_loading.go):

verifyLibraryABICompatibility loads and immediately closes the library, then loadLibrary loads it again. For the happy path (cached library, compatible), that is 2 full dlopen/dlclose cycles. Consider returning the handle from the check or splitting out a checkSymbolsAndVersion(handle) helper the caller invokes after opening.

API behavioral change

GetAvailableVersions now silently returns at most 1 version - the []string return type implies a list, but the new implementation always returns a single-element slice. The in-code comment explains this, but callers expecting multiple versions will silently get less data. Worth noting more prominently in the function doc comment or changelog.

Minor

• getReleasesBaseURL(), getReleasesProject(), getGitHubRepo() are now pure identity wrappers returning constants with no added behaviour - callers could reference ReleasesBaseURL, ReleasesProject, GitHubRepo directly.
• TestGetReleasesProject tests that getReleasesProject() returns ReleasesProject - this exercises Go's constant semantics, not any real logic.
• fetchLatestGitHubRustRelease fetches only page 1 of /releases (30 results by default). Fine for finding the latest, but a comment noting that rust-v* tags are assumed to appear on page 1 would help future readers.

[github-actions]

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Go Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite	Current: e07d837	Previous: 659e4cf	Ratio
BenchmarkDecode/WithSpecialTokens	22208 ns/op 740 B/op 10 allocs/op	18443 ns/op 740 B/op 10 allocs/op	1.20
BenchmarkDecode/WithSpecialTokens - ns/op	22208 ns/op	18443 ns/op	1.20

This comment was automatically generated by workflow using github-action-benchmark.

CC: @tazarov

[claude]

Review

Good overall direction — the releases-first architecture, auth header scoping, strict checksum enforcement, and replacing panic-recovery symbol probing with explicit Dlsym/GetProcAddress are all solid improvements.

Performance Regression

The benchmark CI flagged regressions (1.2–1.3x) on BenchmarkConcurrentCacheRead, BenchmarkConcurrentCacheValidation, and BenchmarkFromHuggingFaceWithCache. The likely cause is that verifyLibraryABICompatibilityHandle now loads/unloads the library on every cached-library validation path (LoadTokenizerLibrary priority 3, and again inside DownloadAndCacheLibrary). Consider whether ABI re-verification on every cache hit is necessary, or whether it should only run once at startup.

GetAvailableVersions Semantic Change

This now returns at most one version instead of the full list. The doc comment acknowledges this, but the function signature is unchanged, so callers expecting multiple versions will silently get a truncated result. Worth a deprecation note or a new function name to signal the contract change.

GitHub Fallback Pagination

fetchLatestGitHubRustRelease fetches only the first page (default 30 releases). If there are ever enough non-rust-v releases ahead of the latest Rust release, this will silently fail. Adding ?per_page=100 to the request or an explicit page scan would make this more robust.

Edge Case in normalizeReleaseVersion

Input "rust-" (bare prefix with empty suffix) returns "rust-" unchanged, which is neither DefaultTag nor a valid version. The version resolvers then proceed and will produce a confusing URL/error. A check for this case (e.g. treat it as DefaultTag or return an error early) would prevent user confusion.

Minor

• warnDownloadFallbackOnce / warnVersionsFallbackOnce are package-level singletons; fine for production but makes it impossible to assert warning behavior in parallel test suites without process isolation.
• Missing test: resolveChecksumsURL with a non-nil idx but empty ChecksumsURL (falls through to default path — the logic is correct but not explicitly covered).

[claude]

Supplementary Review Note

Three prior Claude reviews already cover the key issues well (double-load on the hot path, resolveChecksumsURL host allowlist, GetAvailableVersions semantic change, benchmark regression, GitHub fallback pagination). One additional gap worth noting:

Stale ABI verification cache on external file replacement

libraryABIVerifiedByPath caches successful ABI checks by file path indefinitely. ClearLibraryCache() correctly calls clearLibraryABIVerified(), but if the cached library file is replaced externally (e.g. another process writes a new binary to the same path), the in-process map will still report the old path as "verified" and skip the ABI check for the new binary. The fix would be to include the file's mtime or a content hash as part of the cache key, or accept the risk given the niche scenario.

Summary of open items from prior reviews

• Double-load inefficiency: verifyLibraryABICompatibility opens/closes the library, then loadLibrary opens it again — accounts for the observed 1.2–1.3x regression on cache-read benchmarks.
• resolveChecksumsURL host allowlist: An absolute URL in latest.json pointing to any allowed-list host is accepted; if releases.amikos.tech is compromised, a checksums_url field can redirect verification to a controlled server on that allowlist.
• GetAvailableVersions: Now always returns ≤ 1 version; the function signature implies a list — consider a doc note or changelog entry.
• GitHub fallback pagination: fetchLatestGitHubRustRelease fetches only the first page of /releases; a comment noting the assumption would help future readers.

[claude]

Review

Overall this is a solid improvement — the primary/fallback download architecture is clean, checksum enforcement is now strict, and auth token scoping to api.github.com only is a good security fix. A few observations:

Medium

Double library load in post-download verification (download.go):
DownloadAndCacheLibrary calls verifyLibraryABICompatibility(cachedPath) which does loadLibrary → verify → closeLibrary. Then the caller (LoadTokenizerLibrary) does loadLibrary again. The isLibraryABIVerified cache avoids re-checking on the second call, but the library is still opened twice. Not a correctness issue, just slightly wasteful especially on slower file systems.

purego.RegisterLibFunc with a transient handle (verifyLibraryABICompatibilityHandle):
purego.RegisterLibFunc registers a persistent mapping in purego's internal table, even though the library handle is closed immediately after the check. Calling it per-validation cycle for temporary handles could accumulate stale entries. A safer pattern would be to resolve the symbol via Dlsym and call it through unsafe.Pointer directly, without going through RegisterLibFunc.

Minor

isAllowedChecksumsHost re-parses ReleasesBaseURL on every call:
url.Parse(ReleasesBaseURL) is called inside the function. Since this is a package-level constant, parsing it once at init (or using a simple strings.HasSuffix against a precomputed hostname) would be cleaner.

warnDownloadFallbackOnce fires only once per process lifetime:
If the primary endpoint is down for all downloads in a long-running process, only the first fallback emits a warning. Subsequent downloads silently use GitHub fallback. This is fine for CLIs but for long-lived servers it could silently mask a persistent outage. Worth documenting this behavior.

fetchLatestGitHubRustRelease uses per_page=100 without pagination:
If there are ever >100 releases and all rust-v* tags are pushed past the first page, the fallback would fail to find any version. Extremely unlikely in practice, but a comment noting the limitation would help future maintainers.

Nits

• Good catch replacing the shared os.TempDir()/tokenizers-download path with os.MkdirTemp — that was a real concurrent-download race.
• The test coverage for checksumForAsset, setRequestHeaders, and resolveChecksumsURL is thorough and well-structured.
• map[string]interface{} → map[string]any modernization is a nice touch.

[claude]

Follow-up to prior reviews

Two previous review comments raised concerns that are actually already addressed in this PR — clarifying for record:

Corrections to prior review concerns

Checksums URL host restriction — both prior reviews flagged resolveChecksumsURL as accepting arbitrary HTTPS hosts. The PR already implements a host allowlist (allowedChecksumsHosts) initialized at package load time and enforced in resolveChecksumsURL. The allowlist contains releases.amikos.tech and objects.githubusercontent.com. The concern is resolved.

Double-load in LoadTokenizerLibrary — prior reviews cited verifyLibraryABICompatibility loading+closing then loadLibrary re-opening. The PR fixes this: LoadTokenizerLibrary now calls loadLibrary once to get libh and passes the already-open handle to verifyLibraryABICompatibilityHandle(libh), avoiding the redundant syscall on the hot path.

One new minor observation

libraryABIFingerprint uses size:mtime as a weak cache key (download.go):

A local-process cache replacement preserving size and mtime would bypass the ABI re-check. Acceptable for a locally-trusted cache but worth a comment explaining the trade-off.

---

Overall this is a well-structured PR. The security hardening (scoped auth headers, strict checksum enforcement, host allowlist, os.MkdirTemp replacing shared temp path) are all meaningful improvements.

[claude]

Supplemental Review Notes

Noting a correction and a couple of minor items not covered in the prior review.

Correction on prior review point #3 (checksums URL host):

The existing review claims resolveChecksumsURL accepts "any HTTPS host". This is inaccurate. The code does enforce an allowlist via isAllowedChecksumsHost, which only permits releases.amikos.tech and objects.githubusercontent.com. The tests confirm rejected hosts return an error. The security concern does not apply.

---

Minor: setRequestHeaders test name is slightly misleading

The test "github host uses GH_TOKEN when GITHUB_TOKEN is absent" sets GITHUB_TOKEN to an empty string (not truly absent). t.Setenv("GITHUB_TOKEN", "") sets the variable to "" in the environment. The behavior is correct because os.Getenv("GITHUB_TOKEN") != "" handles it identically — but renaming to "empty" would be more precise.

---

Minor: package-level libraryABIVerifiedByPath persists across test cases

libraryABIVerifiedByPath is module-level state. If multiple test cases in the same process exercise verifyLibraryABICompatibility on the same path, the second call short-circuits without re-loading. This is correct for production (avoiding redundant syscalls) but could hide isolation issues in tests if a path is reused with a different binary. Not a blocker — just worth noting if a future test asserts re-verification happens.

[claude]

Supplementary Review

Several prior reviews cover the main points well. Two gaps worth adding:

Benchmark regression root cause: double os.Stat on fast path

Prior reviews attributed the BenchmarkConcurrentCacheRead/ConcurrentCacheValidation/FromHuggingFaceWithCache regressions (1.2–1.3x) to the double-load, but the PR already fixed that. The remaining cause is likely double os.Stat per library load:

// LoadTokenizerLibrary - Priority 3 path:
if _, statErr := os.Stat(cachedPath); statErr == nil {  // stat #1
    ...
    if !isLibraryABIVerified(cachedPath) {
        // isLibraryABIVerified calls libraryABIFingerprint → os.Stat(cachedPath)  // stat #2
    }
}

The previous code called isLibraryValid which did one os.Stat. The new code does two. The fix would be to reuse the os.FileInfo from the first stat inside libraryABIFingerprint, or avoid the second stat by tracking fingerprint differently (e.g. store size+mtime from the first stat call).

purego.RegisterLibFunc on a handle that may be closed on failure

In verifyLibraryABICompatibilityHandle (on the error path), the caller closes libh after this function returns an error, but purego.RegisterLibFunc may have already registered a mapping against that handle. If the OS later reuses the same handle value for a different library, the stale entry could cause incorrect dispatch. A safer pattern (as noted in one prior review) would be to use purego.Dlsym + syscall/unsafe.Pointer to call get_version directly without RegisterLibFunc:

sym, err := purego.Dlsym(libh, "get_version")
if err != nil { return err }
// call via unsafe.Pointer rather than registering a persistent binding

---

Overall the PR is in good shape. These are minor — the security hardening (scoped auth headers, host allowlist, strict checksum enforcement, os.MkdirTemp) are meaningful improvements.