diff --git a/frontend/resources/vue/utils/rerunArguments.js b/frontend/resources/vue/utils/rerunArguments.js index 82499e9..180711b 100644 --- a/frontend/resources/vue/utils/rerunArguments.js +++ b/frontend/resources/vue/utils/rerunArguments.js @@ -1,5 +1,14 @@ import { needsArgumentForm } from './needsArgumentForm.js' +const nonStorableArgumentTypes = new Set([ + 'password', + 'very_dangerous_raw_string' +]) + +function isNonStorableArgumentType (type) { + return nonStorableArgumentTypes.has(type) +} + export function logEntryArgumentsToStartActionArgs (logEntry) { return (logEntry?.arguments ?? []).map((arg) => ({ name: arg.name, @@ -23,7 +32,7 @@ export function hasMissingRerunArguments (action, storedArgs) { const stored = new Map(storedArgs.map((arg) => [arg.name, arg.value])) for (const arg of action?.arguments ?? []) { - if (arg.type === 'password') { + if (isNonStorableArgumentType(arg.type)) { return true } diff --git a/frontend/resources/vue/utils/rerunArguments.test.mjs b/frontend/resources/vue/utils/rerunArguments.test.mjs index b5725f2..1caf5e7 100644 --- a/frontend/resources/vue/utils/rerunArguments.test.mjs +++ b/frontend/resources/vue/utils/rerunArguments.test.mjs @@ -37,6 +37,20 @@ test('hasMissingRerunArguments requires password fields to be re-entered', () => ) }) +test('hasMissingRerunArguments requires very_dangerous_raw_string fields to be re-entered', () => { + const action = { + arguments: [ + { name: 'host', type: 'ascii_identifier', required: true }, + { name: 'payload', type: 'very_dangerous_raw_string', required: false } + ] + } + + assert.equal( + hasMissingRerunArguments(action, [{ name: 'host', value: 'db-1' }]), + true + ) +}) + test('hasMissingRerunArguments detects missing required stored arguments', () => { const action = { arguments: [{ name: 'host', type: 'ascii_identifier', required: true }] diff --git a/service/internal/api/api.go b/service/internal/api/api.go index 736d639..2439a5d 100644 --- a/service/internal/api/api.go +++ b/service/internal/api/api.go @@ -7,7 +7,6 @@ import ( "os" "path" "sort" - "strings" "connectrpc.com/connect" "google.golang.org/protobuf/encoding/protojson" @@ -1550,8 +1549,8 @@ func (api *oliveTinAPI) RestartAction(ctx ctx.Context, req *connect.Request[apiv return nil, err } - if execReqLogEntry.Binding.Action.Justification && strings.TrimSpace(execReqLogEntry.Justification) == "" { - return nil, restartRequiresJustificationError() + if err := validateRestartLogEntry(execReqLogEntry); err != nil { + return nil, err } authenticatedUser := auth.UserFromApiCall(ctx, req, api.cfg) diff --git a/service/internal/api/api_log_arguments.go b/service/internal/api/api_log_arguments.go index 442cc89..b085d03 100644 --- a/service/internal/api/api_log_arguments.go +++ b/service/internal/api/api_log_arguments.go @@ -1,9 +1,14 @@ package api import ( + "fmt" "sort" + "strings" + + "connectrpc.com/connect" apiv1 "github.com/OliveTin/OliveTin/gen/olivetin/api/v1" + "github.com/OliveTin/OliveTin/internal/executor" ) func logEntryArgumentsToProto(args map[string]string) []*apiv1.StartActionArgument { @@ -36,3 +41,19 @@ func copyStringMap(source map[string]string) map[string]string { return copied } + +func restartArgumentsIncompleteError() error { + return connect.NewError(connect.CodeInvalidArgument, fmt.Errorf("stored arguments are incomplete for restart; use StartAction with the required arguments instead")) +} + +func validateRestartLogEntry(entry *executor.InternalLogEntry) error { + if entry.Binding.Action.Justification && strings.TrimSpace(entry.Justification) == "" { + return restartRequiresJustificationError() + } + + if executor.RestartArgumentsIncomplete(entry.Binding.Action, entry.Binding.Entity, entry.Arguments) { + return restartArgumentsIncompleteError() + } + + return nil +} diff --git a/service/internal/api/api_log_arguments_test.go b/service/internal/api/api_log_arguments_test.go index 68ca4e5..a1924cf 100644 --- a/service/internal/api/api_log_arguments_test.go +++ b/service/internal/api/api_log_arguments_test.go @@ -158,6 +158,75 @@ func TestRestartActionReusesStoredArguments(t *testing.T) { assert.Equal(t, "server-a", restartedArgs["host"]) } +func TestRestartActionRejectsIncompleteStoredArguments(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Actions = []*config.Action{ + { + Title: "Connect", + Exec: []string{"echo", "{{ user }}"}, + MaxConcurrent: 1, + Arguments: []config.ActionArgument{ + {Name: "user", Type: "ascii_identifier"}, + {Name: "pass", Type: "password"}, + }, + }, + } + + ex := executor.DefaultExecutor(cfg) + ex.RebuildActionMap() + binding := ex.FindBindingWithNoEntity(cfg.Actions[0]) + require.NotNil(t, binding) + + ts, client := getNewTestServerAndClientWithExecutor(cfg, ex) + defer ts.Close() + + startResp, err := client.StartAction(context.Background(), connect.NewRequest(&apiv1.StartActionRequest{ + BindingId: binding.ID, + Arguments: []*apiv1.StartActionArgument{ + {Name: "user", Value: "alice"}, + {Name: "pass", Value: "secret"}, + }, + })) + require.NoError(t, err) + + _, err = client.RestartAction(context.Background(), connect.NewRequest(&apiv1.RestartActionRequest{ + ExecutionTrackingId: startResp.Msg.ExecutionTrackingId, + })) + require.Error(t, err) + assert.Contains(t, err.Error(), "stored arguments are incomplete for restart") +} + +func TestRestartActionRejectsMissingRequiredStoredArguments(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Actions = []*config.Action{ + argumentAction("Ping host", "echo {{ host }}", []config.ActionArgument{ + {Name: "host", Type: "ascii_identifier"}, + }), + } + + ex := executor.DefaultExecutor(cfg) + ex.RebuildActionMap() + binding := ex.FindBindingWithNoEntity(cfg.Actions[0]) + require.NotNil(t, binding) + + trackingID := "a1b2c3d4-e5f6-7890-abcd-ef1234567890" + ex.SetLog(trackingID, &executor.InternalLogEntry{ + Binding: binding, + ExecutionFinished: true, + ExecutionTrackingID: trackingID, + Arguments: map[string]string{}, + }) + + ts, client := getNewTestServerAndClientWithExecutor(cfg, ex) + defer ts.Close() + + _, err := client.RestartAction(context.Background(), connect.NewRequest(&apiv1.RestartActionRequest{ + ExecutionTrackingId: trackingID, + })) + require.Error(t, err) + assert.Contains(t, err.Error(), "stored arguments are incomplete for restart") +} + func TestLogEntryArgumentsToProto(t *testing.T) { assert.Nil(t, logEntryArgumentsToProto(nil)) assert.Nil(t, logEntryArgumentsToProto(map[string]string{})) diff --git a/service/internal/config/config_reloader_test.go b/service/internal/config/config_reloader_test.go index a8adf76..1ee2050 100644 --- a/service/internal/config/config_reloader_test.go +++ b/service/internal/config/config_reloader_test.go @@ -113,6 +113,15 @@ func loadConfigFromYAML(t *testing.T, yamlStr string, cfg *Config) bool { } func TestEnvInConfig(t *testing.T) { + originalInput, hadInput := os.LookupEnv("INPUT") + t.Cleanup(func() { + if hadInput { + setTestEnvInput(t, originalInput) + return + } + setTestEnvInput(t, "") + }) + for _, tt := range envConfigTests { cfg := DefaultConfig() setTestEnvInput(t, tt.input) diff --git a/service/internal/executor/log_arguments.go b/service/internal/executor/log_arguments.go index 0c71b30..c207d0b 100644 --- a/service/internal/executor/log_arguments.go +++ b/service/internal/executor/log_arguments.go @@ -4,6 +4,8 @@ import ( "strings" config "github.com/OliveTin/OliveTin/internal/config" + "github.com/OliveTin/OliveTin/internal/entities" + "github.com/OliveTin/OliveTin/internal/tpl" ) func argumentTypeStorableInLog(argType string) bool { @@ -93,3 +95,47 @@ func copyStorableArgumentsToLogEntry(req *ExecutionRequest) { entry.Arguments = args }) } + +func restartArgumentMissingFromStored(arg *config.ActionArgument, entity *entities.Entity, storedArgs map[string]string) bool { + if !argumentTypeStorableInLog(arg.Type) { + return true + } + + if !restartArgumentRequired(arg, entity) { + return false + } + + if storedArgs == nil { + return true + } + + _, ok := storedArgs[arg.Name] + return !ok +} + +func RestartArgumentsIncomplete(action *config.Action, entity *entities.Entity, storedArgs map[string]string) bool { + if action == nil { + return false + } + + for i := range action.Arguments { + if restartArgumentMissingFromStored(&action.Arguments[i], entity, storedArgs) { + return true + } + } + + return false +} + +func restartArgumentRequired(arg *config.ActionArgument, entity *entities.Entity) bool { + if argumentSkipsValidation(arg) { + return false + } + + defaultValue := arg.Default + if defaultValue != "" { + defaultValue = tpl.ParseTemplateOfActionBeforeExec(defaultValue, entity) + } + + return defaultValue == "" +} diff --git a/service/internal/executor/log_arguments_test.go b/service/internal/executor/log_arguments_test.go index 9a3ef32..6078f5b 100644 --- a/service/internal/executor/log_arguments_test.go +++ b/service/internal/executor/log_arguments_test.go @@ -119,3 +119,39 @@ func TestExecRequestStoresArgumentsOnLogEntry(t *testing.T) { require.NotNil(t, logEntry.Arguments) assert.Equal(t, "yourself", logEntry.Arguments["person"]) } + +func TestRestartArgumentsIncompleteDetectsNonStorableArguments(t *testing.T) { + action := &config.Action{ + Arguments: []config.ActionArgument{ + {Name: "host", Type: "ascii_identifier"}, + {Name: "pass", Type: "password"}, + }, + } + + assert.True(t, RestartArgumentsIncomplete(action, nil, map[string]string{ + "host": "db-1", + })) +} + +func TestRestartArgumentsIncompleteDetectsMissingRequiredStoredArguments(t *testing.T) { + action := &config.Action{ + Arguments: []config.ActionArgument{ + {Name: "host", Type: "ascii_identifier"}, + }, + } + + assert.True(t, RestartArgumentsIncomplete(action, nil, map[string]string{})) + assert.False(t, RestartArgumentsIncomplete(action, nil, map[string]string{ + "host": "db-1", + })) +} + +func TestRestartArgumentsIncompleteAllowsOptionalArgumentsWithDefaults(t *testing.T) { + action := &config.Action{ + Arguments: []config.ActionArgument{ + {Name: "host", Type: "ascii_identifier", Default: "example.com"}, + }, + } + + assert.False(t, RestartArgumentsIncomplete(action, nil, map[string]string{})) +}