Added some tests for censoring.

render/detailed/censor.go

@@ -4,40 +4,45 @@ import (
 	"github.com/weaveworks/scope/report"
 )
 
-func censorNodeSummary(s *NodeSummary, cfg report.CensorConfig) {
-	if cfg.HideCommandLineArguments {
+func censorNodeSummary(s NodeSummary, cfg report.CensorConfig) NodeSummary {
+	if cfg.HideCommandLineArguments && s.Metadata != nil {
 		// Iterate through all the metadata rows and strip the
-		// arguments from all the values containing a command.
-		for index := range s.Metadata {
-			row := &s.Metadata[index]
+		// arguments from all the values containing a command
+		// (while making sure everything is done in a non-mutable way).
+		metadata := []report.MetadataRow{}
+		for _, row := range s.Metadata {
 			if report.IsCommandEntry(row.ID) {
 				row.Value = report.StripCommandArgs(row.Value)
 			}
+			metadata = append(metadata, row)
 		}
+		s.Metadata = metadata
 	}
-	if cfg.HideEnvironmentVariables {
-		// Go through all the tables and if environment variables
-		// table is found, drop it from the list and stop the loop.
-		for index, table := range s.Tables {
-			if report.IsEnvironmentVarsEntry(table.ID) {
-				s.Tables = append(s.Tables[:index], s.Tables[index+1:]...)
-				break
+	if cfg.HideEnvironmentVariables && s.Tables != nil {
+		// Copy across all the tables except the environment
+		// variable ones (ensuring the operation is non-mutable).
+		tables := []report.Table{}
+		for _, table := range s.Tables {
+			if !report.IsEnvironmentVarsEntry(table.ID) {
+				tables = append(tables, table)
 			}
 		}
+		s.Tables = tables
 	}
+	return s
 }
 
 // CensorNode removes any sensitive data from a node.
-func CensorNode(n Node, cfg report.CensorConfig) Node {
-	censorNodeSummary(&n.NodeSummary, cfg)
-	return n
+func CensorNode(node Node, cfg report.CensorConfig) Node {
+	node.NodeSummary = censorNodeSummary(node.NodeSummary, cfg)
+	return node
 }
 
 // CensorNodeSummaries removes any sensitive data from a list of node summaries.
-func CensorNodeSummaries(ns NodeSummaries, cfg report.CensorConfig) NodeSummaries {
-	for key, summary := range ns {
-		censorNodeSummary(&summary, cfg)
-		ns[key] = summary
+func CensorNodeSummaries(summaries NodeSummaries, cfg report.CensorConfig) NodeSummaries {
+	censored := NodeSummaries{}
+	for key := range summaries {
+		censored[key] = censorNodeSummary(summaries[key], cfg)
 	}
-	return ns
+	return censored
 }

render/detailed/censor_test.go

@@ -0,0 +1,228 @@
+package detailed_test
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/weaveworks/common/test"
+	"github.com/weaveworks/scope/render/detailed"
+	"github.com/weaveworks/scope/report"
+)
+
+func TestCensorNode(t *testing.T) {
+	node := detailed.Node{
+		NodeSummary: detailed.NodeSummary{
+			Metadata: []report.MetadataRow{
+				{ID: "cmdline", Label: "Command", Value: "prog -a --b=c"},
+			},
+			Tables: []report.Table{
+				{ID: "blibli", Rows: []report.Row{{ID: "bli"}}},
+				{ID: "docker_env_", Rows: []report.Row{{ID: "env_var"}}},
+			},
+		},
+	}
+
+	for _, c := range []struct {
+		label      string
+		have, want detailed.Node
+	}{
+		{
+			label: "no censoring",
+			have: detailed.CensorNode(node, report.CensorConfig{
+				HideCommandLineArguments: false,
+				HideEnvironmentVariables: false,
+			}),
+			want: detailed.Node{
+				NodeSummary: detailed.NodeSummary{
+					Metadata: []report.MetadataRow{
+						{ID: "cmdline", Label: "Command", Value: "prog -a --b=c"},
+					},
+					Tables: []report.Table{
+						{ID: "blibli", Rows: []report.Row{{ID: "bli"}}},
+						{ID: "docker_env_", Rows: []report.Row{{ID: "env_var"}}},
+					},
+				},
+			},
+		},
+		{
+			label: "censor only command line args",
+			have: detailed.CensorNode(node, report.CensorConfig{
+				HideCommandLineArguments: true,
+				HideEnvironmentVariables: false,
+			}),
+			want: detailed.Node{
+				NodeSummary: detailed.NodeSummary{
+					Metadata: []report.MetadataRow{
+						{ID: "cmdline", Label: "Command", Value: "prog"},
+					},
+					Tables: []report.Table{
+						{ID: "blibli", Rows: []report.Row{{ID: "bli"}}},
+						{ID: "docker_env_", Rows: []report.Row{{ID: "env_var"}}},
+					},
+				},
+			},
+		},
+		{
+			label: "censor only env variables",
+			have: detailed.CensorNode(node, report.CensorConfig{
+				HideCommandLineArguments: false,
+				HideEnvironmentVariables: true,
+			}),
+			want: detailed.Node{
+				NodeSummary: detailed.NodeSummary{
+					Metadata: []report.MetadataRow{
+						{ID: "cmdline", Label: "Command", Value: "prog -a --b=c"},
+					},
+					Tables: []report.Table{
+						{ID: "blibli", Rows: []report.Row{{ID: "bli"}}},
+					},
+				},
+			},
+		},
+		{
+			label: "censor both command line args and env vars",
+			have: detailed.CensorNode(node, report.CensorConfig{
+				HideCommandLineArguments: true,
+				HideEnvironmentVariables: true,
+			}),
+			want: detailed.Node{
+				NodeSummary: detailed.NodeSummary{
+					Metadata: []report.MetadataRow{
+						{ID: "cmdline", Label: "Command", Value: "prog"},
+					},
+					Tables: []report.Table{
+						{ID: "blibli", Rows: []report.Row{{ID: "bli"}}},
+					},
+				},
+			},
+		},
+	} {
+		if !reflect.DeepEqual(c.want, c.have) {
+			t.Errorf("%s - %s", c.label, test.Diff(c.want, c.have))
+		}
+	}
+}
+
+func TestCensorNodeSummaries(t *testing.T) {
+	summaries := detailed.NodeSummaries{
+		"a": detailed.NodeSummary{
+			Metadata: []report.MetadataRow{
+				{ID: "blublu", Label: "blabla", Value: "blu blu"},
+				{ID: "docker_container_command", Label: "Command", Value: "scope --token=blibli"},
+			},
+		},
+		"b": detailed.NodeSummary{
+			Metadata: []report.MetadataRow{
+				{ID: "cmdline", Label: "Command", Value: "prog -a --b=c"},
+			},
+			Tables: []report.Table{
+				{ID: "blibli", Rows: []report.Row{{ID: "bli"}}},
+				{ID: "docker_env_", Rows: []report.Row{{ID: "env_var"}}},
+			},
+		},
+	}
+
+	for _, c := range []struct {
+		label      string
+		have, want detailed.NodeSummaries
+	}{
+		{
+			label: "no censoring",
+			have: detailed.CensorNodeSummaries(summaries, report.CensorConfig{
+				HideCommandLineArguments: false,
+				HideEnvironmentVariables: false,
+			}),
+			want: detailed.NodeSummaries{
+				"a": detailed.NodeSummary{
+					Metadata: []report.MetadataRow{
+						{ID: "blublu", Label: "blabla", Value: "blu blu"},
+						{ID: "docker_container_command", Label: "Command", Value: "scope --token=blibli"},
+					},
+				},
+				"b": detailed.NodeSummary{
+					Metadata: []report.MetadataRow{
+						{ID: "cmdline", Label: "Command", Value: "prog -a --b=c"},
+					},
+					Tables: []report.Table{
+						{ID: "blibli", Rows: []report.Row{{ID: "bli"}}},
+						{ID: "docker_env_", Rows: []report.Row{{ID: "env_var"}}},
+					},
+				},
+			},
+		},
+		{
+			label: "censor only command line args",
+			have: detailed.CensorNodeSummaries(summaries, report.CensorConfig{
+				HideCommandLineArguments: true,
+				HideEnvironmentVariables: false,
+			}),
+			want: detailed.NodeSummaries{
+				"a": detailed.NodeSummary{
+					Metadata: []report.MetadataRow{
+						{ID: "blublu", Label: "blabla", Value: "blu blu"},
+						{ID: "docker_container_command", Label: "Command", Value: "scope"},
+					},
+				},
+				"b": detailed.NodeSummary{
+					Metadata: []report.MetadataRow{
+						{ID: "cmdline", Label: "Command", Value: "prog"},
+					},
+					Tables: []report.Table{
+						{ID: "blibli", Rows: []report.Row{{ID: "bli"}}},
+						{ID: "docker_env_", Rows: []report.Row{{ID: "env_var"}}},
+					},
+				},
+			},
+		},
+		{
+			label: "censor only env variables",
+			have: detailed.CensorNodeSummaries(summaries, report.CensorConfig{
+				HideCommandLineArguments: false,
+				HideEnvironmentVariables: true,
+			}),
+			want: detailed.NodeSummaries{
+				"a": detailed.NodeSummary{
+					Metadata: []report.MetadataRow{
+						{ID: "blublu", Label: "blabla", Value: "blu blu"},
+						{ID: "docker_container_command", Label: "Command", Value: "scope --token=blibli"},
+					},
+				},
+				"b": detailed.NodeSummary{
+					Metadata: []report.MetadataRow{
+						{ID: "cmdline", Label: "Command", Value: "prog -a --b=c"},
+					},
+					Tables: []report.Table{
+						{ID: "blibli", Rows: []report.Row{{ID: "bli"}}},
+					},
+				},
+			},
+		},
+		{
+			label: "censor both command line args and env vars",
+			have: detailed.CensorNodeSummaries(summaries, report.CensorConfig{
+				HideCommandLineArguments: true,
+				HideEnvironmentVariables: true,
+			}),
+			want: detailed.NodeSummaries{
+				"a": detailed.NodeSummary{
+					Metadata: []report.MetadataRow{
+						{ID: "blublu", Label: "blabla", Value: "blu blu"},
+						{ID: "docker_container_command", Label: "Command", Value: "scope"},
+					},
+				},
+				"b": detailed.NodeSummary{
+					Metadata: []report.MetadataRow{
+						{ID: "cmdline", Label: "Command", Value: "prog"},
+					},
+					Tables: []report.Table{
+						{ID: "blibli", Rows: []report.Row{{ID: "bli"}}},
+					},
+				},
+			},
+		},
+	} {
+		if !reflect.DeepEqual(c.want, c.have) {
+			t.Errorf("%s - %s", c.label, test.Diff(c.want, c.have))
+		}
+	}
+}

report/censor.go

@@ -15,8 +15,8 @@ type CensorConfig struct {
 // GetCensorConfigFromRequest extracts censor config from request query params.
 func GetCensorConfigFromRequest(req *http.Request) CensorConfig {
 	return CensorConfig{
-		HideCommandLineArguments: true || req.URL.Query().Get("hideCommandLineArguments") == "true",
-		HideEnvironmentVariables: true || req.URL.Query().Get("hideEnvironmentVariables") == "true",
+		HideCommandLineArguments: req.URL.Query().Get("hideCommandLineArguments") == "true",
+		HideEnvironmentVariables: req.URL.Query().Get("hideEnvironmentVariables") == "true",
 	}
 }
 
@@ -37,27 +37,32 @@ func StripCommandArgs(command string) string {
 	return strings.Split(command, " ")[0]
 }
 
-// CensorRawReport removes any sensitive data from
-// the raw report based on the request query params.
-func CensorRawReport(r Report, cfg CensorConfig) Report {
-	r.WalkTopologies(func(t *Topology) {
+// CensorRawReport removes any sensitive data from the raw report based on the request query params.
+func CensorRawReport(rawReport Report, cfg CensorConfig) Report {
+	// Create a copy of the report first to make sure the operation is immutable.
+	censoredReport := rawReport.Copy()
+	censoredReport.ID = rawReport.ID
+
+	censoredReport.WalkTopologies(func(t *Topology) {
 		for nodeID, node := range t.Nodes {
-			latest := StringLatestMap{}
-			for _, entry := range node.Latest {
-				// If environment variables are to be hidden, omit passing them to the final report.
-				if cfg.HideEnvironmentVariables && IsEnvironmentVarsEntry(entry.key) {
-					continue
+			if node.Latest != nil {
+				latest := make(StringLatestMap, 0, cap(node.Latest))
+				for _, entry := range node.Latest {
+					// If environment variables are to be hidden, omit passing them to the final report.
+					if cfg.HideEnvironmentVariables && IsEnvironmentVarsEntry(entry.key) {
+						continue
+					}
+					// If command line arguments are to be hidden, strip them away.
+					if cfg.HideCommandLineArguments && IsCommandEntry(entry.key) {
+						entry.Value = StripCommandArgs(entry.Value)
+					}
+					// Pass the latest entry to the final report.
+					latest = append(latest, entry)
 				}
-				// If command line arguments are to be hidden, strip them away.
-				if cfg.HideCommandLineArguments && IsCommandEntry(entry.key) {
-					entry.Value = StripCommandArgs(entry.Value)
-				}
-				// Pass the latest entry to the final report.
-				latest = append(latest, entry)
+				node.Latest = latest
+				t.Nodes[nodeID] = node
 			}
-			node.Latest = latest
-			t.Nodes[nodeID] = node
 		}
 	})
-	return r
+	return censoredReport
 }

report/censor_test.go

@@ -0,0 +1,102 @@
+package report_test
+
+import (
+	"testing"
+
+	"github.com/weaveworks/common/test"
+	"github.com/weaveworks/scope/report"
+	"github.com/weaveworks/scope/test/reflect"
+)
+
+func TestCensorRawReport(t *testing.T) {
+	r := report.Report{
+		Container: report.Topology{
+			Nodes: report.Nodes{
+				"a": report.MakeNodeWith("a", map[string]string{
+					"docker_container_command": "prog -a --b=c",
+					"blublu":                   "blu blu",
+					"docker_env_":              "env_var",
+				}),
+			},
+		},
+		Process: report.Topology{
+			Nodes: report.Nodes{
+				"b": report.MakeNodeWith("b", map[string]string{
+					"cmdline": "scope --token=blibli",
+					"blibli":  "bli bli",
+				}),
+				"c": report.MakeNodeWith("c", map[string]string{
+					"docker_env_": "var",
+				}),
+			},
+		},
+	}
+
+	for _, c := range []struct {
+		label      string
+		have, want report.Report
+	}{
+		{
+			label: "no censoring",
+			have: report.CensorRawReport(r, report.CensorConfig{
+				HideCommandLineArguments: false,
+				HideEnvironmentVariables: false,
+			}),
+			want: report.Report{
+				Container: report.Topology{
+					Nodes: report.Nodes{
+						"a": report.MakeNodeWith("a", map[string]string{
+							"docker_container_command": "prog -a --b=c",
+							"blublu":                   "blu blu",
+							"docker_env_":              "env_var",
+						}),
+					},
+				},
+				Process: report.Topology{
+					Nodes: report.Nodes{
+						"b": report.MakeNodeWith("b", map[string]string{
+							"cmdline": "scope --token=blibli",
+							"blibli":  "bli bli",
+						}),
+						"c": report.MakeNodeWith("c", map[string]string{
+							"docker_env_": "var",
+						}),
+					},
+				},
+			},
+		},
+		// {
+		// 	label: "censor only command line args",
+		// 	have: report.CensorRawReport(r, report.CensorConfig{
+		// 		HideCommandLineArguments: true,
+		// 		HideEnvironmentVariables: false,
+		// 	}),
+		// 	want: report.Report{
+		// 		Container: report.Topology{
+		// 			Nodes: report.Nodes{
+		// 				"a": report.MakeNodeWith("a", map[string]string{
+		// 					"docker_container_command": "prog",
+		// 					"blublu":                   "blu blu",
+		// 					"docker_env_":              "env_var",
+		// 				}),
+		// 			},
+		// 		},
+		// 		Process: report.Topology{
+		// 			Nodes: report.Nodes{
+		// 				"b": report.MakeNodeWith("b", map[string]string{
+		// 					"cmdline": "scope",
+		// 					"blibli":  "bli bli",
+		// 				}),
+		// 				"c": report.MakeNodeWith("c", map[string]string{
+		// 					"docker_env_": "var",
+		// 				}),
+		// 			},
+		// 		},
+		// 	},
+		// },
+	} {
+		if !reflect.DeepEqual(c.want, c.have) {
+			t.Errorf("%s - %s", c.label, test.Diff(c.want, c.have))
+		}
+	}
+}

report/controls.go

@@ -35,6 +35,9 @@ func (cs Controls) Merge(other Controls) Controls {
 
 // Copy produces a copy of cs.
 func (cs Controls) Copy() Controls {
+	if cs == nil {
+		return nil
+	}
 	result := Controls{}
 	for k, v := range cs {
 		result[k] = v

report/dns.go

@@ -11,6 +11,9 @@ type DNSRecords map[string]DNSRecord
 
 // Copy makes a copy of the DNSRecords
 func (r DNSRecords) Copy() DNSRecords {
+	if r == nil {
+		return nil
+	}
 	cp := make(DNSRecords, len(r))
 	for k, v := range r {
 		cp[k] = v

report/table.go

@@ -136,7 +136,7 @@ func (node Node) ExtractTable(template TableTemplate) (rows []Row, truncationCou
 	truncationCount = 0
 	if str, ok := node.Latest.Lookup(truncationCountPrefix + template.Prefix); ok {
 		if n, err := fmt.Sscanf(str, "%d", &truncationCount); n != 1 || err != nil {
-			log.Warn("Unexpected truncation count format %q", str)
+			log.Warnf("Unexpected truncation count format %q", str)
 		}
 	}
 

report/topology.go

@@ -214,6 +214,9 @@ type Nodes map[string]Node
 
 // Copy returns a value copy of the Nodes.
 func (n Nodes) Copy() Nodes {
+	if n == nil {
+		return nil
+	}
 	cp := make(Nodes, len(n))
 	for k, v := range n {
 		cp[k] = v