chore: User REDIS URL instead of HOST and PORT#114
Conversation
| environment("GHAPP_SERVER_PORT", "8083") | ||
| environment("GHAPP_REDIS_HOST", "redis://localhost") | ||
| environment("GHAPP_REDIS_PORT", "6379") | ||
| environment("GHAPP_REDIS_URL", "redis://localhost:6379") |
There was a problem hiding this comment.
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.
| GHAPP_SERVER_PORT=8083 | ||
| GHAPP_REDIS_HOST=redis://redis | ||
| GHAPP_REDIS_PORT=6380 | ||
| GHAPP_REDIS_URL=redis://username:password@host:port |
There was a problem hiding this comment.
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.
| restart: unless-stopped | ||
| ports: | ||
| - "${GHAPP_REDIS_PORT}:${GHAPP_REDIS_PORT}" | ||
| - "6379:6379" |
There was a problem hiding this comment.
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:
- An attacker discovers your service's IP address (via DNS enumeration, nmap scanning, or other reconnaissance)
- The attacker runs
redis-cli -h YOUR_IP_ADDRESS -p 6379to connect to the exposed Redis instance - They can immediately execute arbitrary commands like
FLUSHALL(delete all data),SET(modify data), orCONFIG GET requirepass(check if authentication is enabled) - 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
| - "6379:6379" | |
| - "127.0.0.1:6379:6379" |
View step-by-step instructions
-
Restrict the Redis port binding to localhost so it is not exposed on all network interfaces.
Changeports: - "6379:6379"toports: - "127.0.0.1:6379:6379". -
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 asredis:6379, without publishing the port externally. -
Remove the
portsentry entirely if only theappservice needs to connect to Redis.
In that case, keepdepends_onand use the internal connection stringredis://redis:6379or the equivalent value inGHAPP_REDIS_URL. -
Update any host-based Redis client configuration if needed.
If something on the host connects to Redis, point it to127.0.0.1:6379instead 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.
| "GHAPP_REDIS_PORT", | ||
| "6379" | ||
| "GHAPP_REDIS_URL", | ||
| "redis://localhost:6379" |
There was a problem hiding this comment.
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:
- An attacker positions themselves on the network path between your application and Redis server (via ARP spoofing, BGP hijacking, or compromising network infrastructure).
- The attacker runs a packet sniffer like
tcpdumpto capture traffic:tcpdump -i eth0 'tcp port 6379' - When your application connects to Redis at
redis://localhost:6379, the attacker captures the connection handshake and all subsequent commands. - If the Redis connection includes authentication (the commented example shows
redis://[:password@]host), the attacker captures the plaintext password. - 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
| "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
-
Change the Redis URI scheme from
redis://torediss://anywhere this value is documented, defaulted, or constructed.
Update the examples in the comment fromredis://hostandredis://[:password@]hosttorediss://hostandrediss://[:password@]host. -
Update the default Redis host value to use TLS.
Replace the fallback valueredis://localhostwithrediss://localhostin theGHAPP_REDIS_HOSTconfiguration. -
Keep the environment variable format consistent with the TLS scheme.
If any deployment config,.envvalue, or container setting setsGHAPP_REDIS_HOST, change it to start withrediss://instead ofredis://. -
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. -
Alternatively, if your Redis provider only exposes a separate TLS port, point
GHAPP_REDIS_HOSTand 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.
|
PR Review: Use REDIS URL instead of HOST and PORT Thanks for this cleanup! Overall a focused, well-executed refactor. Replacing the manually-concatenated host:port string with a single connection URL is the right move — it lets Lettuce handle URL parsing (TLS via rediss://, credentials, database selection) instead of reinventing it. I verified the migration is complete and consistent: no stray references to GHAPP_REDIS_HOST / GHAPP_REDIS_PORT / hostKey / portKey remain across source, Helm, docker-compose, README, and the Gradle test config. Code quality
Minor nits (non-blocking)
Security
Test coverage
Nice, low-risk change. The nits above are optional. |
PR Submission Checklist for internal contributors
The PR Title
SQPIT-764The PR Description
What's new in this PR?
Issues
Instead of trying to build the full connection url for redis, we can receive the full connection url directly.
Solutions
Use redis full connection url directly