From e412456457378966d40af515d433573cd37ffe9a Mon Sep 17 00:00:00 2001 From: "Harper, Jason M" Date: Wed, 10 Dec 2025 10:23:57 -0800 Subject: [PATCH 1/6] Add defensive coding fixes to prevent index out of range errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit adds bounds checking in several locations where array/slice indexing could potentially cause panics: 1. report.go:520 - Add check that Values slices have elements before accessing [0] when looking up reference data 2. report.go:386 - Use cached memLatTableValues instead of calling getTableValues twice, ensuring consistency 3. dimm.go:628 - Add check that dimms[0] has sufficient elements before accessing BankLocatorIdx and LocatorIdx 4. report_tables.go:2036 - Add bounds checks before accessing DerivedSocketIdx, DerivedChannelIdx, and DerivedSlotIdx Values[0] 5. report_tables.go:1222-1224 - Store Split results and check length before accessing [0] to make code more defensive 6. system.go:105 - Add length check before accessing fields[len-2] to prevent panic on short slices 7. system.go:129 - Add bounds checks for both regex match results and fields slice before accessing indices These changes prevent potential panics when processing unexpected or malformed data while maintaining the existing error handling behavior. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- cmd/report/dimm.go | 4 ++++ cmd/report/report.go | 15 ++++++++------- cmd/report/report_tables.go | 32 +++++++++++++++++++++----------- cmd/report/system.go | 4 ++-- 4 files changed, 35 insertions(+), 20 deletions(-) diff --git a/cmd/report/dimm.go b/cmd/report/dimm.go index 0d814750..c7e04869 100644 --- a/cmd/report/dimm.go +++ b/cmd/report/dimm.go @@ -625,6 +625,10 @@ func deriveDIMMInfoOther(dimms [][]string, channelsPerSocket int) ([]derivedFiel err := fmt.Errorf("no DIMMs") return nil, err } + if len(dimms[0]) <= max(BankLocatorIdx, LocatorIdx) { + err := fmt.Errorf("DIMM data has insufficient fields") + return nil, err + } dimmType, reBankLoc, reLoc := getDIMMParseInfo((dimms)[0][BankLocatorIdx], (dimms)[0][LocatorIdx]) if dimmType == DIMMTypeUNKNOWN { err := fmt.Errorf("unknown DIMM identification format") diff --git a/cmd/report/report.go b/cmd/report/report.go index ff8bd84a..bb64312c 100644 --- a/cmd/report/report.go +++ b/cmd/report/report.go @@ -383,7 +383,7 @@ func benchmarkSummaryFromTableValues(allTableValues []table.TableValues, outputs memLatTableValues := getTableValues(allTableValues, MemoryBenchmarkTableName) var bandwidthValues []string if len(memLatTableValues.Fields) > 1 { - bandwidthValues = getTableValues(allTableValues, MemoryBenchmarkTableName).Fields[1].Values + bandwidthValues = memLatTableValues.Fields[1].Values } maxBandwidth := 0.0 for _, bandwidthValue := range bandwidthValues { @@ -517,7 +517,8 @@ func summaryHTMLTableRenderer(tv table.TableValues, targetName string) string { panic(err) } // if we have reference data that matches the microarchitecture and sockets, use it - if refData, ok := referenceData[ReferenceDataKey{tv.Fields[uarchFieldIdx].Values[0], tv.Fields[socketsFieldIdx].Values[0]}]; ok { + if len(tv.Fields[uarchFieldIdx].Values) > 0 && len(tv.Fields[socketsFieldIdx].Values) > 0 { + if refData, ok := referenceData[ReferenceDataKey{tv.Fields[uarchFieldIdx].Values[0], tv.Fields[socketsFieldIdx].Values[0]}]; ok { // remove microarchitecture and sockets fields fields := tv.Fields[:len(tv.Fields)-2] refTableValues := table.TableValues{ @@ -532,12 +533,12 @@ func summaryHTMLTableRenderer(tv table.TableValues, targetName string) string { {Name: "Memory Minimum Latency", Values: []string{fmt.Sprintf("%.0f ns", refData.MemMinLatency)}}, }, } - return report.RenderMultiTargetTableValuesAsHTML([]table.TableValues{{TableDefinition: tv.TableDefinition, Fields: fields}, refTableValues}, []string{targetName, refData.Description}) - } else { - // remove microarchitecture and sockets fields - fields := tv.Fields[:len(tv.Fields)-2] - return report.DefaultHTMLTableRendererFunc(table.TableValues{TableDefinition: tv.TableDefinition, Fields: fields}) + return report.RenderMultiTargetTableValuesAsHTML([]table.TableValues{{TableDefinition: tv.TableDefinition, Fields: fields}, refTableValues}, []string{targetName, refData.Description}) + } } + // remove microarchitecture and sockets fields + fields := tv.Fields[:len(tv.Fields)-2] + return report.DefaultHTMLTableRendererFunc(table.TableValues{TableDefinition: tv.TableDefinition, Fields: fields}) } func summaryXlsxTableRenderer(tv table.TableValues, f *excelize.File, targetName string, row *int) { diff --git a/cmd/report/report_tables.go b/cmd/report/report_tables.go index 603a923d..4081b86d 100644 --- a/cmd/report/report_tables.go +++ b/cmd/report/report_tables.go @@ -1219,19 +1219,23 @@ func dimmTableInsights(outputs map[string]script.ScriptOutput, tableValues table for i, speed := range tableValues.Fields[SpeedIndex].Values { configuredSpeed := tableValues.Fields[ConfiguredSpeedIndex].Values[i] if speed != "" && configuredSpeed != "" && speed != "Unknown" && configuredSpeed != "Unknown" { - speedVal, err := strconv.Atoi(strings.Split(speed, " ")[0]) - if err != nil { - slog.Warn(err.Error()) - } else { - configuredSpeedVal, err := strconv.Atoi(strings.Split(configuredSpeed, " ")[0]) + speedParts := strings.Split(speed, " ") + configuredSpeedParts := strings.Split(configuredSpeed, " ") + if len(speedParts) > 0 && len(configuredSpeedParts) > 0 { + speedVal, err := strconv.Atoi(speedParts[0]) if err != nil { slog.Warn(err.Error()) } else { - if speedVal < configuredSpeedVal { - insights = append(insights, table.Insight{ - Recommendation: "Consider configuring DIMMs for their maximum speed.", - Justification: fmt.Sprintf("DIMMs configured for %s when their maximum speed is %s.", configuredSpeed, speed), - }) + configuredSpeedVal, err := strconv.Atoi(configuredSpeedParts[0]) + if err != nil { + slog.Warn(err.Error()) + } else { + if speedVal < configuredSpeedVal { + insights = append(insights, table.Insight{ + Recommendation: "Consider configuring DIMMs for their maximum speed.", + Justification: fmt.Sprintf("DIMMs configured for %s when their maximum speed is %s.", configuredSpeed, speed), + }) + } } } } @@ -2033,7 +2037,13 @@ func dimmDetails(dimm []string) (details string) { } func dimmTableHTMLRenderer(tableValues table.TableValues, targetName string) string { - if tableValues.Fields[DerivedSocketIdx].Values[0] == "" || tableValues.Fields[DerivedChannelIdx].Values[0] == "" || tableValues.Fields[DerivedSlotIdx].Values[0] == "" { + if len(tableValues.Fields) <= max(DerivedSocketIdx, DerivedChannelIdx, DerivedSlotIdx) || + len(tableValues.Fields[DerivedSocketIdx].Values) == 0 || + len(tableValues.Fields[DerivedChannelIdx].Values) == 0 || + len(tableValues.Fields[DerivedSlotIdx].Values) == 0 || + tableValues.Fields[DerivedSocketIdx].Values[0] == "" || + tableValues.Fields[DerivedChannelIdx].Values[0] == "" || + tableValues.Fields[DerivedSlotIdx].Values[0] == "" { return report.DefaultHTMLTableRendererFunc(tableValues) } htmlColors := []string{"lightgreen", "orange", "aqua", "lime", "yellow", "beige", "magenta", "violet", "salmon", "pink"} diff --git a/cmd/report/system.go b/cmd/report/system.go index a00d2beb..608c252c 100644 --- a/cmd/report/system.go +++ b/cmd/report/system.go @@ -102,7 +102,7 @@ func filesystemFieldValuesFromOutput(outputs map[string]script.ScriptOutput) []t } fields := strings.Fields(line) // "Mounted On" gets split into two fields, rejoin - if i == 0 && fields[len(fields)-2] == "Mounted" && fields[len(fields)-1] == "on" { + if i == 0 && len(fields) >= 2 && fields[len(fields)-2] == "Mounted" && fields[len(fields)-1] == "on" { fields[len(fields)-2] = "Mounted on" fields = fields[:len(fields)-1] for _, field := range fields { @@ -126,7 +126,7 @@ func filesystemFieldValuesFromOutput(outputs map[string]script.ScriptOutput) []t continue } match := reFindmnt.FindStringSubmatch(line) - if match != nil { + if match != nil && len(match) > 4 && len(fields) > 5 { target := match[1] source := match[2] if fields[0] == source && fields[5] == target { From 0bffecc16d5963016d12f3199131e8233069e0fc Mon Sep 17 00:00:00 2001 From: "Harper, Jason M" Date: Wed, 10 Dec 2025 14:58:50 -0800 Subject: [PATCH 2/6] Add defensive coding fixes to internal/common MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit adds defensive bounds checking in internal/common to prevent index out of range errors when processing system data: ### nic.go fixes: 1. Line 117 - Check bounds before accessing Split result when parsing model names with parentheses 2. Line 182 - Validate deviceFunc slice has elements before accessing [0] when building card identifiers from PCI addresses ### power.go fixes: 3. Lines 40, 60, 88 - Add bounds checks before accessing Split result [1] when parsing EPP-related MSR values from colon-separated output 4. Line 127 - Consolidate bounds checks for fieldValues array access 5. Lines 212, 220 - Add bounds checks before accessing row indices when interpreting ELC mode values These changes prevent panics when: - NIC model names have unexpected formats - PCI addresses are malformed - MSR register output has unexpected format - CSV data from ELC scripts has missing columns All changes maintain existing error handling behavior while making the code more robust against unexpected input formats. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- internal/common/nic.go | 7 +++++-- internal/common/power.go | 30 ++++++++++++++++++------------ 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/internal/common/nic.go b/internal/common/nic.go index 765f3bcc..58ef009d 100644 --- a/internal/common/nic.go +++ b/internal/common/nic.go @@ -114,7 +114,10 @@ func ParseNicInfo(scriptOutput string) []nicInfo { } } // special case for model as it sometimes has additional information in parentheses - nic.Model = strings.TrimSpace(strings.Split(nic.Model, "(")[0]) + modelParts := strings.Split(nic.Model, "(") + if len(modelParts) > 0 { + nic.Model = strings.TrimSpace(modelParts[0]) + } nics = append(nics, nic) } // Assign card and port information @@ -176,7 +179,7 @@ func assignCardAndPort(nics []nicInfo) { } // Further split the last part to separate device from function deviceFunc := strings.Split(parts[2], ".") - if len(deviceFunc) != 2 { + if len(deviceFunc) < 1 { continue } // Card identifier is domain:bus:device diff --git a/internal/common/power.go b/internal/common/power.go index 06f1c73b..cb08093c 100644 --- a/internal/common/power.go +++ b/internal/common/power.go @@ -37,7 +37,11 @@ func EPPFromOutput(outputs map[string]script.ScriptOutput) string { if line == "" { continue } - currentEpbValid := strings.TrimSpace(strings.Split(line, ":")[1]) + lineParts := strings.Split(line, ":") + if len(lineParts) < 2 { + continue + } + currentEpbValid := strings.TrimSpace(lineParts[1]) if i == 0 { eppValid = currentEpbValid continue @@ -53,7 +57,11 @@ func EPPFromOutput(outputs map[string]script.ScriptOutput) string { if line == "" { continue } - currentEppPkgCtrl := strings.TrimSpace(strings.Split(line, ":")[1]) + lineParts := strings.Split(line, ":") + if len(lineParts) < 2 { + continue + } + currentEppPkgCtrl := strings.TrimSpace(lineParts[1]) if i == 0 { eppPkgCtrl = currentEppPkgCtrl continue @@ -77,7 +85,11 @@ func EPPFromOutput(outputs map[string]script.ScriptOutput) string { if line == "" { continue } - currentEpp := strings.TrimSpace(strings.Split(line, ":")[1]) + lineParts := strings.Split(line, ":") + if len(lineParts) < 2 { + continue + } + currentEpp := strings.TrimSpace(lineParts[1]) if i == 0 { epp = currentEpp continue @@ -112,13 +124,7 @@ func EPBFromOutput(outputs map[string]script.ScriptOutput) string { func ELCSummaryFromOutput(outputs map[string]script.ScriptOutput) string { fieldValues := ELCFieldValuesFromOutput(outputs) - if len(fieldValues) == 0 { - return "" - } - if len(fieldValues) < 10 { - return "" - } - if len(fieldValues[9].Values) == 0 { + if len(fieldValues) < 10 || len(fieldValues[9].Values) == 0 { return "" } summary := fieldValues[9].Values[0] @@ -203,7 +209,7 @@ func ELCFieldValuesFromOutput(outputs map[string]script.ScriptOutput) (fieldValu // value rows for _, row := range rows[1:] { var mode string - if row[2] == "IO" { + if len(row) > 7 && row[2] == "IO" { if row[5] == "0" && row[6] == "0" && row[7] == "0" { mode = "Latency Optimized" } else if row[5] == "800" && row[6] == "10" && row[7] == "94" { @@ -211,7 +217,7 @@ func ELCFieldValuesFromOutput(outputs map[string]script.ScriptOutput) (fieldValu } else { mode = "Custom" } - } else { // COMPUTE + } else if len(row) > 5 { // COMPUTE switch row[5] { case "0": mode = "Latency Optimized" From a8de56829099f3465883fcd5bc5807e995ce4663 Mon Sep 17 00:00:00 2001 From: "Harper, Jason M" Date: Wed, 10 Dec 2025 15:00:12 -0800 Subject: [PATCH 3/6] unnecessary parens Signed-off-by: Harper, Jason M --- cmd/report/dimm.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/report/dimm.go b/cmd/report/dimm.go index c7e04869..3371997c 100644 --- a/cmd/report/dimm.go +++ b/cmd/report/dimm.go @@ -629,7 +629,7 @@ func deriveDIMMInfoOther(dimms [][]string, channelsPerSocket int) ([]derivedFiel err := fmt.Errorf("DIMM data has insufficient fields") return nil, err } - dimmType, reBankLoc, reLoc := getDIMMParseInfo((dimms)[0][BankLocatorIdx], (dimms)[0][LocatorIdx]) + dimmType, reBankLoc, reLoc := getDIMMParseInfo(dimms[0][BankLocatorIdx], dimms[0][LocatorIdx]) if dimmType == DIMMTypeUNKNOWN { err := fmt.Errorf("unknown DIMM identification format") return nil, err From d828b6563e43722e792664449b84988e6d69db05 Mon Sep 17 00:00:00 2001 From: "Harper, Jason M" Date: Wed, 10 Dec 2025 15:00:21 -0800 Subject: [PATCH 4/6] formatting Signed-off-by: Harper, Jason M --- cmd/report/report.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/cmd/report/report.go b/cmd/report/report.go index bb64312c..28100255 100644 --- a/cmd/report/report.go +++ b/cmd/report/report.go @@ -519,20 +519,20 @@ func summaryHTMLTableRenderer(tv table.TableValues, targetName string) string { // if we have reference data that matches the microarchitecture and sockets, use it if len(tv.Fields[uarchFieldIdx].Values) > 0 && len(tv.Fields[socketsFieldIdx].Values) > 0 { if refData, ok := referenceData[ReferenceDataKey{tv.Fields[uarchFieldIdx].Values[0], tv.Fields[socketsFieldIdx].Values[0]}]; ok { - // remove microarchitecture and sockets fields - fields := tv.Fields[:len(tv.Fields)-2] - refTableValues := table.TableValues{ - Fields: []table.Field{ - {Name: "CPU Speed", Values: []string{fmt.Sprintf("%.0f Ops/s", refData.CPUSpeed)}}, - {Name: "Single-core Maximum frequency", Values: []string{fmt.Sprintf("%.0f MHz", refData.SingleCoreFreq)}}, - {Name: "All-core Maximum frequency", Values: []string{fmt.Sprintf("%.0f MHz", refData.AllCoreFreq)}}, - {Name: "Maximum Power", Values: []string{fmt.Sprintf("%.0f W", refData.MaxPower)}}, - {Name: "Maximum Temperature", Values: []string{fmt.Sprintf("%.0f C", refData.MaxTemp)}}, - {Name: "Minimum Power", Values: []string{fmt.Sprintf("%.0f W", refData.MinPower)}}, - {Name: "Memory Peak Bandwidth", Values: []string{fmt.Sprintf("%.0f GB/s", refData.MemPeakBandwidth)}}, - {Name: "Memory Minimum Latency", Values: []string{fmt.Sprintf("%.0f ns", refData.MemMinLatency)}}, - }, - } + // remove microarchitecture and sockets fields + fields := tv.Fields[:len(tv.Fields)-2] + refTableValues := table.TableValues{ + Fields: []table.Field{ + {Name: "CPU Speed", Values: []string{fmt.Sprintf("%.0f Ops/s", refData.CPUSpeed)}}, + {Name: "Single-core Maximum frequency", Values: []string{fmt.Sprintf("%.0f MHz", refData.SingleCoreFreq)}}, + {Name: "All-core Maximum frequency", Values: []string{fmt.Sprintf("%.0f MHz", refData.AllCoreFreq)}}, + {Name: "Maximum Power", Values: []string{fmt.Sprintf("%.0f W", refData.MaxPower)}}, + {Name: "Maximum Temperature", Values: []string{fmt.Sprintf("%.0f C", refData.MaxTemp)}}, + {Name: "Minimum Power", Values: []string{fmt.Sprintf("%.0f W", refData.MinPower)}}, + {Name: "Memory Peak Bandwidth", Values: []string{fmt.Sprintf("%.0f GB/s", refData.MemPeakBandwidth)}}, + {Name: "Memory Minimum Latency", Values: []string{fmt.Sprintf("%.0f ns", refData.MemMinLatency)}}, + }, + } return report.RenderMultiTargetTableValuesAsHTML([]table.TableValues{{TableDefinition: tv.TableDefinition, Fields: fields}, refTableValues}, []string{targetName, refData.Description}) } } From a8de7372cb51de714731d7b8e9365772ebdde5b0 Mon Sep 17 00:00:00 2001 From: "Harper, Jason M" Date: Wed, 10 Dec 2025 15:00:44 -0800 Subject: [PATCH 5/6] remove unnecessary len check Signed-off-by: Harper, Jason M --- cmd/report/system.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/report/system.go b/cmd/report/system.go index 608c252c..814a6683 100644 --- a/cmd/report/system.go +++ b/cmd/report/system.go @@ -126,7 +126,7 @@ func filesystemFieldValuesFromOutput(outputs map[string]script.ScriptOutput) []t continue } match := reFindmnt.FindStringSubmatch(line) - if match != nil && len(match) > 4 && len(fields) > 5 { + if match != nil && len(fields) > 5 { target := match[1] source := match[2] if fields[0] == source && fields[5] == target { From 0fc480e8ed80c8be0d0d0087c49597d3fdf79e8a Mon Sep 17 00:00:00 2001 From: "Harper, Jason M" Date: Wed, 10 Dec 2025 15:10:37 -0800 Subject: [PATCH 6/6] Add defensive coding fixes to internal/report MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit adds defensive bounds checking in internal/report rendering functions to prevent index out of range errors when processing table data: ### render_excel.go fixes: 1. Line 72 - Add check that targetTableValues has elements before accessing [0] in renderXlsxTableMultiTarget 2. Line 208 - Check each field's Values length instead of assuming all fields have same structure as Fields[0] ### render_html.go fixes: 3. Line 306 - Add check that allTargetsTableValues has elements before accessing [0] in createHtmlReportMultiTarget 4. Line 599 - Add bounds check before accessing field.Values[0] when rendering non-row tables 5. Line 614 - Add check that tableValues has elements before accessing [0] in RenderMultiTargetTableValuesAsHTML These changes prevent panics when: - Rendering reports with empty target lists - Processing tables where fields have varying numbers of values - Generating multi-target HTML reports with no data All changes maintain existing rendering behavior while making the code more robust against edge cases with missing or incomplete data. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- internal/report/render_excel.go | 5 ++++- internal/report/render_html.go | 12 +++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/internal/report/render_excel.go b/internal/report/render_excel.go index 1d72665f..98bac2a0 100644 --- a/internal/report/render_excel.go +++ b/internal/report/render_excel.go @@ -69,6 +69,9 @@ func renderXlsxTable(tableValues table.TableValues, f *excelize.File, sheetName } func renderXlsxTableMultiTarget(targetTableValues []table.TableValues, targetNames []string, f *excelize.File, sheetName string, row *int) { + if len(targetTableValues) == 0 { + return + } col := 1 // print the table name tableNameStyle, _ := f.NewStyle(&excelize.Style{ @@ -202,7 +205,7 @@ func DefaultXlsxTableRendererFunc(tableValues table.TableValues, f *excelize.Fil col := 1 for _, field := range tableValues.Fields { var fieldValue string - if len(tableValues.Fields[0].Values) > 0 { + if len(field.Values) > 0 { fieldValue = field.Values[0] } _ = f.SetCellValue(sheetName, cellName(col, *row), field.Name) diff --git a/internal/report/render_html.go b/internal/report/render_html.go index 27bd5971..044ed814 100644 --- a/internal/report/render_html.go +++ b/internal/report/render_html.go @@ -303,6 +303,9 @@ func createHtmlReport(allTableValues []table.TableValues, targetName string) (ou } func createHtmlReportMultiTarget(allTargetsTableValues [][]table.TableValues, targetNames []string, allTableNames []string) (out []byte, err error) { + if len(allTargetsTableValues) == 0 { + return nil, fmt.Errorf("no target table values provided") + } var sb strings.Builder sb.WriteString(getHtmlReportBegin()) @@ -593,7 +596,11 @@ func DefaultHTMLTableRendererFunc(tableValues table.TableValues) string { for _, field := range tableValues.Fields { rowValues := []string{} rowValues = append(rowValues, CreateFieldNameWithDescription(field.Name, field.Description)) - rowValues = append(rowValues, htmltemplate.HTMLEscapeString(field.Values[0])) + if len(field.Values) > 0 { + rowValues = append(rowValues, htmltemplate.HTMLEscapeString(field.Values[0])) + } else { + rowValues = append(rowValues, "") + } values = append(values, rowValues) tableValueStyles = append(tableValueStyles, []string{"font-weight:bold"}) } @@ -604,6 +611,9 @@ func DefaultHTMLTableRendererFunc(tableValues table.TableValues) string { // RenderMultiTargetTableValuesAsHTML renders a table for multiple targets // tableValues is a slice of table.TableValues, each of which represents the same table from a single target func RenderMultiTargetTableValuesAsHTML(tableValues []table.TableValues, targetNames []string) string { + if len(tableValues) == 0 { + return "" + } values := [][]string{} var tableValueStyles [][]string for fieldIndex, field := range tableValues[0].Fields {