fix(docs): stop telling people to publish the worker API on every interface - #280
Conversation
…erface
README's "Adding remote workers" snippet told the reader to run the worker with
a bare `"8081:8081"`. Docker reads that as 0.0.0.0, and the worker's API is
backed by the Docker socket -- deploy, stop or remove ANY container -- so
following the README exposed full control of that host to the whole network.
The shipped docker-compose.fleet.yml has bound it to
${CASHPILOT_WORKER_BIND_ADDR:-127.0.0.1}:8081:8081 all along, with a comment
saying it must never be published.
THIS WAS FIXED ONCE ALREADY, IN THE WRONG PLACE. docs/getting-started.md had the
identical defect and was corrected, guarded by two tests. Both read
GETTING_STARTED by name -- so the file most people read FIRST was never covered
and kept the defect.
Naming one page was the bug. The checks now sweep EVERY shipped doc, and found
a third instance immediately: docs/fleet.md was still pinning :latest.
Also corrects the pins to the 1.19 series, matching the shipped compose files
and SECURITY.md, which says :latest "makes what you are running unknowable and
can carry a breaking change into a routine pull".
CHANGELOG is excluded from the sweep on purpose: it records what WAS true, and
rewriting history to satisfy a linter would make it useless.
Six controls, because a decorative security check is worse than none -- it gets
cited as evidence. They prove the port pattern matches a bare publish, accepts
an address-scoped one, and ignores prose naming the port; and that the :latest
pattern matches a real image line while ignoring SECURITY.md discussing
`:latest` in order to warn about it.
Mutation-tested: reintroducing the bare publish in README fails the sweep.
4347 passed.
|
Warning Review limit reached
Next review available in: 46 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
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 |
The comment introduced in #280 cited CashPilot-o6mu, which does not exist. The bead is CashPilot-wqkd. A wrong ID is worse than none: it sends the next reader looking for a record that was never there, and it looks authoritative while doing it.
DECIDED 2026-08-07 by Sergio: keep :latest. CashPilot-3c2n asked the question with three options; this is option 1, and the work is making the choice legible rather than changing it. The reasoning, recorded beside the tag itself: Community Applications is how unraid users receive updates at all. A template pinned to 1.19 leaves every CA user on 1.19 until the template is re-published -- trading "unknowable version" for "silently frozen version", which is worse for an app whose whole job is to keep earning. Everywhere else in the project still pins major.minor, and #280's sweep still fails any DOC that ships :latest. The tag alone reads as an oversight. Without the comment beside it, the next person tidying the repository removes it in good faith and quietly changes how every unraid user upgrades -- so a test now asserts both that the templates use :latest AND that they explain why and point at the decision record. Controlled: pinning one template to 1.19 fails the test with a message explaining what that would do. Also asserts the templates parse as XML, since a malformed one is rejected by CA with no useful message, and that there ARE templates to check -- otherwise every assertion would hold vacuously.
1 participant
Found while checking
9fg's "do any docs carry stale pins?" question. The pin was the small half.The defect
README's "Adding remote workers" snippet told the reader to run:
Docker reads a bare
"8081:8081"as 0.0.0.0. The worker's API is backed by the Docker socket — deploy, stop or remove any container — so anyone following the README to add a remote worker exposed full control of that host to their entire network.The shipped
docker-compose.fleet.ymlhas bound it to${CASHPILOT_WORKER_BIND_ADDR:-127.0.0.1}:8081:8081all along, with a comment saying it must never be published.This was fixed once already, in the wrong place
docs/getting-started.mdhad the identical defect, was corrected, and is guarded by two tests:test_the_page_no_longer_publishes_the_worker_porttest_the_page_no_longer_pins_latestBoth read
GETTING_STARTEDby name. So the file most people read first was never covered, and kept the defect.Naming one page was the bug. The checks now sweep every shipped doc — and found a third instance immediately:
docs/fleet.mdwas still pinning:latest.Also
Pins corrected to the
1.19series, matching the shipped compose files and SECURITY.md, which says:latest"makes what you are running unknowable and can carry a breaking change into a routine pull".CHANGELOGis excluded from the sweep on purpose: it records what was true, and rewriting history to satisfy a linter would make it useless.Controls
Six of them, because a decorative security check is worse than none — it gets cited as evidence. They prove the port pattern matches a bare publish, accepts an address-scoped one, and ignores prose naming the port; and that the
:latestpattern matches a realimage:line while ignoring SECURITY.md discussing:latestin order to warn about it.Mutation-tested: reintroducing the bare publish in README fails the sweep.
4347 passed, coverage 95.57%,
mkdocs --strictbuilds, ruff clean.Filed separately, not bundled
unraid/cashpilot.xmlandunraid/cashpilot-worker.xmlboth declare<Repository>...:latest</Repository>. That may well be defensible — Community Applications is how unraid users receive updates at all — but it contradicts the same rule and deserves its own decision rather than being swept in here.