From b01c7691211a0c0e6f00fc71b2d7cb5cc3913515 Mon Sep 17 00:00:00 2001 From: Vito Sansevero Date: Fri, 30 Oct 2026 09:16:01 -0700 Subject: [PATCH] test(images): pin that a reverse-proxied browser is never local (#533) Direct loopback, proxied with forwarded headers, proxied with a loopback peer and a spoofed local Host, plus the case that cannot be told apart, recorded in TESTING.md. --- TESTING.md | 12 +++++--- tests/test_image_reveal.py | 62 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 4 deletions(-) diff --git a/TESTING.md b/TESTING.md index dc3be25c..1a2d8bdf 100644 --- a/TESTING.md +++ b/TESTING.md @@ -1615,10 +1615,14 @@ Recorded deviations on this page (spec ยง8): LoRA Manager's open-location route has no local-only gate, so it also opens the server's file manager for a proxied or remote browser; ours says it cannot. - Residual risk, accepted: a reverse proxy or tunnel that adds no forwarding header - (a bare nginx `proxy_pass` to 127.0.0.1, `ssh -R`, `socat`) cannot be told apart - from a local client, and a non-browser client behind it can forge a matching - `Host` and `Origin`. Such a client can open a folder of a listed file on the host's + A browser behind a proxy that keeps its `Host` or `Origin`, or adds a forwarding + header, or connects from another address (a Docker bridge) is always `not-local`. + + Residual risk, accepted: a tunnel or proxy that adds no forwarding header and + rewrites `Host` and `Origin` to loopback (`ssh -L` or `-R`, `socat`, a bare + `proxy_pass` to 127.0.0.1 with both set), and a non-browser client behind a bare + proxy that forges them, cannot be told apart from a local client: the folder opens + on the host's screen and the toast says it opened. Such a client can open a folder of a listed file on the host's screen, at most once a second, and learn whether that file exists (which the listing routes already tell it). Operators who expose ComfyUI that way accept this. - **D39** (ruled 2026-10-21: fallback): a recipe-linked image shows its recipe's own diff --git a/tests/test_image_reveal.py b/tests/test_image_reveal.py index 3522e4ba..5fa492f6 100644 --- a/tests/test_image_reveal.py +++ b/tests/test_image_reveal.py @@ -118,6 +118,56 @@ class TestIsLocalRequest(unittest.TestCase): self.assertFalse(is_local_request("127.0.0.1", headers(Origin=origin))) self.assertFalse(is_local_request("127.0.0.1", headers(Origin=ORIGIN), "https")) + def test_a_browser_behind_a_reverse_proxy_is_never_local(self): + # The maintainer's setup (#533): a Mac reaching the Linux box through + # a Docker proxy at comfy.sansfam.org. Every line must fail safe. + site = "comfy.sansfam.org" + origin = f"https://{site}" + forwarded = {"X_Forwarded_For": "192.168.1.30", "X_Forwarded_Proto": "https"} + cases = { + "docker bridge peer, forwarded headers": ( + "172.18.0.2", + headers(Host=site, Origin=origin, **forwarded), + ), + "docker bridge peer, bare proxy": ( + "172.18.0.2", + headers(Host=site, Origin=origin), + ), + "loopback peer, forwarded headers": ( + "127.0.0.1", + headers(Host=site, Origin=origin, **forwarded), + ), + "loopback peer, Host kept, no forwarded headers": ( + "127.0.0.1", + headers(Host=site, Origin=origin), + ), + "loopback peer, Host rewritten, Origin kept": ( + "127.0.0.1", + headers(Origin=origin), + ), + "loopback peer, everything rewritten but a forwarded header": ( + "127.0.0.1", + headers(X_Real_IP="192.168.1.30"), + ), + "spoofed local Host from another computer": ( + "192.168.1.30", + headers(), + ), + "spoofed local forwarded value": ( + "127.0.0.1", + headers(X_Forwarded_For="127.0.0.1"), + ), + } + for name, (remote, request_headers) in cases.items(): + with self.subTest(name): + self.assertFalse(is_local_request(remote, request_headers, "https")) + self.assertFalse(is_local_request(remote, request_headers, "http")) + + def test_what_cannot_be_told_apart_from_a_local_browser(self): + # Known limit, in TESTING.md: an ssh -L tunnel, or a proxy that rewrites + # Host and Origin and adds no header, looks exactly like this. + self.assertTrue(is_local_request("127.0.0.1", headers())) + def test_sec_fetch_site_must_be_same_origin_when_sent(self): self.assertTrue( is_local_request("127.0.0.1", headers(Sec_Fetch_Site="same-origin")) @@ -840,6 +890,18 @@ class TestRevealRoute(ImageAPITestCase): self.assertEqual((status, data["code"]), (403, "not-local")) self.assertEqual(self.opened, []) + async def test_a_proxied_browser_gets_the_message_and_opens_nothing(self): + make_png(self.output_dir / "sub" / "a.png") + status, data = await self.post( + self.body(), + Host="comfy.sansfam.org", + Origin="https://comfy.sansfam.org", + **{"X-Forwarded-For": "192.168.1.30"}, + ) + self.assertEqual((status, data["code"]), (403, "not-local")) + self.assertIn("only on the computer running ComfyUI", data["error"]) + self.assertEqual(self.opened, []) + async def test_missing_or_foreign_origin_is_not_local(self): make_png(self.output_dir / "sub" / "a.png") port = self.client.make_url("/").port