Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/staging.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: Build
name: Staging

on:
workflow_dispatch:
Expand Down
6 changes: 1 addition & 5 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,8 @@ services:
- WIRE_SDK_API_TOKEN=${WIRE_SDK_API_TOKEN}
- WIRE_SDK_APP_ID=${WIRE_SDK_APP_ID}
- WIRE_SDK_CRYPTOGRAPHY_STORAGE_PASSWORD=${WIRE_SDK_CRYPTOGRAPHY_STORAGE_PASSWORD}
- WIRE_SDK_USER_ID=${WIRE_SDK_USER_ID}
- WIRE_SDK_EMAIL=${WIRE_SDK_EMAIL}
- WIRE_SDK_PASSWORD=${WIRE_SDK_PASSWORD}
- WIRE_SDK_ENVIRONMENT=${WIRE_SDK_ENVIRONMENT}
ports:
- "${GHAPP_SERVER_PORT}:${GHAPP_SERVER_PORT}"
- "${GHAPP_SERVER_PORT:-8083}:${GHAPP_SERVER_PORT:-8083}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Semgrep identified an issue in your code:

The app service maps GHAPP_SERVER_PORT without a host-IP restriction, exposing the GitHub app on all host interfaces. Any network-reachable attacker can scan the host and send requests to the application port.

More details about this

The app service publishes ${GHAPP_SERVER_PORT:-8083} as a host port without a host IP, so Docker binds it to all host interfaces (0.0.0.0). If GHAPP_SERVER_PORT is unset, the application listens at http://<host>:8083; if it is set, the same value controls the externally reachable host port. This makes the GitHub app endpoint accessible from any network that can reach the Docker host, rather than only from the local machine or an explicitly trusted interface.

A plausible attack is:

  1. An attacker scans the host, for example with nmap -p 8083 <host-ip>, and finds the port open because the Compose mapping exposes it on every interface.
  2. The attacker sends requests such as curl http://<host-ip>:8083/ or probes application API routes served by the app container.
  3. The app processes those requests as external traffic; any unauthenticated route, exposed debugging endpoint, or request-handling flaw can then be used remotely to access application data or invoke GitHub integration behavior.
  4. Because GHAPP_SERVER_PORT also comes from the environment, changing that variable can unintentionally move the same service to another publicly reachable port without changing the Compose file.

To resolve this comment:

✨ Commit fix suggestion

Suggested change
- "${GHAPP_SERVER_PORT:-8083}:${GHAPP_SERVER_PORT:-8083}"
- "127.0.0.1:${GHAPP_SERVER_PORT:-8083}:${GHAPP_SERVER_PORT:-8083}"
View step-by-step instructions
  1. Bind the published application port to localhost by changing the mapping to 127.0.0.1:${GHAPP_SERVER_PORT:-8083}:${GHAPP_SERVER_PORT:-8083}.
  2. Alternatively, if the application does not need host access, remove the ports entry and let other Compose services access it through the internal network.
  3. Remove Redis’s ports entry unless Redis must be accessed from the host. The app service can reach Redis using redis:6379 without publishing Redis externally.
  4. Alternatively, if external access is required, replace 127.0.0.1 with the specific trusted host interface address and restrict access with firewall rules. Binding without a host address exposes the port on all host interfaces.
💬 Ignore this finding

Reply with Semgrep commands to ignore this finding.

  • /fp <comment> for false positive
  • /ar <comment> for acceptable risk
  • /other <comment> for all other reasons

Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by port-all-interfaces.

You can view more details about this finding in the Semgrep AppSec Platform.

volumes:
- github-app:/app
depends_on:
Expand Down
6 changes: 0 additions & 6 deletions helm/githubapp/templates/NOTES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,6 @@ MONITORING & DEBUGGING:

CONFIGURATION:
{{- range .Values.env }}
{{- if eq .name "WIRE_SDK_ENVIRONMENT" }}
- Wire SDK Environment: {{ .value | default "Not configured" }}
{{- end }}
{{- if eq .name "WIRE_ENV" }}
- Wire Environment: {{ .value | default "Not configured" }}
{{- end }}
{{- if eq .name "PORT" }}
- Application Port: {{ .value | default "8080" }}
{{- end }}
Expand Down
10 changes: 8 additions & 2 deletions src/main/kotlin/com/wire/github/Routing.kt
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import io.ktor.server.routing.post
import io.ktor.server.routing.routing
import java.util.UUID
import kotlinx.serialization.ExperimentalSerializationApi
import kotlinx.serialization.SerializationException
Comment thread
bbaarriiss marked this conversation as resolved.
import org.koin.core.context.GlobalContext

@Suppress("LongMethod")
Expand All @@ -37,7 +38,7 @@ fun Application.configureRouting() {

routing {
trace {
application.log.info(it.buildText())
application.log.debug(it.buildText())
}

get("/health") {
Expand Down Expand Up @@ -84,7 +85,12 @@ fun Application.configureRouting() {
)
}

val response = KtxSerializer.json.decodeFromString<GitHubResponse>(payload)
val response = try {
KtxSerializer.json.decodeFromString<GitHubResponse>(payload)
} catch (exception: SerializationException) {
application.log.error("Failed to deserialize $event delivery $delivery", exception)
return@post call.response.status(HttpStatusCode.BadRequest)
}

// Handle event response and send message
val messageTemplate = templateHandler.handleEvent(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import kotlinx.serialization.Serializable

@Serializable
data class Comment(
val body: String,
val body: String? = null,
Comment thread
bbaarriiss marked this conversation as resolved.
val user: User,
@SerialName("html_url")
val htmlUrl: String,
Expand Down
2 changes: 1 addition & 1 deletion src/main/kotlin/com/wire/github/response/model/Issue.kt
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ data class Issue(
@SerialName("html_url")
val htmlUrl: String,
val title: String,
val body: String,
val body: String? = null,
val user: User,
val number: Int
)
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ data class PullRequest(
@SerialName("html_url")
val htmlUrl: String,
val title: String,
val body: String,
val body: String? = null,
val user: User,
val merged: Boolean,
val merged: Boolean? = null,
val number: Int
)
13 changes: 10 additions & 3 deletions src/main/kotlin/com/wire/github/response/model/Review.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,14 @@ import kotlinx.serialization.Serializable

@Serializable
data class Review(
val body: String,
val user: User,
val body: String? = null,
val user: User? = null,
val state: String
)
) {
val emoji: String
get() = when (state) {
"approved" -> "✅"
"changes_requested" -> "🔄"
else -> "📝"
}
}
4 changes: 3 additions & 1 deletion src/main/kotlin/com/wire/github/util/TemplateHandler.kt
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,14 @@ class TemplateHandler {
event = event,
action = response.action
)
logger.info("Template found for this action: ${template.name}")

populateTemplate(
mustache = template,
model = response
)
} catch (exception: MustacheNotFoundException) {
logger.error("MustacheNotFoundException: $exception")
logger.info("MustacheNotFoundException: $exception")
null
}

Expand Down Expand Up @@ -62,6 +63,7 @@ class TemplateHandler {
.apply {
mustache.execute(PrintWriter(this), model).flush()
}.toString()
.takeIf { it.isNotBlank() }

private companion object {
const val LANGUAGE_ENGLISH = "en"
Expand Down

This file was deleted.

11 changes: 5 additions & 6 deletions src/main/resources/templates/en/issue_comment.created.template
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
**[{{repository.fullName}}]** New comment by **{{sender.login}}** on issue **{{{issue.title}}}**

{{{comment.body}}}

[comment]({{comment.htmlUrl}})
📝 **New comment** on PR/issue **{{{issue.title}}}** by **{{sender.login}}**
**Repository:** {{repository.fullName}}
**PR/issue:** [#{{issue.number}}]({{issue.htmlUrl}}) - {{issue.title}}
**Comment:** [link]({{comment.htmlUrl}})
**Text:** {{{comment.body}}}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
🟢 **New Pull Request Ready for Review!** by **{{sender.login}}**
**Repository:** {{repository.fullName}}
**PR:** [#{{pullRequest.number}}]({{pullRequest.htmlUrl}}) - {{pullRequest.title}}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
🟢 **New Pull Request Reopened!** by **{{sender.login}}**
**Repository:** {{repository.fullName}}
**PR:** [#{{pullRequest.number}}]({{pullRequest.htmlUrl}}) - {{pullRequest.title}}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{{#review.body}}**[{{repository.fullName}}]** Pull request review **{{pullRequest.title}}** was {{action}} by **{{review.user.login}}**

{{{review.body}}}

[pull request]({{pullRequest.htmlUrl}})
{{/review.body}}
{{#review.body}}
{{review.emoji}} **Pull request** update: **{{review.state}}** by **{{review.user.login}}**
**Repository:** {{repository.fullName}}
**PR:** [#{{pullRequest.number}}]({{pullRequest.htmlUrl}}) - {{pullRequest.title}}
**Text:** {{{review.body}}}
{{/review.body}}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
**[{{repository.fullName}}]** Pull request review **{{pullRequest.title}}** was commented by **{{comment.user.login}}**

{{{comment.body}}}

[comment]({{comment.htmlUrl}})
📝 **Pull request** was **commented** by **{{comment.user.login}}**
**Repository:** {{repository.fullName}}
**PR:** [#{{pullRequest.number}}]({{pullRequest.htmlUrl}}) - {{pullRequest.title}}
**Comment:** [link]({{comment.htmlUrl}})
**Text:** {{{comment.body}}}
7 changes: 3 additions & 4 deletions src/main/resources/templates/en/push.template
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
{{^commits.isEmpty}}
**[{{repository.fullName}}]** Push by **{{sender.login}}**

🫸 **Push** by **{{sender.login}}**
**Repository:** {{repository.fullName}}
{{#commits}}
- {{{message}}}
{{/commits}}

[compare]({{compare}})
**Compare:** [link]({{compare}})
—{{/commits.isEmpty}}
127 changes: 127 additions & 0 deletions src/test/kotlin/com/wire/github/util/TemplateHandlerTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
package com.wire.github.util

import com.wire.github.response.model.Commit
import com.wire.github.response.model.GitHubResponse
import com.wire.github.response.model.PullRequest
import com.wire.github.response.model.Repository
import com.wire.github.response.model.Review
import com.wire.github.response.model.User
import kotlin.test.Test
import kotlin.test.assertContains
import kotlin.test.assertNull

class TemplateHandlerTest {
private val templateHandler = TemplateHandler()

@Test
fun `does not render a submitted pull request review without a body`() {
val message = templateHandler.handleEvent(
event = "pull_request_review",
response = reviewResponse(body = null)
)

assertNull(message)
}

@Test
fun `does not render a submitted pull request review with a blank body`() {
val message = templateHandler.handleEvent(
event = "pull_request_review",
response = reviewResponse(body = "")
)

assertNull(message)
}

@Test
fun `renders a submitted pull request review with a body`() {
val message = templateHandler.handleEvent(
event = "pull_request_review",
response = reviewResponse(body = "Looks good")
)

assertContains(message.orEmpty(), "Looks good")
assertContains(message.orEmpty(), "✅")
}

@Test
fun `renders a note emoji for a commented pull request review`() {
val message = templateHandler.handleEvent(
event = "pull_request_review",
response = reviewResponse(body = "A comment", state = "commented")
)

assertContains(message.orEmpty(), "📝")
}

@Test
fun `renders a change emoji for a pull request review with requested changes`() {
val message = templateHandler.handleEvent(
event = "pull_request_review",
response = reviewResponse(body = "Please update this", state = "changes_requested")
)

assertContains(message.orEmpty(), "🔄")
}

@Test
fun `does not render a push with no commits`() {
val message = templateHandler.handleEvent(
event = "push",
response = pushResponse(commits = emptyList())
)

assertNull(message)
}

@Test
fun `renders a push with commits`() {
val message = templateHandler.handleEvent(
event = "push",
response = pushResponse(commits = listOf(Commit(message = "Add feature")))
)

assertContains(message.orEmpty(), "Add feature")
}

private fun reviewResponse(
body: String?,
state: String = "approved"
) = GitHubResponse(
action = "submitted",
pullRequest = PullRequest(
htmlUrl = "https://github.com/wire/example/pull/1",
title = "Example pull request",
user = user,
number = 1
),
review = Review(
body = body,
user = user,
state = state
),
sender = user,
repository = Repository(
fullName = "wire/example",
name = "example"
)
)

private fun pushResponse(commits: List<Commit>) =
GitHubResponse(
commits = commits,
sender = user,
compare = "https://github.com/wire/example/compare/main",
repository = Repository(
fullName = "wire/example",
name = "example"
)
)

private companion object {
val user = User(
avatarUrl = "https://github.com/wire.png",
login = "wire"
)
}
}
Loading