497f2788d49fda531c0836618688f954486eb87c
mspeir
  Fri Sep 4 09:44:13 2026 -0700
trackLists: do not let the public page claim the download cross-check is clean, refs #37781

The static-page rewrite collapsed mkPage.py's three-way branch on the
hgdownload cross-check into two, so the public run fell into the all-clear
wording whether or not collect.py had found a restricted file that is still
reachable. collect.py's current output has three such files, so the page was
asserting the opposite of what the check found. Restored as three cases: the
public page says only that the check runs and that anything found is reported
privately, the --internal page keeps the table of paths, and the all-clear
wording is used only when the list really is empty. The comment explaining why
the public branch must not be merged back into the all-clear one is back too.

collect.py counted any HTTP code that was not literally 404 as reachable, which
made a curl timeout or a failed connection (empty output, or 000) look like an
exposed file. That mails false alarms and, now that the branch above depends on
it, would drop the all-clear line from the public page on a network blip. Only
2xx and 3xx count as served, 4xx as blocked, and anything else is reported
separately as not checked, in collected.json and on stderr.

Also: the remaining unquoted interpolations into shell=True commands go through
the existing q() helper, rows_by_track() no longer builds the "why" set that the
new table does not use, and US spelling throughout.

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

diff --git src/hg/utils/otto/trackLists/collect.py src/hg/utils/otto/trackLists/collect.py
index 7ee57448b00..1f756fd66b5 100755
--- src/hg/utils/otto/trackLists/collect.py
+++ src/hg/utils/otto/trackLists/collect.py
@@ -1,28 +1,28 @@
 #!/usr/bin/env python3
 """
 Collect the three lists behind the mirror/redistribution page (RM #37781):
 
   1. Tracks we are not allowed to redistribute
   2. Tracks that update themselves (otto)
   3. Contributed tracks (GenArk)
 
 No single trackDb setting marks every restricted track, so list 1 is the union of
 several tests, and each row records which ones fired:
 
   tableBrowser off                    the usual marker
-  noGenomeReason citing licence terms how OMIM is marked; a query for "off" misses it
+  noGenomeReason citing license terms how OMIM is marked; a query for "off" misses it
   absent from hgdownload              ground truth for MySQL tables
   reachable on hgdownload             ground truth the other way, and the only test
                                       that catches a restricted file we are still serving
 
 Writes collected.json for mkPage.py.
 """
 import re, os, sys, json, time, argparse, subprocess, collections
 from concurrent.futures import ThreadPoolExecutor
 
 BETA     = "hgwbeta"
 DL       = "https://hgdownload.soe.ucsc.edu"
 SKIP_DBS = {"information_schema", "mysql", "performance_schema", "sys", "hgFixed",
             "go", "proteome", "uniProt", "visiGene"}
 GENARK   = "/gbdb/genark"
 CONTRIB_MAX_AGE = 7 * 86400          # re-crawl GenArk at most weekly
@@ -33,50 +33,52 @@
         return subprocess.run(cmd, shell=True, capture_output=True, text=True,
                               errors="replace", timeout=timeout).stdout
     except subprocess.TimeoutExpired:
         return ""
 
 def q(s):
     return "'" + s.replace("'", "'\\''") + "'"
 
 def note(msg):
     print(msg, file=sys.stderr, flush=True)
 
 # --- trackDb ---------------------------------------------------------------
 
 def databases():
     out = []
-    for d in sh("hgsql -h %s -N -e 'show databases' 2>/dev/null" % BETA).split():
+    for d in sh("hgsql -h %s -N -e %s 2>/dev/null"
+                % (q(BETA), q("show databases"))).split():
         if d in SKIP_DBS or d.startswith("hgcentral"):
             continue
         out.append(d)
     return sorted(out)
 
 def parse_settings(blob):
     d = {}
     for ln in blob.replace("\\n", "\n").split("\n"):
         if " " in ln:
             k, v = ln.split(" ", 1)
             d[k.strip()] = v.strip()
     return d
 
 def trackdb(dbs):
     tdb = collections.defaultdict(dict)
     def one(db):
         rows = {}
         for line in sh("hgsql -h %s -N -e %s %s 2>/dev/null"
-                       % (BETA, q("select tableName, settings from trackDb"), db)).splitlines():
+                       % (q(BETA), q("select tableName, settings from trackDb"),
+                          q(db))).splitlines():
             p = line.split("\t")
             if len(p) >= 2:
                 rows[p[0]] = parse_settings(p[1])
         return db, rows
     with ThreadPoolExecutor(max_workers=8) as ex:
         for db, rows in ex.map(one, dbs):
             tdb[db] = rows
     return tdb
 
 LICENSE_RE = re.compile(r"distribut|licen|restrict|permission|agreement", re.I)
 
 def restricted_from_trackdb(tdb):
     out = {}
     for db, tt in tdb.items():
         for t, s in tt.items():
@@ -87,57 +89,57 @@
             elif ngr and LICENSE_RE.search(ngr):
                 why = "noGenomeReason cites distribution terms"
             if why:
                 out[(db, t)] = dict(shortLabel=s.get("shortLabel", ""),
                                     longLabel=s.get("longLabel", ""),
                                     bigDataUrl=s.get("bigDataUrl", ""),
                                     reason=ngr, why=[why])
     return out
 
 # --- hgdownload ------------------------------------------------------------
 
 def dl_listing(db, cache):
     f = os.path.join(cache, "dl_%s.txt" % db)
     if os.path.exists(f) and time.time() - os.path.getmtime(f) < DL_MAX_AGE:
         return db, set(open(f).read().split())
-    html = sh("curl -s --max-time 90 '%s/goldenPath/%s/database/'" % (DL, db))
+    html = sh("curl -s --max-time 90 %s" % q("%s/goldenPath/%s/database/" % (DL, db)))
     tabs = sorted(set(re.findall(r"([A-Za-z0-9_]+)\.txt\.gz", html)))
     with open(f, "w") as fh:
         fh.write("\n".join(tabs))
     return db, set(tabs)
 
 def http_code(path):
-    return sh("curl -s -o /dev/null --max-time 30 -w '%%{http_code}' -r 0-99 '%s%s' < /dev/null"
-              % (DL, path)).strip()
+    return sh("curl -s -o /dev/null --max-time 30 -w '%%{http_code}' -r 0-99 %s < /dev/null"
+              % q(DL + path)).strip()
 
 # --- GenArk contributed ----------------------------------------------------
 
 def contrib_crawl(cache, refresh=False):
     """Crawl /gbdb/genark for contrib/<name> dirs. Slow (>10 min), so cached.
 
     Written to a temp file and renamed, because a crawl cut short mid-write
     leaves a plausible-looking but wrong list."""
     f = os.path.join(cache, "contrib.txt")
     fresh = os.path.exists(f) and time.time() - os.path.getmtime(f) < CONTRIB_MAX_AGE
     if fresh and not refresh:
         note("contrib: using cache (%.1f days old)"
              % ((time.time() - os.path.getmtime(f)) / 86400))
     else:
         note("contrib: crawling %s, this takes >10 minutes ..." % GENARK)
         tmp = f + ".tmp"
         rc = subprocess.run("find -L %s -mindepth 7 -maxdepth 7 -type d -path '*/contrib/*' "
-                            "> %s 2>/dev/null" % (GENARK, tmp), shell=True)
+                            "> %s 2>/dev/null" % (q(GENARK), q(tmp)), shell=True)
         if rc.returncode == 0 and os.path.getsize(tmp) > 0:
             os.replace(tmp, f)
             note("contrib: crawl done, %d rows" % sum(1 for _ in open(f)))
         else:
             if os.path.exists(tmp):
                 os.remove(tmp)
             note("contrib: crawl FAILED; keeping previous cache" if os.path.exists(f)
                  else "contrib: crawl FAILED and no cache exists")
     if not os.path.exists(f):
         return []
     per = collections.defaultdict(set)
     for line in open(f):
         line = line.strip()
         if "/contrib/" not in line:
             continue
@@ -271,31 +273,32 @@
     dbs = databases()
     note("databases: %d" % len(dbs))
     tdb = trackdb(dbs)
     note("trackDb loaded (%.0fs)" % (time.time() - t0))
 
     restricted = restricted_from_trackdb(tdb)
     note("flagged in trackDb: %d" % len(restricted))
 
     # ground truth 1: real MySQL tracks absent from hgdownload
     t1 = time.time()
     with ThreadPoolExecutor(max_workers=12) as ex:
         listings = dict(ex.map(lambda d: dl_listing(d, a.cache), dbs))
     note("hgdownload listings: %.0fs" % (time.time() - t1))
 
     def tables(db):
-        return set(sh("hgsql -h %s -N -e 'show tables' %s 2>/dev/null" % (BETA, db)).split())
+        return set(sh("hgsql -h %s -N -e %s %s 2>/dev/null"
+                      % (q(BETA), q("show tables"), q(db))).split())
     with ThreadPoolExecutor(max_workers=8) as ex:
         have = dict(zip(dbs, ex.map(tables, dbs)))
     partial = []
     for db in dbs:
         pub = listings.get(db) or set()
         if not pub:
             continue                       # nothing published for this db; no signal
         mine = have[db] & set(tdb[db])
         missing = mine - pub
         # An assembly whose downloads are simply not published yet makes every one
         # of its tracks look withheld. Only believe this test when the db is
         # otherwise well published; otherwise say so and move on.
         if mine and len(missing) > 0.2 * len(mine):
             partial.append(dict(db=db, missing=len(missing), tracks=len(mine),
                                 published=len(pub)))
@@ -304,54 +307,69 @@
             if (db, t) in restricted:
                 restricted[(db, t)]["why"].append("absent from hgdownload")
             else:
                 s = tdb[db][t]
                 restricted[(db, t)] = dict(shortLabel=s.get("shortLabel", ""),
                                            longLabel=s.get("longLabel", ""),
                                            bigDataUrl=s.get("bigDataUrl", ""),
                                            reason="", why=["absent from hgdownload"])
 
     # ground truth 2: anything we call restricted that hgdownload still serves
     checks = [(db, t, v["bigDataUrl"].replace("$D", db))
               for (db, t), v in restricted.items()
               if v["bigDataUrl"] and not v["bigDataUrl"].startswith("http")]
     with ThreadPoolExecutor(max_workers=8) as ex:
         codes = list(ex.map(lambda c: http_code(c[2]), checks))
-    exposed = []
+    exposed, unchecked = [], []
     for (db, t, p), code in zip(checks, codes):
         restricted[(db, t)]["hgdownload"] = code
-        if code != "404":
+        # Only a real HTTP response says anything about the file. A curl that timed
+        # out or could not connect comes back empty or as 000, and calling that
+        # "reachable" both mails a false alarm and drops the all-clear line from the
+        # public page on nothing more than a network blip. 4xx means blocked, 2xx and
+        # 3xx mean served, and anything else means the test did not run.
+        if code[:1] in ("2", "3"):
             exposed.append(dict(db=db, track=t, path=p, code=code,
                                 shortLabel=restricted[(db, t)]["shortLabel"]))
+        elif code[:1] != "4":
+            unchecked.append(dict(db=db, track=t, path=p, code=code or "none"))
 
     contrib = [] if a.no_contrib else contrib_crawl(a.cache, a.refresh_contrib)
 
     result = dict(
         restricted=[dict(db=db, track=t, **v) for (db, t), v in sorted(restricted.items())],
         exposed=exposed,
         otto=parse_otto(a.otto_crontab, tdb),
         contrib=contrib,
         partialDownloads=partial,
+        uncheckedDownloads=unchecked,
         counts=dict(databases=len(dbs)),
         generated=time.strftime("%Y-%m-%d"),
     )
     json.dump(result, open(a.out, "w"), indent=1)
     note("wrote %s in %.0fs: %d restricted rows, %d exposed, %d otto jobs, %d contributors"
          % (a.out, time.time() - t0, len(result["restricted"]), len(exposed),
             len(result["otto"]), len(contrib)))
     if partial:
         note("")
         note("note: %d database(s) have too little published on hgdownload for the"
              % len(partial))
         note("      'absent from hgdownload' test to mean anything, so they were skipped:")
         for p in partial:
             note("      %-14s %d of %d trackDb tables published"
                  % (p["db"], p["published"], p["tracks"]))
     if exposed:
         note("")
         note("*** %d file(s) marked restricted are reachable on hgdownload:" % len(exposed))
         for e in exposed:
             note("      %s  %s  %s" % (e["db"], e["track"], e["path"]))
+    if unchecked:
+        note("")
+        note("note: %d file(s) could not be checked against hgdownload (no HTTP"
+             % len(unchecked))
+        note("      response); they are neither reported as reachable nor as blocked:")
+        for u in unchecked:
+            note("      %-6s %-16s %s (%s)" % (u["db"], u["track"], u["path"], u["code"]))
     return 0
 
 if __name__ == "__main__":
     sys.exit(main())