You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opennlp.apache.org by "sumanpal97 (via GitHub)" <gi...@apache.org> on 2026/08/22 10:07:08 UTC
[PR] OPENNLP-1902: Verify cached models against their SHA-512 checksum (opennlp)
sumanpal97 opened a new pull request, #1239:
URL: https://github.com/apache/opennlp/pull/1239
Fixes [OPENNLP-1902](https://issues.apache.org/jira/browse/OPENNLP-1902).
`downloadModel(URL, Class)` verified the SHA-512 checksum only on the path where the model was actually fetched, so once a model was in the download home every subsequent load trusted it unconditionally.
### Approach
- The published `.sha512` is stored next to the model on first download, so the cache carries its own reference checksum.
- A cached model is re-verified against that stored checksum on every load. This is done locally, with no request to the CDN — calling `validateModel(...)` directly on the cache path would have closed the gap but turned every cached load into a network request, which the ticket notes would break offline reuse of the download home.
- `existsModel(...)` and `downloadModel(...)` now share the same verification routine, so the two entry points no longer disagree.
- Caches populated by earlier versions have no stored checksum. For those the published one is fetched once and then stored; if it cannot be retrieved, the model is loaded and a warning is logged so existing offline setups keep working. I am happy to make this fail closed instead if that is preferred.
- The sidecar keeps the published file's content unmodified, so a cache stays verifiable with standard tooling (`shasum -a 512 -c`).
- `SECURITY.md` has been updated under "Downloaded models".
### Tests
`DownloadUtilCacheIntegrityTest` was added, covering:
- a first download populates the cache and stores the published checksum
- a tampered cached model is rejected
- a valid cached model still loads when the published artifacts are unavailable
- a legacy cache without a sidecar is verified when the checksum is reachable
- a legacy cache without a sidecar loads when the checksum is unreachable, storing nothing
The tests never touch the network: the remote side is a `file:` URL in a temporary directory and the download home is redirected via `OPENNLP_DOWNLOAD_HOME`. The tampering test substitutes a different but valid model rather than random bytes, so a failure can only come from the checksum comparison and not from the model parser rejecting malformed input.
### For all changes:
- [x] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
- [x] Does your PR title start with OPENNLP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [x] Has your PR been rebased against the latest commit within the target branch (typically main)?
- [x] Is your initial contribution a single, squashed commit?
### For code changes:
- [x] Have you ensured that the full suite of tests is executed via mvn clean install at the root opennlp folder?
- [x] Have you written or updated unit tests to verify your changes?
- [x] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? (No new dependencies.)
- [x] If applicable, have you updated the LICENSE file, including the main LICENSE file in opennlp folder? (Not applicable.)
- [x] If applicable, have you updated the NOTICE file, including the main NOTICE file found in opennlp folder? (Not applicable.)
### For documentation related changes:
- [x] Have you ensured that format looks appropriate for the output in which it is rendered?
@rzo1 — you raised this one, so I would appreciate your review when you have a moment. In particular I would welcome your view on the legacy-cache behaviour, since the ticket left that decision open. Thank you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-1902: Verify cached models against their SHA-512 checksum (opennlp)
Posted by "mawiesne (via GitHub)" <gi...@apache.org>.
mawiesne merged PR #1239:
URL: https://github.com/apache/opennlp/pull/1239
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-1902: Verify cached models against their SHA-512 checksum (opennlp)
Posted by "sumanpal97 (via GitHub)" <gi...@apache.org>.
sumanpal97 commented on code in PR #1239:
URL: https://github.com/apache/opennlp/pull/1239#discussion_r3836519479
##########
SECURITY.md:
##########
@@ -148,10 +148,12 @@ JVM arguments, or the `OPENNLP_MAX_ENTRIES` setting.
CDN (`https://dlcdn.apache.org/opennlp/`) and verifies each download against the
published SHA-512 checksum before use. A mismatch fails the load.
-Note that verification happens at **download** time. A model already present in
-the local download cache is currently not re-verified when it is loaded again, so
-treat the cache directory (`~/.opennlp` by default) as trusted storage and protect
Review Comment:
Makes sense, restored. Agreed the guidance still applies regardless of the checksum
verification. Thanks for the review!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-1902: Verify cached models against their SHA-512 checksum (opennlp)
Posted by "jzonthemtn (via GitHub)" <gi...@apache.org>.
jzonthemtn commented on code in PR #1239:
URL: https://github.com/apache/opennlp/pull/1239#discussion_r3836355538
##########
SECURITY.md:
##########
@@ -148,10 +148,12 @@ JVM arguments, or the `OPENNLP_MAX_ENTRIES` setting.
CDN (`https://dlcdn.apache.org/opennlp/`) and verifies each download against the
published SHA-512 checksum before use. A mismatch fails the load.
-Note that verification happens at **download** time. A model already present in
-the local download cache is currently not re-verified when it is loaded again, so
-treat the cache directory (`~/.opennlp` by default) as trusted storage and protect
Review Comment:
Agree with removing the first part, but I think I'd keep the second part "Treat the cache directory (`~/.opennlp` by default) as trusted storage and protect it accordingly." I think we should still encourage users to protect that directory regardless.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-1902: Verify cached models against their SHA-512 checksum (opennlp)
Posted by "jzonthemtn (via GitHub)" <gi...@apache.org>.
jzonthemtn commented on PR #1239:
URL: https://github.com/apache/opennlp/pull/1239#issuecomment-5381154260
Thanks for the PR @sumanpal97!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org