Skip to content

Commit f264426

Browse files
authored
[Fix #1604] Shell refactor (#1605)
* [Fix #1604] Shell refactor Signed-off-by: Francisco Javier Tirado Sarti <ftirados@ibm.com> * [Fix #1604] Zanini comments Signed-off-by: Francisco Javier Tirado Sarti <ftirados@ibm.com> --------- Signed-off-by: Francisco Javier Tirado Sarti <ftirados@ibm.com>
1 parent f92c103 commit f264426

18 files changed

Lines changed: 218 additions & 246 deletions

File tree

‎impl/core/src/main/java/io/serverlessworkflow/impl/WorkflowApplication.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
import java.util.Optional;
7171
import java.util.ServiceLoader;
7272
import java.util.ServiceLoader.Provider;
73+
import java.util.Set;
7374
import java.util.concurrent.ConcurrentHashMap;
7475
import java.util.concurrent.ExecutorService;
7576
import java.util.concurrent.Executors;
@@ -112,6 +113,7 @@ public class WorkflowApplication implements AutoCloseable {
112113
private final AllStrategyCorrelationInfoFactory allStrategyCorrelationInfoFactory;
113114
private final WorkflowLifeCycleCloudEventFactory lifeCycleCloudEventFactory;
114115
private final ScheduledExecutorService schedulerExecutorService;
116+
private final Set<String> allowedCommands;
115117

116118
private WorkflowApplication(Builder builder) {
117119
this.taskFactory = builder.taskFactory;
@@ -147,6 +149,7 @@ private WorkflowApplication(Builder builder) {
147149
this.allStrategyCorrelationInfoFactory = builder.allStrategyCorrelationInfoFactory;
148150
this.lifeCycleCloudEventFactory = builder.lifeCycleCloudEventFactory;
149151
this.schedulerExecutorService = builder.schedulerExecutorService;
152+
this.allowedCommands = Collections.unmodifiableSet(builder.allowedCommands);
150153
}
151154

152155
public TaskExecutorFactory taskFactory() {
@@ -274,6 +277,7 @@ public SchemaValidator getValidator(SchemaInline inline) {
274277
private WorkflowLifeCycleCloudEventFactory lifeCycleCloudEventFactory;
275278
private CronResolverFactory cronResolverFactory;
276279
private ScheduledExecutorService schedulerExecutorService;
280+
private Set<String> allowedCommands = new HashSet<>();
277281

278282
private Builder() {
279283
ServiceLoader.load(NamedWorkflowAdditionalObject.class)
@@ -396,6 +400,16 @@ public Builder withModelFactory(WorkflowModelFactory modelFactory) {
396400
return this;
397401
}
398402

403+
public Builder withAllowedCommand(String command) {
404+
this.allowedCommands.add(command);
405+
return this;
406+
}
407+
408+
public Builder withAllowedCommands(Collection<String> commands) {
409+
this.allowedCommands.addAll(commands);
410+
return this;
411+
}
412+
399413
public Builder withContextFactory(WorkflowModelFactory contextFactory) {
400414
this.contextFactory = contextFactory;
401415
return this;
@@ -690,4 +704,8 @@ public AllStrategyCorrelationInfoFactory allStrategyCorrelationInfoFactory() {
690704
public WorkflowLifeCycleCloudEventFactory lifeCycleCloudEventFactory() {
691705
return lifeCycleCloudEventFactory;
692706
}
707+
708+
public Set<String> allowedCommands() {
709+
return allowedCommands;
710+
}
693711
}

‎impl/core/src/main/java/io/serverlessworkflow/impl/executors/RunShellExecutor.java‎

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,23 +23,23 @@
2323
import io.serverlessworkflow.impl.WorkflowModel;
2424
import io.serverlessworkflow.impl.WorkflowValueResolver;
2525
import io.serverlessworkflow.impl.scripts.ScriptUtils;
26+
import java.util.ArrayList;
27+
import java.util.List;
2628
import java.util.Map;
2729
import java.util.Optional;
2830
import java.util.concurrent.CompletableFuture;
2931

3032
public class RunShellExecutor implements CallableTask {
3133
private final WorkflowValueResolver<String> shellCommand;
32-
private final Map<WorkflowValueResolver<String>, Optional<WorkflowValueResolver<String>>>
33-
shellArguments;
34+
private final List<WorkflowValueResolver<String>> shellArguments;
3435
private final Optional<WorkflowValueResolver<Map<String, Object>>> shellEnv;
3536
private final Optional<ProcessReturnType> returnType;
3637

3738
public RunShellExecutor(
3839
WorkflowValueResolver<String> shellCommand,
39-
Map<WorkflowValueResolver<String>, Optional<WorkflowValueResolver<String>>> shellArguments,
40+
List<WorkflowValueResolver<String>> shellArguments,
4041
Optional<WorkflowValueResolver<Map<String, Object>>> shellEnv,
4142
Optional<ProcessReturnType> returnType) {
42-
super();
4343
this.shellCommand = shellCommand;
4444
this.shellArguments = shellArguments;
4545
this.shellEnv = shellEnv;
@@ -49,16 +49,21 @@ public RunShellExecutor(
4949
@Override
5050
public CompletableFuture<WorkflowModel> apply(
5151
WorkflowContext workflowContext, TaskContext taskContext, WorkflowModel model) {
52-
StringBuilder commandBuilder =
53-
new StringBuilder(shellCommand.apply(workflowContext, taskContext, model));
54-
for (var entry : shellArguments.entrySet()) {
55-
commandBuilder.append(" ").append(entry.getKey().apply(workflowContext, taskContext, model));
56-
entry
57-
.getValue()
58-
.ifPresent(
59-
v -> commandBuilder.append("=").append(v.apply(workflowContext, taskContext, model)));
52+
53+
String command = shellCommand.apply(workflowContext, taskContext, model);
54+
if (!workflowContext.definition().application().allowedCommands().contains(command)) {
55+
return CompletableFuture.failedFuture(
56+
new SecurityException(
57+
"Command "
58+
+ command
59+
+ " is not allowed. Please verify the set of allowed commands passed to the application"));
6060
}
61-
ProcessBuilder builder = new ProcessBuilder("sh", "-c", commandBuilder.toString());
61+
62+
List<String> commandAndArgs = new ArrayList<>();
63+
commandAndArgs.add(command);
64+
shellArguments.forEach(f -> commandAndArgs.add(f.apply(workflowContext, taskContext, model)));
65+
66+
ProcessBuilder builder = new ProcessBuilder(commandAndArgs);
6267
shellEnv.ifPresent(
6368
map -> ScriptUtils.addEnviromment(builder, map.apply(workflowContext, taskContext, model)));
6469

‎impl/core/src/main/java/io/serverlessworkflow/impl/executors/RunShellExecutorBuilder.java‎

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,8 @@
2020
import io.serverlessworkflow.api.types.Shell;
2121
import io.serverlessworkflow.impl.WorkflowDefinition;
2222
import io.serverlessworkflow.impl.WorkflowUtils;
23-
import java.util.LinkedHashMap;
24-
import java.util.Map;
23+
import java.util.List;
2524
import java.util.Optional;
26-
import java.util.stream.Collectors;
2725

2826
public class RunShellExecutorBuilder implements RunnableTaskBuilder<RunShell> {
2927

@@ -37,19 +35,10 @@ public CallableTask build(RunShell taskConfiguration, WorkflowDefinition definit
3735
WorkflowUtils.buildStringFilter(
3836
definition.application(), taskConfiguration.getShell().getCommand()),
3937
shell.getArguments() != null
40-
? shell.getArguments().getAdditionalProperties().entrySet().stream()
41-
.collect(
42-
Collectors.toMap(
43-
e -> WorkflowUtils.buildStringFilter(definition.application(), e.getKey()),
44-
e ->
45-
e.getValue() != null
46-
? Optional.of(
47-
WorkflowUtils.buildStringFilter(
48-
definition.application(), e.getValue().toString()))
49-
: Optional.empty(),
50-
(x, y) -> y,
51-
LinkedHashMap::new))
52-
: Map.of(),
38+
? shell.getArguments().stream()
39+
.map(s -> WorkflowUtils.buildStringFilter(definition.application(), s))
40+
.toList()
41+
: List.of(),
5342
shell.getEnvironment() != null
5443
? Optional.of(
5544
WorkflowUtils.buildMapResolver(

0 commit comments

Comments
 (0)