From 93f87a0f07d735bd7b4ce86cbcd684d429d243a5 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sat, 9 May 2026 12:46:02 -0400 Subject: [PATCH 1/3] fix: restrict app names to prevent command injection The previous app name validation regex permitted shell metacharacters such as `;`, `$`, backticks, `|`, and `&`. These names were embedded unquoted into the generated git pre-receive hook script, allowing an authenticated user to execute arbitrary commands as the dokku user simply by pushing to a remote with a crafted app name. App names are now restricted to lowercase alphanumerics, dots, and hyphens, and the hook script also quotes the app variable as a defense-in-depth measure. --- plugins/common/common.go | 8 ++--- plugins/common/common_test.go | 54 ++++++++++++++++++++++++++++++++++ plugins/common/functions | 16 ++++------ plugins/git/internal-functions | 2 +- tests/unit/apps_1.bats | 20 +++++++++++++ 5 files changed, 85 insertions(+), 15 deletions(-) diff --git a/plugins/common/common.go b/plugins/common/common.go index a3ec11acd..d6833bae2 100644 --- a/plugins/common/common.go +++ b/plugins/common/common.go @@ -923,12 +923,12 @@ func IsValidAppName(appName string) error { return errors.New("Please specify an app to run the command on") } - r, _ := regexp.Compile("^[a-z0-9][^/:_A-Z]*$") + r, _ := regexp.Compile("^[a-z0-9][a-z0-9.-]*$") if r.MatchString(appName) { return nil } - return errors.New("App name must begin with lowercase alphanumeric character, and cannot include uppercase characters, colons, or underscores") + return errors.New("App name must begin with lowercase alphanumeric character, and may only contain lowercase alphanumerics, dots, and hyphens") } // isValidAppNameOld verifies that the app name matches the old naming restrictions @@ -937,12 +937,12 @@ func isValidAppNameOld(appName string) error { return errors.New("Please specify an app to run the command on") } - r, _ := regexp.Compile("^[a-z0-9][^/:A-Z]*$") + r, _ := regexp.Compile("^[a-z0-9][a-z0-9.-]*$") if r.MatchString(appName) { return nil } - return errors.New("App name must begin with lowercase alphanumeric character, and cannot include uppercase characters, or colons") + return errors.New("App name must begin with lowercase alphanumeric character, and may only contain lowercase alphanumerics, dots, and hyphens") } // AppDoesNotExist wraps error to include the app name diff --git a/plugins/common/common_test.go b/plugins/common/common_test.go index f6fa98123..14c0ef032 100644 --- a/plugins/common/common_test.go +++ b/plugins/common/common_test.go @@ -77,6 +77,60 @@ func TestCommonVerifyAppNameInvalid(t *testing.T) { Expect(VerifyAppName("1994testApp")).To(HaveOccurred()) } +func TestCommonIsValidAppName(t *testing.T) { + RegisterTestingT(t) + validNames := []string{ + "myapp", + "my-app", + "my.app", + "app123", + "123app", + "my-app.prod", + "a", + "0", + } + for _, name := range validNames { + Expect(IsValidAppName(name)).To(Succeed(), "expected %q to be a valid app name", name) + } +} + +func TestCommonIsValidAppNameRejectsShellMetacharacters(t *testing.T) { + RegisterTestingT(t) + invalidNames := []string{ + "app;id", + "app$cmd", + "app`whoami`", + "app|cat", + "app&background", + "app>file", + "app Date: Sat, 9 May 2026 13:02:54 -0400 Subject: [PATCH 2/3] refactor: route bash app name validation through go The bash and go validators previously each kept their own copy of the regex, which had to be updated in lockstep. Both bash wrappers now invoke the existing common binary via the same pattern as `verify_app_name`, leaving go as the single source of truth. The legacy `IsValidAppNameOld` rule is also widened to allow underscores again so apps created under the old naming rules can still be looked up through `VerifyAppName`'s either-rule fallback. --- plugins/common/common.go | 10 +++++----- plugins/common/common_test.go | 30 +++++++++++++++++++++++++++++ plugins/common/functions | 12 ++---------- plugins/common/src/common/common.go | 6 ++++++ 4 files changed, 43 insertions(+), 15 deletions(-) diff --git a/plugins/common/common.go b/plugins/common/common.go index d6833bae2..227c18b50 100644 --- a/plugins/common/common.go +++ b/plugins/common/common.go @@ -931,18 +931,18 @@ func IsValidAppName(appName string) error { return errors.New("App name must begin with lowercase alphanumeric character, and may only contain lowercase alphanumerics, dots, and hyphens") } -// isValidAppNameOld verifies that the app name matches the old naming restrictions -func isValidAppNameOld(appName string) error { +// IsValidAppNameOld verifies that the app name matches the old naming restrictions +func IsValidAppNameOld(appName string) error { if appName == "" { return errors.New("Please specify an app to run the command on") } - r, _ := regexp.Compile("^[a-z0-9][a-z0-9.-]*$") + r, _ := regexp.Compile("^[a-z0-9][a-z0-9._-]*$") if r.MatchString(appName) { return nil } - return errors.New("App name must begin with lowercase alphanumeric character, and may only contain lowercase alphanumerics, dots, and hyphens") + return errors.New("App name must begin with lowercase alphanumeric character, and may only contain lowercase alphanumerics, dots, hyphens, and underscores") } // AppDoesNotExist wraps error to include the app name @@ -981,7 +981,7 @@ func VarArgs(arguments []string, skip int) []string { // naming conventions exists func VerifyAppName(appName string) error { newErr := IsValidAppName(appName) - oldErr := isValidAppNameOld(appName) + oldErr := IsValidAppNameOld(appName) if newErr != nil && oldErr != nil { return newErr } diff --git a/plugins/common/common_test.go b/plugins/common/common_test.go index 14c0ef032..0123a6c56 100644 --- a/plugins/common/common_test.go +++ b/plugins/common/common_test.go @@ -94,6 +94,36 @@ func TestCommonIsValidAppName(t *testing.T) { } } +func TestCommonIsValidAppNameOld(t *testing.T) { + RegisterTestingT(t) + validNames := []string{ + "myapp", + "my-app", + "my.app", + "my_app", + "legacy_app_name", + "app_v2", + } + for _, name := range validNames { + Expect(IsValidAppNameOld(name)).To(Succeed(), "expected %q to be a valid app name", name) + } + + invalidNames := []string{ + "app;id", + "app$cmd", + "app|cat", + "app/cmd", + "app:cmd", + "App", + "-app", + "_app", + "", + } + for _, name := range invalidNames { + Expect(IsValidAppNameOld(name)).To(HaveOccurred(), "expected %q to be rejected", name) + } +} + func TestCommonIsValidAppNameRejectsShellMetacharacters(t *testing.T) { RegisterTestingT(t) invalidNames := []string{ diff --git a/plugins/common/functions b/plugins/common/functions index 2d9877810..c933c4791 100755 --- a/plugins/common/functions +++ b/plugins/common/functions @@ -222,22 +222,14 @@ fn-is-valid-app-name() { declare desc="verify that the app name matches naming restrictions" local APP="$1" [[ -z "$APP" ]] && dokku_log_fail "Please specify an app to run the command on" - if [[ "$APP" =~ ^[a-z0-9][a-z0-9.-]*$ ]]; then - return 0 - fi - - return 1 + "$PLUGIN_CORE_AVAILABLE_PATH/common/common" --quiet is-valid-app-name "$APP" } fn-is-valid-app-name-old() { declare desc="verify that the app name matches the old naming restrictions" local APP="$1" [[ -z "$APP" ]] && dokku_log_fail "Please specify an app to run the command on" - if [[ "$APP" =~ ^[a-z0-9][a-z0-9.-]*$ ]]; then - return 0 - fi - - return 1 + "$PLUGIN_CORE_AVAILABLE_PATH/common/common" --quiet is-valid-app-name-old "$APP" } fn-plugn-trigger-exists() { diff --git a/plugins/common/src/common/common.go b/plugins/common/src/common/common.go index 5c95746aa..58412f591 100644 --- a/plugins/common/src/common/common.go +++ b/plugins/common/src/common/common.go @@ -86,6 +86,12 @@ func main() { } else { fmt.Print("false") } + case "is-valid-app-name": + appName := flag.Arg(1) + err = common.IsValidAppName(appName) + case "is-valid-app-name-old": + appName := flag.Arg(1) + err = common.IsValidAppNameOld(appName) case "verify-app-name": appName := flag.Arg(1) err = common.VerifyAppName(appName) From 4f4fed710117f83d6064015b81d622f6782ffc6a Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sat, 9 May 2026 16:06:27 -0400 Subject: [PATCH 3/3] test: update apps:rename assertions for quoted git-hook app name The security fix that quoted `$APP` inside the pre-receive hook heredoc changed the literal hook contents from `dokku git-hook foo` to `dokku git-hook "foo"`, so the existing substring assertions no longer match. --- tests/unit/apps_1.bats | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/apps_1.bats b/tests/unit/apps_1.bats index 6746c9d3a..e1dbab0c0 100644 --- a/tests/unit/apps_1.bats +++ b/tests/unit/apps_1.bats @@ -176,7 +176,7 @@ teardown() { echo "output: $output" echo "status: $status" assert_success - assert_output_contains "git-hook $TEST_APP" + assert_output_contains "git-hook \"$TEST_APP\"" run /bin/bash -c "dokku apps:rename $TEST_APP great-test-name" echo "output: $output" echo "status: $status" @@ -185,7 +185,7 @@ teardown() { echo "output: $output" echo "status: $status" assert_success - assert_output_contains "git-hook great-test-name" + assert_output_contains "git-hook \"great-test-name\"" run /bin/bash -c "dokku apps:list | grep $TEST_APP" echo "output: $output" echo "status: $status"