Merge pull request #16 from leanix/feature/CID-2777/secure-webhook-li…

…stener-endpoint

CID-2777: Secure webhook listener endpoint

README.md

@@ -28,6 +28,7 @@ The SAP LeanIX agent discovers self-built software in self-hosted GitHub Enterpr
     - `GITHUB_APP_ID`: The ID of your GitHub App.
     - `PEM_FILE`: The path to your GitHub App's PEM file inside the Docker container.
     - `MANIFEST_FILE_DIRECTORY`: The directory path where the manifest files are stored in each repository. Manifest files are crucial for microservice discovery as they provide essential information about the service. For more information, see [Microservice Discovery Through a Manifest File](https://docs-eam.leanix.net/reference/microservice-discovery-manifest-file) in our documentation.
+    - `WEBHOOK_SECRET`: The secret used to validate incoming webhook events from GitHub. (Optional, but recommended. [Needs to be set in the GitHub App settings first](https://docs.github.com/en/enterprise-server@3.8/webhooks/using-webhooks/validating-webhook-deliveries).)
 
 5. **Start the agent**: To start the agent, run the following Docker command. Replace the variables in angle brackets with your actual values.
 
@@ -38,6 +39,7 @@ The SAP LeanIX agent discovers self-built software in self-hosted GitHub Enterpr
     -e GITHUB_APP_ID=<github_app_id> \
     -e PEM_FILE=/privateKey.pem \
     -e MANIFEST_FILE_DIRECTORY=<manifest_file_directory> \
+    -e WEBHOOK_SECRET=<webhook_secret> \
     leanix-github-agent
     ```
 

config/detekt/detekt.yml

@@ -28,6 +28,8 @@ style:
     active: false
   UnusedPrivateMember:
     active: false
+  ThrowsCount:
+    active: false
 
 empty-blocks:
   EmptyFunctionBlock:

src/main/kotlin/net/leanix/githubagent/config/GitHubEnterpriseProperties.kt

@@ -8,4 +8,5 @@ data class GitHubEnterpriseProperties(
     val gitHubAppId: String,
     val pemFile: String,
     val manifestFileDirectory: String,
+    val webhookSecret: String
 )

src/main/kotlin/net/leanix/githubagent/controllers/GitHubWebhookController.kt

@@ -1,8 +1,6 @@
 package net.leanix.githubagent.controllers
 
-import net.leanix.githubagent.services.WebhookService
-import net.leanix.githubagent.shared.SUPPORTED_EVENT_TYPES
-import org.slf4j.LoggerFactory
+import net.leanix.githubagent.services.GitHubWebhookHandler
 import org.springframework.http.HttpStatus
 import org.springframework.web.bind.annotation.PostMapping
 import org.springframework.web.bind.annotation.RequestBody
@@ -13,22 +11,16 @@ import org.springframework.web.bind.annotation.RestController
 
 @RestController
 @RequestMapping("github")
-class GitHubWebhookController(private val webhookService: WebhookService) {
-
-    private val logger = LoggerFactory.getLogger(GitHubWebhookController::class.java)
+class GitHubWebhookController(private val gitHubWebhookHandler: GitHubWebhookHandler) {
 
     @PostMapping("/webhook")
     @ResponseStatus(HttpStatus.ACCEPTED)
     fun hook(
         @RequestHeader("X-Github-Event") eventType: String,
+        @RequestHeader("X-GitHub-Enterprise-Host") host: String,
+        @RequestHeader("X-Hub-Signature-256", required = false) signature256: String?,
         @RequestBody payload: String
     ) {
-        runCatching {
-            if (SUPPORTED_EVENT_TYPES.contains(eventType.uppercase())) {
-                webhookService.consumeWebhookEvent(eventType, payload)
-            } else {
-                logger.warn("Received an unsupported event of type: $eventType")
-            }
-        }
+        gitHubWebhookHandler.handleWebhookEvent(eventType, host, signature256, payload)
     }
 }

src/main/kotlin/net/leanix/githubagent/controllers/advice/GlobalExceptionHandler.kt

@@ -0,0 +1,31 @@
+package net.leanix.githubagent.controllers.advice
+
+import net.leanix.githubagent.exceptions.InvalidEventSignatureException
+import net.leanix.githubagent.exceptions.WebhookSecretNotSetException
+import org.slf4j.Logger
+import org.slf4j.LoggerFactory
+import org.springframework.http.HttpStatus
+import org.springframework.http.ProblemDetail
+import org.springframework.web.bind.annotation.ControllerAdvice
+import org.springframework.web.bind.annotation.ExceptionHandler
+
+@ControllerAdvice
+class GlobalExceptionHandler {
+
+    val exceptionLogger: Logger = LoggerFactory.getLogger(GlobalExceptionHandler::class.java)
+
+    @ExceptionHandler(InvalidEventSignatureException::class)
+    fun handleInvalidEventSignatureException(exception: InvalidEventSignatureException): ProblemDetail {
+        val problemDetail = ProblemDetail.forStatusAndDetail(HttpStatus.UNAUTHORIZED, "Invalid event signature")
+        problemDetail.title = exception.message
+        exceptionLogger.warn(exception.message)
+        return problemDetail
+    }
+
+    @ExceptionHandler(WebhookSecretNotSetException::class)
+    fun handleWebhookSecretNotSetException(exception: WebhookSecretNotSetException): ProblemDetail {
+        val problemDetail = ProblemDetail.forStatusAndDetail(HttpStatus.BAD_REQUEST, "Webhook secret not set")
+        problemDetail.title = exception.message
+        return problemDetail
+    }
+}

src/main/kotlin/net/leanix/githubagent/exceptions/Exceptions.kt

@@ -11,3 +11,5 @@ class UnableToConnectToGitHubEnterpriseException(message: String) : RuntimeExcep
 class JwtTokenNotFound : RuntimeException("JWT token not found")
 class GraphQLApiException(errors: List<GraphQLClientError>) :
     RuntimeException("Errors: ${errors.joinToString(separator = "\n") { it.message }}")
+class WebhookSecretNotSetException : RuntimeException("Webhook secret not set")
+class InvalidEventSignatureException : RuntimeException("Invalid event signature")

src/main/kotlin/net/leanix/githubagent/services/GitHubWebhookHandler.kt

@@ -0,0 +1,46 @@
+package net.leanix.githubagent.services
+
+import net.leanix.githubagent.config.GitHubEnterpriseProperties
+import net.leanix.githubagent.exceptions.InvalidEventSignatureException
+import net.leanix.githubagent.exceptions.WebhookSecretNotSetException
+import net.leanix.githubagent.shared.SUPPORTED_EVENT_TYPES
+import net.leanix.githubagent.shared.hmacSHA256
+import net.leanix.githubagent.shared.timingSafeEqual
+import org.slf4j.LoggerFactory
+import org.springframework.stereotype.Service
+
+@Service
+class GitHubWebhookHandler(
+    private val webhookEventService: WebhookEventService,
+    private val gitHubEnterpriseProperties: GitHubEnterpriseProperties
+) {
+
+    private val logger = LoggerFactory.getLogger(GitHubWebhookHandler::class.java)
+
+    fun handleWebhookEvent(eventType: String, host: String, signature256: String?, payload: String) {
+        if (SUPPORTED_EVENT_TYPES.contains(eventType.uppercase())) {
+            if (!gitHubEnterpriseProperties.baseUrl.contains(host)) {
+                logger.error("Received a webhook event from an unknown host: $host")
+                return
+            }
+            if (gitHubEnterpriseProperties.webhookSecret.isBlank() && signature256 != null) {
+                logger.error(
+                    "Event signature is present but webhook secret is not set, " +
+                        "please restart the agent with a valid secret, " +
+                        "or remove the secret from the GitHub App settings."
+                )
+                throw WebhookSecretNotSetException()
+            }
+            if (gitHubEnterpriseProperties.webhookSecret.isNotBlank() && signature256 != null) {
+                val hashedSecret = hmacSHA256(gitHubEnterpriseProperties.webhookSecret, payload)
+                val isEqual = timingSafeEqual(signature256.removePrefix("sha256="), hashedSecret)
+                if (!isEqual) throw InvalidEventSignatureException()
+            } else {
+                logger.warn("Webhook secret is not set, Skipping signature verification")
+            }
+            webhookEventService.consumeWebhookEvent(eventType, payload)
+        } else {
+            logger.warn("Received an unsupported event of type: $eventType")
+        }
+    }
+}

src/main/kotlin/net/leanix/githubagent/services/WebhookService.kt → src/main/kotlin/net/leanix/githubagent/services/WebhookEventService.kt

@@ -11,15 +11,15 @@ import org.slf4j.LoggerFactory
 import org.springframework.stereotype.Service
 
 @Service
-class WebhookService(
+class WebhookEventService(
     private val webSocketService: WebSocketService,
     private val gitHubGraphQLService: GitHubGraphQLService,
     private val gitHubEnterpriseProperties: GitHubEnterpriseProperties,
     private val cachingService: CachingService,
     private val gitHubAuthenticationService: GitHubAuthenticationService
 ) {
 
-    private val logger = LoggerFactory.getLogger(WebhookService::class.java)
+    private val logger = LoggerFactory.getLogger(WebhookEventService::class.java)
     private val objectMapper = jacksonObjectMapper()
 
     fun consumeWebhookEvent(eventType: String, payload: String) {

src/main/kotlin/net/leanix/githubagent/shared/GitHubWebHookEventHelper.kt

@@ -0,0 +1,24 @@
+package net.leanix.githubagent.shared
+
+import javax.crypto.Mac
+import javax.crypto.spec.SecretKeySpec
+
+fun hmacSHA256(secret: String, data: String): String {
+    val secretKey = SecretKeySpec(secret.toByteArray(), "HmacSHA256")
+    val mac = Mac.getInstance("HmacSHA256")
+    mac.init(secretKey)
+    val hmacData = mac.doFinal(data.toByteArray())
+    return hmacData.joinToString("") { "%02x".format(it) }
+}
+
+fun timingSafeEqual(a: String, b: String): Boolean {
+    val aBytes = a.toByteArray()
+    val bBytes = b.toByteArray()
+    if (aBytes.size != bBytes.size) return false
+
+    var diff = 0
+    for (i in aBytes.indices) {
+        diff = diff or (aBytes[i].toInt() xor bBytes[i].toInt())
+    }
+    return diff == 0
+}

src/main/resources/application.yaml

@@ -3,6 +3,7 @@ github-enterprise:
   githubAppId: ${GITHUB_APP_ID:}
   pemFile: ${PEM_FILE:}
   manifestFileDirectory: ${MANIFEST_FILE_DIRECTORY:}
+  webhookSecret: ${WEBHOOK_SECRET:}
 leanix:
   base-url: https://${LEANIX_DOMAIN}/services
   ws-base-url: wss://${LEANIX_DOMAIN}/services/git-integrations/v1/git-ws

src/test/kotlin/net/leanix/githubagent/controllers/GitHubWebhookControllerTest.kt

@@ -1,10 +1,10 @@
 package net.leanix.githubagent.controllers
 
 import com.ninjasquad.springmockk.MockkBean
-import io.mockk.verify
-import net.leanix.githubagent.services.WebhookService
+import io.mockk.every
+import net.leanix.githubagent.exceptions.WebhookSecretNotSetException
+import net.leanix.githubagent.services.GitHubWebhookHandler
 import org.junit.jupiter.api.Test
-import org.mockito.ArgumentMatchers.anyString
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest
 import org.springframework.test.web.servlet.MockMvc
@@ -18,20 +18,45 @@ class GitHubWebhookControllerTest {
     private lateinit var mockMvc: MockMvc
 
     @MockkBean
-    private lateinit var webhookService: WebhookService
+    private lateinit var gitHubWebhookHandler: GitHubWebhookHandler
 
     @Test
-    fun `should not process not supported events`() {
-        val eventType = "UNSUPPORTED_EVENT"
+    fun `should return 202 if webhook event is processed successfully`() {
+        val eventType = "PUSH"
         val payload = "{}"
+        val host = "valid.host"
+
+        every { gitHubWebhookHandler.handleWebhookEvent(any(), any(), any(), any()) } returns Unit
 
         mockMvc.perform(
             MockMvcRequestBuilders.post("/github/webhook")
                 .header("X-GitHub-Event", eventType)
+                .header("X-GitHub-Enterprise-Host", host)
                 .content(payload)
         )
             .andExpect(MockMvcResultMatchers.status().isAccepted)
+    }
+
+    @Test
+    fun `should return 400 if missing webhook secret when event had signature`() {
+        val eventType = "UNSUPPORTED_EVENT"
+        val payload = "{}"
+        val host = "host"
+        val signature256 = "sha256=invalidsignature"
 
-        verify(exactly = 0) { webhookService.consumeWebhookEvent(anyString(), anyString()) }
+        every {
+            gitHubWebhookHandler.handleWebhookEvent(
+                eventType, host, signature256, payload
+            )
+        } throws WebhookSecretNotSetException()
+
+        mockMvc.perform(
+            MockMvcRequestBuilders.post("/github/webhook")
+                .header("X-GitHub-Event", eventType)
+                .header("X-GitHub-Enterprise-Host", host)
+                .header("X-Hub-Signature-256", signature256)
+                .content(payload)
+        )
+            .andExpect(MockMvcResultMatchers.status().isBadRequest)
     }
 }

src/test/kotlin/net/leanix/githubagent/services/GitHubWebhookHandlerTest.kt

@@ -0,0 +1,72 @@
+package net.leanix.githubagent.services
+
+import io.mockk.every
+import io.mockk.mockk
+import io.mockk.verify
+import net.leanix.githubagent.config.GitHubEnterpriseProperties
+import net.leanix.githubagent.exceptions.InvalidEventSignatureException
+import net.leanix.githubagent.exceptions.WebhookSecretNotSetException
+import org.junit.jupiter.api.BeforeEach
+import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.assertThrows
+
+class GitHubWebhookHandlerTest {
+
+    private val webhookEventService = mockk<WebhookEventService>()
+    private val gitHubEnterpriseProperties = mockk<GitHubEnterpriseProperties>()
+    private val gitHubWebhookHandler = GitHubWebhookHandler(webhookEventService, gitHubEnterpriseProperties)
+
+    @BeforeEach
+    fun setUp() {
+    }
+
+    @Test
+    fun `should not process event if unknown host`() {
+        every { gitHubEnterpriseProperties.baseUrl } returns "known.host"
+
+        gitHubWebhookHandler.handleWebhookEvent("PUSH", "unknown.host", null, "{}")
+
+        verify(exactly = 0) { webhookEventService.consumeWebhookEvent(any(), any()) }
+    }
+
+    @Test
+    fun `should throw WebhookSecretNotSetException when signature is present but secret is not set`() {
+        every { gitHubEnterpriseProperties.baseUrl } returns "known.host"
+        every { gitHubEnterpriseProperties.webhookSecret } returns ""
+
+        assertThrows<WebhookSecretNotSetException> {
+            gitHubWebhookHandler.handleWebhookEvent("PUSH", "known.host", "sha256=signature", "{}")
+        }
+    }
+
+    @Test
+    fun `should throw InvalidEventSignatureException for invalid signature`() {
+        every { gitHubEnterpriseProperties.baseUrl } returns "known.host"
+        every { gitHubEnterpriseProperties.webhookSecret } returns "secret"
+
+        assertThrows<InvalidEventSignatureException> {
+            gitHubWebhookHandler.handleWebhookEvent("PUSH", "known.host", "sha256=signature", "{}")
+        }
+    }
+
+    @Test
+    fun `should not process unsupported event type`() {
+        every { gitHubEnterpriseProperties.baseUrl } returns "known.host"
+        every { gitHubEnterpriseProperties.webhookSecret } returns ""
+
+        gitHubWebhookHandler.handleWebhookEvent("UNSUPPORTED_EVENT", "known.host", null, "{}")
+
+        verify(exactly = 0) { webhookEventService.consumeWebhookEvent(any(), any()) }
+    }
+
+    @Test
+    fun `should process supported event type successfully`() {
+        every { gitHubEnterpriseProperties.baseUrl } returns "host"
+        every { gitHubEnterpriseProperties.webhookSecret } returns ""
+        every { webhookEventService.consumeWebhookEvent(any(), any()) } returns Unit
+
+        gitHubWebhookHandler.handleWebhookEvent("PUSH", "host", null, "{}")
+
+        verify { webhookEventService.consumeWebhookEvent("PUSH", "{}") }
+    }
+}

src/test/kotlin/net/leanix/githubagent/services/WebhookServiceTest.kt → src/test/kotlin/net/leanix/githubagent/services/WebhookEventServiceTest.kt

@@ -13,7 +13,7 @@ import org.springframework.test.context.ActiveProfiles
 
 @SpringBootTest
 @ActiveProfiles("test")
-class WebhookServiceTest {
+class WebhookEventServiceTest {
 
     @MockkBean
     private lateinit var webSocketService: WebSocketService
@@ -28,7 +28,7 @@ class WebhookServiceTest {
     private lateinit var gitHubAuthenticationService: GitHubAuthenticationService
 
     @Autowired
-    private lateinit var webhookService: WebhookService
+    private lateinit var webhookEventService: WebhookEventService
 
     @BeforeEach
     fun setUp() {
@@ -56,7 +56,7 @@ class WebhookServiceTest {
 
         every { cachingService.get("installationToken:1") } returns null andThen "token"
 
-        webhookService.consumeWebhookEvent("PUSH", payload)
+        webhookEventService.consumeWebhookEvent("PUSH", payload)
 
         verify(exactly = 1) { gitHubAuthenticationService.refreshTokens() }
     }
@@ -82,7 +82,7 @@ class WebhookServiceTest {
             "ref": "refs/heads/main"
         }"""
 
-        webhookService.consumeWebhookEvent("PUSH", payload)
+        webhookEventService.consumeWebhookEvent("PUSH", payload)
 
         verify(exactly = 1) {
             webSocketService.sendMessage(
@@ -114,7 +114,7 @@ class WebhookServiceTest {
             "ref": "refs/heads/main"
         }"""
 
-        webhookService.consumeWebhookEvent("OTHER", payload)
+        webhookEventService.consumeWebhookEvent("OTHER", payload)
 
         verify(exactly = 1) { webSocketService.sendMessage("/events/other", payload) }
     }

src/test/resources/application.yaml

@@ -3,6 +3,7 @@ github-enterprise:
   githubAppId: ${GITHUB_APP_ID:dummy}
   pemFile: ${PEM_FILE:dummy}
   manifestFileDirectory: ${MANIFEST_FILE_DIRECTORY:}
+  webhookSecret: ${WEBHOOK_SECRET:}
 leanix:
     base-url: https://${LEANIX_DOMAIN:dummy}/services
     ws-base-url: wss://${LEANIX_DOMAIN:dummy}/services