From 93f87a0f07d735bd7b4ce86cbcd684d429d243a5 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sat, 9 May 2026 12:46:02 -0400 Subject: [PATCH] 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