554396e44745165ee4baf5214aa771f2b17b3be6
braney
  Mon Aug 17 16:04:26 2026 -0700
hgPcr, cart: screen the PCR result file names read back out of the cart, refs #37623

The hgPcrResult_<db> cart variable holds two file names and an optional target
name in one value.  The cart.c arrays compare a whole value against
isServerUserFilePath(), so none of them fit that shape.  Add a fourth array for
it and check the first two words.  hgPcrResult_targetStyle shares the prefix and
is a display setting, so it is excluded by name.

Check both names where they are used as well, in pcrResultParseCart() and in
hgPcr's append path, the way dupTrack.c already does.

Two other things in writePcrResultTrack().  pcrFiles[2] was read without ever
being set whenever the value held only two words, which is the usual case.  And
the saved-session test was a plain prefix compare that missed sessionDataDirOld;
it now asks whether the file is in the trash instead.

hg/utils/cartFileVarCatalog knows about the new array and has a row for
hgPcrResult_<db> saying why its scan cannot see this one.

diff --git src/hg/utils/cartFileVarCatalog/cartFileVarCatalog.py src/hg/utils/cartFileVarCatalog/cartFileVarCatalog.py
index ad79f0bf487..a318ccf2f00 100755
--- src/hg/utils/cartFileVarCatalog/cartFileVarCatalog.py
+++ src/hg/utils/cartFileVarCatalog/cartFileVarCatalog.py
@@ -1,25 +1,27 @@
 #!/usr/bin/env python3
 """cartFileVarCatalog.py - the registry of cart variables that hold a file name.
 
 Refs #37623.  Most cart variables hold a setting.  A few hold the name of a
 file the server made for the user, and a CGI reads one back out of the cart and
 opens it.  Those are the ones an attacker can retarget, so hg/lib/cart.c screens
-them on the way in, against two hand-written arrays:
+them on the way in, against hand-written arrays:
 
     fileNameCartVars[]              exact names
     fileNameCartVarPrefixes[]       families whose name carries the db or an id
+    urlOrFileNameCartVars[]         may hold a remote URL instead
+    fileNamePairCartVarPrefixes[]   value is two file names and a trailing word
 
 A hand-written list goes stale the moment somebody adds a variable and does not
 know the list exists.  It was already incomplete on the day it was written.  So
 this catalog says, for every cart variable the tree opens as a file, whether
 cart.c screens it and why, and --reconcile checks that claim against both the
 tree and cart.c itself.
 
 Usage:
     cartFileVarCatalog.py --list        # the catalog, grouped by verdict
     cartFileVarCatalog.py --check       # sanity checks on this file alone
     cartFileVarCatalog.py --reconcile   # diff the catalog against the tree
     cartFileVarCatalog.py --reconcile --verbose
     cartFileVarCatalog.py --json out.json
     cartFileVarCatalog.py --html out.html
 
@@ -175,30 +177,49 @@
            "next to blatPslFile, not by the scan."),
 
     e("near.customFile", "screened", "hg/near/hgNear/customColumn.c",
       "fileExists", screen="near.customFile",
       note="hgNear custom column file, normally makeTempName(near, .col).  "
            "hgNear is in hg/makefile, so this one is built and reachable."),
 
     e("gsTemp", "screened", "hg/hgTables/genomeSpace.c",
       "fileSize, md5ForFile, gsS3Upload", screen="gsTemp",
       note="The file hgTables uploads to GenomeSpace.  A retargeted value is "
            "read and sent to a remote service, not only read locally, so the "
            "point-of-use check errAborts rather than returning quietly.  Gated "
            "as well: genomeSpace is enabled only by the presence of its hg.conf "
            "settings (genomeSpace.c:110) and no conf in confs/ sets them."),
 
+    # ---- two file names in one value -----------------------------------------
+    # Screened against fileNamePairCartVarPrefixes[], which checks the first two
+    # words rather than the whole value.
+
+    e("hgPcrResult_<db>", "screened", "hg/cgilib/pcrResult.c",
+      "fileExists, lineFileOpen", ident="{cartVar}", screen="pair:hgPcrResult_",
+      note="In-silico PCR results.  The value is two trash file names and an "
+           "optional targetDb name, not one file name, which is why it needs an "
+           "array of its own.  Found by the daily code review of the commit that "
+           "wrote this catalog, not by the scan: pcrResultParseCart() chopLine()s "
+           "the value into different locals before opening them, so no flow is "
+           "harvested, and a name built at run time reads as {cartVar}, which the "
+           "--suspects name test cannot recognize either.  hgPcr appends to both "
+           "files when the user checks 'Append to existing PCR result', so an "
+           "unscreened value was an arbitrary file write as well as a read.  Both "
+           "names are checked at the point of use too, in pcrResultParseCart() "
+           "and in hgPcr's pcrResultCartFiles().  hgPcrResult_targetStyle shares "
+           "the prefix and is a display setting, so cart.c excludes it by name."),
+
     # ---- either a URL or a file we made --------------------------------------
     # Screened against urlOrFileNameCartVars[] with isServerUserFileOrUrl(),
     # because isServerUserFilePath() alone would reject every legitimate URL.
 
     e("multiRegionsBedUrl", "screened", "hg/hgTracks/config.c",
       "fileExists, lineFileMayOpen", screen="urlOrFile:multiRegionsBedUrl",
       note="Multi-region custom BED: either a URL the user gave, or the trash "
            "file hgTracks wrote the pasted BED to (hgTracks.c:4357).  The code "
            "picks the branch by looking for '://', so a value with no protocol "
            "falls through to opening a local file.  config.c prints what it "
            "reads back into the multi-region textarea, so an unscreened value "
            "was a file read the user could see.  Both local-file branches now "
            "check the path as well."),
 
     e("hgS_loadUrlName", "screened", "hg/lib/cart.c", "netLineFileOpen",
@@ -259,42 +280,44 @@
     """
     out = {}
     for entry in cat:
         for key in (entry["ident"], entry["name"]):
             if key and not ("<" in key and ">" in key):
                 out.setdefault(key, []).append(entry)
         if entry["ident"]:
             out.setdefault(entry["ident"], [])
             if entry not in out[entry["ident"]]:
                 out[entry["ident"]].append(entry)
     return out
 
 
 def screen_entries(cat):
     """The cart.c entries the catalog claims, one set per array."""
-    names, prefixes, urlOrFile = set(), set(), set()
+    names, prefixes, urlOrFile, pairPrefixes = set(), set(), set(), set()
     for entry in cat:
         s = entry["screen"]
         if not s:
             continue
         if s.startswith("prefix:"):
             prefixes.add(s[len("prefix:"):])
         elif s.startswith("urlOrFile:"):
             urlOrFile.add(s[len("urlOrFile:"):])
+        elif s.startswith("pair:"):
+            pairPrefixes.add(s[len("pair:"):])
         else:
             names.add(s)
-    return names, prefixes, urlOrFile
+    return names, prefixes, urlOrFile, pairPrefixes
 
 
 # ---------------------------------------------------------------------------
 # --check
 # ---------------------------------------------------------------------------
 
 def check(cat, out=sys.stderr):
     """Sanity checks on this file alone.  Returns the number of problems."""
     problems = 0
     seen = set()
     for entry in cat:
         name = entry["name"]
         if name in seen:
             print("duplicate entry: %s" % name, file=out)
             problems += 1
@@ -332,42 +355,44 @@
 
     Silent and 0 when nothing has changed.  See the module docstring for the
     four things that make it fail.
     """
     h = harvest.harvest()
     flows = h["flows"]
 
     if len(flows) < MIN_TREE_FLOWS:
         print("only %d cart-to-file flows found: expected at least %d, so the "
               "scan is\nbroken rather than the tree being clean.  Check "
               "KENT_SRC." % (len(flows), MIN_TREE_FLOWS), file=out)
         return 1
 
     missing = [name for name, key in zip(harvest.SCREEN_ARRAYS,
                                          ("screenNames", "screenPrefixes",
-                                          "screenUrlOrFile"))
+                                          "screenUrlOrFile",
+                                          "screenPairPrefixes"))
                if h[key] is None]
     if missing:
         print("no %s array found in %s: either it was removed or it was "
               "reformatted\npast what read_screen() can parse.  Either way "
               "nothing in it is being screened."
               % (", ".join(missing), harvest.CART_C), file=out)
         return 1
 
     tree_names = set(h["screenNames"])
     tree_prefixes = set(h["screenPrefixes"])
     tree_urlOrFile = set(h["screenUrlOrFile"])
+    tree_pairPrefixes = set(h["screenPairPrefixes"])
     keys = by_harvest_key(cat)
     problems = 0
 
     # 1. the tree opens something this catalog has never heard of
     found = sorted({f["name"] for f in flows})
     site = {}
     for f in flows:
         site.setdefault(f["name"], "%s:%d" % (f["file"], f["line"]))
     unknown = [n for n in found if n not in keys]
     if unknown:
         problems += len(unknown)
         print("\nopened as a file by the tree, not in the catalog (%d):"
               % len(unknown), file=out)
         print("    (add a row to cartFileVarCatalog.py saying what the "
               "variable is,\n     and if it holds a server file name add it to "
@@ -375,79 +400,87 @@
               % (harvest.SCREEN_ARRAYS[0], harvest.CART_C), file=out)
         for n in unknown:
             print("    %-32s %s" % (n, site.get(n, "")), file=out)
 
     # 2 and 3. every claim about cart.c, checked against cart.c
     lost, fixed = [], []
     for entry in cat:
         name = entry["screen"]
         if entry["verdict"] == "screened":
             if name.startswith("prefix:"):
                 if name[len("prefix:"):] not in tree_prefixes:
                     lost.append(entry)
             elif name.startswith("urlOrFile:"):
                 if name[len("urlOrFile:"):] not in tree_urlOrFile:
                     lost.append(entry)
+            elif name.startswith("pair:"):
+                if name[len("pair:"):] not in tree_pairPrefixes:
+                    lost.append(entry)
             elif name not in tree_names:
                 lost.append(entry)
         elif entry["verdict"] in ("gap", "otherCheck"):
             probe = entry["name"]
             if "<" not in probe and harvest.screened(probe, tree_names,
                                                      tree_prefixes,
-                                                     tree_urlOrFile):
+                                                     tree_urlOrFile,
+                                                     tree_pairPrefixes):
                 fixed.append(entry)
 
     if lost:
         problems += len(lost)
         print("\nthe catalog says cart.c screens these and it does not (%d):"
               % len(lost), file=out)
         for entry in lost:
             print("    %-32s wanted %s" % (entry["name"], entry["screen"]),
                   file=out)
 
     if fixed:
         problems += len(fixed)
         print("\ncart.c now screens these and the catalog still calls them a "
               "gap (%d):" % len(fixed), file=out)
         print("    (change the verdict to screened and set screen=)", file=out)
         for entry in fixed:
             print("    %s" % entry["name"], file=out)
 
     # 4. cart.c screens something the catalog does not describe at all.  A
     # variable the catalog knows as a gap is not an orphan: adding it to cart.c
     # is the fix, and check 3 above already asks for the verdict to be updated.
-    cat_names, cat_prefixes, cat_urlOrFile = screen_entries(cat)
+    cat_names, cat_prefixes, cat_urlOrFile, cat_pairPrefixes = \
+        screen_entries(cat)
     known = {entry["name"] for entry in cat}
     orphan = sorted(
         {n for n in tree_names - cat_names if n not in known}
         | {n for n in tree_urlOrFile - cat_urlOrFile if n not in known}
         | {"prefix:" + p for p in tree_prefixes - cat_prefixes
+           if not any(k.startswith(p) for k in known)}
+        | {"pair:" + p for p in tree_pairPrefixes - cat_pairPrefixes
            if not any(k.startswith(p) for k in known)})
     if orphan:
         problems += len(orphan)
         print("\nscreened by %s, not described in the catalog (%d):"
               % (harvest.CART_C, len(orphan)), file=out)
         for n in orphan:
             print("    %s" % n, file=out)
 
     if verbose:
         print("\ncatalog entries    %d" % len(cat), file=out)
         print("tree flows         %d over %d names" % (len(flows), len(found)),
               file=out)
-        print("cart.c screens     %d names, %d prefixes, %d url-or-file"
-              % (len(tree_names), len(tree_prefixes), len(tree_urlOrFile)),
-              file=out)
+        print("cart.c screens     %d names, %d prefixes, %d url-or-file, "
+              "%d pair prefixes"
+              % (len(tree_names), len(tree_prefixes), len(tree_urlOrFile),
+                 len(tree_pairPrefixes)), file=out)
         quiet = [entry["name"] for entry in cat
                  if entry["name"] not in found
                  and (entry["ident"] or entry["name"]) not in found]
         print("\nin the catalog, no flow found in the tree (%d)" % len(quiet),
               file=out)
         print("    (expected: the scan is intraprocedural, so a value handed "
               "to a\n     helper that opens it does not show up)", file=out)
         for n in quiet:
             print("    %s" % n, file=out)
         gaps = [entry["name"] for entry in cat if entry["verdict"] == "gap"]
         print("\nknown gaps, unscreened by cart.c (%d)" % len(gaps), file=out)
         for n in gaps:
             print("    %s" % n, file=out)
 
     return problems