software: auto-configure SWAG reverse proxy for all modules with stock proxy-confs#906
software: auto-configure SWAG reverse proxy for all modules with stock proxy-confs#906igorpecovnik wants to merge 17 commits into
Conversation
github-actions
Bot
added
size/large
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR migrates 30+ Docker-based services from the Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
tools/modules/functions/module_docker_utils.sh (1)
508-510: 💤 Low valueConsider more precise sed regex patterns.
The regex
[0-9]*matches zero or more digits, which could match an empty string. In standard sed (BRE), use[0-9]\+to require at least one digit, preventing overly broad matches if the sample config format varies.Suggested refinement
- docker exec swag sed -i "s/set \\\$upstream_port [0-9]*/set \\\$upstream_port ${port}/g" "$proxy_actual" 2>/dev/null + docker exec swag sed -i "s/set \\\$upstream_port [0-9][0-9]*/set \\\$upstream_port ${port}/g" "$proxy_actual" 2>/dev/nullSimilarly for protocol (line 514):
- docker exec swag sed -i "s/set \\\$upstream_proto [a-z]*/set \\\$upstream_proto ${protocol}/g" "$proxy_actual" 2>/dev/null + docker exec swag sed -i "s/set \\\$upstream_proto [a-z][a-z]*/set \\\$upstream_proto ${protocol}/g" "$proxy_actual" 2>/dev/null🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/modules/functions/module_docker_utils.sh` around lines 508 - 510, The sed regex in the docker exec call (affecting variable names port, proxy_actual and the upstream_port directive) uses [0-9]* which allows zero digits; update the pattern to require at least one digit (use [0-9]\+ in BRE sed) so the replacement only matches numeric ports, and make the same change for the protocol-related sed on the other line (the protocol replacement using a similar pattern). Ensure escaping is preserved in the shell string so the backslash before + survives when passed to sed.tools/modules/software/module_immich.sh (1)
150-150: 💤 Low valueConsider using
$dockernamefor consistency.Line 150 hardcodes
"immich"instead of using the$dockernamevariable. While functionally equivalent (since$dockernameis set to"immich"on line 22), other modules in this PR use the variable for consistency and future maintainability.♻️ Suggested change for consistency
- docker_configure_swag_proxy "immich" "8080" + docker_configure_swag_proxy "$dockername" "8080"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/modules/software/module_immich.sh` at line 150, Replace the hardcoded "immich" string with the module variable to keep naming consistent: update the call to docker_configure_swag_proxy to use $dockername (the variable set earlier) instead of the literal "immich" so the invocation in module_immich.sh matches other modules and stays maintainable.tools/modules/software/module_homepage.sh (1)
53-54: 💤 Low valueLGTM — internal port
3000is the right value here.Unlike most other modules where the internal and external ports coincide, homepage maps
${port}:3000, and SWAG proxies via the lsio bridge to the container-internal port — so passing"3000"(not$port) is correct. Worth a one-line comment so future maintainers don't "fix" it to$port.📝 Optional clarifying comment
- # Auto-configure SWAG reverse proxy if available - docker_configure_swag_proxy "homepage" "3000" + # Auto-configure SWAG reverse proxy if available. + # Pass the container-internal port (3000), not $port (3021), + # because SWAG proxies to homepage over the lsio bridge. + docker_configure_swag_proxy "$dockername" "3000"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/modules/software/module_homepage.sh` around lines 53 - 54, Add a one-line comment above the docker_configure_swag_proxy "homepage" "3000" call explaining that the homepage container listens on internal port 3000 and the compose maps ${port}:3000, so the SWAG proxy must be configured with the container-internal port "3000" (not the external $port) to avoid future maintainers changing it to $port; update the comment near the docker_configure_swag_proxy invocation in module_homepage.sh.tools/modules/runtime/config.runtime.sh (1)
30-30: 💤 Low valueUse
docker psto match the "is running" intent.The comment on line 29 says "Check if SWAG is running", but
docker container ls -aincludes stopped containers, so a stoppedswagcontainer slips past this guard. It still works because the nextdocker execfails and the http fallback is taken — but that's an implicit correctness path, and the same pattern is repeated at lines 155–156 in the portainer block.♻️ Suggested change
- # Check if SWAG is running - if ! docker container ls -a --format "{{.Names}}" 2>/dev/null | grep -q "^swag$"; then + # Check if SWAG is running + if ! docker ps --format "{{.Names}}" 2>/dev/null | grep -q "^swag$"; then echo "http://$DISPLAY_URL:$service_port" return fi-if docker container ls -a --format "{{.Names}}" 2>/dev/null | grep -q "^swag$" && \ +if docker ps --format "{{.Names}}" 2>/dev/null | grep -q "^swag$" && \ docker exec swag test -f "/config/nginx/proxy-confs/portainer.subfolder.conf.enabled" 2>/dev/null; thenAlso applies to: 155-156
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/modules/runtime/config.runtime.sh` at line 30, The check that intends to verify SWAG (and Portainer) is running uses "docker container ls -a" which lists stopped containers too; change the condition in the SWAG check (the command that currently runs docker container ls -a --format "{{.Names}}" | grep -q "^swag$") to only list running containers (use docker ps or docker container ls without -a) so the guard truly reflects "is running" before attempting docker exec; apply the same change to the Portainer check (the analogous docker container ls -a ... | grep -q "^portainer$" block) so both guards only match running containers.tools/modules/software/module_swag.sh (1)
47-47: ⚡ Quick winValidate domain format before proceeding
Line 47 strips protocol/path, but invalid hostnames still pass through and are reused by
hostnamectland SWAG env config. Add FQDN validation and fail fast.Proposed patch
# Clean URL (remove protocol if present) swag_url=$(echo "$swag_url" | sed -E 's|^\s*https?://||' | sed 's|/.*$||') + + # Validate FQDN + if [[ ! "$swag_url" =~ ^([A-Za-z0-9]([A-Za-z0-9-]{0,61}[A-Za-z0-9])?\.)+[A-Za-z]{2,63}$ ]]; then + dialog_msgbox "Invalid Domain" \ + "Please enter a valid fully qualified domain name (e.g. example.com)." 10 70 + return 1 + fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/modules/software/module_swag.sh` at line 47, After normalizing swag_url, validate that the resulting host is a valid FQDN before using it with hostnamectl or writing SWAG env config: add a check that the normalized swag_url matches an FQDN regex (allowing labels with letters/digits/hyphens, not starting/ending with a hyphen, and at least one dot/TLD), and if it fails print a clear error and exit non‑zero to fail fast; if it passes, proceed to call hostnamectl and update the SWAG env variables. Use the existing swag_url variable name in the validation and the same control flow paths that currently invoke hostnamectl and the SWAG env write so the failure prevents any further changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tools/modules/runtime/config.runtime.sh`:
- Around line 19-21: The DISPLAY_URL fallback makes non-proxied service links
point to SWAG_URL (public domain) causing unreachable ported URLs; update the
code so service URLs use get_service_url() (which already picks SWAG proxied
paths when proxy-conf is enabled) instead of interpolating DISPLAY_URL, or
explicitly use LOCALIPADD for non-proxied entries; search for occurrences of
DISPLAY_URL and replace bare "http://$DISPLAY_URL:$port" patterns (including the
bulk-rewritten menu entries) with calls to get_service_url <service> or with
"http://$LOCALIPADD:$port" when proxy is not enabled, leaving SWAG_URL and
DISPLAY_URL logic unchanged otherwise.
- Line 196: The call to update_sub_submenu_data with get_service_url uses
unquoted expansions and lacks the same "%% *" secondary-port trimming used
elsewhere; change the call so both get_service_url arguments are quoted and
apply the same parameter expansion (%% *) to the port value from
module_options["module_transmission,port"], mirroring the pattern used for
qbittorrent/deluge/pi_hole, and keep the servicename expansion quoted
(module_options["module_transmission,servicename"]) so word-splitting and
accidental trimming do not occur.
In `@tools/modules/software/module_netdata.sh`:
- Around line 59-60: Netdata runs with host networking so calling
docker_configure_swag_proxy("$dockername", "19999") produces a broken SWAG
upstream; change tools/modules/software/module_netdata.sh to first detect the
container's network mode (use docker inspect --format
'{{.HostConfig.NetworkMode}}' on $dockername or equivalent helper) and only call
docker_configure_swag_proxy when the network mode is not "host"; if it is
"host", skip the docker_configure_swag_proxy call so the menu continues to
advertise the host URL (keep references to docker_configure_swag_proxy and
$dockername).
In `@tools/modules/software/module_swag.sh`:
- Around line 105-113: The wait loop using wait_count and dockername checks for
/config/nginx/dynamicssl.conf but silently continues after 30s even if the file
never appears; after the while loop, add an explicit timeout branch that tests
for the file once more and if missing emits a clear error/warning (e.g., echo or
logger mentioning dockername and that SSL init timed out) and then returns a
non‑zero status or exits the script to fail fast; reference the existing wait
loop variables (wait_count, dockername) and the path
/config/nginx/dynamicssl.conf when adding this check so callers can detect and
handle the SSL-init failure.
- Line 188: The current command in module_swag.sh uses docker exec -it and
htpasswd -b which requires a TTY and exposes the password in argv; change to run
docker exec without -t (use -i only) and switch htpasswd to read the password
from stdin (htpasswd -i) while piping the password securely into docker exec so
that "${swag_password}" is not passed on the command line; update the invocation
that references "$dockername", htpasswd, "${swag_user}", "${swag_password}" and
the target file /config/nginx/.htpasswd to use the stdin-based approach and
remove the -t and -b flags.
In `@tools/modules/software/module_transmission.sh`:
- Around line 62-63: The two lines calling
docker_configure_swag_proxy("transmission", "9091") are mis-indented; change
their indentation to use a single leading tab (matching the surrounding case
block style used in functions like docker_configure_swag_proxy and nearby lines
44-61) so the call aligns with other statements inside the case branch and
preserves consistent tab-based formatting.
---
Nitpick comments:
In `@tools/modules/functions/module_docker_utils.sh`:
- Around line 508-510: The sed regex in the docker exec call (affecting variable
names port, proxy_actual and the upstream_port directive) uses [0-9]* which
allows zero digits; update the pattern to require at least one digit (use
[0-9]\+ in BRE sed) so the replacement only matches numeric ports, and make the
same change for the protocol-related sed on the other line (the protocol
replacement using a similar pattern). Ensure escaping is preserved in the shell
string so the backslash before + survives when passed to sed.
In `@tools/modules/runtime/config.runtime.sh`:
- Line 30: The check that intends to verify SWAG (and Portainer) is running uses
"docker container ls -a" which lists stopped containers too; change the
condition in the SWAG check (the command that currently runs docker container ls
-a --format "{{.Names}}" | grep -q "^swag$") to only list running containers
(use docker ps or docker container ls without -a) so the guard truly reflects
"is running" before attempting docker exec; apply the same change to the
Portainer check (the analogous docker container ls -a ... | grep -q
"^portainer$" block) so both guards only match running containers.
In `@tools/modules/software/module_homepage.sh`:
- Around line 53-54: Add a one-line comment above the
docker_configure_swag_proxy "homepage" "3000" call explaining that the homepage
container listens on internal port 3000 and the compose maps ${port}:3000, so
the SWAG proxy must be configured with the container-internal port "3000" (not
the external $port) to avoid future maintainers changing it to $port; update the
comment near the docker_configure_swag_proxy invocation in module_homepage.sh.
In `@tools/modules/software/module_immich.sh`:
- Line 150: Replace the hardcoded "immich" string with the module variable to
keep naming consistent: update the call to docker_configure_swag_proxy to use
$dockername (the variable set earlier) instead of the literal "immich" so the
invocation in module_immich.sh matches other modules and stays maintainable.
In `@tools/modules/software/module_swag.sh`:
- Line 47: After normalizing swag_url, validate that the resulting host is a
valid FQDN before using it with hostnamectl or writing SWAG env config: add a
check that the normalized swag_url matches an FQDN regex (allowing labels with
letters/digits/hyphens, not starting/ending with a hyphen, and at least one
dot/TLD), and if it fails print a clear error and exit non‑zero to fail fast; if
it passes, proceed to call hostnamectl and update the SWAG env variables. Use
the existing swag_url variable name in the validation and the same control flow
paths that currently invoke hostnamectl and the SWAG env write so the failure
prevents any further changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8c761358-8eee-415f-8331-dca29125b810
📒 Files selected for processing (34)
tools/modules/functions/module_docker_utils.shtools/modules/functions/module_env_init.shtools/modules/runtime/config.runtime.shtools/modules/software/module_bazarr.shtools/modules/software/module_code-server.shtools/modules/software/module_deluge.shtools/modules/software/module_domoticz.shtools/modules/software/module_dozzle.shtools/modules/software/module_duplicati.shtools/modules/software/module_embyserver.shtools/modules/software/module_filebrowser.shtools/modules/software/module_ghost.shtools/modules/software/module_grafana.shtools/modules/software/module_homepage.shtools/modules/software/module_immich.shtools/modules/software/module_jellyfin.shtools/modules/software/module_lidarr.shtools/modules/software/module_mariadb.shtools/modules/software/module_medusa.shtools/modules/software/module_netbox.shtools/modules/software/module_netdata.shtools/modules/software/module_nextcloud.shtools/modules/software/module_phpmyadmin.shtools/modules/software/module_pi-hole.shtools/modules/software/module_portainer.shtools/modules/software/module_prowlarr.shtools/modules/software/module_qbittorrent.shtools/modules/software/module_radarr.shtools/modules/software/module_sabnzbd.shtools/modules/software/module_sonarr.shtools/modules/software/module_swag.shtools/modules/software/module_syncthing.shtools/modules/software/module_transmission.shtools/modules/software/module_wireguard.sh
| # Use SWAG domain URL if available, otherwise fall back to local IP | ||
| # This makes all menu URLs show the actual domain instead of IP when SWAG is installed | ||
| DISPLAY_URL="${SWAG_URL:-$LOCALIPADD}" |
There was a problem hiding this comment.
DISPLAY_URL fallback is wrong for non-proxied services.
When SWAG_URL is set to a public domain (the common case once a user runs SWAG with Let's Encrypt), every plain http://$DISPLAY_URL:<service-port> line points the menu at e.g. http://example.com:8200, http://example.com:9696, etc. In a typical home/router setup only 80/443 are forwarded externally for SWAG, so those direct-port URLs are unreachable from the WAN and confusing on the LAN (where users would expect the LAN IP).
get_service_url() already encodes the right semantics: prefer SWAG_URL/<service> when the proxy-conf is enabled, otherwise fall back to LOCALIPADD (not DISPLAY_URL). The bulk-rewritten lines (134, 137–149, 151, 163, 166, 172–175, 178–179, 182–186, 189–190, 194–205, 208) need the same treatment — either route every entry through get_service_url(), or have non-proxied lines keep using $LOCALIPADD.
🛠️ Suggested shape
- # Check if SWAG proxy is enabled for this service
- local proxy_enabled_file="/config/nginx/proxy-confs/${service_name}.subfolder.conf.enabled"
- if ! docker exec swag test -f "$proxy_enabled_file" 2>/dev/null; then
- echo "http://$DISPLAY_URL:$service_port"
- return
- fi
-
- # SWAG proxy is enabled - return subfolder URL
- echo "https://$DISPLAY_URL/$service_name"
+ # Check if SWAG proxy is enabled for this service
+ local proxy_enabled_file="/config/nginx/proxy-confs/${service_name}.subfolder.conf.enabled"
+ if docker exec swag test -f "$proxy_enabled_file" 2>/dev/null; then
+ echo "https://${SWAG_URL:-$LOCALIPADD}/$service_name"
+ return
+ fi
+
+ # Not proxied: always use LAN IP for direct-port access, never the public domain
+ echo "http://$LOCALIPADD:$service_port"…and migrate the bare http://$DISPLAY_URL:$port lines to get_service_url.
Also applies to: 134-205
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/modules/runtime/config.runtime.sh` around lines 19 - 21, The
DISPLAY_URL fallback makes non-proxied service links point to SWAG_URL (public
domain) causing unreachable ported URLs; update the code so service URLs use
get_service_url() (which already picks SWAG proxied paths when proxy-conf is
enabled) instead of interpolating DISPLAY_URL, or explicitly use LOCALIPADD for
non-proxied entries; search for occurrences of DISPLAY_URL and replace bare
"http://$DISPLAY_URL:$port" patterns (including the bulk-rewritten menu entries)
with calls to get_service_url <service> or with "http://$LOCALIPADD:$port" when
proxy is not enabled, leaving SWAG_URL and DISPLAY_URL logic unchanged
otherwise.
| update_sub_submenu_data "Software" "Downloaders" "JEL002" "http://$LOCALIPADD:${module_options["module_jellyseerr,port"]}" | ||
| update_sub_submenu_data "Software" "Downloaders" "DOW002" "http://$DISPLAY_URL:${module_options["module_qbittorrent,port"]%% *}" # removing second port from url | ||
| update_sub_submenu_data "Software" "Downloaders" "DEL002" "http://$DISPLAY_URL:${module_options["module_deluge,port"]%% *}" # removing second port from url | ||
| update_sub_submenu_data "Software" "Downloaders" "TRA002" "$(get_service_url ${module_options["module_transmission,servicename"]} ${module_options["module_transmission,port"]})" # removing second port from url |
There was a problem hiding this comment.
Quote the args and apply the same %% * port trim used elsewhere.
The arguments to get_service_url are unquoted, and unlike the surrounding lines (e.g. 194 qbittorrent, 195 deluge, 178 pi_hole) there is no %% * to drop the secondary port. The trailing # removing second port from url is misleading — that's currently happening only by accident through word-splitting on the unquoted expansion, which will misbehave the moment the servicename or port value contains anything unexpected.
🔧 Proposed fix
-update_sub_submenu_data "Software" "Downloaders" "TRA002" "$(get_service_url ${module_options["module_transmission,servicename"]} ${module_options["module_transmission,port"]})" # removing second port from url
+update_sub_submenu_data "Software" "Downloaders" "TRA002" "$(get_service_url "${module_options["module_transmission,servicename"]}" "${module_options["module_transmission,port"]%% *}")" # removing second port from url📝 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.
| update_sub_submenu_data "Software" "Downloaders" "TRA002" "$(get_service_url ${module_options["module_transmission,servicename"]} ${module_options["module_transmission,port"]})" # removing second port from url | |
| update_sub_submenu_data "Software" "Downloaders" "TRA002" "$(get_service_url "${module_options["module_transmission,servicename"]}" "${module_options["module_transmission,port"]%% *}")" # removing second port from url |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/modules/runtime/config.runtime.sh` at line 196, The call to
update_sub_submenu_data with get_service_url uses unquoted expansions and lacks
the same "%% *" secondary-port trimming used elsewhere; change the call so both
get_service_url arguments are quoted and apply the same parameter expansion (%%
*) to the port value from module_options["module_transmission,port"], mirroring
the pattern used for qbittorrent/deluge/pi_hole, and keep the servicename
expansion quoted (module_options["module_transmission,servicename"]) so
word-splitting and accidental trimming do not occur.
| docker restart "$dockername" >/dev/null | ||
|
|
||
| # Set the password in the container | ||
| if docker exec -it "$dockername" htpasswd -b -c /config/nginx/.htpasswd "${swag_user}" "${swag_password}" >/dev/null 2>&1; then |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify risky patterns in shell modules (TTY-dependent exec and password in argv)
rg -n -C2 --type sh 'docker exec -it|htpasswd\s+-b'Repository: armbian/configng
Length of output: 1393
🌐 Web query:
htpasswd -i flag stdin password input documentation
💡 Result:
The htpasswd command’s -i flag means: “Read the password from stdin without verification (for script usage).” [1] So when you use -i, htpasswd does NOT prompt/verify interactively; it takes the password value from standard input (stdin) and proceeds to generate/update the entry in the passwd file. [1] How it’s described: - Apache HTTP Server htpasswd (2.4 docs): “-i Read the password from stdin without verification (for script usage).” [1] - Ubuntu man page for htpasswd: “-i Read the password from stdin without verification (for script usage).” [2] In practice, this is typically used like (example): - echo "plaintext-password" | htpasswd -i username passwdfile (That piping pattern matches the meaning “read the password from stdin”.) [1][2] Note: -b is the other “non-interactive” option (“batch mode”), but it takes the password from the command line; Apache warns this is unsafe because it’s visible in process listings. For script usage, Apache points you to -i. [1]
Citations:
- 1: https://httpd.apache.org/docs/en/programs/htpasswd.html
- 2: https://manpages.ubuntu.com/manpages/focal/man1/htpasswd.1.html
🌐 Web query:
docker exec -it vs -i non-interactive context failures
💡 Result:
When you use docker exec -it, the -t request is “allocate a pseudo-TTY”, while -i means “keep STDIN open even if not attached” [1][2]. The common non-interactive failure mode happens when the environment you run docker exec from does not have a real terminal (STDIN isn’t a TTY—e.g., cron/Jenkins/systemd, or SSH sessions with no TTY). In that case Docker may error with messages like “cannot enable tty mode on non tty input”, and people fix it by using only -i (or by using a compose command that has different defaults, or by ensuring a TTY exists upstream). 1) What -it vs -i do - -i / --interactive keeps container STDIN open even if Docker isn’t attached to a terminal [1]. This is useful when the command needs input (including non-interactive stdin piping). - -t / --tty allocates a pseudo-TTY for the command [1]. Many interactive programs (shells, sudo prompting, password entry, etc.) rely on a controlling terminal. 2) Why non-interactive contexts fail with -it - If STDIN is not a TTY (for example, the command is launched from cron, Jenkins, systemd, or an SSH session without a pseudo-terminal), docker exec cannot “enable tty mode” and you can get errors like “cannot enable tty mode on non tty input”. A reported workaround is switching from docker exec -it to docker exec -i when the script runs under a scheduler that isn’t attached to a terminal [3]. - Related issues show the general theme: -t requires a usable terminal on the caller side; without it, interactive/TTY semantics break or cause errors [4][5]. 3) Practical guidance / fixes A) If you don’t actually need a TTY (non-interactive command or piped input) - Prefer docker exec -i (or even no flags) and pipe input/arguments instead. Example pattern: echo "..." | docker exec -i . B) If you do need interactive behavior (bash, menu prompts, sudo asking questions) - Ensure the docker exec process is started with a real TTY (e.g., run from an interactive terminal, or force upstream tty allocation in SSH). - For sudo-specific failures, the same root issue applies: sudo expects a TTY unless you provide an alternate stdin/askpass strategy; the fix may be to allocate a pseudo-terminal or configure sudo to read credentials from stdin / use an askpass helper [6]. C) docker compose exec differs in defaults - docker compose exec allocates a TTY by default and enters interactive mode by default, and docker exec requires explicit --interactive --tty to match that behavior [7]. If your compose exec usage is inside a script/CI where stdin isn’t a TTY, you may need to disable tty allocation (compose: use -T / --no-tty) or explicitly control -i/-t behavior. 4) Quick decision rule - If your command works with piped stdin or runs fully non-interactively: use -i, avoid -t. - If your command requires a terminal device: keep -t, but start docker exec from a context that provides a TTY (or force one upstream). If you share the exact failing command and the environment you run it from (cron/Jenkins/systemd/SSH-without-tty/CI runner, plus the exact error text), I can suggest the most appropriate exact flags (e.g., docker exec -i vs docker exec -it, or compose -T).
Citations:
- 1: https://docs.docker.com/reference/cli/docker/container/exec/
- 2: https://stackoverflow.com/questions/35550694/what-is-the-purpose-of-the-i-and-t-options-for-the-docker-exec-command
- 3: https://stackoverflow.com/questions/29380344/docker-exec-it-returns-cannot-enable-tty-mode-on-non-tty-input
- 4: "cannot enable tty mode on non tty input" in Cygwin in Windows moby/moby#12469
- 5: TTY error when launching container from systemd moby/moby#10121
- 6: https://www.simplified.guide/ssh/sudo-no-tty-askpass
- 7: https://docs.docker.com/reference/cli/docker/compose/exec/
🏁 Script executed:
sed -n '185,192p' tools/modules/software/module_swag.shRepository: armbian/configng
Length of output: 399
Avoid TTY-dependent exec and plaintext password argv
Line 188 uses docker exec -it and htpasswd -b. The -t flag fails in non-interactive contexts (cron, CI/CD, systemd), and -b exposes the password in process arguments. Use stdin-based password input with -i instead.
Proposed patch
- if docker exec -it "$dockername" htpasswd -b -c /config/nginx/.htpasswd "${swag_user}" "${swag_password}" >/dev/null 2>&1; then
+ if printf '%s\n' "${swag_password}" | docker exec -i "$dockername" \
+ htpasswd -i -c /config/nginx/.htpasswd "${swag_user}" >/dev/null 2>&1; then📝 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.
| if docker exec -it "$dockername" htpasswd -b -c /config/nginx/.htpasswd "${swag_user}" "${swag_password}" >/dev/null 2>&1; then | |
| if printf '%s\n' "${swag_password}" | docker exec -i "$dockername" \ | |
| htpasswd -i -c /config/nginx/.htpasswd "${swag_user}" >/dev/null 2>&1; then |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/modules/software/module_swag.sh` at line 188, The current command in
module_swag.sh uses docker exec -it and htpasswd -b which requires a TTY and
exposes the password in argv; change to run docker exec without -t (use -i only)
and switch htpasswd to read the password from stdin (htpasswd -i) while piping
the password securely into docker exec so that "${swag_password}" is not passed
on the command line; update the invocation that references "$dockername",
htpasswd, "${swag_user}", "${swag_password}" and the target file
/config/nginx/.htpasswd to use the stdin-based approach and remove the -t and -b
flags.
igorpecovnik
force-pushed
the
enable-swag-proxy-for-supported-modules
branch
3 times, most recently
from
c8f0fb8 to
a6b610a
Compare
igorpecovnik
force-pushed
the
enable-swag-proxy-for-supported-modules
branch
from
a6b610a to
9491aee
Compare
igorpecovnik
force-pushed
the
enable-swag-proxy-for-supported-modules
branch
18 times, most recently
from
bfa6974 to
865d734
Compare
igorpecovnik
force-pushed
the
enable-swag-proxy-for-supported-modules
branch
from
865d734 to
f488b75
Compare
…menu URLs
Wires SWAG into the configng software stack so that installing any of the
supported modules on a host running SWAG also enables the matching nginx
proxy-conf, and the menu UI shows the SWAG subfolder URL instead of the
direct host:port whenever the proxy is live.
Core helpers (module_docker_utils.sh, runtime/config.runtime.sh,
module_env_init.sh):
- SWAG_URL is read from ${SOFTWARE_FOLDER}/swag/config/SWAG_URL at env
init and exposed to all modules. DISPLAY_URL=${SWAG_URL:-$LOCALIPADD}
is the single source of truth for the menu's right column.
- get_service_url <service> <port>: returns
https://$DISPLAY_URL/<service> when SWAG is up and the
proxy-conf for <service> is
enabled,
http://$DISPLAY_URL:<port> otherwise.
- docker_configure_swag_proxy <service> [port] [proto]: copies the
SWAG sample to its enabled .conf, sed-rewrites $upstream_port and
$upstream_proto, touches the .enabled marker, and reloads nginx.
Silent no-op (returns 1) if the sample isn't present, returns 2 if
SWAG isn't installed — modules can call it unconditionally.
- docker_seed_swag_proxy_conf <service>: reads an nginx subfolder
proxy-conf from stdin and writes it as
/config/nginx/proxy-confs/<service>.subfolder.conf.sample inside
the SWAG container, for services where linuxserver/reverse-proxy-
confs:master doesn't ship a stock sample (currently netbox; same
pattern applicable to immich, vaultwarden, …). Skips writing if a
sample is already present so an upstream sample or hand-edited
admin override wins on the next install.
Per-module wiring:
- 22 modules whose dockername matches a stock proxy-conf in
linuxserver/reverse-proxy-confs:master gain a
docker_configure_swag_proxy call after their docker run:
bazarr, deluge, domoticz, dozzle, duplicati, embyserver→emby,
filebrowser, ghost, grafana, jellyfin, lidarr, medusa, netdata,
nextcloud (https), phpmyadmin, pi-hole→pihole, prowlarr,
qbittorrent, radarr, sabnzbd, sonarr, syncthing, transmission.
- homepage / immich / netbox / portainer pre-existing custom paths
are folded in, with portainer correctly using HTTPS+9443.
netbox SWAG support (subfolder mode):
- Seeds a hand-authored netbox.subfolder.conf.sample into SWAG via
docker_seed_swag_proxy_conf so docker_configure_swag_proxy has
something to enable. The conf does not rewrite the path —
upstream NetBox sees /netbox unchanged.
- When the swag container is present at install time, bakes
BASE_PATH = 'netbox/' and CSRF_TRUSTED_ORIGINS = ['https://$SWAG_URL']
into the generated configuration.py. Without BASE_PATH, Django
emits absolute URLs (form action=/login/?next=/netbox, /static/…)
that 404 once SWAG strips them at /netbox; without
CSRF_TRUSTED_ORIGINS, login POSTs 403 even with paths fixed.
These settings live in configuration.py because netboxcommunity/
netbox does not consume BASE_PATH= / CSRF_TRUSTED_ORIGINS= from
the container env. Trade-off: with BASE_PATH set, direct port
access at / no longer works — only http://host:port/netbox/.
netbox postgres args:
- module_postgres install takes six positional args
(<user> <pass> <db> <image-repo> <image-tag> <container-name>);
netbox was passing five and fusing image+tag into one. The result
was Docker 404'ing on 'postgres:17-alpine:postgres-netbox' and the
postgres container defaulting to the name 'postgres' (collision
with any standalone postgres install). Split DATABASE_IMAGE into
DATABASE_IMAGE + DATABASE_TAG and pass DATABASE_HOST in its
proper sixth slot, matching the pattern module_immich already uses.
Menu URL routing (config.runtime.sh):
- 23 update_sub_submenu_data lines (the SWAG-applicable subset) now
go through get_service_url — flipping each menu's right-column URL
from 'http://host:port' to 'https://host/<service>' the moment
that service's proxy-conf is enabled in SWAG, and back if it ever
isn't. pi-hole and ghost retain their /admin and /ghost subpaths
by appending the suffix after the helper output.
- NCT002 (nextcloud) and CPT002 (cockpit) intentionally untouched —
direct URLs are HTTPS-only on a self-signed port; helper falls
back to plain http otherwise. Worth a follow-up that gives the
helper a fallback-protocol arg.
No regression for users without SWAG: get_service_url,
docker_configure_swag_proxy and docker_seed_swag_proxy_conf all fall
back to existing behaviour when the swag container isn't present.
igorpecovnik
force-pushed
the
enable-swag-proxy-for-supported-modules
branch
from
f488b75 to
4032d4a
Compare
…ssets Beszel's served HTML emits absolute asset paths (`/static/...`, `/_app/...`) and PocketBase has no base-path support, so LSIO's stock sample (which only strip-rewrites /beszel/ → / at the upstream) returns 200 with a blank page in the browser. Replace the LSIO sample with a custom seed that adds sub_filter to rewrite asset URLs in HTML/CSS/JS back to /beszel/-prefixed paths. Same approach as uptime-kuma.
… confs LSIO's /config/nginx/proxy.conf (included at the top of every proxy location block) already sets proxy_read_timeout. Our seeded subfolder confs redeclared it as 86400 hoping to extend it for SSE/Socket.IO, but nginx treats the duplicate as an emerg-level config error: the 'nginx -s reload' silently fails and SWAG keeps serving the previous (broken) config. That's why sub_filter never took effect even though the conf file on disk had the right directives. Drop the redeclaration. If realtime/long-poll connections turn out to need a longer timeout, override at a different scope (e.g. via the SWAG site conf) rather than re-declaring inside the included scope.
text/html is always processed by sub_filter regardless of the directive; listing it explicitly triggers an nginx [warn] 'duplicate MIME type' on every reload. Harmless but noisy.
…r hack Beszel officially supports subpath deployment via the APP_URL env var (https://beszel.dev/guide/reverse-proxy). The hub parses the URL, derives BASE_PATH and HUB_URL, and injects them into the rendered index.html so the frontend JS builds API URLs against the right path. Previous attempt (custom proxy-conf with rewrite + sub_filter) only fixed asset paths in the static HTML; runtime-built fetch URLs in the JS bundle (PocketBase JS SDK calls) still resolved against / and hit SWAG's root, returning 404. With APP_URL set, BASE_PATH = "/beszel/" is baked into the runtime config and all calls go through the proxy correctly. Drop the custom seed entirely — LSIO's stock beszel.subfolder.conf.sample works as-is once APP_URL is configured. Note: PocketBase's own Settings.Meta.AppURL (used for Shoutrrr alert links) is independent of APP_URL and must still be set in the admin UI for emailed alerts to point at the right host.
docker_configure_swag_proxy only copies the LSIO .sample to the active conf when the active conf is absent. After dropping our previous sub_filter-based seed in favour of APP_URL, an existing install that already has the custom seeded conf would not get reset on reinstall, and the lingering sub_filter would double-prefix Beszel's now-natively- correct asset URLs to /beszel/beszel/... . Explicitly remove the active conf (and its .enabled marker) before configure when SWAG is present, so the LSIO stock sample is copied fresh on every install.
Adds SERVER_SERVLET_CONTEXT_PATH=/stirling-pdf when SWAG is present so the Spring Boot app routes itself under /stirling-pdf and emits correctly prefixed asset URLs, then calls docker_configure_swag_proxy to enable LSIO's stock stirling-pdf.subfolder.conf. Switches the STR002 menu URL to get_service_url so it shows the public https://<host>/stirling-pdf when SWAG is enabled, falling back to http://<localip>:<port> otherwise.
linuxserver/reverse-proxy-confs:master has no stirling-pdf sample, so docker_configure_swag_proxy silently no-op'd and the public URL returned 404. Seed a custom subfolder.conf via docker_seed_swag_proxy_conf. The container is already configured with SERVER_SERVLET_CONTEXT_PATH=/stirling-pdf so upstream and incoming URIs match — no rewrite needed. Bumps client_max_body_size to 0 (unlimited) because Stirling's PDF manipulation tools accept multi-hundred-MB uploads, and SWAG's default 2M cap returns 413 on anything larger than a small report.
LSIO's stock filebrowser.subfolder.conf forwards /filebrowser/ to the upstream as-is (no rewrite). Without --baseurl, filebrowser serves at / and its frontend JS makes /api/... calls against the SWAG root, which 404s and the page is stuck on the loading spinner forever.
Hastebin's remove menu entry had no URL appended because there was no update_sub_submenu_data line for it. Wire to http://LOCALIPADD:port using the standard non-proxied form for now. Hastebin (haste-server fork) emits absolute-path assets and has no native baseurl support; subpath SWAG proxying would require a sub_filter best-effort, which is out of scope here.
Hastebin (haste-server fork) has no native subpath support — its application.js makes /documents, /raw/ and /about.md calls as absolute paths, and the inline JS extracts the document key via window.location.pathname.substring(1), which breaks paste viewing under /hastebin/<key>. Seed a custom subfolder.conf that rewrites the absolute API paths in the served JS so paste *creation* works through the proxy. Paste *viewing* via /hastebin/<key> is intentionally broken — would require source patches to haste-server. Subdomain proxying remains the recommended deployment. Switch the HPS002 menu URL to get_service_url so it shows the public https://<host>/hastebin URL when SWAG is enabled.
The x86 hwacc branch unconditionally passed --device=/dev/dri to the container, which docker rejects with 'error gathering device information ... no such file or directory' on hosts without an iGPU (VPS, headless servers). The whole install aborts before the proxy is even configured. Mirror the rk35xx branch's existence check: skip the device flag silently when /dev/dri is missing. Same fix applied to the bcm2711 branch for VMs without the VC4 video10/11/12 nodes.
Compare the user-typed FQDN's A-record against this host's detected public IPv4 before pulling SWAG. If DNS doesn't resolve at all, or resolves to a different host, surface a dialog with both addresses and let the user proceed or cancel. Catches the most common installer mistake — "domain not pointed yet" — before burning a 200 MB image pull and a Let's Encrypt rate-limit slot on a guaranteed-failed cert request. Soft check, not a hard block: split-horizon DNS, in-progress propagation, and tests behind a corporate proxy are legitimate reasons to override. Public-IP detection silently falls back to skipping the comparison if outbound HTTPS fails (firewall, offline) rather than producing false-positive warnings. Uses getent (libc resolver, matches what Docker/SWAG see at runtime) and curl with a 4 s per-endpoint timeout against api.ipify.org / ifconfig.me / icanhazip.com.
The htpasswd-set path called docker restart, which kills every active connection — disruptive for users browsing other proxied services on the same SWAG instance. nginx re-reads auth_basic_user_file on every request, so the new credentials take effect immediately even without a reload; sending SIGHUP via 'nginx -s reload' is the conservative zero-downtime alternative when other config edits may be pending.
The post-install wait loop checked for /config/nginx/dynamicssl.conf, which is generated during SWAG's nginx config setup regardless of whether Let's Encrypt succeeded — meaning the install reported success for every run, including ones where the cert request was rejected. Wait for /config/keys/letsencrypt/fullchain.pem instead (the actual cert file). Bump the timeout from 30 s to 90 s to cover slow ACME provisioning. If the file is still missing afterwards, surface a diagnostic dialog with: - the most likely root causes (DNS, port 80, rate limits) - a grep of the container log for typical ACME error markers - the exact retry command (docker restart swag) Don't fail the install — SWAG is running and the operator can fix DNS / unblock port 80 / wait out a rate limit, then retry without reinstalling. Better-than-silent diagnostics matter more than a non-zero exit code here.
armbian-config runs with root privileges, so the yes/no dialog asking permission to call hostnamectl was needless friction — every install where the user typed the FQDN already implies they want the hostname to match. Drop the prompt and just do it; only surface a dialog if hostnamectl actually fails.
Setting the SWAG password previously created /config/nginx/.htpasswd
but never wired it up — LSIO's stock proxy-confs ship auth_basic and
auth_basic_user_file commented out, and our seeded confs didn't
include them at all. The password feature was effectively a no-op.
Add docker_swag_enable_basic_auth helper that:
- is a no-op if .htpasswd doesn't exist (lets users install services
before setting a password and have auth applied retroactively),
- uncomments the htpasswd-flavoured auth_basic lines for LSIO stock
confs, leaving ldap/authelia/authentik blocks alone,
- injects the lines inside the location block for our custom-seeded
confs that don't ship them at all.
Wire it into:
- docker_configure_swag_proxy, so newly-installed services are
auth-gated immediately if a password is already set,
- the SWAG password command, so retroactively setting/changing the
password walks every enabled subfolder.conf and applies auth.
Helper is idempotent and reload-free by design — caller batches its
own SIGHUP so a single reload covers both the touch+enable and the
auth uncomment.
1 participant
Summary
docker_configure_swag_proxy()enables the matching SWAG proxy-conf for a service after install. Until now it was wired for only 5 modules:homepage,immich,netbox,portainer,transmission. Every other proxied service had to be hand-enabled inside the SWAG container.This PR wires the call into the 22 modules whose dockername matches a stock proxy-conf in linuxserver/reverse-proxy-confs:master.
The call passes the container's internal port (not the host-mapped one). The function
sedsupstream_portin the proxy-conf before enabling, so SWAG reaches each service via the LSIO docker bridge (http://${dockername}:${port}) regardless of how it was published on the host.Test matrix
URLs are the predicted SWAG subfolder paths (
https://my.home.org/<service>/); replacemy.home.orgwith your own SWAG-fronted hostname when testing. Tick the box once verified.Notes on dockername mapping:
embyserveralready setsdockername="emby"so the proxy-conf name matches.pi-holealready setsdockername="pihole"so the proxy-conf name matches.nextcloud's container exposes 443/TLS, so the call passesprotocol="https".No regression for users without SWAG
docker_configure_swag_proxyreturns 2 if theswagcontainer doesn't exist and 1 if the proxy-conf sample isn't present, neither of which is checked by the caller. Modules behave unchanged on a host without SWAG.Modules deliberately not wired
Modules whose service has no stock proxy-conf in
linuxserver/reverse-proxy-confs:masterare skipped — adding the call would just produce a silent no-op:actualbudget,adguardhome,armbianrouter,code-server,evcc,hastebin,jellyseerr,mariadb,mysql,navidrome,netalertx,octoprint,openhab,owncloud,postgres,prometheus,redis,stirling,unbound,uptime-kuma,wallos,watchtower,wireguard.(Note:
module_immich.shandmodule_netbox.shalready call the function but the upstream repo doesn't shipimmich.subfolder.conf.sampleornetbox.subfolder.conf.sample— those calls are silent no-ops too. Left as-is for forward-compat if upstream adds them.)