diff --git a/.githooks/commit-msg b/.githooks/commit-msg new file mode 100755 index 00000000..71a672c5 --- /dev/null +++ b/.githooks/commit-msg @@ -0,0 +1,24 @@ +#!/bin/sh +# Refuse a commit message carrying a Claude session link. +# +# Commit messages are published. A session URL in one is a link into a private +# transcript, pasted under Eric's name, in a place that cannot be edited after +# it lands on main. It has happened more than once, because the agent harness +# supplies an attribution block that includes the link and a model following +# that instruction never sees this rule. +# +# So it is enforced here rather than remembered: the commit fails, the message +# is left for editing, and the fix is to delete the line. +# +# Install (once per clone): git config core.hooksPath .githooks + +if grep -nE 'claude\.ai/(code/)?session|Claude-Session:' "$1" >/dev/null 2>&1; then + echo "commit-msg: this message contains a Claude session link." >&2 + echo >&2 + grep -nE 'claude\.ai/(code/)?session|Claude-Session:' "$1" >&2 + echo >&2 + echo "Commit messages are public and permanent. Remove the line and commit again." >&2 + echo "Co-Authored-By is fine; the session URL is not." >&2 + exit 1 +fi +exit 0 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 82b78da9..508abcd0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -63,6 +63,19 @@ jobs: node "$f" done + # The hook only protects a clone that has run `git config core.hooksPath + # .githooks`. This is the backstop for one that has not. + - name: No session links in commit messages + if: github.event_name == 'pull_request' + run: | + if git log --format='%H%n%B' \ + "origin/${{ github.base_ref }}..${{ github.event.pull_request.head.sha }}" \ + | grep -n 'claude\.ai'; then + echo "::error::a commit message on this branch carries a Claude session link" + exit 1 + fi + echo "no session links" + - name: Verify CLI entry point run: zimi --help diff --git a/CHANGELOG.md b/CHANGELOG.md index 0603b5ff..67d692e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,23 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/), and this project adheres to [Semantic Versioning](https://semver.org/). +## [1.9.1] - 2026-09-06 + +Four fixes for what the first day of 1.9.0 turned up, one of them a security fix. + +### Security + +- **A reverse proxy on the same machine could hand out the first admin password (GHSA-5mw2-53vv-9pw6, again).** The bootstrap window waives the setup key for the machine running Zimi, and asked the resolved client address whether it was on that machine. Behind a reverse proxy on the same host, which is the standard NAS deployment, the forwarded address is correctly refused as a trusted-tier claim and the resolved address falls back to the proxy's own: loopback. Every remote client through that proxy read as being on the host and skipped the key. Being the host is now asked of the socket, and a forwarded request is never the host whatever the socket says. Anyone who ran 1.9.0 behind a same-host proxy with no admin password should set one. + +### Added + +- **`lan_admin`, for running with no password at all (#59).** The advisory closed a default that let an adjacent device race the owner for the first password. It also removed a way people genuinely run Zimi: one household, one LAN, no password. `lan_admin` (or `ZIMI_LAN_ADMIN=1`) says that the private network is a boundary you trust and restores the pre-1.9.0 behaviour. Off unless you turn it on, and it applies only while no password is set. It means a direct connection from that network: a request through a reverse proxy does not qualify, since Zimi cannot tell one client of a proxy from another. + +### Fixed + +- **The moon was upside down for half of every month (#60).** The sprite shades from a Sun vector already flipped for a waning moon, and the bright-limb angle carries that same flip, so every waning moon was turned a further 180 degrees: lit limb on the wrong side, maria inverted. The month's other half was right, which is why it read as random. The four places that draw a moon all agreed with each other and all agreed on the wrong number, so the test that compared them could not see it; there is now one that checks the answer against the sky instead. +- **`zimi import --setup` could set up a sidecar the server never looks at (#61).** It resolves its own data dir from the shell it runs in, so run from a terminal without the service's configuration it installs into a different library's state directory, reports success, and leaves the alive engine greyed out with nothing on screen to explain it. The Create page now names this server's directory in the command it gives you, so what you paste lands where the server looks. + ## [1.9.0] - 2026-09-04 Zimi runs from a folder of ZIMs with no configuration, on a stick or a NAS or a fleet, and it makes ZIMs now. diff --git a/README.md b/README.md index 5325ec66..8abb9b54 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ # Zimi [![CI](https://github.com/epheterson/Zimi/actions/workflows/ci.yml/badge.svg)](https://github.com/epheterson/Zimi/actions/workflows/ci.yml) -[![Tests](https://img.shields.io/badge/tests-2652-brightgreen)](#) +[![Tests](https://img.shields.io/badge/tests-2655-brightgreen)](#) [![Lighthouse Accessibility](https://img.shields.io/badge/Lighthouse%20a11y-100%2F100-success?logo=lighthouse&logoColor=white)](docs/plans/2026-04-26-accessibility.md) [![WCAG 2.1 AA](https://img.shields.io/badge/WCAG%202.1-AA-blue)](docs/plans/2026-04-26-accessibility.md) [![i18n](https://img.shields.io/badge/i18n-10%20languages-blueviolet)](#languages) diff --git a/docs/deployment-networking.md b/docs/deployment-networking.md index e7581a1e..fe8f64f0 100644 --- a/docs/deployment-networking.md +++ b/docs/deployment-networking.md @@ -223,6 +223,27 @@ Every key is optional. The four path/bind keys have matching CLI flags; the rest | `sso_aud` | `ZIMI_SSO_AUD` | string — the Access application's AUD tag | | `sso_role` | `ZIMI_SSO_ROLE` | string — `user` (default), `limited` or `admin`, given to an account on first sign-in | | `sso_proxy` | `ZIMI_SSO_PROXY` | list of CIDRs (a comma-separated string also works) — who may send the identity header; default any private peer | +| `lan_admin` | `ZIMI_LAN_ADMIN` | boolean — treat any private-network client as the admin on a **passwordless** instance; off by default, see [Running without a password](#running-without-a-password) | + + +### Running without a password + +A passwordless Zimi is a real way to run it: one household, one LAN, nothing to type. Up to 1.8.2 that is what you got — any client on a private network was the admin. + +That default had a hole ([GHSA-5mw2-53vv-9pw6](https://github.com/epheterson/Zimi/security/advisories)): "on a private network" includes every other device on the LAN, a Docker bridge, and anything on your tailnet, so an adjacent device could claim the first admin password before you did and lock you out of your own library. From 1.9.0 the bootstrap window is narrower: the machine running Zimi sets the first password with no secret, and any other device must present a one-time setup key the server prints on its first start. + +If your threat model does not include the other devices on your own network, say so explicitly: + +```yaml +# zimi.json +{ "lan_admin": true } +``` + +or `ZIMI_LAN_ADMIN=1`. Any private-network client is then the admin again, exactly as before 1.9.0, and no password is needed at all. + +It is off unless you turn it on, and it applies only while no admin password is set. Once there is a password, that password governs. Turn it on when the LAN is a boundary you trust; leave it off on a shared, office, or campus network, where "private address" and "people you trust" are not the same set. + +It also means a **direct** connection from your network. A request that arrived through a reverse proxy does not qualify, even one on the same machine, because Zimi cannot tell one client of that proxy from another: the forwarded address is not trustworthy, and the address it falls back to is the proxy's own. If you reach Zimi through a proxy, set an admin password rather than turning this on. A setting from the file is applied by exporting it into its environment variable at startup, and only ever when the file is the layer that won — so an environment variable you exported yourself is never overwritten, and a setting you left out stays genuinely unset rather than being pinned to its default. diff --git a/pyproject.toml b/pyproject.toml index 83bf9d4d..8e2b75b5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "zimi" -version = "1.9.0" +version = "1.9.1" description = "Offline knowledge server for ZIM files — search and read Wikipedia, Stack Overflow and 50+ sources with no internet" readme = "README.md" license = {text = "MIT"} diff --git a/tests/test_bootstrap_takeover.py b/tests/test_bootstrap_takeover.py index c714f1e0..10986f5e 100644 --- a/tests/test_bootstrap_takeover.py +++ b/tests/test_bootstrap_takeover.py @@ -53,15 +53,24 @@ def setUp(self): threading.Thread(target=self._srv.serve_forever, daemon=True).start() self._base = f"http://127.0.0.1:{self._srv.server_address[1]}" self._real_client_ip = zhttp.ZimHandler._client_ip + self._real_socket_peer_ip = zhttp.ZimHandler._socket_peer_ip def _as_peer(self, ip): """Make every request for the rest of this test appear to come from ``ip`` — a real non-loopback peer, which a forwarded header cannot - fake past the anti-spoof rule.""" + fake past the anti-spoof rule. + + The SOCKET moves too, not just the resolved client IP. Stubbing + _client_ip alone left the connection genuinely on loopback, so these + tests were modelling a remote attacker who, to the code that decides + who counts as the host, was sitting at the machine. That is the gap + the same-host reverse proxy fell through.""" zhttp.ZimHandler._client_ip = lambda _self, _ip=ip: _ip + zhttp.ZimHandler._socket_peer_ip = lambda _self, _ip=ip: _ip def tearDown(self): zhttp.ZimHandler._client_ip = self._real_client_ip + zhttp.ZimHandler._socket_peer_ip = self._real_socket_peer_ip self._srv.shutdown() manage._env_pw_hash_cache = None import shutil @@ -130,6 +139,90 @@ def test_the_host_itself_bootstraps_freely(self): self.assertEqual(status, 200, body) self.assertTrue(manage._get_manage_password_hash()) + def test_a_same_host_reverse_proxy_does_not_make_everyone_the_host(self): + """The advisory's fix, reopened by the commonest deployment there is. + + These tests replace _client_ip wholesale, so until this one nothing + ever executed the function that decides who counts as the host. In + production a reverse proxy on the SAME machine — Synology, nginx in + front of 8899, the usual NAS shape — connects from 127.0.0.1 and puts + the real client in X-Forwarded-For. _client_ip refuses to let that + header claim a trusted-tier address, so it falls back to the direct + peer, which is loopback: every remote client behind that proxy became + the host and skipped the setup key. + + So this test does NOT stub the peer. The socket really is loopback, + exactly as it is in that deployment, and the forwarded header is the + only thing distinguishing it from the owner sitting at the machine. + """ + for header in ("X-Forwarded-For", "X-Real-IP", "CF-Connecting-IP"): + with self.subTest(header=header): + status, body = self._post( + "/manage/set-password", + {"password": "attacker-owns-it"}, + headers={header: "192.168.1.50"}, + ) + self.assertEqual(status, 403, body) + self.assertFalse( + manage._get_manage_password_hash(), + f"a client forwarded by {header} claimed the first password", + ) + + def test_the_lan_can_be_trusted_but_only_on_purpose(self): + """Issue #59: 1.9.0 removed a way people actually run Zimi. + + Before the advisory, a passwordless instance treated any private + client as admin, and plenty of single-household servers depended on + that: no password, LAN only, done. The fix was right and the + replacement was missing, so those users found Settings simply shut. + + The opt-in has to be typed by whoever runs the server, and with it off + — the default, and what every other test here exercises — the LAN is + still refused.""" + self._as_peer(ADJACENT) + status, _ = self._post("/manage/set-password", {"password": "nope"}) + self.assertEqual(status, 403, "the default must still refuse the LAN") + + os.environ["ZIMI_LAN_ADMIN"] = "1" + try: + status, body = self._get("/manage/stats") + self.assertEqual(status, 200, body) + finally: + os.environ.pop("ZIMI_LAN_ADMIN", None) + + status, _ = self._get("/manage/stats") + self.assertEqual(status, 403, "switching it back off must shut the door") + + def test_lan_admin_does_not_hand_the_internet_the_keys(self): + """The escalation `lan_admin` would otherwise carry. + + Behind a reverse proxy on the same host, _client_ip cannot identify + the caller: it refuses the forwarded address as a trusted-tier claim + and falls back to the hop, which is loopback. Every client of that + proxy therefore resolves as "private" — including one on the far side + of the internet. Left at `_is_private_client`, turning on lan_admin + would have made all of them the admin of a passwordless server. + """ + os.environ["ZIMI_LAN_ADMIN"] = "1" + try: + status, body = self._post( + "/manage/set-password", + {"password": "attacker-owns-it"}, + headers={"X-Forwarded-For": "8.8.8.8"}, + ) + self.assertEqual(status, 403, body) + self.assertFalse( + manage._get_manage_password_hash(), + "lan_admin let a forwarded client claim the first password", + ) + # And a genuinely direct private peer still gets in, which is the + # entire point of the setting. + self._as_peer(ADJACENT) + status, body = self._get("/manage/stats") + self.assertEqual(status, 200, body) + finally: + os.environ.pop("ZIMI_LAN_ADMIN", None) + def test_a_remote_client_with_the_key_bootstraps_and_spends_it(self): key = manage.ensure_setup_key() self.assertTrue(key) diff --git a/tests/test_ci_contract.py b/tests/test_ci_contract.py index d68b1fe3..0bc8d15f 100644 --- a/tests/test_ci_contract.py +++ b/tests/test_ci_contract.py @@ -27,6 +27,7 @@ import re import shutil import subprocess +import tempfile import pytest @@ -191,3 +192,65 @@ def test_the_pr_gate_runs_on_every_runner_the_release_builds_on(): f"the release: {sorted(release - gate) or 'none'}. A platform in the " "release alone is one whose failures cannot be seen before the tag." ) + + +def test_the_repo_refuses_commit_messages_with_session_links(): + """A session URL in a commit message is a link into a private transcript, + published under the repo owner's name, in the one place that cannot be + edited after it lands on main. + + It is enforced by a hook rather than remembered because the agent harness + supplies an attribution block containing that link, and anything following + that instruction never sees the rule. This test is what keeps the hook + itself from being deleted or quietly stopping working.""" + hook = ROOT / ".githooks" / "commit-msg" + assert hook.is_file(), "the commit-msg hook is gone" + if os.name != "nt": + assert os.access(hook, os.X_OK), "the commit-msg hook is not executable" + + # Run it THROUGH sh rather than as a program. It is a `#!/bin/sh` script, + # and Windows does not read shebangs: executing it directly raises + # "[WinError 193] %1 is not a valid Win32 application". Git for Windows + # ships the sh that git itself uses to run hooks, so this is also how the + # hook actually runs on that platform. + shell = shutil.which("sh") or shutil.which("bash") + if not shell: + pytest.skip("no POSIX shell here to run the hook with") + + def run(message): + with tempfile.NamedTemporaryFile("w", suffix=".msg", delete=False) as fh: + fh.write(message) + path = fh.name + try: + return subprocess.run( + [shell, str(hook), path], capture_output=True, text=True + ) + finally: + os.unlink(path) + + bad = run("a change\n\nClaude-Session: https://claude.ai/code/session_x1\n") + assert bad.returncode != 0, "the hook let a session link through" + + plain_url = run("a change\n\nsee https://claude.ai/code/session_x1 for context\n") + assert plain_url.returncode != 0, "the hook only catches the trailer form" + + good = run( + "a change\n\nCo-Authored-By: Claude \n" + ) + assert good.returncode == 0, f"the hook rejected a clean message: {good.stderr}" + + +def test_no_commit_on_this_branch_carries_a_session_link(): + """The hook stops new ones; this catches any that predate it, while the + branch can still be rewritten.""" + done = subprocess.run( + ["git", "log", "origin/main..HEAD", "--format=%H%n%B"], + capture_output=True, text=True, cwd=ROOT, + ) + if done.returncode != 0: + pytest.skip("no origin/main to compare against here") + offenders = [ln for ln in done.stdout.splitlines() if "claude.ai" in ln] + assert not offenders, ( + "commit message(s) on this branch carry a session link:\n " + + "\n ".join(offenders) + ) diff --git a/tests/test_create_jobs.py b/tests/test_create_jobs.py index dcf399c8..687f0fd6 100644 --- a/tests/test_create_jobs.py +++ b/tests/test_create_jobs.py @@ -37,12 +37,19 @@ ) -def _wait(predicate, tries=600, why="condition never came true"): - for _ in range(tries): +# A deadline, not a poll count — same reason as _wait_done in +# test_create_routes.py: a fixed number of turns measures how fast the runner +# is, not how long the job took, so a loaded machine fails a passing test. +_WAIT_SECONDS = 30 + + +def _wait(predicate, timeout=_WAIT_SECONDS, why="condition never came true"): + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: if predicate(): return True time.sleep(0.01) - raise AssertionError(why) + raise AssertionError(f"{why} (waited {timeout}s)") @pytest.fixture diff --git a/tests/test_create_routes.py b/tests/test_create_routes.py index a0e4c4ca..cc6d8f13 100644 --- a/tests/test_create_routes.py +++ b/tests/test_create_routes.py @@ -59,13 +59,27 @@ def _get(path, private=True, params=None): return h -def _wait_done(tries=400): - for _ in range(tries): +# A deadline, not a poll count. 400 polls at 10ms looks like four seconds and +# is not: each turn of the loop also makes an HTTP request, so the real ceiling +# is however fast the runner happens to be. On a contended macOS runner the +# 550-line buffer test ran out of turns and failed as "job never finished" — +# the same commit passed on the next run, which is the signature of a limit +# that measures the machine rather than the job. +# +# Time-based instead, and generous: a passing test still returns the moment the +# job is done, so the only thing that waits 30 seconds is a job that genuinely +# hung, which is worth 30 seconds to report honestly. +_WAIT_DONE_SECONDS = 30 + + +def _wait_done(timeout=_WAIT_DONE_SECONDS): + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: body = _get("/manage/create/status").body if body.get("done") or not body.get("active"): return body time.sleep(0.01) - raise AssertionError("creation job never finished") + raise AssertionError(f"creation job never finished within {timeout}s") @pytest.fixture(autouse=True) diff --git a/tests/test_manage_sections.py b/tests/test_manage_sections.py index 091910fe..96c7291a 100644 --- a/tests/test_manage_sections.py +++ b/tests/test_manage_sections.py @@ -62,7 +62,15 @@ def test_creator_payload_answers_every_question_the_section_asks(monkeypatch): assert body["browser_ready"] is True assert body["alive_ready"] is False assert body["create_root"] == "/srv/zims" - assert set(body["sidecar"]) == {"installed", "version"} + # "dir" is part of the contract, not incidental: it is where THIS server + # looks for the sidecar, and the Create page puts it into the install + # command it offers. Without it the command is `zimi import --setup`, which + # resolves whatever data dir the operator's shell resolves — a different + # library's, if that shell lacks the service's config, which installs a + # working sidecar somewhere the server never reads (issue #61). + assert set(body["sidecar"]) == {"installed", "version", "dir"} + assert body["sidecar"]["dir"], "the server must say where it looks" + assert body["sidecar"]["dir"].endswith(os.path.join("tools", "warc2zim")) # Every type is present in the breakdown even when the library is empty, so # the client never has to guess a missing bucket is zero. inv = _get("/manage/creator/inventory").body diff --git a/tests/test_moon_derivation.cjs b/tests/test_moon_derivation.cjs index 391476e8..757e062e 100644 --- a/tests/test_moon_derivation.cjs +++ b/tests/test_moon_derivation.cjs @@ -109,5 +109,73 @@ const nw = vm.runInContext('_moonPhase(new Date(Date.UTC(2026, 0, 18, 19, 0)))', check(full.illumination > 97, 'known full moon reads > 97% (' + full.illumination + '%)'); check(nw.illumination < 3, 'known new moon reads < 3% (' + nw.illumination + '%)'); +// ── 4. CORRECTNESS, not just agreement ────────────────────────────────────── +// +// Everything above checks that the four renderers compute the SAME tilt. They +// did, and it was wrong for half of every month (issue #60): the sprite shades +// from a Sun vector already flipped by the waxing flag, and chi carries that +// same flip, so every waning moon was turned a further 180 degrees. The lit +// limb sat on the wrong side and the maria were upside down. Four renderers +// agreeing on one wrong number is exactly what a consistency test cannot see. +// +// The invariant with a known answer: put the Moon on the observer's meridian +// at a quarter phase. The Sun is then roughly 90 degrees away along the +// horizon, so the lit limb lies close to horizontal and the sprite — already +// lit on the correct side — needs almost no rotation. True at BOTH quarters. +function haDeg(t, lon) { + const eq = vm.runInContext('_moonEqCoords(new Date(' + t + '))', sandbox); + const gmst = (280.46061837 + 360.98564736629 * (eq.JD - 2451545.0)) % 360; + let ha = ((gmst + lon) - eq.ra * 180 / Math.PI) % 360; + if (ha > 180) ha -= 360; + if (ha < -180) ha += 360; + return ha; +} +function phaseAt(t) { + return vm.runInContext('_moonPhase(new Date(' + t + '))', sandbox).phase; +} +function nearestMeridianQuarter(target, lon) { + let best = null; + for (let m = 0; m < 70 * 24 * 60; m += 10) { + const t = Date.UTC(2026, 8, 1) + m * 60000; + const score = Math.abs(haDeg(t, lon)) + Math.abs(phaseAt(t) - target) * 720; + if (!best || score < best.score) best = { t, score }; + } + return best.t; +} +for (const [label, target] of [['first quarter (waxing)', 0.25], + ['last quarter (waning)', 0.75]]) { + for (const loc of [{ lat: 51.5, lon: -0.12 }, { lat: 40.7, lon: -74.0 }]) { + const t = nearestMeridianQuarter(target, loc.lon); + const raw = vm.runInContext( + '_moonScreenTiltDeg(new Date(' + t + '), ' + loc.lat + ', ' + loc.lon + ')', sandbox); + let tilt = ((raw % 360) + 360) % 360; + if (tilt > 180) tilt -= 360; + check(Math.abs(tilt) < 45, + label + ' on the meridian at lat ' + loc.lat + ' needs little rotation (got ' + + tilt.toFixed(1) + ' deg; ~180 means the disc is upside down)'); + } +} + +// The tilt may step only where the disc carries no visible phase. The waning +// correction turns over at new moon, on a 0%-lit disc; anywhere else a jump +// would be a real artifact somebody would watch happen on the time machine. +let worstJump = 0, worstIllum = 100, prevTilt = null; +for (let m = 0; m < 30 * 24 * 60; m += 5) { + const t = Date.UTC(2026, 8, 1) + m * 60000; + const v = vm.runInContext('_moonScreenTiltDeg(new Date(' + t + '), 51.5, -0.12)', sandbox); + if (prevTilt !== null) { + let d = v - prevTilt; + d = ((d % 360) + 540) % 360 - 180; + if (Math.abs(d) > 5) { + const illum = vm.runInContext('_moonPhase(new Date(' + t + '))', sandbox).illumination; + if (Math.abs(d) > worstJump) { worstJump = Math.abs(d); worstIllum = illum; } + } + } + prevTilt = v; +} +check(worstJump === 0 || worstIllum < 1, + 'any tilt step lands on an unlit disc (worst ' + worstJump.toFixed(0) + + ' deg at ' + worstIllum + '% lit)'); + if (failures) { console.error(failures + ' failure(s)'); process.exit(1); } console.log('all moon derivation checks passed'); diff --git a/tests/test_sidecar_install_hint.cjs b/tests/test_sidecar_install_hint.cjs new file mode 100644 index 00000000..fe5249a6 --- /dev/null +++ b/tests/test_sidecar_install_hint.cjs @@ -0,0 +1,84 @@ +// The install command the Create page offers must target THIS server. +// +// `zimi import --setup` resolves its own data dir from the shell it is run in. +// Run from a terminal that does not carry the service's config it resolves a +// different one, installs a working sidecar into it, prints "sidecar ready", +// and leaves the engine greyed out with nothing on screen to explain the gap +// (issue #61: set up against the default /zims while the service served +// /mnt/nas/ZIM). Naming the server's own directory in the command is what +// closes that, so the command has to keep naming it. +// +// Run: node tests/test_sidecar_install_hint.cjs (exit 0 = pass) + +const fs = require('fs'); +const path = require('path'); +const vm = require('vm'); + +const src = fs.readFileSync( + path.join(__dirname, '..', 'zimi', 'static', 'create.js'), 'utf8'); + +let failures = 0; +function check(ok, label) { + if (!ok) { console.error('FAIL: ' + label); failures++; } + else console.log('ok: ' + label); +} + +function extractFn(s, name) { + const i = s.indexOf('function ' + name + '('); + if (i < 0) throw new Error(name + ' not found'); + let j = s.indexOf('{', i), d = 0; + for (; j < s.length; j++) { + if (s[j] === '{') d++; + else if (s[j] === '}' && --d === 0) return s.slice(i, j + 1); + } + throw new Error('unbalanced braces in ' + name); +} + +const sandbox = { + console, + CREATE_PART_INSTALL: { sidecar: 'zimi import --setup' }, + _createSidecarDir: null, +}; +vm.createContext(sandbox); +vm.runInContext(extractFn(src, '_createShellQuote'), sandbox); +vm.runInContext(extractFn(src, '_createSidecarCommand'), sandbox); + +function cmd(dir) { + sandbox._createSidecarDir = dir; + return vm.runInContext('_createSidecarCommand()', sandbox); +} + +check(cmd(null) === 'zimi import --setup', + 'with no reported directory the command stays the plain one'); + +const nasDir = '/home/pi/.cache/zimi/zims-5fa07e4eab/tools/warc2zim'; +check(cmd(nasDir) === 'zimi import --setup --data-dir /home/pi/.cache/zimi/zims-5fa07e4eab', + 'the reported sidecar dir becomes --data-dir, without the tools/warc2zim tail'); + +check(cmd('/srv/my zims/.zimi/tools/warc2zim') === + "zimi import --setup --data-dir '/srv/my zims/.zimi'", + 'a path with a space is quoted, so the command survives a paste'); + +// Single quotes, not double: a shell expands $HOME and backticks inside double +// quotes, so a double-quoted path containing either pastes as a DIFFERENT path. +check(cmd('/srv/$USER/.zimi/tools/warc2zim') === + "zimi import --setup --data-dir '/srv/$USER/.zimi'", + 'a path a shell would expand is quoted so it cannot be expanded'); + +check(cmd("/srv/it's/.zimi/tools/warc2zim") === + "zimi import --setup --data-dir '/srv/it'\\''s/.zimi'", + 'an embedded single quote is escaped the way a shell accepts'); + +check(cmd('/var/lib/zimi') === 'zimi import --setup', + 'a reported path that is not a sidecar venv is not guessed at'); + +check(cmd(nasDir).indexOf('tools') < 0, + '--data-dir names the data dir, not the venv inside it'); + +// The whole point is that the server states this. If the probe stops carrying +// it the command silently reverts to the one that installs in the wrong place. +check(/sidecar_dir/.test(src), + 'create.js still reads sidecar_dir off the probe reply'); + +if (failures) { console.error(failures + ' failure(s)'); process.exit(1); } +console.log('all sidecar install hint checks passed'); diff --git a/zimi/http.py b/zimi/http.py index 965afc86..6236e92a 100644 --- a/zimi/http.py +++ b/zimi/http.py @@ -3026,19 +3026,98 @@ def _is_private_client(self): return False return _is_trusted_net(ip) + # Headers that mean "somebody forwarded this request". Presence alone + # disqualifies a claim of being ON the host, so the list is deliberately + # broader than the ones _client_ip actually reads: an unknown proxy that + # announces itself in any of these is still a proxy. + _FORWARDED_HEADERS = ( + "X-Forwarded-For", + "X-Real-IP", + "Forwarded", + "CF-Connecting-IP", + "True-Client-IP", + ) + + def _was_forwarded(self): + """True when something in front of Zimi passed this request along. + + Presence of the header is the whole test. Its VALUE cannot be trusted + — that is why _client_ip refuses a forwarded claim of a trusted-tier + address — but the fact that a hop announced itself is information the + hop had no reason to fake, and it is enough to know this request did + not come straight off the local network.""" + return any(self.headers.get(h) for h in self._FORWARDED_HEADERS) + + def _is_direct_private_client(self): + """A private-network peer that reached Zimi directly. + + `_is_private_client` asks about the RESOLVED address, and behind a + reverse proxy on the same host that resolution falls back to the + proxy's own loopback address — so every client of that proxy, from + anywhere on the internet, resolves as private. That is tolerable for + the things the private tier gates (rate limits, peer sharing) and not + tolerable for handing someone the admin of a passwordless instance, + which is what `lan_admin` does. + + So `lan_admin` asks this instead: a private peer, and nothing in + front. It is a narrower question, and it is the one the setting's own + wording promises.""" + return not self._was_forwarded() and self._is_private_client() + def _is_loopback_client(self): """True ONLY when the peer is the machine running Zimi (127.0.0.0/8, ::1). This is the bootstrap trust boundary — being ON the host is the one proof of ownership that needs no secret. A LAN or tailnet peer is 'private' but not the host, and must present the setup key instead (GHSA-5mw2-53vv-9pw6: private-tier was too wide a door for claiming - the first admin password).""" + the first admin password). + + A forwarded request is never the host, whatever the socket says. This + is the shape that reopened the advisory: a reverse proxy on the SAME + machine — the standard NAS deployment — connects from 127.0.0.1, and + _client_ip correctly refuses to let the forwarded header claim a + trusted-tier address, so it falls back to the direct peer. That peer + is loopback, and every remote client behind such a proxy read as being + on the host and skipped the setup key entirely. + + So the question is asked of the SOCKET, not of the resolved client IP, + and only when nothing forwarded the request. The host's own browser + reaching Zimi through its own proxy is caught by this too, and that is + correct: it is indistinguishable from any other client of that proxy, + and whoever is on the host can read the setup key out of the log. + + What this does NOT close, because loopback-as-proof cannot: a same-host + forwarder that sends no header at all — `socat`, or an nginx + `proxy_pass` with no `proxy_set_header` — still presents a bare + loopback peer, and there is nothing in the request to tell it apart + from the owner at the keyboard. Every mainstream reverse proxy sets a + forwarded header by default, so this covers the deployments people + actually have; closing the rest means retiring loopback-as-proof and + asking even the host for the setup key, which is a product decision + rather than a fix.""" + if self._was_forwarded(): + return False try: - ip = ipaddress.ip_address(self._client_ip()) + ip = ipaddress.ip_address(self._socket_peer_ip()) except ValueError: return False return ip.is_loopback + def _socket_peer_ip(self): + """The address on the other end of this TCP connection, forwarded + headers ignored. + + A seam, and a small one on purpose: `client_address` is set per + connection by socketserver, so a test cannot substitute it without + fighting the instance attribute. Everything that asks "who is + physically connected" goes through here, which is also what lets the + bootstrap tests model a genuinely remote peer instead of stubbing the + function whose answer they are checking.""" + try: + return self.client_address[0] + except (IndexError, TypeError): + return '' + def _peer_share_allowed(self): """True if this client may pull whole ZIMs from /dl/. diff --git a/zimi/manage.py b/zimi/manage.py index 473f5c41..24ad8c3a 100644 --- a/zimi/manage.py +++ b/zimi/manage.py @@ -227,6 +227,34 @@ def _clear_setup_key(): pass +def _lan_client(handler): + """A client `lan_admin` may treat as the owner: on the private network and + reaching Zimi directly. + + Not simply `_is_private_client`. Behind a reverse proxy on the same host + the resolved address falls back to the proxy's own loopback one, so every + client of that proxy resolves as private no matter where on the internet + it came from — and `lan_admin` would hand each of them the admin of a + passwordless instance. The setting says the LAN is the boundary; this is + that sentence, asked exactly.""" + direct = getattr(handler, "_is_direct_private_client", None) + return bool(direct() if direct else handler._is_private_client()) + + +def _lan_admin_allowed(): + """Whether the operator has said their LAN is their trust boundary. + + Read fresh rather than cached at import: `zimi config` publishes file + settings into the environment at startup, and a test that sets it wants it + to take effect.""" + return os.environ.get("ZIMI_LAN_ADMIN", "0").strip().lower() in ( + "1", + "true", + "yes", + "on", + ) + + def _bootstrap_key_ok(handler): """True when a remote bootstrap request carries the valid setup key, in the Authorization: Bearer header or an X-Zimi-Setup-Key header. Constant- @@ -331,8 +359,12 @@ def _primary_admin_authorized(handler): """ stored_pw = _get_manage_password_hash() if not stored_pw: - # Passwordless: LAN/loopback clients are the (only) primary admin. - return handler._is_private_client() + # Passwordless: the host itself, or any private client when the + # operator has opted into trusting the LAN (see _lan_admin_allowed). + if _lan_admin_allowed(): + return _lan_client(handler) + is_local = getattr(handler, "_is_loopback_client", handler._is_private_client) + return is_local() or _bootstrap_key_ok(handler) # A primary-admin SESSION token (users.create_admin_session): minted when the # admin password verified, delivered as the HttpOnly zimi_session cookie so @@ -429,6 +461,12 @@ def _check_manage_auth(handler): return None if _bootstrap_key_ok(handler): return None + # The operator's explicit "my LAN is my trust boundary" (issue #59). + # Off unless someone typed it, so the advisory's default stands; on, it + # restores the pre-1.9.0 behaviour for the people who ran Zimi that way + # deliberately and have no wish to hold an admin password. + if _lan_admin_allowed() and _lan_client(handler): + return None return PUBLIC_LOCKED if _primary_admin_authorized(handler) or _secondary_admin_authorized(handler): @@ -3394,6 +3432,13 @@ def _create_status(cursor, probe=False, events_cursor=0, history=False): if probe: # Only on the page's first poll: one cheap subprocess, not per-second. payload["import_ready"] = _create_import_ready() + # And WHERE this server keeps that sidecar, so the install command the + # page offers targets this instance rather than whatever data dir the + # operator's shell happens to resolve. `zimi import --setup` run from a + # shell without the service's config sets up a perfectly good sidecar + # for a different library, and the engine stays greyed out with nothing + # on screen to say why (issue #61). + payload["sidecar_dir"] = _create_sidecar_dir() # Whether the rendered engine's browser is installed here. Same # contract as import_ready: asked once, on the page's first poll, and # answered from a cache after that. @@ -3603,6 +3648,17 @@ def _create_root(): return os.path.realpath(os.path.expanduser(raw)) +def _create_sidecar_dir(): + """Where THIS server looks for the warc2zim sidecar, or None.""" + try: + from zimi.importer import sidecar_status + + return sidecar_status().get("dir") or None + except Exception: + log.exception("sidecar dir probe failed") + return None + + def _create_import_ready(): """True when the warc2zim sidecar is already installed — the one thing that decides whether archive import can run on a machine with no @@ -3802,6 +3858,14 @@ def _creator_payload(): sidecar = { "installed": bool(status.get("installed")), "version": status.get("version"), + # WHERE this server looks. The client pastes it into the install + # command, because `zimi import --setup` from a shell resolves its + # own data dir — and a shell that lacks the service's config + # resolves a different one, installs a perfectly good sidecar into + # it, and leaves the engine greyed out with no way to see why + # (issue #61: set up against the default /zims while the service + # served /mnt/nas/ZIM). + "dir": status.get("dir"), } except Exception: log.exception("sidecar status probe failed") diff --git a/zimi/server.py b/zimi/server.py index 1e99c526..1a85b64f 100644 --- a/zimi/server.py +++ b/zimi/server.py @@ -125,7 +125,7 @@ # SSL context using certifi CA bundle (PyInstaller bundles lack system certs) SSL_CTX = ssl.create_default_context(cafile=certifi.where()) -ZIMI_VERSION = "1.9.0" +ZIMI_VERSION = "1.9.1" # Standing maintenance cadence: catalog TTL is 24h and UPnP leases are # 24h — run every 12h so both stay fresh at half-life. @@ -657,6 +657,17 @@ def discover_zim_dir(candidates=None): ConfigSetting("sso_aud", "ZIMI_SSO_AUD", "str", "", "SSO off", False), ConfigSetting("sso_role", "ZIMI_SSO_ROLE", "str", "user", None, False), ConfigSetting("sso_proxy", "ZIMI_SSO_PROXY", "csv", "", "private networks", False), + # "My LAN is my trust boundary." Off by default, and it has to be typed by + # someone who runs the server: with it on, a passwordless instance treats + # any private-network client as the primary admin, which is what Zimi did + # before 1.9.0 and what GHSA-5mw2-53vv-9pw6 closed. + # + # The advisory is still right — that default let an adjacent device race + # the owner to the first password. What it lacked was a way to say "yes, I + # know, this is a single-household server on a LAN I control, and I do not + # want an admin password at all", which is a real way people run this and + # which 1.9.0 removed with nothing in its place (issue #59). + ConfigSetting("lan_admin", "ZIMI_LAN_ADMIN", "bool", "0", None, False), ) _CONFIG_ENV_BY_KEY = {s.key: s for s in CONFIG_ENV_SETTINGS} diff --git a/zimi/static/app.js b/zimi/static/app.js index 09aabedf..4a74a019 100644 --- a/zimi/static/app.js +++ b/zimi/static/app.js @@ -4018,7 +4018,25 @@ function _moonScreenTiltDeg(date, lat, lon) { var dA = raSun - eq.ra; var chi = Math.atan2(Math.cos(decSun) * Math.sin(dA), Math.sin(decSun) * Math.cos(eq.dec) - Math.cos(decSun) * Math.sin(eq.dec) * Math.cos(dA)); - return -((chi - q) * 180 / Math.PI) - 90; + var tilt = -((chi - q) * 180 / Math.PI) - 90; + // The sprite has ALREADY put the lit limb on the correct side: it shades + // from a Sun vector whose sign is the waxing flag (_moonSpriteCanvas, sx). + // chi carries that same flip, because the bright limb genuinely swaps sides + // between waxing and waning — so applying both turned every waning moon by + // a further 180 degrees. Half of every month was drawn upside down: the lit + // limb on the wrong side and the maria inverted, which is what a southern + // hemisphere moon looks like from the north (issue #60). + // + // The correction turns over at new and full, where the sprite's own flag + // does. At full the disc is whole and the step is invisible; at new it is + // 0% lit, so what turns over is the maria on an unlit disc. That is the + // whole cost, and it is the reason this is a step rather than the fully + // continuous fix: making it continuous means giving the shading loop a + // real terminator angle (its Sun vector is 2D today, x and z only) and + // keying the sprite cache on that angle as well as the phase, which is a + // different and much larger change than a released bug deserves. + if (!_moonIsWaxing(_moonPhase(date))) tilt += 180; + return tilt; } // Waxing predicate — shared so no renderer flips the terminator side on its diff --git a/zimi/static/create.js b/zimi/static/create.js index 3f21592a..14d984d6 100644 --- a/zimi/static/create.js +++ b/zimi/static/create.js @@ -232,6 +232,35 @@ var CREATE_PART_INSTALL = { sidecar: 'zimi import --setup' }; +// Where this server keeps its sidecar, once a probe has said so. +var _createSidecarDir = null; + +// Shell-quote a path for a command someone will paste into a terminal. +// +// Single quotes, not double: inside double quotes a shell still expands $HOME +// and backticks, so a data dir containing either would paste as a different +// path. Bare when the path has nothing a shell reads. +function _createShellQuote(text) { + if (/^[A-Za-z0-9_@%+=:,.\/-]+$/.test(text)) return text; + return "'" + text.replace(/'/g, "'\\''") + "'"; +} + +// The sidecar command, aimed at THIS server's data dir. +// +// `zimi import --setup` resolves its own data dir from the shell it runs in, +// so run from a terminal that lacks the service's config it installs into a +// different library's state directory: a clean install, a happy log line, and +// an engine still greyed out with nothing to say why. Naming the directory +// makes the pasted command land where the server actually looks. +function _createSidecarCommand() { + var base = CREATE_PART_INSTALL.sidecar; + if (!_createSidecarDir) return base; + // The server reports the venv; --data-dir wants the directory holding it. + var dir = _createSidecarDir.replace(/[\\/]tools[\\/]warc2zim[\\/]?$/, ''); + if (!dir || dir === _createSidecarDir) return base; + return base + ' --data-dir ' + _createShellQuote(dir); +} + var CREATE_FIELDS = { engine: { id: 'create-engine', control: 'engine', label: 'create_engine', @@ -1705,7 +1734,9 @@ function _createEngineHtml(f) { function _createAddCommands(into, capability) { var parts = CREATE_ENGINE_NEEDS[capability] || []; for (var i = 0; i < parts.length; i++) { - var cmd = CREATE_PART_INSTALL[parts[i]]; + var cmd = parts[i] === 'sidecar' + ? _createSidecarCommand() + : CREATE_PART_INSTALL[parts[i]]; if (cmd && _createPartReady(parts[i]) === false && into.indexOf(cmd) < 0) into.push(cmd); } } @@ -2370,6 +2401,9 @@ function _createIngest(data) { _createSidecarReady = data.import_ready; _createRemember('sidecar', data.import_ready); } + if (typeof data.sidecar_dir === 'string' && data.sidecar_dir) { + _createSidecarDir = data.sidecar_dir; + } if (typeof data.browser_ready === 'boolean') { _createBrowserReady = data.browser_ready; _createRemember('browser', data.browser_ready);