Conversation
Contributor
WalkthroughAdds strict Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Traefik as Traefik
participant App as App Container
participant Disk as Host FS (configs/certs)
participant CF as Cloudflare (origin cert source)
Client->>Traefik: HTTPS request
Traefik->>Disk: Read `traefik.toml` and `tls.toml` (cert/key)
Disk-->>Traefik: Cloudflare origin cert & key (written from `.env.setup`)
Traefik->>App: Forward request over internal network
App-->>Traefik: Response
Traefik-->>Client: TLS-terminated response
Note over Disk,CF: Certs validated and stored on host by setup script
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@setup.sh`:
- Around line 185-186: The script calls "docker compose -f
\"$TRAEFIK_CONFIG_DIR/compose.yaml\" ..." without verifying the Compose plugin
is available; add a preflight check before these calls that ensures the Compose
plugin exists (e.g. run a harmless probe like "docker compose version" or check
for the legacy "docker-compose" binary) and handle absence by printing a clear
error and exiting; update the block that runs the compose commands (the two
lines invoking docker compose against $TRAEFIK_CONFIG_DIR/compose.yaml) to only
execute after this check, and if desired, attempt a fallback to "docker-compose"
when present or fail fast with an explanatory message.
- Around line 121-124: The script currently uses echo to write CF_ORIGIN_CERT
and CF_ORIGIN_KEY into origin.crt and origin.key, which preserves literal "\n"
sequences; change the writes to use printf '%b' so escape sequences are
interpreted (e.g., write CF_ORIGIN_CERT with printf '%b' "$CF_ORIGIN_CERT" to
the origin.crt file and similarly for CF_ORIGIN_KEY to origin.key) and keep the
existing chmod on origin.crt and origin.key; update the commands that reference
CF_ORIGIN_CERT, CF_ORIGIN_KEY, origin.crt, and origin.key.
github-project-automation
bot
moved this from Triage
to Development
in OpenFront Release Management
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
setup.sh (2)
123-124:⚠️ Potential issue | 🟠 MajorUse
printf '%b'for cert/key writes to avoid invalid PEM output.Line 123 and Line 124 still use
echo. If values contain escaped\n, Traefik gets broken certificate files.Suggested fix
-echo "$CF_ORIGIN_CERT" > "$TRAEFIK_CERTS_DIR/origin.crt" -echo "$CF_ORIGIN_KEY" > "$TRAEFIK_CERTS_DIR/origin.key" +printf '%b' "$CF_ORIGIN_CERT" > "$TRAEFIK_CERTS_DIR/origin.crt" +printf '%b' "$CF_ORIGIN_KEY" > "$TRAEFIK_CERTS_DIR/origin.key"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setup.sh` around lines 123 - 124, The current writes use echo which can misinterpret escaped newlines and corrupt PEM files; change the writes that create origin.crt and origin.key to use printf '%b' to preserve embedded `\n` sequences from the CF_ORIGIN_CERT and CF_ORIGIN_KEY variables and redirect the output into "$TRAEFIK_CERTS_DIR/origin.crt" and "$TRAEFIK_CERTS_DIR/origin.key" respectively so Traefik receives valid PEM files.
186-187:⚠️ Potential issue | 🟠 MajorAdd a Compose preflight check (or fallback) before running commands.
Line 186 and Line 187 call
docker composedirectly. Systems can have Docker Engine installed but no Compose plugin.Suggested fix
+if docker compose version >/dev/null 2>&1; then + COMPOSE_CMD=(docker compose) +elif command -v docker-compose >/dev/null 2>&1; then + COMPOSE_CMD=(docker-compose) +else + echo "❌ Docker Compose is required but not available." + exit 1 +fi + -docker compose -f "$TRAEFIK_CONFIG_DIR/compose.yaml" pull -docker compose -f "$TRAEFIK_CONFIG_DIR/compose.yaml" up -d +"${COMPOSE_CMD[@]}" -f "$TRAEFIK_CONFIG_DIR/compose.yaml" pull +"${COMPOSE_CMD[@]}" -f "$TRAEFIK_CONFIG_DIR/compose.yaml" up -d#!/bin/bash # Verify whether setup.sh already has a Compose availability check and fallback rg -n 'docker compose version|docker-compose version|COMPOSE_CMD' setup.sh rg -n 'docker compose -f "\$TRAEFIK_CONFIG_DIR/compose.yaml" (pull|up -d)' setup.sh🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setup.sh` around lines 186 - 187, Add a Compose preflight check and fallback before invoking the hardcoded docker compose calls: detect whether the Docker Compose V2 plugin is available (e.g., "docker compose version") or the standalone binary "docker-compose", set a COMPOSE_CMD variable accordingly, and replace the direct calls to "docker compose -f \"$TRAEFIK_CONFIG_DIR/compose.yaml\" pull" and "docker compose -f \"$TRAEFIK_CONFIG_DIR/compose.yaml\" up -d" to use "$COMPOSE_CMD -f \"$TRAEFIK_CONFIG_DIR/compose.yaml\" pull" and "$COMPOSE_CMD -f \"$TRAEFIK_CONFIG_DIR/compose.yaml\" up -d"; ensure the script exits with an error if neither command is found.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@setup.sh`:
- Line 184: The chown -R openfront:openfront "$TRAEFIK_CONFIG_DIR" command is
recursively changing ownership of the certs directory (including origin.key) and
must be avoided; instead, change ownership only for non-sensitive traefik config
files and directories under TRAEFIK_CONFIG_DIR while leaving certs (and
origin.key) owned by root with strict permissions. Update the script to stop
using a blanket recursive chown on TRAEFIK_CONFIG_DIR (the chown -R
openfront:openfront "$TRAEFIK_CONFIG_DIR" line), and implement one of:
explicitly chown known safe files/dirs only, or perform a recursive chown that
excludes the certs subdirectory, and separately ensure certs/origin.key remains
owned by root with 600 permissions.
- Around line 12-17: The script currently sources ENV_FILE directly which can
lead to privilege escalation; before the existing source "$ENV_FILE" call add
validation that ENV_FILE is a regular file, is not a symlink, is owned by root
(use stat %U or %u to check ownership) and is not writable by group/others (use
stat %a or %A to assert no group/other write bits), and fail/exit with an error
if any check fails; update the block around ENV_FILE and the source invocation
to perform these checks and only source when they pass.
- Around line 189-193: The current checks use substring grep (docker ps | grep
-q traefik) and can falsely match names like my-traefik; replace the pipeline
with a precise name check using docker ps --format '{{.Names}}' and exact-match
grep (e.g., docker ps --format '{{.Names}}' | grep -xq 'traefik') inside the
existing conditional for traefik, and apply the same change to the other checks
that use node-exporter and otel-collector (replace their docker ps | grep -q
lines with docker ps --format '{{.Names}}' | grep -xq 'node-exporter' and docker
ps --format '{{.Names}}' | grep -xq 'otel-collector' respectively) so the script
only succeeds for exact container names.
---
Duplicate comments:
In `@setup.sh`:
- Around line 123-124: The current writes use echo which can misinterpret
escaped newlines and corrupt PEM files; change the writes that create origin.crt
and origin.key to use printf '%b' to preserve embedded `\n` sequences from the
CF_ORIGIN_CERT and CF_ORIGIN_KEY variables and redirect the output into
"$TRAEFIK_CERTS_DIR/origin.crt" and "$TRAEFIK_CERTS_DIR/origin.key" respectively
so Traefik receives valid PEM files.
- Around line 186-187: Add a Compose preflight check and fallback before
invoking the hardcoded docker compose calls: detect whether the Docker Compose
V2 plugin is available (e.g., "docker compose version") or the standalone binary
"docker-compose", set a COMPOSE_CMD variable accordingly, and replace the direct
calls to "docker compose -f \"$TRAEFIK_CONFIG_DIR/compose.yaml\" pull" and
"docker compose -f \"$TRAEFIK_CONFIG_DIR/compose.yaml\" up -d" to use
"$COMPOSE_CMD -f \"$TRAEFIK_CONFIG_DIR/compose.yaml\" pull" and "$COMPOSE_CMD -f
\"$TRAEFIK_CONFIG_DIR/compose.yaml\" up -d"; ensure the script exits with an
error if neither command is found.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c7d9a690-9e82-4eb7-a03a-db12c9374e78
📒 Files selected for processing (2)
setup.shupdate.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- update.sh
github-project-automation
bot
moved this from Development
to Final Review
in OpenFront Release Management
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (5)
setup.sh (5)
123-124:⚠️ Potential issue | 🟠 MajorWrite PEM values with
printf '%b', notecho.Line 123–124 can write literal
\ninto cert/key files and produce invalid PEM content when env values are escaped. Useprintf '%b'.Suggested patch
-echo "$CF_ORIGIN_CERT" > "$TRAEFIK_CERTS_DIR/origin.crt" -echo "$CF_ORIGIN_KEY" > "$TRAEFIK_CERTS_DIR/origin.key" +printf '%b' "$CF_ORIGIN_CERT" > "$TRAEFIK_CERTS_DIR/origin.crt" +printf '%b' "$CF_ORIGIN_KEY" > "$TRAEFIK_CERTS_DIR/origin.key"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setup.sh` around lines 123 - 124, The script uses echo to write environment PEM variables which can leave literal escape sequences (e.g. "\n") in the output; replace the two echo commands that write CF_ORIGIN_CERT and CF_ORIGIN_KEY into "$TRAEFIK_CERTS_DIR/origin.crt" and "$TRAEFIK_CERTS_DIR/origin.key" with printf '%b' to correctly expand backslash-escaped newlines and produce valid PEM files while keeping the same redirection.
187-188:⚠️ Potential issue | 🟠 MajorAvoid recursive
chownover the cert subtree.Line 187 still applies recursive ownership changes to the whole Traefik directory, including sensitive cert paths. Please chown only non-sensitive config files and keep cert material root-owned.
Suggested patch
-chown -R openfront:openfront "$TRAEFIK_CONFIG_DIR" -chown root:root "$TRAEFIK_CERTS_DIR" "$TRAEFIK_CERTS_DIR/origin.crt" "$TRAEFIK_CERTS_DIR/origin.key" +chown openfront:openfront \ + "$TRAEFIK_CONFIG_DIR/traefik.toml" \ + "$TRAEFIK_CONFIG_DIR/tls.toml" \ + "$TRAEFIK_CONFIG_DIR/compose.yaml" +chown root:root "$TRAEFIK_CERTS_DIR" "$TRAEFIK_CERTS_DIR/origin.crt" "$TRAEFIK_CERTS_DIR/origin.key" +chmod 700 "$TRAEFIK_CERTS_DIR"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setup.sh` around lines 187 - 188, The recursive chown on TRAEFIK_CONFIG_DIR is changing ownership of the cert subtree; change the ownership logic to avoid -R over the entire "$TRAEFIK_CONFIG_DIR" and instead chown only non-sensitive config files or specific subdirectories (e.g., config templates, dynamic config files) while leaving "$TRAEFIK_CERTS_DIR" and its files root-owned; update the lines that call chown (the commands referencing TRAEFIK_CONFIG_DIR and TRAEFIK_CERTS_DIR) to limit recursion so cert material remains owned by root and only intended config files are assigned to openfront.
190-191:⚠️ Potential issue | 🟠 MajorAdd a preflight check for
docker composeavailability.Line 190–191 call
docker composedirectly. If the compose plugin is missing, setup fails late. Add a clear preflight check and fail fast.Suggested patch
+if ! docker compose version >/dev/null 2>&1; then + echo "❌ docker compose plugin is required but not available." + exit 1 +fi + docker compose -f "$TRAEFIK_CONFIG_DIR/compose.yaml" pull docker compose -f "$TRAEFIK_CONFIG_DIR/compose.yaml" up -d🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setup.sh` around lines 190 - 191, Add a preflight check that verifies the `docker compose` command is available before running the two calls that use "$TRAEFIK_CONFIG_DIR/compose.yaml" (the existing `docker compose -f ... pull` and `docker compose -f ... up -d` lines); if the check fails, print a clear error message asking the user to install the Docker Compose plugin (or provide guidance for installing the legacy `docker-compose`), and exit non‑zero so the script fails fast instead of erroring later. Use a simple command existence/version check for `docker compose` and short-circuit the script with a descriptive message and exit code when not present.
193-193:⚠️ Potential issue | 🟡 MinorUse exact container-name matching in the Traefik running check.
Line 193 uses substring grep and can match wrong containers (for example,
my-traefik). Use exact-name filtering.Suggested patch
-if docker ps | grep -q traefik; then +if docker ps --filter "name=^/traefik$" --filter "status=running" --format '{{.Names}}' | grep -xq 'traefik'; then🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setup.sh` at line 193, The check uses substring grep and can match wrong container names; replace the line that calls "docker ps | grep -q traefik" with an exact-name check such as piping docker ps --format '{{.Names}}' into grep -xq 'traefik' (i.e. use the existing shell test around the command that currently contains "docker ps | grep -q traefik" to run "docker ps --format '{{.Names}}' | grep -xq 'traefik'") so only a container named exactly "traefik" will match.
12-17:⚠️ Potential issue | 🟠 MajorHarden
.env.setupchecks beforesource.Line 12–17 still sources a file as root with only
-fvalidation. Please enforce regular-file, non-symlink, root-owner, and non-group/other-writable checks beforesource.Suggested patch
if [ -f "$ENV_FILE" ]; then echo "📂 Loading environment from $ENV_FILE" + if [ -L "$ENV_FILE" ]; then + echo "❌ $ENV_FILE must not be a symlink" + exit 1 + fi + owner_uid="$(stat -c '%u' "$ENV_FILE")" + perms="$(stat -c '%a' "$ENV_FILE")" + if [ "$owner_uid" -ne 0 ] || [ $((8#$perms & 022)) -ne 0 ]; then + echo "❌ $ENV_FILE must be root-owned and not writable by group/others" + exit 1 + fi set -a # shellcheck source=/dev/null source "$ENV_FILE"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setup.sh` around lines 12 - 17, The block that sources ENV_FILE must first validate it: ensure ENV_FILE is a regular file and not a symlink (use -f and ! -L), verify it is owned by root (stat -c %u "$ENV_FILE" == 0), and ensure it is not group- or other-writable (check file mode via stat -c %a and assert (mode & 022) == 0); if any check fails, print an error to stderr and exit 1 instead of sourcing; only after all checks pass, keep the existing set -a / source "$ENV_FILE" / set +a sequence.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@setup.sh`:
- Around line 123-124: The script uses echo to write environment PEM variables
which can leave literal escape sequences (e.g. "\n") in the output; replace the
two echo commands that write CF_ORIGIN_CERT and CF_ORIGIN_KEY into
"$TRAEFIK_CERTS_DIR/origin.crt" and "$TRAEFIK_CERTS_DIR/origin.key" with printf
'%b' to correctly expand backslash-escaped newlines and produce valid PEM files
while keeping the same redirection.
- Around line 187-188: The recursive chown on TRAEFIK_CONFIG_DIR is changing
ownership of the cert subtree; change the ownership logic to avoid -R over the
entire "$TRAEFIK_CONFIG_DIR" and instead chown only non-sensitive config files
or specific subdirectories (e.g., config templates, dynamic config files) while
leaving "$TRAEFIK_CERTS_DIR" and its files root-owned; update the lines that
call chown (the commands referencing TRAEFIK_CONFIG_DIR and TRAEFIK_CERTS_DIR)
to limit recursion so cert material remains owned by root and only intended
config files are assigned to openfront.
- Around line 190-191: Add a preflight check that verifies the `docker compose`
command is available before running the two calls that use
"$TRAEFIK_CONFIG_DIR/compose.yaml" (the existing `docker compose -f ... pull`
and `docker compose -f ... up -d` lines); if the check fails, print a clear
error message asking the user to install the Docker Compose plugin (or provide
guidance for installing the legacy `docker-compose`), and exit non‑zero so the
script fails fast instead of erroring later. Use a simple command
existence/version check for `docker compose` and short-circuit the script with a
descriptive message and exit code when not present.
- Line 193: The check uses substring grep and can match wrong container names;
replace the line that calls "docker ps | grep -q traefik" with an exact-name
check such as piping docker ps --format '{{.Names}}' into grep -xq 'traefik'
(i.e. use the existing shell test around the command that currently contains
"docker ps | grep -q traefik" to run "docker ps --format '{{.Names}}' | grep -xq
'traefik'") so only a container named exactly "traefik" will match.
- Around line 12-17: The block that sources ENV_FILE must first validate it:
ensure ENV_FILE is a regular file and not a symlink (use -f and ! -L), verify it
is owned by root (stat -c %u "$ENV_FILE" == 0), and ensure it is not group- or
other-writable (check file mode via stat -c %a and assert (mode & 022) == 0); if
any check fails, print an error to stderr and exit 1 instead of sourcing; only
after all checks pass, keep the existing set -a / source "$ENV_FILE" / set +a
sequence.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
setup.sh (4)
187-188:⚠️ Potential issue | 🟠 MajorAdd a preflight check for Docker Compose availability.
Line 187 and Line 188 assume
docker composeexists. On hosts missing the plugin, setup fails at runtime without a clear prerequisite error.Suggested fix
+if ! docker compose version >/dev/null 2>&1; then + echo "❌ docker compose plugin is required but not available." + exit 1 +fi + docker compose -f "$TRAEFIK_CONFIG_DIR/compose.yaml" pull docker compose -f "$TRAEFIK_CONFIG_DIR/compose.yaml" up -d🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setup.sh` around lines 187 - 188, Add a preflight check in setup.sh to verify Docker Compose is available before running the two commands that use it; implement a check (e.g., a function like ensure_docker_compose_available or a simple command existence test) that verifies either the docker plugin subcommand ("docker compose") is supported or the legacy "docker-compose" binary exists, and if neither is present print a clear error and exit non‑zero so the subsequent docker compose -f "$TRAEFIK_CONFIG_DIR/compose.yaml" pull and up -d calls are not attempted.
12-17:⚠️ Potential issue | 🟠 MajorHarden
.env.setupbefore sourcing it as root.Line 16 executes file content directly. Please validate ownership, symlink status, and permissions before
sourceto prevent local privilege escalation.Suggested fix
if [ -f "$ENV_FILE" ]; then echo "📂 Loading environment from $ENV_FILE" + if [ -L "$ENV_FILE" ]; then + echo "❌ $ENV_FILE must not be a symlink" + exit 1 + fi + if [ "$(stat -c '%u' "$ENV_FILE")" -ne 0 ] || [ $((8#$(stat -c '%a' "$ENV_FILE") & 022)) -ne 0 ]; then + echo "❌ $ENV_FILE must be root-owned and not writable by group/others" + exit 1 + fi set -a # shellcheck source=/dev/null source "$ENV_FILE"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setup.sh` around lines 12 - 17, Before sourcing ENV_FILE, validate it: ensure ENV_FILE exists and is a regular file (not a symlink), is owned by the expected user (when running as root require root ownership), and has restrictive permissions (no group/other write/read — e.g. mode 600 or stricter) and is readable; only then run the existing source "$ENV_FILE" step. In other words, add checks around the current source invocation referencing ENV_FILE to reject symlinks, verify ownership via uid/gid, and enforce secure file mode before proceeding.
190-190:⚠️ Potential issue | 🟡 MinorUse exact container-name checks to avoid false positives.
Line 190 and Line 279 use substring matching (
grep -q), so similarly named containers can produce a false “healthy” result.Suggested fix
-if docker ps | grep -q traefik; then +if docker ps --format '{{.Names}}' | grep -xq 'traefik'; then @@ -if docker ps | grep -q node-exporter && docker ps | grep -q otel-collector; then +if docker ps --format '{{.Names}}' | grep -xq 'node-exporter' && \ + docker ps --format '{{.Names}}' | grep -xq 'otel-collector'; thenAlso applies to: 279-279
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setup.sh` at line 190, The check using substring matching ("docker ps | grep -q traefik") can return false positives for similarly named containers; update both occurrences (the check at the shown diff and the one around line 279) to match the container name exactly by listing container names and using an exact-match test (e.g., using docker ps --format '{{.Names}}' piped to an exact-match grep or docker ps --filter "name=..." with a format check) so only a container named exactly "traefik" is considered healthy; replace the current substring grep-based conditions with this exact-name approach in the shell checks.
123-125:⚠️ Potential issue | 🟠 MajorWrite PEM values with
printf '%b'instead ofecho.Line 123 and Line 124 can write literal
\nsequences when cert/key are escaped in.env.setup, which can break Traefik TLS parsing.Suggested fix
-echo "$CF_ORIGIN_CERT" > "$TRAEFIK_CERTS_DIR/origin.crt" -echo "$CF_ORIGIN_KEY" > "$TRAEFIK_CERTS_DIR/origin.key" +printf '%b' "$CF_ORIGIN_CERT" > "$TRAEFIK_CERTS_DIR/origin.crt" +printf '%b' "$CF_ORIGIN_KEY" > "$TRAEFIK_CERTS_DIR/origin.key" chmod 600 "$TRAEFIK_CERTS_DIR/origin.crt" "$TRAEFIK_CERTS_DIR/origin.key"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setup.sh` around lines 123 - 125, The current setup.sh writes escaped PEM literals via echo, which can leave literal "\n" sequences and break Traefik; update the writes that use CF_ORIGIN_CERT and CF_ORIGIN_KEY to use printf '%b' to expand backslash escapes when writing to "$TRAEFIK_CERTS_DIR/origin.crt" and "$TRAEFIK_CERTS_DIR/origin.key" (preserve the chmod 600 step afterwards) so the files contain real newline characters instead of escaped sequences.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@setup.sh`:
- Line 291: The success echo message currently claims Traefik is running on HTTP
:80 and HTTPS :443 but the setup only exposes HTTPS; update the text emitted by
the echo statement (the line that prints "Traefik reverse proxy is running...")
to remove the incorrect HTTP :80 mention and state only HTTPS :443 with the
Cloudflare origin cert so the operator message matches the actual exposed port.
---
Duplicate comments:
In `@setup.sh`:
- Around line 187-188: Add a preflight check in setup.sh to verify Docker
Compose is available before running the two commands that use it; implement a
check (e.g., a function like ensure_docker_compose_available or a simple command
existence test) that verifies either the docker plugin subcommand ("docker
compose") is supported or the legacy "docker-compose" binary exists, and if
neither is present print a clear error and exit non‑zero so the subsequent
docker compose -f "$TRAEFIK_CONFIG_DIR/compose.yaml" pull and up -d calls are
not attempted.
- Around line 12-17: Before sourcing ENV_FILE, validate it: ensure ENV_FILE
exists and is a regular file (not a symlink), is owned by the expected user
(when running as root require root ownership), and has restrictive permissions
(no group/other write/read — e.g. mode 600 or stricter) and is readable; only
then run the existing source "$ENV_FILE" step. In other words, add checks around
the current source invocation referencing ENV_FILE to reject symlinks, verify
ownership via uid/gid, and enforce secure file mode before proceeding.
- Line 190: The check using substring matching ("docker ps | grep -q traefik")
can return false positives for similarly named containers; update both
occurrences (the check at the shown diff and the one around line 279) to match
the container name exactly by listing container names and using an exact-match
test (e.g., using docker ps --format '{{.Names}}' piped to an exact-match grep
or docker ps --filter "name=..." with a format check) so only a container named
exactly "traefik" is considered healthy; replace the current substring
grep-based conditions with this exact-name approach in the shell checks.
- Around line 123-125: The current setup.sh writes escaped PEM literals via
echo, which can leave literal "\n" sequences and break Traefik; update the
writes that use CF_ORIGIN_CERT and CF_ORIGIN_KEY to use printf '%b' to expand
backslash escapes when writing to "$TRAEFIK_CERTS_DIR/origin.crt" and
"$TRAEFIK_CERTS_DIR/origin.key" (preserve the chmod 600 step afterwards) so the
files contain real newline characters instead of escaped sequences.
| echo "=====================================================" | ||
| echo "The openfront user has been set up and has Docker permissions." | ||
| echo "UDP buffer sizes have been configured for optimal QUIC/WebSocket performance." | ||
| echo "Traefik reverse proxy is running (HTTP :80, HTTPS :443 with Cloudflare origin cert)." |
Contributor
There was a problem hiding this comment.
Update the success message to match actual exposed ports.
Line 291 says HTTP :80, but this setup only configures/exposes HTTPS on :443 for Traefik. The message is misleading for operators.
Suggested fix
-echo "Traefik reverse proxy is running (HTTP :80, HTTPS :443 with Cloudflare origin cert)."
+echo "Traefik reverse proxy is running (HTTPS :443 with Cloudflare origin cert)."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| echo "Traefik reverse proxy is running (HTTP :80, HTTPS :443 with Cloudflare origin cert)." | |
| echo "Traefik reverse proxy is running (HTTPS :443 with Cloudflare origin cert)." |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@setup.sh` at line 291, The success echo message currently claims Traefik is
running on HTTP :80 and HTTPS :443 but the setup only exposes HTTPS; update the
text emitted by the echo statement (the line that prints "Traefik reverse proxy
is running...") to remove the incorrect HTTP :80 mention and state only HTTPS
:443 with the Cloudflare origin cert so the operator message matches the actual
exposed port.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description:
We are migrating to traefik and away from CF tunnels due to reliability issues. This PR sets up traefik with tls configured.
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
evan