fix(plugins): reject relative plugin dirs to prevent cwd fallback (CWE-426) - #3199
fix(plugins): reject relative plugin dirs to prevent cwd fallback (CWE-426)#3199divyansh42 wants to merge 3 commits into
Conversation
…E-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-<arg>") 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 <diagrawa@redhat.com>
tekton-robot
added
the
release-note
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
tekton-robot
added
the
size/M
There was a problem hiding this comment.
🟡 Changes recommended
Relative HOME values still permit cwd-relative plugin lookup, and the cwd regression test does not exercise that behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Hardens plugin discovery against current-working-directory fallback.
Changes:
- Rejects relative plugin configuration paths.
- Falls back to
PATHwhen plugin-directory resolution fails. - Adds security regression tests.
File summaries
| File | Description |
|---|---|
pkg/plugins/plugins.go |
Validates plugin paths and changes lookup fallback. |
pkg/plugins/plugins_test.go |
Tests relative paths and fallback behavior. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…iew) 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 <diagrawa@redhat.com>
There was a problem hiding this comment.
🟡 Changes recommended
The working-directory regression test currently passes against the vulnerable baseline and should directly exercise that behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
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 <diagrawa@redhat.com>
Changes
FindPlugin()inpkg/plugins/plugins.gosilently discarded the error fromgetPluginDir(). In minimal environments (scratch/distroless containers, Kubernetes pods running as arbitrary UIDs without/etc/passwdentries), all home-dir resolution fallbacks (dscl,getent,sh -c "cd && pwd") can fail, causinggetPluginDir()to return("", err). The discarded error leftdiras an empty string, sofilepath.Join("", "tkn-<arg>")produced a relative path.syscall.Execthen resolved it against the current working directory — a classic untrusted search path (CWE-426).Additionally,
TKN_PLUGINS_DIRandXDG_CONFIG_HOMEwere accepted even when set to relative paths, creating the same cwd-resolution risk without any home-dir failure at all.Fix:
getPluginDir()now returns an error ifTKN_PLUGINS_DIRorXDG_CONFIG_HOMEis not an absolute path.FindPlugin()properly handles the error fromgetPluginDir()and skips the dir lookup (falling through toexec.LookPath) instead of silently using an empty string.Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
make checkmake generatedSee the contribution guide
for more details.
Release Notes