mirror of
https://github.com/Matysh/houseplan-card
synced 2026-07-31 16:38:31 +00:00
v1.46.0: full external audit of v1.45.4 — HP-1454-01 … -10
HP-1454-01 (high, release blocker): an uploaded SVG plan opened directly is a top-level document of Home Assistant's own origin, so a <script> inside it reaches the session's localStorage and API. Uploading needs write access, which by default every authenticated user has. SVG responses now carry a sandbox CSP; only SVG, because a CSP on a PDF can break the browser's viewer and a raster image has nothing to disable. Verified in Chromium both ways: the script runs without the header and does not with it. HP-1454-02: attachment uploads wrote straight to <marker>/<filename>, outside the config transaction — a cancelled dialog or a rejected save left the stored url serving new bytes, and every new icon shared one 'new' folder, so two of them attaching manual.pdf pointed at one file. Uploads take a free name, a new icon gets a per-dialog staging folder promoted on an accepted save, and config/set collects superseded and aged-orphan attachments like it does plans. HP-1454-03: the debounce spaced out the starts of a write, not the writes. A save slower than 500 ms let the next edit go out with the same expected_rev; the server accepted the first, rejected the second, and the conflict handler reloaded over the local copy. Writes are chained now — one in flight, each with the revision the previous returned. HP-1454-04: _openPairsCache keyed on room ids and links only, so an aspect change or a dragged vertex left open boundaries and their glow cuts at old coordinates. It keys on the rendered model object now — the same invalidation the model cache already has, not a second strategy. The fingerprint also gained an O(1) geometry roll-up per room. HP-1454-05: outer collections were capped, inner ones were not. Limits for poly points, open_to, controls, pdfs, text and url lengths, plus a total serialized size cap; legacy is dropped server-side. HP-1454-06: upload streams to a temp file and downloads use FileResponse, so a 50 MB manual no longer costs ~100 MB of RSS per transfer. HP-1454-07: spaceModels() dropped room.settings, so the static card ignored the per-room fill override. HP-1454-08: layout had no revision on point-wise writes and no event, leaving static cards stale forever; it now keeps a revision, returns it and fires houseplan_layout_updated. HP-1454-09: repair cleanup only walked existing spaces, so a deleted space kept its warning. HP-1454-10: serialize-javascript pinned past two advisories. Tests: smoke_svg_sandbox (proves both directions), smoke_config_writer and smoke_render_parity (both verified failing against a v1.45.4 build), six pure tests for attachment collection and inner limits, four HA-harness tests for the CSP, non-overwriting uploads, the size cap and layout revisions. Docs: CHANGELOG.md + CHANGELOG.ru.md + ARCHITECTURE.md + TESTING.md + STATUS.md.
This commit is contained in:
Matysh
@@ -545,3 +545,151 @@ async def test_signing_one_path_may_fail_without_failing_the_request(
|
||||
urls = resp["result"]["urls"]
|
||||
assert good in urls and "authSig=" in urls[good]
|
||||
assert bad not in urls, "an unsignable path is absent, never an unsigned url"
|
||||
|
||||
|
||||
async def test_config_write_is_capped_by_total_size(
|
||||
hass: HomeAssistant, hass_ws_client: WebSocketGenerator
|
||||
) -> None:
|
||||
"""HP-1454-05: per-field limits bound each list, this bounds their product."""
|
||||
from custom_components.houseplan.validation import MAX_CONFIG_BYTES, MAX_TEXT
|
||||
|
||||
await _setup(hass)
|
||||
client = await hass_ws_client(hass)
|
||||
cfg = await _cfg([{"id": "f1", "plan_url": None}])
|
||||
# every field inside the caps, the whole thing far past them
|
||||
blob = "d" * MAX_TEXT
|
||||
cfg["settings"] = {"known_devices": [blob] * (MAX_CONFIG_BYTES // MAX_TEXT + 10)}
|
||||
resp = await _save(client, cfg, 0)
|
||||
assert not resp["success"] and resp["error"]["code"] == "too_large"
|
||||
|
||||
cfg["settings"] = {"known_devices": ["ok"]}
|
||||
assert (await _save(client, cfg, 0))["success"]
|
||||
|
||||
|
||||
async def test_layout_keeps_its_revision_and_announces_changes(
|
||||
hass: HomeAssistant, hass_ws_client: WebSocketGenerator
|
||||
) -> None:
|
||||
"""HP-1454-08: point-wise writes used to drop the revision and say nothing.
|
||||
|
||||
layout/set offered optimistic locking, but every drag wrote {"layout": …}
|
||||
and reset the counter to 0, so the lock protected nothing; and a static card
|
||||
on the same dashboard never learned that a marker had moved.
|
||||
"""
|
||||
await _setup(hass)
|
||||
client = await hass_ws_client(hass)
|
||||
events: list[dict] = []
|
||||
hass.bus.async_listen("houseplan_layout_updated", lambda ev: events.append(ev.data))
|
||||
|
||||
await client.send_json_auto_id({"type": "houseplan/layout/get"})
|
||||
assert (await client.receive_json())["result"]["rev"] == 0
|
||||
|
||||
await client.send_json_auto_id(
|
||||
{"type": "houseplan/layout/set", "layout": {"a": {"x": 1, "y": 2}}, "expected_rev": 0}
|
||||
)
|
||||
rev = (await client.receive_json())["result"]["rev"]
|
||||
assert rev == 1
|
||||
|
||||
await client.send_json_auto_id(
|
||||
{"type": "houseplan/layout/update", "device_id": "b", "pos": {"x": 3, "y": 4}}
|
||||
)
|
||||
assert (await client.receive_json())["result"]["rev"] == 2
|
||||
|
||||
await client.send_json_auto_id({"type": "houseplan/layout/delete", "device_id": "b"})
|
||||
assert (await client.receive_json())["result"]["rev"] == 3
|
||||
|
||||
await client.send_json_auto_id({"type": "houseplan/layout/get"})
|
||||
got = await client.receive_json()
|
||||
assert got["result"]["rev"] == 3 and got["result"]["layout"] == {"a": {"x": 1, "y": 2}}
|
||||
|
||||
# a stale wholesale write is refused, which it could not be before
|
||||
await client.send_json_auto_id(
|
||||
{"type": "houseplan/layout/set", "layout": {}, "expected_rev": 1}
|
||||
)
|
||||
bad = await client.receive_json()
|
||||
assert not bad["success"] and bad["error"]["code"] == "conflict"
|
||||
|
||||
await hass.async_block_till_done()
|
||||
assert [e["rev"] for e in events] == [1, 2, 3]
|
||||
|
||||
|
||||
async def test_uploaded_svg_is_sandboxed_and_a_pdf_is_not(
|
||||
hass: HomeAssistant, hass_ws_client: WebSocketGenerator, hass_client
|
||||
) -> None:
|
||||
"""HP-1454-01: user SVG served from HA's origin must not be a live document.
|
||||
|
||||
Only SVG gets the header: a CSP on a PDF response can break the browser's
|
||||
built-in viewer, and a raster image cannot execute anything anyway.
|
||||
"""
|
||||
import os
|
||||
|
||||
from custom_components.houseplan.const import CONTENT_URL, FILES_DIR, PLANS_DIR
|
||||
|
||||
await _setup(hass)
|
||||
plans = hass.config.path(PLANS_DIR)
|
||||
files = os.path.join(hass.config.path(FILES_DIR), "m1")
|
||||
|
||||
def _write() -> None:
|
||||
os.makedirs(plans, exist_ok=True)
|
||||
os.makedirs(files, exist_ok=True)
|
||||
with open(os.path.join(plans, "x.svg"), "wb") as fh:
|
||||
fh.write(b"<svg xmlns='http://www.w3.org/2000/svg'/>")
|
||||
with open(os.path.join(plans, "x.png"), "wb") as fh:
|
||||
fh.write(b"PNG")
|
||||
with open(os.path.join(files, "m.pdf"), "wb") as fh:
|
||||
fh.write(b"%PDF-1.4")
|
||||
|
||||
await hass.async_add_executor_job(_write)
|
||||
http = await hass_client()
|
||||
|
||||
svg = await http.get(f"{CONTENT_URL}/plans/_/x.svg")
|
||||
assert svg.status == 200
|
||||
csp = svg.headers.get("Content-Security-Policy", "")
|
||||
assert "sandbox" in csp and "script-src 'none'" in csp
|
||||
assert svg.headers["Content-Type"].startswith("image/svg+xml")
|
||||
|
||||
png = await http.get(f"{CONTENT_URL}/plans/_/x.png")
|
||||
assert png.status == 200 and "Content-Security-Policy" not in png.headers
|
||||
|
||||
pdf = await http.get(f"{CONTENT_URL}/files/m1/m.pdf")
|
||||
assert pdf.status == 200 and "Content-Security-Policy" not in pdf.headers
|
||||
assert await pdf.read() == b"%PDF-1.4"
|
||||
|
||||
|
||||
async def test_upload_never_overwrites_an_existing_attachment(
|
||||
hass: HomeAssistant, hass_ws_client: WebSocketGenerator, hass_client
|
||||
) -> None:
|
||||
"""HP-1454-02: an upload is not part of the config transaction.
|
||||
|
||||
Writing straight to `<marker>/<filename>` meant a cancelled dialog — or a
|
||||
rejected save — left the stored url serving the new bytes. And two new
|
||||
markers both uploading `manual.pdf` shared one physical file.
|
||||
"""
|
||||
import os
|
||||
|
||||
from custom_components.houseplan.const import CONTENT_URL, FILES_DIR
|
||||
|
||||
await _setup(hass)
|
||||
http = await hass_client()
|
||||
|
||||
async def upload(marker_id: str, name: str, data: bytes) -> str:
|
||||
import aiohttp
|
||||
|
||||
writer = aiohttp.FormData()
|
||||
writer.add_field("marker_id", marker_id)
|
||||
writer.add_field("file", data, filename=name)
|
||||
resp = await http.post("/api/houseplan/upload", data=writer)
|
||||
assert resp.status == 200, await resp.text()
|
||||
return (await resp.json())["url"]
|
||||
|
||||
first = await upload("m1", "manual.pdf", b"ONE")
|
||||
second = await upload("m1", "manual.pdf", b"TWO")
|
||||
assert first != second, "the second upload must not take the first name"
|
||||
|
||||
folder = os.path.join(hass.config.path(FILES_DIR), "m1")
|
||||
names = sorted(await hass.async_add_executor_job(os.listdir, folder))
|
||||
assert names == ["manual (2).pdf", "manual.pdf"]
|
||||
|
||||
got = await http.get(first.replace(CONTENT_URL, CONTENT_URL))
|
||||
assert await got.read() == b"ONE", "the first file is untouched"
|
||||
got2 = await http.get(second)
|
||||
assert await got2.read() == b"TWO"
|
||||
|
||||
@@ -381,3 +381,112 @@ def test_every_room_fill_mode_the_editor_offers_is_accepted():
|
||||
v.SPACE_SCHEMA(room(None)) # inherit from the space
|
||||
with pytest.raises(vol.Invalid):
|
||||
v.SPACE_SCHEMA(room("rainbow"))
|
||||
|
||||
|
||||
# ---------- attachments & inner limits (HP-1454-02, -05) ----------
|
||||
|
||||
|
||||
def test_unique_filename_never_returns_a_taken_name(tmp_path):
|
||||
unique_filename = plans.unique_filename
|
||||
d = tmp_path / "m1"
|
||||
d.mkdir()
|
||||
assert unique_filename(d, "manual.pdf") == "manual.pdf"
|
||||
(d / "manual.pdf").write_bytes(b"x")
|
||||
assert unique_filename(d, "manual.pdf") == "manual (2).pdf"
|
||||
(d / "manual (2).pdf").write_bytes(b"x")
|
||||
assert unique_filename(d, "manual.pdf") == "manual (3).pdf"
|
||||
# no extension, and a name that needs sanitising
|
||||
(d / "readme").write_bytes(b"x")
|
||||
assert unique_filename(d, "readme") == "readme (2)"
|
||||
assert unique_filename(d, "../../etc/passwd") == "passwd"
|
||||
|
||||
|
||||
def _acfg(*pairs):
|
||||
return {"markers": [{"id": f"m{i}", "pdfs": [{"url": f"/api/houseplan/content/files/{p}"}]}
|
||||
for i, p in enumerate(pairs)]}
|
||||
|
||||
|
||||
def test_attachment_refs_reads_marker_urls():
|
||||
attachment_refs = plans.attachment_refs
|
||||
assert attachment_refs(None) == set()
|
||||
assert attachment_refs(_acfg("m1/a.pdf", "m2/b.pdf")) == {"m1/a.pdf", "m2/b.pdf"}
|
||||
# legacy and foreign urls are not ours to collect against
|
||||
cfg = {"markers": [{"id": "m", "pdfs": [{"url": "/local/x.pdf"}, {"url": "/api/houseplan/content/files/deep/a/b.pdf"}]}]}
|
||||
assert plans.attachment_refs(cfg) == set()
|
||||
|
||||
|
||||
def test_collect_attachments_supersedes_and_ages(tmp_path):
|
||||
import os
|
||||
import time
|
||||
|
||||
collect_attachments = plans.collect_attachments
|
||||
files = tmp_path / "files"
|
||||
(files / "m1").mkdir(parents=True)
|
||||
for n in ("old.pdf", "new.pdf", "cancelled.pdf"):
|
||||
(files / "m1" / n).write_bytes(b"x")
|
||||
|
||||
# the commit swapped old.pdf for new.pdf; cancelled.pdf is a fresh upload
|
||||
# nobody saved — it may belong to a dialog that is still open
|
||||
removed = collect_attachments(files, _acfg("m1/old.pdf"), _acfg("m1/new.pdf"))
|
||||
assert removed == 1
|
||||
assert not (files / "m1" / "old.pdf").exists()
|
||||
assert (files / "m1" / "new.pdf").is_file()
|
||||
assert (files / "m1" / "cancelled.pdf").is_file()
|
||||
|
||||
old = time.time() - const.PLAN_ORPHAN_TTL_S - 60
|
||||
os.utime(files / "m1" / "cancelled.pdf", (old, old))
|
||||
assert collect_attachments(files, _acfg("m1/new.pdf"), _acfg("m1/new.pdf")) == 1
|
||||
assert not (files / "m1" / "cancelled.pdf").exists()
|
||||
assert (files / "m1" / "new.pdf").is_file()
|
||||
|
||||
|
||||
def test_collect_attachments_removes_the_empty_folder_and_never_raises(tmp_path):
|
||||
import os
|
||||
import time
|
||||
|
||||
files = tmp_path / "files"
|
||||
(files / "up_x").mkdir(parents=True)
|
||||
f = files / "up_x" / "orphan.pdf"
|
||||
f.write_bytes(b"x")
|
||||
old = time.time() - const.PLAN_ORPHAN_TTL_S - 60
|
||||
os.utime(f, (old, old))
|
||||
assert plans.collect_attachments(files, {}, {}) == 1
|
||||
assert not (files / "up_x").exists(), "the staging folder goes with its last file"
|
||||
assert plans.collect_attachments(tmp_path / "nope", {}, {}) == 0
|
||||
|
||||
|
||||
def test_inner_collection_limits():
|
||||
room = {"id": "r", "name": "R", "poly": [[0.1, 0.1]] * v.MAX_POLY_POINTS}
|
||||
v.ROOM_SCHEMA(room)
|
||||
with pytest.raises(vol.Invalid):
|
||||
v.ROOM_SCHEMA({**room, "poly": [[0.1, 0.1]] * (v.MAX_POLY_POINTS + 1)})
|
||||
|
||||
rect = {"id": "r", "name": "R", "x": 0.1, "y": 0.1, "w": 0.2, "h": 0.2}
|
||||
v.ROOM_SCHEMA({**rect, "open_to": ["x"] * v.MAX_OPEN_TO})
|
||||
with pytest.raises(vol.Invalid):
|
||||
v.ROOM_SCHEMA({**rect, "open_to": ["x"] * (v.MAX_OPEN_TO + 1)})
|
||||
|
||||
m = {"id": "m", "binding": "virtual"}
|
||||
v.MARKER_SCHEMA({**m, "controls": ["light.x"] * v.MAX_CONTROLS})
|
||||
with pytest.raises(vol.Invalid):
|
||||
v.MARKER_SCHEMA({**m, "controls": ["light.x"] * (v.MAX_CONTROLS + 1)})
|
||||
|
||||
pdf = {"name": "n", "url": "/api/houseplan/content/files/m/a.pdf"}
|
||||
v.MARKER_SCHEMA({**m, "pdfs": [pdf] * v.MAX_PDFS})
|
||||
with pytest.raises(vol.Invalid):
|
||||
v.MARKER_SCHEMA({**m, "pdfs": [pdf] * (v.MAX_PDFS + 1)})
|
||||
|
||||
v.MARKER_SCHEMA({**m, "name": "n" * v.MAX_TEXT})
|
||||
with pytest.raises(vol.Invalid):
|
||||
v.MARKER_SCHEMA({**m, "name": "n" * (v.MAX_TEXT + 1)})
|
||||
with pytest.raises(vol.Invalid):
|
||||
v.MARKER_SCHEMA({**m, "link": "u" * (v.MAX_URL + 1)})
|
||||
|
||||
|
||||
def test_legacy_segments_are_dropped_by_the_server():
|
||||
"""A limit that depends on the client stripping the field is not a limit."""
|
||||
out = v.SPACE_SCHEMA({
|
||||
"id": "f1", "title": "F", "aspect": 1.4, "view_box": [0, 0, 1, 1], "rooms": [],
|
||||
"segments": [[1, 2, 3, 4]] * 100000,
|
||||
})
|
||||
assert "segments" not in out
|
||||
|
||||
Reference in New Issue
Block a user