harden: verify dependency integrity and add download timeouts#1381
harden: verify dependency integrity and add download timeouts#1381Jakubk15 wants to merge 1 commit into
Conversation
Fixes two findings from the loader audit: 1. Runtime-downloaded dependencies were loaded with no integrity check. DependencyDownloader now verifies every artifact against the strongest checksum the repository publishes (sha512 > sha256 > sha1) before it is written to the local cache and added to the classpath. Fails closed: a mismatch or the absence of any published checksum rejects that repository so resolution falls back to another source (and errors if none can vouch for the artifact). Adds a pure, unit-tested Checksum utility. 3. Jar, checksum, and POM fetches had no connect/read timeouts, so a slow or half-open repository could stall plugin startup. All three now set a 15s connect / 30s read timeout. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JrkLFxzmmn7BpB9y6vMTeg
There was a problem hiding this comment.
Code Review
This pull request introduces dependency checksum verification (supporting SHA-512, SHA-256, and SHA-1) during the dependency download process, along with connection and read timeouts for network requests. Feedback suggests enhancing security by limiting the input stream size when downloading checksums to prevent potential Denial of Service (DoS) attacks, and simplifying the codebase by replacing the custom hex conversion logic with Java's standard HexFormat utility.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| try (InputStream in = openStream(dependency.toMavenJarChecksum(repository, checksum.extension()).toURL().openConnection())) { | ||
| byte[] bytes = ByteStreams.toByteArray(in); |
There was a problem hiding this comment.
To prevent potential OutOfMemoryError or Denial of Service (DoS) attacks from a malicious or misconfigured repository serving an excessively large or infinite stream for the checksum file, it is highly recommended to limit the number of bytes read from the input stream. Since a valid checksum is always very small (typically under 200 bytes), we can safely limit the download to 1024 bytes using ByteStreams.limit.
| try (InputStream in = openStream(dependency.toMavenJarChecksum(repository, checksum.extension()).toURL().openConnection())) { | |
| byte[] bytes = ByteStreams.toByteArray(in); | |
| try (InputStream in = openStream(dependency.toMavenJarChecksum(repository, checksum.extension()).toURL().openConnection())) { | |
| byte[] bytes = ByteStreams.toByteArray(ByteStreams.limit(in, 1024)); |
| private static String toHex(byte[] bytes) { | ||
| StringBuilder builder = new StringBuilder(bytes.length * 2); | ||
|
|
||
| for (byte value : bytes) { | ||
| builder.append(Character.forDigit((value >> 4) & 0xF, 16)); | ||
| builder.append(Character.forDigit(value & 0xF, 16)); | ||
| } | ||
|
|
||
| return builder.toString(); | ||
| } |
There was a problem hiding this comment.
Since the project is using Java 25, we can leverage the standard java.util.HexFormat utility introduced in Java 17 instead of implementing a custom hex conversion method. This simplifies the codebase, improves maintainability, and reduces the risk of bugs in custom bit-manipulation logic.
private static String toHex(byte[] bytes) {
return java.util.HexFormat.of().formatHex(bytes);
}
Fixes two findings from the loader audit (done by Claude):
Runtime-downloaded dependencies were loaded with no integrity check. DependencyDownloader now verifies every artifact against the strongest checksum the repository publishes (sha512 > sha256 > sha1) before it is written to the local cache and added to the classpath. Fails closed: a mismatch or the absence of any published checksum rejects that repository, so resolution falls back to another source (and errors if none can vouch for the artifact). Adds a pure, unit-tested Checksum utility.
Jar, checksum, and POM fetches had no connect/read timeouts, so a slow or half-open repository could stall plugin startup. All three now set a 15s connect / 30s read timeout.