mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-26 13:39:47 +00:00
Keep report CSV text from becoming spreadsheet formulas (#3237)
**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. **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. --- Fleet candidate `20261025T034027Z-web-product` from the `web-product` lane. Reviewed tip `477be1e7c3e83ff17657c442f4611246b0fed768` on `main`. [How Pulse handles triage](https://github.com/rcourtman/Pulse/blob/main/docs/AI_TRANSPARENCY.md)
This commit is contained in:
@@ -0,0 +1,68 @@
|
||||
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")
|
||||
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)
|
||||
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{
|
||||
canonicalID, "'+1+1", "'@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, canonicalID, snapshot.Resources[0].ID)
|
||||
require.Equal(t, "=1+1", snapshot.Resources[0].Name, "escape only the exported cell, never the monitored name")
|
||||
}
|
||||
+14
-14
@@ -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))
|
||||
|
||||
@@ -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
|
||||
}
|
||||
@@ -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")
|
||||
}
|
||||
})
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user