SCI Embedding - read git.properties file (#3135)

* read git.properties

* more and better tests

* fix tests

Author: juan-fernandez

packages/dd-trace/src/config.js

@@ -2,14 +2,15 @@
 
 const fs = require('fs')
 const os = require('os')
+const uuid = require('crypto-randomuuid')
 const URL = require('url').URL
 const log = require('./log')
 const pkg = require('./pkg')
 const coalesce = require('koalas')
 const tagger = require('./tagger')
 const { isTrue, isFalse } = require('./util')
-const uuid = require('crypto-randomuuid')
 const { GIT_REPOSITORY_URL, GIT_COMMIT_SHA } = require('./plugins/util/tags')
+const { getGitMetadataFromGitProperties } = require('./git_properties')
 
 const fromEntries = Object.fromEntries || (entries =>
   entries.reduce((obj, [k, v]) => Object.assign(obj, { [k]: v }), {}))
@@ -543,6 +544,18 @@ ken|consumer_?(?:id|key|secret)|sign(?:ed|ature)?|auth(?:entication|orization)?)
         process.env.DD_GIT_COMMIT_SHA,
         this.tags[GIT_COMMIT_SHA]
       )
+      if (!this.repositoryUrl || !this.commitSHA) {
+        const DD_GIT_PROPERTIES_FILE = coalesce(
+          process.env.DD_GIT_PROPERTIES_FILE,
+          `${process.cwd()}/git.properties`
+        )
+        const gitPropertiesString = maybeFile(DD_GIT_PROPERTIES_FILE)
+        if (gitPropertiesString) {
+          const { commitSHA, repositoryUrl } = getGitMetadataFromGitProperties(gitPropertiesString)
+          this.commitSHA = this.commitSHA || commitSHA
+          this.repositoryUrl = this.repositoryUrl || repositoryUrl
+        }
+      }
     }
 
     this.stats = {

packages/dd-trace/src/git_properties.js

@@ -0,0 +1,32 @@
+const commitSHARegex = /git\.commit\.sha=([a-f\d]{40})/
+const repositoryUrlRegex = /git\.repository_url=([\w\d:@/.-]+)/
+
+function getGitMetadataFromGitProperties (gitPropertiesString) {
+  if (!gitPropertiesString) {
+    return {}
+  }
+  const commitSHAMatch = gitPropertiesString.match(commitSHARegex)
+  const repositoryUrlMatch = gitPropertiesString.match(repositoryUrlRegex)
+
+  const repositoryUrl = repositoryUrlMatch ? repositoryUrlMatch[1] : undefined
+  let parsedUrl = repositoryUrl
+
+  if (repositoryUrl) {
+    try {
+      // repository URLs can contain username and password, so we want to filter those out
+      parsedUrl = new URL(repositoryUrl)
+      if (parsedUrl.password) {
+        parsedUrl = `${parsedUrl.origin}${parsedUrl.pathname}`
+      }
+    } catch (e) {
+      // if protocol isn't https, no password will be used
+    }
+  }
+
+  return {
+    commitSHA: commitSHAMatch ? commitSHAMatch[1] : undefined,
+    repositoryUrl: parsedUrl
+  }
+}
+
+module.exports = { getGitMetadataFromGitProperties }

packages/dd-trace/test/config.spec.js

@@ -24,6 +24,7 @@ describe('Config', () => {
   const BLOCKED_TEMPLATE_HTML = readFileSync(BLOCKED_TEMPLATE_HTML_PATH, { encoding: 'utf8' })
   const BLOCKED_TEMPLATE_JSON_PATH = require.resolve('./fixtures/config/appsec-blocked-template.json')
   const BLOCKED_TEMPLATE_JSON = readFileSync(BLOCKED_TEMPLATE_JSON_PATH, { encoding: 'utf8' })
+  const DD_GIT_PROPERTIES_FILE = require.resolve('./fixtures/config/git.properties')
 
   beforeEach(() => {
     pkg = {
@@ -936,7 +937,7 @@ describe('Config', () => {
       }
     })
 
-    expect(log.error).to.be.calledThrice
+    expect(log.error).to.be.callCount(4)
     expect(log.error.firstCall).to.have.been.calledWithExactly(error)
     expect(log.error.secondCall).to.have.been.calledWithExactly(error)
     expect(log.error.thirdCall).to.have.been.calledWithExactly(error)
@@ -1076,4 +1077,73 @@ describe('Config', () => {
       })
     })
   })
+
+  context('sci embedding', () => {
+    const DUMMY_COMMIT_SHA = 'b7b5dfa992008c77ab3f8a10eb8711e0092445b0'
+    const DUMMY_REPOSITORY_URL = '[email protected]:DataDog/dd-trace-js.git'
+    let ddTags
+    beforeEach(() => {
+      ddTags = process.env.DD_TAGS
+    })
+    afterEach(() => {
+      delete process.env.DD_GIT_PROPERTIES_FILE
+      delete process.env.DD_GIT_COMMIT_SHA
+      delete process.env.DD_GIT_REPOSITORY_URL
+      delete process.env.DD_TRACE_GIT_METADATA_ENABLED
+      process.env.DD_TAGS = ddTags
+    })
+    it('reads DD_GIT_* env vars', () => {
+      process.env.DD_GIT_COMMIT_SHA = DUMMY_COMMIT_SHA
+      process.env.DD_GIT_REPOSITORY_URL = DUMMY_REPOSITORY_URL
+      const config = new Config({})
+      expect(config).to.have.property('commitSHA', DUMMY_COMMIT_SHA)
+      expect(config).to.have.property('repositoryUrl', DUMMY_REPOSITORY_URL)
+    })
+    it('reads DD_TAGS env var', () => {
+      process.env.DD_TAGS = `git.commit.sha:${DUMMY_COMMIT_SHA},git.repository_url:${DUMMY_REPOSITORY_URL}`
+      process.env.DD_GIT_REPOSITORY_URL = DUMMY_REPOSITORY_URL
+      const config = new Config({})
+      expect(config).to.have.property('commitSHA', DUMMY_COMMIT_SHA)
+      expect(config).to.have.property('repositoryUrl', DUMMY_REPOSITORY_URL)
+    })
+    it('reads git.properties if it is available', () => {
+      process.env.DD_GIT_PROPERTIES_FILE = DD_GIT_PROPERTIES_FILE
+      const config = new Config({})
+      expect(config).to.have.property('commitSHA', '4e7da8069bcf5ffc8023603b95653e2dc99d1c7d')
+      expect(config).to.have.property('repositoryUrl', DUMMY_REPOSITORY_URL)
+    })
+    it('does not crash if git.properties is not available', () => {
+      process.env.DD_GIT_PROPERTIES_FILE = '/does/not/exist'
+      const config = new Config({})
+      expect(config).to.have.property('commitSHA', undefined)
+      expect(config).to.have.property('repositoryUrl', undefined)
+    })
+    it('does not read git.properties if env vars are passed', () => {
+      process.env.DD_GIT_PROPERTIES_FILE = DD_GIT_PROPERTIES_FILE
+      process.env.DD_GIT_COMMIT_SHA = DUMMY_COMMIT_SHA
+      process.env.DD_GIT_REPOSITORY_URL = 'https://github.com:env-var/dd-trace-js.git'
+      const config = new Config({})
+      expect(config).to.have.property('commitSHA', DUMMY_COMMIT_SHA)
+      expect(config).to.have.property('repositoryUrl', 'https://github.com:env-var/dd-trace-js.git')
+    })
+    it('still reads git.properties if one of the env vars is missing', () => {
+      process.env.DD_GIT_PROPERTIES_FILE = DD_GIT_PROPERTIES_FILE
+      process.env.DD_GIT_COMMIT_SHA = DUMMY_COMMIT_SHA
+      const config = new Config({})
+      expect(config).to.have.property('commitSHA', DUMMY_COMMIT_SHA)
+      expect(config).to.have.property('repositoryUrl', DUMMY_REPOSITORY_URL)
+    })
+    it('reads git.properties and filters out credentials', () => {
+      process.env.DD_GIT_PROPERTIES_FILE = require.resolve('./fixtures/config/git.properties.credentials')
+      const config = new Config({})
+      expect(config).to.have.property('commitSHA', '4e7da8069bcf5ffc8023603b95653e2dc99d1c7d')
+      expect(config).to.have.property('repositoryUrl', 'https://github.com/datadog/dd-trace-js')
+    })
+    it('does not read git metadata if DD_TRACE_GIT_METADATA_ENABLED is false', () => {
+      process.env.DD_TRACE_GIT_METADATA_ENABLED = 'false'
+      const config = new Config({})
+      expect(config).not.to.have.property('commitSHA')
+      expect(config).not.to.have.property('repositoryUrl')
+    })
+  })
 })

packages/dd-trace/test/fixtures/config/git.properties

@@ -0,0 +1,2 @@
+git.commit.sha=4e7da8069bcf5ffc8023603b95653e2dc99d1c7d
+git.repository_url[email protected]:DataDog/dd-trace-js.git

packages/dd-trace/test/fixtures/config/git.properties.credentials

@@ -0,0 +1,3 @@
+git.commit.sha=4e7da8069bcf5ffc8023603b95653e2dc99d1c7d
+git.repository_url=https://username:[email protected]/datadog/dd-trace-js
+[email protected]

packages/dd-trace/test/git_properties.spec.js

@@ -0,0 +1,49 @@
+require('./setup/tap')
+
+const { getGitMetadataFromGitProperties } = require('../src/git_properties')
+
+describe('git_properties', () => {
+  context('getGitMetadataFromGitProperties', () => {
+    it('reads commit SHA and repository URL', () => {
+      const { commitSHA, repositoryUrl } = getGitMetadataFromGitProperties(`
+git.commit.sha=4e7da8069bcf5ffc8023603b95653e2dc99d1c7d
+[email protected]:DataDog/dd-trace-js.git
+      `)
+      expect(commitSHA).to.equal('4e7da8069bcf5ffc8023603b95653e2dc99d1c7d')
+      expect(repositoryUrl).to.equal('[email protected]:DataDog/dd-trace-js.git')
+    })
+    it('filters out credentials', () => {
+      const { commitSHA, repositoryUrl } = getGitMetadataFromGitProperties(`
+git.commit.sha=4e7da8069bcf5ffc8023603b95653e2dc99d1c7d
+git.repository_url=https://username:[email protected]/datadog/dd-trace-js.git
+      `)
+      expect(commitSHA).to.equal('4e7da8069bcf5ffc8023603b95653e2dc99d1c7d')
+      expect(repositoryUrl).to.equal('https://github.com/datadog/dd-trace-js.git')
+    })
+    it('ignores other fields', () => {
+      const { commitSHA, repositoryUrl } = getGitMetadataFromGitProperties(`
+git.commit.sha=4e7da8069bcf5ffc8023603b95653e2dc99d1c7d
+[email protected]:DataDog/dd-trace-js.git
+[email protected]
+      `)
+      expect(commitSHA).to.equal('4e7da8069bcf5ffc8023603b95653e2dc99d1c7d')
+      expect(repositoryUrl).to.equal('[email protected]:DataDog/dd-trace-js.git')
+    })
+    it('ignores badly formatted files', () => {
+      const { commitSHA, repositoryUrl } = getGitMetadataFromGitProperties(`
+git.commit.sha=; rm -rf ;
+git.repository_url=; rm -rf ;
+      `)
+      expect(commitSHA).to.equal(undefined)
+      expect(repositoryUrl).to.equal(undefined)
+    })
+    it('does not crash with empty files', () => {
+      const emptyStringResult = getGitMetadataFromGitProperties('')
+      expect(emptyStringResult.commitSHA).to.equal(undefined)
+      expect(emptyStringResult.repositoryUrl).to.equal(undefined)
+      const undefinedResult = getGitMetadataFromGitProperties(undefined)
+      expect(undefinedResult.commitSHA).to.equal(undefined)
+      expect(undefinedResult.repositoryUrl).to.equal(undefined)
+    })
+  })
+})

packages/dd-trace/test/serverless.spec.js

@@ -75,7 +75,7 @@ describe('Serverless', () => {
 
     // trying to spawn with an invalid path will return a non-descriptive error, so we want to catch
     // invalid paths and log our own error.
-    expect(logErrorSpy).to.have.been.calledOnceWith(
+    expect(logErrorSpy).to.have.been.calledWith(
       'Serverless Mini Agent did not start. Could not find mini agent binary.'
     )
     existsSyncStub.returns(true)