Skip to content

Commit 9f0d2e6

Browse files
jhadvigjhadvig
committed
Address comments
1 parent f3a701a commit 9f0d2e6

File tree

4 files changed

+29
-40
lines changed

4 files changed

+29
-40
lines changed

‎cmd/bridge/main.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ func main() {
129129
fContentSecurityPolicyEnabled := fs.Bool("content-security-policy-enabled", false, "Flag to indicate if Content Secrity Policy features should be enabled.")
130130

131131
consoleCSPFlags := serverconfig.MultiKeyValue{}
132-
fs.Var(&consoleCSPFlags, "content-security-policy", "List of CSP directives that are enabled for the console. Each entry consist of csp-directive-name as a key and csp-directive-value as a value. Example: --content-security-policy ScriptSrc=\"https://example.com\" --content-security-policy FontSrc=\"https://example2.com https://example3.com\"")
132+
fs.Var(&consoleCSPFlags, "content-security-policy", "List of CSP directives that are enabled for the console. Each entry consist of csp-directive-name as a key and csp-directive-value as a value. Example: --content-security-policy script-src=\"https://example.com\" --content-security-policy font-src=\"https://example2.com https://example3.com\"")
133133

134134
telemetryFlags := serverconfig.MultiKeyValue{}
135135
fs.Var(&telemetryFlags, "telemetry", "Telemetry configuration that can be used by console plugins. Each entry should be a key=value pair.")

‎pkg/server/server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -685,7 +685,7 @@ func (s *Server) indexHandler(w http.ResponseWriter, r *http.Request) {
685685
}
686686

687687
if s.ContentSecurityPolicyEnabled {
688-
contentSecurityPolicy := utils.BuildCSPDirectives(s.K8sMode, s.ContentSecurityPolicy, indexPageScriptNonce)
688+
contentSecurityPolicy, err := utils.BuildCSPDirectives(s.K8sMode, s.ContentSecurityPolicy, indexPageScriptNonce)
689689
if err != nil {
690690
klog.Fatalf("Error building Content Security Policy directives: %s", err)
691691
os.Exit(1)

‎pkg/utils/utils.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func RandomString(length int) (string, error) {
5353

5454
// BuildCSPDirectives constructs a complete set of Content Security Policy (CSP) directives
5555
// based on the provided configuration and mode.
56-
func BuildCSPDirectives(k8sMode string, pluginsCSP serverconfig.MultiKeyValue, indexPageScriptNonce string) []string {
56+
func BuildCSPDirectives(k8sMode string, pluginsCSP serverconfig.MultiKeyValue, indexPageScriptNonce string) ([]string, error) {
5757
nonce := fmt.Sprintf("'nonce-%s'", indexPageScriptNonce)
5858

5959
// Initialize directives with default values
@@ -98,7 +98,8 @@ func BuildCSPDirectives(k8sMode string, pluginsCSP serverconfig.MultiKeyValue, i
9898
case connectSrc:
9999
directives[connectSrc] = append(directives[connectSrc], sources)
100100
default:
101-
klog.Infof("ignored unsupported CSP directive: %v", directive)
101+
klog.Errorf("ignored unsupported CSP directive: %v", directive)
102+
return nil, fmt.Errorf("unsupported CSP directive: %v", directive)
102103
}
103104
}
104105

@@ -120,5 +121,5 @@ func BuildCSPDirectives(k8sMode string, pluginsCSP serverconfig.MultiKeyValue, i
120121
for _, directive := range directiveKeys {
121122
cspDirectives = append(cspDirectives, fmt.Sprintf("%s %s", directive, strings.Join(directives[directive], " ")))
122123
}
123-
return cspDirectives
124+
return cspDirectives, nil
124125
}

‎pkg/utils/utils_test.go

Lines changed: 23 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -25,46 +25,14 @@ const (
2525
frameAncestorsDirective = "frame-ancestors 'none'"
2626
)
2727

28-
// func TestParseContentSecurityPolicyConfig(t *testing.T) {
29-
// tests := []struct {
30-
// name string
31-
// csp serverconfig.MultiKeyValue
32-
// want *map[v1.DirectiveType][]string
33-
// }{
34-
// {
35-
// name: "empty string",
36-
// csp: serverconfig.MultiKeyValue{},
37-
// want: &map[consolev1.DirectiveType][]string{},
38-
// },
39-
// {
40-
// name: "valid CSP",
41-
// csp: serverconfig.MultiKeyValue{
42-
// "DefaultSrc": "foo.bar.default",
43-
// "ScriptSrc": "foo.bar.script",
44-
// },
45-
// want: &map[consolev1.DirectiveType][]string{
46-
// "DefaultSrc": {"foo.bar.default"},
47-
// "ScriptSrc": {"foo.bar.script"},
48-
// },
49-
// },
50-
// }
51-
// for _, tt := range tests {
52-
// t.Run(tt.name, func(t *testing.T) {
53-
// parsedCSP := ParseContentSecurityPolicyConfig(tt.csp)
54-
// if !reflect.DeepEqual(parsedCSP, tt.want) {
55-
// t.Errorf("ParseContentSecurityPolicyConfig() error = %v, want %v", parsedCSP, tt.want)
56-
// }
57-
// })
58-
// }
59-
// }
60-
6128
func TestBuildCSPDirectives(t *testing.T) {
6229
tests := []struct {
6330
name string
6431
k8sMode string
6532
contentSecurityPolicy serverconfig.MultiKeyValue
6633
indexPageScriptNonce string
6734
want []string
35+
shouldError bool
6836
}{
6937
{
7038
name: "on-cluster",
@@ -82,6 +50,7 @@ func TestBuildCSPDirectives(t *testing.T) {
8250
onClusterScriptSrc + " 'unsafe-eval' 'nonce-foobar'",
8351
onClusterStyleSrc + " 'unsafe-inline'",
8452
},
53+
shouldError: false,
8554
},
8655
{
8756
name: "off-cluster",
@@ -99,6 +68,7 @@ func TestBuildCSPDirectives(t *testing.T) {
9968
offClusterScriptSrc + " 'unsafe-eval' 'nonce-foobar'",
10069
offClusterStyleSrc + " 'unsafe-inline'",
10170
},
71+
shouldError: false,
10272
},
10373
{
10474
name: "on-cluster with config",
@@ -123,6 +93,7 @@ func TestBuildCSPDirectives(t *testing.T) {
12393
onClusterScriptSrc + " foo.bar foo.bar.baz 'unsafe-eval' 'nonce-foobar'",
12494
onClusterStyleSrc + " foo.bar foo.bar.baz 'unsafe-inline'",
12595
},
96+
shouldError: false,
12697
},
12798
{
12899
name: "off-cluster with config",
@@ -147,14 +118,31 @@ func TestBuildCSPDirectives(t *testing.T) {
147118
offClusterScriptSrc + " foo.bar foo.bar.baz 'unsafe-eval' 'nonce-foobar'",
148119
offClusterStyleSrc + " foo.bar foo.bar.baz 'unsafe-inline'",
149120
},
121+
shouldError: false,
122+
},
123+
{
124+
name: "config with invalid CSP",
125+
k8sMode: "off-cluster",
126+
indexPageScriptNonce: "foobar",
127+
contentSecurityPolicy: serverconfig.MultiKeyValue{
128+
"invalid-src": "foo.bar.baz",
129+
},
130+
want: []string{},
131+
shouldError: true,
150132
},
151133
}
152134

153135
for _, tt := range tests {
154136
t.Run(tt.name, func(t *testing.T) {
155137

156-
got := BuildCSPDirectives(tt.k8sMode, tt.contentSecurityPolicy, tt.indexPageScriptNonce)
157-
if !reflect.DeepEqual(got, tt.want) {
138+
got, err := BuildCSPDirectives(tt.k8sMode, tt.contentSecurityPolicy, tt.indexPageScriptNonce)
139+
140+
if (err != nil) != tt.shouldError {
141+
t.Errorf("buildCSPDirectives() error = %v, wantErr %v", err, tt.shouldError)
142+
return
143+
}
144+
145+
if err == nil && !reflect.DeepEqual(got, tt.want) {
158146
t.Errorf("buildCSPDirectives() got = %v, want %v", got, tt.want)
159147
}
160148
})

0 commit comments

Comments
 (0)