Boundaries that were supposed to hold

The security pass. Six findings, none reachable by visiting the site and
every one a boundary this codebase says it keeps.

A subagent is pinned to a list of read-only commands, in every mode,
unattended, with no card anybody could approve -- and `find *` was on it.
find writes files with -fprintf, runs programs with -exec and removes them
with -delete, and none of that needs a character the metacharacter guard
refuses. A page the model had just read could ask for a helper and get a
key into authorized_keys, from Plan mode, which promises to change
nothing. Refused in `subject()` rather than trimmed from the list: a
pattern cannot say "and no dangerous flags", and "this one looks
read-only" is exactly what put find there.

The loopback guard missed `0.0.0.0`, which is not is_loopback but does
connect to localhost -- so it answered a *decided* False and skipped the
DNS half too. The one spelling of "this machine" that walked past a guard
whose whole job is that sentence.

Twice in the update helper, which is the one place this deliberately
crosses a privilege boundary: root ran a script the service account owns,
and root sourced a file that account can replace. Either turns a
compromise of the web application into root. The first needed no
compromise at all -- a pull happens as the service user and root runs
whatever it fetched, so control of the branch was control of root. The
old test asserted that exact ExecStart line and had pinned it in place.

Push endpoints skipped check_url, the only outbound request that did. And
a chat could be filed in another account's folder, which hands over its
system prompt -- `_new_chat` resolved the folder, discarded it when it was
not the caller's, and stored the raw id anyway.

An existing helper install keeps the old wiring until install.sh is
re-run; update.sh now says so when it finds itself inside the checkout.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

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

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
 
 ```

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

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

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=$(sed -n 's/^SITE_HOST=\([A-Za-z0-9._-]\{1,253\}\)$/\1/p' \
-  site_host="$SITE_HOST"; app_port="$APP_PORT"
+                "$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)

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?

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"

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.

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()

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:

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
 
 

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)

src/lembas/web/templates/library/_share_panel.html

@@ -56,8 +56,8 @@
       <label class="checkbox">
         <input type="checkbox" {{ 'checked' if on }}
                hx-post="/api/library/share/{{ kind }}/{{ resource.id }}"
-               hx-vals='{"principal_type": "group", "principal_id": "{{ group.id }}",
+               hx-vals='{{ {"principal_type": "group", "principal_id": group.id,
-                         "on": "{{ 'false' if on else 'true' }}", "q": "{{ q }}"}'
+                           "on": "false" if on else "true", "q": q} | tojson }}'
                hx-target="#share-panel"
                hx-swap="outerHTML">
         <span>{{ group.name }}</span>
@@ -76,8 +76,8 @@
       <label class="checkbox">
         <input type="checkbox" {{ 'checked' if on }}
                hx-post="/api/library/share/{{ kind }}/{{ resource.id }}"
-               hx-vals='{"principal_type": "user", "principal_id": "{{ person.id }}",
+               hx-vals='{{ {"principal_type": "user", "principal_id": person.id,
-                         "on": "{{ 'false' if on else 'true' }}", "q": "{{ q }}"}'
+                           "on": "false" if on else "true", "q": q} | tojson }}'
                hx-target="#share-panel"
                hx-swap="outerHTML">
         <span>{{ person.name }} <span class="faint text-xs">{{ person.email }}</span></span>

tests/test_agent_hosts.py

@@ -267,3 +267,22 @@ def test_a_draft_cannot_be_opened_against_a_refused_connection(
     profile = _profile(db)
 
     assert client.get(f"/api/agents/{profile.id}/draft").status_code == 403
+
+
+def test_the_unspecified_address_is_this_machine_too():
+    """`0.0.0.0` and `::` are neither a real destination nor a refused one:
+    connect() to either goes to loopback on Linux, so a 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, and the one spelling of
+    "this machine" that mattered walked past a guard whose whole job is that
+    sentence.
+    """
+    for spelling in ("0.0.0.0", "::", "[::]", "0.0.0.0 "):
+        assert hosts.is_loopback(spelling), spelling
+
+
+def test_a_real_address_is_still_not_this_machine():
+    for spelling in ("192.168.1.5", "10.0.0.1", "example.com", "203.0.113.9"):
+        assert not hosts.is_loopback(spelling), spelling

tests/test_agent_policy.py

@@ -308,3 +308,57 @@ def test_an_mcp_tool_is_assumed_to_change_things(db):
     )
     db.commit()
     assert mcp_registry.tool_defs(db, None, everything=True)[0].risk == RISK_WRITE
+
+
+# --- A read-only name is not a read-only command --------------------------------
+@pytest.mark.parametrize(
+    "line",
+    [
+        "find . -maxdepth 0 -fprintf /root/.ssh/authorized_keys 'ssh-ed25519 AAAA'",
+        "find / -maxdepth 1 -exec /bin/sh /tmp/payload +",
+        "find . -execdir /bin/sh {} +",
+        "find . -delete",
+        "find . -ok rm {} ;",
+        "rg --pre /bin/sh pattern",
+        "rg --pre=/bin/sh pattern",
+    ],
+)
+def test_a_command_that_writes_or_executes_matches_no_pattern(line):
+    """`_UNSAFE` stops a line being *composed* of two commands. It says nothing
+    about one command that composes another itself, and the obvious read-only
+    tools do: `find -exec` runs a program, `-fprintf` writes a file, `-delete`
+    removes one, `rg --pre` runs a preprocessor per file. None needs a character
+    `_UNSAFE` refuses.
+
+    That mattered because `find *` was on `SAFE_COMMANDS` -- the list a
+    **subagent** is pinned to, in every mode, unattended, with no approval card
+    possible. It was arbitrary write and arbitrary execution wearing a read-only
+    name.
+    """
+    assert policy.subject("shell_run", line) is None
+
+
+@pytest.mark.parametrize(
+    "line",
+    [
+        "find . -name '*.py'",
+        "find src -type f",
+        "grep -rn TODO src/",
+        "rg --json pattern",
+        "git log --oneline -5",
+        "ls -la",
+    ],
+)
+def test_ordinary_reading_still_matches(line):
+    """Or the guard has taken the tool away rather than the escape."""
+    assert policy.subject("shell_run", line) == line
+
+
+def test_the_subagent_allow_list_is_all_reachable():
+    """Every entry on `SAFE_COMMANDS` has to still resolve, or the list quietly
+    promises a helper something it cannot do."""
+    from lembas.services.subagent import SAFE_COMMANDS
+
+    for entry in SAFE_COMMANDS:
+        sample = entry.replace("*", "x").strip()
+        assert policy.subject("shell_run", sample) is not None, entry

tests/test_folders.py

@@ -436,3 +436,41 @@ def test_moving_respects_the_depth_cap(client: TestClient, db, registered):
     refused = client.patch(f"/api/folders/{loose}", data={"parent_id": chain[-1]})
     assert refused.status_code == 400
     assert db.get(Folder, loose).parent_id is None
+
+
+def test_a_chat_cannot_be_put_in_somebody_elses_folder(client: TestClient, db, registered):
+    """A folder is not just a label. `effective_system_prompt` walks up from the
+    chat through its folder and that folder's parents, so a chat attached to
+    another account's folder would take their system prompt -- a setting read
+    across an ownership boundary through a field that looks like a tag.
+
+    Both paths had it. `_new_chat` resolved the folder, discarded it when it was
+    not the caller's, and then stored the **raw id** anyway -- so the check
+    governed which seeds were applied and not where the chat went.
+    """
+    from lembas.security.passwords import hash_password
+
+    stranger = User(name="Sam", email="sam@shire.test", password_hash=hash_password("x"))
+    db.add(stranger)
+    db.commit()
+    theirs = Folder(user_id=stranger.id, name="Private", system_prompt="Speak as Sam.")
+    db.add(theirs)
+    db.commit()
+
+    mine = db.scalar(select(User).where(User.email != "sam@shire.test"))
+    chat = Chat(user_id=mine.id, model_id="m")
+    db.add(chat)
+    db.commit()
+
+    client.patch(f"/api/chats/{chat.id}", data={"folder_id": theirs.id})
+    db.refresh(chat)
+    assert chat.folder_id is None
+
+    started = client.post(
+        "/api/chats/start",
+        data={"content": "hello", "folder_id": theirs.id},
+        follow_redirects=False,
+    )
+    assert started.status_code in (200, 204, 303)
+    made = db.scalars(select(Chat).where(Chat.id != chat.id)).all()
+    assert all(c.folder_id != theirs.id for c in made)

tests/test_push.py

@@ -28,6 +28,46 @@ from lembas.services import push as push_service
 from lembas.services import reports as reports_service
 
 
+@pytest.fixture(autouse=True)
+def resolvable_push_service(monkeypatch):
+    """`push.example` is not a real host, and `check_url` resolves.
+
+    The endpoint a browser hands back is now checked by the SSRF guard on both
+    sides -- at subscribe, and again before the POST, because the row outlives
+    the first check. That guard does DNS, so every fixture endpoint in this file
+    would be refused for not existing rather than for being anywhere bad.
+
+    Stood in with something that keeps the part under test: the *shape* of the
+    refusal. A literal private or loopback address is still refused, so the
+    tests that assert the guard is wired in are asserting the real thing.
+    """
+    import ipaddress
+    from urllib.parse import urlsplit
+
+    from lembas.services import fetch as real_fetch
+
+    def stand_in(url: str, *, allow_private: bool = False) -> str:
+        host = (urlsplit(url).hostname or "").strip("[]")
+        try:
+            address = ipaddress.ip_address(host)
+        except ValueError:
+            if host.endswith(".example"):
+                return url
+            return real_fetch.check_url(url, allow_private=allow_private)
+        if not allow_private and (
+            address.is_loopback
+            or address.is_private
+            or address.is_link_local
+            or address.is_unspecified
+        ):
+            raise real_fetch.FetchError("That address is not reachable from here.")
+        return url
+
+    for module in ("lembas.api.push", "lembas.services.push"):
+        monkeypatch.setattr(f"{module}.fetch_service.check_url", stand_in)
+    return stand_in
+
+
 # --- A browser, standing in for one --------------------------------------------
 class Browser:
     """The client half of RFC 8291, written out rather than imported.
@@ -404,3 +444,49 @@ def test_the_poll_announces_the_messages_conversation(client: TestClient, db, re
     # And it is recorded as said, so the next tick does not say it again.
     db.expire_all()
     assert db.get(Chat, conversation.id).unread_notified
+
+
+def test_a_push_endpoint_pointing_inside_the_network_is_refused(client, db, registered):
+    """The endpoint is a URL the browser hands us and the server later POSTs to,
+    and it was the one outbound client not going through the SSRF guard --
+    `https://` alone says nothing about *where*. Delivery is triggered by the
+    caller themselves: send a message, close the tab, and `_persist` announces
+    it because nobody is following."""
+    refused = client.post(
+        "/api/push/subscribe",
+        json={
+            "endpoint": "https://10.0.0.5:8443/admin/shutdown",
+            "keys": {"p256dh": "BAA" + "A" * 84, "auth": "AAAAAAAA"},
+        },
+    )
+    assert refused.status_code == 400
+    assert db.scalar(select(PushSubscription)) is None
+
+
+async def test_a_stored_endpoint_is_checked_again_before_the_post(db, registered):
+    """A row outlives the check made when it was written: a name that pointed at
+    a push service can point inside the network later. The same split
+    `agent/hosts.py` makes."""
+    owner = db.scalar(select(User))
+    row = PushSubscription(
+        user_id=owner.id,
+        endpoint="https://127.0.0.1:9/hijacked",
+        p256dh=push_service.b64(
+            ec.generate_private_key(ec.SECP256R1())
+            .public_key()
+            .public_bytes(
+                encoding=serialization.Encoding.X962,
+                format=serialization.PublicFormat.UncompressedPoint,
+            )
+        ),
+        auth_secret=push_service.b64(b"0123456789abcdef"),
+    )
+    db.add(row)
+    db.commit()
+
+    sent = await push_service.send_one(db, row, {"title": "x", "body": "y", "url": "/"})
+
+    assert sent is False
+    # Refused rather than dropped: the row is somebody's registration, and being
+    # unreachable today is not the same as being gone.
+    assert db.scalar(select(PushSubscription)) is not None

tests/test_sharing_grants.py

@@ -209,6 +209,40 @@ def test_the_panel_searches_rather_than_listing_everybody(db, client, registered
     assert "Person 29" in found
 
 
+def test_a_quote_in_the_search_does_not_break_the_panels_buttons(
+    db, client, registered, owner, reader
+):
+    """`hx-vals` carries the values, and it was built by pasting the search term
+    into a JSON string. Jinja escapes the quote for HTML, but the parser decodes
+    it again before htmx parses the JSON -- so a `"` ended the string, made the
+    attribute unparseable, and every checkbox in the panel silently stopped
+    submitting anything. `q` is the last key, so an injected one would also have
+    won a duplicate-key parse.
+
+    Built with `| tojson` over the whole object now, which escapes for JSON
+    first and lets Jinja escape that for HTML. An existing grant is what keeps a
+    row on screen whatever the search says.
+    """
+    import html
+    import json
+    import re
+
+    note = notes_service.create(db, owner=owner, title="Note", body="x")
+    sharing.set_grants(db, note, user_ids=[reader.id], group_ids=[])
+
+    hostile = '", "principal_id": "smuggled'
+    page = client.get(
+        f"/api/library/share/note/{note.id}", params={"q": hostile}
+    ).text
+
+    values = re.findall(r"hx-vals='([^']*)'", page)
+    assert values, "the panel rendered no hx-vals at all"
+    for raw in values:
+        parsed = json.loads(html.unescape(raw))
+        assert parsed["principal_id"] != "smuggled"
+        assert parsed["q"] == hostile
+
+
 def test_an_existing_grant_stays_listed_whatever_the_search_says(
     db, client, registered, owner, reader
 ):

tests/test_updates.py

@@ -376,12 +376,86 @@ def test_the_helper_units_take_no_branch_from_the_request():
     assert "__UPDATE_BRANCH__" in unit
     assert "__UPDATE_CHANNEL__" in unit
     assert "update-requested" in unit  # deleted, not read
-    assert "ExecStart=/bin/bash __PREFIX__/app/deploy/update.sh" in unit
     # Deleted before the script runs, or the path unit re-arms on a file that is
     # still there and the update loops.
     assert unit.index("ExecStartPre") < unit.index("ExecStart=")
 
 
+def test_root_does_not_run_a_script_the_service_account_owns():
+    """The unit has no `User=`, so ExecStart is root. It used to be
+    `__PREFIX__/app/deploy/update.sh` -- inside the checkout, owned by the
+    unprivileged service account -- so anything able to write as that account
+    could rewrite it and be root, and so could whoever controlled the branch,
+    since the pull runs as that account and root runs what it fetched.
+
+    **The old test asserted that exact line.** It passed for the whole life of
+    the feature and pinned the vulnerability in place, which is this codebase's
+    recurring failure applied to a privilege boundary: an assertion about the
+    text rather than about the property the text was supposed to have.
+    """
+    from pathlib import Path
+
+    import lembas
+
+    root = Path(lembas.__file__).resolve().parents[2]
+    unit = (root / "deploy/lembas-update.service").read_text()
+    install = (root / "deploy/install.sh").read_text()
+
+    exec_line = next(line for line in unit.splitlines() if line.startswith("ExecStart="))
+    assert "__PREFIX__" not in exec_line, "root would run a file inside the checkout"
+    assert "__UPDATE_HELPER__" in exec_line
+
+    # And the installer puts that copy somewhere root owns.
+    assert 'install -o root -g root -m 755' in install
+    assert "__UPDATE_HELPER__|$UPDATE_HELPER" in install
+
+
+def test_the_root_script_never_sources_a_file_the_service_account_can_replace():
+    """`.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 at mode
+    755, and write permission on a directory is all it takes to unlink a file
+    and put another there. `.` would have run its contents **as root**, and this
+    script became root-triggerable by anyone who can create one file in
+    `$PREFIX/data`, which is that same account.
+
+    Asserted as "nothing under $PREFIX is sourced" rather than as the shape of
+    one line, because the next thing to be read from there would have the same
+    problem and a test naming `.deploy-env` would not notice.
+    """
+    from pathlib import Path
+
+    import lembas
+
+    root = Path(lembas.__file__).resolve().parents[2]
+    script = (root / "deploy/update.sh").read_text()
+
+    for line in script.splitlines():
+        stripped = line.strip()
+        if stripped.startswith((". ", "source ")):
+            assert "$PREFIX" not in stripped and "$APP" not in stripped, stripped
+
+    # And the two values it does want are extracted by pattern.
+    assert "s/^SITE_HOST=" in script
+    assert "s/^APP_PORT=" in script
+
+
+def test_the_update_script_notices_the_old_wiring():
+    """A host installed before the fix keeps the old unit, and re-running the
+    installer is the only thing that moves it. The script therefore has to say
+    so when it finds itself running from inside the checkout -- otherwise the
+    hosts that are vulnerable are exactly the ones that never hear about it."""
+    from pathlib import Path
+
+    import lembas
+
+    root = Path(lembas.__file__).resolve().parents[2]
+    script = (root / "deploy/update.sh").read_text()
+
+    assert "INSECURE WIRING" in script
+    assert 'readlink -f "$0"' in script
+    assert "INSTALL_UPDATE_HELPER=1" in script
+
+
 def test_the_image_bakes_no_secret_and_no_data():
     from pathlib import Path