From 2e57f842c8b62e5270a4ce7353666aefb2d845c7 Mon Sep 17 00:00:00 2001 From: Alex Soffronow-Pagonidis Date: Thu, 30 Jul 2026 12:55:03 +0200 Subject: [PATCH 1/2] Serve a favicon from the tracked config subtree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop favicon.svg, favicon.png or favicon.ico into workspace/config/ and the gateway serves it at /favicon.ico. A fleet pointed at one config repo is the case this is for: six locked instances are otherwise six identical browser tabs. A convention, not a setting. The file has to travel with the config repo to be worth anything, and that subtree is already synced, already reviewed, and already the surface a proposal may touch — so a path setting would only add a second thing to keep in step with the file it names, and a machine-local path in shared settings is the mistake gateway.ssl.cert is documented as. Nothing here can be misconfigured; the file is either tracked or it isn't. Nothing is copied into the web bundle. web/dist is build output in the install tree, so writing there would mean the daemon mutating its own installation — undone by the next `npm run build`, refused by a read-only image, and invisible to an instance that never syncs. Reading the file per request instead costs nothing, works under lockdown, and means a favicon that arrives by sync is served without a restart. The route resolves the candidate and requires it to stay inside config/. Git tracks symlinks, and this route is unauthenticated because a browser asks for a favicon before anyone has logged in — so config/favicon.png -> /etc/shadow would otherwise be an unauthenticated read of any file the daemon can open, behind a filename a reviewer has no reason to question. is_file() follows symlinks and answers True there, so the containment check is the whole guard. It is registered ahead of the SPA catch-all, which until now answered /favicon.ico with index.html and a 200 — handing the browser markup where it asked for an image. The ordering test builds a web/dist so the catch-all is actually present, and asserts it is, because without that the comparison passes for the wrong reason. Co-Authored-By: Claude --- docs/config.md | 21 ++ nerve/config.py | 48 ++++ nerve/gateway/server.py | 24 ++ nerve/templates/config-repo/README.md | 2 + .../templates/skills/nerve-workspace/SKILL.md | 6 + tests/test_favicon.py | 243 ++++++++++++++++++ web/index.html | 3 + 7 files changed, 347 insertions(+) create mode 100644 tests/test_favicon.py diff --git a/docs/config.md b/docs/config.md index d01bf1ad..b27bd6ea 100644 --- a/docs/config.md +++ b/docs/config.md @@ -998,6 +998,27 @@ Approve/Decline cards in the web UI. | `gateway.ssl.cert` | path | - | SSL certificate path | | `gateway.ssl.key` | path | - | SSL private key path | +### Favicon + +Drop `favicon.svg`, `favicon.png` or `favicon.ico` into `workspace/config/` and +the gateway serves it at `/favicon.ico`. There is no setting: the file is tracked +config like anything else under `config/`, so a config repo carries it to every +instance that syncs, and a fleet can be told apart by its browser tabs. Nothing +is copied into the web bundle — the file is read per request, so one that arrives +by sync appears without a restart, and removing it goes back to the browser +default. + +Use one. If several are present the first of `.svg`, `.png`, `.ico` wins. + +A symlink is followed only while it stays inside `workspace/config/`. Git tracks +symlinks, and `/favicon.ico` is served without authentication because a browser +asks for it before anyone logs in — so a `config/favicon.png` pointing at +`/etc/shadow` would otherwise be readable by anyone who can reach the port. One +pointing at another file under `config/` is fine. + +An agent can propose an SVG through `propose_config_change`, since proposals +carry text. A `.png` or `.ico` has to be committed by a human. + ## Telegram | Key | Type | Default | Description | diff --git a/nerve/config.py b/nerve/config.py index 83917e89..7eccba0f 100644 --- a/nerve/config.py +++ b/nerve/config.py @@ -307,6 +307,54 @@ def workspace_settings_file(workspace: Path) -> Path: return workspace_config_dir(workspace) / "settings.yaml" +# Filenames served at /favicon.ico when one is present in the tracked config +# subtree, best format first. Only one should exist; the order decides it if +# several do. +# +# A convention rather than a setting. The file has to travel with the config repo +# to be worth anything — a fleet pointed at one repo is exactly the case where +# telling the instances apart in a tab bar matters — and the config subtree is +# already synced, already reviewed, and already the surface a proposal may touch. +# A path setting would add a second thing to keep in step with the file, and a +# machine-local path in shared settings is the mistake `gateway.ssl.cert` is +# documented as: a box without that file. There is nothing here to misconfigure. +_FAVICON_FILES: tuple[tuple[str, str], ...] = ( + ("favicon.svg", "image/svg+xml"), + ("favicon.png", "image/png"), + ("favicon.ico", "image/x-icon"), +) + + +def workspace_favicon(workspace: Path) -> tuple[Path, str] | None: + """The tracked favicon and its content type, or ``None`` if there is none. + + Answered per call rather than cached at start-up, so a favicon that arrives + by sync — or is simply dropped in — is served without a restart. + + The candidate is resolved and required to stay inside the config subtree, + which matters more here than the feature's size suggests. Git tracks + symlinks, so a config repo can carry ``config/favicon.png -> + /etc/shadow``; the route that serves this is unauthenticated by design, + since a browser asks for a favicon before anyone logs in. Following the link + would turn "set your own favicon" into an unauthenticated read of any file + the daemon can open, and a reviewer skimming a config PR would see a + plausible filename. + """ + config_root = workspace_config_dir(workspace) + for name, content_type in _FAVICON_FILES: + candidate = config_root / name + if not candidate.is_file(): + continue + if not _is_within(candidate, config_root): + logger.warning( + "Ignoring %s: it resolves outside the tracked config subtree %s", + candidate, config_root, + ) + continue + return candidate, content_type + return None + + def _load_workspace_settings(workspace: Path) -> dict[str, Any]: """Load ``workspace/config/settings.yaml``. diff --git a/nerve/gateway/server.py b/nerve/gateway/server.py index 5314dc2b..188ed535 100644 --- a/nerve/gateway/server.py +++ b/nerve/gateway/server.py @@ -964,6 +964,30 @@ async def ws_broadcast(session_id: str, message: dict): async def health(): return {"status": "ok", "version": "0.1.0"} + # Favicon from the tracked config subtree (see config.workspace_favicon). + # No auth: a browser asks for this before anyone has logged in, so requiring + # a token would mean the login page never has an icon. + # + # Before the static mount for the same reason as /health, and the reason is + # not cosmetic here: the SPA catch-all answers every unmatched path with + # index.html, so /favicon.ico currently returns HTML with a 200 and the + # browser is left to make sense of markup it asked for an image. + # + # Registered whether or not the frontend has been built. The favicon is + # config, not build output, and an instance serving the API without a bundled + # UI can still be someone's browser tab. + @app.get("/favicon.ico", include_in_schema=False) + async def favicon(): + from fastapi.responses import FileResponse, Response + + from nerve.config import workspace_favicon + + found = workspace_favicon(get_config().workspace) + if found is None: + return Response(status_code=404) + path, content_type = found + return FileResponse(str(path), media_type=content_type) + # Serve static web UI files if built web_dist = Path(__file__).parent.parent.parent / "web" / "dist" if web_dist.exists(): diff --git a/nerve/templates/config-repo/README.md b/nerve/templates/config-repo/README.md index 8e2255a5..3c96f691 100644 --- a/nerve/templates/config-repo/README.md +++ b/nerve/templates/config-repo/README.md @@ -7,6 +7,8 @@ holds: - `config/settings.yaml` — shareable settings (secrets are `${ENV_VAR}` refs) - `config/cron/` — cron jobs, system jobs, and drop-in gate plugins +- `config/favicon.{svg,png,ico}` — optional; served at `/favicon.ico`, so every + instance syncing this repo is identifiable in a browser tab - `skills//SKILL.md` — skills ## How changes are made diff --git a/nerve/templates/skills/nerve-workspace/SKILL.md b/nerve/templates/skills/nerve-workspace/SKILL.md index fd6ff011..ab01dd56 100644 --- a/nerve/templates/skills/nerve-workspace/SKILL.md +++ b/nerve/templates/skills/nerve-workspace/SKILL.md @@ -94,6 +94,12 @@ dropped quietly. Fix the reported paths and re-submit. `skills//scripts/` **is** proposable — it's a normal part of a skill — so a script there reaches the instance through this route like anything else. +Proposals carry **text**, so a binary file cannot go through this tool at all. +That matters for one thing in practice: the instance's favicon +(`config/favicon.svg` / `.png` / `.ico`, served at `/favicon.ico`). You can +propose an `.svg`, since SVG is text. A `.png` or `.ico` has to be committed by a +human — say so rather than submitting something that will be rejected. + ### Why this exists, and what it isn't So that a change to your configuration is reviewed, attributable, and visible in diff --git a/tests/test_favicon.py b/tests/test_favicon.py new file mode 100644 index 00000000..a6bcf05f --- /dev/null +++ b/tests/test_favicon.py @@ -0,0 +1,243 @@ +"""Tests for the tracked favicon — lookup convention and the route serving it.""" + +from __future__ import annotations + +import shutil +from pathlib import Path + +import pytest + +from nerve.config import workspace_favicon + +# A one-pixel PNG, so the served bytes are a real image rather than a marker. +_PNG = bytes.fromhex( + "89504e470d0a1a0a0000000d49484452000000010000000108060000001f15c4" + "890000000a49444154789c6300010000050001" + "0d0a2db40000000049454e44ae426082" +) + + +def _ws(tmp_path: Path) -> Path: + ws = tmp_path / "ws" + (ws / "config").mkdir(parents=True) + return ws + + +class TestLookup: + def test_no_favicon_is_none(self, tmp_path): + assert workspace_favicon(_ws(tmp_path)) is None + + def test_missing_config_subtree_is_none(self, tmp_path): + """A workspace that predates the config subtree, not an error.""" + ws = tmp_path / "bare" + ws.mkdir() + assert workspace_favicon(ws) is None + + @pytest.mark.parametrize( + ("name", "content_type"), + [ + ("favicon.svg", "image/svg+xml"), + ("favicon.png", "image/png"), + ("favicon.ico", "image/x-icon"), + ], + ) + def test_each_format_is_found_with_its_type(self, tmp_path, name, content_type): + ws = _ws(tmp_path) + (ws / "config" / name).write_bytes(_PNG) + found = workspace_favicon(ws) + assert found is not None + assert found[0] == ws / "config" / name + assert found[1] == content_type + + def test_best_format_wins_when_several_exist(self, tmp_path): + ws = _ws(tmp_path) + for name in ("favicon.ico", "favicon.png", "favicon.svg"): + (ws / "config" / name).write_bytes(_PNG) + assert workspace_favicon(ws)[0].name == "favicon.svg" + + def test_only_the_conventional_names(self, tmp_path): + """`icon.png` is not it — one name per format, so there is one answer.""" + ws = _ws(tmp_path) + (ws / "config" / "icon.png").write_bytes(_PNG) + (ws / "config" / "logo.svg").write_bytes(_PNG) + assert workspace_favicon(ws) is None + + def test_a_directory_named_like_one_is_not_served(self, tmp_path): + ws = _ws(tmp_path) + (ws / "config" / "favicon.png").mkdir() + assert workspace_favicon(ws) is None + + def test_a_symlink_out_of_the_subtree_is_refused(self, tmp_path, caplog): + """The reason this lookup resolves at all. + + Git tracks symlinks, so a config repo can carry one; the route is + unauthenticated, so following it would serve any file the daemon can read + to anyone who can reach the port. The filename gives a reviewer nothing + to notice. + """ + ws = _ws(tmp_path) + secret = tmp_path / "secret.txt" + secret.write_text("shadow contents") + (ws / "config" / "favicon.png").symlink_to(secret) + with caplog.at_level("WARNING"): + assert workspace_favicon(ws) is None + assert "outside the tracked config subtree" in caplog.text + + def test_a_symlink_inside_the_subtree_is_fine(self, tmp_path): + """Containment is the rule, not "no symlinks" — a link to a real asset.""" + ws = _ws(tmp_path) + (ws / "config" / "brand").mkdir() + (ws / "config" / "brand" / "logo.png").write_bytes(_PNG) + (ws / "config" / "favicon.png").symlink_to(ws / "config" / "brand" / "logo.png") + found = workspace_favicon(ws) + assert found is not None and found[1] == "image/png" + + def test_a_workspace_reached_through_a_symlink_still_works(self, tmp_path): + """Resolving must not make the ordinary case look like an escape. + + A workspace under a symlinked home is normal, and its own config subtree + is inside itself however the path was spelled. + """ + real = tmp_path / "real" + (real / "config").mkdir(parents=True) + (real / "config" / "favicon.png").write_bytes(_PNG) + link = tmp_path / "via-link" + link.symlink_to(real) + assert workspace_favicon(link) is not None + + def test_a_dangling_symlink_is_not_served(self, tmp_path): + ws = _ws(tmp_path) + (ws / "config" / "favicon.png").symlink_to(tmp_path / "gone.png") + assert workspace_favicon(ws) is None + + def test_the_favicon_is_not_cached_between_calls(self, tmp_path): + """A synced favicon has to appear without a restart, and go the same way.""" + ws = _ws(tmp_path) + assert workspace_favicon(ws) is None + (ws / "config" / "favicon.png").write_bytes(_PNG) + assert workspace_favicon(ws) is not None + (ws / "config" / "favicon.png").unlink() + assert workspace_favicon(ws) is None + + +class TestRoute: + def _client(self, tmp_path, monkeypatch): + from fastapi import FastAPI + from fastapi.testclient import TestClient + + import nerve.config as cfgmod + from nerve.config import NerveConfig + + ws = _ws(tmp_path) + config = NerveConfig() + config.workspace = ws + monkeypatch.setattr(cfgmod, "_config", config, raising=False) + + # The route as create_app registers it, and the catch-all behind it in + # the same order, so what is exercised is the resolution between the two + # rather than a favicon route on its own. Standing up the real gateway + # would drag in its lifespan for a static-file question. + app = FastAPI() + + @app.get("/favicon.ico", include_in_schema=False) + async def favicon(): + from fastapi.responses import FileResponse, Response + + found = cfgmod.workspace_favicon(cfgmod.get_config().workspace) + if found is None: + return Response(status_code=404) + path, content_type = found + return FileResponse(str(path), media_type=content_type) + + @app.get("/{path:path}") + async def spa_fallback(path: str): + from fastapi.responses import HTMLResponse + + return HTMLResponse("index") + + return TestClient(app), ws + + def test_the_catch_all_does_not_win(self, tmp_path, monkeypatch): + """Proves the ordering, not just that the routes are listed in an order. + + The catch-all is registered second and matches /favicon.ico perfectly + well; that it does not answer is the whole mechanism. + """ + client, _ = self._client(tmp_path, monkeypatch) + r = client.get("/favicon.ico") + assert b"" not in r.content.lower() + # And it is still there for anything else. + assert b"index" in client.get("/some/spa/route").content + + def test_404_when_none_is_tracked(self, tmp_path, monkeypatch): + """A 404, not index.html with a 200 — which is what the catch-all gave.""" + client, _ = self._client(tmp_path, monkeypatch) + assert client.get("/favicon.ico").status_code == 404 + + def test_serves_the_bytes_and_the_type(self, tmp_path, monkeypatch): + client, ws = self._client(tmp_path, monkeypatch) + (ws / "config" / "favicon.png").write_bytes(_PNG) + r = client.get("/favicon.ico") + assert r.status_code == 200 + assert r.content == _PNG + # The suffix decides the type, so a PNG at the .ico URL is still a PNG. + assert r.headers["content-type"] == "image/png" + + def test_a_symlinked_secret_is_not_served(self, tmp_path, monkeypatch): + client, ws = self._client(tmp_path, monkeypatch) + secret = tmp_path / "secret.txt" + secret.write_text("shadow contents") + (ws / "config" / "favicon.png").symlink_to(secret) + r = client.get("/favicon.ico") + assert r.status_code == 404 + assert b"shadow" not in r.content + + +@pytest.fixture +def built_frontend(): + """A minimal ``web/dist`` so ``create_app`` registers the SPA catch-all. + + Without one the catch-all is never registered, and an assertion about + ordering against it passes because neither route is there to compare — which + is the failure the ordering test exists to catch. ``web/dist`` is gitignored, + so creating it dirties nothing; a real build is left alone. + """ + import nerve.gateway.server as server_mod + + dist = Path(server_mod.__file__).parent.parent.parent / "web" / "dist" + if dist.exists(): + yield dist + return + (dist / "assets").mkdir(parents=True) + (dist / "index.html").write_text("built") + try: + yield dist + finally: + shutil.rmtree(dist, ignore_errors=True) + + +class TestRegistrationOrder: + def test_favicon_is_matched_before_the_spa_catch_all(self, built_frontend): + """The catch-all answers everything, so order is what makes this work. + + Asserted against the real app, because the failure is silent: + /favicon.ico goes back to returning 200 with index.html in it. + """ + from nerve.gateway.server import create_app + + paths = [getattr(r, "path", None) for r in create_app().routes] + assert "/favicon.ico" in paths, paths + # Guards the fixture as much as the app: without the catch-all present + # the comparison below would be vacuous. + assert "/{path:path}" in paths, paths + assert paths.index("/favicon.ico") < paths.index("/{path:path}") + + def test_the_route_needs_no_authentication(self): + """A browser requests a favicon before the user has a token.""" + from nerve.gateway.server import create_app + + for route in create_app().routes: + if getattr(route, "path", None) == "/favicon.ico": + assert not getattr(route, "dependencies", []), route.dependencies + return + pytest.fail("no /favicon.ico route registered") diff --git a/web/index.html b/web/index.html index 874dadd2..954271f9 100644 --- a/web/index.html +++ b/web/index.html @@ -3,6 +3,9 @@ + + Nerve " + '' + ) + + def test_csp_forbids_script_and_network(self, tmp_path, monkeypatch): + client, ws = _client(tmp_path, monkeypatch) + (ws / "config" / "favicon.svg").write_text(self._SVG_WITH_SCRIPT) + r = client.get("/favicon.ico") + assert r.status_code == 200 + csp = r.headers["content-security-policy"] + # 'none' by default is what denies script-src and connect-src both, so + # the script neither runs nor could reach anywhere if it did. + assert "default-src 'none'" in csp + assert "script" not in csp # nothing re-permits it + assert r.headers["x-content-type-options"] == "nosniff" + + def test_the_policy_still_allows_an_ordinary_icon(self): + """Blocking script must not block SVG that legitimately styles itself.""" + csp = FAVICON_RESPONSE_HEADERS["Content-Security-Policy"] + assert "style-src 'unsafe-inline'" in csp + assert "img-src data:" in csp + + def test_the_route_create_app_registers_sends_them(self, tmp_path, monkeypatch): + """Asks the real route, not the copy the other tests build. + + A hand-written route cannot notice the headers being dropped from the one + ``create_app`` actually registers, which is the drift that would matter. + """ + import asyncio + + import nerve.config as cfgmod + from nerve.config import NerveConfig + from nerve.gateway.server import create_app + + ws = _ws(tmp_path) + (ws / "config" / "favicon.svg").write_text(self._SVG_WITH_SCRIPT) + config = NerveConfig() + config.workspace = ws + monkeypatch.setattr(cfgmod, "_config", config, raising=False) + + endpoint = next( + r.endpoint for r in create_app().routes + if getattr(r, "path", None) == "/favicon.ico" + ) + response = asyncio.run(endpoint()) + assert response.media_type == "image/svg+xml" + assert "default-src 'none'" in response.headers["content-security-policy"] + assert response.headers["x-content-type-options"] == "nosniff" + + def test_raster_formats_get_the_headers_too(self, tmp_path, monkeypatch): + """nosniff is for these: a .png whose bytes are HTML. + + Without it the browser may sniff past the declared image type and render + the markup, which puts the same script on the same origin. + """ + client, ws = _client(tmp_path, monkeypatch) + (ws / "config" / "favicon.png").write_text( + "" + ) + r = client.get("/favicon.ico") + assert r.headers["x-content-type-options"] == "nosniff" + assert r.headers["content-type"] == "image/png" + assert "default-src 'none'" in r.headers["content-security-policy"] + + @pytest.fixture def built_frontend(): """A minimal ``web/dist`` so ``create_app`` registers the SPA catch-all.