diff --git a/docs/modules/ROOT/pages/args/input_confirmation.adoc b/docs/modules/ROOT/pages/args/input_confirmation.adoc index 1209f22..edeb3da 100644 --- a/docs/modules/ROOT/pages/args/input_confirmation.adoc +++ b/docs/modules/ROOT/pages/args/input_confirmation.adoc @@ -3,6 +3,8 @@ The `confirmation` type argument is a special argument type, which simply disables the "Start" button until a checkbox is ticked. This can be useful if you have an action with no other arguments, but you want to prevent accidental button-clicks starting the action. +Confirmation arguments are usually left unnamed, so nothing is substituted into the command. If you do give the argument a `name`, the only allowed values are `0` (unchecked) and `1` (checked), so confirmation is safe to use with `shell:` actions. + [source,yaml] ---- actions: diff --git a/docs/modules/ROOT/pages/args/types.adoc b/docs/modules/ROOT/pages/args/types.adoc index 272db8e..e9e4cb5 100644 --- a/docs/modules/ROOT/pages/args/types.adoc +++ b/docs/modules/ROOT/pages/args/types.adoc @@ -20,7 +20,7 @@ A full list of argument types are below; | regex:... | xref:args/input.adoc[Textbox] | Version 2024.03.081 and above support custom regex patterns. See xref:args/regex.adoc[Custom regex arguments]. | int | xref:args/input.adoc[Textbox] | Any number, made up of the characters 0 to 9. Negative numbers are not supported. | url | xref:args/input.adoc[Textbox] | A URL (e.g. https://example.com). Accepts any scheme, including `file://` and `ftp://`. See warning below. -| confirmation | xref:args/input_confirmation.adoc[Confirmation] | A "hidden" argument that makes the action require a confirmation before launching. +| confirmation | xref:args/input_confirmation.adoc[Confirmation] | A UI gate that requires a checkbox before starting. Usually unnamed (nothing is substituted). If named, the value is only `0` or `1`. | checklist | xref:args/input_checklist.adoc[Checklist] | Multiple checkboxes from predefined choices. Selected values are passed as a comma-separated string. | n/a, but `choices` used | xref:args/input_dropdown.adoc[Dropdown] | A "hidden" argument that makes the action require a confirmation before launching. | raw_string_multiline | xref:args/input_textarea.adoc[Textarea] | Anything. This is **dangerous**, as effectively people can type anything they like diff --git a/service/internal/executor/arguments.go b/service/internal/executor/arguments.go index e9ff778..53ce43f 100644 --- a/service/internal/executor/arguments.go +++ b/service/internal/executor/arguments.go @@ -150,12 +150,7 @@ func redactExecArgs(execArgs []string, arguments []config.ActionArgument, argume } func argumentSkipsValidation(arg *config.ActionArgument) bool { - switch arg.Type { - case "confirmation", "html": - return true - default: - return false - } + return arg.Type == "html" } func typecheckActionArgument(arg *config.ActionArgument, value string, action *config.Action) error { @@ -163,6 +158,10 @@ func typecheckActionArgument(arg *config.ActionArgument, value string, action *c return nil } + if arg.Type == "confirmation" { + return typecheckConfirmation(arg, value) + } + if arg.Name == "" { return fmt.Errorf("argument name cannot be empty") } @@ -170,6 +169,20 @@ func typecheckActionArgument(arg *config.ActionArgument, value string, action *c return typecheckActionArgumentFound(value, arg) } +// typecheckConfirmation allows unnamed confirmation args as UI-only gates. +// Named confirmation values are only ever "0" or "1", matching the web UI. +func typecheckConfirmation(arg *config.ActionArgument, value string) error { + if arg.Name == "" { + return nil + } + + if value == "0" || value == "1" { + return nil + } + + return fmt.Errorf("argument %q of type confirmation must be \"0\" or \"1\"", arg.Name) +} + // ValidateArgument validates a single argument value using the same logic as the executor. // It applies mangling transformations and performs full validation including null checks, // choice validation, and type safety checks. @@ -403,7 +416,6 @@ var shellUnsafeArgumentTypes = map[string]struct{}{ "very_dangerous_raw_string": {}, "password": {}, "html": {}, - "confirmation": {}, } func isUnsafeShellArgumentType(arg *config.ActionArgument) bool { diff --git a/service/internal/executor/arguments_test.go b/service/internal/executor/arguments_test.go index 6d28626..23ae6f5 100644 --- a/service/internal/executor/arguments_test.go +++ b/service/internal/executor/arguments_test.go @@ -523,8 +523,20 @@ func TestCheckShellArgumentSafetyWithConfirmation(t *testing.T) { } err := checkShellArgumentSafety(&a1) - assert.NotNil(t, err) - assert.Contains(t, err.Error(), "unsafe argument type 'confirmation'") + assert.Nil(t, err, "confirmation is constrained to 0/1 and is safe with shell") +} + +func TestCheckShellArgumentSafetyWithUnnamedConfirmation(t *testing.T) { + a1 := config.Action{ + Title: "Confirm shell unnamed", + Shell: "echo ok", + Arguments: []config.ActionArgument{ + {Type: "confirmation", Title: "Are you sure?!"}, + }, + } + + err := checkShellArgumentSafety(&a1) + assert.Nil(t, err) } func TestCheckShellArgumentSafetyWithChoicelessCheckbox(t *testing.T) { @@ -930,8 +942,27 @@ func TestTypecheckActionArgumentConfirmation(t *testing.T) { } action := config.Action{Title: "Test"} + assert.Nil(t, typecheckActionArgument(&arg, "0", &action)) + assert.Nil(t, typecheckActionArgument(&arg, "1", &action)) + err := typecheckActionArgument(&arg, "any_value", &action) - assert.Nil(t, err, "Confirmation type should always pass validation") + assert.NotNil(t, err) + assert.Contains(t, err.Error(), "must be \"0\" or \"1\"") + + err = typecheckActionArgument(&arg, "", &action) + assert.NotNil(t, err) + assert.Contains(t, err.Error(), "must be \"0\" or \"1\"") +} + +func TestTypecheckActionArgumentUnnamedConfirmation(t *testing.T) { + arg := config.ActionArgument{ + Type: "confirmation", + Title: "Are you sure?!", + } + action := config.Action{Title: "Test"} + + assert.Nil(t, typecheckActionArgument(&arg, "", &action)) + assert.Nil(t, typecheckActionArgument(&arg, "ignored", &action)) } func TestTypecheckActionArgumentHtmlWithoutName(t *testing.T) { diff --git a/service/internal/executor/executor.go b/service/internal/executor/executor.go index 08b9328..bbe421c 100644 --- a/service/internal/executor/executor.go +++ b/service/internal/executor/executor.go @@ -1097,6 +1097,7 @@ func appendErrorToStderr(req *ExecutionRequest, err error) { type OutputStreamer struct { Req *ExecutionRequest + mu sync.Mutex output bytes.Buffer } @@ -1105,10 +1106,31 @@ func (ost *OutputStreamer) Write(o []byte) (n int, err error) { listener.OnOutputChunk(o, ost.Req.TrackingID) } - return ost.output.Write(o) + ost.mu.Lock() + n, err = ost.output.Write(o) + outputSoFar := "" + if err == nil { + outputSoFar = ost.output.String() + } + ost.mu.Unlock() + + if err != nil { + return n, err + } + + // Keep the log entry's Output in sync while the command is still running so + // ExecutionStatus / mid-run result views can show output produced so far. + ost.Req.mutateLogEntry(func(entry *InternalLogEntry) { + entry.Output = outputSoFar + }) + + return n, nil } func (ost *OutputStreamer) String() string { + ost.mu.Lock() + defer ost.mu.Unlock() + return ost.output.String() } @@ -1155,9 +1177,10 @@ func stepExec(req *ExecutionRequest) bool { }) ctx.setProcess(cmd.Process) waiterr := cmd.Wait() + finalOutput := streamer.String() req.mutateLogEntry(func(entry *InternalLogEntry) { entry.ExitCode = int32(commandExitCode(cmd)) - entry.Output = streamer.String() + entry.Output = finalOutput }) appendErrorToStderr(req, runerr) diff --git a/service/internal/executor/executor_test.go b/service/internal/executor/executor_test.go index 1869a83..bb40aa7 100644 --- a/service/internal/executor/executor_test.go +++ b/service/internal/executor/executor_test.go @@ -8,6 +8,7 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/OliveTin/OliveTin/internal/auth" authpublic "github.com/OliveTin/OliveTin/internal/auth/authpublic" @@ -947,3 +948,49 @@ func TestStepSaveLogReturnsFalseWhenDependenciesMissing(t *testing.T) { missingCfg.Cfg = nil assert.False(t, stepSaveLog(&missingCfg)) } + +func TestLogEntryOutputAvailableWhileRunning(t *testing.T) { + cfg := config.DefaultConfig() + e := DefaultExecutor(cfg) + action := &config.Action{ + Title: "Slow output", + Shell: "echo hello-mid-run; sleep 2", + } + cfg.Actions = append(cfg.Actions, action) + cfg.Sanitize() + e.RebuildActionMap() + + binding := e.FindBindingWithNoEntity(action) + require.NotNil(t, binding) + + wg, trackingID := e.ExecRequest(&ExecutionRequest{ + Binding: binding, + Cfg: cfg, + AuthenticatedUser: auth.UserFromSystem(cfg, "testuser"), + }) + + var sawOutputWhileRunning bool + require.Eventually(t, func() bool { + snapshot, ok := e.SnapshotLog(trackingID) + if !ok { + return false + } + if snapshot.ExecutionFinished { + return false + } + if strings.Contains(snapshot.Output, "hello-mid-run") { + sawOutputWhileRunning = true + return true + } + return false + }, 2*time.Second, 10*time.Millisecond) + + wg.Wait() + + require.True(t, sawOutputWhileRunning, "expected Output to contain printed text before ExecutionFinished") + + snapshot, ok := e.SnapshotLog(trackingID) + require.True(t, ok) + assert.True(t, snapshot.ExecutionFinished) + assert.Contains(t, snapshot.Output, "hello-mid-run") +} diff --git a/service/internal/executor/log_arguments.go b/service/internal/executor/log_arguments.go index c207d0b..1e21fb2 100644 --- a/service/internal/executor/log_arguments.go +++ b/service/internal/executor/log_arguments.go @@ -128,7 +128,8 @@ func RestartArgumentsIncomplete(action *config.Action, entity *entities.Entity, } func restartArgumentRequired(arg *config.ActionArgument, entity *entities.Entity) bool { - if argumentSkipsValidation(arg) { + // confirmation is a UI gate; html skips validation. Neither needs a stored value to restart. + if argumentSkipsValidation(arg) || arg.Type == "confirmation" { return false }