Repository navigation
Only force the bind address when it is unset or loopback - #275
Open
ivan-pinatti wants to merge 2 commits into
Open
ivan-pinatti wants to merge 2 commits into
ivan-pinatti wants to merge 2 commits into
Conversation
The service script passes --server unconditionally, which overrides host in sabnzbd.ini and is persisted back into it, so a configured host cannot survive a container start. A configured host is now passed back through the same flag, making it a no-op, unless the value is unset or loopback, in which case FAMILY is imposed exactly as before. That keeps the lockout protection the flag exists for: a loopback bind is unreachable from outside the container either way. The check runs once per branch rather than once above the if, because only the root branch mutates FAMILY for the IPv6 fallback, and a single check ahead of it would let that fallback clobber a configured host on a host without IPv6. Section tracking when reading the ini, because the NNTP server entries under [servers] carry their own unindented host line. closes linuxserver#274
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks for opening this pull request! Be sure to follow the pull request template!
Author
|
Heads up on the red The job fails before it reaches any of my files:
For what it is worth the job itself looks harmless in substance, since it only runs Three ways out, in the order I would pick them:
The file this PR touches is already |
pull Bot
pushed a commit
to abraxas678/docker-torrent-box-with-vpn
that referenced
this pull request
Sep 3, 2026
…am (ivan-pinatti-labs#146) * chore: drop the jdownloader-2 and lazylibrarian patches, fixed upstream Two of the four `patches/` directories exist to work around bugs that upstream has since fixed. Both fixes have reached the image, so the patches are now shadowing files that no longer need shadowing. **jdownloader-2 (ivan-pinatti-labs#107).** The image's secret loader only set a variable that was currently unset, and the Dockerfile pre-declares `WEB_AUTHENTICATION_USERNAME`/`PASSWORD` as empty strings, so it always found them "set" and skipped the secret. `baseimage-gui` 4.13.0 fixed it by giving `load_env_var()` a `force` argument that the secret path passes. Nothing had to move to collect this: `JDOWNLOADER2_VERSION` is `v26.08.1`, which is built on `baseimage-gui` 4.13.1, and the pin had already drifted onto it. Verified by pulling that exact image and extracting `/init`: # Always apply secrets: they are user-provided and take precedence over # environment variables (including empty image ENV defaults). load_env_var "${env_var_name}" "$(head -n1 < "${fpath}")" force **lazylibrarian (ivan-pinatti-labs#109).** `login()` and `logout()` prepended `HTTP_ROOT` to a `from_page` that nginx had already prefixed, so the first redirect after logging in 404'd. Upstream !1832 merged on 2026-08-08 and has reached the image, carrying the same guard this repository was patching in. This one did need a bump: `40a389ea-ls310` does not have the fix and `f9f62f7a-ls342` does, confirmed by extracting `auth.py` from both. Worth noting the image has moved a long way since the patch was written, gaining a `get_remote_ip()` and a `PROXY_LOCAL` whitelist path the patch predates, so the patch had started overwriting newer upstream code to deliver a fix upstream already had. **Renovate holds.** jdownloader-2 comes off the `enabled: false` list with its patch. lazylibrarian does not, and the comment now says why: issue ivan-pinatti-labs#119, its tag carrying no version number, so `loose` versioning ranks releases by reading the leading hex of a commit fragment as a number. That was always going to matter "on the day `patches/lazylibrarian/` is dropped", and this is that day. It stays held and hand-bumped until ivan-pinatti-labs#119 settles the scheme. `test_patched_services_were_found`'s floor drops from four to two, which is what now mount a patch: sabnzbd and mylar. Both remain blocked upstream, checked the same day: linuxserver/docker-sabnzbd#275 is still open and the shipped `run` still forces `--server` unconditionally, and mylar3's fix is on `stable` but its image builds from the `v0.9.0` release tag, so even the newest `ls257` rebuild does not carry it. * fix: name the jdownloader-2 secrets so the image's own loader finds them Dropping `patches/jdownloader2/10-webauth.sh` would have broken web authentication, and the review caught it. The patch read `/run/secrets/jdownloader2_username` and `_password` directly, so the compose secret names never mattered. The image's own loader is not so relaxed. `/init`'s cont-secrets stage does: find /run/secrets -maxdepth 1 -type f -name "CONT_ENV_*" env_var_name="${fpath#/run/secrets/CONT_ENV_}" It only sees files whose name starts with `CONT_ENV_`, and derives the environment variable by stripping exactly that prefix. Secrets called `jdownloader2_username` and `jdownloader2_password` are invisible to that loop, so `WEB_AUTHENTICATION_USERNAME` and `WEB_AUTHENTICATION_PASSWORD` would have stayed at the image's empty defaults and the web UI would have come up with no credential: the exact failure the patch existed to prevent, reintroduced by removing it. They are now named `CONT_ENV_WEB_AUTHENTICATION_USERNAME` and `CONT_ENV_WEB_AUTHENTICATION_PASSWORD`. The host paths are unchanged, so `rotate-passwords.sh` and its test, which reference `configs/jdownloader2/secrets/password.txt`, are unaffected. Renaming rather than using the long-syntax `target:` because nothing else in this repository uses that form and podman's support for it is inconsistent. `test_rotate_jdownloader2_password` is the check that matters here: it rotates the password and then performs a real login against the running container, asserting HTTP 200. `WEB_AUTHENTICATION=1` comes from the tracked `configs/jdownloader2/.env`, not from the image, whose own default is 0, so that test is not vacuous in CI. Also picks up three stale references the review flagged and the previous commit missed: `renovate.json5` still described four held images and still said jdownloader-2 opens no pull requests, and `DEPENDENCY_UPDATES.md` still said each of four patches has an open issue.
Collaborator
|
This pull request has been automatically marked as stale because it has not had recent activity. This might be due to missing feedback from OP. It will be closed if no further activity occurs. Thank you for your contributions. |
Author
|
@LinuxServer-CI keep it open please. |
SABnzbd persists --server into sabnzbd.ini, so almost every existing config holds host = ::, the value this script imposes by default. Respecting it as a configured host bypassed the fallback to 0.0.0.0 on a host without IPv6, where SABnzbd would then fail to bind. :: now goes through the same path as an unset or loopback host, so FAMILY decides it exactly as before. A deliberate 0.0.0.0 or a specific address is still respected.
Author
|
@thespad could you take a look when you get a chance? It's a small change, two files. Since opening this I pushed one fix. I also ran it in the real image ( The red |
pull Bot
pushed a commit
to abraxas678/docker-torrent-box-with-vpn
that referenced
this pull request
Sep 27, 2026
…tti-labs#231) * fix(devcontainer): allow the ubuntu egress set for the L2 image build .devcontainer/l2/Dockerfile installs shfmt with apt, but egress-sets did not list ubuntu, so the proxy refused archive.ubuntu.com and security.ubuntu.com with a 403 and the L2 image could not be built in a fresh engine. Every commit from the workbench failed at the pre-commit hook as a result. * fix(sabnzbd): treat :: as unset in the vendored run script SABnzbd persists --server into sabnzbd.ini, so almost every existing config holds host = ::, the image's own default. The patch respected it as a configured host, which bypassed the fallback to 0.0.0.0 on a machine without IPv6, where SABnzbd would then fail to bind. :: now takes the same path as an unset or loopback host. A deliberate host = 0.0.0.0, which this stack sets, is still respected. Mirrors the same fix pushed to linuxserver/docker-sabnzbd#275, so the code stays identical to the upstream proposal. * revert: drop the redundant ubuntu egress set The workbench already adds ubuntu and nodesource to the proxy for any checkout with a .devcontainer/l2/Dockerfile, so listing it here changes nothing. The 403 that prompted it came from starting the workbench against a main clone old enough to predate the L2 Dockerfile, while the commit ran in a current worktree. Not a defect in this repository. This reverts commit 2944fe8.
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
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:
root/etc/s6-overlay/s6-rc.d/svc-sabnzbd/runalways passes--server "$FAMILY"to SABnzbd, which overrides whatever host is stored insabnzbd.iniand is then written straight back into it. This makes that override conditional. Before theexec, the script reads thehostkey out of the[misc]section of/config/sabnzbd.ini. If that value is empty, loopback (127.0.0.1,::1,localhost) or::(the script's own default, which SABnzbd saves back into the ini),$FAMILYis imposed exactly as before. If it is anything else, that value is passed back through--server, so the flag becomes a no-op and the configured host survives the restart.The check runs once in each branch rather than once above the
if, because only the root branch mutates$FAMILYfor the IPv6 fallback, and that mutation happens after where a shared check would sit. A single check ahead of theifwould let the fallback clobber a configured host on a machine without IPv6, which is the bug this is meant to fix.Reading the ini tracks the current section, because the NNTP server entries under
[servers]carry their own unindentedhost = ...line and matching the wrong one would hand a news server's hostname to--server.Benefits of this PR and context:
closes #274
This is a narrower version of something raised several times before, most recently in #240, and closed as intended behavior. I am not asking to revisit that decision. Forcing the bind address away from loopback still happens exactly as it does today, for exactly the reason given there, so nobody locks themselves out of the web UI by binding to
127.0.0.1. This only stops overriding a host that is already something other than unset or loopback, which cannot be the lockout case by definition.The setup it unblocks is running sabnzbd with
network_mode: container:<other container>, sharing another container's network namespace, which is the usual way to put a download client behind a VPN container for a kill switch. There is no bridge network to put a reverse proxy on and no port to map in that mode, because the network stack belongs to the other container, so the host has to be set directly and needs to survive a restart.How Has This Been Tested?
shellcheck -s bashagainst the modified script, clean, andbash -nfor syntax. The extraction was checked against four ini shapes:[misc]before[servers],[servers]before[misc], a loopback value, and nohostkey at all. The reordered case is the one that matters, since a plaingrep '^host'returns the news server there.Also run in
lscr.io/linuxserver/sabnzbd:5.1.3-ls274with the script mounted over the shipped one: a fresh config,0.0.0.0,::and127.0.0.1all start, serve the web UI, and keep the expected host across a restart.Source / References:
#240, #116, and the earlier reports of the same behaviour in #2, #35 and #42.