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
3 changes: 1 addition & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@ An example of this project environment variables:
```
GHAPP_API_HOST=https://127.0.0.1/github
GHAPP_SERVER_PORT=8083
GHAPP_REDIS_HOST=redis://redis
GHAPP_REDIS_PORT=6380
GHAPP_REDIS_URL=redis://username:password@host:port

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Semgrep identified an issue, but thinks it may be safe to ignore.
Found unencrypted Redis connection, prefer TLS encrypted rediss:// transport

Why this might be safe to ignore:

This appears in README example environment variables, not executable application code, so it is documentation for local setup rather than an actual network connection implementation. While the example uses unencrypted Redis, changing docs may be nice, but this finding does not meaningfully improve production security by itself.

To resolve this comment:

🔧 No guidance has been designated for this issue. Fix according to your organization's approved methods.

💬 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 redis-unencrypted-transport.

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

WIRE_SDK_API_HOST=https://nginz-https.chala.wire.link
WIRE_SDK_API_TOKEN=myApiToken
WIRE_SDK_APP_ID=f562e146-dec2-4d85-93c7-7132746b5cca
Expand Down
3 changes: 1 addition & 2 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,7 @@ tasks {
test {
environment("GHAPP_API_HOST", "http://0.0.0.0")
environment("GHAPP_SERVER_PORT", "8083")
environment("GHAPP_REDIS_HOST", "redis://localhost")
environment("GHAPP_REDIS_PORT", "6379")
environment("GHAPP_REDIS_URL", "redis://localhost:6379")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Semgrep identified an issue, but thinks it may be safe to ignore.
Found unencrypted Redis connection, prefer TLS encrypted rediss:// transport

Why this might be safe to ignore:

This match is in the Gradle test task and is explicitly labeled as dummy environment variables for tests, so it is not a production Redis connection. The rule intent is valid for real runtime configuration, but here it matched non-deployed test setup where changing to TLS would not meaningfully improve security.

To resolve this comment:

🔧 No guidance has been designated for this issue. Fix according to your organization's approved methods.

💬 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 redis-unencrypted-transport.

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

environment("WIRE_SDK_API_HOST", "https://nginz-https.chala.wire.link")
environment("WIRE_SDK_API_TOKEN", "myApiToken")
environment("WIRE_SDK_APP_ID", "f562e146-dec2-4d85-93c7-7132746b5cca")
Expand Down
9 changes: 3 additions & 6 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ services:
environment:
- GHAPP_API_HOST=${GHAPP_API_HOST}
- GHAPP_SERVER_PORT=${GHAPP_SERVER_PORT}
- GHAPP_REDIS_HOST=${GHAPP_REDIS_HOST}
- GHAPP_REDIS_PORT=${GHAPP_REDIS_PORT}
- GHAPP_REDIS_URL=${GHAPP_REDIS_URL}
- WIRE_SDK_API_HOST=${WIRE_SDK_API_HOST}
- WIRE_SDK_API_TOKEN=${WIRE_SDK_API_TOKEN}
- WIRE_SDK_APP_ID=${WIRE_SDK_APP_ID}
Expand All @@ -28,14 +27,12 @@ services:
redis:
image: redis:7
container_name: github_redis
environment:
- GHAPP_REDIS_PORT=${GHAPP_REDIS_PORT}
restart: unless-stopped
ports:
- "${GHAPP_REDIS_PORT}:${GHAPP_REDIS_PORT}"
- "6379:6379"

Copy link
Copy Markdown

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:

Redis port 6379 is exposed on all network interfaces (0.0.0.0), allowing unauthenticated network access to your database.

More details about this

The Redis service is bound to 0.0.0.0:6379, making it accessible from any network interface including the internet. An attacker can connect directly to this port and interact with Redis without authentication (Redis has no authentication by default when not configured).

Here's an exploitation scenario:

  1. An attacker discovers your service's IP address (via DNS enumeration, nmap scanning, or other reconnaissance)
  2. The attacker runs redis-cli -h YOUR_IP_ADDRESS -p 6379 to connect to the exposed Redis instance
  3. They can immediately execute arbitrary commands like FLUSHALL (delete all data), SET (modify data), or CONFIG GET requirepass (check if authentication is enabled)
  4. If your application stores session data, user credentials, or other sensitive information in Redis, the attacker gains direct access without going through your application layer

This port mapping exposes the entire redis service to the network when it should only be accessible internally to the app container.

To resolve this comment:

✨ Commit fix suggestion

Suggested change
- "6379:6379"
- "127.0.0.1:6379:6379"
View step-by-step instructions
  1. Restrict the Redis port binding to localhost so it is not exposed on all network interfaces.
    Change ports: - "6379:6379" to ports: - "127.0.0.1:6379:6379".

  2. Keep the port mapping only if Redis must be reachable from the host machine.
    Docker services on the same Compose network can already reach Redis by service name, such as redis:6379, without publishing the port externally.

  3. Remove the ports entry entirely if only the app service needs to connect to Redis.
    In that case, keep depends_on and use the internal connection string redis://redis:6379 or the equivalent value in GHAPP_REDIS_URL.

  4. Update any host-based Redis client configuration if needed.
    If something on the host connects to Redis, point it to 127.0.0.1:6379 instead of a non-local interface.

💬 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:
- redis-data:/data
command: sh -c "redis-server --port $GHAPP_REDIS_PORT --save 60 1 --loglevel warning --appendonly yes"
command: sh -c "redis-server --save 60 1 --loglevel warning --appendonly yes"

volumes:
redis-data:
Expand Down
9 changes: 2 additions & 7 deletions helm/githubapp/templates/statefulset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -68,16 +68,11 @@ spec:
key: WIRE_SDK_CRYPTOGRAPHY_STORAGE_PASSWORD
{{- end }}
{{- if .Values.redis }}
- name: GHAPP_REDIS_HOST
- name: GHAPP_REDIS_URL
valueFrom:
secretKeyRef:
name: {{ .Values.redis.secretName }}
key: {{ .Values.redis.hostKey }}
- name: GHAPP_REDIS_PORT
valueFrom:
secretKeyRef:
name: {{ .Values.redis.secretName }}
key: {{ .Values.redis.portKey }}
key: {{ .Values.redis.urlKey }}
{{- end }}
volumeMounts:
- name: data
Expand Down
3 changes: 1 addition & 2 deletions helm/githubapp/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ secrets:

redis:
secretName: "githubapp-valkey-secrets"
hostKey: "host"
portKey: "port"
urlKey: "url"

# Persistent storage configuration
persistence:
Expand Down
5 changes: 2 additions & 3 deletions src/main/kotlin/com/wire/github/config/Modules.kt
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ import com.wire.github.util.ENV_VAR_API_HOST
import com.wire.github.util.ENV_VAR_API_TOKEN
import com.wire.github.util.ENV_VAR_APPLICATION_ID
import com.wire.github.util.ENV_VAR_CRYPTOGRAPHY_STORAGE_KEY
import com.wire.github.util.ENV_VAR_REDIS_HOST
import com.wire.github.util.ENV_VAR_REDIS_PORT
import com.wire.github.util.ENV_VAR_REDIS_URL
import com.wire.github.util.SignatureValidator
import com.wire.github.util.TemplateHandler
import com.wire.sdk.WireAppSdk
Expand All @@ -23,7 +22,7 @@ val projectModules = module {
}
single { SignatureValidator() }
single { TemplateHandler() }
single { RedisClient.create("$ENV_VAR_REDIS_HOST:$ENV_VAR_REDIS_PORT") }
single { RedisClient.create(ENV_VAR_REDIS_URL) }
single<StatefulRedisConnection<String, String>> { get<RedisClient>().connect() }
}

Expand Down
25 changes: 6 additions & 19 deletions src/main/kotlin/com/wire/github/util/EnvironmentVariables.kt
Original file line number Diff line number Diff line change
Expand Up @@ -33,28 +33,15 @@ val ENV_VAR_HOST: String = System
)

/**
* Redis Host URL
* In case it needs a password, must be included in this same environment variable.
* Examples:
* - "redis://host"
* - "redis://[:password@]host"
* Redis Connection URL
* Should contain all the necessary information
* Example: rediss://username:password@host:port
*/
val ENV_VAR_REDIS_HOST: String = System
val ENV_VAR_REDIS_URL: String = System
.getenv()
.getOrDefault(
"GHAPP_REDIS_HOST",
"redis://localhost"
)

/**
* Redis Port Number
* To used when connecting and also exposed via docker-compose.
*/
val ENV_VAR_REDIS_PORT: String = System
.getenv()
.getOrDefault(
"GHAPP_REDIS_PORT",
"6379"
"GHAPP_REDIS_URL",
"redis://localhost:6379"

Copy link
Copy Markdown

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:

Unencrypted Redis connection using redis:// exposes all traffic (including passwords and data) to network eavesdropping attacks.

More details about this

The Redis connection URL uses the unencrypted redis:// protocol instead of the TLS-encrypted rediss:// protocol. This means all data transmitted between your application and Redis—including commands, keys, and values—is sent in plaintext over the network.

Exploit scenario:

  1. An attacker positions themselves on the network path between your application and Redis server (via ARP spoofing, BGP hijacking, or compromising network infrastructure).
  2. The attacker runs a packet sniffer like tcpdump to capture traffic: tcpdump -i eth0 'tcp port 6379'
  3. When your application connects to Redis at redis://localhost:6379, the attacker captures the connection handshake and all subsequent commands.
  4. If the Redis connection includes authentication (the commented example shows redis://[:password@]host), the attacker captures the plaintext password.
  5. The attacker can now read sensitive data stored in Redis (user sessions, cache, tokens) or execute commands to inject malicious data into your application's cache, causing data corruption or unauthorized access.

The lack of TLS encryption in ENV_VAR_REDIS_HOST leaves the entire Redis communication channel exposed to network-level attacks.

To resolve this comment:

✨ Commit fix suggestion

Suggested change
"redis://localhost:6379"
package com.wire.github.util
import java.util.UUID
/**
* Host Port to be used when setting up Ktor server.
*/
val ENV_VAR_PORT: Int = System
.getenv()
.getOrDefault(
"GHAPP_SERVER_PORT",
"8083"
).toInt()
/**
* Host URL to be used to contact via webhook.
* Must contain the protocol (`HTTP` or `HTTPS`) ???
* Note: This is not the Host URL for the Ktor server in the docker container.
*/
val ENV_VAR_HOST: String = System
.getenv()
.getOrDefault(
"GHAPP_API_HOST",
"http://0.0.0.0"
)
/**
* Redis Host URL
* In case it needs a password, must be included in this same environment variable.
* Examples:
* - "rediss://host"
* - "rediss://[:password@]host"
* Note: The configured Redis endpoint must support TLS when using `rediss://`.
*/
val ENV_VAR_REDIS_HOST: String = System
.getenv()
.getOrDefault(
"GHAPP_REDIS_HOST",
"rediss://localhost"
)
/**
* Redis Port Number
* To used when connecting and also exposed via docker-compose.
*/
val ENV_VAR_REDIS_PORT: String = System
.getenv()
.getOrDefault(
"GHAPP_REDIS_PORT",
"6379"
)
/**
* Application ID received when Onboarding the App.
*/
val ENV_VAR_APPLICATION_ID: UUID = UUID.fromString(
System
.getenv()
.getOrDefault(
"WIRE_SDK_APP_ID",
UUID.randomUUID().toString()
)
View step-by-step instructions
  1. Change the Redis URI scheme from redis:// to rediss:// anywhere this value is documented, defaulted, or constructed.
    Update the examples in the comment from redis://host and redis://[:password@]host to rediss://host and rediss://[:password@]host.

  2. Update the default Redis host value to use TLS.
    Replace the fallback value redis://localhost with rediss://localhost in the GHAPP_REDIS_HOST configuration.

  3. Keep the environment variable format consistent with the TLS scheme.
    If any deployment config, .env value, or container setting sets GHAPP_REDIS_HOST, change it to start with rediss:// instead of redis://.

  4. Confirm the Redis server accepts TLS connections on the configured endpoint.
    rediss:// enables encrypted transport, so the target Redis instance must have TLS enabled or be fronted by a TLS-terminating proxy.

  5. Alternatively, if your Redis provider only exposes a separate TLS port, point GHAPP_REDIS_HOST and the Redis port setting to that TLS-enabled endpoint instead of the plaintext one.

💬 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 redis-unencrypted-transport.

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

)

/**
Expand Down