Fix the final review's findings on the combined features

- Links to localhost need FRAME_CONTROL_LOCAL_LINKS=1: otherwise any website's
  link could make the app fetch from services on this computer.
- Title staging folders (unzipped titles) carry the server's PID and are swept
  on the next start like download folders, so quitting mid-install doesn't leave
  gigabytes behind.
- An install from a link refreshes Sideloaded titles.

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

docs/web-install.md

@@ -60,9 +60,11 @@ What gets installed depends on the file name's extension:
 Frame Control refuses a link, and downloads nothing, unless:
 
 - Every URL (the manifest's, the file's and each redirect) is `https://`.
-  `http://` works only for `localhost` or `127.0.0.1`, for testing, and only
+  `http://` works only for `localhost` or `127.0.0.1`, for testing: only when
-  when the link itself points there: a public manifest can't send Frame
+  Frame Control runs with `FRAME_CONTROL_LOCAL_LINKS=1`, and only when the
-  Control to your own computer.
+  link itself points there. It's off by default so a website's link can't make
+  the app fetch from services on your computer, and a public manifest can
+  never send it there.
 - No URL has a user name or password in it (`https://user:pw@…`).
 - No host is, or resolves to, a private, loopback, link-local, CGNAT
   (100.64.0.0/10), multicast or otherwise non-public address. Every address
@@ -116,7 +118,9 @@ Control" link. It isn't published anywhere yet; host a copy to use it.
 
 ## Testing locally
 
-Serve the manifest and file from your own computer:
+Start Frame Control with `FRAME_CONTROL_LOCAL_LINKS=1` in its environment (for
+example `FRAME_CONTROL_LOCAL_LINKS=1 npm start` in `app/`), then serve the
+manifest and file from your own computer:
 
 ```sh
 cd mygame && python3 -m http.server 8000

tests/test_webinstall.py

@@ -181,10 +181,20 @@ class Downloads(unittest.TestCase):
 
     def setUp(self):
         self.tmp = tempfile.mkdtemp()
+        env = mock.patch.dict(os.environ, {wi.LOCAL_LINKS_ENV: "1"})
+        env.start()
+        self.addCleanup(env.stop)
 
     def tearDown(self):
         shutil.rmtree(self.tmp, ignore_errors=True)
 
+    def test_localhost_links_need_the_developer_switch(self):
+        # Without it, a website's link can't make the app fetch from local services.
+        with mock.patch.dict(os.environ, {wi.LOCAL_LINKS_ENV: ""}):
+            for kw in ({"manifest": f"{self.base}/manifest.json"}, {"url": f"{self.base}/game.apk"}):
+                with self.assertRaisesRegex(wi.WebInstallError, wi.LOCAL_LINKS_ENV):
+                    wi.plan(**kw)
+
     def test_manifest_round_trip(self):
         p = wi.plan(manifest=f"{self.base}/manifest.json")
         self.assertEqual((p["name"], p["file"], p["kind"], p["host"], p["size"]),
@@ -371,16 +381,17 @@ class ServerJobs(unittest.TestCase):
     def test_dead_servers_leftovers_swept(self):
         dead = subprocess.Popen([sys.executable, "-c", "pass"])
         dead.wait()
-        prefix = self.server.WEB_TMP_PREFIX
+        # Downloads and title staging (unzipped titles) are both swept.
-        gone = tempfile.mkdtemp(prefix=f"{prefix}{dead.pid}-")
+        for prefix in (self.server.WEB_TMP_PREFIX, self.server.frame_titles.TMP_PREFIX):
-        live = tempfile.mkdtemp(prefix=f"{prefix}{os.getpid()}-")
+            gone = tempfile.mkdtemp(prefix=f"{prefix}{dead.pid}-")
-        try:
+            live = tempfile.mkdtemp(prefix=f"{prefix}{os.getpid()}-")
-            self.server.sweep_webinstall_tmp()
+            try:
-            self.assertFalse(os.path.exists(gone))
+                self.server.sweep_tmp()
-            self.assertTrue(os.path.exists(live))
+                self.assertFalse(os.path.exists(gone), prefix)
-        finally:
+                self.assertTrue(os.path.exists(live), prefix)
-            shutil.rmtree(gone, ignore_errors=True)
+            finally:
-            shutil.rmtree(live, ignore_errors=True)
+                shutil.rmtree(gone, ignore_errors=True)
+                shutil.rmtree(live, ignore_errors=True)
 
     def test_temp_dir_failure_ends_the_job(self):
         job, dispatch = self.run_job(mkdtemp_error=OSError("disk full"))

ui/frame_titles.py

@@ -39,6 +39,7 @@ DIR_RE = re.compile(r'^/[A-Za-z0-9_./-]+$')
 # Zip limits: well above any real game, well below a zip bomb.
 MAX_UNPACKED = 64 * 1024**3
 MAX_ENTRIES = 200000
+TMP_PREFIX = 'frame-title-'  # then the server's PID, so server.sweep_tmp can clear a killed run's
 MAX_RATIO = 200                           # uncompressed / compressed, once past 1 GB
 
 # The Steam compat tool aliases Valve's client uses (devkit_client RUNTIME_ALIASES).
@@ -423,15 +424,15 @@ def inspect(path, name=None):
             if _has_links(root):
                 # scp -r follows links, so a link out of the folder could upload
                 # anything; copy the folder with its links made safe first.
-                work = tempfile.mkdtemp(prefix='frame-title-')
+                work = tempfile.mkdtemp(prefix=f'{TMP_PREFIX}{os.getpid()}-')
                 root = _stage_folder(root, os.path.join(work, os.path.basename(root)))
         elif path.lower().endswith('.zip'):
-            work = tempfile.mkdtemp(prefix='frame-title-')
+            work = tempfile.mkdtemp(prefix=f'{TMP_PREFIX}{os.getpid()}-')
             extract_zip(path, work)
             root = _unwrap(work)
         elif classify(path):
             # A single executable is uploaded on its own; don't copy a whole Downloads folder.
-            work = tempfile.mkdtemp(prefix='frame-title-')
+            work = tempfile.mkdtemp(prefix=f'{TMP_PREFIX}{os.getpid()}-')
             shutil.copy2(path, os.path.join(work, os.path.basename(path)))
             root = work
         else:

ui/frame_webinstall.py

@@ -10,10 +10,10 @@ A manifest is the same JSON FrameDrop uses, so one works for both tools:
 "frame-control.install/v1" is accepted with the same shape.
 
 Rules: HTTPS only, except http(s)://localhost or 127.0.0.1 for testing, and then
-only when the link itself points there. No credentials in URLs, no private,
+only with FRAME_CONTROL_LOCAL_LINKS=1 set and when the link itself points there.
-loopback, link-local or CGNAT addresses (checked on every redirect, and the
+No credentials in URLs, no private, loopback, link-local or CGNAT addresses
-connection goes to the address that was checked, so DNS can't change it in
+(checked on every redirect, and the connection goes to the address that was
-between). The file URL must end in a file name.
+checked, so DNS can't change it in between). The file URL must end in a file name.
 
 Python stdlib only, 3.9 compatible.
 """
@@ -39,6 +39,8 @@ MAX_REDIRECTS = 5
 TIMEOUT = 30                  # seconds per socket operation
 CHUNK = 1 << 20
 LOCAL_HOSTS = ("localhost", "127.0.0.1")
+# Off by default: otherwise any website could make the app fetch from local services.
+LOCAL_LINKS_ENV = "FRAME_CONTROL_LOCAL_LINKS"
 USER_AGENT = "FrameControl (+https://github.com/saphid/steam-frame)"
 # What dispatch() can install, by file extension.
 KINDS = {".apk": "apk", ".zip": "title", ".exe": "title"}
@@ -101,7 +103,7 @@ def check_url(url, allow_local=False):
         raise WebInstallError("the URL has no host")
     local = host in LOCAL_HOSTS
     if local and not allow_local:
-        raise WebInstallError("localhost is only allowed when the link itself points there (for testing)")
+        raise WebInstallError(f"localhost links are for testing: set {LOCAL_LINKS_ENV}=1, and the link itself must point there")
     if scheme == "http" and not local:
         raise WebInstallError("only https:// is allowed (http:// only for localhost while testing)")
     if not local:
@@ -342,9 +344,10 @@ def plan(manifest=None, url=None):
     if (manifest is None) == (url is None):
         raise WebInstallError("give either manifest or url")
     link = manifest if manifest is not None else url
-    # localhost is for testing a link on your own computer, so only a link that
+    # localhost is for testing a link on your own computer: only with the developer
-    # starts there may reach it: a public manifest can't point at localhost.
+    # switch on, and only for a link that starts there (a public manifest can't
-    allow_local = check_url(link, allow_local=True)[3]
+    # point at localhost).
+    allow_local = check_url(link, allow_local=os.environ.get(LOCAL_LINKS_ENV) == "1")[3]
     if manifest is not None:
         m = fetch_manifest(manifest, allow_local)
         name, f = m["name"], m["file"]

ui/index.html

@@ -1880,7 +1880,7 @@ async function wiPoll(gen) {
     if (j.phase === "done") {
       log(j.message, "ok"); toast(j.message);
       $("wiDlg").close();
-      loadAndroid(); refresh();
+      loadAndroid(); loadTitles(); refresh();
     } else {
       $("wiMsg").textContent = j.error;
       log(`Install from link failed: ${j.error}`, "e"); toast(`Install failed: ${j.error}`, true);

ui/server.py

@@ -890,7 +890,7 @@ _web_jobs = {}    # id -> progress of the confirmed install (only the latest is
 _web_workers = set()  # threads running an install, joined on shutdown
 _web_closing = False  # set on shutdown; no new installs after that
 MAX_WEB_PLANS = 8
-WEB_TMP_PREFIX = "frame-webinstall-"  # then the server's PID, for sweep_webinstall_tmp
+WEB_TMP_PREFIX = "frame-webinstall-"  # then the server's PID, for sweep_tmp
 
 
 def webinstall_check(body):
@@ -1003,7 +1003,7 @@ def webinstall_cancel(body):
 def webinstall_shutdown():
     """Stop downloads and give workers a moment to delete their temporary files.
 
-    An install already copying to the Frame may outlive this; sweep_webinstall_tmp
+    An install already copying to the Frame may outlive this; sweep_tmp
     removes what it leaves on a later start.
     """
     global _web_closing
@@ -1044,21 +1044,26 @@ def _pid_alive(pid):
     return True
 
 
-def sweep_webinstall_tmp():
+def sweep_tmp():
-    """Delete download folders left by a server that was killed mid-install.
+    """Delete download and title staging folders left by a server killed mid-install.
 
     Folders carry the server's PID, so only a dead server's are taken.
     """
-    for d in Path(tempfile.gettempdir()).glob(f"{WEB_TMP_PREFIX}*"):
+    for prefix in (WEB_TMP_PREFIX, frame_titles.TMP_PREFIX):
-        m = re.fullmatch(re.escape(WEB_TMP_PREFIX) + r"(\d+)-.*", d.name)
+        for d in Path(tempfile.gettempdir()).glob(f"{prefix}*"):
-        if not m:
+            _sweep_one(prefix, d)
-            continue
+
-        pid = int(m[1])
+
-        try:
+def _sweep_one(prefix, d):
-            if pid != os.getpid() and not _pid_alive(pid) and d.is_dir():
+    m = re.fullmatch(re.escape(prefix) + r"(\d+)-.*", d.name)
-                shutil.rmtree(d, ignore_errors=True)
+    if not m:
-        except OSError:
+        return
-            pass
+    pid = int(m[1])
+    try:
+        if pid != os.getpid() and not _pid_alive(pid) and d.is_dir():
+            shutil.rmtree(d, ignore_errors=True)
+    except OSError:
+        pass
 
 
 POST = {"/api/android/display": android_display, "/api/android": android, "/api/titles": titles, "/api/launch": launch, "/api/steam": steam, "/api/volume": set_volume, "/api/clipboard": clipboard,
@@ -1350,7 +1355,7 @@ def main():
                          "Windows has no SIGTERM to catch)")
     args = ap.parse_args()
     httpd = ThreadingHTTPServer(("127.0.0.1", args.port), Handler)
-    sweep_webinstall_tmp()
+    sweep_tmp()
     if not frame_host.WINDOWS:
         signal.signal(signal.SIGTERM, lambda *_: (_ for _ in ()).throw(KeyboardInterrupt))
     if args.exit_on_eof: