validate CI tags before sending them (#3310)

* ciapp-3571: add url validator * ciapp-3571: add url validator and change tests from ci-spec * fix variables to follow camelcase * Remove valid import Co-authored-by: Juan Antonio Fernández de Alba <juan.fernandezdealba@datadoghq.com> * Remove unnecessary check Co-authored-by: Juan Antonio Fernández de Alba <juan.fernandezdealba@datadoghq.com> * Remove unnecessary check for CI_PIPELINE_URL Co-authored-by: Juan Antonio Fernández de Alba <juan.fernandezdealba@datadoghq.com> * Format typo Co-authored-by: Juan Antonio Fernández de Alba <juan.fernandezdealba@datadoghq.com> * Use validateMetadata return directly Co-authored-by: Juan Antonio Fernández de Alba <juan.fernandezdealba@datadoghq.com> * use new ci-spec JSONs * fix spacing and sort imports * add test to validateMetadata * Update JSONs and refactor URL checking * Update JSONs and refactor URL checking * Add URL validation tests * Apply suggestions from code review Co-authored-by: Juan Antonio Fernández de Alba <juan.fernandezdealba@datadoghq.com> * Add tag to test suite --------- Co-authored-by: Juan Antonio Fernández de Alba <juan.fernandezdealba@datadoghq.com>
DataDog · Jul 11, 2023 · 74ebe56 · 74ebe56
1 parent 8f3a4eb
commit 74ebe56
Show file tree

Hide file tree

Showing 15 changed files with 644 additions and 574 deletions.
diff --git a/packages/dd-trace/src/plugins/util/ci.js b/packages/dd-trace/src/plugins/util/ci.js
@@ -77,7 +77,7 @@ function filterSensitiveInfoFromRepository (repositoryUrl) {
 
     return `${protocol}//${hostname}${pathname}`
   } catch (e) {
-    return repositoryUrl
+    return ''
   }
 }
 

diff --git a/packages/dd-trace/src/plugins/util/test.js b/packages/dd-trace/src/plugins/util/test.js
@@ -1,11 +1,13 @@
 const path = require('path')
 const fs = require('fs')
+const { URL } = require('url')
+const log = require('../../log')
 
 const istanbul = require('istanbul-lib-coverage')
 const ignore = require('ignore')
 
 const { getGitMetadata } = require('./git')
-const { getUserProviderGitMetadata } = require('./user-provided-git')
+const { getUserProviderGitMetadata, validateGitRepositoryUrl, validateGitCommitSha } = require('./user-provided-git')
 const { getCIMetadata } = require('./ci')
 const { getRuntimeAndOSMetadata } = require('./env')
 const {
@@ -16,7 +18,8 @@ const {
   GIT_COMMIT_AUTHOR_EMAIL,
   GIT_COMMIT_AUTHOR_NAME,
   GIT_COMMIT_MESSAGE,
-  CI_WORKSPACE_PATH
+  CI_WORKSPACE_PATH,
+  CI_PIPELINE_URL
 } = require('./tags')
 const id = require('../../id')
 
@@ -104,7 +107,8 @@ module.exports = {
   mergeCoverage,
   fromCoverageMapToCoverage,
   getTestLineStart,
-  getCallSites
+  getCallSites,
+  removeInvalidMetadata
 }
 
 // Returns pkg manager and its version, separated by '-', e.g. npm-8.15.0 or yarn-1.22.19
@@ -116,6 +120,39 @@ function getPkgManager () {
   }
 }
 
+function validateUrl (url) {
+  try {
+    const urlObject = new URL(url)
+    return (urlObject.protocol === 'https:' || urlObject.protocol === 'http:')
+  } catch (e) {
+    return false
+  }
+}
+
+function removeInvalidMetadata (metadata) {
+  return Object.keys(metadata).reduce((filteredTags, tag) => {
+    if (tag === GIT_REPOSITORY_URL) {
+      if (!validateGitRepositoryUrl(metadata[GIT_REPOSITORY_URL])) {
+        log.error('DD_GIT_REPOSITORY_URL must be a valid URL')
+        return filteredTags
+      }
+    }
+    if (tag === GIT_COMMIT_SHA) {
+      if (!validateGitCommitSha(metadata[GIT_COMMIT_SHA])) {
+        log.error('DD_GIT_COMMIT_SHA must be a full-length git SHA')
+        return filteredTags
+      }
+    }
+    if (tag === CI_PIPELINE_URL) {
+      if (!validateUrl(metadata[CI_PIPELINE_URL])) {
+        return filteredTags
+      }
+    }
+    filteredTags[tag] = metadata[tag]
+    return filteredTags
+  }, {})
+}
+
 function getTestEnvironmentMetadata (testFramework, config) {
   // TODO: eventually these will come from the tracer (generally available)
   const ciMetadata = getCIMetadata()
@@ -155,7 +192,7 @@ function getTestEnvironmentMetadata (testFramework, config) {
   if (config && config.service) {
     metadata['service.name'] = config.service
   }
-  return metadata
+  return removeInvalidMetadata(metadata)
 }
 
 function getTestParametersString (parametersByTestName, testName) {

diff --git a/packages/dd-trace/src/plugins/util/user-provided-git.js b/packages/dd-trace/src/plugins/util/user-provided-git.js
@@ -13,7 +13,6 @@ const {
 } = require('./tags')
 
 const { normalizeRef } = require('./ci')
-const log = require('../../log')
 const { URL } = require('url')
 
 function removeEmptyValues (tags) {
@@ -53,25 +52,6 @@ function validateGitCommitSha (gitCommitSha) {
   return isValidSha1 || isValidSha256
 }
 
-function removeInvalidGitMetadata (metadata) {
-  return Object.keys(metadata).reduce((filteredTags, tag) => {
-    if (tag === GIT_REPOSITORY_URL) {
-      if (!validateGitRepositoryUrl(metadata[GIT_REPOSITORY_URL])) {
-        log.error('DD_GIT_REPOSITORY_URL must be a valid URL')
-        return filteredTags
-      }
-    }
-    if (tag === GIT_COMMIT_SHA) {
-      if (!validateGitCommitSha(metadata[GIT_COMMIT_SHA])) {
-        log.error('DD_GIT_COMMIT_SHA must be a full-length git SHA')
-        return filteredTags
-      }
-    }
-    filteredTags[tag] = metadata[tag]
-    return filteredTags
-  }, {})
-}
-
 function getUserProviderGitMetadata () {
   const {
     DD_GIT_COMMIT_SHA,
@@ -95,7 +75,7 @@ function getUserProviderGitMetadata () {
     tag = normalizeRef(DD_GIT_BRANCH)
   }
 
-  const metadata = removeEmptyValues({
+  return removeEmptyValues({
     [GIT_COMMIT_SHA]: DD_GIT_COMMIT_SHA,
     [GIT_BRANCH]: branch,
     [GIT_REPOSITORY_URL]: filterSensitiveInfoFromRepository(DD_GIT_REPOSITORY_URL),
@@ -108,7 +88,6 @@ function getUserProviderGitMetadata () {
     [GIT_COMMIT_AUTHOR_EMAIL]: DD_GIT_COMMIT_AUTHOR_EMAIL,
     [GIT_COMMIT_AUTHOR_DATE]: DD_GIT_COMMIT_AUTHOR_DATE
   })
-  return removeInvalidGitMetadata(metadata)
 }
 
 module.exports = { getUserProviderGitMetadata, validateGitRepositoryUrl, validateGitCommitSha }