From d58328aaef145a379e0e34373799659e067a574f Mon Sep 17 00:00:00 2001 From: hervee Date: Wed, 4 Nov 2026 15:43:54 +0100 Subject: [PATCH] WKS-3612 - Escape server-supplied action metadata in jf worker init templates --- commands/common/cmd_io.go | 10 ++++ commands/common/test_commons.go | 2 +- commands/init_cmd.go | 10 +++- commands/init_cmd_test.go | 60 +++++++++++++++++++++++ commands/templates/manifest.json_template | 10 ++-- 5 files changed, 85 insertions(+), 7 deletions(-) diff --git a/commands/common/cmd_io.go b/commands/common/cmd_io.go index 937b456..aaafb00 100644 --- a/commands/common/cmd_io.go +++ b/commands/common/cmd_io.go @@ -8,6 +8,7 @@ import ( "io" "os" "strings" + "text/template" "github.com/jfrog/jfrog-client-go/utils/log" @@ -20,6 +21,15 @@ var ( cliIn io.Reader = os.Stdin ) +// TemplateFuncs are the functions available to the scaffold templates. "json" JSON-encodes a value +// so that server-supplied data cannot break out of a JSON string literal. +var TemplateFuncs = template.FuncMap{ + "json": func(v any) (string, error) { + b, err := json.Marshal(v) + return string(b), err + }, +} + type InputReader struct { ctx *components.Context } diff --git a/commands/common/test_commons.go b/commands/common/test_commons.go index c204ea1..862f788 100644 --- a/commands/common/test_commons.go +++ b/commands/common/test_commons.go @@ -65,7 +65,7 @@ func PrepareWorkerDirForTest(t *testing.T) (string, string) { } func GenerateFromSamples(t require.TestingT, templates embed.FS, action string, workerName string, projectKey string, templateName string, skipTests ...bool) string { - tpl, err := template.New(templateName).ParseFS(templates, "templates/"+templateName) + tpl, err := template.New(templateName).Funcs(TemplateFuncs).ParseFS(templates, "templates/"+templateName) require.NoErrorf(t, err, "cannot initialize the template for %s", action) actionsMeta := LoadSampleActions(t) diff --git a/commands/init_cmd.go b/commands/init_cmd.go index c97fcd8..83ab16b 100644 --- a/commands/init_cmd.go +++ b/commands/init_cmd.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "path" + "regexp" "strings" "text/template" @@ -18,6 +19,9 @@ import ( "github.com/jfrog/jfrog-client-go/utils/log" ) +// tsIdentifierPattern matches a TypeScript identifier, the only form accepted for server-supplied names used in code templates. +var tsIdentifierPattern = regexp.MustCompile(`^[A-Za-z_$][A-Za-z0-9_$]*$`) + //go:embed templates/* var templates embed.FS @@ -116,6 +120,10 @@ func (c *initHandler) initWorker(targetDir string, action string, workerName str return err } + if actionMeta.ExecutionRequestType != "" && !tsIdentifierPattern.MatchString(actionMeta.ExecutionRequestType) { + return fmt.Errorf("invalid execution request type %q returned by the server for action %q", actionMeta.ExecutionRequestType, action) + } + generate := c.initGenerator(targetDir, workerName, projectKey, force, skipTests, actionMeta) if err := generate("package.json_template", "package.json"); err != nil { @@ -203,7 +211,7 @@ func (c *initHandler) initGenerator(targetDir string, workerName string, project } return func(templateName, outputFilename string) error { - tpl, err := template.New(templateName).ParseFS(templates, "templates/"+templateName) + tpl, err := template.New(templateName).Funcs(common.TemplateFuncs).ParseFS(templates, "templates/"+templateName) if err != nil { return err } diff --git a/commands/init_cmd_test.go b/commands/init_cmd_test.go index 0e93ce8..3ea3363 100644 --- a/commands/init_cmd_test.go +++ b/commands/init_cmd_test.go @@ -4,6 +4,7 @@ package commands import ( + "encoding/json" "fmt" "net/http" "os" @@ -75,6 +76,54 @@ func TestInitWorker(t *testing.T) { assert.NoError(t, err) }, }, + { + name: "should escape server-supplied application in manifest", + stub: common.NewServerStub(t).WithActionsMetadataEndpoint(stubActionWith(t, func(md *model.ActionMetadata) { + md.Action.Application = "artifactory\", \"sourceCodePath\": \"/etc/passwd\", \"projectKey\": \"evil\", \"x\": \"" + })), + test: func(t *testing.T, runCommand runCommandFunc) { + dir, workerName := common.PrepareWorkerDirForTest(t) + require.NoError(t, runCommand("worker", "init", "BEFORE_DOWNLOAD", workerName)) + + raw, err := os.ReadFile(path.Join(dir, "manifest.json")) + require.NoError(t, err) + require.True(t, json.Valid(raw)) + + var generic map[string]any + require.NoError(t, json.Unmarshal(raw, &generic)) + assert.NotContains(t, generic, "x") + + manifest, err := common.ReadManifest(dir) + require.NoError(t, err) + assert.Equal(t, "./worker.ts", manifest.SourceCodePath) + assert.Empty(t, manifest.ProjectKey) + assert.Equal(t, "artifactory\", \"sourceCodePath\": \"/etc/passwd\", \"projectKey\": \"evil\", \"x\": \"", manifest.Application) + }, + }, + { + name: "should reject a non-identifier executionRequestType", + stub: common.NewServerStub(t).WithActionsMetadataEndpoint(stubActionWith(t, func(md *model.ActionMetadata) { + md.ExecutionRequestType = "X } from './types'; import fs from 'fs" + })), + test: func(t *testing.T, runCommand runCommandFunc) { + dir, workerName := common.PrepareWorkerDirForTest(t) + err := runCommand("worker", "init", "BEFORE_DOWNLOAD", workerName) + assert.Regexp(t, `invalid execution request type`, err) + for _, f := range []string{"worker.spec.ts", "manifest.json", "package.json"} { + assert.NoFileExists(t, path.Join(dir, f)) + } + }, + }, + { + name: "should escape a worker name containing quotes", + test: func(t *testing.T, runCommand runCommandFunc) { + dir, _ := common.PrepareWorkerDirForTest(t) + require.NoError(t, runCommand("worker", "init", "BEFORE_DOWNLOAD", `my"worker`)) + manifest, err := common.ReadManifest(dir) + require.NoError(t, err) + assert.Equal(t, `my"worker`, manifest.Name) + }, + }, { name: "generate", test: testGenerateAllActions, @@ -248,6 +297,17 @@ func testForceOverwriteTSConfigFromServer(t *testing.T, runCommand runCommandFun assert.Equal(t, `{"compilerOptions":{"strict":true,"fromServer":true}}`, string(got)) } +// stubActionWith returns the sample actions where BEFORE_DOWNLOAD has been altered by modify. +func stubActionWith(t *testing.T, modify func(md *model.ActionMetadata)) common.ActionsMetadata { + actions := common.LoadSampleActions(t) + for _, md := range actions { + if md.Action.Name == "BEFORE_DOWNLOAD" { + modify(md) + } + } + return actions +} + func testGenerateAction(actionName string, withTests bool, runCommand runCommandFunc) func(t *testing.T) { return func(t *testing.T) { dir, workerName := common.PrepareWorkerDirForTest(t) diff --git a/commands/templates/manifest.json_template b/commands/templates/manifest.json_template index fd76810..b13f850 100644 --- a/commands/templates/manifest.json_template +++ b/commands/templates/manifest.json_template @@ -1,6 +1,6 @@ { - "name": "{{.WorkerName}}", - "description": "Run a script on {{.Action}}", + "name": {{json .WorkerName}}, + "description": {{json (printf "Run a script on %s" .Action)}}, {{- if .HasRepoFilterCriteria }} "filterCriteria": { "artifactFilterCriteria": { @@ -19,9 +19,9 @@ "secrets": {}, "properties": {}, "sourceCodePath": "./worker.ts", - "action": "{{.Action}}", + "action": {{json .Action}}, "enabled": false, "debug": false, - "projectKey": "{{.ProjectKey}}", - "application": "{{.Application}}" + "projectKey": {{json .ProjectKey}}, + "application": {{json .Application}} } \ No newline at end of file