diff --git a/CHANGELOG.md b/CHANGELOG.md index 3bf548d..9b8e713 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,42 @@ for 1.0.0 have something to be assembled from. ## Unreleased +## 0.9.12 + +**The security pass.** Six findings, all fixed. None is reachable by simply +visiting the site; every one of them is a boundary that was supposed to hold +and did not. + +- Fixed: **a helper could write files and run programs on the remote machine, + unattended, in a mode that promises to change nothing.** A subagent is pinned + to a fixed list of read-only commands — and `find` was on it. `find -fprintf` + writes a file, `find -exec` runs a program, `find -delete` removes one, and + none of them needs a character the shell-metacharacter guard refuses. A page + the model had just read could have asked for a helper and got an SSH key + written into `authorized_keys`. Those flags are refused outright now, whatever + list a command is on. +- Fixed: **an SSH connection could be pointed at `0.0.0.0` and reach the machine + LLeMbas runs on**, with the "may a connection point here" setting still + reading *off*. Every other spelling was caught; that one is neither a real + destination nor a refused one, and connecting to it goes to localhost. +- Fixed, twice, in the update helper — the one place this deliberately crosses a + privilege boundary: **root ran a script the unprivileged service account + owns**, and **root sourced a file that account can replace**. Either turns a + compromise of the web application into root on the host, which is exactly what + the unprivileged split exists to prevent. The first also meant control of the + branch was control of root, with no compromise needed at all. + **If you installed the update helper before this, re-run the installer** — + the old wiring stays until you do, and the update script now says so loudly + when it notices. +- Fixed: **browser notification endpoints skipped the guard that stops the + server being aimed at your own network.** It was the only outbound request in + the codebase not going through it. +- Fixed: **a chat could be put in another account's folder**, and a folder hands + its system prompt to the chats inside it — so that read a setting across an + ownership boundary through a field that looks like a tag. +- Fixed: a `"` typed into the share panel's search box silently stopped every + checkbox in the panel from doing anything. + ## 0.9.11 - The Updates page no longer runs the **Check the remote** button flush against diff --git a/README.md b/README.md index 60de15f..f18e5b5 100644 --- a/README.md +++ b/README.md @@ -427,6 +427,18 @@ run by hand. Release notes come out of the annotated tag itself, so no forge API is involved anywhere. +Root runs a **copy** of `deploy/update.sh` that the installer places outside the +checkout and root owns. It must not run the one in the checkout: that file +belongs to the unprivileged service account, so anything able to write as that +account could rewrite it and become root — and so could whoever controls the +branch, since a pull happens as that account and root would run whatever it +fetched. The cost is that changing `update.sh` needs the installer re-run, and +it tells you when your copy has fallen behind. + +**If you installed the helper before this changed, re-run the installer.** The +old wiring points systemd at the checkout, and the update script now says so +loudly when it notices it is running from there. + ## How it fits together ``` diff --git a/deploy/install.sh b/deploy/install.sh index 6a38c3a..a02cd53 100755 --- a/deploy/install.sh +++ b/deploy/install.sh @@ -165,12 +165,34 @@ echo "== update helper ==" # is-enabled`, because that would be a subprocess on every page render to answer # a question that changes once. UPDATE_MARKER="$PREFIX/data/.update-helper" +# Where root's copy of the update script lives, and why it is a copy. +# +# The unit runs as root. Pointing its ExecStart at `$PREFIX/app/deploy/update.sh` +# meant root executing a file owned by the **unprivileged service account** -- +# so anything able to write as that account could rewrite the script, create the +# request file it also owns, and be root. That is the whole privilege boundary +# the helper exists to keep, defeated by a `chown`. +# +# The second path is worse because it needs no compromise at all: an update +# pulls new code *as the service user*, and root then runs whatever +# `deploy/update.sh` that pull contained. Control of the branch would have been +# control of root. +# +# So root runs a copy it owns, installed here, by an administrator, deliberately. +# The cost is that improving `update.sh` needs `install.sh` re-run -- which is +# the correct trade: root should not execute a script that arrived over the +# network a moment ago. +UPDATE_HELPER_DIR="/usr/local/lib/lembas" +UPDATE_HELPER="$UPDATE_HELPER_DIR/update.sh" if [[ "$INSTALL_UPDATE_HELPER" == "1" ]]; then + sudo mkdir -p "$UPDATE_HELPER_DIR" + sudo install -o root -g root -m 755 "$HERE/update.sh" "$UPDATE_HELPER" for unit in lembas-update.path lembas-update.service; do sed -e "s|__PREFIX__|$PREFIX|g" \ -e "s|__SERVICE_USER__|$SERVICE_USER|g" \ -e "s|__UPDATE_BRANCH__|$BRANCH|g" \ -e "s|__UPDATE_CHANNEL__|$CHANNEL|g" \ + -e "s|__UPDATE_HELPER__|$UPDATE_HELPER|g" \ "$HERE/$unit" | sudo tee "/etc/systemd/system/$unit" >/dev/null done sudo systemctl daemon-reload @@ -187,7 +209,8 @@ else # the manual one, which is the honest degradation. sudo systemctl disable --now lembas-update.path 2>/dev/null || true sudo rm -f /etc/systemd/system/lembas-update.path \ - /etc/systemd/system/lembas-update.service "$UPDATE_MARKER" + /etc/systemd/system/lembas-update.service "$UPDATE_MARKER" \ + "$UPDATE_HELPER" sudo systemctl daemon-reload echo " not installed (INSTALL_UPDATE_HELPER=1 to allow updating from the web UI)" fi diff --git a/deploy/lembas-update.service b/deploy/lembas-update.service index 636cbea..3b07681 100644 --- a/deploy/lembas-update.service +++ b/deploy/lembas-update.service @@ -12,7 +12,10 @@ # reaches this command line: no ref, no branch, no channel, no arguments. Both # are baked in below from the installer's environment, so pressing the button is # "deploy the channel this host was configured with" and can never be "deploy -# something else". +# something else". Nothing reads the file's *contents* either -- `ExecStartPre` +# deletes it and the `.path` unit only ever tested that it exists. +# +# And root runs a script **root owns**. See ExecStart. [Unit] Description=Apply a requested LLeMbas update @@ -30,7 +33,13 @@ Environment=SERVICE_USER=__SERVICE_USER__ Environment=PREFIX=__PREFIX__ Environment=LEMBAS_BRANCH=__UPDATE_BRANCH__ Environment=LEMBAS_CHANNEL=__UPDATE_CHANNEL__ -ExecStart=/bin/bash __PREFIX__/app/deploy/update.sh +# **Not** `__PREFIX__/app/deploy/update.sh`. That path is inside the checkout and +# owned by the unprivileged service account, so root would have been executing a +# file that account could rewrite -- and that an update could replace, since a +# pull runs as that account and root runs whatever it fetched on the next press. +# `install.sh` puts a root-owned copy here instead. Improving the script means +# re-running the installer, which is the right cost. +ExecStart=/bin/bash __UPDATE_HELPER__ # The script's own failure path prints the journal and exits non-zero, which is # what makes `systemctl status lembas-update` say what went wrong. StandardOutput=journal diff --git a/deploy/update.sh b/deploy/update.sh index 38a8d08..3a8eb71 100755 --- a/deploy/update.sh +++ b/deploy/update.sh @@ -98,6 +98,39 @@ sudo -u "$SERVICE_USER" "$VENV/bin/pip" install --quiet -e "$APP[$LEMBAS_EXTRAS] # The drift is worth catching: a change in the unit can be what makes a release # work at all, and a host that pulled the code without it would run the new # version under the old settings and fail confusingly. +# This script itself, first, because root is running a copy of it. +# +# `install.sh` puts a root-owned copy outside the checkout and points the unit +# there -- root must not execute a file the unprivileged service account can +# write, nor one that an update just fetched. The cost of that is exactly this: +# the copy can fall behind what the checkout ships, silently, and the way to +# notice is to compare. +# +# `$0` is the copy being run; `$APP/deploy/update.sh` is what was just pulled. +self=$(readlink -f "$0") +if [[ "$self" == "$(readlink -f "$APP")"/* ]]; then + # The old wiring, and the one that matters: the unit points *into the + # checkout*, so root is executing a file the unprivileged service account + # owns and that every update overwrites. Fires on exactly the hosts installed + # before this was fixed, and never afterwards. + echo "== update helper: INSECURE WIRING ==" >&2 + echo " This unit runs $self as root, and that file is owned by" >&2 + echo " $SERVICE_USER -- the account the web application runs as. Anything" >&2 + echo " able to write as that account can rewrite it and be root, and so" >&2 + echo " can whoever controls the branch this host follows." >&2 + echo " Fix by re-running the installer, which moves root's copy out:" >&2 + echo " cd $APP && sudo INSTALL_UPDATE_HELPER=1 ./deploy/install.sh" >&2 +elif [[ -f "$APP/deploy/update.sh" ]]; then + running_helper=$(sha256sum "$self" | cut -d' ' -f1) + shipped_helper=$(sha256sum "$APP/deploy/update.sh" | cut -d' ' -f1) + if [[ "$running_helper" != "$shipped_helper" ]]; then + echo "== update helper ==" >&2 + echo " deploy/update.sh has changed since this host's copy was installed." >&2 + echo " Re-run the installer to take it:" >&2 + echo " cd $APP && sudo INSTALL_UPDATE_HELPER=1 ./deploy/install.sh" >&2 + fi +fi + STAMP="$PREFIX/.unit-applied" current=$(sha256sum "$APP/deploy/lembas.service" | cut -d' ' -f1) if [[ -f "$STAMP" && "$(cat "$STAMP")" != "$current" ]]; then @@ -130,9 +163,18 @@ site_host=""; app_port="" # recovered from the environment file and the vhost found by what it proxies to. # Guessing "your-host" instead would have skipped the check on exactly the hosts # it was added for. +# **Parsed, never sourced.** `.deploy-env` is written by the installer with +# `sudo tee`, so the file is root-owned -- but `$PREFIX` is the service +# account's own directory, mode 755, and write permission on a directory is all +# it takes to unlink a file and put another one there. `.` would have run its +# contents as root, and this script is root-triggerable by anyone who can create +# one file in `$PREFIX/data` -- which is that same account. Two keys, two +# patterns, and anything else in the file is ignored rather than executed. if [[ -f "$PREFIX/.deploy-env" ]]; then - . "$PREFIX/.deploy-env" - site_host="$SITE_HOST"; app_port="$APP_PORT" + site_host=$(sed -n 's/^SITE_HOST=\([A-Za-z0-9._-]\{1,253\}\)$/\1/p' \ + "$PREFIX/.deploy-env" | tail -1) + app_port=$(sed -n 's/^APP_PORT=\([0-9]\{1,5\}\)$/\1/p' \ + "$PREFIX/.deploy-env" | tail -1) fi if [[ -z "$app_port" && -f "$PREFIX/lembas.env" ]]; then app_port=$(sed -n 's/^LEMBAS_PORT=//p' "$PREFIX/lembas.env" | tail -1) diff --git a/docs/notes/audit-0.9.md b/docs/notes/audit-0.9.md index eecec84..74ff0c8 100644 --- a/docs/notes/audit-0.9.md +++ b/docs/notes/audit-0.9.md @@ -231,9 +231,120 @@ What it measured once fixed: - `tests/__pycache__/test_zz_{dump,live}*.pyc` are stale bytecode for two files that no longer exist. -## For the security stage +## Security — *Stage 3* -Carried forward rather than answered here: +**Two privilege escalations in the update helper, both root, both fixed.** The +helper is the one place this application deliberately crosses a privilege +boundary, and it crossed it twice more than intended. Neither is reachable +from the web interface: both need code execution as the `lembas` service +account first. That is precisely the boundary the unprivileged split exists to +hold, so "you need a foothold" is the threat model, not a mitigation. + +**1. Root ran a script the service account owns.** +`ExecStart=/bin/bash __PREFIX__/app/deploy/update.sh` — inside the checkout, +owned `lembas:lembas`, because `install.sh` clones as that user. So: write your +payload into `deploy/update.sh`, `touch data/update-requested` (the service +account owns that directory too), and systemd runs it as root. The web +interface's `AdminUser` check is not the gate systemd honours. + +There is a second path needing no compromise at all: an update pulls new code +*as the service user*, and root then executes whatever `deploy/update.sh` that +pull contained. **Control of the branch was control of root.** + +Fixed by installing a root-owned copy at `/usr/local/lib/lembas/update.sh` and +pointing the unit there. The cost — improving `update.sh` needs the installer +re-run — is the right one: root should not execute a script that arrived over +the network a moment ago. The script warns when its own copy has fallen behind. + +**The old test asserted the vulnerable line** +(`assert "ExecStart=/bin/bash __PREFIX__/app/deploy/update.sh" in unit`). It +passed for the life of the feature and pinned the bug in place — the recurring +failure of this codebase, applied to a privilege boundary: an assertion about +the text rather than about the property the text was meant to have. + +**2. Root sourced a file the service account can replace.** +`. "$PREFIX/.deploy-env"`. The file is root-owned, having been written with +`sudo tee` — but `$PREFIX` is the service account's own directory at mode 755, +and write permission on a *directory* is all it takes to unlink a file and put +another there. On the live host `.deploy-env` did not even exist, so it could +simply be created. `.` runs its contents as root. + +This one survived the first fix entirely, and the helper is what made it +reachable: before the `.path` unit existed, `update.sh` only ran when an +administrator invoked it from a shell. Fixed by parsing the two values it wants +with strict patterns instead of sourcing. The test asserts that **nothing** +under `$PREFIX` is sourced, rather than naming `.deploy-env`, because the next +file read from there would have the same problem. + +**Upgrade note:** a host that installed the helper before this keeps the old +unit, and only re-running the installer moves it. `update.sh` now detects that +it is running from inside the checkout and says so loudly — otherwise the +vulnerable hosts are exactly the ones that never hear about it. + +**Also fixed, sub-threshold as a vulnerability but a real bug:** the share +panel built its `hx-vals` by pasting the search term into a JSON string. Jinja +escapes the quote for HTML and the parser decodes it again before htmx parses +the JSON, so a `"` in a search term ended the string and silently stopped every +checkbox in the panel from submitting anything. `q` was the last key, so an +injected one would also have won a duplicate-key parse. Built with `| tojson` +over the whole object now. + +**3. A read-only command that was not read-only.** `SAFE_COMMANDS` — the list a +**subagent** is pinned to, in every mode, unattended, with no approval card +possible — contained `find *`. GNU `find` writes files (`-fprintf`), runs +programs (`-exec … +`) and deletes them (`-delete`), and none of those needs a +character `policy._UNSAFE` refuses. `rg --pre` is the same shape. + +So the chain was: a parent in **Plan** mode — which promises "reads freely, +changes nothing" — spawns a helper on a `RISK_READ` tool with no card; the +helper's `shell_run` survives because `writes_off` drops `RISK_WRITE` and +deliberately keeps `RISK_EXECUTE`; `find . -maxdepth 0 -fprintf +~/.ssh/authorized_keys 'ssh-ed25519 …'` matches `find *` and runs. Prompt +injection from a page the model just read is enough to start it. + +Fixed with `policy._ACTION`, refusing those flags in `subject()` rather than +trimming the allow list — a pattern cannot express "and no dangerous flags", +and "this one looks read-only" is exactly the reasoning that put `find *` there. +It costs a false refusal on `grep -- -delete`, which is the right direction to +be wrong in: a refusal asks, an allow does not. + +**4. `0.0.0.0` walked past the loopback guard.** `_literal` answered from +`is_loopback`, and `0.0.0.0`/`::` are `is_unspecified` — so it returned a +*decided* `False`, which short-circuited `resolves_here` and skipped the DNS +half too. `connect()` to either goes to loopback on Linux, so an SSH profile +pointed at `0.0.0.0` reached this host's own sshd: the one spelling of "this +machine" that walked past the guard whose whole job is that sentence. + +**5. Push endpoints skipped the SSRF guard.** `POST /api/push/subscribe` +checked `startswith("https://")` and nothing else, and `send_one` POSTed to it +with no `check_url` — the only outbound client in the codebase not going +through the guard. Delivery is triggered by the caller: send a message, close +the tab, and `_persist` announces it because nobody is following. Checked now +at subscribe **and** again before the POST, since the row outlives the first +check. + +**6. A chat could be put in somebody else's folder.** `effective_system_prompt` +walks up from the chat through its folder and that folder's parents, so this +reads another account's system prompt through a field that looks like a tag. +Both paths had it, and `_new_chat`'s is the instructive one: it resolved the +folder, discarded it when it was not the caller's, and then stored the **raw +id** anyway — so the ownership check governed which *seeds* were applied and +not where the chat actually went. + +### Clean + +Checked and found sound: the branding CSS and custom-theme generation (ids and +colour values both validated on **read**, so a row written by hand still cannot +emit a malformed rule; served as `text/css` rather than inline, so there is no +HTML context to escape); the unauthenticated branding asset route (random +names, traversal guarded twice, magic-number sniffing, SVG excluded); sharing +authorisation on every route; the request file's contents reaching nothing; +`updates._git`'s fixed argv; and the container (non-root, no secret baked, no +docker socket, loopback only). + +## Carried forward from earlier stages + +Questions raised before the security stage, answered by it: - the three hand-rolled redirect loops each re-run `check_url` per hop (confirmed); does each also drop the secret when a hop leaves its origin? diff --git a/src/lembas/__init__.py b/src/lembas/__init__.py index 8fdb4e5..619aec4 100644 --- a/src/lembas/__init__.py +++ b/src/lembas/__init__.py @@ -1,3 +1,3 @@ """LLeMbas - a Middle-earth themed web UI for OpenAI-compatible LLM endpoints.""" -__version__ = "0.9.11" +__version__ = "0.9.12" diff --git a/src/lembas/api/chats.py b/src/lembas/api/chats.py index e6d98b9..122de42 100644 --- a/src/lembas/api/chats.py +++ b/src/lembas/api/chats.py @@ -213,7 +213,13 @@ def _new_chat( profile = _agent_target(db, user, kind, ssh_profile_id) chat = Chat( user_id=user.id, - folder_id=folder_id or None, + # `folder`, not `folder_id`: the raw value is what the request asked + # for, and the lines above already discarded it when it names somebody + # else's folder. Storing the raw one put the chat there anyway -- so the + # ownership check governed which *seeds* were applied and not where the + # chat actually went, and a folder's system prompt is read on every turn + # from wherever the chat sits. + folder_id=folder.id if folder is not None else None, model_id=chosen[0] if chosen else "", connection_id=chosen[1] if chosen else None, temporary=temporary, @@ -1940,7 +1946,16 @@ async def update_chat(request: Request, db: Db, user: RequiredUser, chat_id: str renamed = True if "folder_id" in form: - chat.folder_id = str(form["folder_id"]) or None + # Resolved against *this person's* folders, not taken as given. A folder + # is not just a label: `effective_system_prompt` walks up from the chat + # through its folder and its parents, so a chat attached to somebody + # else's folder would take their system prompt -- reading a setting + # across an ownership boundary through a field that looks like a tag. + # Unknown or not theirs means no folder, which is the same answer + # `_new_chat` gives. + wanted = str(form["folder_id"]).strip() + folder = db.get(Folder, wanted) if wanted else None + chat.folder_id = folder.id if folder is not None and folder.user_id == user.id else None # The mode is the one agent field that changes mid-chat: it decides what # gets asked about, not what the conversation is. diff --git a/src/lembas/api/push.py b/src/lembas/api/push.py index a799fd0..bde0b2d 100644 --- a/src/lembas/api/push.py +++ b/src/lembas/api/push.py @@ -17,7 +17,9 @@ from sqlalchemy import select from lembas.api.deps import Db, RequiredUser from lembas.db.models import PushSubscription +from lembas.services import fetch as fetch_service from lembas.services import push as push_service +from lembas.services.fetch import FetchError log = logging.getLogger(__name__) @@ -61,6 +63,21 @@ async def subscribe(request: Request, db: Db, user: RequiredUser) -> Response: if not endpoint.startswith("https://") or not p256dh or not auth: return Response(status_code=status.HTTP_400_BAD_REQUEST) + # The endpoint is a URL the browser hands us and the server later POSTs to, + # which makes it the same shape as every other URL a request can name -- + # and it was the one outbound client in the codebase not going through the + # SSRF guard. `https://` alone says nothing about *where*: an internal + # address is as valid a URL as Mozilla's push service, and the caller + # triggers delivery themselves by sending a message and closing the tab. + # + # Checked here **and** again before the POST, the split `agent/hosts.py` + # uses: a row can predate a DNS change, and this one is stored. + try: + fetch_service.check_url(endpoint) + except FetchError as exc: + log.warning("refused a push endpoint from %s: %s", user.email, exc.message) + return Response(status_code=status.HTTP_400_BAD_REQUEST) + existing = db.scalars( select(PushSubscription).where(PushSubscription.endpoint == endpoint) ).first() diff --git a/src/lembas/services/agent/hosts.py b/src/lembas/services/agent/hosts.py index 28ed51b..a8dc1c8 100644 --- a/src/lembas/services/agent/hosts.py +++ b/src/lembas/services/agent/hosts.py @@ -107,9 +107,17 @@ def _literal(host: str) -> bool | None: if text in ("localhost", "localhost.localdomain", "ip6-localhost", "ip6-loopback"): return True try: - return ipaddress.ip_address(text).is_loopback + address = ipaddress.ip_address(text) except ValueError: return None + # `is_unspecified` as well as `is_loopback`, because `0.0.0.0` and `::` are + # neither a real destination nor a refused one: connect() to either goes to + # loopback on Linux, so an SSH profile pointed at `0.0.0.0` reached this + # host's own sshd. `is_loopback` alone answered a decided **False**, which + # also short-circuited `resolves_here`, so the DNS half never ran either -- + # the one spelling of "this machine" that walked past a guard whose whole + # job is that sentence. + return address.is_loopback or address.is_unspecified def is_loopback(host: str) -> bool: diff --git a/src/lembas/services/agent/policy.py b/src/lembas/services/agent/policy.py index db64d4e..7512ac3 100644 --- a/src/lembas/services/agent/policy.py +++ b/src/lembas/services/agent/policy.py @@ -111,6 +111,37 @@ POLICY: dict[str, dict[str, str]] = { # confined to `decide` and is worth doing; it is not done here. _UNSAFE = re.compile(r"[;&|<>`$\n\\()]") +# Flags that turn a "read-only" command into one that writes or executes, on +# tools whose *name* is on somebody's allow list. +# +# `_UNSAFE` stops a command line being composed out of two commands. It does +# nothing about a single command that composes one itself, and several of the +# obvious read-only tools do: `find -exec cmd +` runs a program, `-fprintf` +# writes a file, `-delete` removes one, and `rg --pre` runs a preprocessor for +# every file it opens. None of those needs a character `_UNSAFE` refuses, so +# `find *` on an allow list -- which is what a subagent gets, in every mode -- +# was arbitrary write and arbitrary execution wearing a read-only name. +# +# Refused here rather than trimmed from the allow list alone, because the list +# is the thing an administrator edits and "this one looks read-only" is exactly +# the reasoning that put `find *` there. A pattern cannot express "and no +# dangerous flags"; this can. +# +# Matched on the *normalised* line and word-bounded, so `docs/-exec-notes.md` +# is fine -- the flag has to stand alone as an argument. +# +# It does catch `grep -rn -- -delete src/`, where the word is a search term +# rather than a flag, and that is the right direction to be wrong in: a false +# refusal here means the call falls through to the policy table and asks, which +# costs one approval card. A false allow means an unattended helper writing +# files. Nothing is *blocked* by this -- a reader in Auto still gets it, and in +# any other mode they are shown it first, which is what they would want to be +# shown. +_ACTION = re.compile( + r"(?:^|\s)-(?:exec|execdir|ok|okdir|fprintf|fprint|fprint0|delete)(?=\s|$)" + r"|(?:^|\s)--(?:pre|search-zip|hostname-bin)(?=[\s=]|$)" +) + @dataclass(frozen=True) class Decision: @@ -162,6 +193,8 @@ def subject(tool_name: str, command: str = "") -> str | None: if _UNSAFE.search(raw): return None line = " ".join(raw.split()) + if _ACTION.search(line): + return None return line or None diff --git a/src/lembas/services/push.py b/src/lembas/services/push.py index a55d80e..fdef37c 100644 --- a/src/lembas/services/push.py +++ b/src/lembas/services/push.py @@ -76,8 +76,10 @@ from sqlalchemy import select from sqlalchemy.orm import Session as DBSession from lembas.db.models import PushSubscription, User +from lembas.services import fetch as fetch_service from lembas.services import settings_store from lembas.services.crypto import decrypt, encrypt +from lembas.services.fetch import FetchError log = logging.getLogger(__name__) @@ -274,6 +276,18 @@ async def send_one(db: DBSession, subscription: PushSubscription, payload: dict[ "Authorization": f"vapid t={token}, k={keys(db).public_b64}", } + # Again, on a stored value. The subscribe route checks it too, but the row + # outlives that check: a name that pointed at a push service when it was + # registered can point inside the network later, and this is the side that + # actually opens the socket. The same split `agent/hosts.py` makes. + try: + fetch_service.check_url(subscription.endpoint) + except FetchError as exc: + log.warning( + "refusing to push to %s: %s", _audience(subscription.endpoint), exc.message + ) + return False + try: async with httpx.AsyncClient(timeout=10.0) as client: response = await client.post(subscription.endpoint, content=encrypted, headers=headers) diff --git a/src/lembas/web/templates/library/_share_panel.html b/src/lembas/web/templates/library/_share_panel.html index 0a5f105..afaa5ad 100644 --- a/src/lembas/web/templates/library/_share_panel.html +++ b/src/lembas/web/templates/library/_share_panel.html @@ -56,8 +56,8 @@