32515a6797b6e68daa3aa94c663760216b24dd67
lrnassar
  Thu Sep 3 13:30:26 2026 -0700
Usage stats cron fixes from code review. refs #38232

Apply the same db mismatch guard to assemblyStatsCron.py that generateUsageStats.py
already had. Both files ask hgTracks for one assembly's default tracks and parse the
answer off stderr, but only one of them checked that hgTracks answered about the
assembly it was asked about. Without it, a repeat of the db= bug this ticket fixed
would quietly fill the default track filter with another assembly's tracks and say
nothing.

Drop any database name that does not look like one before interpolating it into a
shell command. The trimmed logs only ever carry real assembly names, so this is
defence in depth rather than a live hole.

Fix a comment in resolveHub that still described the lastOkTime fallback, which was
replaced by mirror order earlier in this ticket.

No change to the report. The row counts for the database usage and non-public hub
tables were cut from 15 to 10 in the previous commit, which its message did not
mention; that was intentional, to make room for the new GenArk and hubSpace sections.

diff --git src/utils/qa/assemblyStatsCron.py src/utils/qa/assemblyStatsCron.py
index f42c94eafb1..2a6dc42d1da 100755
--- src/utils/qa/assemblyStatsCron.py
+++ src/utils/qa/assemblyStatsCron.py
@@ -1,21 +1,22 @@
 #07/20/19
 #This was adapted from a jupyter notebook - hence lots of weird bash calls
 
 import datetime
 from collections import OrderedDict
 import getpass
+import re
 import subprocess
 import os
 import urllib.parse
 
 def bash(cmd):
     """Run the cmd in bash subprocess"""
     try:
         rawBashOutput = subprocess.run(cmd, check=True, shell=True,\
                                        stdout=subprocess.PIPE, universal_newlines=True,
                                        encoding="utf-8", errors="replace", stderr=subprocess.STDOUT)
         bashStdoutt = rawBashOutput.stdout
     except subprocess.CalledProcessError as e:
         raise RuntimeError("command '{}' return with error (code {}): {}".format(e.cmd, e.returncode, e.output))
     return(bashStdoutt)
 
@@ -214,31 +215,31 @@
     try:
         return datetime.datetime.strptime(lastOkTime.split(" ")[0], '%Y-%m-%d') > lastMonth
     except ValueError:
         return False
 
 ambiguousHubIds = []
 
 def resolveHub(hubId):
     """Return [hubUrl, shortLabel, machine] for a hub id, or None if no mirror can vouch for it"""
     candidates = [(machine, fields) for machine, fields in hubStatusByMirror.get(hubId, {}).items()
                   if hubIsCurrent(fields[2])]
     if not candidates:
         return None
     if len(candidates) > 1:
         # Same id, different hubs. The track name we saw in the logs usually matches the right
-        # hub's shortLabel, so trust that before falling back to whoever refreshed most recently.
+        # hub's shortLabel, so trust that first.
         observedTrack = stripHubPrefix(bestTrackForHub[hubId][2]).lower() if hubId in bestTrackForHub else ""
         #Needs enough label to be evidence - a two letter shortLabel matches almost anything
         matching = [c for c in candidates if len(c[1][1]) >= 8 and
                     (c[1][1].lower() in observedTrack or observedTrack.startswith(c[1][1].lower()[:20]))]
         if len(matching) == 1:
             candidates = matching
         #Otherwise fall through on mirror order, RR first, which is how this has always
         #resolved and matches a report written from the RR's point of view
         if len({normalizeHubUrl(c[1][0]) for c in candidates}) > 1:
             ambiguousHubIds.append(hubId)
     machine, fields = candidates[0]
     return [fields[0], fields[1], machine]
 
 resolvedHubs = {} #hubId -> [hubUrl, shortLabel, machine]
 for hubId in hubIdsSeen:
@@ -301,54 +302,66 @@
 with open(outputDir+'/dbCounts.tsv') as dbCountsForGenArk:
     for line in dbCountsForGenArk:
         dbName = line.split("\t")[0]
         hubId = hubIdFromName(dbName)
         if hubId is not None and hubId in resolvedHubs and hubCategory(resolvedHubs[hubId][0]) == 'genark':
             genArkAccessions.add(stripHubPrefix(dbName))
 
 #The following section pulls out a list of default track for the top X assemblies for filtering
 defaultsFile = open(outputDir+"/defaults.txt", "w")
 
 #The head command can be expanded to be more inclusive if additional assembly defaults are finding their way onto the list
 bash("sort "+outputDir+'/dbCounts.tsv -rnk2 > '+outputDir+'/dbCountsTopSorted.tsv')
 topDbs = bash('head -n 10 '+outputDir+'/dbCountsTopSorted.tsv | cut -f1 -d "\t"').rstrip().split("\n")
 #Hub backed databases are skipped - hub ids differ between hgcentrals, so the track names
 #hgTracks hands back here would never match the ones seen in the logs.
-topDbs = [db for db in topDbs if not db.startswith("hub_")][0:4]
+#Assembly names are interpolated into a shell command below, so anything that does not look
+#like one is dropped rather than passed to sh
+topDbs = [db for db in topDbs
+          if not db.startswith("hub_") and re.match(r'^[A-Za-z0-9_.-]+$', db)][0:4]
 
 defaultTracks = set()
 for db in topDbs:
     #The following part queries hgTracks for each of the assemblies and extracts the list of defaults
     bash('echo '+db+' > '+outputDir+'/temp.txt')
     #Must run from the cgi-bin directory or hgTracks cannot find ../htdocs/urw-fonts and dies
     #before it ever logs the track list. The space before db= matters just as much - without it
     #cgiSpoof swallows the whole string and every assembly silently returns hg38's defaults.
     defaults = bashNoErrorCatch('cd /usr/local/apache/cgi-bin && HGDB_CONF=$HOME/.hg.conf.beta '
                                 './hgTracks hgt.trackImgOnly=1 db='+db+' > /dev/null')
 
     #hgTracks writes "trackLog N db hgsid track:vis,track:vis" to stderr, split into ~800 byte
     #blocks so Apache does not chop the lines. Take every numbered block; the trailing
     #"trackLog position" line and the CGI_TIME/RESOURCE lines are not track lists.
     tracksForDb = []
+    mismatch = None
     for defaultsLine in defaults:
         splitLine = defaultsLine.split(" ")
         if len(splitLine) > 4 and splitLine[0] == "trackLog" and splitLine[1].isdigit():
+            #Check hgTracks answered about the assembly we asked for. Silently accepting
+            #another assembly's list is exactly how the defaults went wrong for two years.
+            if splitLine[2] != db:
+                mismatch = splitLine[2]
+                tracksForDb = []
+                break
             tracksForDb.extend(splitLine[4].split(","))
 
     if not tracksForDb:
-        reportWarnings.append("WARNING: hgTracks returned no default track list for "+db+
-                              ", its tracks are not filtered out of the non-default list below.")
+        reason = "it reported "+mismatch+" instead" if mismatch else "it returned no track list"
+        reportWarnings.append("WARNING: asked hgTracks for the default tracks of "+db+" and "+
+                              reason+", so its tracks are not filtered out of the non-default "
+                              "list below.")
         continue
 
     for track in tracksForDb:
         track = track.rsplit(":", 1)[0] #drop the trailing visibility
         if track:
             defaultsFile.write(db+"\t"+track+"\n")
             defaultTracks.add((db, track))
     defaultsFile.write(db+"\t"+"cytoBand"+"\n")
     defaultTracks.add((db, "cytoBand"))
 defaultsFile.close()
 
 #############################################
 ##### Build the report, section by section ##
 #############################################