Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions api/src/org/labkey/api/assay/AbstractAssayProvider.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -1271,6 +1273,8 @@ public Pair<ValidationException, Pair<String, String>> 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)
{
Expand Down Expand Up @@ -1326,6 +1330,51 @@ public Pair<ValidationException, Pair<String, String>> 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<AnalysisScript> getValidationAndAnalysisScripts(ExpProtocol protocol, Scope scope)
Expand Down
11 changes: 11 additions & 0 deletions assay/src/org/labkey/assay/AssayModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions experiment/src/org/labkey/experiment/ExperimentModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,10 @@ public List<Container> 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;
Expand All @@ -278,6 +282,13 @@ public List<Container> 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;
Expand Down