[jira] [Created] (OPENNLP-1901) Verify and pin teacher model downloads in opennlp-embeddings

["Kristian Rickert (Jira)" <ji...@apache.org>]

Kristian Rickert created OPENNLP-1901:
-----------------------------------------

             Summary: Verify and pin teacher model downloads in opennlp-embeddings
                 Key: OPENNLP-1901
                 URL: https://issues.apache.org/jira/browse/OPENNLP-1901
             Project: OpenNLP
          Issue Type: Improvement
            Reporter: Kristian Rickert



{panel:title=Status|borderStyle=solid}
The implementation is already written and pushed to the OPENNLP-1877 branch (PR #1152); this ticket records the change rather than requesting it. Nothing here is a defect in a released version: {{opennlp-embeddings}} does not exist in {{main}} and shipped in neither 3.0.0-M4 nor 3.0.0-M5.
{panel}

h2. Why

The distiller added in OPENNLP-1877 can name its teacher by hub id, in which case {{HuggingFaceModelCache}} downloads the teacher's {{tokenizer.json}} and {{onnx/model.onnx}} and the distiller then executes that ONNX graph. As first written it fetched from {{/resolve/main/}}, a mutable ref, and verified nothing.

That is out of step with the project on two counts.

{{DownloadUtil}} has always fetched a {{.sha512}} sidecar, computed SHA-512 over the download, and thrown {{IOException}} both on mismatch and when the checksum cannot be retrieved at all. OpenNLP already refuses to use a model it cannot verify.

The security model published in OPENNLP-1900 states the same thing as a property of the project: under "Downloaded models" it records the trust level as "trusted, integrity-checked in transit," and it names as reportable weaknesses "a checksum that is not actually checked," "a redirect that bypasses verification," and "a path traversal in how a downloaded artifact is named or cached." It also says ONNX models should be treated exactly as native libraries, since OpenNLP adds no sandbox of its own.

An unverified download path in {{opennlp-embeddings}} would have made that document inaccurate on the day the module shipped.

h2. What changed

* The teacher's ref is resolved to a commit sha once, from the hub's {{x-repo-commit}} header, and every file is then requested at {{/resolve/<sha>/}}. A force-push midway can no longer mix two revisions into one cache directory. A teacher may now name a revision explicitly as {{org/model@revision}}.
* Every file is verified against the digest the hub publishes in {{x-linked-etag}}, choosing the algorithm by hex length: 40 is the git blob SHA-1 over {{"blob <len>\\0"}} and the bytes, 64 is the SHA-256 of the content. The digest is computed over the bytes written to disk. A mismatch deletes the partial file and throws, in {{DownloadUtil}}'s wording.
* A file whose digest the hub does not publish is refused rather than used, which is the point of the change and matches {{DownloadUtil}}'s posture.
* The resolved commit is recorded in {{.opennlp-revision}} in the cache directory, and carried into the distilled model's {{config.json}} as {{teacher_revision}}, so a table can name the teacher revision it came from.

Two findings from the review of the change are worth recording, because both map onto weaknesses OPENNLP-1900 explicitly asks to hear about:

* *A redirect that bypassed verification.* With {{Redirect.NORMAL}} the final CDN response carries neither the commit nor the etag header; only the redirecting hub response does. Headers are now read by walking {{HttpResponse.previousResponse()}} back to the origin hop. A test fails if that walk is removed.
* *Cache naming that was not injective.* The directory name replaced {{/}}, {{.}} and {{@}} without distinguishing them, so {{acme/model@v1}}, {{acme/model.v1}} and {{acme/model_v1}} all mapped to one directory. Because a complete cache directory is reused without contacting the hub, one teacher could have been served another repository's files. The name now carries a digest of the exact reference.

h2. Verification

Tests need no network: a loopback {{com.sun.net.httpserver.HttpServer}} serves canned replies and records the requests made. Coverage includes both digest forms, a corrupted body, a missing and six malformed etags, optional and required 404s, the redirect path, the zero request cached path, revision pinning, and the directory collision above. The git blob helper is pinned against a digest produced by {{git hash-object}} over the same bytes, and a separate test proves the plain SHA-1 of that content is rejected.

Module tests: 310, none skipped.

h2. Not addressed here

Two related decisions on the same PR are deliberately left open, since they are design calls rather than integrity gaps:

* {{onnxruntime}} is a compile-scope, non-optional dependency, so consumers who only load a distilled table pull a 42 MB native runtime they never call.
* The {{embeddings}} launcher hardcodes {{-Xmx1024m}}, which cannot run the {{BAAI/bge-m3}} example the training guide opens with.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)