diff --git a/service/internal/entities/entities.go b/service/internal/entities/entities.go index 0cbd7d9..6be2b7d 100644 --- a/service/internal/entities/entities.go +++ b/service/internal/entities/entities.go @@ -104,7 +104,6 @@ func loadEntityFileJson(filename string, entityname string) { if err != nil { log.Errorf("ReadIn: %v", err) - ClearEntitiesOfType(entityname) return } @@ -119,7 +118,6 @@ func loadEntityFileJson(filename string, entityname string) { if err != nil { log.Errorf("%v", err) - ClearEntitiesOfType(entityname) return } @@ -139,7 +137,6 @@ func loadEntityFileYaml(filename string, entityname string) { if err != nil { log.Errorf("ReadIn: %v", err) - ClearEntitiesOfType(entityname) return } @@ -149,7 +146,6 @@ func loadEntityFileYaml(filename string, entityname string) { if err != nil { log.Errorf("Unmarshal: %v", err) - ClearEntitiesOfType(entityname) return } diff --git a/service/internal/entities/entities_test.go b/service/internal/entities/entities_test.go index b13fd08..deb411b 100644 --- a/service/internal/entities/entities_test.go +++ b/service/internal/entities/entities_test.go @@ -1,6 +1,8 @@ package entities import ( + "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -59,3 +61,34 @@ func TestGetEntityInstancesOrdered_emptyOrMissing(t *testing.T) { ordered = GetEntityInstancesOrdered("empty_test") assert.Nil(t, ordered) } + +func TestLoadEntityFile_preservesEntitiesOnTransientFailure(t *testing.T) { + const entityName = "preserve_on_fail" + ClearEntitiesOfType(entityName) + defer ClearEntitiesOfType(entityName) + + dir := t.TempDir() + yamlPath := filepath.Join(dir, "hosts.yaml") + require.NoError(t, os.WriteFile(yamlPath, []byte("- title: kept\n"), 0o600)) + + loadEntityFile(yamlPath, entityName) + require.Len(t, GetEntityInstancesOrdered(entityName), 1) + + loadEntityFile(filepath.Join(dir, "missing.yaml"), entityName) + require.Len(t, GetEntityInstancesOrdered(entityName), 1, "read failure should keep last good entities") + + require.NoError(t, os.WriteFile(yamlPath, []byte("not: valid: yaml: ["), 0o600)) + loadEntityFile(yamlPath, entityName) + require.Len(t, GetEntityInstancesOrdered(entityName), 1, "parse failure should keep last good entities") + + jsonPath := filepath.Join(dir, "hosts.json") + require.NoError(t, os.WriteFile(jsonPath, []byte("{\"title\":\"json-kept\"}\n"), 0o600)) + loadEntityFile(jsonPath, entityName) + require.Len(t, GetEntityInstancesOrdered(entityName), 1) + + require.NoError(t, os.WriteFile(jsonPath, []byte("{bad json"), 0o600)) + loadEntityFile(jsonPath, entityName) + ordered := GetEntityInstancesOrdered(entityName) + require.Len(t, ordered, 1, "JSON parse failure should keep last good entities") + assert.Equal(t, "json-kept", ordered[0].Title) +} diff --git a/service/internal/executor/executor.go b/service/internal/executor/executor.go index 08dca61..08b9328 100644 --- a/service/internal/executor/executor.go +++ b/service/internal/executor/executor.go @@ -1391,6 +1391,11 @@ func triggerLoop(req *ExecutionRequest) { } func stepSaveLog(req *ExecutionRequest) bool { + if !canSaveExecutionLog(req) { + log.Warnf("Cannot save execution log; missing request, log entry, binding/action, or config") + return false + } + filename := fmt.Sprintf("%v.%v.%v", sanitizeLogFilename(req.logEntry.ActionTitle), req.logEntry.DatetimeStarted.Unix(), req.logEntry.ExecutionTrackingID) saveLogResults(req, filename) @@ -1399,10 +1404,14 @@ func stepSaveLog(req *ExecutionRequest) bool { return true } +func canSaveExecutionLog(req *ExecutionRequest) bool { + return req != nil && req.logEntry != nil && req.Binding != nil && req.Binding.Action != nil && req.Cfg != nil +} + // sanitizeLogFilename replaces characters that are unsafe in filenames so action // titles like "Create/update Report" do not create nested paths or fail to write. func sanitizeLogFilename(title string) string { - replacer := strings.NewReplacer( + oldnew := []string{ "/", "_", "\\", "_", ":", "_", @@ -1412,9 +1421,15 @@ func sanitizeLogFilename(title string) string { "<", "_", ">", "_", "|", "_", - ) + } - return replacer.Replace(title) + // NUL and other C0 controls plus DEL are invalid or problematic in filenames. + for i := 0; i < 32; i++ { + oldnew = append(oldnew, string(rune(i)), "_") + } + oldnew = append(oldnew, "\x7f", "_") + + return strings.NewReplacer(oldnew...).Replace(title) } func firstNonEmpty(one, two string) string { diff --git a/service/internal/executor/executor_test.go b/service/internal/executor/executor_test.go index b2fed8c..1869a83 100644 --- a/service/internal/executor/executor_test.go +++ b/service/internal/executor/executor_test.go @@ -790,6 +790,8 @@ func TestSanitizeLogFilename(t *testing.T) { {"Create/update Monthly Report", "Create_update Monthly Report"}, {`path\with\backslashes`, "path_with_backslashes"}, {`a:b*c?d"eg|h`, "a_b_c_d_e_f_g_h"}, + {"has\x00nul", "has_nul"}, + {"tab\there\nand\rreturn", "tab_here_and_return"}, } for _, tt := range tests { @@ -870,3 +872,78 @@ func TestStepSaveLogKeepsSafeTitleFilename(t *testing.T) { expectedPath := filepath.Join(resultsDir, "Echo Test.1714333384."+trackingID+".yaml") assert.FileExists(t, expectedPath) } + +func TestStepSaveLogSanitizesNULInTitle(t *testing.T) { + resultsDir := t.TempDir() + outputDir := t.TempDir() + started := time.Unix(1714333384, 0) + trackingID := "bbbbbbbb-cccc-dddd-eeee-ffffffffffff" + title := "Bad\x00Title" + + req := &ExecutionRequest{ + Cfg: &config.Config{ + SaveLogs: config.SaveLogsConfig{ + ResultsDirectory: resultsDir, + OutputDirectory: outputDir, + }, + }, + Binding: &ActionBinding{ + Action: &config.Action{}, + }, + logEntry: &InternalLogEntry{ + ActionTitle: title, + DatetimeStarted: started, + ExecutionTrackingID: trackingID, + Output: "nul ok", + }, + } + + assert.True(t, stepSaveLog(req)) + + expectedBase := "Bad_Title.1714333384." + trackingID + resultsPath := filepath.Join(resultsDir, expectedBase+".yaml") + outputPath := filepath.Join(outputDir, expectedBase+".log") + + assert.FileExists(t, resultsPath) + assert.FileExists(t, outputPath) + assert.NotContains(t, resultsPath, "\x00") + assert.NotContains(t, outputPath, "\x00") + + output, err := os.ReadFile(outputPath) + assert.NoError(t, err) + assert.Equal(t, "nul ok", string(output)) +} + +func TestStepSaveLogReturnsFalseWhenDependenciesMissing(t *testing.T) { + started := time.Unix(1714333384, 0) + valid := &ExecutionRequest{ + Cfg: &config.Config{}, + Binding: &ActionBinding{ + Action: &config.Action{}, + }, + logEntry: &InternalLogEntry{ + ActionTitle: "Echo", + DatetimeStarted: started, + ExecutionTrackingID: "cccccccc-dddd-eeee-ffff-000000000000", + }, + } + + assert.False(t, stepSaveLog(nil)) + assert.False(t, stepSaveLog(&ExecutionRequest{})) + + missingLog := *valid + missingLog.logEntry = nil + assert.False(t, stepSaveLog(&missingLog)) + + missingBinding := *valid + missingBinding.Binding = nil + assert.False(t, stepSaveLog(&missingBinding)) + + missingAction := *valid + missingAction.Binding = &ActionBinding{} + assert.False(t, stepSaveLog(&missingAction)) + + missingCfg := *valid + missingCfg.Cfg = nil + assert.False(t, stepSaveLog(&missingCfg)) +}