Skip to content

Commit

Permalink
Handle GitHub GraphQL API rate limiting
Browse files Browse the repository at this point in the history
Ports DependencyTrack/dependency-track#4578 from Dependency-Track v4.12.3

Signed-off-by: nscuro <[email protected]>
  • Loading branch information
nscuro committed Jan 27, 2025
1 parent afdd790 commit 28611a6
Show file tree
Hide file tree
Showing 13 changed files with 524 additions and 58 deletions.
5 changes: 5 additions & 0 deletions mirror-service/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,11 @@
<artifactId>cvss-calculator</artifactId>
</dependency>

<dependency>
<groupId>org.apache.httpcomponents.client5</groupId>
<artifactId>httpclient5</artifactId>
</dependency>

<dependency>
<groupId>com.github.package-url</groupId>
<artifactId>packageurl-java</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.google.protobuf.util.Timestamps;
import io.github.jeremylong.openvulnerability.client.ghsa.CWEs;
import io.github.jeremylong.openvulnerability.client.ghsa.Identifier;
import io.github.jeremylong.openvulnerability.client.ghsa.Package;
import io.github.jeremylong.openvulnerability.client.ghsa.SecurityAdvisory;
import org.apache.commons.collections4.CollectionUtils;
import org.apache.commons.lang3.StringUtils;
Expand Down Expand Up @@ -58,9 +59,9 @@
import java.util.HashMap;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.UUID;

import static com.github.packageurl.PackageURLBuilder.aPackageURL;
import static io.github.nscuro.versatile.VersUtils.versFromGhsaRange;

public class GitHubAdvisoryToCdxParser {
Expand Down Expand Up @@ -106,7 +107,7 @@ public static Bom parse(final SecurityAdvisory advisory, boolean aliasSyncEnable
CollectionUtils.isNotEmpty(advisory.getVulnerabilities().getEdges())) {

for (final io.github.jeremylong.openvulnerability.client.ghsa.Vulnerability gitHubVulnerability : advisory.getVulnerabilities().getEdges()) {
PackageURL purl = generatePurlFromGitHubVulnerability(gitHubVulnerability);
PackageURL purl = convertToPurl(gitHubVulnerability.getPackage());
if (purl == null) {
//drop mapping if purl is null
break;
Expand Down Expand Up @@ -152,8 +153,10 @@ public static Bom parse(final SecurityAdvisory advisory, boolean aliasSyncEnable
}

private static Optional<VulnerabilityRating> parseRating(final SecurityAdvisory advisory) {
if (advisory.getCvss() != null && StringUtils.trimToNull(advisory.getCvss().getVectorString()) != null) {
final String cvssVector = StringUtils.trimToNull(advisory.getCvss().getVectorString());
if (advisory.getCvssSeverities() != null
&& advisory.getCvssSeverities().getCvssV3() != null
&& StringUtils.trimToNull(advisory.getCvssSeverities().getCvssV3().getVectorString()) != null) {
final String cvssVector = StringUtils.trimToNull(advisory.getCvssSeverities().getCvssV3().getVectorString());
final Cvss cvss;
try {
cvss = Cvss.fromVector(cvssVector);
Expand Down Expand Up @@ -262,27 +265,54 @@ private static List<Integer> parseCwes(CWEs weaknesses) {
return cwes;
}

private static PackageURL generatePurlFromGitHubVulnerability(final io.github.jeremylong.openvulnerability.client.ghsa.Vulnerability vuln) {
final String purlType = ParserUtil.mapGitHubEcosystemToPurlType(vuln.getPackage().getEcosystem());
try {
if (purlType != null) {
if (PackageURL.StandardTypes.NPM.equals(purlType) && vuln.getPackage().getName().contains("/")) {
final String[] parts = vuln.getPackage().getName().split("/");
return PackageURLBuilder.aPackageURL().withType(purlType).withNamespace(parts[0]).withName(parts[1]).build();
} else if (PackageURL.StandardTypes.MAVEN.equals(purlType) && vuln.getPackage().getName().contains(":")) {
final String[] parts = vuln.getPackage().getName().split(":");
return PackageURLBuilder.aPackageURL().withType(purlType).withNamespace(parts[0]).withName(parts[1]).build();
} else if (Set.of(PackageURL.StandardTypes.COMPOSER, PackageURL.StandardTypes.GOLANG).contains(purlType) && vuln.getPackage().getName().contains("/")) {
final String[] parts = vuln.getPackage().getName().split("/");
final String namespace = String.join("/", Arrays.copyOfRange(parts, 0, parts.length - 1));
return PackageURLBuilder.aPackageURL().withType(purlType).withNamespace(namespace).withName(parts[parts.length - 1]).build();
} else {
return PackageURLBuilder.aPackageURL().withType(purlType).withName(vuln.getPackage().getName()).build();
}
private static PackageURL convertToPurl(final Package pkg) {

Check warning on line 268 in mirror-service/src/main/java/org/dependencytrack/vulnmirror/datasource/github/GitHubAdvisoryToCdxParser.java

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

mirror-service/src/main/java/org/dependencytrack/vulnmirror/datasource/github/GitHubAdvisoryToCdxParser.java#L268

The method 'convertToPurl(Package)' has an NPath complexity of 208, current threshold is 200
final String purlType = switch (pkg.getEcosystem().toLowerCase()) {
case "composer" -> PackageURL.StandardTypes.COMPOSER;
case "erlang" -> PackageURL.StandardTypes.HEX;
case "go" -> PackageURL.StandardTypes.GOLANG;
case "maven" -> PackageURL.StandardTypes.MAVEN;
case "npm" -> PackageURL.StandardTypes.NPM;
case "nuget" -> PackageURL.StandardTypes.NUGET;
case "other" -> PackageURL.StandardTypes.GENERIC;
case "pip" -> PackageURL.StandardTypes.PYPI;
case "pub" -> "pub"; // https://github.com/package-url/purl-spec/blob/master/PURL-TYPES.rst#pub
case "rubygems" -> PackageURL.StandardTypes.GEM;
case "rust" -> PackageURL.StandardTypes.CARGO;
case "swift" -> "swift"; // https://github.com/package-url/purl-spec/blob/master/PURL-TYPES.rst#swift
default -> {
// Not optimal, but still better than ignoring the package entirely.
LOGGER.warn("Unrecognized ecosystem %s; Assuming PURL type %s for %s".formatted(
pkg.getEcosystem(), PackageURL.StandardTypes.GENERIC, pkg));
yield PackageURL.StandardTypes.GENERIC;
}
};

final PackageURLBuilder purlBuilder = aPackageURL().withType(purlType);
if (PackageURL.StandardTypes.MAVEN.equals(purlType) && pkg.getName().contains(":")) {
final String[] nameParts = pkg.getName().split(":", 2);
purlBuilder
.withNamespace(nameParts[0])
.withName(nameParts[1]);
} else if ((PackageURL.StandardTypes.COMPOSER.equals(purlType)
|| PackageURL.StandardTypes.GOLANG.equals(purlType)
|| PackageURL.StandardTypes.NPM.equals(purlType)
|| PackageURL.StandardTypes.GENERIC.equals(purlType))
&& pkg.getName().contains("/")) {
final String[] nameParts = pkg.getName().split("/");
final String namespace = String.join("/", Arrays.copyOfRange(nameParts, 0, nameParts.length - 1));
purlBuilder
.withNamespace(namespace)
.withName(nameParts[nameParts.length - 1]);
} else {
purlBuilder.withName(pkg.getName());
}

try {
return purlBuilder.build();
} catch (MalformedPackageURLException e) {
LOGGER.warn("Unable to create purl from GitHub Vulnerability. Skipping " + vuln.getPackage().getEcosystem() + " : " + vuln.getPackage().getName());
LOGGER.warn("Failed to assemble a valid PURL from {}", pkg, e);
return null;
}
return null;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@

import io.github.jeremylong.openvulnerability.client.ghsa.GitHubSecurityAdvisoryClient;
import io.github.jeremylong.openvulnerability.client.ghsa.GitHubSecurityAdvisoryClientBuilder;
import jakarta.enterprise.context.ApplicationScoped;
import org.apache.hc.client5.http.impl.async.HttpAsyncClientBuilder;
import org.apache.hc.client5.http.impl.async.HttpAsyncClients;
import org.dependencytrack.common.SecretDecryptor;

import jakarta.enterprise.context.ApplicationScoped;
import java.time.Instant;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
Expand All @@ -41,17 +43,23 @@ class GitHubApiClientFactory {
}

GitHubSecurityAdvisoryClient create(final long lastUpdatedEpochSeconds) {
final GitHubSecurityAdvisoryClientBuilder builder = aGitHubSecurityAdvisoryClient();
final HttpAsyncClientBuilder httpClientBuilder = HttpAsyncClients.custom()
.setRetryStrategy(new GitHubHttpRequestRetryStrategy())
.useSystemProperties();

final GitHubSecurityAdvisoryClientBuilder builder = aGitHubSecurityAdvisoryClient()
.withHttpClientSupplier(httpClientBuilder::build);

config.baseUrl().ifPresent(builder::withEndpoint);
config.apiKey()
.map(encryptedApiKey -> {
try {
return secretDecryptor.decryptAsString(encryptedApiKey);
} catch (Exception e) {
throw new IllegalStateException("Failed to decrypt API key", e);
}
})
// TODO: https://github.com/DependencyTrack/dependency-track/issues/3332
// .map(encryptedApiKey -> {
// try {
// return secretDecryptor.decryptAsString(encryptedApiKey);
// } catch (Exception e) {
// throw new IllegalStateException("Failed to decrypt API key", e);
// }
// })
.ifPresent(builder::withApiKey);

if (lastUpdatedEpochSeconds > 0) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
/*
* This file is part of Dependency-Track.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
* Copyright (c) OWASP Foundation. All Rights Reserved.
*/
package org.dependencytrack.vulnmirror.datasource.github;

import org.apache.hc.client5.http.impl.DefaultHttpRequestRetryStrategy;
import org.apache.hc.core5.http.ConnectionClosedException;
import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.HttpResponse;
import org.apache.hc.core5.http.protocol.HttpContext;
import org.apache.hc.core5.util.TimeValue;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.net.ssl.SSLException;
import java.io.InterruptedIOException;
import java.net.ConnectException;
import java.net.NoRouteToHostException;
import java.net.UnknownHostException;
import java.time.Duration;
import java.time.Instant;
import java.util.List;

final class GitHubHttpRequestRetryStrategy extends DefaultHttpRequestRetryStrategy {

private enum RateLimitStrategy {
RETRY_AFTER,
LIMIT_RESET
}

private record RateLimitInfo(
RateLimitStrategy strategy,
Duration retryAfter,
Long remainingRequests,
Long requestLimit,
Instant requestLimitResetAt) {

private static RateLimitInfo of(final HttpResponse response) {
final Header retryAfterHeader = response.getFirstHeader("retry-after");
if (retryAfterHeader != null) {
final long retryAfterSeconds = Long.parseLong(retryAfterHeader.getValue().trim());
return new RateLimitInfo(RateLimitStrategy.RETRY_AFTER, Duration.ofSeconds(retryAfterSeconds), null, null, null);
}

final Header remainingRequestsHeader = response.getFirstHeader("x-ratelimit-remaining");
if (remainingRequestsHeader != null) {
final long remainingRequests = Long.parseLong(remainingRequestsHeader.getValue().trim());
final long requestLimit = Long.parseLong(response.getFirstHeader("x-ratelimit-limit").getValue().trim());
final long requestLimitResetEpochSeconds = Long.parseLong(response.getFirstHeader("x-ratelimit-reset").getValue().trim());
return new RateLimitInfo(RateLimitStrategy.LIMIT_RESET, null, remainingRequests, requestLimit, Instant.ofEpochSecond(requestLimitResetEpochSeconds));
}

return null;
}

}

private static final Logger LOGGER = LoggerFactory.getLogger(GitHubHttpRequestRetryStrategy.class);

private final Duration maxRetryDelay = Duration.ofMinutes(3);

GitHubHttpRequestRetryStrategy() {
super(
/* maxRetries */ 6,
/* defaultRetryInterval */ TimeValue.ofSeconds(1L),
// Same as DefaultHttpRequestRetryStrategy.
/* retryableExceptions */ List.of(
ConnectException.class,
ConnectionClosedException.class,
InterruptedIOException.class,
NoRouteToHostException.class,
SSLException.class,
UnknownHostException.class),
// Same as DefaultHttpRequestRetryStrategy, with addition of 403,
// since GitHub might use that status to indicate rate limiting.
/* retryableCodes */ List.of(403, 429, 503));
}

@Override
public boolean retryRequest(final HttpResponse response, final int execCount, final HttpContext context) {
if (response.getCode() != 403 && response.getCode() != 429) {
return super.retryRequest(response, execCount, context);
}

final var rateLimitInfo = RateLimitInfo.of(response);
if (rateLimitInfo == null) {
if (response.getCode() == 403) {
// Authorization failure. Do not retry.
return false;
}

return super.retryRequest(response, execCount, context);
}

return switch (rateLimitInfo.strategy()) {
case RETRY_AFTER -> {
// Usually GitHub will request to wait for 1min. This may change though, and we can't risk
// blocking a worker thread unnecessarily for a long period of time.
if (rateLimitInfo.retryAfter().compareTo(maxRetryDelay) > 0) {
LOGGER.warn("""
Rate limiting detected; GitHub API indicates retries to be acceptable after {}, \
which exceeds the maximum retry duration of {}. \
Not performing any further retries.""",
rateLimitInfo.retryAfter(), maxRetryDelay);
yield false;
}

yield true;
}
case LIMIT_RESET -> {
if (rateLimitInfo.remainingRequests() > 0) {
// Still have requests budget remaining. Failure reason is not rate limiting.
yield super.retryRequest(response, execCount, context);
}

// The duration after which the limit is reset is not defined in GitHub's API docs.
// Need to safeguard ourselves from blocking the worker thread for too long.
final var untilResetDuration = Duration.between(Instant.now(), rateLimitInfo.requestLimitResetAt());
if (untilResetDuration.compareTo(maxRetryDelay) > 0) {
LOGGER.warn("""
Primary rate limit of {} requests exhausted. The rate limit will reset at {} (in {}), \
which exceeds the maximum retry duration of {}. Not performing any further retries.""",
rateLimitInfo.requestLimit(), rateLimitInfo.requestLimitResetAt(), untilResetDuration, maxRetryDelay);
yield false;
}

yield true;
}
};
}

@Override
public TimeValue getRetryInterval(final HttpResponse response, final int execCount, final HttpContext context) {
// When this is called, retryRequest was already invoked to determine whether
// a retry should be performed. So we can skip the status code check here.

final var rateLimitInfo = RateLimitInfo.of(response);
if (rateLimitInfo == null) {
return super.getRetryInterval(response, execCount, context);
}

return switch (rateLimitInfo.strategy()) {
case RETRY_AFTER -> {
LOGGER.warn("""
Rate limiting detected; GitHub indicates retries to be acceptable after {}; \
Will wait and try again.""", rateLimitInfo.retryAfter());
yield TimeValue.ofMilliseconds(rateLimitInfo.retryAfter().toMillis());
}
case LIMIT_RESET -> {
final var retryAfter = Duration.between(Instant.now(), rateLimitInfo.requestLimitResetAt());
LOGGER.warn("""
Primary rate limit of {} requests exhausted. Limit will reset at {}; \
Will wait for {} and try again.""",
rateLimitInfo.requestLimit(), rateLimitInfo.requestLimitResetAt(), retryAfter);
yield TimeValue.ofMilliseconds(retryAfter.toMillis());
}
};
}

}
Loading

0 comments on commit 28611a6

Please sign in to comment.