Security - Git argument injection via untrusted Kptfile ref enables file overwrite

[tgzsolt]

• Description
  When kpt fetches a package, it recursively fetches sub-packages whose Kptfiles declare an upstream block (internal/util/get/get.go fetchPackages). The sub-package Kptfile is attacker-controlled content from the remote repository. Its upstream.git.ref field flows unmodified into GitUpstreamRepo.cacheRepo() as a requiredRefs entry. In the default branch of the switch (when the ref is neither a known branch/tag nor a full SHA), the ref s is passed directly to gitRunner.Run(ctx, "show", s) at line 441 with no -{-} separator and no validation that it does not begin with {-}. The git process runs with cmd.Dir set to {~}/.kpt/repos/<sha> (lines 105, 385/393). A ref value such as -{-}output=../../../.bashrc is interpreted by git show as the {-}{-}output=<file> option, causing git to write the HEAD commit display (whose commit message and diff are fully attacker-controlled in the attacker's second repository) to {-}{~}/.bashrc. A similar lack of - separator affects git reset --hard <commit> at internal/util/fetch/fetch.go:216 and git remote add origin <uri> at gitutil.go:386

• Motivaton
  Remove security vulnerability

[netlify]

✅ Deploy Preview for kptdocs ready!

Name	Link
🔨 Latest commit	f1c312c
🔍 Latest deploy log	https://app.netlify.com/projects/kptdocs/deploys/6a4625c767ae190008dd518a
😎 Deploy Preview	https://deploy-preview-4604--kptdocs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[Copilot]

Pull request overview

This PR addresses a Git argument-injection vulnerability where attacker-controlled refs/URIs could be interpreted as git command-line options (e.g. git show --output=...), potentially enabling arbitrary file overwrite during package fetch/update flows.

Changes:

• Introduces centralized validation (validateGitArg) to reject git refs/commits/URIs that begin with -, preventing option injection.
• Refactors GitUpstreamRepo from a concrete struct into an interface backed by a broker implementation, adding deterministic ordering for Heads()/Tags().
• Updates fetch/update/parse call sites and adds a regression test ensuring option-like refs are rejected.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
pkg/lib/util/parse/parse.go	Simplifies branch listing by using gur.Heads() directly.
pkg/lib/util/fetch/fetch.go	Updates cached repo plumbing to store GitUpstreamRepo interface values; adjusts ref resolution API usage.
pkg/lib/update/update.go	Updates cached upstream repo cache type to the new GitUpstreamRepo interface.
pkg/lib/update/update_test.go	Updates upstream repo URI access to gur.Uri() to match refactor.
internal/gitutil/gitutil.go	Implements injection guard, refactors upstream repo into interface + broker, validates attacker-controlled inputs before invoking git.
internal/gitutil/gitutil_test.go	Updates tests for new APIs and adds option-injection regression coverage.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[liamfallon]

Does this overlap with #4598 ?

[liamfallon]

Hi @tgzsolt , the CI tests won't run till we merge #4605 and you do a rebase. We'll try and get that PR merged ASAP.

[tgzsolt]

Hi @tgzsolt , the CI tests won't run till we merge #4605 and you do a rebase. We'll try and get that PR merged ASAP.

Ofc i can wait. Meanwhile i fixed (i think) it on this branch: bumped up the version in the testdata