Skip to content

harden: verify dependency integrity and add download timeouts#1381

Draft
Jakubk15 wants to merge 1 commit into
masterfrom
harden/dependency-loader-integrity
Draft

harden: verify dependency integrity and add download timeouts#1381
Jakubk15 wants to merge 1 commit into
masterfrom
harden/dependency-loader-integrity

Conversation

@Jakubk15

@Jakubk15 Jakubk15 commented Jul 2, 2026

Copy link
Copy Markdown
Member

Fixes two findings from the loader audit (done by Claude):

  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.

  2. 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.

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

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Comment on lines +134 to +135
try (InputStream in = openStream(dependency.toMavenJarChecksum(repository, checksum.extension()).toURL().openConnection())) {
byte[] bytes = ByteStreams.toByteArray(in);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

security-medium medium

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.

Suggested change
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));

Comment on lines +81 to +90
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();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

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);
    }

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.

1 participant