Skip to content

Commit fec14b0

Browse files
freeznetfreeznet
committed
fix(auth): ensure proper shell quoting for auth parameters
1 parent 2587caf commit fec14b0

5 files changed

Lines changed: 192 additions & 19 deletions

File tree

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,3 +37,4 @@ index_build_*/
3737
config/crd/bases/vpa-v1-crd.yaml
3838
Listeners
3939
.tool-versions
40+
/tmp

‎api/compute/v1alpha1/common.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ func (o *OAuth2Config) GetMountFile() string {
127127
}
128128

129129
func (o *OAuth2Config) AuthenticationParameters() string {
130-
return fmt.Sprintf(`'{"credentials_url":"file://%s","privateKey":"%s","private_key":"%s","issuerUrl":"%s","issuer_url":"%s","audience":"%s","scope":"%s"}'`, o.GetMountFile(), o.GetMountFile(), o.GetMountFile(), o.IssuerURL, o.IssuerURL, o.Audience, o.Scope)
130+
return fmt.Sprintf(`{"credentials_url":"file://%s","privateKey":"%s","private_key":"%s","issuerUrl":"%s","issuer_url":"%s","audience":"%s","scope":"%s"}`, o.GetMountFile(), o.GetMountFile(), o.GetMountFile(), o.IssuerURL, o.IssuerURL, o.Audience, o.Scope)
131131
}
132132

133133
type GenericAuth struct {

‎controllers/spec/common.go‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -831,12 +831,16 @@ func getOAuth2MountFile(authConfig *v1alpha1.OAuth2Config, mountPath string) str
831831
return fmt.Sprintf("%s/%s", mountPath, authConfig.KeySecretKey)
832832
}
833833

834+
func shellQuoteLiteral(value string) string {
835+
return "'" + strings.ReplaceAll(value, "'", `'"'"'`) + "'"
836+
}
837+
834838
func makeOAuth2AuthenticationParameters(authConfig *v1alpha1.OAuth2Config, mountPath string) string {
835839
if authConfig == nil {
836840
return ""
837841
}
838842
credentialsFile := getOAuth2MountFile(authConfig, mountPath)
839-
return fmt.Sprintf(`'{"credentials_url":"file://%s","privateKey":"%s","private_key":"%s","issuerUrl":"%s","issuer_url":"%s","audience":"%s","scope":"%s"}'`,
843+
return fmt.Sprintf(`{"credentials_url":"file://%s","privateKey":"%s","private_key":"%s","issuerUrl":"%s","issuer_url":"%s","audience":"%s","scope":"%s"}`,
840844
credentialsFile, credentialsFile, credentialsFile, authConfig.IssuerURL, authConfig.IssuerURL, authConfig.Audience, authConfig.Scope)
841845
}
842846

@@ -859,14 +863,14 @@ func getPulsarAdminCommandWithEnv(authProvided, tlsProvided bool, tlsConfig TLSC
859863
"--auth-plugin",
860864
OAuth2AuthenticationPlugin,
861865
"--auth-params",
862-
makeOAuth2AuthenticationParameters(authConfig.OAuth2Config, oauth2MountPath),
866+
shellQuoteLiteral(makeOAuth2AuthenticationParameters(authConfig.OAuth2Config, oauth2MountPath)),
863867
}...)
864868
} else if authConfig.GenericAuth != nil {
865869
args = append(args, []string{
866870
"--auth-plugin",
867871
authConfig.GenericAuth.ClientAuthenticationPlugin,
868872
"--auth-params",
869-
"'" + authConfig.GenericAuth.ClientAuthenticationParameters + "'",
873+
shellQuoteLiteral(authConfig.GenericAuth.ClientAuthenticationParameters),
870874
}...)
871875
}
872876
} else if authProvided {
@@ -960,13 +964,13 @@ func getPulsarctlCommandWithEnv(authProvided, tlsProvided bool, tlsConfig TLSCon
960964
"oauth2",
961965
"activate",
962966
"--auth-params",
963-
"'" + authConfig.GenericAuth.ClientAuthenticationParameters + "'",
967+
shellQuoteLiteral(authConfig.GenericAuth.ClientAuthenticationParameters),
964968
"|| true ) &&",
965969
PulsarctlExecutableFile,
966970
"--auth-plugin",
967971
authConfig.GenericAuth.ClientAuthenticationPlugin,
968972
"--auth-params",
969-
"'" + authConfig.GenericAuth.ClientAuthenticationParameters + "'",
973+
shellQuoteLiteral(authConfig.GenericAuth.ClientAuthenticationParameters),
970974
"--admin-service-url",
971975
"$" + envNames.webServiceURL,
972976
}
@@ -1610,7 +1614,7 @@ func getSharedArgs(details, clusterName, uid string, authProvided bool, tlsProvi
16101614
"--function_version",
16111615
"0",
16121616
"--function_details",
1613-
"'" + details + "'", //in json format
1617+
shellQuoteLiteral(details), // in json format
16141618
"--pulsar_serviceurl",
16151619
"$brokerServiceURL",
16161620
"--max_buffered_tuples",
@@ -1631,13 +1635,13 @@ func getSharedArgs(details, clusterName, uid string, authProvided bool, tlsProvi
16311635
"--client_auth_plugin",
16321636
OAuth2AuthenticationPlugin,
16331637
"--client_auth_params",
1634-
authConfig.OAuth2Config.AuthenticationParameters()}...)
1638+
shellQuoteLiteral(authConfig.OAuth2Config.AuthenticationParameters())}...)
16351639
} else if authConfig.GenericAuth != nil {
16361640
args = append(args, []string{
16371641
"--client_auth_plugin",
16381642
authConfig.GenericAuth.ClientAuthenticationPlugin,
16391643
"--client_auth_params",
1640-
"'" + authConfig.GenericAuth.ClientAuthenticationParameters + "'"}...)
1644+
shellQuoteLiteral(authConfig.GenericAuth.ClientAuthenticationParameters)}...)
16411645
}
16421646
} else if authProvided {
16431647
args = append(args, []string{

‎controllers/spec/common_test.go‎

Lines changed: 65 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -205,11 +205,11 @@ func TestGetDownloadCommand(t *testing.T) {
205205
"oauth2",
206206
"activate",
207207
"--auth-params",
208-
"'auth-params'",
208+
shellQuoteLiteral("auth-params"),
209209
"|| true ) &&",
210210
PulsarctlExecutableFile,
211211
"--auth-plugin", "auth-plugin",
212-
"--auth-params", "'auth-params'",
212+
"--auth-params", shellQuoteLiteral("auth-params"),
213213
"--admin-service-url", "$webServiceURL",
214214
"--tls-allow-insecure=true",
215215
"--tls-enable-hostname-verification=false",
@@ -239,7 +239,7 @@ func TestGetDownloadCommand(t *testing.T) {
239239
PulsarAdminExecutableFile,
240240
"--admin-url", "$webServiceURL",
241241
"--auth-plugin", OAuth2AuthenticationPlugin,
242-
"--auth-params", testOauth2.AuthenticationParameters(),
242+
"--auth-params", shellQuoteLiteral(testOauth2.AuthenticationParameters()),
243243
"packages", "download", "function://public/default/test@v1", "--path", "function-package.jar",
244244
},
245245
false,
@@ -252,7 +252,7 @@ func TestGetDownloadCommand(t *testing.T) {
252252
PulsarAdminExecutableFile,
253253
"--admin-url", "$webServiceURL",
254254
"--auth-plugin", "auth-plugin",
255-
"--auth-params", "'auth-params'",
255+
"--auth-params", shellQuoteLiteral("auth-params"),
256256
"packages", "download", "sink://public/default/test@v1", "--path", "sink-package.jar",
257257
},
258258
false,
@@ -335,6 +335,67 @@ func TestGetDownloadCommand(t *testing.T) {
335335
}
336336
}
337337

338+
func TestShellQuoteLiteral(t *testing.T) {
339+
testCases := []struct {
340+
name string
341+
input string
342+
expected string
343+
}{
344+
{
345+
name: "empty",
346+
input: "",
347+
expected: "''",
348+
},
349+
{
350+
name: "plain",
351+
input: "auth-params",
352+
expected: "'auth-params'",
353+
},
354+
{
355+
name: "embedded single quote",
356+
input: "a'b",
357+
expected: `'a'"'"'b'`,
358+
},
359+
{
360+
name: "time partition pattern",
361+
input: "yyyy-MM-dd/HH'h'-mm'm'",
362+
expected: `'yyyy-MM-dd/HH'"'"'h'"'"'-mm'"'"'m'"'"''`,
363+
},
364+
}
365+
366+
for _, tc := range testCases {
367+
t.Run(tc.name, func(t *testing.T) {
368+
assert.Equal(t, tc.expected, shellQuoteLiteral(tc.input))
369+
})
370+
}
371+
}
372+
373+
func TestGetPulsarAdminCommandWithQuotedAuthParams(t *testing.T) {
374+
authConfig := &v1alpha1.AuthConfig{
375+
GenericAuth: &v1alpha1.GenericAuth{
376+
ClientAuthenticationPlugin: "auth-plugin",
377+
ClientAuthenticationParameters: `{"token":"a'b"}`,
378+
},
379+
}
380+
381+
command := getPulsarAdminCommand(false, false, nil, authConfig)
382+
assert.Equal(t, "auth-plugin", command[4])
383+
assert.Equal(t, shellQuoteLiteral(`{"token":"a'b"}`), command[6])
384+
}
385+
386+
func TestGetPulsarctlCommandWithQuotedAuthParams(t *testing.T) {
387+
authConfig := &v1alpha1.AuthConfig{
388+
GenericAuth: &v1alpha1.GenericAuth{
389+
ClientAuthenticationPlugin: "auth-plugin",
390+
ClientAuthenticationParameters: `{"token":"a'b"}`,
391+
},
392+
}
393+
394+
command := getPulsarctlCommand(false, false, nil, authConfig)
395+
assert.Equal(t, shellQuoteLiteral(`{"token":"a'b"}`), command[5])
396+
assert.Equal(t, shellQuoteLiteral(`{"token":"a'b"}`), command[11])
397+
}
398+
338399
func TestGetFunctionRunnerImage(t *testing.T) {
339400
javaRuntime := v1alpha1.Runtime{Java: &v1alpha1.JavaRuntime{
340401
Jar: "test.jar",

‎controllers/spec/function_test.go‎

Lines changed: 113 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ package spec
1919

2020
import (
2121
"encoding/json"
22-
"regexp"
2322
"strings"
2423
"testing"
2524

@@ -198,11 +197,9 @@ func TestJavaFunctionCommandWithConnectorOverrides(t *testing.T) {
198197
startCommand := commands[2]
199198
assert.Assert(t, strings.Contains(startCommand, "--connectors_directory "+DefaultConnectorsDirectory),
200199
"start command should include connectors directory but got %s", startCommand)
201-
re := regexp.MustCompile(`--function_details '([^']+)'`)
202-
matches := re.FindStringSubmatch(startCommand)
203-
assert.Assert(t, len(matches) == 2, "unable to locate function details in command: %s", startCommand)
204-
205-
functionDetailsJSON := matches[1]
200+
functionDetailsJSON := generateFunctionDetailsInJSON(function)
201+
assert.Assert(t, strings.Contains(startCommand, "--function_details "+shellQuoteLiteral(functionDetailsJSON)),
202+
"start command should include shell quoted function details but got %s", startCommand)
206203
details := &proto.FunctionDetails{}
207204
err := protojson.Unmarshal([]byte(functionDetailsJSON), details)
208205
assert.NilError(t, err)
@@ -232,6 +229,116 @@ func TestJavaFunctionCommandWithConnectorOverrides(t *testing.T) {
232229
assert.Equal(t, producerConfig["enable.idempotence"], true)
233230
}
234231

232+
func TestSinkCommandShellQuotesFunctionDetails(t *testing.T) {
233+
replicas := int32(1)
234+
trueVal := true
235+
sinkConfig := v1alpha1.NewConfig(map[string]interface{}{
236+
"partitionerType": "time",
237+
"timePartitionDuration": "1m",
238+
"timePartitionPattern": "yyyy-MM-dd/HH'h'-mm'm'",
239+
})
240+
sink := &v1alpha1.Sink{
241+
TypeMeta: metav1.TypeMeta{
242+
Kind: "Sink",
243+
APIVersion: "compute.functionmesh.io/v1alpha1",
244+
},
245+
ObjectMeta: *makeSampleObjectMeta("time-pattern-sink"),
246+
Spec: v1alpha1.SinkSpec{
247+
Name: "time-pattern-sink",
248+
ClassName: "org.apache.pulsar.io.jcloud.sink.CloudStorageGenericRecordSink",
249+
Tenant: "public",
250+
Namespace: "default",
251+
ClusterName: TestClusterName,
252+
Input: v1alpha1.InputConf{
253+
Topics: []string{
254+
"persistent://public/default/input",
255+
},
256+
TypeClassName: "org.apache.pulsar.client.api.schema.GenericRecord",
257+
},
258+
SinkConfig: &sinkConfig,
259+
Replicas: &replicas,
260+
AutoAck: &trueVal,
261+
Messaging: v1alpha1.Messaging{
262+
Pulsar: &v1alpha1.PulsarMessaging{
263+
PulsarConfig: TestClusterName,
264+
},
265+
},
266+
Runtime: v1alpha1.Runtime{
267+
Java: &v1alpha1.JavaRuntime{
268+
Jar: "connectors/pulsar-io-cloud-storage.nar",
269+
JarLocation: "",
270+
},
271+
},
272+
Image: "streamnative/pulsar-io-cloud-storage:latest",
273+
},
274+
}
275+
276+
commands := MakeSinkCommand(sink)
277+
assert.Assert(t, len(commands) == 3, "commands should be 3 but got %d", len(commands))
278+
279+
sinkDetailsJSON := generateSinkDetailsInJSON(sink)
280+
assert.Assert(t, strings.Contains(commands[2], "--function_details "+shellQuoteLiteral(sinkDetailsJSON)),
281+
"sink command should include shell quoted function details but got %s", commands[2])
282+
assert.Assert(t, strings.Contains(commands[2], `'"'"'`),
283+
"sink command should escape embedded single quotes but got %s", commands[2])
284+
}
285+
286+
func TestSinkCommandShellQuotesClientAuthParams(t *testing.T) {
287+
replicas := int32(1)
288+
trueVal := true
289+
authParams := `{"token":"a'b"}`
290+
sink := &v1alpha1.Sink{
291+
TypeMeta: metav1.TypeMeta{
292+
Kind: "Sink",
293+
APIVersion: "compute.functionmesh.io/v1alpha1",
294+
},
295+
ObjectMeta: *makeSampleObjectMeta("auth-config-sink"),
296+
Spec: v1alpha1.SinkSpec{
297+
Name: "auth-config-sink",
298+
ClassName: "org.apache.pulsar.io.elasticsearch.ElasticSearchSink",
299+
Tenant: "public",
300+
Namespace: "default",
301+
ClusterName: TestClusterName,
302+
Input: v1alpha1.InputConf{
303+
Topics: []string{
304+
"persistent://public/default/input",
305+
},
306+
TypeClassName: "[B",
307+
},
308+
SinkConfig: &v1alpha1.Config{
309+
Data: map[string]interface{}{
310+
"elasticSearchUrl": "http://localhost:9200",
311+
},
312+
},
313+
Replicas: &replicas,
314+
AutoAck: &trueVal,
315+
Messaging: v1alpha1.Messaging{
316+
Pulsar: &v1alpha1.PulsarMessaging{
317+
PulsarConfig: TestClusterName,
318+
AuthConfig: &v1alpha1.AuthConfig{
319+
GenericAuth: &v1alpha1.GenericAuth{
320+
ClientAuthenticationPlugin: "auth-plugin",
321+
ClientAuthenticationParameters: authParams,
322+
},
323+
},
324+
},
325+
},
326+
Runtime: v1alpha1.Runtime{
327+
Java: &v1alpha1.JavaRuntime{
328+
Jar: "connectors/pulsar-io-elastic-search.nar",
329+
JarLocation: "",
330+
},
331+
},
332+
Image: "streamnative/pulsar-io-elastic-search:latest",
333+
},
334+
}
335+
336+
commands := MakeSinkCommand(sink)
337+
assert.Assert(t, len(commands) == 3, "commands should be 3 but got %d", len(commands))
338+
assert.Assert(t, strings.Contains(commands[2], "--client_auth_params "+shellQuoteLiteral(authParams)),
339+
"sink command should shell quote client auth params but got %s", commands[2])
340+
}
341+
235342
func TestFunctionPulsarPackageServiceDownloadCommandAndPodWiring(t *testing.T) {
236343
previous := utils.EnableInitContainers
237344
defer func() {

0 commit comments

Comments
 (0)