Merge branch 'shell-workflow-step'
diff --git a/core/src/main/java/org/apache/brooklyn/core/BrooklynFeatureEnablement.java b/core/src/main/java/org/apache/brooklyn/core/BrooklynFeatureEnablement.java
index 2991799..f5e57b6 100644
--- a/core/src/main/java/org/apache/brooklyn/core/BrooklynFeatureEnablement.java
+++ b/core/src/main/java/org/apache/brooklyn/core/BrooklynFeatureEnablement.java
@@ -138,6 +138,12 @@
     public static final String FEATURE_DISALLOW_REPARENTING = "brooklyn.disallowReparenting";
 
     /**
+     * Whether lookup of the location of the host is permitted, either 'true' or 'false'.
+     * Since 1.1 defaults to 'false' normally to prevent odd outbound traffic, but 'true' in development environments for tests to run.
+     */
+    public static final String FEATURE_HOST_GEO_LOOKUP = "brooklyn.hostGeoLookup";
+
+    /**
      * Values explicitly set by Java calls.
      */
     private static final Map<String, Boolean> FEATURE_ENABLEMENTS = Maps.newLinkedHashMap();
@@ -175,6 +181,7 @@
         setDefault(FEATURE_AUTO_FIX_CATALOG_REF_ON_REBIND, false);
         setDefault(FEATURE_SSH_ASYNC_EXEC, false);
         setDefault(FEATURE_VALIDATE_LOCATION_SSH_KEYS, true);
+        setDefault(FEATURE_HOST_GEO_LOOKUP, BrooklynVersion.isDevelopmentEnvironment());
     }
     
     static {
diff --git a/core/src/main/java/org/apache/brooklyn/core/location/geo/GeoBytesHostGeoLookup.java b/core/src/main/java/org/apache/brooklyn/core/location/geo/GeoBytesHostGeoLookup.java
index 9863603..73f3a52 100644
--- a/core/src/main/java/org/apache/brooklyn/core/location/geo/GeoBytesHostGeoLookup.java
+++ b/core/src/main/java/org/apache/brooklyn/core/location/geo/GeoBytesHostGeoLookup.java
@@ -80,6 +80,8 @@
     
     @Override
     public HostGeoInfo getHostGeoInfo(InetAddress address) throws MalformedURLException, IOException {
+        if (isHostGeoLookupGloballyDisabled()) return null;
+
         String url = getPropertiesLookupUrlFor(address);
         if (log.isDebugEnabled())
             log.debug("Geo info lookup for "+address+" at "+url);
@@ -101,5 +103,4 @@
             return null;
         }
     }
-    
 }
diff --git a/core/src/main/java/org/apache/brooklyn/core/location/geo/HostGeoLookup.java b/core/src/main/java/org/apache/brooklyn/core/location/geo/HostGeoLookup.java
index ec25e07..73345e3 100644
--- a/core/src/main/java/org/apache/brooklyn/core/location/geo/HostGeoLookup.java
+++ b/core/src/main/java/org/apache/brooklyn/core/location/geo/HostGeoLookup.java
@@ -20,8 +20,13 @@
 
 import java.net.InetAddress;
 
+import org.apache.brooklyn.core.BrooklynFeatureEnablement;
+
 public interface HostGeoLookup {
 
-    public HostGeoInfo getHostGeoInfo(InetAddress address) throws Exception;
-    
+    HostGeoInfo getHostGeoInfo(InetAddress address) throws Exception;
+
+    default boolean isHostGeoLookupGloballyDisabled() {
+        return !BrooklynFeatureEnablement.isEnabled(BrooklynFeatureEnablement.FEATURE_HOST_GEO_LOOKUP);
+    }
 }
diff --git a/core/src/main/java/org/apache/brooklyn/core/location/geo/MaxMind2HostGeoLookup.java b/core/src/main/java/org/apache/brooklyn/core/location/geo/MaxMind2HostGeoLookup.java
index cb7994c..43f073a 100644
--- a/core/src/main/java/org/apache/brooklyn/core/location/geo/MaxMind2HostGeoLookup.java
+++ b/core/src/main/java/org/apache/brooklyn/core/location/geo/MaxMind2HostGeoLookup.java
@@ -64,6 +64,7 @@
     
     @Override
     public HostGeoInfo getHostGeoInfo(InetAddress address) throws MalformedURLException, IOException {
+        if (isHostGeoLookupGloballyDisabled()) return null;
         if (lookupFailed) return null;
         
         DatabaseReader ll = getDatabaseReader();
diff --git a/core/src/main/java/org/apache/brooklyn/core/location/geo/UtraceHostGeoLookup.java b/core/src/main/java/org/apache/brooklyn/core/location/geo/UtraceHostGeoLookup.java
index 62fd437..725650d 100644
--- a/core/src/main/java/org/apache/brooklyn/core/location/geo/UtraceHostGeoLookup.java
+++ b/core/src/main/java/org/apache/brooklyn/core/location/geo/UtraceHostGeoLookup.java
@@ -99,6 +99,7 @@
     /** does the {@link #retrieveHostGeoInfo(InetAddress)}, but in the background with a default timeout */
     @Override
     public HostGeoInfo getHostGeoInfo(InetAddress address) throws MalformedURLException, IOException {
+        if (isHostGeoLookupGloballyDisabled()) return null;
         if (Duration.sinceUtc(LAST_FAILURE_UTC).compareTo(RETRY_INTERVAL) < 0) {
             // wait at least 60s since a failure
             return null;
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/entitlement/EntitlementManagerAdapter.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/entitlement/EntitlementManagerAdapter.java
index 5dd6e24..e41ffe8 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/entitlement/EntitlementManagerAdapter.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/entitlement/EntitlementManagerAdapter.java
@@ -111,6 +111,11 @@
         }
 
         @Override
+        public Boolean handleExecuteScript() {
+            return isEntitledToExecuteScripts(context);
+        }
+
+        @Override
         public Boolean handleRoot() {
             return isEntitledToRoot(context);
         }
@@ -145,6 +150,7 @@
     protected abstract boolean isEntitledToSeeAllServerInfo(EntitlementContext context);
     protected abstract boolean isEntitledToSeeServerStatus(EntitlementContext context);
     protected abstract boolean isEntitledToExecuteGroovyScripts(EntitlementContext context);
+    protected abstract boolean isEntitledToExecuteScripts(EntitlementContext context);
     protected abstract boolean isEntitledToRoot(EntitlementContext context);
     
 }
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/entitlement/Entitlements.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/entitlement/Entitlements.java
index 3eac82b..5aac735 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/entitlement/Entitlements.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/entitlement/Entitlements.java
@@ -131,10 +131,15 @@
     public static EntitlementClass<Void> LOGBOOK_LOG_STORE_QUERY = new BasicEntitlementClassDefinition<Void>("logbook.query", Void.class);
 
     /**
-     * Permission to execute groovy scripts
+     * Permission to execute groovy scripts. Since 1.1 prefer {@link #EXECUTE_SCRIPT}.
      */
     public static EntitlementClass<Void> EXECUTE_GROOVY_SCRIPT = new BasicEntitlementClassDefinition<Void>("groovy_script.execute", Void.class);
 
+    /**
+     * Permission to run local script commands, eg 'shell' workflow step, groovy scripts, etc
+     */
+    public static EntitlementClass<Void> EXECUTE_SCRIPT = new BasicEntitlementClassDefinition<Void>("script.execute", Void.class);
+
     @SuppressWarnings("unchecked")
     public enum EntitlementClassesEnum {
         ENTITLEMENT_SEE_CATALOG_ITEM(SEE_CATALOG_ITEM) { public <T> T handle(EntitlementClassesHandler<T> handler, Object argument) { return handler.handleSeeCatalogItem((String)argument); } },
@@ -154,6 +159,7 @@
         ENTITLEMENT_SERVER_STATUS(SERVER_STATUS) { public <T> T handle(EntitlementClassesHandler<T> handler, Object argument) { return handler.handleSeeServerStatus(); } },
         ENTITLEMENT_ROOT(ROOT) { public <T> T handle(EntitlementClassesHandler<T> handler, Object argument) { return handler.handleRoot(); } },
         ENTITLEMENT_EXECUTE_GROOVY_SCRIPT(EXECUTE_GROOVY_SCRIPT) { public <T> T handle(EntitlementClassesHandler<T> handler, Object argument) { return handler.handleExecuteGroovyScript(); } },
+        ENTITLEMENT_EXECUTE_SCRIPT(EXECUTE_SCRIPT) { public <T> T handle(EntitlementClassesHandler<T> handler, Object argument) { return handler.handleExecuteScript(); } },
 
         /* NOTE, 'ROOT' USER ONLY IS ALLOWED TO SEE THE LOGS. */
         ENTITLEMENT_LOGBOOK_QUERY(LOGBOOK_LOG_STORE_QUERY) { public <T> T handle(EntitlementClassesHandler<T> handler, Object argument) { return handler.handleRoot(); } },
@@ -191,6 +197,7 @@
         public T handleAddJava(Object app);
         public T handleSeeAllServerInfo();
         public T handleExecuteGroovyScript();
+        public T handleExecuteScript();
         public T handleRoot();
     }
     
@@ -267,13 +274,13 @@
     }
 
     /**
-     * @return An entitlement manager allowing everything but {@link #EXECUTE_GROOVY_SCRIPT}.
+     * @return An entitlement manager allowing everything but {@link #EXECUTE_SCRIPT}, {@link #EXECUTE_GROOVY_SCRIPT}.
      */
     public static EntitlementManager powerUser() {
         return new EntitlementManager() {
             @Override
             public <T> boolean isEntitled(EntitlementContext context, EntitlementClass<T> permission, T entitlementClassArgument) {
-                return !EXECUTE_GROOVY_SCRIPT.equals(permission);
+                return !EXECUTE_GROOVY_SCRIPT.equals(permission) && !EXECUTE_SCRIPT.equals(permission);
             }
             @Override
             public String toString() {
@@ -284,7 +291,7 @@
 
     /**
      * @return An entitlement manager allowing everything but {@link #ROOT}, {@link #LOGBOOK_LOG_STORE_QUERY}, {@link #SEE_ALL_SERVER_INFO},
-     * {@link #EXECUTE_GROOVY_SCRIPT}, {@link #MODIFY_ENTITY}, and {@link #HA_ADMIN}.
+     * {@link #EXECUTE_GROOVY_SCRIPT}, {@link #EXECUTE_SCRIPT}, {@link #MODIFY_ENTITY}, and {@link #HA_ADMIN}.
      */
     public static EntitlementManager user() {
         return new EntitlementManager() {
@@ -295,6 +302,7 @@
                         !ROOT.equals(permission) &&
                         !LOGBOOK_LOG_STORE_QUERY.equals(permission) &&
                         !EXECUTE_GROOVY_SCRIPT.equals(permission) &&
+                        !EXECUTE_SCRIPT.equals(permission) &&
                         !MODIFY_ENTITY.equals(permission) &&
                         !HA_ADMIN.equals(permission);
             }
diff --git a/core/src/main/java/org/apache/brooklyn/core/server/BrooklynServerConfig.java b/core/src/main/java/org/apache/brooklyn/core/server/BrooklynServerConfig.java
index fc6979e..b1d7331 100644
--- a/core/src/main/java/org/apache/brooklyn/core/server/BrooklynServerConfig.java
+++ b/core/src/main/java/org/apache/brooklyn/core/server/BrooklynServerConfig.java
@@ -178,4 +178,7 @@
     public static final ConfigKey<List<String>> SENSITIVE_FIELDS_EXT_BLOCKED_PHRASES = ConfigKeys.newConfigKey(new TypeToken<List<String>>() {}, "brooklyn.security.sensitive.fields.ext.blocked.phrases",
             "Extended blocking settings when plaintext values are blocked, allowing also to block DSL and complex values which contain any of the phrases supplied in this config key (comma-separated list)");
 
+    public static final ConfigKey<Boolean> SHELL_WORKFLOW_STEP_DISABLED = ConfigKeys.newBooleanConfigKey("brooklyn.security.shell.workflow_step.disabled",
+            "Whether the ShellWorkflowStep is disabled for security; default false");
+
 }
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/external/ShellWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/external/ShellWorkflowStep.java
new file mode 100644
index 0000000..961ae5c
--- /dev/null
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/external/ShellWorkflowStep.java
@@ -0,0 +1,87 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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.apache.brooklyn.core.workflow.steps.external;
+
+import java.util.Map;
+import java.util.function.Supplier;
+
+import com.google.common.reflect.TypeToken;
+import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.config.ConfigKeys;
+import org.apache.brooklyn.core.config.MapConfigKey;
+import org.apache.brooklyn.core.location.Locations;
+import org.apache.brooklyn.core.mgmt.BrooklynTags;
+import org.apache.brooklyn.core.mgmt.entitlement.Entitlements;
+import org.apache.brooklyn.core.mgmt.entitlement.Entitlements.EntityAndItem;
+import org.apache.brooklyn.core.mgmt.entitlement.Entitlements.StringAndArgument;
+import org.apache.brooklyn.core.mgmt.entitlement.WebEntitlementContext;
+import org.apache.brooklyn.core.workflow.WorkflowStepDefinition;
+import org.apache.brooklyn.core.workflow.WorkflowStepInstanceExecutionContext;
+import org.apache.brooklyn.core.workflow.steps.variables.SetVariableWorkflowStep;
+import org.apache.brooklyn.location.ssh.SshMachineLocation;
+import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.core.json.ShellEnvironmentSerializer;
+import org.apache.brooklyn.util.core.predicates.DslPredicates;
+import org.apache.brooklyn.util.core.task.DynamicTasks;
+import org.apache.brooklyn.util.core.task.ssh.ConnectionDefinition;
+import org.apache.brooklyn.util.core.task.ssh.SshTasks;
+import org.apache.brooklyn.util.core.task.ssh.internal.RemoteExecTaskConfigHelper;
+import org.apache.brooklyn.util.core.task.system.ProcessTaskFactory;
+import org.apache.brooklyn.util.core.task.system.ProcessTaskWrapper;
+import org.apache.brooklyn.util.core.task.system.internal.SystemProcessTaskFactory;
+import org.apache.brooklyn.util.core.text.TemplateProcessor;
+import org.apache.brooklyn.util.text.Strings;
+
+import static org.apache.brooklyn.core.server.BrooklynServerConfig.SHELL_WORKFLOW_STEP_DISABLED;
+
+public class ShellWorkflowStep extends WorkflowStepDefinition {
+
+    public static final String SHORTHAND = "${command...}";
+
+    public static final ConfigKey<String> COMMAND = ConfigKeys.newStringConfigKey("command");
+    //TODO public static final ConfigKey<String> COMMAND_URL = ConfigKeys.newStringConfigKey("command_url");
+    public static final ConfigKey<Map<String,Object>> ENV = new MapConfigKey.Builder(Object.class, "env").build();
+    public static final ConfigKey<DslPredicates.DslPredicate<Integer>> EXIT_CODE = ConfigKeys.newConfigKey(new TypeToken<DslPredicates.DslPredicate<Integer>>() {}, "exit_code");
+    public static final ConfigKey<Integer> OUTPUT_MAX_SIZE = ConfigKeys.newIntegerConfigKey("output_max_size", "Maximum size for stdout and stderr, or -1 for no limit", 100000);
+
+    ConfigKey<SetVariableWorkflowStep.InterpolationMode> INTERPOLATION_MODE = ConfigKeys.newConfigKeyWithDefault(SetVariableWorkflowStep.INTERPOLATION_MODE, SetVariableWorkflowStep.InterpolationMode.FULL);
+    ConfigKey<TemplateProcessor.InterpolationErrorMode> INTERPOLATION_ERRORS = ConfigKeys.newConfigKeyWithDefault(SetVariableWorkflowStep.INTERPOLATION_ERRORS, TemplateProcessor.InterpolationErrorMode.IGNORE);
+
+    @Override
+    public void populateFromShorthand(String expression) {
+        populateFromShorthandTemplate(SHORTHAND, expression);
+    }
+
+    @Override
+    protected Object doTaskBody(WorkflowStepInstanceExecutionContext context) {
+        if (Boolean.TRUE.equals(context.getManagementContext().getConfig().getConfig(SHELL_WORKFLOW_STEP_DISABLED))) {
+            throw new IllegalStateException("The 'shell' workflow step is disabled in this intance of Cloudsoft AMP.");
+        }
+        Entitlements.checkEntitled(context.getManagementContext().getEntitlementManager(), Entitlements.EXECUTE_SCRIPT, null);
+
+        String command = new SetVariableWorkflowStep.ConfigurableInterpolationEvaluation<>(context, TypeToken.of(String.class), getInput().get(COMMAND.getName()),
+                context.getInputOrDefault(INTERPOLATION_MODE), context.getInputOrDefault(INTERPOLATION_ERRORS)).evaluate();
+
+        if (Strings.isBlank(command)) throw new IllegalStateException("'command' is required");
+
+        return DynamicTasks.queue(SshWorkflowStep.customizeProcessTaskFactory(context, new SystemProcessTaskFactory(command)).newTask()).asTask().getUnchecked();
+    }
+
+    @Override protected Boolean isDefaultIdempotent() { return false; }
+}
diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/entitlement/EntitlementsTest.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/entitlement/EntitlementsTest.java
index 34690bb..5f7802d 100644
--- a/core/src/test/java/org/apache/brooklyn/core/mgmt/entitlement/EntitlementsTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/entitlement/EntitlementsTest.java
@@ -92,6 +92,7 @@
         assertFalse(allowSeeEntity.isEntitled(null, Entitlements.DEPLOY_APPLICATION, null));
         assertTrue(allowSeeEntity.isEntitled(null, Entitlements.SEE_ALL_SERVER_INFO, null));
         assertFalse(allowSeeEntity.isEntitled(null, Entitlements.EXECUTE_GROOVY_SCRIPT, null));
+        assertFalse(allowSeeEntity.isEntitled(null, Entitlements.EXECUTE_SCRIPT, null));
     }
 
     // nonSecret
diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBasicTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBasicTest.java
index 347ac09..8d0ce20 100644
--- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBasicTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBasicTest.java
@@ -47,6 +47,7 @@
 import org.apache.brooklyn.core.workflow.steps.*;
 import org.apache.brooklyn.core.workflow.steps.appmodel.*;
 import org.apache.brooklyn.core.workflow.steps.external.HttpWorkflowStep;
+import org.apache.brooklyn.core.workflow.steps.external.ShellWorkflowStep;
 import org.apache.brooklyn.core.workflow.steps.external.SshWorkflowStep;
 import org.apache.brooklyn.core.workflow.steps.flow.*;
 import org.apache.brooklyn.core.workflow.steps.variables.*;
@@ -125,6 +126,7 @@
         addRegisteredTypeBean(mgmt, "workflow", CustomWorkflowStep.class);
         addRegisteredTypeBean(mgmt, "foreach", ForeachWorkflowStep.class);
         addRegisteredTypeBean(mgmt, "ssh", SshWorkflowStep.class);
+        addRegisteredTypeBean(mgmt, "shell", ShellWorkflowStep.class);
         addRegisteredTypeBean(mgmt, "http", HttpWorkflowStep.class);
 
         addRegisteredTypeBean(mgmt, "workflow-effector", WorkflowEffector.class);
@@ -407,13 +409,12 @@
             Object workflowId = ids.get("workflow");
             List tasksIds = (List) ids.get("tasks");
 
-            if (logWatcher.getMessages().size()!=8) {
-                // add logging for intermittent failure; sometimes we are getting way more messages than we expect
-                // on slow servers we might see 9, with a "Blocked by lock on lock-for-incrementor, currently held by <other task>" at the end
-                throw new IllegalStateException("Wrong number of messages found ("+logWatcher.getMessages().size()+", not 8): "+logWatcher.getMessages());
-            }
+            List<String> msgs = logWatcher.getMessages().stream().filter(x -> !x.startsWith("Blocked by lock")).collect(Collectors.toList());
+            // can have "Blocked by lock on lock-for-incrementor, currently held by JPuhvC9I" from a previous invocation?
 
-            Asserts.assertEquals(logWatcher.getMessages(), MutableList.of(
+            if (msgs.size()!=8) throw new IllegalStateException("Wrong number of messages found ("+msgs.size()+", not 8): "+msgs);
+
+            Asserts.assertEquals(msgs, MutableList.of(
                     "Starting workflow 'myWorkflow (workflow effector)', moving to first step "+workflowId+"-1",
                     "Starting step "+workflowId+"-1 in task "+tasksIds.get(0),
                     "one",
diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBeefyStepTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBeefyStepTest.java
index 228c391..e9abdcd 100644
--- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBeefyStepTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBeefyStepTest.java
@@ -136,6 +136,12 @@
         Asserts.assertEquals(r, MutableMap.of());
     }
 
+    @Test(groups="Integration")
+    public void testShell() {
+        Object result = runStep("shell echo foo", null);
+        Asserts.assertEquals(result, MutableMap.of("exit_code", 0, "stdout", "foo\n", "stderr", ""));
+    }
+
     @Test
     public void testSshLocalhost() throws NoMachinesAvailableException {
         LocalhostMachineProvisioningLocation loc = mgmt.getLocationManager().createLocation(LocationSpec.create(LocalhostMachineProvisioningLocation.class)
diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java
index 32481c4..88962ea 100644
--- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java
@@ -596,7 +596,9 @@
                     null);
             Asserts.assertEquals(lastInvocation.getUnchecked(), "error-handler worked!");
 
-            List<String> msgs = logWatcher.getMessages();
+            List<String> msgs = logWatcher.getMessages().stream().filter(x -> !x.startsWith("Blocked by lock")).collect(Collectors.toList());
+            // can have "Blocked by lock on lock-for-incrementor, currently held by JPuhvC9I" from a previous invocation?
+
             log.info("Error handler output:\n"+msgs.stream().collect(Collectors.joining("\n")));
             Asserts.assertEntriesSatisfy(msgs, MutableList.of(
                 m -> m.matches("Starting workflow 'myWorkflow .workflow effector.', moving to first step .*-1"),
@@ -627,7 +629,9 @@
                                             "output", "error-handler worked!"))));
             Asserts.assertEquals(lastInvocation.getUnchecked(), "error-handler worked!");
 
-            List<String> msgs = logWatcher.getMessages();
+            List<String> msgs = logWatcher.getMessages().stream().filter(x -> !x.startsWith("Blocked by lock")).collect(Collectors.toList());
+            // can have "Blocked by lock on lock-for-incrementor, currently held by JPuhvC9I" from a previous invocation?
+
             log.info("Error handler output:\n"+msgs.stream().collect(Collectors.joining("\n")));
             Asserts.assertEntriesSatisfy(msgs, MutableList.of(
                     m -> m.matches("Starting workflow 'myWorkflow .workflow effector.', moving to first step .*-1"),
diff --git a/karaf/init/src/main/resources/catalog.bom b/karaf/init/src/main/resources/catalog.bom
index 9adc45d..1dfba42 100644
--- a/karaf/init/src/main/resources/catalog.bom
+++ b/karaf/init/src/main/resources/catalog.bom
@@ -236,6 +236,11 @@
     item:
       type: org.apache.brooklyn.core.workflow.steps.appmodel.UpdateChildrenWorkflowStep
 
+  - id: shell
+    format: java-type-name
+    itemType: bean
+    item:
+      type: org.apache.brooklyn.core.workflow.steps.external.ShellWorkflowStep
   - id: ssh
     format: java-type-name
     itemType: bean
diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ScriptResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ScriptResource.java
index fa1aa09..83356a5 100644
--- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ScriptResource.java
+++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ScriptResource.java
@@ -46,8 +46,12 @@
     @SuppressWarnings("rawtypes")
     @Override
     public ScriptExecutionSummary groovy(HttpServletRequest request, String script) {
-        if (!Entitlements.isEntitled(mgmt().getEntitlementManager(), Entitlements.EXECUTE_GROOVY_SCRIPT, null)) {
-            throw WebResourceUtils.forbidden("User '%s' is not authorized to perform this operation", Entitlements.getEntitlementContext().user());
+        boolean entitledGS = Entitlements.isEntitled(mgmt().getEntitlementManager(), Entitlements.EXECUTE_GROOVY_SCRIPT, null);
+        boolean entitledS = Entitlements.isEntitled(mgmt().getEntitlementManager(), Entitlements.EXECUTE_SCRIPT, null);
+
+        if (!entitledS || !entitledGS) {
+            if (entitledGS) log.warn("User '"+Entitlements.getEntitlementContext().user()+"' is entitled to run groovy scripts but not scripts. The two permissions should be equivalent and the former may be removed, blocking access. Either grant permission to execute scripts or remove permission to execute groovy scripts.");
+            else throw WebResourceUtils.forbidden("User '%s' is not authorized to perform this operation", Entitlements.getEntitlementContext().user());
         }
 
         log.info("Web REST executing user-supplied script");