fuzzyworld

fuzzyworld-tools / commit

deploy-tools: migrate legacy ledger keys, harden against malformed input (Q118)

ab35fc22289d5f0e7b1c4ebceed2dfc4f280d471 · Pavel Safronov · 2026-11-06 21:21:35 -0800 · parents 258a9da83744

Claude Code usage: this commit 80.2K out / 9.4M cached (first commit of its session: everything since it began) · est. $4.15 (API list-price equivalent at 2026-10-22 rates, not what was paid) · session 17 tally 194 in / 80.2K out / 9.4M cached · usage by commit and session

deploy-tools: migrate legacy ledger keys, harden against malformed input (Q118)

Bare ledger keys (server, laptop-runner@host) are renamed to
<repo>/<target> in memory on load. A non-object ledger, a record without
a hash, a non-object target entry, or paths/command with pathspec magic,
globs or .. now give a clear error or are tolerated instead of crashing.
Deploy tab: time column labelled laptop time; clearer wording for
unknown commits; deploys.py docstring corrected. Two new tests.

FW-Tokens-In: 194
FW-Tokens-Out: 80198
FW-Tokens-Cached: 9357400
FW-Tokens-Method: 2
FW-Model-Tokens: claude-opus-5-5 in=72 out=23293 read=2153132 write5m=134429 write1h=0
FW-Model-Tokens: claude-sonnet-5-5 in=122 out=56905 read=7204268 write5m=0 write1h=323116
FW-Session: a21e027c-b96b-47a8-b49a-fcdf660be9b2
FW-Rtk-Saved: 4671172
FW-Rtk-Cmds: 14263

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
 bin/deploy-tools           | 37 +++++++++++++++++++++++++++----------
 docs/operations.md         |  1 +
 lib/admin.py               |  4 ++--
 server/deploys.py          | 10 ++++++----
 tests/test_admin.py        |  2 +-
 tests/test_deploy_tools.py | 23 +++++++++++++++++++++++
 6 files changed, 60 insertions(+), 17 deletions(-)

diff --git a/bin/deploy-tools b/bin/deploy-tools
index 907d5d8..a205bf0 100755
--- a/bin/deploy-tools
+++ b/bin/deploy-tools
@@ -37,7 +37,8 @@ PER_MACHINE = set()   # targets whose ledger key carries the hostname
 
 def clean_path(p):
     """A folder inside the repo, written plainly; as server/deploys.py clean_path (the two must agree)."""
-    return isinstance(p, str) and p != "" and posixpath.normpath(p) == p and not p.startswith(("/", "../")) and p != ".."
+    return (isinstance(p, str) and p != "" and posixpath.normpath(p) == p and not p.startswith(("/", "../")) and p != ".."
+            and not any(c in p for c in ":*?[]\\"))
 
 
 def declared_targets():
@@ -47,9 +48,9 @@ def declared_targets():
         pkg = json.load(f)
     found = {}
     for name, t in ((pkg.get("fuzzyworld") or {}).get("deploy") or {}).items():
-        paths, cmd = t.get("paths"), t.get("command")
+        paths, cmd = (t.get("paths"), t.get("command")) if isinstance(t, dict) else (None, None)
         ok = (NAME_RE.match(name) and isinstance(paths, list) and paths and all(clean_path(p) for p in paths)
-              and isinstance(cmd, list) and cmd and all(isinstance(c, str) for c in cmd))
+              and isinstance(cmd, list) and cmd and all(isinstance(c, str) for c in cmd) and clean_path(cmd[0]))
         if not ok:
             raise SystemExit(f'package.json: fuzzyworld.deploy.{name} needs a plain name, "paths" (folders inside the repo, '
                              'written plainly: no "./", trailing "/" or "..") and "command" (a list)')
@@ -95,9 +96,22 @@ def load():
     if not r.stdout.strip():
         return {}
     try:
-        return json.loads(r.stdout)
+        ledger = json.loads(r.stdout)
     except ValueError:
         raise SystemExit(f"{HOST}:{LEDGER} is not valid JSON; fix or delete it (saving now would wipe the other targets' records)")
+    if not isinstance(ledger, dict):
+        raise SystemExit(f"{HOST}:{LEDGER} is not a JSON object; fix or delete it (saving now would wipe the other targets' records)")
+    return ledger
+
+
+def migrate(ledger):
+    """Before Q111 the keys were "<target>" (and "laptop-runner@<host>"); now "<repo>/<target>". Rename each legacy
+    key of a declared target in memory, unless the new key exists; the next save writes the result."""
+    for name in TARGETS:
+        old = name + (f"@{socket.gethostname()}" if name in PER_MACHINE else "")
+        if old in ledger and key(name) not in ledger:
+            ledger[key(name)] = ledger.pop(old)
+    return ledger
 
 
 def save(ledger):
@@ -120,22 +134,25 @@ def main(argv):
         for n in names:
             if n not in TARGETS:
                 raise SystemExit(f"unknown target {n!r}; targets: {', '.join(TARGETS)}, all, status")
-    ledger = load()
+    ledger = migrate(load())
     if args == ["status"]:
         head = git("rev-parse", "--short", "HEAD")
         for name, (paths, _) in TARGETS.items():
             rec = ledger.get(key(name))
+            rec = rec if isinstance(rec, dict) else None
             state = ("never deployed" if not rec else
-                     "up to date" if rec["hash"] == content_hash(paths) else "PENDING (changed since deploy)")
-            when = f' commit {rec["commit"]}, {rec["at"]}' if rec else ""
+                     "up to date" if rec.get("hash") == content_hash(paths) else "PENDING (changed since deploy)")
+            when = f' commit {rec.get("commit", "?")}, {rec.get("at", "?")}' if rec else ""
             print(f"{name:14} {state}{when}")
         print(f"(HEAD is {head}; uncommitted edits count as changes)")
         return 0
     for name in names:
         paths, cmd = TARGETS[name]
         h = content_hash(paths)
-        if not force and ledger.get(key(name), {}).get("hash") == h:
-            print(f"{name}: already deployed (commit {ledger[key(name)]['commit']}), nothing to do. --force to redo.")
+        rec = ledger.get(key(name))
+        rec = rec if isinstance(rec, dict) else {}
+        if not force and rec.get("hash") == h:
+            print(f"{name}: already deployed (commit {rec.get('commit', '?')}), nothing to do. --force to redo.")
             continue
         print(f"{name}: deploying ({' '.join(cmd)})")
         if subprocess.run([os.path.join(ROOT, cmd[0]), *cmd[1:]], cwd=ROOT).returncode != 0:
@@ -143,7 +160,7 @@ def main(argv):
             return 1
         dirty = bool(git("status", "--porcelain", "--", *paths))
         ledger[key(name)] = {"hash": h, "commit": git("rev-parse", "--short", "HEAD") + ("+uncommitted" if dirty else ""),
-                        "at": time.strftime("%Y-%m-%d %H:%M")}
+                        "at": time.strftime("%Y-%m-%d %H:%M")}   # laptop time
         save(ledger)
     return 0
 
diff --git a/docs/operations.md b/docs/operations.md
index 84f3375..58a7b59 100644
--- a/docs/operations.md
+++ b/docs/operations.md
@@ -43,6 +43,7 @@ kept on the server, `~/fw/state/deployed.json`.
   | `vscode` | `vscode-web/` | `vscode-web/install.sh` |
 
 - **The ledger** records, for each `<repo>/<target>`, the commit and a content hash of the folders that target ships. A target is redeployed only when that hash differs. So uncommitted edits count, unrelated commits (README, tests) do not, and committing the files you already deployed does not trigger a second deploy. Gitignored files in those folders are not hashed.
+- Ledger keys used to be bare (`server`, `laptop-runner@<hostname>`). `bin/deploy-tools` renames such a key to `<repo>/<target>` in memory when it loads the ledger (an existing new key wins), and the next successful deploy saves the result. Times in the ledger are the laptop's local time.
 - The ledger is written only after the script succeeds, and any laptop sees the same state. The exception is `laptop-runner`, which is installed per machine and so is recorded per hostname (key `<repo>/laptop-runner@<hostname>`).
 - `fw deploys` and the [admin panel](admin.md#deploy-deploy-read-only-for-now) read the same ledger.
 - For flags `deploy-tools` does not pass (`--authorize`, `--remove`, `--out`) run the install script directly. To regenerate pages without installing, run `bin/fw publish-all`.
diff --git a/lib/admin.py b/lib/admin.py
index 639c59e..edb9911 100644
--- a/lib/admin.py
+++ b/lib/admin.py
@@ -211,7 +211,7 @@ def behind_cell(behind, state):
     if state == "never":
         return ""
     if behind is None:
-        return '<span class="meta">not on the server (uncommitted or unpushed deploy)</span>'
+        return '<span class="meta">not on the server (unpushed commit, or unknown repo)</span>'
     if not behind["count"]:
         return '<span class="meta">none</span>'
     items = "<br>".join(f"<code>{esc(sha)}</code> {esc(subject)}" for sha, subject in behind["commits"])
@@ -504,7 +504,7 @@ class Handler(BaseHTTPRequestHandler):
                                 f'<td>{state_badge(e["state"])}</td><td>{commit_cell(e["commit"])}</td><td>{esc(e["at"])}</td>'
                                 f'<td>{behind_cell(e["behind"], e["state"])}</td></tr>')
             out.append(f'<h2>{esc(r["repo"])} <span class="meta">(default branch at {esc(r["head"])})</span></h2>'
-                       '<table><tr><th>Target</th><th>State</th><th>Deployed from</th><th>When</th>'
+                       '<table><tr><th>Target</th><th>State</th><th>Deployed from</th><th>When (laptop time)</th>'
                        f'<th>Changes since</th></tr>{"".join(rows)}</table>')
         out.append("<h2>Apps</h2>")
         if data["apps"]:
diff --git a/server/deploys.py b/server/deploys.py
index 3ee3c60..154a137 100644
--- a/server/deploys.py
+++ b/server/deploys.py
@@ -1,15 +1,16 @@
 #!/usr/bin/env python3
 """What is deployed where. Python standard library only.
 
-Usage (on the server: `fw deploys`; from the laptop: `bin/fw deploys`):
+Usage (on the server: `fw deploys`; from the laptop: `ssh dh ~/bin/fw deploys`, since bin/fw does not forward it yet, Q114):
 
   deploys [--json]    plain text, or JSON for `fw admin`:
                       {"repos": [{repo, head, targets: [{target, description, paths, entries: [...]}]}], "apps": [...]}
 
 Two sources, both read-only:
   * the deploy ledger ~/fw/state/deployed.json (FW_STATE), written by `bin/deploy-tools` on the laptop: per
-    "<repo>/<target>" (<repo> is the bare repo's name: package.json "fuzzyworld": {"repo": ...} if the package is named differently) (a per-machine target also "@<hostname>") the commit, the content hash of the folders the target
-    ships, and the time. A repo declares its targets in package.json, "fuzzyworld": {"deploy": {<target>: {"paths":
+    "<repo>/<target>" (a per-machine target also "@<hostname>") the commit, the content hash of the folders the
+    target ships, and the time. <repo> is the bare repo's name (package.json "fuzzyworld": {"repo": ...} when the
+    package is named differently). A repo declares its targets in package.json, "fuzzyworld": {"deploy": {<target>: {"paths":
     [...], "description": "..."}}}, read here from the repo's default branch (HEAD of the bare repo = what is shipped).
   * each deployed app's apps.fuzzyworld.net/<app>/.fuzzyworld.json (written by `fw deploy`): the repo, the commit and
     the time of its last deploy.
@@ -17,7 +18,8 @@ Two sources, both read-only:
 State of a target entry: "current" (the ledger's hash equals the hash of those folders at the repo's HEAD),
 "pending" (it differs), "never" (no ledger entry). `behind` lists the commits at HEAD since the deployed commit that
 touch the target's folders (for an app: any file except documentation and CI config, see APP_IGNORED); it is null when the deployed commit is not in the server's
-repo (for instance a deploy of uncommitted work, or of a commit that was never pushed).
+repo (a commit that was never pushed, or a repo or commit field that is missing). A "+uncommitted" suffix is ignored
+when looking the commit up.
 """
 
 import json
diff --git a/tests/test_admin.py b/tests/test_admin.py
index adef4a7..c38cd1c 100644
--- a/tests/test_admin.py
+++ b/tests/test_admin.py
@@ -357,7 +357,7 @@ class Pages(Base):
         self.assertIn("Fix &lt;i&gt;it&lt;/i&gt;", body)
         self.assertIn("and 2 more", body)                     # 10 commits behind, 8 listed
         self.assertIn("on mac-a", body)
-        self.assertIn("not on the server (uncommitted or unpushed deploy)", body)
+        self.assertIn("not on the server (unpushed commit, or unknown repo)", body)
         self.assertIn("never deployed", body)
         self.assertLess(body.index("&lt;script&gt;"), body.index("Zed"))   # a pending app is listed before a current one
         self.assertEqual(self.fake.calls[-1], ["deploys", "--json"])
diff --git a/tests/test_deploy_tools.py b/tests/test_deploy_tools.py
index e5ff067..246052c 100644
--- a/tests/test_deploy_tools.py
+++ b/tests/test_deploy_tools.py
@@ -159,6 +159,29 @@ class DeployTools(unittest.TestCase):
             with self.assertRaises(SystemExit):
                 dt.declared_targets()
 
+    def test_legacy_ledger_keys_are_renamed_in_memory(self):
+        targets = {"server": (["server"], ["x"]), "laptop-runner": (["server"], ["x"])}
+        with mock.patch.object(dt, "TARGETS", targets), mock.patch.object(dt, "REPO", "demo"), \
+                mock.patch.object(dt, "PER_MACHINE", {"laptop-runner"}), \
+                mock.patch.object(dt.socket, "gethostname", return_value="mac-a"):
+            ledger = {"server": {"hash": "h1"}, "laptop-runner@mac-a": {"hash": "h2"}, "laptop-runner@mac-b": {"hash": "h3"},
+                      "demo/server": {"hash": "new"}}
+            out = dt.migrate(ledger)
+            self.assertEqual(out["demo/server"], {"hash": "new"})  # an existing new key wins
+            self.assertEqual(out["demo/laptop-runner@mac-a"], {"hash": "h2"})
+            self.assertNotIn("laptop-runner@mac-a", out)
+            self.assertIn("laptop-runner@mac-b", out)  # another machine's record is left alone
+
+    def test_malformed_ledger_and_targets_give_clear_errors(self):
+        self.ledger = {"demo/server": {"commit": "abc"}}  # a record without a hash is just "changed"
+        with mock.patch.object(dt, "TARGETS", {"server": (["server"], ["x"])}), mock.patch.object(dt, "REPO", "demo"):
+            self.assertEqual(dt.main(["x", "status"]), 0)
+        for bad in ({"a": "x"}, {"a": {"paths": ["server"], "command": ["../x"]}},
+                    {"a": {"paths": [":(top)"], "command": ["x"]}}, {"a": {"paths": ["s*"], "command": ["x"]}}):
+            self.write("package.json", json.dumps({"name": "demo", "fuzzyworld": {"deploy": bad}}))
+            with self.assertRaises(SystemExit):
+                dt.declared_targets()
+
     def test_repo_name_can_be_overridden_and_must_be_plain(self):
         self.write("package.json", json.dumps({"name": "@scope/pkg", "fuzzyworld": {"repo": "server-name", "deploy": {}}}))
         self.assertEqual(dt.declared_targets()[0], "server-name")