From 8f4ff5b39b8120d63e7957251a732f641391f71d Mon Sep 17 00:00:00 2001 From: divyansh42 Date: Wed, 2 Sep 2026 13:24:13 +0530 Subject: [PATCH 1/3] fix(plugins): reject relative plugin dirs to prevent cwd fallback (CWE-426) FindPlugin() silently discarded the error from getPluginDir(), so when $HOME was unresolvable in minimal environments (scratch containers, pods running as arbitrary UIDs), filepath.Join("", "tkn-") produced a relative path. syscall.Exec then resolved it against the current working directory, allowing an attacker who controls cwd to execute an arbitrary binary. Additionally, TKN_PLUGINS_DIR and XDG_CONFIG_HOME were accepted even when set to relative paths, creating the same cwd-resolution risk without any home-dir failure. Fix: getPluginDir() now returns an error for non-absolute env var paths. FindPlugin() properly handles the error and skips the dir lookup instead of silently falling through with an empty string. Fixes: SRVKP-13570 Signed-off-by: Divyanshu Agrawal --- pkg/plugins/plugins.go | 19 ++++++++++++------- pkg/plugins/plugins_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/pkg/plugins/plugins.go b/pkg/plugins/plugins.go index a2154f786a..7b9f48b2cb 100644 --- a/pkg/plugins/plugins.go +++ b/pkg/plugins/plugins.go @@ -20,10 +20,16 @@ func getPluginDir() (string, error) { dir := os.Getenv(pluginDirEnv) // if TKN_PLUGINS_DIR is set, follow it if dir != "" { + if !filepath.IsAbs(dir) { + return "", fmt.Errorf("plugin dir %q is not an absolute path", dir) + } return dir, nil } // Respect XDG_CONFIG_HOME if set if xdgHome := os.Getenv("XDG_CONFIG_HOME"); xdgHome != "" { + if !filepath.IsAbs(xdgHome) { + return "", fmt.Errorf("XDG_CONFIG_HOME %q is not an absolute path", xdgHome) + } return filepath.Join(xdgHome, "tkn", "plugins"), nil } // Fallback to default pluginDir (~/.config/tkn/plugins) @@ -33,15 +39,14 @@ func getPluginDir() (string, error) { // Find a binary in plugin homedir directory or user paths. func FindPlugin(pluginame string) (string, error) { cmd := tknPrefix + pluginame - dir, _ := getPluginDir() - path := filepath.Join(dir, cmd) - _, err := os.Stat(path) - if err == nil { - // Found in dir - return path, nil + if dir, err := getPluginDir(); err == nil { + path := filepath.Join(dir, cmd) + if _, err := os.Stat(path); err == nil { + return path, nil + } } - path, err = exec.LookPath(cmd) + path, err := exec.LookPath(cmd) if err == nil { return path, nil } diff --git a/pkg/plugins/plugins_test.go b/pkg/plugins/plugins_test.go index 404b5a5de0..8ff82f7c5f 100644 --- a/pkg/plugins/plugins_test.go +++ b/pkg/plugins/plugins_test.go @@ -3,6 +3,7 @@ package plugins import ( "fmt" "os" + "path/filepath" "testing" "gotest.tools/v3/assert" @@ -49,6 +50,35 @@ func TestGetAllTknPluginFromPathPlugindir(t *testing.T) { assert.Equal(t, paths[0], "fromplugindir") } +func TestGetPluginDirRelativeTKNPluginsDir(t *testing.T) { + t.Setenv(pluginDirEnv, "relative/path") + _, err := getPluginDir() + assert.ErrorContains(t, err, "not an absolute path") +} + +func TestGetPluginDirRelativeXDGConfigHome(t *testing.T) { + t.Setenv(pluginDirEnv, "") + t.Setenv("XDG_CONFIG_HOME", "relative/xdg") + _, err := getPluginDir() + assert.ErrorContains(t, err, "not an absolute path") +} + +func TestFindPluginSkipsCwdWhenPluginDirFails(t *testing.T) { + nd := fs.NewDir(t, "TestFindPluginCwd") + defer nd.Remove() + err := os.WriteFile(nd.Join("tkn-evil"), []byte("evil"), 0o700) + assert.NilError(t, err) + + // Simulate relative TKN_PLUGINS_DIR so getPluginDir() returns error + t.Setenv(pluginDirEnv, "relative/bad/path") + t.Setenv("PATH", nd.Path()) + + // Plugin is in PATH so it should still be found via LookPath + path, err := FindPlugin("evil") + assert.NilError(t, err) + assert.Assert(t, filepath.IsAbs(path), "expected absolute path, got %q", path) +} + // as well tested differently in root_test.go func TestGetAllTknPluginFromPaths(t *testing.T) { nd := fs.NewDir(t, "TestGetAllTknPluginFromPaths1") From 67d65d824a837d046d3b3a4136e923fc4b581d59 Mon Sep 17 00:00:00 2001 From: divyansh42 Date: Wed, 2 Sep 2026 14:03:48 +0530 Subject: [PATCH 2/3] fix(plugins): validate homedir-expanded path is absolute (Copilot review) homedir.Expand accepts $HOME verbatim, so HOME=relative/dir yielded a relative plugin dir without an error. Add an IsAbs check on the expanded result to cover this case. Also replace the cwd regression test with one that actually exercises the attack path: chdir into the malicious dir, keep it off PATH, and assert FindPlugin returns an error rather than the cwd binary. Signed-off-by: Divyanshu Agrawal --- pkg/plugins/plugins.go | 9 ++++++++- pkg/plugins/plugins_test.go | 21 +++++++++++++-------- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/pkg/plugins/plugins.go b/pkg/plugins/plugins.go index 7b9f48b2cb..7b4652dce6 100644 --- a/pkg/plugins/plugins.go +++ b/pkg/plugins/plugins.go @@ -33,7 +33,14 @@ func getPluginDir() (string, error) { return filepath.Join(xdgHome, "tkn", "plugins"), nil } // Fallback to default pluginDir (~/.config/tkn/plugins) - return homedir.Expand(pluginDir) + dir, err := homedir.Expand(pluginDir) + if err != nil { + return "", err + } + if !filepath.IsAbs(dir) { + return "", fmt.Errorf("plugin dir %q is not an absolute path", dir) + } + return dir, nil } // Find a binary in plugin homedir directory or user paths. diff --git a/pkg/plugins/plugins_test.go b/pkg/plugins/plugins_test.go index 8ff82f7c5f..726de4808a 100644 --- a/pkg/plugins/plugins_test.go +++ b/pkg/plugins/plugins_test.go @@ -3,7 +3,6 @@ package plugins import ( "fmt" "os" - "path/filepath" "testing" "gotest.tools/v3/assert" @@ -63,20 +62,26 @@ func TestGetPluginDirRelativeXDGConfigHome(t *testing.T) { assert.ErrorContains(t, err, "not an absolute path") } -func TestFindPluginSkipsCwdWhenPluginDirFails(t *testing.T) { +func TestFindPluginDoesNotFallBackToCwd(t *testing.T) { nd := fs.NewDir(t, "TestFindPluginCwd") defer nd.Remove() err := os.WriteFile(nd.Join("tkn-evil"), []byte("evil"), 0o700) assert.NilError(t, err) - // Simulate relative TKN_PLUGINS_DIR so getPluginDir() returns error + // Change into the directory that contains the malicious binary. + orig, err := os.Getwd() + assert.NilError(t, err) + defer os.Chdir(orig) //nolint:errcheck + assert.NilError(t, os.Chdir(nd.Path())) + + // Force getPluginDir() to fail via a relative TKN_PLUGINS_DIR. + // Keep nd off PATH so LookPath cannot find the binary either. t.Setenv(pluginDirEnv, "relative/bad/path") - t.Setenv("PATH", nd.Path()) + t.Setenv("PATH", "") - // Plugin is in PATH so it should still be found via LookPath - path, err := FindPlugin("evil") - assert.NilError(t, err) - assert.Assert(t, filepath.IsAbs(path), "expected absolute path, got %q", path) + // The binary is only reachable via cwd — FindPlugin must not find it. + _, err = FindPlugin("evil") + assert.ErrorContains(t, err, "cannot find plugin") } // as well tested differently in root_test.go From ddfe50ef75f13e05d0bfdbbf2b130d9bf354c3a7 Mon Sep 17 00:00:00 2001 From: divyansh42 Date: Wed, 2 Sep 2026 14:50:46 +0530 Subject: [PATCH 3/3] test(plugins): use TKN_PLUGINS_DIR=. to actually catch the cwd fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous test used a non-existent relative path, so both old and new code returned "cannot find plugin" — the test couldn't distinguish them. Using "." means the old code resolves filepath.Join(".", "tkn-evil") against cwd and finds the binary (fail), while the fixed code rejects "." as non-absolute (pass), proving the vulnerable path is closed. Signed-off-by: Divyanshu Agrawal --- pkg/plugins/plugins_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/plugins/plugins_test.go b/pkg/plugins/plugins_test.go index 726de4808a..d89896c417 100644 --- a/pkg/plugins/plugins_test.go +++ b/pkg/plugins/plugins_test.go @@ -74,9 +74,10 @@ func TestFindPluginDoesNotFallBackToCwd(t *testing.T) { defer os.Chdir(orig) //nolint:errcheck assert.NilError(t, os.Chdir(nd.Path())) - // Force getPluginDir() to fail via a relative TKN_PLUGINS_DIR. + // Use "." as TKN_PLUGINS_DIR: the old code would resolve filepath.Join(".", "tkn-evil") + // against cwd and find the binary; the fixed code rejects "." as non-absolute. // Keep nd off PATH so LookPath cannot find the binary either. - t.Setenv(pluginDirEnv, "relative/bad/path") + t.Setenv(pluginDirEnv, ".") t.Setenv("PATH", "") // The binary is only reachable via cwd — FindPlugin must not find it.