diff --git a/dd-java-agent/agent-ci-visibility/build.gradle b/dd-java-agent/agent-ci-visibility/build.gradle index 1f3c4866ff3..36da0b46a90 100644 --- a/dd-java-agent/agent-ci-visibility/build.gradle +++ b/dd-java-agent/agent-ci-visibility/build.gradle @@ -38,6 +38,7 @@ excludedClassesCoverage += [ "datadog.trace.civisibility.config.CachingModuleExecutionSettingsFactory", "datadog.trace.civisibility.config.CachingModuleExecutionSettingsFactory.Key", "datadog.trace.civisibility.config.CiVisibilitySettings", + "datadog.trace.civisibility.config.ConfigurationApiImpl", "datadog.trace.civisibility.config.ConfigurationApiImpl.MultiEnvelopeDto", "datadog.trace.civisibility.config.ConfigurationApi.1", "datadog.trace.civisibility.config.ModuleExecutionSettingsFactoryImpl", diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityRepoServices.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityRepoServices.java index 4daa6e638cd..f767726c8c4 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityRepoServices.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityRepoServices.java @@ -2,6 +2,7 @@ import datadog.trace.api.Config; import datadog.trace.api.civisibility.config.ModuleExecutionSettings; +import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector; import datadog.trace.api.git.GitInfoProvider; import datadog.trace.civisibility.ci.CIInfo; import datadog.trace.civisibility.ci.CIProviderInfo; @@ -61,6 +62,7 @@ public class CiVisibilityRepoServices { gitDataUploader = buildGitDataUploader( services.config, + services.metricCollector, services.gitInfoProvider, services.gitClientFactory, services.backendApi, @@ -75,7 +77,12 @@ public class CiVisibilityRepoServices { } else { moduleExecutionSettingsFactory = buildModuleExecutionSettingsFactory( - services.config, services.backendApi, gitDataUploader, repoIndexProvider, repoRoot); + services.config, + services.metricCollector, + services.backendApi, + gitDataUploader, + repoIndexProvider, + repoRoot); } } @@ -112,6 +119,7 @@ private static ModuleExecutionSettingsFactory buildModuleExecutionSettingsFetche private static ModuleExecutionSettingsFactory buildModuleExecutionSettingsFactory( Config config, + CiVisibilityMetricCollector metricCollector, BackendApi backendApi, GitDataUploader gitDataUploader, RepoIndexProvider repoIndexProvider, @@ -122,7 +130,7 @@ private static ModuleExecutionSettingsFactory buildModuleExecutionSettingsFactor "Remote config and skippable tests requests will be skipped since backend API client could not be created"); configurationApi = ConfigurationApi.NO_OP; } else { - configurationApi = new ConfigurationApiImpl(backendApi); + configurationApi = new ConfigurationApiImpl(backendApi, metricCollector); } return new CachingModuleExecutionSettingsFactory( config, @@ -132,6 +140,7 @@ private static ModuleExecutionSettingsFactory buildModuleExecutionSettingsFactor private static GitDataUploader buildGitDataUploader( Config config, + CiVisibilityMetricCollector metricCollector, GitInfoProvider gitInfoProvider, GitClient.Factory gitClientFactory, BackendApi backendApi, @@ -153,10 +162,10 @@ private static GitDataUploader buildGitDataUploader( } String remoteName = config.getCiVisibilityGitRemoteName(); - GitDataApi gitDataApi = new GitDataApi(backendApi); + GitDataApi gitDataApi = new GitDataApi(backendApi, metricCollector); GitClient gitClient = gitClientFactory.create(repoRoot); return new GitDataUploaderImpl( - config, gitDataApi, gitClient, gitInfoProvider, repoRoot, remoteName); + config, metricCollector, gitDataApi, gitClient, gitInfoProvider, repoRoot, remoteName); } private static SourcePathResolver buildSourcePathResolver( diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/communication/BackendApi.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/communication/BackendApi.java index 02f4712950c..52aa8f70641 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/communication/BackendApi.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/communication/BackendApi.java @@ -1,13 +1,19 @@ package datadog.trace.civisibility.communication; +import datadog.communication.http.OkHttpUtils; import datadog.trace.civisibility.utils.IOThrowingFunction; import java.io.IOException; import java.io.InputStream; +import javax.annotation.Nullable; import okhttp3.RequestBody; /** API for posting HTTP requests to backend */ public interface BackendApi { - T post(String uri, RequestBody requestBody, IOThrowingFunction responseParser) + T post( + String uri, + RequestBody requestBody, + IOThrowingFunction responseParser, + @Nullable OkHttpUtils.CustomListener requestListener) throws IOException; } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/communication/EvpProxyApi.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/communication/EvpProxyApi.java index 50b8a3bf811..eea481345d6 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/communication/EvpProxyApi.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/communication/EvpProxyApi.java @@ -6,6 +6,7 @@ import java.io.IOException; import java.io.InputStream; import java.util.zip.GZIPInputStream; +import javax.annotation.Nullable; import okhttp3.HttpUrl; import okhttp3.OkHttpClient; import okhttp3.Request; @@ -56,7 +57,10 @@ public EvpProxyApi( @Override public T post( - String uri, RequestBody requestBody, IOThrowingFunction responseParser) + String uri, + RequestBody requestBody, + IOThrowingFunction responseParser, + @Nullable OkHttpUtils.CustomListener requestListener) throws IOException { final HttpUrl url = evpProxyUrl.resolve(uri); @@ -67,6 +71,10 @@ public T post( .addHeader(X_DATADOG_TRACE_ID_HEADER, traceId) .addHeader(X_DATADOG_PARENT_ID_HEADER, traceId); + if (requestListener != null) { + requestBuilder.tag(OkHttpUtils.CustomListener.class, requestListener); + } + if (gzipEnabled) { requestBuilder.addHeader(ACCEPT_ENCODING_HEADER, GZIP_ENCODING); } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/communication/IntakeApi.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/communication/IntakeApi.java index bb0769990f1..a92f1bc5ddb 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/communication/IntakeApi.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/communication/IntakeApi.java @@ -6,6 +6,7 @@ import java.io.IOException; import java.io.InputStream; import java.util.zip.GZIPInputStream; +import javax.annotation.Nullable; import okhttp3.HttpUrl; import okhttp3.OkHttpClient; import okhttp3.Request; @@ -61,7 +62,10 @@ public IntakeApi( @Override public T post( - String uri, RequestBody requestBody, IOThrowingFunction responseParser) + String uri, + RequestBody requestBody, + IOThrowingFunction responseParser, + @Nullable OkHttpUtils.CustomListener requestListener) throws IOException { HttpUrl url = hostUrl.resolve(uri); Request.Builder requestBuilder = @@ -72,6 +76,10 @@ public T post( .addHeader(X_DATADOG_TRACE_ID_HEADER, traceId) .addHeader(X_DATADOG_PARENT_ID_HEADER, traceId); + if (requestListener != null) { + requestBuilder.tag(OkHttpUtils.CustomListener.class, requestListener); + } + if (gzipEnabled) { requestBuilder.addHeader(ACCEPT_ENCODING_HEADER, GZIP_ENCODING); } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/communication/TelemetryListener.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/communication/TelemetryListener.java new file mode 100644 index 00000000000..4fb25df98a3 --- /dev/null +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/communication/TelemetryListener.java @@ -0,0 +1,132 @@ +package datadog.trace.civisibility.communication; + +import datadog.communication.http.OkHttpUtils; +import datadog.trace.api.civisibility.telemetry.CiVisibilityCountMetric; +import datadog.trace.api.civisibility.telemetry.CiVisibilityDistributionMetric; +import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector; +import datadog.trace.api.civisibility.telemetry.tag.ErrorType; +import java.io.IOException; +import javax.annotation.Nullable; +import okhttp3.Call; +import okhttp3.Response; + +public class TelemetryListener extends OkHttpUtils.CustomListener { + + private final CiVisibilityMetricCollector metricCollector; + private final @Nullable CiVisibilityCountMetric requestCountMetric; + private final @Nullable CiVisibilityCountMetric requestErrorsMetric; + private final @Nullable CiVisibilityDistributionMetric requestBytesMetric; + private final @Nullable CiVisibilityDistributionMetric requestDurationMetric; + private final @Nullable CiVisibilityDistributionMetric responseBytesMetric; + private long callStartTimestamp; + + private TelemetryListener( + CiVisibilityMetricCollector metricCollector, + @Nullable CiVisibilityCountMetric requestCountMetric, + @Nullable CiVisibilityCountMetric requestErrorsMetric, + @Nullable CiVisibilityDistributionMetric requestBytesMetric, + @Nullable CiVisibilityDistributionMetric requestDurationMetric, + @Nullable CiVisibilityDistributionMetric responseBytesMetric) { + this.metricCollector = metricCollector; + this.requestCountMetric = requestCountMetric; + this.requestErrorsMetric = requestErrorsMetric; + this.requestBytesMetric = requestBytesMetric; + this.requestDurationMetric = requestDurationMetric; + this.responseBytesMetric = responseBytesMetric; + } + + public void callStart(Call call) { + callStartTimestamp = System.currentTimeMillis(); + if (requestCountMetric != null) { + metricCollector.add(requestCountMetric, 1); + } + } + + public void requestBodyEnd(Call call, long byteCount) { + if (requestBytesMetric != null) { + metricCollector.add(requestBytesMetric, (int) byteCount); + } + } + + public void responseHeadersEnd(Call call, Response response) { + if (requestErrorsMetric != null) { + if (!response.isSuccessful()) { + int responseCode = response.code(); + metricCollector.add(requestErrorsMetric, 1, ErrorType.from(responseCode)); + } + } + } + + @Override + public void responseBodyEnd(Call call, long byteCount) { + if (responseBytesMetric != null) { + metricCollector.add(responseBytesMetric, (int) byteCount); + } + } + + public void callEnd(Call call) { + if (requestDurationMetric != null) { + int durationMillis = (int) (System.currentTimeMillis() - callStartTimestamp); + metricCollector.add(requestDurationMetric, durationMillis); + } + } + + public void callFailed(Call call, IOException ioe) { + if (requestDurationMetric != null) { + int durationMillis = (int) (System.currentTimeMillis() - callStartTimestamp); + metricCollector.add(requestDurationMetric, durationMillis); + } + + if (requestErrorsMetric != null) { + metricCollector.add(requestErrorsMetric, 1, ErrorType.NETWORK); + } + } + + public static final class Builder { + private final CiVisibilityMetricCollector metricCollector; + private @Nullable CiVisibilityCountMetric requestCountMetric; + private @Nullable CiVisibilityCountMetric requestErrorsMetric; + private @Nullable CiVisibilityDistributionMetric requestBytesMetric; + private @Nullable CiVisibilityDistributionMetric requestDurationMetric; + private @Nullable CiVisibilityDistributionMetric responseBytesMetric; + + public Builder(CiVisibilityMetricCollector metricCollector) { + this.metricCollector = metricCollector; + } + + public Builder requestCount(@Nullable CiVisibilityCountMetric requestCountMetric) { + this.requestCountMetric = requestCountMetric; + return this; + } + + public Builder requestErrors(@Nullable CiVisibilityCountMetric requestErrorsMetric) { + this.requestErrorsMetric = requestErrorsMetric; + return this; + } + + public Builder requestBytes(@Nullable CiVisibilityDistributionMetric requestBytesMetric) { + this.requestBytesMetric = requestBytesMetric; + return this; + } + + public Builder requestDuration(@Nullable CiVisibilityDistributionMetric requestDurationMetric) { + this.requestDurationMetric = requestDurationMetric; + return this; + } + + public Builder responseBytes(@Nullable CiVisibilityDistributionMetric responseBytesMetric) { + this.responseBytesMetric = responseBytesMetric; + return this; + } + + public OkHttpUtils.CustomListener build() { + return new TelemetryListener( + metricCollector, + requestCountMetric, + requestErrorsMetric, + requestBytesMetric, + requestDurationMetric, + responseBytesMetric); + } + } +} diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/CiVisibilitySettings.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/CiVisibilitySettings.java index 523cab55226..b98ec9e9103 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/CiVisibilitySettings.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/CiVisibilitySettings.java @@ -5,24 +5,31 @@ public class CiVisibilitySettings { public static final CiVisibilitySettings DEFAULT = - new CiVisibilitySettings(false, false, false, false); + new CiVisibilitySettings(false, false, false, false, false); + private final boolean itr_enabled; private final boolean code_coverage; private final boolean tests_skipping; private final boolean require_git; private final boolean flaky_test_retries_enabled; public CiVisibilitySettings( + boolean itr_enabled, boolean code_coverage, boolean tests_skipping, boolean require_git, boolean flaky_test_retries_enabled) { + this.itr_enabled = itr_enabled; this.code_coverage = code_coverage; this.tests_skipping = tests_skipping; this.require_git = require_git; this.flaky_test_retries_enabled = flaky_test_retries_enabled; } + public boolean isItrEnabled() { + return itr_enabled; + } + public boolean isCodeCoverageEnabled() { return code_coverage; } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/ConfigurationApiImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/ConfigurationApiImpl.java index 5870d67be66..6e71c33135f 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/ConfigurationApiImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/ConfigurationApiImpl.java @@ -3,8 +3,17 @@ import com.squareup.moshi.JsonAdapter; import com.squareup.moshi.Moshi; import com.squareup.moshi.Types; +import datadog.communication.http.OkHttpUtils; import datadog.trace.api.civisibility.config.TestIdentifier; +import datadog.trace.api.civisibility.telemetry.CiVisibilityCountMetric; +import datadog.trace.api.civisibility.telemetry.CiVisibilityDistributionMetric; +import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector; +import datadog.trace.api.civisibility.telemetry.tag.CoverageEnabled; +import datadog.trace.api.civisibility.telemetry.tag.ItrEnabled; +import datadog.trace.api.civisibility.telemetry.tag.ItrSkipEnabled; +import datadog.trace.api.civisibility.telemetry.tag.RequireGit; import datadog.trace.civisibility.communication.BackendApi; +import datadog.trace.civisibility.communication.TelemetryListener; import java.io.IOException; import java.lang.reflect.ParameterizedType; import java.util.Collection; @@ -24,18 +33,23 @@ public class ConfigurationApiImpl implements ConfigurationApi { private static final String FLAKY_TESTS_URI = "ci/libraries/tests/flaky"; private final BackendApi backendApi; + private final CiVisibilityMetricCollector metricCollector; private final Supplier uuidGenerator; private final JsonAdapter> requestAdapter; private final JsonAdapter> settingsResponseAdapter; private final JsonAdapter> testIdentifiersResponseAdapter; - public ConfigurationApiImpl(BackendApi backendApi) { - this(backendApi, () -> UUID.randomUUID().toString()); + public ConfigurationApiImpl(BackendApi backendApi, CiVisibilityMetricCollector metricCollector) { + this(backendApi, metricCollector, () -> UUID.randomUUID().toString()); } - ConfigurationApiImpl(BackendApi backendApi, Supplier uuidGenerator) { + ConfigurationApiImpl( + BackendApi backendApi, + CiVisibilityMetricCollector metricCollector, + Supplier uuidGenerator) { this.backendApi = backendApi; + this.metricCollector = metricCollector; this.uuidGenerator = uuidGenerator; Moshi moshi = @@ -65,26 +79,65 @@ public CiVisibilitySettings getSettings(TracerEnvironment tracerEnvironment) thr new DataDto<>(uuid, "ci_app_test_service_libraries_settings", tracerEnvironment)); String json = requestAdapter.toJson(settingsRequest); RequestBody requestBody = RequestBody.create(JSON, json); - return backendApi.post( - SETTINGS_URI, - requestBody, - is -> settingsResponseAdapter.fromJson(Okio.buffer(Okio.source(is))).data.attributes); + + OkHttpUtils.CustomListener telemetryListener = + new TelemetryListener.Builder(metricCollector) + .requestCount(CiVisibilityCountMetric.GIT_REQUESTS_SETTINGS) + .requestErrors(CiVisibilityCountMetric.GIT_REQUESTS_SETTINGS_ERRORS) + .requestDuration(CiVisibilityDistributionMetric.GIT_REQUESTS_SETTINGS_MS) + .build(); + + CiVisibilitySettings settings = + backendApi.post( + SETTINGS_URI, + requestBody, + is -> settingsResponseAdapter.fromJson(Okio.buffer(Okio.source(is))).data.attributes, + telemetryListener); + + metricCollector.add( + CiVisibilityCountMetric.GIT_REQUESTS_SETTINGS_RESPONSE, + 1, + settings.isItrEnabled() ? ItrEnabled.TRUE : null, + settings.isTestsSkippingEnabled() ? ItrSkipEnabled.TRUE : null, + settings.isCodeCoverageEnabled() ? CoverageEnabled.TRUE : null, + settings.isGitUploadRequired() ? RequireGit.TRUE : null); + + return settings; } @Override public Collection getSkippableTests(TracerEnvironment tracerEnvironment) throws IOException { - return getTestsData(SKIPPABLE_TESTS_URI, "test_params", tracerEnvironment); + OkHttpUtils.CustomListener telemetryListener = + new TelemetryListener.Builder(metricCollector) + .requestCount(CiVisibilityCountMetric.ITR_SKIPPABLE_TESTS_REQUEST) + .requestErrors(CiVisibilityCountMetric.ITR_SKIPPABLE_TESTS_REQUEST_ERRORS) + .requestDuration(CiVisibilityDistributionMetric.ITR_SKIPPABLE_TESTS_REQUEST_MS) + .responseBytes(CiVisibilityDistributionMetric.ITR_SKIPPABLE_TESTS_RESPONSE_BYTES) + .build(); + + Collection skippableTests = + getTestsData(SKIPPABLE_TESTS_URI, "test_params", tracerEnvironment, telemetryListener); + + metricCollector.add( + CiVisibilityCountMetric.ITR_SKIPPABLE_TESTS_RESPONSE_TESTS, skippableTests.size()); + + return skippableTests; } @Override public Collection getFlakyTests(TracerEnvironment tracerEnvironment) throws IOException { - return getTestsData(FLAKY_TESTS_URI, "flaky_test_from_libraries_params", tracerEnvironment); + return getTestsData( + FLAKY_TESTS_URI, "flaky_test_from_libraries_params", tracerEnvironment, null); } private Collection getTestsData( - String endpoint, String dataType, TracerEnvironment tracerEnvironment) throws IOException { + String endpoint, + String dataType, + TracerEnvironment tracerEnvironment, + OkHttpUtils.CustomListener requestListener) + throws IOException { String uuid = uuidGenerator.get(); EnvelopeDto request = new EnvelopeDto<>(new DataDto<>(uuid, dataType, tracerEnvironment)); @@ -94,7 +147,8 @@ private Collection getTestsData( backendApi.post( endpoint, requestBody, - is -> testIdentifiersResponseAdapter.fromJson(Okio.buffer(Okio.source(is))).data); + is -> testIdentifiersResponseAdapter.fromJson(Okio.buffer(Okio.source(is))).data, + requestListener); return response.stream().map(DataDto::getAttributes).collect(Collectors.toList()); } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitDataApi.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitDataApi.java index 9b6fad20684..11448c6f3d5 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitDataApi.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitDataApi.java @@ -3,7 +3,12 @@ import com.squareup.moshi.Json; import com.squareup.moshi.JsonAdapter; import com.squareup.moshi.Moshi; +import datadog.communication.http.OkHttpUtils; +import datadog.trace.api.civisibility.telemetry.CiVisibilityCountMetric; +import datadog.trace.api.civisibility.telemetry.CiVisibilityDistributionMetric; +import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector; import datadog.trace.civisibility.communication.BackendApi; +import datadog.trace.civisibility.communication.TelemetryListener; import datadog.trace.civisibility.utils.IOUtils; import java.io.IOException; import java.nio.file.Files; @@ -29,12 +34,14 @@ public class GitDataApi { private static final String UPLOAD_PACKFILES_URI = "git/repository/packfile"; private final BackendApi backendApi; + private final CiVisibilityMetricCollector metricCollector; private final JsonAdapter searchCommitsRequestAdapter; private final JsonAdapter searchCommitsResponseAdapter; private final JsonAdapter pushedShaAdapter; - public GitDataApi(BackendApi backendApi) { + public GitDataApi(BackendApi backendApi, CiVisibilityMetricCollector metricCollector) { this.backendApi = backendApi; + this.metricCollector = metricCollector; Moshi moshi = new Moshi.Builder().build(); searchCommitsRequestAdapter = moshi.adapter(SearchCommitsRequest.class); @@ -58,11 +65,20 @@ public Collection searchCommits(String gitRemoteUrl, List commit String json = searchCommitsRequestAdapter.toJson(searchCommitsRequest); RequestBody requestBody = RequestBody.create(JSON, json); + + OkHttpUtils.CustomListener telemetryListener = + new TelemetryListener.Builder(metricCollector) + .requestCount(CiVisibilityCountMetric.GIT_REQUESTS_SEARCH_COMMITS) + .requestErrors(CiVisibilityCountMetric.GIT_REQUESTS_SEARCH_COMMITS_ERRORS) + .requestDuration(CiVisibilityDistributionMetric.GIT_REQUESTS_SEARCH_COMMITS_MS) + .build(); + SearchCommitsResponse response = backendApi.post( SEARCH_COMMITS_URI, requestBody, - is -> searchCommitsResponseAdapter.fromJson(Okio.buffer(Okio.source(is)))); + is -> searchCommitsResponseAdapter.fromJson(Okio.buffer(Okio.source(is))), + telemetryListener); return response.data.stream().map(Commit::getId).collect(Collectors.toSet()); } @@ -94,7 +110,16 @@ public void uploadPackFile(String gitRemoteUrl, String currentCommitHash, Path p .addFormDataPart("packfile", packFileNameWithoutRandomPrefix, packFileBody) .build(); - String response = backendApi.post(UPLOAD_PACKFILES_URI, requestBody, IOUtils::readFully); + OkHttpUtils.CustomListener telemetryListener = + new TelemetryListener.Builder(metricCollector) + .requestCount(CiVisibilityCountMetric.GIT_REQUESTS_OBJECTS_PACK) + .requestErrors(CiVisibilityCountMetric.GIT_REQUESTS_OBJECTS_PACK_ERRORS) + .requestDuration(CiVisibilityDistributionMetric.GIT_REQUESTS_OBJECTS_PACK_MS) + .requestBytes(CiVisibilityDistributionMetric.GIT_REQUESTS_OBJECTS_PACK_BYTES) + .build(); + + String response = + backendApi.post(UPLOAD_PACKFILES_URI, requestBody, IOUtils::readFully, telemetryListener); LOGGER.debug("Uploading pack file {} returned response {}", packFile, response); } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitDataUploaderImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitDataUploaderImpl.java index e8da31861f5..03a7bee890a 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitDataUploaderImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitDataUploaderImpl.java @@ -1,6 +1,8 @@ package datadog.trace.civisibility.git.tree; import datadog.trace.api.Config; +import datadog.trace.api.civisibility.telemetry.CiVisibilityDistributionMetric; +import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector; import datadog.trace.api.git.GitInfo; import datadog.trace.api.git.GitInfoProvider; import datadog.trace.civisibility.utils.FileUtils; @@ -16,7 +18,7 @@ import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; -import java.util.stream.Stream; +import java.util.stream.Collectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -25,6 +27,7 @@ public class GitDataUploaderImpl implements GitDataUploader { private static final Logger LOGGER = LoggerFactory.getLogger(GitDataUploaderImpl.class); private final Config config; + private final CiVisibilityMetricCollector metricCollector; private final GitDataApi gitDataApi; private final GitClient gitClient; private final GitInfoProvider gitInfoProvider; @@ -35,12 +38,14 @@ public class GitDataUploaderImpl implements GitDataUploader { public GitDataUploaderImpl( Config config, + CiVisibilityMetricCollector metricCollector, GitDataApi gitDataApi, GitClient gitClient, GitInfoProvider gitInfoProvider, String repoRoot, String remoteName) { this.config = config; + this.metricCollector = metricCollector; this.gitDataApi = gitDataApi; this.gitClient = gitClient; this.gitInfoProvider = gitInfoProvider; @@ -133,17 +138,24 @@ private void uploadGitData() { String currentCommit = latestCommits.get(0); Path packFilesDirectory = gitClient.createPackFiles(objectHashes); - try (Stream packFiles = Files.list(packFilesDirectory)) { - packFiles - .filter(pf -> pf.getFileName().toString().endsWith(".pack")) // skipping ".idx" files - .forEach( - pf -> { - try { - gitDataApi.uploadPackFile(remoteUrl, currentCommit, pf); - } catch (Exception e) { - throw new RuntimeException("Could not upload pack file " + pf, e); - } - }); + try { + List packFiles = + Files.list(packFilesDirectory) + .filter( + pf -> pf.getFileName().toString().endsWith(".pack")) // skipping ".idx" files + .collect(Collectors.toList()); + + metricCollector.add( + CiVisibilityDistributionMetric.GIT_REQUESTS_OBJECTS_PACK_FILES, packFiles.size()); + + for (Path packFile : packFiles) { + try { + gitDataApi.uploadPackFile(remoteUrl, currentCommit, packFile); + } catch (Exception e) { + throw new RuntimeException("Could not upload pack file " + packFile, e); + } + } + } finally { FileUtils.delete(packFilesDirectory); } diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/config/ConfigurationApiImplTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/config/ConfigurationApiImplTest.groovy index d7cf3381cfd..1f66eca1066 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/config/ConfigurationApiImplTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/config/ConfigurationApiImplTest.groovy @@ -6,6 +6,7 @@ import datadog.communication.http.OkHttpUtils import datadog.trace.agent.test.server.http.TestHttpServer import datadog.trace.api.civisibility.config.Configurations import datadog.trace.api.civisibility.config.TestIdentifier +import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector import datadog.trace.civisibility.communication.BackendApi import datadog.trace.civisibility.communication.EvpProxyApi import datadog.trace.civisibility.communication.IntakeApi @@ -143,9 +144,10 @@ class ConfigurationApiImplTest extends Specification { def "test settings request: #displayName"() { given: def tracerEnvironment = givenTracerEnvironment() + def metricCollector = Stub(CiVisibilityMetricCollector) when: - def configurationApi = new ConfigurationApiImpl(api, () -> "1234") + def configurationApi = new ConfigurationApiImpl(api, metricCollector, () -> "1234") def settings = configurationApi.getSettings(tracerEnvironment) then: @@ -165,9 +167,10 @@ class ConfigurationApiImplTest extends Specification { def "test skippable tests request: #displayName"() { given: def tracerEnvironment = givenTracerEnvironment() + def metricCollector = Stub(CiVisibilityMetricCollector) when: - def configurationApi = new ConfigurationApiImpl(api, () -> "1234") + def configurationApi = new ConfigurationApiImpl(api, metricCollector, () -> "1234") def skippableTests = configurationApi.getSkippableTests(tracerEnvironment) then: diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitDataApiTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitDataApiTest.groovy index c280fe51f76..9b15c9d7b15 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitDataApiTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitDataApiTest.groovy @@ -4,6 +4,7 @@ import com.squareup.moshi.Moshi import datadog.communication.http.HttpRetryPolicy import datadog.communication.http.OkHttpUtils import datadog.trace.agent.test.server.http.TestHttpServer +import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector import datadog.trace.civisibility.communication.BackendApi import datadog.trace.civisibility.communication.EvpProxyApi import datadog.trace.test.util.MultipartRequestParser @@ -90,10 +91,11 @@ class GitDataApiTest extends Specification { def "test commits search"() { given: + def metricCollector = Stub(CiVisibilityMetricCollector) def evpProxy = givenEvpProxy() when: - def gitDataApi = new GitDataApi(evpProxy) + def gitDataApi = new GitDataApi(evpProxy, metricCollector) def commits = new ArrayList<>(gitDataApi.searchCommits("gitRemoteUrl", ["sha1", "sha2"])) then: @@ -102,11 +104,12 @@ class GitDataApiTest extends Specification { def "test pack file upload"() { given: + def metricCollector = Stub(CiVisibilityMetricCollector) def evpProxy = givenEvpProxy() def packFile = givenPackFile() when: - def gitDataApi = new GitDataApi(evpProxy) + def gitDataApi = new GitDataApi(evpProxy, metricCollector) gitDataApi.uploadPackFile("gitRemoteUrl", "sha1", packFile) then: diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitDataUploaderImplTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitDataUploaderImplTest.groovy index b632296217e..8d18ffab8a1 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitDataUploaderImplTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitDataUploaderImplTest.groovy @@ -40,7 +40,7 @@ class GitDataUploaderImplTest extends Specification { gitInfoProvider.getGitInfo(repoRoot) >> new GitInfo(repoUrl, null, null, null) def gitClient = new GitClient(metricCollector, repoRoot, "25 years ago", 3, TIMEOUT_MILLIS) - def uploader = new GitDataUploaderImpl(config, api, gitClient, gitInfoProvider, repoRoot, "origin") + def uploader = new GitDataUploaderImpl(config, metricCollector, api, gitClient, gitInfoProvider, repoRoot, "origin") when: def future = uploader.startOrObserveGitDataUpload() diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/ApplicationModuleImpl.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/ApplicationModuleImpl.java index 10bbef8d362..e655b1796b1 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/ApplicationModuleImpl.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/ApplicationModuleImpl.java @@ -1,5 +1,9 @@ package com.datadog.iast.sink; +import static com.datadog.iast.util.StringUtils.endsWithIgnoreCase; +import static com.datadog.iast.util.StringUtils.substringTrim; +import static datadog.trace.api.telemetry.LogCollector.SEND_TELEMETRY; + import com.datadog.iast.Dependencies; import com.datadog.iast.model.Evidence; import com.datadog.iast.model.Location; @@ -11,28 +15,37 @@ import java.io.File; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.nio.file.FileVisitOption; +import java.nio.file.FileVisitResult; +import java.nio.file.FileVisitor; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.ArrayList; -import java.util.List; +import java.nio.file.attribute.BasicFileAttributes; +import java.util.Collection; +import java.util.EnumSet; +import java.util.HashSet; +import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; +import java.util.stream.Stream; +import javax.annotation.Nonnull; import javax.annotation.Nullable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class ApplicationModuleImpl extends SinkModuleBase implements ApplicationModule { - private static final String CONTEXT_LOADER_LISTENER_PATTERN = - "org\\.springframework\\.web\\.context\\.ContextLoaderListener"; + private static final Logger LOGGER = LoggerFactory.getLogger(ApplicationModule.class); - private static final String CONTEXT_LOADER_LISTENER_VALUE = - "org.springframework.web.context.ContextLoaderListener"; + /** Bounds for the file visitor depth when trying to locate insecure JSP folders */ + private static final int JSP_MAX_WALK_DEPTH = 32; - private static final String DISPATCHER_SERVLET_PATTERN = - "org\\.springframework\\.web\\.servlet\\.DispatcherServlet"; + private static final String CONTEXT_LOADER_LISTENER = + "org.springframework.web.context.ContextLoaderListener"; - private static final String DISPATCHER_SERVLET_VALUE = + private static final String DISPATCHER_SERVLET = "org.springframework.web.servlet.DispatcherServlet"; private static final String DEFAULT_HTML_ESCAPE = "defaultHtmlEscape"; @@ -57,18 +70,18 @@ public class ApplicationModuleImpl extends SinkModuleBase implements Application public static final String WEB_XML = "web.xml"; - private static final String REGEX = - String.join( - "|", - CONTEXT_LOADER_LISTENER_PATTERN, - DISPATCHER_SERVLET_PATTERN, - DEFAULT_HTML_ESCAPE, - TOMCAT_MANAGER_APPLICATION, - LISTINGS_PATTERN, - SESSION_TIMEOUT_START_TAG, - SECURITY_CONSTRAINT_START_TAG); - - private static final Pattern PATTERN = Pattern.compile(REGEX); + private static final Pattern PATTERN = + Pattern.compile( + Stream.of( + CONTEXT_LOADER_LISTENER, + DISPATCHER_SERVLET, + DEFAULT_HTML_ESCAPE, + TOMCAT_MANAGER_APPLICATION, + LISTINGS_PATTERN, + SESSION_TIMEOUT_START_TAG, + SECURITY_CONSTRAINT_START_TAG) + .map(Pattern::quote) + .collect(Collectors.joining("|"))); private static final int NO_LINE = -1; @@ -78,16 +91,20 @@ public ApplicationModuleImpl(final Dependencies dependencies) { @Override public void onRealPath(final @Nullable String realPath) { - if (realPath == null) { return; } - + final Path root = Paths.get(realPath); + if (!Files.exists(root)) { + return; + } final AgentSpan span = AgentTracer.activeSpan(); + checkInsecureJSPLayout(root, span); + checkWebXmlVulnerabilities(root, span); + } - checkInsecureJSPLayout(realPath, span); - - String webXmlContent = webXmlContent(realPath); + private void checkWebXmlVulnerabilities(@Nonnull Path path, AgentSpan span) { + String webXmlContent = webXmlContent(path); if (webXmlContent == null) { return; } @@ -99,8 +116,8 @@ public void onRealPath(final @Nullable String realPath) { while (matcher.find()) { String match = matcher.group(); switch (match) { - case DISPATCHER_SERVLET_VALUE: - case CONTEXT_LOADER_LISTENER_VALUE: + case DISPATCHER_SERVLET: + case CONTEXT_LOADER_LISTENER: isSpring = true; break; case DEFAULT_HTML_ESCAPE: @@ -124,19 +141,19 @@ public void onRealPath(final @Nullable String realPath) { } if (isSpring) { - checkDefaultHtmlEscapeInvalid(webXmlContent, span, defaultHtmlEscapeIndex); + checkDefaultHtmlEscapeInvalid(webXmlContent, defaultHtmlEscapeIndex, span); } } private void checkDefaultHtmlEscapeInvalid( - String webXmlContent, AgentSpan span, int defaultHtmlEscapeIndex) { + @Nonnull String webXmlContent, int defaultHtmlEscapeIndex, AgentSpan span) { if (defaultHtmlEscapeIndex != -1) { int start = webXmlContent.indexOf(PARAM_VALUE_START_TAG, defaultHtmlEscapeIndex) + PARAM_VALUE_START_TAG.length(); String value = - webXmlContent.substring(start, webXmlContent.indexOf(PARAM_VALUE_END_TAG, start)); - if (!value.trim().toLowerCase().equals("true")) { + substringTrim(webXmlContent, start, webXmlContent.indexOf(PARAM_VALUE_END_TAG, start)); + if (!value.equalsIgnoreCase("true")) { report( span, VulnerabilityType.DEFAULT_HTML_ESCAPE_INVALID, @@ -161,8 +178,8 @@ private void checkDirectoryListingLeak( int valueIndex = webXmlContent.indexOf(PARAM_VALUE_START_TAG, index) + PARAM_VALUE_START_TAG.length(); int valueLast = webXmlContent.indexOf(PARAM_VALUE_END_TAG, valueIndex); - String data = webXmlContent.substring(valueIndex, valueLast); - if (data.trim().toLowerCase().equals("true")) { + String data = substringTrim(webXmlContent, valueIndex, valueLast); + if (data.equalsIgnoreCase("true")) { report( span, VulnerabilityType.DIRECTORY_LISTING_LEAK, @@ -174,11 +191,10 @@ private void checkDirectoryListingLeak( private void checkSessionTimeOut(final String webXmlContent, int index, final AgentSpan span) { try { String innerText = - webXmlContent - .substring( - index + SESSION_TIMEOUT_START_TAG.length(), - webXmlContent.indexOf(SESSION_TIMEOUT_END_TAG, index)) - .trim(); + substringTrim( + webXmlContent, + index + SESSION_TIMEOUT_START_TAG.length(), + webXmlContent.indexOf(SESSION_TIMEOUT_END_TAG, index)); int timeoutValue = Integer.parseInt(innerText); if (timeoutValue > 30 || timeoutValue == -1) { report( @@ -194,11 +210,10 @@ private void checkSessionTimeOut(final String webXmlContent, int index, final Ag private void checkVerbTampering(final String webXmlContent, int index, final AgentSpan span) { String innerText = - webXmlContent - .substring( - index + SECURITY_CONSTRAINT_START_TAG.length(), - webXmlContent.indexOf(SECURITY_CONSTRAINT_END_TAG, index)) - .trim(); + substringTrim( + webXmlContent, + index + SECURITY_CONSTRAINT_START_TAG.length(), + webXmlContent.indexOf(SECURITY_CONSTRAINT_END_TAG, index)); if (!innerText.contains("")) { report( span, @@ -208,17 +223,6 @@ private void checkVerbTampering(final String webXmlContent, int index, final Age } } - private int getLine(String webXmlContent, int index) { - int line = 1; - int limit = Math.min(index, webXmlContent.length()); - for (int i = limit; i > 0; i--) { - if (webXmlContent.charAt(i) == '\n') { - line++; - } - } - return line; - } - private void report(AgentSpan span, VulnerabilityType type, String value, int line) { reporter.report( span, @@ -226,14 +230,14 @@ private void report(AgentSpan span, VulnerabilityType type, String value, int li type, Location.forSpanAndFileAndLine(span, WEB_XML, line), new Evidence(value))); } - private void checkInsecureJSPLayout(String realPath, AgentSpan span) { - List jspPaths = findRecursively(new File(realPath)); + private void checkInsecureJSPLayout(@Nonnull Path path, AgentSpan span) { + final Collection jspPaths = findInsecureJspPaths(path); if (jspPaths.isEmpty()) { return; } String result = jspPaths.stream() - .map(s -> File.separatorChar + s) + .map(jspFolder -> relativize(path, jspFolder)) .collect(Collectors.joining(System.lineSeparator())); reporter.report( span, @@ -241,49 +245,86 @@ private void checkInsecureJSPLayout(String realPath, AgentSpan span) { VulnerabilityType.INSECURE_JSP_LAYOUT, Location.forSpan(span), new Evidence(result))); } + private static int getLine(String webXmlContent, int index) { + int line = 1; + int limit = Math.min(index, webXmlContent.length()); + for (int i = limit; i > 0; i--) { + if (webXmlContent.charAt(i) == '\n') { + line++; + } + } + return line; + } + @Nullable - private String webXmlContent(final String realPath) { - if (realPath != null) { - Path path = Paths.get(realPath + File.separatorChar + WEB_INF + File.separatorChar + WEB_XML); - if (path.toFile().exists()) { - try { - return new String(Files.readAllBytes(path), StandardCharsets.UTF_8); - } catch (IOException e) { - // Nothing to do - } + private static String webXmlContent(final Path realPath) { + Path path = realPath.resolve(WEB_INF).resolve(WEB_XML); + if (Files.exists(path)) { + try { + return new String(Files.readAllBytes(path), StandardCharsets.UTF_8); + } catch (IOException e) { + LOGGER.debug(SEND_TELEMETRY, "Failed to read {}, encoding issue?", path, e); } } return null; } - private List findRecursively(final File root) { - List jspPaths = new ArrayList<>(); - if (root.listFiles() != null) { - for (File file : root.listFiles()) { - if (file.isFile() && isJsp(file.getName().toLowerCase())) { - jspPaths.add(file.getName()); - } else if (file.isDirectory() && !WEB_INF.equals(file.getName())) { - addDirectoryJSPs("", file, jspPaths); - } - } + private static Collection findInsecureJspPaths(final Path root) { + try { + final InsecureJspFolderVisitor visitor = new InsecureJspFolderVisitor(); + // TODO is it OK to ignore symlinks here? + Files.walkFileTree(root, EnumSet.noneOf(FileVisitOption.class), JSP_MAX_WALK_DEPTH, visitor); + return visitor.folders; + } catch (Exception e) { + throw new RuntimeException(e); } - return jspPaths; } - private void addDirectoryJSPs(final String path, final File dir, final List jspPaths) { - String currentPath = path + dir.getName() + File.separatorChar; - if (dir.listFiles() != null) { - for (File file : dir.listFiles()) { - if (file.isFile() && isJsp(file.getName().toLowerCase())) { - jspPaths.add(currentPath + file.getName()); - } else if (file.isDirectory()) { - addDirectoryJSPs(currentPath, file, jspPaths); - } - } + private static String relativize(final Path root, final Path path) { + final String relative = root.relativize(path).toString(); + if (relative.isEmpty()) { + return File.separator; } + if (relative.charAt(0) == File.separatorChar) { + return relative; + } + return File.separatorChar + relative; } - private boolean isJsp(final String name) { - return name.endsWith(".jsp") || name.endsWith(".jspx"); + private static class InsecureJspFolderVisitor implements FileVisitor { + private final Set folders = new HashSet<>(); + + @Override + public FileVisitResult preVisitDirectory(final Path dir, final BasicFileAttributes attrs) + throws IOException { + final String folder = dir.getFileName().toString(); + if (endsWithIgnoreCase(folder, WEB_INF)) { + return FileVisitResult.SKIP_SUBTREE; + } + return FileVisitResult.CONTINUE; + } + + @Override + public FileVisitResult visitFile(final Path file, final BasicFileAttributes attrs) + throws IOException { + final String fileName = file.getFileName().toString(); + if (endsWithIgnoreCase(fileName, ".jsp") || endsWithIgnoreCase(fileName, ".jspx")) { + folders.add(file.getParent()); + return FileVisitResult.SKIP_SIBLINGS; + } + return FileVisitResult.CONTINUE; + } + + @Override + public FileVisitResult visitFileFailed(final Path file, final IOException exc) + throws IOException { + return FileVisitResult.CONTINUE; + } + + @Override + public FileVisitResult postVisitDirectory(final Path dir, final IOException exc) + throws IOException { + return FileVisitResult.CONTINUE; + } } } diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/util/StringUtils.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/util/StringUtils.java new file mode 100644 index 00000000000..4e45a44cde7 --- /dev/null +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/util/StringUtils.java @@ -0,0 +1,41 @@ +package com.datadog.iast.util; + +import javax.annotation.Nonnull; + +public abstract class StringUtils { + + private StringUtils() {} + + /** + * Checks if the string ends with the selected suffix ignoring case. Note that this method does + * not take locale into account. + */ + public static boolean endsWithIgnoreCase( + @Nonnull final String value, @Nonnull final String suffix) { + if (value.length() < suffix.length()) { + return false; + } + if (suffix.isEmpty()) { + return true; + } + final int offset = value.length() - suffix.length(); + return value.regionMatches(true, offset, suffix, 0, suffix.length()); + } + + /** + * Performs a substring of the selected string taking care of leading and trailing whitespaces. + */ + @Nonnull + public static String substringTrim(@Nonnull final String value, int start, int end) { + if (start >= end) { + return ""; + } + while (start < end && Character.isWhitespace(value.charAt(start))) { + start++; + } + while (end > start && Character.isWhitespace(value.charAt(end - 1))) { + end--; + } + return start >= end ? "" : value.substring(start, end); + } +} diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/ApplicationModuleTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/ApplicationModuleTest.groovy index 3b7a09cad8b..ab249758beb 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/ApplicationModuleTest.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/ApplicationModuleTest.groovy @@ -2,10 +2,16 @@ package com.datadog.iast.sink import com.datadog.iast.IastModuleImplTestBase import com.datadog.iast.Reporter -import com.datadog.iast.model.VulnerabilityType import datadog.trace.api.iast.InstrumentationBridge import datadog.trace.api.iast.sink.ApplicationModule +import static com.datadog.iast.model.VulnerabilityType.ADMIN_CONSOLE_ACTIVE +import static com.datadog.iast.model.VulnerabilityType.DEFAULT_HTML_ESCAPE_INVALID +import static com.datadog.iast.model.VulnerabilityType.DIRECTORY_LISTING_LEAK +import static com.datadog.iast.model.VulnerabilityType.INSECURE_JSP_LAYOUT +import static com.datadog.iast.model.VulnerabilityType.SESSION_TIMEOUT +import static com.datadog.iast.model.VulnerabilityType.VERB_TAMPERING + class ApplicationModuleTest extends IastModuleImplTestBase { private static final int NO_LINE = -1 @@ -47,27 +53,22 @@ class ApplicationModuleTest extends IastModuleImplTestBase { } where: - path | expectedVulnType | expectedEvidence | line - 'application/insecurejsplayout/secure' | null | null | _ - 'application/insecurejsplayout/insecure' | VulnerabilityType.INSECURE_JSP_LAYOUT | [ - '/nestedinsecure/insecure2.jsp', - '/nestedinsecure/nestedinsecure/insecure3.jsp' , - '/insecure.jsp', - '/insecure.jspx' - ] as String [] | NO_LINE - 'application/verbtampering/secure' | null | null | _ - 'application/verbtampering/insecure' | VulnerabilityType.VERB_TAMPERING | 'http-method not defined in web.xml' | 6 - 'application/sessiontimeout/secure' | null | null | _ - 'application/sessiontimeout/insecure' | VulnerabilityType.SESSION_TIMEOUT | 'Found vulnerable timeout value: 80' | 7 - 'application/directorylistingleak/secure' | null | null | _ - 'application/directorylistingleak/insecure' | VulnerabilityType.DIRECTORY_LISTING_LEAK | 'Directory listings configured' | 14 - 'application/adminconsoleactive/secure' | null | null | _ - 'application/adminconsoleactive/insecure' | VulnerabilityType.ADMIN_CONSOLE_ACTIVE | 'Tomcat Manager Application' | NO_LINE - 'application/defaulthtmlescapeinvalid/secure' | null | null | _ - 'application/defaulthtmlescapeinvalid/secure_tag' | null | null | _ - 'application/defaulthtmlescapeinvalid/false_tag' | VulnerabilityType.DEFAULT_HTML_ESCAPE_INVALID | 'defaultHtmlEscape tag should be true' | 8 - 'application/defaulthtmlescapeinvalid/no_tag_1' | VulnerabilityType.DEFAULT_HTML_ESCAPE_INVALID | 'defaultHtmlEscape tag should be set' | NO_LINE - 'application/defaulthtmlescapeinvalid/no_tag_2' | VulnerabilityType.DEFAULT_HTML_ESCAPE_INVALID | 'defaultHtmlEscape tag should be set' | NO_LINE + path | expectedVulnType | expectedEvidence | line + 'application/insecurejsplayout/secure' | null | null | _ + 'application/insecurejsplayout/insecure' | INSECURE_JSP_LAYOUT | ['/nestedinsecure', '/nestedinsecure/nestedinsecure', '/'] | NO_LINE + 'application/verbtampering/secure' | null | null | _ + 'application/verbtampering/insecure' | VERB_TAMPERING | 'http-method not defined in web.xml' | 6 + 'application/sessiontimeout/secure' | null | null | _ + 'application/sessiontimeout/insecure' | SESSION_TIMEOUT | 'Found vulnerable timeout value: 80' | 7 + 'application/directorylistingleak/secure' | null | null | _ + 'application/directorylistingleak/insecure' | DIRECTORY_LISTING_LEAK | 'Directory listings configured' | 14 + 'application/adminconsoleactive/secure' | null | null | _ + 'application/adminconsoleactive/insecure' | ADMIN_CONSOLE_ACTIVE | 'Tomcat Manager Application' | NO_LINE + 'application/defaulthtmlescapeinvalid/secure' | null | null | _ + 'application/defaulthtmlescapeinvalid/secure_tag' | null | null | _ + 'application/defaulthtmlescapeinvalid/false_tag' | DEFAULT_HTML_ESCAPE_INVALID | 'defaultHtmlEscape tag should be true' | 8 + 'application/defaulthtmlescapeinvalid/no_tag_1' | DEFAULT_HTML_ESCAPE_INVALID | 'defaultHtmlEscape tag should be set' | NO_LINE + 'application/defaulthtmlescapeinvalid/no_tag_2' | DEFAULT_HTML_ESCAPE_INVALID | 'defaultHtmlEscape tag should be set' | NO_LINE } private static void assertVulnerability(final vuln, final expectedVulnType) { @@ -80,9 +81,15 @@ class ApplicationModuleTest extends IastModuleImplTestBase { assertVulnerability(vuln, expectedVulnType) final evidence = vuln.evidence assert evidence != null - if (expectedEvidence instanceof String[]) { - for (String s : expectedEvidence) { - assert evidence.value.contains(s) + if (expectedEvidence instanceof Collection) { + if (expectedVulnType == INSECURE_JSP_LAYOUT) { + // some of the nested paths can be dropped by the file visitor + final parts = (evidence.value as String).split('\n')*.trim() + assert expectedEvidence.any { parts.contains(it) } + } else { + expectedEvidence.each { + assert evidence.value.contains(it) + } } } else { assert evidence.value == expectedEvidence diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/util/StringUtilsTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/util/StringUtilsTest.groovy new file mode 100644 index 00000000000..6109e366910 --- /dev/null +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/util/StringUtilsTest.groovy @@ -0,0 +1,46 @@ +package com.datadog.iast.util + +import spock.lang.Specification + +class StringUtilsTest extends Specification { + + void 'test ends with ignore case'() { + when: + final endsWith = StringUtils.endsWithIgnoreCase(value, pattern) + + then: + endsWith == expected + + where: + value | pattern | expected + '' | '' | true + 'abc' | '' | true + 'abc' | 'def' | false + 'abc' | '0abc' | false + '0abc' | 'abc' | true + 'abc' | 'aBc' | true + '0abc' | 'c' | true + 'abc' | 'C' | true + } + + void 'test substring with trim'() { + when: + final trimmed = StringUtils.substringTrim(value, start, end) + + then: + trimmed == expected + + where: + value | start | end | expected + '' | 0 | 0 | '' + ' ' | 0 | 1 | '' + 'abc' | 1 | 3 | 'bc' + ' abc' | 1 | 3 | 'a' + 'abc ' | 1 | 3 | 'bc' + ' abc ' | 1 | 3 | 'a' + ' ab c ' | 2 | 6 | 'ab' + ' ab c ' | 0 | 6 | 'ab' + ' ab' | 0 | 3 | '' + ' ab ' | 4 | 7 | '' + } +} diff --git a/dd-java-agent/agent-iast/src/test/resources/application/insecurejsplayout/secure/nested/README.md b/dd-java-agent/agent-iast/src/test/resources/application/insecurejsplayout/secure/nested/README.md new file mode 100644 index 00000000000..ca60906e742 --- /dev/null +++ b/dd-java-agent/agent-iast/src/test/resources/application/insecurejsplayout/secure/nested/README.md @@ -0,0 +1,3 @@ +# Secure JSP layout + +Sample file included in a secure layout diff --git a/internal-api/src/main/java/datadog/trace/api/civisibility/telemetry/CiVisibilityDistributionMetric.java b/internal-api/src/main/java/datadog/trace/api/civisibility/telemetry/CiVisibilityDistributionMetric.java index 518f9696966..258ddd6d0b8 100644 --- a/internal-api/src/main/java/datadog/trace/api/civisibility/telemetry/CiVisibilityDistributionMetric.java +++ b/internal-api/src/main/java/datadog/trace/api/civisibility/telemetry/CiVisibilityDistributionMetric.java @@ -27,7 +27,7 @@ public enum CiVisibilityDistributionMetric { GIT_REQUESTS_OBJECTS_PACK_MS("git_requests.objects_pack_ms"), /** The sum of the sizes of the object pack files inside a single payload */ GIT_REQUESTS_OBJECTS_PACK_BYTES("git_requests.objects_pack_bytes"), - /** The number of files sent in the object pack payload */ + /** The number of pack files created in a test session */ GIT_REQUESTS_OBJECTS_PACK_FILES("git_requests.objects_pack_files"), /** The time it takes to get the response of the settings endpoint request in ms */ GIT_REQUESTS_SETTINGS_MS("git_requests.settings_ms"),