d338d5080783d2ac5828658fe17160668bf64cdb
chmalee
  Thu May 7 15:05:24 2026 -0700
Fixes from code review, refs #37500

- Revalidate shared myVariants tracks against hgcentral on every read
path (hgTracks, hgc, hgTables); cart-supplied owner/db/project no
longer trusted. New myVariantsResolveSharedTrack helper.
- Scope shared-track UPDATE statements by share->project/db so a
recipient can't edit rows outside the granted scope.
- Add hgsid CSRF check to myVariantsJsCommand; pass hgsid in the
hgTracks.js highlight Add-Annotation POST.
- HTML-escape owner-controlled fields in the canEdit branch of
doMyVariantsDetails (Chromosome, Project, project select options,
hidden text input).
- Validate targetUser against gbMembers when creating a share; return
a clear 400 on typos.
- Replace the concat(id,' ',name)='%s' lookup with parsed-id +
name verification.
- Remove cgiMakeColorVar / cgiMakeColorVarWithLabel; the canEdit form
uses spectrum.js (already loaded for the create dialog).
- Strip _hidden_* columns from hgTables field lists for shared tracks,
both the display path and the selected-fields read path.
- Make the per-assembly invariant explicit: myVariantsLoadItems and
doMyVariantsDetails bail out if share->db != current database.
- Memoize myVariantsSharedScopeWhere to avoid per-region hgcentral
round-trips on genome-wide hgTables queries.

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

diff --git src/hg/hgc/myVariantsClick.c src/hg/hgc/myVariantsClick.c
index 6e9dcc7c7d5..8ed8f6e5749 100644
--- src/hg/hgc/myVariantsClick.c
+++ src/hg/hgc/myVariantsClick.c
@@ -4,196 +4,239 @@
  * See kent/LICENSE or http://genome.ucsc.edu/license/ for licensing information. */
 
 #include "common.h"
 #include "hash.h"
 #include "linefile.h"
 #include "hgc.h"
 #include "myVariants.h"
 #include "myVariantsShare.h"
 #include "obscure.h"
 #include "cheapcgi.h"
 #include "hgMaf.h"
 #include "hui.h"
 #include "hCommon.h"
 #include "wikiLink.h"
 #include "jsHelper.h"
+#include "web.h"
 #include "hgConfig.h"
 #include "jsonWrite.h"
 #include "htmshell.h"
 
 void doMyVariantsDetails(struct customTrack *ct, char *itemIdString)
 /* Show details of a myVariants item. */
 {
 jsIncludeFile("hgc.js",NULL);
 char *idString = cloneString(itemIdString);
 char *trackName = ct->tdb->track;
 
-/* Detect shared track and resolve table/permissions */
+/* Detect shared track and resolve table/permissions via hgcentral so that
+ * revoked or downgraded shares no longer return owner data. */
 boolean isShared = startsWith("myVariants_shared_", trackName);
 char *dataOwner = NULL;     /* user whose table holds the data */
+char *scopeProject = NULL;  /* live share's project, or NULL for own track */
+char *scopeDb = NULL;       /* live share's db, or NULL for own track */
 int permission = MYVAR_PERM_READONLY;
 if (isShared)
     {
-    char *token = trackName + strlen("myVariants_shared_");
-    char cartVar[256];
-    safef(cartVar, sizeof(cartVar), MYVAR_SHARED_CART_PREFIX "%s", token);
-    char *cartVal = cartOptionalString(cart, cartVar);
-    if (isEmpty(cartVal))
+    struct myVariantsShare *share = myVariantsResolveSharedTrack(trackName, cart);
+    if (share == NULL)
         {
-        printf("Share information not found.\n");
+        printf("Share is no longer available.\n");
         return;
         }
-    char *project = NULL, *db = NULL;
-    if (!myVariantsParseShareCartValue(cartVal, &dataOwner, &project, &db, &permission, NULL))
+    /* Shared tracks are per-assembly; reject details requests from other dbs. */
+    if (!sameString(share->db, database))
         {
-        printf("Invalid share data.\n");
+        printf("This share is for a different assembly.\n");
+        myVariantsShareFree(&share);
         return;
         }
-    freeMem(project);
-    freeMem(db);
+    dataOwner = cloneString(share->ownerUser);
+    scopeProject = cloneString(share->project);
+    scopeDb = cloneString(share->db);
+    permission = share->permission;
+    myVariantsShareFree(&share);
     }
 else
     dataOwner = cloneString(getUserName());
 
 /* Anon users never edit shared items, even when the share is read-write. */
 boolean canEdit = !isShared || (permission == MYVAR_PERM_READWRITE && getUserName() != NULL);
 
 char *tableName = myVariantsGetDbTable(dataOwner);
+/* idString is "<id> <name>": parse id, query by id, verify name matches. */
+char *idStrCopy = cloneString(idString);
+char *expectedName = strchr(idStrCopy, ' ');
+if (expectedName == NULL)
+    {
+    printf("Invalid item identifier.\n");
+    freeMem(idStrCopy);
+    return;
+    }
+*expectedName++ = '\0';
+if (!isAllDigits(idStrCopy))
+    {
+    printf("Invalid item identifier.\n");
+    freeMem(idStrCopy);
+    return;
+    }
+unsigned itemId = sqlUnsigned(idStrCopy);
+
 struct sqlConnection *conn = hAllocConn(CUSTOM_TRASH);
-char query[512];
-sqlSafef(query, sizeof(query), "select * from %s where concat(id,' ',name)='%s'",
-    tableName, idString);
-struct sqlResult *sr = sqlGetResult(conn, query);
+struct dyString *query = sqlDyStringCreate(
+    "select * from %s where id=%u", tableName, itemId);
+if (isNotEmpty(scopeDb))
+    sqlDyStringPrintf(query, " and db='%s'", scopeDb);
+if (isNotEmpty(scopeProject) && !sameString(scopeProject, "*"))
+    sqlDyStringPrintf(query, " and project='%s'", scopeProject);
+struct sqlResult *sr = sqlGetResult(conn, query->string);
+dyStringFree(&query);
 
-char **row;
-if ((row = sqlNextRow(sr)) != NULL)
-    {
-    struct myVariants *item = myVariantsLoad(row);
-    sqlFreeResult(&sr);  /* Free early so conn is available for custom field queries */
+char **row = sqlNextRow(sr);
+struct myVariants *item = NULL;
+if (row != NULL && sameString(row[4], expectedName))
+    item = myVariantsLoad(row);
+sqlFreeResult(&sr);
+freeMem(idStrCopy);
 
+if (item != NULL)
+    {
     /* Show shared banner */
     if (isShared)
         {
         if (canEdit)
             printf("<div style='padding:6px; margin-bottom:8px; background:#e8f5e9; "
                 "border:1px solid #a5d6a7; border-radius:4px'>"
                 "<B>Shared from %s</B></div>\n", htmlEncode(dataOwner));
         else
             printf("<div style='padding:6px; margin-bottom:8px; background:#e3f2fd; "
                 "border:1px solid #90caf9; border-radius:4px'>"
                 "<B>Shared from %s (read-only)</B></div>\n", htmlEncode(dataOwner));
         }
 
     if (canEdit)
         {
+        webIncludeResourceFile("spectrum.min.css");
+        jsIncludeFile("spectrum.min.js", NULL);
         printf("<FORM ACTION=\"%s\" METHOD=\"POST\">\n\n", hgTracksName());
         cartSaveSession(cart);
 
         /* Save away ID string in hidden var. */
         char varName[128];
         char idStr[128];
         safef(varName, sizeof(varName), "%s_%s", trackName, "id");
         safef(idStr, sizeof(idStr), "%d", item->id);
         cgiMakeHiddenVar(varName, idStr);
 
         /* Put up editable label. */
         safef(varName, sizeof(varName), "%s_%s", trackName, "name");
         printf("<B>Label:</B> ");
         cgiMakeTextVar(varName, item->name, 17);
         printInfoIcon("A short label for this annotation, displayed in the browser.");
         printf("<BR>\n");
 
         /* Put up editable description. */
         safef(varName, sizeof(varName), "%s_%s", trackName, "description");
         printf("<B>Description:</B> ");
         printInfoIcon("Longer notes or comments about this annotation. Displayed on this details page.");
         printf("<BR>\n");
         cgiMakeTextArea(varName, item->description, 8, 80);
         printf("<BR>\n");
 
         /* Non-editable chromosome. */
-        printf("<B>Chromosome:</B> %s<BR>\n", item->chrom);
+        htmlPrintf("<B>Chromosome:</B> %s<BR>\n", item->chrom);
 
         /* Editable start and end. */
         int chromSize = hChromSize(database, item->chrom);
         char chromSizeString[16];
         safef(chromSizeString, sizeof(chromSizeString), "%d", chromSize);
         printf("<B>Start:</B> ");
         safef(varName, sizeof(varName), "%s_%s", trackName, "chromStart");
         cgiMakeIntVarInRange(varName, item->chromStart+1, NULL, 80, "1", chromSizeString);
         printInfoIcon("1-based start position on the chromosome.");
         printf("<BR>\n");
         printf("<B>End:</B> ");
         safef(varName, sizeof(varName), "%s_%s", trackName, "chromEnd");
         cgiMakeIntVarInRange(varName, item->chromEnd, NULL, 80, "1", chromSizeString);
         printInfoIcon("1-based end position on the chromosome (inclusive).");
         printf("<BR>\n");
 
         /* Edit the color */
         safef(varName, sizeof(varName), "%s_%s", trackName, "itemRgb");
         char colorHex[8];
         safef(colorHex, sizeof(colorHex), "#%06X", item->itemRgb);
-        cgiMakeColorVarWithLabel(varName, "Color", colorHex, TRUE);
+        hPrintf("<label for=\"%s\"><b>Color:</b></label> ", varName);
+        hPrintf("<input type=\"text\" name=\"%s\" id=\"%s\" value=\"%s\">\n",
+            varName, varName, colorHex);
+        jsInlineF(
+            "$(function() {"
+                "$(document.getElementById('%s')).spectrum({"
+                    "preferredFormat: 'hex',"
+                    "showInput: true,"
+                    "showPalette: true,"
+                    "hideAfterPaletteSelect: true"
+                "});"
+            "});\n",
+            varName);
         printf("<br>");
 
         /* Edit ref/alt */
         safef(varName, sizeof(varName), "%s_%s", trackName, "ref");
         printf("<B>Ref:</B> ");
         cgiMakeTextVar(varName, item->ref, 17);
         printInfoIcon("Reference allele sequence at this position.");
         printf("<BR>\n");
         safef(varName, sizeof(varName), "%s_%s", trackName, "alt");
         printf("<B>Alt:</B> ");
         cgiMakeTextVar(varName, item->alt, 17);
         printInfoIcon("Alternate (variant) allele sequence.");
         printf("<BR>\n");
 
         /* Project: locked for shared tracks, editable for own track */
         printf("<B>Project:</B> ");
         if (isShared)
             {
-            printf("%s", isNotEmpty(item->project) ? item->project : "(none)");
-            printf("<BR>\n");
+            htmlPrintf("%s<BR>\n", isNotEmpty(item->project) ? item->project : "(none)");
             }
         else
             {
             safef(varName, sizeof(varName), "%s_%s", trackName, "project");
             struct slName *projects = myVariantsGetProjects(dataOwner);
             if (projects)
                 {
                 char selectName[128];
                 safef(selectName, sizeof(selectName), "%s_projectSelect", trackName);
-                printf("<select id='%s'>", selectName);
-                printf("<option value=''>%s</option>", "(none)");
+                htmlPrintf("<select id='%s|attr|'>", selectName);
+                printf("<option value=''>(none)</option>");
                 struct slName *proj;
                 boolean currentFound = FALSE;
                 for (proj = projects; proj != NULL; proj = proj->next)
                     {
-                    char *selected = "";
-                    if (sameString(proj->name, item->project))
-                        {
-                        selected = " selected";
+                    boolean isCurrent = sameString(proj->name, item->project);
+                    if (isCurrent)
                         currentFound = TRUE;
-                        }
-                    printf("<option value='%s'%s>%s</option>", proj->name, selected, proj->name);
+                    htmlPrintf("<option value='%s|attr|'%s|none|>%s</option>",
+                        proj->name, isCurrent ? " selected" : "", proj->name);
                     }
                 if (!currentFound && isNotEmpty(item->project))
-                    printf("<option value='%s' selected>%s</option>", item->project, item->project);
+                    htmlPrintf("<option value='%s|attr|' selected>%s</option>",
+                        item->project, item->project);
                 printf("<option value='__new__'>Add new...</option>");
                 printf("</select> ");
-                printf("<input type='text' name='%s' id='%s' value='%s' style='display:none'"
-                    " placeholder='Enter new project'>", varName, varName, item->project);
+                htmlPrintf("<input type='text' name='%s|attr|' id='%s|attr|' value='%s|attr|'"
+                    " style='display:none' placeholder='Enter new project'>",
+                    varName, varName, item->project);
                 slFreeList(&projects);
                 }
             else
                 cgiMakeTextVar(varName, item->project, 40);
             printInfoIcon("Group annotations by project.");
             printf("<BR>\n");
             }
 
         /* Mouseover */
         safef(varName, sizeof(varName), "%s_%s", trackName, "mouseover");
         printf("<B>Mouseover:</B> ");
         cgiMakeTextVar(varName, item->mouseover, 60);
         printInfoIcon("Short text shown when hovering over this item.");
         printf("<BR>\n");
 
@@ -317,22 +360,20 @@
         jsonWriteListStart(jw, NULL);
         struct slName *trackNames = slNameListFromComma(overlapList);
         struct slName *t;
         for (t = trackNames; t != NULL; t = t->next)
             jsonWriteString(jw, NULL, t->name);
         jsonWriteListEnd(jw);
         jsInline("var doItemOverlaps = true;\n");
         jsInlineF("var overlapTracks = %s;\n", jw->dy->string);
         printf("<div id='itemOverlaps' style=\"display:none\"></div>\n");
         slFreeList(&trackNames);
         jsonWriteFree(&jw);
         }
 
     printPosOnChrom(item->chrom, item->chromStart, item->chromEnd, NULL, TRUE, NULL);
     }
-else
-    sqlFreeResult(&sr);
 
 freeMem(dataOwner);
 hFreeConn(&conn);
 }