From 7bd95a9b52fc9c399d4f25265593d69c17cda879 Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Mon, 22 Jun 2026 21:26:07 -0600 Subject: [PATCH 1/3] Scope laboratory assay template lookups to the request container The assay_run_templates lookups in DefaultAssayParser.saveTemplate, DefaultAssayParser.getTemplateRowMap, and AssayHelper.saveTemplate filtered only by rowid, which is the table's global primary key. Because Table.update also operates by global PK, a user could pass a templateId belonging to another container and have that foreign template loaded and updated (runid/status), bypassing container authorization. Each lookup now adds a container condition scoped to the request container before reading or updating the row. Follow-up to #286. --- .../api/laboratory/assay/DefaultAssayParser.java | 10 +++++++--- .../src/org/labkey/laboratory/assay/AssayHelper.java | 11 +++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/laboratory/api-src/org/labkey/api/laboratory/assay/DefaultAssayParser.java b/laboratory/api-src/org/labkey/api/laboratory/assay/DefaultAssayParser.java index bbcd2ac4..5cd423a8 100644 --- a/laboratory/api-src/org/labkey/api/laboratory/assay/DefaultAssayParser.java +++ b/laboratory/api-src/org/labkey/api/laboratory/assay/DefaultAssayParser.java @@ -466,9 +466,11 @@ protected void saveTemplate(ViewContext ctx, int templateId, int runId) throws B { try { - //validate the template exists + //validate the template exists in this container TableInfo ti = DbSchema.get("laboratory").getTable("assay_run_templates"); - TableSelector ts = new TableSelector(ti, new SimpleFilter(FieldKey.fromString("rowid"), templateId), null); + SimpleFilter filter = new SimpleFilter(FieldKey.fromString("rowid"), templateId); + filter.addCondition(FieldKey.fromString("container"), _container); + TableSelector ts = new TableSelector(ti, filter, null); if (ts.getRowCount() == 0) { throw new BatchValidationException(Collections.singletonList(new ValidationException("Unknown template: " + templateId)), null); @@ -586,7 +588,9 @@ protected Map> getTemplateRowMap(ImportContext conte TableInfo ti = DbSchema.get("laboratory").getTable("assay_run_templates"); - TableSelector ts = new TableSelector(ti, new SimpleFilter(FieldKey.fromString("rowid"), templateId), null); + SimpleFilter filter = new SimpleFilter(FieldKey.fromString("rowid"), templateId); + filter.addCondition(FieldKey.fromString("container"), _container); + TableSelector ts = new TableSelector(ti, filter, null); Map[] maps = ts.getMapArray(); if (maps.length == 0) { diff --git a/laboratory/src/org/labkey/laboratory/assay/AssayHelper.java b/laboratory/src/org/labkey/laboratory/assay/AssayHelper.java index ee4a9816..beb3973c 100644 --- a/laboratory/src/org/labkey/laboratory/assay/AssayHelper.java +++ b/laboratory/src/org/labkey/laboratory/assay/AssayHelper.java @@ -33,6 +33,7 @@ import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; import org.labkey.api.data.RuntimeSQLException; +import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.TSVMapWriter; import org.labkey.api.data.Table; import org.labkey.api.data.TableInfo; @@ -51,6 +52,7 @@ import org.labkey.api.laboratory.assay.AssayDataProvider; import org.labkey.api.laboratory.assay.AssayImportMethod; import org.labkey.api.query.BatchValidationException; +import org.labkey.api.query.FieldKey; import org.labkey.api.query.ValidationException; import org.labkey.api.security.User; import org.labkey.api.util.FileUtil; @@ -139,6 +141,15 @@ public Map saveTemplate(User u, Container c, ExpProtocol protoco } else { + // Table.update operates by global PK, so verify the existing template lives in this container before updating it + SimpleFilter filter = new SimpleFilter(FieldKey.fromString("rowid"), templateId); + filter.addCondition(FieldKey.fromString("container"), c); + if (!new TableSelector(ti, filter, null).exists()) + { + errors.addRowError(new ValidationException("Unknown template: " + templateId)); + throw errors; + } + row.put("rowid", templateId); row = Table.update(u, ti, row, templateId); } From 8ac6b0ed238952acf2c48e966ce1d2412e89e155 Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Tue, 23 Jun 2026 12:34:24 -0600 Subject: [PATCH 2/3] Scope assay template lookups to the workbook parent as well The prior container-scoping fix (7bd95a9b) filtered the assay_run_templates lookups by strict equality on the request container, which breaks imports performed inside a workbook: the run-template picker sources templates from the parent container (mirroring Laboratory.Utils.getQueryContainerPath), so the selected templateId points at a parent-owned row that strict scoping rejects with "Unknown template". Widen the three lookups in DefaultAssayParser.saveTemplate, DefaultAssayParser.getTemplateRowMap, and AssayHelper.saveTemplate to an IN filter matching the request container or, when the request runs in a workbook, its parent. This still never reaches an unrelated container, so the original authorization boundary holds. Also stop writing the container into the AssayHelper.saveTemplate update row so an existing parent-owned template is not moved into the workbook by Table.update; the container is now set only on insert. --- .../laboratory/assay/DefaultAssayParser.java | 19 ++++++++++++++++--- .../labkey/laboratory/assay/AssayHelper.java | 15 +++++++++++---- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/laboratory/api-src/org/labkey/api/laboratory/assay/DefaultAssayParser.java b/laboratory/api-src/org/labkey/api/laboratory/assay/DefaultAssayParser.java index 5cd423a8..dc774a73 100644 --- a/laboratory/api-src/org/labkey/api/laboratory/assay/DefaultAssayParser.java +++ b/laboratory/api-src/org/labkey/api/laboratory/assay/DefaultAssayParser.java @@ -26,6 +26,7 @@ import org.json.JSONArray; import org.json.JSONObject; import org.labkey.api.collections.CaseInsensitiveHashMap; +import org.labkey.api.data.CompareType; import org.labkey.api.data.Container; import org.labkey.api.data.ConvertHelper; import org.labkey.api.data.DbSchema; @@ -466,10 +467,17 @@ protected void saveTemplate(ViewContext ctx, int templateId, int runId) throws B { try { - //validate the template exists in this container + // The template may live in this container, or its parent when importing from a workbook + // (the picker sources templates from getQueryContainerPath). Scope to that set, never beyond it. + List templateContainers = new ArrayList<>(); + templateContainers.add(_container.getId()); + if (_container.isWorkbook()) + templateContainers.add(_container.getParent().getId()); + + //validate the template exists in (or above) this container TableInfo ti = DbSchema.get("laboratory").getTable("assay_run_templates"); SimpleFilter filter = new SimpleFilter(FieldKey.fromString("rowid"), templateId); - filter.addCondition(FieldKey.fromString("container"), _container); + filter.addCondition(FieldKey.fromString("container"), templateContainers, CompareType.IN); TableSelector ts = new TableSelector(ti, filter, null); if (ts.getRowCount() == 0) { @@ -586,10 +594,15 @@ protected Map> getTemplateRowMap(ImportContext conte if (templateId == null) return ret; + List templateContainers = new ArrayList<>(); + templateContainers.add(_container.getId()); + if (_container.isWorkbook()) + templateContainers.add(_container.getParent().getId()); + TableInfo ti = DbSchema.get("laboratory").getTable("assay_run_templates"); SimpleFilter filter = new SimpleFilter(FieldKey.fromString("rowid"), templateId); - filter.addCondition(FieldKey.fromString("container"), _container); + filter.addCondition(FieldKey.fromString("container"), templateContainers, CompareType.IN); TableSelector ts = new TableSelector(ti, filter, null); Map[] maps = ts.getMapArray(); if (maps.length == 0) diff --git a/laboratory/src/org/labkey/laboratory/assay/AssayHelper.java b/laboratory/src/org/labkey/laboratory/assay/AssayHelper.java index beb3973c..5adcc6c2 100644 --- a/laboratory/src/org/labkey/laboratory/assay/AssayHelper.java +++ b/laboratory/src/org/labkey/laboratory/assay/AssayHelper.java @@ -30,6 +30,7 @@ import org.labkey.api.cache.CacheManager; import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.collections.CollectionUtils; +import org.labkey.api.data.CompareType; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; import org.labkey.api.data.RuntimeSQLException; @@ -133,17 +134,23 @@ public Map saveTemplate(User u, Container c, ExpProtocol protoco row.put("title", title); row.put("importMethod", importMethod); row.put("json", json.toString()); - row.put("container", c.getId()); if (templateId == null) { + row.put("container", c.getId()); row = Table.insert(u, ti, row); } else { - // Table.update operates by global PK, so verify the existing template lives in this container before updating it + // Table.update operates by global PK; verify the template is reachable from this container + // (here, or the parent when in a workbook) before updating, and leave its container unchanged. + List templateContainers = new ArrayList<>(); + templateContainers.add(c.getId()); + if (c.isWorkbook()) + templateContainers.add(c.getParent().getId()); + SimpleFilter filter = new SimpleFilter(FieldKey.fromString("rowid"), templateId); - filter.addCondition(FieldKey.fromString("container"), c); + filter.addCondition(FieldKey.fromString("container"), templateContainers, CompareType.IN); if (!new TableSelector(ti, filter, null).exists()) { errors.addRowError(new ValidationException("Unknown template: " + templateId)); @@ -151,7 +158,7 @@ public Map saveTemplate(User u, Container c, ExpProtocol protoco } row.put("rowid", templateId); - row = Table.update(u, ti, row, templateId); + row = Table.update(u, ti, row, templateId); // "container" intentionally absent -> not moved } return row; From 4a0ec2a88efe1d77c7afc5301ee966aaf79ffd67 Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Tue, 23 Jun 2026 12:45:40 -0600 Subject: [PATCH 3/3] Extract assay template container-scoping into a shared helper Replace the duplicated workbook-resolution snippet in DefaultAssayParser.saveTemplate, DefaultAssayParser.getTemplateRowMap, and AssayHelper.saveTemplate with a single DefaultAssayParser.getTemplateContainerIds(Container) helper, so the container-scoping rule for assay_run_templates lookups lives in one place. Pure refactor; no behavior change. --- .../laboratory/assay/DefaultAssayParser.java | 31 ++++++++++--------- .../labkey/laboratory/assay/AssayHelper.java | 8 ++--- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/laboratory/api-src/org/labkey/api/laboratory/assay/DefaultAssayParser.java b/laboratory/api-src/org/labkey/api/laboratory/assay/DefaultAssayParser.java index dc774a73..a1a33f32 100644 --- a/laboratory/api-src/org/labkey/api/laboratory/assay/DefaultAssayParser.java +++ b/laboratory/api-src/org/labkey/api/laboratory/assay/DefaultAssayParser.java @@ -463,21 +463,29 @@ protected void validateRows(List> rows, ImportContext contex errors.confirmNoErrors(); } + /** + * Returns the set of container ids in which an assay run template for a request in the given container may + * legitimately live: the container itself, plus its parent when the request runs in a workbook (mirroring the + * run-template picker, which sources templates from Laboratory.Utils.getQueryContainerPath). Used to scope + * assay_run_templates lookups to the request's folder tree without reaching an unrelated container. + */ + public static List getTemplateContainerIds(Container c) + { + List ids = new ArrayList<>(); + ids.add(c.getId()); + if (c.isWorkbook()) + ids.add(c.getParent().getId()); + return ids; + } + protected void saveTemplate(ViewContext ctx, int templateId, int runId) throws BatchValidationException { try { - // The template may live in this container, or its parent when importing from a workbook - // (the picker sources templates from getQueryContainerPath). Scope to that set, never beyond it. - List templateContainers = new ArrayList<>(); - templateContainers.add(_container.getId()); - if (_container.isWorkbook()) - templateContainers.add(_container.getParent().getId()); - //validate the template exists in (or above) this container TableInfo ti = DbSchema.get("laboratory").getTable("assay_run_templates"); SimpleFilter filter = new SimpleFilter(FieldKey.fromString("rowid"), templateId); - filter.addCondition(FieldKey.fromString("container"), templateContainers, CompareType.IN); + filter.addCondition(FieldKey.fromString("container"), getTemplateContainerIds(_container), CompareType.IN); TableSelector ts = new TableSelector(ti, filter, null); if (ts.getRowCount() == 0) { @@ -594,15 +602,10 @@ protected Map> getTemplateRowMap(ImportContext conte if (templateId == null) return ret; - List templateContainers = new ArrayList<>(); - templateContainers.add(_container.getId()); - if (_container.isWorkbook()) - templateContainers.add(_container.getParent().getId()); - TableInfo ti = DbSchema.get("laboratory").getTable("assay_run_templates"); SimpleFilter filter = new SimpleFilter(FieldKey.fromString("rowid"), templateId); - filter.addCondition(FieldKey.fromString("container"), templateContainers, CompareType.IN); + filter.addCondition(FieldKey.fromString("container"), getTemplateContainerIds(_container), CompareType.IN); TableSelector ts = new TableSelector(ti, filter, null); Map[] maps = ts.getMapArray(); if (maps.length == 0) diff --git a/laboratory/src/org/labkey/laboratory/assay/AssayHelper.java b/laboratory/src/org/labkey/laboratory/assay/AssayHelper.java index 5adcc6c2..71086ff1 100644 --- a/laboratory/src/org/labkey/laboratory/assay/AssayHelper.java +++ b/laboratory/src/org/labkey/laboratory/assay/AssayHelper.java @@ -52,6 +52,7 @@ import org.labkey.api.laboratory.LaboratoryService; import org.labkey.api.laboratory.assay.AssayDataProvider; import org.labkey.api.laboratory.assay.AssayImportMethod; +import org.labkey.api.laboratory.assay.DefaultAssayParser; import org.labkey.api.query.BatchValidationException; import org.labkey.api.query.FieldKey; import org.labkey.api.query.ValidationException; @@ -144,13 +145,8 @@ public Map saveTemplate(User u, Container c, ExpProtocol protoco { // Table.update operates by global PK; verify the template is reachable from this container // (here, or the parent when in a workbook) before updating, and leave its container unchanged. - List templateContainers = new ArrayList<>(); - templateContainers.add(c.getId()); - if (c.isWorkbook()) - templateContainers.add(c.getParent().getId()); - SimpleFilter filter = new SimpleFilter(FieldKey.fromString("rowid"), templateId); - filter.addCondition(FieldKey.fromString("container"), templateContainers, CompareType.IN); + filter.addCondition(FieldKey.fromString("container"), DefaultAssayParser.getTemplateContainerIds(c), CompareType.IN); if (!new TableSelector(ti, filter, null).exists()) { errors.addRowError(new ValidationException("Unknown template: " + templateId));