From 63d973f40c68a46af5a199f2b9ec3e9cc95d90a3 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Sun, 25 Oct 2026 03:45:08 +0000 Subject: [PATCH 1/2] Keep report CSV text from becoming spreadsheet formulas Escape formula-like external text in single-resource, fleet and VM inventory CSV exports, including saved inventory reports. Preserve field boundaries, numeric measurements, ordinary text and the runtime inventory. Contract-Neutral: Restores safe literal-text handling in existing report CSV cells; columns, numeric values, endpoints, authorization, archive formats and scheduling contracts are unchanged. Change-source: pulse-maintainer --- internal/api/reporting_csv_safety_test.go | 62 +++++++++++ pkg/reporting/csv.go | 28 ++--- pkg/reporting/csv_text.go | 33 ++++++ pkg/reporting/csv_text_test.go | 125 ++++++++++++++++++++++ pkg/reporting/vm_inventory.go | 14 +-- 5 files changed, 241 insertions(+), 21 deletions(-) create mode 100644 internal/api/reporting_csv_safety_test.go create mode 100644 pkg/reporting/csv_text.go create mode 100644 pkg/reporting/csv_text_test.go diff --git a/internal/api/reporting_csv_safety_test.go b/internal/api/reporting_csv_safety_test.go new file mode 100644 index 0000000000..9129be687f --- /dev/null +++ b/internal/api/reporting_csv_safety_test.go @@ -0,0 +1,62 @@ +package api + +import ( + "context" + "encoding/csv" + "net/http" + "net/http/httptest" + "os" + "strings" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/config" + "github.com/rcourtman/pulse-go-rewrite/internal/models" + "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" + "github.com/stretchr/testify/require" +) + +func TestReportingInventoryDownloadAndSavedScheduleEscapeSpreadsheetText(t *testing.T) { + monitor := newReportingMonitorForTest(t, models.NewState(), []unifiedresources.Resource{{ + ID: "vm-formula", Type: unifiedresources.ResourceTypeVM, Name: "=1+1", + Status: unifiedresources.StatusOnline, + Proxmox: &unifiedresources.ProxmoxData{ + Instance: "+1+1", NodeName: "\t@node", Pool: "@SUM(1,1)", VMID: 101, + CPUs: 2, DiskStatusReason: "\rreason", + }, + }}) + p := config.NewConfigPersistence(t.TempDir()) + setUnexportedField(t, monitor, "configPersist", p) + h := NewReportingHandlers(newReportingMTMForTest(t, monitor), nil) + ctx := context.WithValue(context.Background(), OrgIDContextKey, "default") + request := httptest.NewRequest(http.MethodGet, "/api/admin/reports/inventory/vms/export?format=csv", nil).WithContext(ctx) + download := httptest.NewRecorder() + h.HandleExportVMInventory(download, request) + require.Equal(t, http.StatusOK, download.Code) + require.Equal(t, "text/csv; charset=utf-8", download.Header().Get("Content-Type")) + require.Contains(t, download.Header().Get("Content-Disposition"), "attachment;") + records, err := csv.NewReader(strings.NewReader(download.Body.String())).ReadAll() + require.NoError(t, err) + require.Len(t, records, 2) + require.Equal(t, []string{ + "vm-formula", "'+1+1", "'\t@node", "'@SUM(1,1)", "101", "'=1+1", "online", "2", "0", "0", "0", "'\rreason", + }, records[1]) + + s := validReportSchedulePayload() + s.ID, s.Kind, s.Format = "safe-csv", config.ReportScheduleKindVMInventory, "csv" + s.Scope = config.ReportScheduleScope{} + now := time.Date(2026, 10, 25, 1, 0, 0, 0, time.UTC) + prepared, err := h.prepareReportSchedule(ctx, s, now, false) + require.NoError(t, err) + result, finished := h.runReportSchedule(ctx, p, prepared, now, true, "") + require.Equal(t, "ok", finished.LastRunStatus, finished.LastError) + require.Equal(t, "saved", finished.LastDeliveryStatus) + saved, err := os.ReadFile(result.path) + require.NoError(t, err) + require.Equal(t, download.Body.Bytes(), saved, "scheduled files must use the same safe CSV producer as downloads") + + snapshot, ok := h.getReportingEnrichmentSnapshot(ctx, "default") + require.True(t, ok) + require.Len(t, snapshot.Resources, 1) + require.Equal(t, "=1+1", snapshot.Resources[0].Name, "escape only the exported cell, never the monitored name") +} diff --git a/pkg/reporting/csv.go b/pkg/reporting/csv.go index 82f4405d1e..b988e45386 100644 --- a/pkg/reporting/csv.go +++ b/pkg/reporting/csv.go @@ -48,9 +48,9 @@ func (g *CSVGenerator) Generate(data *ReportData) ([]byte, error) { func (g *CSVGenerator) writeHeader(w *csv.Writer, data *ReportData) error { headers := [][]string{ {"# Pulse Metrics Report"}, - {"# Title:", data.Title}, - {"# Resource Type:", GetResourceTypeDisplayName(data.ResourceType)}, - {"# Resource ID:", data.ResourceID}, + {"# Title:", spreadsheetText(data.Title)}, + {"# Resource Type:", spreadsheetText(GetResourceTypeDisplayName(data.ResourceType))}, + {"# Resource ID:", spreadsheetText(data.ResourceID)}, {"# Period:", fmt.Sprintf("%s to %s", data.Start.Format(time.RFC3339), data.End.Format(time.RFC3339))}, {"# Generated:", data.GeneratedAt.Format(time.RFC3339)}, {"# Total Data Points:", fmt.Sprintf("%d", data.TotalPoints)}, @@ -98,7 +98,7 @@ func (g *CSVGenerator) writeSummary(w *csv.Writer, data *ReportData) error { stats := data.Summary.ByMetric[metricType] unit := GetMetricUnit(metricType) row := []string{ - GetMetricTypeDisplayName(metricType), + spreadsheetText(GetMetricTypeDisplayName(metricType)), fmt.Sprintf("%d", stats.Count), formatValue(stats.Min, unit), formatValue(stats.Max, unit), @@ -138,9 +138,9 @@ func (g *CSVGenerator) writeData(w *csv.Writer, data *ReportData) error { for _, name := range metricNames { unit := GetMetricUnit(name) if unit != "" { - headerRow = append(headerRow, fmt.Sprintf("%s (%s)", GetMetricTypeDisplayName(name), unit)) + headerRow = append(headerRow, spreadsheetText(fmt.Sprintf("%s (%s)", GetMetricTypeDisplayName(name), unit))) } else { - headerRow = append(headerRow, GetMetricTypeDisplayName(name)) + headerRow = append(headerRow, spreadsheetText(GetMetricTypeDisplayName(name))) } } if err := w.Write(headerRow); err != nil { @@ -196,7 +196,7 @@ func (g *CSVGenerator) GenerateMulti(data *MultiReportData) ([]byte, error) { // Write header comment rows headers := [][]string{ {"# Pulse Multi-Resource Metrics Report"}, - {"# Title:", data.Title}, + {"# Title:", spreadsheetText(data.Title)}, {"# Resources:", fmt.Sprintf("%d", len(data.Resources))}, {"# Period:", fmt.Sprintf("%s to %s", data.Start.Format(time.RFC3339), data.End.Format(time.RFC3339))}, {"# Generated:", data.GeneratedAt.Format(time.RFC3339)}, @@ -238,10 +238,10 @@ func (g *CSVGenerator) GenerateMulti(data *MultiReportData) ([]byte, error) { stats := rd.Summary.ByMetric[metricType] unit := GetMetricUnit(metricType) row := []string{ - resourceName, - resourceTypeDisplay, - rd.ResourceID, - GetMetricTypeDisplayName(metricType), + spreadsheetText(resourceName), + spreadsheetText(resourceTypeDisplay), + spreadsheetText(rd.ResourceID), + spreadsheetText(GetMetricTypeDisplayName(metricType)), fmt.Sprintf("%d", stats.Count), formatValue(stats.Min, unit), formatValue(stats.Max, unit), @@ -283,9 +283,9 @@ func (g *CSVGenerator) GenerateMulti(data *MultiReportData) ([]byte, error) { for _, name := range metricNames { unit := GetMetricUnit(name) if unit != "" { - headerRow = append(headerRow, fmt.Sprintf("%s (%s)", GetMetricTypeDisplayName(name), unit)) + headerRow = append(headerRow, spreadsheetText(fmt.Sprintf("%s (%s)", GetMetricTypeDisplayName(name), unit))) } else { - headerRow = append(headerRow, GetMetricTypeDisplayName(name)) + headerRow = append(headerRow, spreadsheetText(GetMetricTypeDisplayName(name))) } } if err := w.Write(headerRow); err != nil { @@ -343,7 +343,7 @@ func (g *CSVGenerator) GenerateMulti(data *MultiReportData) ([]byte, error) { // Write data rows for _, row := range allRows { t := time.Unix(row.timestamp, 0) - csvRow := []string{t.Format(time.RFC3339), row.resourceName, row.resourceType, row.resourceID} + csvRow := []string{t.Format(time.RFC3339), spreadsheetText(row.resourceName), spreadsheetText(row.resourceType), spreadsheetText(row.resourceID)} for _, metricName := range metricNames { if val, ok := row.values[metricName]; ok { csvRow = append(csvRow, fmt.Sprintf("%.2f", val)) diff --git a/pkg/reporting/csv_text.go b/pkg/reporting/csv_text.go new file mode 100644 index 0000000000..657b807c3b --- /dev/null +++ b/pkg/reporting/csv_text.go @@ -0,0 +1,33 @@ +package reporting + +import ( + "strings" + "unicode" +) + +// spreadsheetText keeps externally supplied text from becoming a spreadsheet +// formula. CSV quoting alone only protects field boundaries, not cell types. +// Prefix the original value (without trimming or rewriting it) with the usual +// spreadsheet text marker. Only text cells use this helper: generated numbers, +// including negative measurements, must remain numbers. +func spreadsheetText(value string) string { + if value == "" { + return value + } + // Some importers treat these leading controls specially, even before text. + switch value[0] { + case '\t', '\r', '\n': + return "'" + value + } + // Importers may ignore whitespace or a byte-order mark before a formula. + leading := strings.TrimLeftFunc(value, func(r rune) bool { + return unicode.IsSpace(r) || r == '\uFEFF' + }) + if strings.HasPrefix(leading, "=") || strings.HasPrefix(leading, "+") || + strings.HasPrefix(leading, "-") || strings.HasPrefix(leading, "@") || + strings.HasPrefix(leading, "＝") || strings.HasPrefix(leading, "＋") || + strings.HasPrefix(leading, "－") || strings.HasPrefix(leading, "＠") { + return "'" + value + } + return value +} diff --git a/pkg/reporting/csv_text_test.go b/pkg/reporting/csv_text_test.go new file mode 100644 index 0000000000..a8486542ee --- /dev/null +++ b/pkg/reporting/csv_text_test.go @@ -0,0 +1,125 @@ +package reporting + +import ( + "encoding/csv" + "reflect" + "strings" + "testing" + "time" +) + +func TestSpreadsheetText(t *testing.T) { + for _, value := range []string{ + "=1+1", "+1+1", "-1+1", "@SUM(1,1)", + "\tplain text", "\rplain text", "\nplain text", " =1+1", + "\u00a0+1+1", "\uFEFF-1+1", " \t@SUM(1,1)", + "＝1+1", "＋1+1", "－1+1", "＠SUM(1,1)", + } { + t.Run(value, func(t *testing.T) { + if got := spreadsheetText(value); got != "'"+value { + t.Errorf("spreadsheetText(%q) = %q, want text marker before original bytes", value, got) + } + if got := spreadsheetText(spreadsheetText(value)); got != "'"+value { + t.Errorf("already escaped text was changed again: %q", got) + } + }) + } + for _, value := range []string{ + "", "plain text", " node-1", "node-1", "name=1+1", "O'Brien", "'=-literal", + "comma,quote\"and\nnewline", "東京", "0", "12.50", " ", + } { + t.Run(value, func(t *testing.T) { + if got := spreadsheetText(value); got != value { + t.Errorf("ordinary text changed: %q -> %q", value, got) + } + }) + } +} + +func readSpreadsheetCSV(t *testing.T, data []byte) [][]string { + t.Helper() + r := csv.NewReader(strings.NewReader(string(data))) + r.FieldsPerRecord = -1 // Reports contain header, summary and data sections. + records, err := r.ReadAll() + if err != nil { + t.Fatal(err) + } + return records +} + +func requireCSVRecord(t *testing.T, records [][]string, want []string) { + t.Helper() + for _, record := range records { + if reflect.DeepEqual(record, want) { + return + } + } + t.Errorf("missing exact CSV record %q in %q", want, records) +} + +func TestCSVReportsTreatExternalTextAsLiteral(t *testing.T) { + now := time.Date(2026, 10, 25, 1, 0, 0, 0, time.UTC) + for _, text := range []string{ + "=1+1", "+1+1", "-1+1", "@SUM(1,1)", " \t=1+1", + "\tname", "\rname", "\nname", "\uFEFF=1+1", "＝1+1", + "=\"quoted,comma\nand newline\"", + } { + t.Run(text, func(t *testing.T) { + escaped := "'" + text + data := &ReportData{ + Title: text, ResourceType: text, ResourceID: text, + Resource: &ResourceInfo{Name: text}, + Start: now, End: now.Add(time.Hour), GeneratedAt: now, + Metrics: map[string][]MetricDataPoint{text: {{Timestamp: now, Value: -3.5}}}, + Summary: MetricSummary{ByMetric: map[string]MetricStats{ + text: {Count: 1, Min: -3.5, Max: -3.5, Avg: -3.5, Current: -3.5}, + }}, + } + t.Run("single", func(t *testing.T) { + out, err := NewCSVGenerator().Generate(data) + if err != nil { + t.Fatal(err) + } + records := readSpreadsheetCSV(t, out) + requireCSVRecord(t, records, []string{"# Title:", escaped}) + requireCSVRecord(t, records, []string{"# Resource Type:", escaped}) + requireCSVRecord(t, records, []string{"# Resource ID:", escaped}) + requireCSVRecord(t, records, []string{escaped, "1", "-3.50", "-3.50", "-3.50", "-3.50", ""}) + requireCSVRecord(t, records, []string{"Timestamp", escaped}) + requireCSVRecord(t, records, []string{now.Format(time.RFC3339), "-3.50"}) + }) + t.Run("multi", func(t *testing.T) { + out, err := NewCSVGenerator().GenerateMulti(&MultiReportData{ + Title: text, Start: now, End: now.Add(time.Hour), GeneratedAt: now, + Resources: []*ReportData{data}, + }) + if err != nil { + t.Fatal(err) + } + records := readSpreadsheetCSV(t, out) + requireCSVRecord(t, records, []string{"# Title:", escaped}) + requireCSVRecord(t, records, []string{escaped, escaped, escaped, escaped, "1", "-3.50", "-3.50", "-3.50", "-3.50", ""}) + requireCSVRecord(t, records, []string{"Timestamp", "Resource Name", "Resource Type", "Resource ID", escaped}) + requireCSVRecord(t, records, []string{now.Format(time.RFC3339), escaped, escaped, escaped, "-3.50"}) + }) + t.Run("inventory", func(t *testing.T) { + rows := []VMInventoryRow{{ + ResourceID: text, Instance: text, Node: text, Pool: text, Name: text, + Status: text, DiskStatusReason: text, + VMID: 101, CPUCores: 2, MemoryAllocatedBytes: 0, DiskAllocatedBytes: 512, DiskUsedBytes: -1, + }} + original := append([]VMInventoryRow(nil), rows...) + out, err := GenerateVMInventoryCSV(VMInventoryData{Rows: rows}) + if err != nil { + t.Fatal(err) + } + requireCSVRecord(t, readSpreadsheetCSV(t, out), []string{ + escaped, escaped, escaped, escaped, "101", escaped, escaped, "2", "0", "512", "-1", escaped, + }) + if !reflect.DeepEqual(rows, original) { + t.Fatal("CSV escaping changed the runtime inventory") + } + }) + }) + } +} diff --git a/pkg/reporting/vm_inventory.go b/pkg/reporting/vm_inventory.go index b1520d9276..fea3d6046a 100644 --- a/pkg/reporting/vm_inventory.go +++ b/pkg/reporting/vm_inventory.go @@ -137,18 +137,18 @@ func GenerateVMInventoryCSV(data VMInventoryData) ([]byte, error) { for _, row := range rows { record := []string{ - row.ResourceID, - row.Instance, - row.Node, - row.Pool, + spreadsheetText(row.ResourceID), + spreadsheetText(row.Instance), + spreadsheetText(row.Node), + spreadsheetText(row.Pool), strconv.Itoa(row.VMID), - row.Name, - row.Status, + spreadsheetText(row.Name), + spreadsheetText(row.Status), strconv.Itoa(row.CPUCores), strconv.FormatInt(row.MemoryAllocatedBytes, 10), strconv.FormatInt(row.DiskAllocatedBytes, 10), strconv.FormatInt(row.DiskUsedBytes, 10), - row.DiskStatusReason, + spreadsheetText(row.DiskStatusReason), } if err := w.Write(record); err != nil { return nil, fmt.Errorf("write VM inventory CSV row for %q: %w", row.ResourceID, err) From 477be1e7c3e83ff17657c442f4611246b0fed768 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Sun, 25 Oct 2026 03:57:06 +0000 Subject: [PATCH 2/2] Match CSV export safety assertions to canonical runtime identity Use the real registry's canonical VM ID and normalized node name in the download/saved-schedule regression. The first executable control exposed incorrect fixture expectations; production CSV escaping is unchanged. Change-source: pulse-maintainer --- internal/api/reporting_csv_safety_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/internal/api/reporting_csv_safety_test.go b/internal/api/reporting_csv_safety_test.go index 9129be687f..71ef5985c7 100644 --- a/internal/api/reporting_csv_safety_test.go +++ b/internal/api/reporting_csv_safety_test.go @@ -29,6 +29,11 @@ func TestReportingInventoryDownloadAndSavedScheduleEscapeSpreadsheetText(t *test setUnexportedField(t, monitor, "configPersist", p) h := NewReportingHandlers(newReportingMTMForTest(t, monitor), nil) ctx := context.WithValue(context.Background(), OrgIDContextKey, "default") + before, ok := h.getReportingEnrichmentSnapshot(ctx, "default") + require.True(t, ok) + require.Len(t, before.Resources, 1) + canonicalID := before.Resources[0].ID + require.NotEmpty(t, canonicalID) request := httptest.NewRequest(http.MethodGet, "/api/admin/reports/inventory/vms/export?format=csv", nil).WithContext(ctx) download := httptest.NewRecorder() h.HandleExportVMInventory(download, request) @@ -39,7 +44,7 @@ func TestReportingInventoryDownloadAndSavedScheduleEscapeSpreadsheetText(t *test require.NoError(t, err) require.Len(t, records, 2) require.Equal(t, []string{ - "vm-formula", "'+1+1", "'\t@node", "'@SUM(1,1)", "101", "'=1+1", "online", "2", "0", "0", "0", "'\rreason", + canonicalID, "'+1+1", "'@node", "'@SUM(1,1)", "101", "'=1+1", "online", "2", "0", "0", "0", "'\rreason", }, records[1]) s := validReportSchedulePayload() @@ -58,5 +63,6 @@ func TestReportingInventoryDownloadAndSavedScheduleEscapeSpreadsheetText(t *test snapshot, ok := h.getReportingEnrichmentSnapshot(ctx, "default") require.True(t, ok) require.Len(t, snapshot.Resources, 1) + require.Equal(t, canonicalID, snapshot.Resources[0].ID) require.Equal(t, "=1+1", snapshot.Resources[0].Name, "escape only the exported cell, never the monitored name") }