diff --git a/api/src/org/labkey/api/assay/AbstractAssayProvider.java b/api/src/org/labkey/api/assay/AbstractAssayProvider.java index b99bf13b203..e244c6aff74 100644 --- a/api/src/org/labkey/api/assay/AbstractAssayProvider.java +++ b/api/src/org/labkey/api/assay/AbstractAssayProvider.java @@ -102,6 +102,7 @@ import org.labkey.api.security.User; import org.labkey.api.security.permissions.AdminPermission; import org.labkey.api.settings.AppProps; +import org.labkey.api.settings.OptionalFeatureService; import org.labkey.api.study.Dataset; import org.labkey.api.study.TimepointType; import org.labkey.api.study.assay.ParticipantVisitResolverType; @@ -112,6 +113,7 @@ import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.Pair; import org.labkey.api.util.StringUtilsLabKey; +import org.labkey.api.util.URIUtil; import org.labkey.api.util.logging.LogHelper; import org.labkey.api.view.ActionURL; import org.labkey.api.view.DetailsView; @@ -1271,6 +1273,8 @@ public Pair> setValidationAndAnalysisS String ext = FileUtil.getExtension(scriptFile); if (scriptFile.isFile() && ext != null) { + validateScriptLocation(protocol.getContainer(), scriptFile, validationErrors); + ScriptEngine engine = LabKeyScriptEngineManager.get().getEngineByExtension(protocol.getContainer(), ext, LabKeyScriptEngineManager.EngineContext.pipeline); if (engine != null) { @@ -1326,6 +1330,51 @@ public Pair> setValidationAndAnalysisS /** Property name suffix used when storing transform scripts on assay protocol objects */ public static final String TRANSFORM_SCRIPT_PROPERTY_NAME = "TransformScript"; + /** + * Deprecated feature flag. When enabled, transform scripts may live at arbitrary file system paths instead of + * being confined to the assay design container's @scripts directory. Registered by AssayModule. + */ + public static final String DEPRECATED_ARBITRARY_TRANSFORM_SCRIPT_PATHS = "arbitraryTransformScriptPaths"; + + /** + * GitHub Issue #159: transform scripts configured on an assay design must reside in that design's container's + * {@code @scripts} directory, which only platform developers can write to. Module-provided scripts are supplied + * via {@link Scope#ASSAY_TYPE} and never reach this code path, so every script persisted with a design must be + * under {@code @scripts}. + */ + private void validateScriptLocation(Container container, FileLike scriptFile, ValidationException validationErrors) + { + if (OptionalFeatureService.get().isFeatureEnabled(DEPRECATED_ARBITRARY_TRANSFORM_SCRIPT_PATHS)) + return; + + FileContentService fileContentService = FileContentService.get(); + if (fileContentService == null) + { + LOG.warn("FileContentService is not available; unable to validate the location of transform script '{}'.", scriptFile.toNioPathForRead()); + return; + } + + // Returns null when there's no usable file root and for cloud roots, which have no @scripts directory at all + // (see ScriptsResourceProvider). + java.nio.file.Path scriptsDir = fileContentService.getFileRootPath(container, FileContentService.ContentType.scripts); + if (scriptsDir == null) + { + LOG.warn("Skipping the {} location check for transform script '{}': unable to resolve the directory for container '{}'.", + FileContentService.SCRIPTS_LINK, scriptFile.toNioPathForRead(), container.getPath()); + return; + } + + // Normalize both sides (strip "." and "..", correct case on case-insensitive file systems) before comparing + File scriptsDirFile = FileUtil.getAbsoluteCaseSensitiveFile(scriptsDir.toFile()); + File normalizedScriptFile = FileUtil.getAbsoluteCaseSensitiveFile(scriptFile.toNioPathForRead().toFile()); + + if (!URIUtil.isDescendant(scriptsDirFile.toURI(), normalizedScriptFile.toURI())) + { + validationErrors.addError(new SimpleValidationError("The transform script must exist within this folder's " + + FileContentService.SCRIPTS_LINK + " directory.", null, SEVERITY.ERROR, new HelpTopic("programmaticQC"))); + } + } + @NotNull @Override public List getValidationAndAnalysisScripts(ExpProtocol protocol, Scope scope) diff --git a/assay/src/org/labkey/assay/AssayModule.java b/assay/src/org/labkey/assay/AssayModule.java index 7bd5b425b9f..64adeea58f8 100644 --- a/assay/src/org/labkey/assay/AssayModule.java +++ b/assay/src/org/labkey/assay/AssayModule.java @@ -19,6 +19,7 @@ import jakarta.servlet.ServletContext; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.labkey.api.assay.AbstractAssayProvider; import org.labkey.api.assay.AbstractTsvAssayProvider; import org.labkey.api.assay.AssayBatchDomainKind; import org.labkey.api.assay.AssayProvider; @@ -68,6 +69,8 @@ import org.labkey.api.security.User; import org.labkey.api.security.permissions.ReadPermission; import org.labkey.api.security.roles.RoleManager; +import org.labkey.api.settings.OptionalFeatureFlag; +import org.labkey.api.settings.OptionalFeatureService; import org.labkey.api.usageMetrics.UsageMetricsService; import org.labkey.api.util.ContextListener; import org.labkey.api.util.GUID; @@ -190,6 +193,14 @@ protected void init() PropertyService.get().registerDomainKind(new PlateMetadataDomainKind()); CacheManager.addListener(PlateCache::clearCache); + + OptionalFeatureService.get().addFeatureFlag(new OptionalFeatureFlag( + AbstractAssayProvider.DEPRECATED_ARBITRARY_TRANSFORM_SCRIPT_PATHS, + "Allow assay transform scripts at arbitrary file system paths", + "Transform scripts configured on an assay design are normally required to reside in that design's container's @scripts directory, which only platform developers can write to. Enable this to allow assay designs to reference scripts anywhere on the server's file system. This option will be removed in a future release of LabKey Server.", + false, + false, + OptionalFeatureService.FeatureType.Deprecated)); } @Override diff --git a/experiment/src/org/labkey/experiment/ExperimentModule.java b/experiment/src/org/labkey/experiment/ExperimentModule.java index 0226497abab..a8e1234c39a 100644 --- a/experiment/src/org/labkey/experiment/ExperimentModule.java +++ b/experiment/src/org/labkey/experiment/ExperimentModule.java @@ -666,6 +666,18 @@ public void containerDeleted(Container c, User user) "%\"" + DataTransformService.TransformOperation.INSERT + "\"%" ).getObject(Long.class)); + // GitHub Issue #159: approximate count of assay designs whose transform scripts aren't in an + // @scripts directory. This is done in SQL instead of parsing each script so there are some caveats + // for cases that aren't counted: + // 1. Any path containing "@scripts" matches, so a script in a sibling container's @scripts looks compliant here but is rejected on save. + // 2. Undercounts designs with a MIX of compliant and non-compliant scripts + assayMetrics.put("protocolsWithTransformScriptNotInScriptsDirCount", new SqlSelector(schema, + "SELECT COUNT(*) FROM exp.protocol EP JOIN exp.objectPropertiesView OP ON EP.lsid = OP.objecturi WHERE OP.name = ? AND status = ? AND OP.stringvalue NOT LIKE ?", + AbstractAssayProvider.TRANSFORM_SCRIPT_PROPERTY_NAME, + ExpProtocol.Status.Active.toString(), + "%" + FileContentService.SCRIPTS_LINK + "%" + ).getObject(Long.class)); + assayMetrics.put("standardAssayWithPlateSupportCount", new SqlSelector(schema, "SELECT COUNT(*) FROM exp.protocol EP JOIN exp.objectPropertiesView OP ON EP.lsid = OP.objecturi WHERE OP.name = 'PlateMetadata' AND floatValue = 1").getObject(Long.class)); SQLFragment runsWithPlateSQL = new SQLFragment(""" SELECT COUNT(*) FROM exp.experimentrun r diff --git a/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java b/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java index d39c70821b1..2fc3f5dd8f5 100644 --- a/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java +++ b/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java @@ -259,6 +259,10 @@ public List getContainersForFilePath(java.nio.file.Path path) java.nio.file.Path dir = getFileRootPath(c); return dir != null ? dir.resolve(folderName).toFile() : null; + case scripts: + java.nio.file.Path scriptsRoot = getFileRootPath(c, type); + return scriptsRoot != null ? scriptsRoot.toFile() : null; + case pipeline: PipeRoot root = PipelineService.get().findPipelineRoot(c); return root != null ? root.getRootPath() : null; @@ -278,6 +282,13 @@ public List getContainersForFilePath(java.nio.file.Path path) fileRootPath = fileRootPath.resolve(getFolderName(type)); return fileRootPath; + case scripts: + // Return null rather than the bare file root so callers can't mistake the root itself for the @scripts directory. + java.nio.file.Path scriptsRootPath = getFileRootPath(c); + if (null == scriptsRootPath || FileUtil.hasCloudScheme(scriptsRootPath)) + return null; + return scriptsRootPath.resolve(getFolderName(type)); + case pipeline: PipeRoot root = PipelineService.get().findPipelineRoot(c); return root != null ? root.getRootNioPath() : null;