diff --git a/api/src/org/labkey/api/pipeline/PipelineJob.java b/api/src/org/labkey/api/pipeline/PipelineJob.java index 712c4083157..dc702e6b2d9 100644 --- a/api/src/org/labkey/api/pipeline/PipelineJob.java +++ b/api/src/org/labkey/api/pipeline/PipelineJob.java @@ -20,6 +20,7 @@ import com.fasterxml.jackson.annotation.PropertyAccessor; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.SerializationFeature; +import com.fasterxml.jackson.databind.jsontype.PolymorphicTypeValidator; import com.fasterxml.jackson.databind.module.SimpleModule; import datadog.trace.api.CorrelationIdentifier; import datadog.trace.api.Trace; @@ -1897,12 +1898,30 @@ public static PipelineJob deserializeJob(@NotNull String serialized) } public static ObjectMapper createObjectMapper() + { + return createObjectMapper(null); + } + + /** + * Build the pipeline-job ObjectMapper. Polymorphic default typing (NON_FINAL) is always active so the concrete + * PipelineJob subclass and its field graph round-trip through {@code @class} type ids on the wire. + * + * @param typeValidator when non-null, default typing is activated with this {@link PolymorphicTypeValidator} + * (a deny-by-default allowlist) instead of the deprecated, unrestricted {@code enableDefaultTyping}. It is consulted + * only on deserialization, so only the pipeline-job deserialize path passes one (see {@code PipelineJacksonTyping}); + * serialization and all other callers pass null and keep the historical permissive behavior. + */ + public static ObjectMapper createObjectMapper(@Nullable PolymorphicTypeValidator typeValidator) { ObjectMapper mapper = JsonUtil.DEFAULT_MAPPER.copy() .setVisibility(PropertyAccessor.ALL, JsonAutoDetect.Visibility.NONE) .setVisibility(PropertyAccessor.FIELD, JsonAutoDetect.Visibility.ANY) - .disable(SerializationFeature.FAIL_ON_EMPTY_BEANS) - .enableDefaultTyping(ObjectMapper.DefaultTyping.NON_FINAL); + .disable(SerializationFeature.FAIL_ON_EMPTY_BEANS); + + if (typeValidator != null) + mapper.activateDefaultTyping(typeValidator, ObjectMapper.DefaultTyping.NON_FINAL); + else + mapper.enableDefaultTyping(ObjectMapper.DefaultTyping.NON_FINAL); SimpleModule module = new SimpleModule(); module.addSerializer(new SqlTimeSerialization.SqlTimeSerializer()); diff --git a/pipeline/src/org/labkey/pipeline/PipelineController.java b/pipeline/src/org/labkey/pipeline/PipelineController.java index cd98a0ca7eb..0589a64922b 100644 --- a/pipeline/src/org/labkey/pipeline/PipelineController.java +++ b/pipeline/src/org/labkey/pipeline/PipelineController.java @@ -17,6 +17,7 @@ import jakarta.servlet.http.HttpServletResponse; import org.apache.commons.lang3.StringUtils; +import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.junit.Test; @@ -101,6 +102,7 @@ import org.labkey.api.util.TestContext; import org.labkey.api.util.URIUtil; import org.labkey.api.util.URLHelper; +import org.labkey.api.util.logging.LogHelper; import org.labkey.api.view.ActionURL; import org.labkey.api.view.HBox; import org.labkey.api.view.HtmlView; @@ -142,6 +144,7 @@ public class PipelineController extends SpringActionController { private static final DefaultActionResolver _resolver = new DefaultActionResolver(PipelineController.class); + private static final Logger LOG = LogHelper.getLogger(PipelineController.class, "Pipeline controller actions, including job cancellation"); public enum Params { path, rootset, overrideRoot } @@ -1087,6 +1090,8 @@ public boolean handlePost(StatusController.RowIdForm form, BindException errors) catch (PipelineProvider.HandlerException e) { _successURL = StatusController.urlDetails(getContainer(), form.getRowId(), e.getMessage()); + // Help diagnose rare but recurring test failure in PipelineCancelTest + LOG.info("CancelJobAction.handlePost: cancel rejected for rowId {} ({}); _successURL set to details page {}", form.getRowId(), e.getMessage(), _successURL); } return true; } @@ -1094,6 +1099,14 @@ public boolean handlePost(StatusController.RowIdForm form, BindException errors) @Override public URLHelper getSuccessURL(StatusController.RowIdForm rowIdForm) { + if (_successURL == null) + { + // Diagnostic (PipelineCancelTest intermittent blank page after cancel): capture the redirect target and the response state at the moment the framework + // decides between issuing the redirect and rendering an (empty) view. A null _successURL here, or a response + // that is already committed, would explain the observed 200/empty-body response instead of a 302. + HttpServletResponse response = getViewContext().getResponse(); + LOG.info("CancelJobAction.getSuccessURL: returning _successURL={}, response.isCommitted={}, response.status={}", _successURL, response.isCommitted(), response.getStatus()); + } return _successURL; } } diff --git a/pipeline/src/org/labkey/pipeline/PipelineModule.java b/pipeline/src/org/labkey/pipeline/PipelineModule.java index f5e50cd5482..92790c74814 100644 --- a/pipeline/src/org/labkey/pipeline/PipelineModule.java +++ b/pipeline/src/org/labkey/pipeline/PipelineModule.java @@ -50,6 +50,9 @@ import org.labkey.api.pipeline.trigger.PipelineTriggerRegistry; import org.labkey.api.pipeline.trigger.PipelineTriggerType; import org.labkey.api.security.User; +import org.labkey.api.settings.OptionalFeatureFlag; +import org.labkey.api.settings.OptionalFeatureService; +import org.labkey.api.settings.OptionalFeatureService.FeatureType; import org.labkey.api.usageMetrics.UsageMetricsService; import org.labkey.api.util.ContextListener; import org.labkey.api.util.PageFlowUtil; @@ -71,6 +74,7 @@ import org.labkey.pipeline.api.CommandLineTokenizer; import org.labkey.pipeline.api.ExecTaskFactory; import org.labkey.pipeline.api.PipelineEmailPreferences; +import org.labkey.pipeline.api.PipelineJacksonTyping; import org.labkey.pipeline.api.PipelineJobMarshaller; import org.labkey.pipeline.api.PipelineJobServiceImpl; import org.labkey.pipeline.api.PipelineManager; @@ -89,6 +93,7 @@ import org.labkey.pipeline.mule.EPipelineQueueImpl; import org.labkey.pipeline.mule.RemoteServerStartup; import org.labkey.pipeline.mule.filters.TaskJmsSelectorFilter; +import org.labkey.pipeline.mule.transformers.PipelineXStreamSecurity; import org.labkey.pipeline.status.StatusController; import org.labkey.pipeline.trigger.PipelineTriggerRegistryImpl; import org.labkey.pipeline.validators.PipelineSetupValidatorFactory; @@ -206,6 +211,10 @@ protected void startupAfterSpringConfig(ModuleContext moduleContext) StatusController.registerAdminConsoleLinks(); WebdavService.get().addProvider(new PipelineWebdavProvider()); + OptionalFeatureService.get().addFeatureFlag(new OptionalFeatureFlag(PipelineJacksonTyping.FEATUREFLAG_DISABLE_JOB_TYPE_ALLOWLIST, + "Disable pipeline job deserialization type allowlist", + "Reverts pipeline job JSON deserialization to unrestricted Jackson default typing instead of the deny-by-default type allowlist. Enable only as a temporary escape hatch if the allowlist rejects a legitimate job type.", false, true, FeatureType.Deprecated)); + if (null != FileContentService.get()) FileContentService.get().addFileListener(new TableUpdaterFileListener(PipelineSchema.getInstance().getTableInfoStatusFiles(), "FilePath", TableUpdaterFileListener.Type.filePathForwardSlash, "RowId")); SiteValidationService svc = SiteValidationService.get(); @@ -323,7 +332,9 @@ public void containerDeleted(Container c, User user) PathMapperImpl.TestCase.class, PipelineCommandTestCase.class, PipelineJobMarshaller.TestCase.class, - PipelineJobServiceImpl.TestCase.class + PipelineJacksonTyping.TestCase.class, + PipelineJobServiceImpl.TestCase.class, + PipelineXStreamSecurity.TestCase.class ); } diff --git a/pipeline/src/org/labkey/pipeline/api/PipelineJacksonTyping.java b/pipeline/src/org/labkey/pipeline/api/PipelineJacksonTyping.java new file mode 100644 index 00000000000..dc222c5f809 --- /dev/null +++ b/pipeline/src/org/labkey/pipeline/api/PipelineJacksonTyping.java @@ -0,0 +1,266 @@ +/* + * Copyright (c) 2026 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.labkey.pipeline.api; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.JavaType; +import com.fasterxml.jackson.databind.JsonMappingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.cfg.MapperConfig; +import com.fasterxml.jackson.databind.jsontype.PolymorphicTypeValidator; +import org.junit.Assert; +import org.junit.Test; +import org.labkey.api.pipeline.PipelineJob; +import org.labkey.api.settings.AppProps; + +import java.sql.Timestamp; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; + +/** + * Deny-by-default type allowlist for deserializing pipeline jobs off the JMS {@code job.queue} channel. + * + *

{@link PipelineJob#createObjectMapper()} enables Jackson polymorphic default typing ({@code NON_FINAL}, + * {@code WRAPPER_ARRAY}) so a job's concrete subclass and its field graph round-trip through {@code @class}-style type + * ids on the wire. Serialization is unaffected by any validator, but on deserialization an unrestricted default + * typer lets the wire pick the instantiated class — the JSON twin of an XStream {@code AnyTypePermission} gadget-chain + * RCE primitive (see {@link org.labkey.pipeline.mule.transformers.PipelineXStreamSecurity}, which locks down the + * sibling {@code status.queue} channel). + * + *

A single {@code org.labkey.} prefix covers every {@link PipelineJob} subclass and LabKey domain object in the graph; + * the remaining allowlist entries are the JDK collection/scalar and known-safe library packages that appear as job + * fields (most JDK scalars — {@code String}, {@code Integer}, {@code File}, {@code URI} — are either final or have + * custom string serializers, so they never carry a type id). + */ +public final class PipelineJacksonTyping +{ + private PipelineJacksonTyping() + { + } + + /** + * Deprecated feature flag that reverts pipeline job deserialization to the historical unrestricted default typing. + * Off by default (i.e. the allowlist is enforced). Acts as a workaround if the allowlist rejects a legitimate type. + */ + public static final String FEATUREFLAG_DISABLE_JOB_TYPE_ALLOWLIST = "PipelineJobDisableTypeAllowlist"; + + // Package prefixes whose subtypes may be instantiated during job deserialization. + private static final List ALLOWED_PREFIXES = List.of( + "org.labkey.", // every PipelineJob subclass and LabKey domain object (module convention) + "java.util.", // HashMap, ArrayList, HashSet, LinkedHashMap, ... (container types) + "java.sql.", // Timestamp, Time, Date (non-final; tagged under NON_FINAL) + "java.time.", // java.time value types + "java.math.", // BigInteger, BigDecimal + "org.json.", // JsonOrgModule types registered on the base mapper + "it.unimi.dsi.fastutil." // fastutil collections used by the sequence-analysis job family + ); + + // Exact JDK / third-party class names whose type ids are pinned by the custom serializers registered in + // PipelineJob.createObjectMapper(): FileSerialization, PathSerialization and URISerialization call + // typeSer.typeId(value, File.class/Path.class/URI.class, ...) and CronExpressionSerialization emits the runtime + // org.quartz.CronExpression. These write the base class as the type id (never a subclass), so an exact allow is both + // necessary and sufficient. + private static final Set ALLOWED_EXACT = Set.of( + "java.io.File", + "java.nio.file.Path", + "java.net.URI", + "org.quartz.CronExpression" + ); + + // Explicit denials, checked before the allowlist. None overlap ALLOWED_PREFIXES today, so this is belt-and-suspenders + // documenting intent and guarding against a future widening of the allowlist (e.g. a broad java.lang. entry). + private static final List DENIED_PREFIXES = List.of( + "java.lang.Runtime", + "java.lang.Process", // Process, ProcessBuilder, ProcessImpl + "javax.naming.", // JNDI (LDAP/RMI lookup gadgets) + "javax.script.", + "javax.management.", + "java.rmi.", + "com.sun.", + "sun.", + "org.springframework.", + "org.apache.xalan.", + "org.apache.commons.collections.functors.", + "org.apache.commons.collections4.functors.", + "org.apache.commons.beanutils.", + "org.codehaus.groovy.runtime.", + "bsh.", + "clojure." + ); + + private static final PolymorphicTypeValidator VALIDATOR = new AllowlistValidator(); + + public static boolean isEnforced() + { + return !AppProps.getInstance().isOptionalFeatureEnabled(FEATUREFLAG_DISABLE_JOB_TYPE_ALLOWLIST); + } + + /** + * The deny-by-default validator; always enforcing, regardless of the escape-hatch flag. + */ + public static PolymorphicTypeValidator validator() + { + return VALIDATOR; + } + + /** + * Build the ObjectMapper used to deserialize pipeline jobs. Applies {@link #validator()} unless the deprecated + * escape-hatch flag has been set, in which case it falls back to the historical unrestricted default typing. + */ + public static ObjectMapper createJobDeserializationMapper() + { + return PipelineJob.createObjectMapper(isEnforced() ? VALIDATOR : null); + } + + private static PolymorphicTypeValidator.Validity classify(String className) + { + String element = elementType(className); + if (element.isEmpty()) + return PolymorphicTypeValidator.Validity.ALLOWED; // primitive or primitive[] + + for (String denied : DENIED_PREFIXES) + { + if (element.startsWith(denied)) + return PolymorphicTypeValidator.Validity.DENIED; + } + if (ALLOWED_EXACT.contains(element)) + return PolymorphicTypeValidator.Validity.ALLOWED; + for (String allowed : ALLOWED_PREFIXES) + { + if (element.startsWith(allowed)) + return PolymorphicTypeValidator.Validity.ALLOWED; + } + return PolymorphicTypeValidator.Validity.DENIED; + } + + // Strip array markers so an array type is judged by its element type. Returns "" for a primitive (or primitive + // array) element, which is always allowed. + private static String elementType(String className) + { + int depth = 0; + while (depth < className.length() && className.charAt(depth) == '[') + depth++; + if (depth == 0) + return className; + String rest = className.substring(depth); + if (rest.startsWith("L") && rest.endsWith(";")) + return rest.substring(1, rest.length() - 1); + return ""; // primitive array element descriptor (e.g. "[I") + } + + private static class AllowlistValidator extends PolymorphicTypeValidator.Base + { + // validateBaseType is left at the inherited INDETERMINATE so that per-subtype checks always run — returning + // ALLOWED here would accept every subtype of a base type without inspection. + + @Override + public Validity validateSubClassName(MapperConfig config, JavaType baseType, String subClassName) + { + return classify(subClassName); + } + + @Override + public Validity validateSubType(MapperConfig config, JavaType baseType, JavaType subType) + { + Class raw = subType.getRawClass(); + return raw == null ? Validity.DENIED : classify(raw.getName()); + } + } + + public static class TestCase extends Assert + { + public static class Holder + { + public Object value; + } + + @Test + public void allowsExpectedJobTypes() throws Exception + { + // The Holder root (org.labkey.*), the HashMap/ArrayList (java.util.*), and the java.sql.Timestamp value all + // carry type ids and must round-trip through the enforcing mapper without a ForbiddenClass-style rejection. + ObjectMapper writeMapper = PipelineJob.createObjectMapper(); + ObjectMapper secureMapper = PipelineJob.createObjectMapper(validator()); + + Map map = new HashMap<>(); + map.put("string", "hello"); + map.put("number", 42); + map.put("timestamp", new Timestamp(1400938833L)); + map.put("list", new ArrayList<>(List.of("a", "b"))); + Holder holder = new Holder(); + holder.value = map; + + String json = writeMapper.writeValueAsString(holder); + Holder result = secureMapper.readValue(json, Holder.class); + + assertTrue("Expected the map value to survive", result.value instanceof Map); + assertTrue("Expected the Timestamp to survive", ((Map) result.value).get("timestamp") instanceof Timestamp); + } + + @Test + public void rejectsGadgetType() + { + // A denied JDK gadget must be refused during deserialization before the class is resolved/instantiated. + ObjectMapper secureMapper = PipelineJob.createObjectMapper(validator()); + String payload = "[\"java.lang.ProcessBuilder\",{\"command\":[\"/bin/sh\"]}]"; + try + { + secureMapper.readValue(payload, Object.class); + fail("Expected java.lang.ProcessBuilder to be rejected by the type allowlist"); + } + catch (JsonProcessingException expected) + { + // expected: PolymorphicTypeValidator denied the subtype + } + } + + @Test + public void rejectsUnlistedType() + { + // Deny-by-default: even a benign class outside the allowlist (java.awt.Point) is refused. + ObjectMapper secureMapper = PipelineJob.createObjectMapper(validator()); + String payload = "[\"java.awt.Point\",{\"x\":0,\"y\":0}]"; + try + { + secureMapper.readValue(payload, Object.class); + fail("Expected java.awt.Point to be rejected by the deny-by-default allowlist"); + } + catch (JsonProcessingException expected) + { + // expected + } + } + + @Test + public void allowsFrameworkPinnedTypeIds() throws JsonMappingException + { + // The custom serializers in PipelineJob.createObjectMapper() (File/Path/URI/CronExpression) pin these exact + // type ids on the wire. If the allowlist doesn't permit them, every job carrying such a field fails to + // deserialize. Assert at the validator level so the check is independent of PipelineJobService/server state. + PolymorphicTypeValidator v = validator(); + for (String cn : new String[]{"java.io.File", "java.nio.file.Path", "java.net.URI", "org.quartz.CronExpression"}) + { + assertEquals(cn + " must be allowed (pinned by a registered custom serializer)", + PolymorphicTypeValidator.Validity.ALLOWED, v.validateSubClassName(null, null, cn)); + } + // A gadget must still be denied, and deny takes precedence over any allow. + assertEquals(PolymorphicTypeValidator.Validity.DENIED, v.validateSubClassName(null, null, "java.lang.ProcessBuilder")); + } + } +} diff --git a/pipeline/src/org/labkey/pipeline/api/PipelineJobMarshaller.java b/pipeline/src/org/labkey/pipeline/api/PipelineJobMarshaller.java index 5fd6cf3a9a3..d7c34427678 100644 --- a/pipeline/src/org/labkey/pipeline/api/PipelineJobMarshaller.java +++ b/pipeline/src/org/labkey/pipeline/api/PipelineJobMarshaller.java @@ -150,7 +150,9 @@ public String serializeToJSON(PipelineJob job, boolean ensureDeserialize) @Override public PipelineJob deserializeFromJSON(String json, Class cls) { - ObjectMapper mapper = PipelineJob.createObjectMapper(); + // Deny-by-default type allowlist: jobs arrive as JSON with polymorphic default typing, so an unrestricted mapper + // would let the wire pick the instantiated class. See PipelineJacksonTyping. + ObjectMapper mapper = PipelineJacksonTyping.createJobDeserializationMapper(); try { diff --git a/pipeline/src/org/labkey/pipeline/mule/transformers/ObjectToXml.java b/pipeline/src/org/labkey/pipeline/mule/transformers/ObjectToXml.java index ff43a0ed3eb..ba489d4a959 100644 --- a/pipeline/src/org/labkey/pipeline/mule/transformers/ObjectToXml.java +++ b/pipeline/src/org/labkey/pipeline/mule/transformers/ObjectToXml.java @@ -15,8 +15,6 @@ */ package org.labkey.pipeline.mule.transformers; -import com.thoughtworks.xstream.XStream; -import com.thoughtworks.xstream.security.AnyTypePermission; import org.mule.umo.UMOEventContext; import org.mule.umo.transformer.TransformerException; @@ -27,13 +25,12 @@ public class ObjectToXml extends org.mule.transformers.xml.ObjectToXml @Override public Object transform(Object src, String encoding, UMOEventContext context) throws TransformerException { - // Allow XStream to deserialize into any class. We are using this internally and trust the content of - // the messages since the JMS endpoint is considered secure. - // https://x-stream.github.io/security.html#framework + // Restrict XStream deserialization to the types reachable from a StatusRequest rather than granting + // AnyTypePermission. The JMS endpoint is considered secure, but deny-by-default is defense-in-depth + // against gadget-chain attacks. https://x-stream.github.io/security.html#framework if (!_securityInitialized) { - XStream x = getXStream(); - x.addPermission(AnyTypePermission.ANY); + PipelineXStreamSecurity.configure(getXStream()); _securityInitialized = true; } return super.transform(src, encoding, context); diff --git a/pipeline/src/org/labkey/pipeline/mule/transformers/PipelineXStreamSecurity.java b/pipeline/src/org/labkey/pipeline/mule/transformers/PipelineXStreamSecurity.java new file mode 100644 index 00000000000..c9058889808 --- /dev/null +++ b/pipeline/src/org/labkey/pipeline/mule/transformers/PipelineXStreamSecurity.java @@ -0,0 +1,178 @@ +/* + * Copyright (c) 2026 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.labkey.pipeline.mule.transformers; + +import com.thoughtworks.xstream.XStream; +import com.thoughtworks.xstream.security.ForbiddenClassException; +import com.thoughtworks.xstream.security.NoTypePermission; +import com.thoughtworks.xstream.security.NullPermission; +import com.thoughtworks.xstream.security.PrimitiveTypePermission; +import com.thoughtworks.xstream.security.TypeHierarchyPermission; +import org.junit.Assert; +import org.junit.Test; +import org.labkey.api.collections.CaseInsensitiveHashSet; +import org.labkey.api.pipeline.PipelineJob; +import org.labkey.api.pipeline.TaskId; +import org.labkey.api.util.URLHelper; +import org.labkey.pipeline.mule.RequeueLostJobsRequest; +import org.labkey.pipeline.mule.StatusChangeRequest; +import org.labkey.pipeline.mule.StatusRequest; + +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +/** + * Locks down the XStream type-permission allowlist for the pipeline JMS status channel, used by the + * {@link XmlToObject}/{@link ObjectToXml} transformers in the remote/cluster Mule configs. + *

+ * The JMS endpoint is treated as a trusted-internal channel, but we still deny-by-default rather than granting + * {@code AnyTypePermission.ANY}, which would let any class on the classpath be instantiated during deserialization + * (a classic XStream gadget-chain RCE primitive). See https://x-stream.github.io/security.html. This is + * defense-in-depth: only the types that legitimately appear in a {@link StatusRequest} object graph are allowed. + */ +public class PipelineXStreamSecurity +{ + private PipelineXStreamSecurity() + { + } + + public static void configure(XStream x) + { + // Deny everything, then carve out exactly the types reachable from a StatusRequest. NONE must be added + // first: XStream evaluates permissions in reverse registration order, so the catch-all deny goes in last. + x.addPermission(NoTypePermission.NONE); + x.addPermission(NullPermission.NULL); + x.addPermission(PrimitiveTypePermission.PRIMITIVES); + + // StatusChangeRequest, RequeueLostJobsRequest, and any future StatusRequest implementations. + x.addPermission(new TypeHierarchyPermission(StatusRequest.class)); + + x.allowTypes(new Class[]{ + String.class, + TaskId.class, // StatusChangeRequest._activeTaskId + TaskId.Type.class, // TaskId._type + Class.class, // TaskId._namespaceClass + }); + + // HashSet serializes as which comes as java.util.Set but then resolves to HashSet, so allow the interface + x.allowTypes(new Class[]{Set.class}); + x.addPermission(new TypeHierarchyPermission(HashSet.class)); + x.allowTypes(new String[]{"java.util.Collections$SingletonSet"}); + } + + /** + * Verifies that the allowlist permits every type reachable from a real StatusRequest object graph (so the + * configuration does not break legitimate JMS status traffic) while still rejecting arbitrary classes. + */ + public static class TestCase extends Assert + { + private static Object roundTrip(Object o) + { + XStream x = new XStream(); + configure(x); + // Serialization is not gated by permissions; deserialization is what the allowlist guards. A missing + // entry surfaces here as a ForbiddenClassException. + return x.fromXML(x.toXML(o)); + } + + private static StatusChangeRequest statusChangeRequest(TaskId activeTaskId, String guid) + { + PipelineJob job = new PipelineJob() + { + @Override + public URLHelper getStatusHref() + { + return null; + } + + @Override + public String getDescription() + { + return "test job"; + } + + @Override + public String getJobGUID() + { + return guid; + } + + @Override + public TaskId getActiveTaskId() + { + return activeTaskId; + } + }; + return new StatusChangeRequest(job, "RUNNING", "status info", "remote-host"); + } + + @Test + public void statusChangeWithNamespaceClassTaskId() + { + // TaskId(Class) populates _namespaceClass, exercising the Class.class allowlist entry. + Object result = roundTrip(statusChangeRequest(new TaskId(PipelineJob.class), "job-guid-1")); + assertTrue("Expected a StatusChangeRequest", result instanceof StatusChangeRequest); + } + + @Test + public void statusChangeWithModuleTaskId() + { + // TaskId(module, Type, name, version) populates the Type enum and the double version. + Object result = roundTrip(statusChangeRequest(new TaskId("myModule", TaskId.Type.task, "myTask", 1.0), "job-guid-2")); + assertTrue("Expected a StatusChangeRequest", result instanceof StatusChangeRequest); + } + + @Test + public void statusChangeWithNullTaskId() + { + assertTrue(roundTrip(statusChangeRequest(null, "job-guid-3")) instanceof StatusChangeRequest); + } + + @Test + public void requeueWithHashSet() + { + RequeueLostJobsRequest req = new RequeueLostJobsRequest(new HashSet<>(List.of("location1")), new HashSet<>(List.of("job1")), "remote-host"); + assertTrue(roundTrip(req) instanceof RequeueLostJobsRequest); + } + + @Test + public void requeueWithCaseInsensitiveHashSet() + { + // PipelineModule builds the locations set as a CaseInsensitiveHashSet. + RequeueLostJobsRequest req = new RequeueLostJobsRequest(new CaseInsensitiveHashSet("location1"), new HashSet<>(List.of("job1")), null); + assertTrue(roundTrip(req) instanceof RequeueLostJobsRequest); + } + + @Test + public void requeueWithSingletonSet() + { + // RemoteServerStartup builds the locations set with Collections.singleton(...). + RequeueLostJobsRequest req = new RequeueLostJobsRequest(Collections.singleton("location1"), Collections.singleton("job1"), "remote-host"); + assertTrue(roundTrip(req) instanceof RequeueLostJobsRequest); + } + + @Test(expected = ForbiddenClassException.class) + public void rejectsArbitraryType() + { + // A class outside the allowlist must be refused during deserialization. + XStream x = new XStream(); + configure(x); + x.fromXML("/tmp/evil"); + } + } +} diff --git a/pipeline/src/org/labkey/pipeline/mule/transformers/XmlToObject.java b/pipeline/src/org/labkey/pipeline/mule/transformers/XmlToObject.java index 5566f495582..99d4199c83c 100644 --- a/pipeline/src/org/labkey/pipeline/mule/transformers/XmlToObject.java +++ b/pipeline/src/org/labkey/pipeline/mule/transformers/XmlToObject.java @@ -15,8 +15,6 @@ */ package org.labkey.pipeline.mule.transformers; -import com.thoughtworks.xstream.XStream; -import com.thoughtworks.xstream.security.AnyTypePermission; import org.mule.umo.UMOEventContext; import org.mule.umo.transformer.TransformerException; @@ -27,13 +25,12 @@ public class XmlToObject extends org.mule.transformers.xml.XmlToObject @Override public Object transform(Object src, String encoding, UMOEventContext context) throws TransformerException { - // Allow XStream to deserialize into any class. We are using this internally and trust the content of - // the messages since the JMS endpoint is considered secure. - // https://x-stream.github.io/security.html#framework + // Restrict XStream deserialization to the types reachable from a StatusRequest rather than granting + // AnyTypePermission. The JMS endpoint is considered secure, but deny-by-default is defense-in-depth + // against gadget-chain attacks. https://x-stream.github.io/security.html#framework if (!_securityInitialized) { - XStream x = getXStream(); - x.addPermission(AnyTypePermission.ANY); + PipelineXStreamSecurity.configure(getXStream()); _securityInitialized = true; } return super.transform(src, encoding, context);