From 58e4f955e2c28ef3afa9857f439981e6b3feaa81 Mon Sep 17 00:00:00 2001 From: Aravindhan Ayyanathan Date: Tue, 23 Jun 2026 09:36:23 +0100 Subject: [PATCH 1/2] Validate git ref and repo values to reject dash-prefixed inputs Signed-off-by: Aravindhan Ayyanathan --- internal/gitutil/gitutil.go | 17 +++++++++++++++++ internal/gitutil/gitutil_test.go | 27 +++++++++++++++++++++++++++ pkg/lib/kptops/pkgupdate.go | 9 ++++++++- pkg/lib/update/update.go | 8 ++++++++ pkg/lib/util/fetch/fetch.go | 6 ++++++ pkg/lib/util/get/get.go | 6 ++++++ 6 files changed, 72 insertions(+), 1 deletion(-) diff --git a/internal/gitutil/gitutil.go b/internal/gitutil/gitutil.go index 2a6ce25add..6ed292ab90 100644 --- a/internal/gitutil/gitutil.go +++ b/internal/gitutil/gitutil.go @@ -357,6 +357,23 @@ func (gur *GitUpstreamRepo) getRepoCacheDir() (string, error) { // cacheRepo fetches a remote repo to a cache location, and fetches the provided refs. func (gur *GitUpstreamRepo) cacheRepo(ctx context.Context, uri string, requiredRefs []string, optionalRefs []string) (string, error) { const op errors.Op = "gitutil.cacheRepo" + + // Validate that refs and URI do not start with '-' to prevent them from + // being interpreted as git command-line options. + if strings.HasPrefix(uri, "-") { + return "", errors.E(op, errors.Git, fmt.Errorf("invalid git repo %q: must not start with '-'", uri)) + } + for _, ref := range requiredRefs { + if strings.HasPrefix(ref, "-") { + return "", errors.E(op, errors.Git, fmt.Errorf("invalid git ref %q: must not start with '-'", ref)) + } + } + for _, ref := range optionalRefs { + if strings.HasPrefix(ref, "-") { + return "", errors.E(op, errors.Git, fmt.Errorf("invalid git ref %q: must not start with '-'", ref)) + } + } + kptCacheDir, err := gur.getRepoCacheDir() if err != nil { return "", errors.E(op, err) diff --git a/internal/gitutil/gitutil_test.go b/internal/gitutil/gitutil_test.go index 0ae5ebb4fa..e219dc5618 100644 --- a/internal/gitutil/gitutil_test.go +++ b/internal/gitutil/gitutil_test.go @@ -441,3 +441,30 @@ func toKeys(m map[string]string) []string { sort.Strings(keys) return keys } + +// TestGitUpstreamRepo_GetRepo_flagLikeRefRejected verifies that a ref +// starting with '--' is rejected as invalid input. +func TestGitUpstreamRepo_GetRepo_flagLikeRefRejected(t *testing.T) { + repoContent := map[string][]testutil.Content{ + testutil.Upstream: { + { + Pkg: pkgbuilder.NewRootPkg(). + WithResource(pkgbuilder.DeploymentResource), + Branch: "main", + }, + }, + } + + g, _, clean := testutil.SetupReposAndWorkspace(t, repoContent) + defer clean() + + gur, err := internalgitutil.NewGitUpstreamRepo(fake.CtxWithDefaultPrinter(), g[testutil.Upstream].RepoDirectory) + if !assert.NoError(t, err) { + t.FailNow() + } + + // A ref starting with '--' should be rejected by input validation. + _, err = gur.GetRepo(fake.CtxWithDefaultPrinter(), []string{"--some-invalid-ref"}) + assert.Error(t, err) + assert.Contains(t, err.Error(), "must not start with '-'") +} diff --git a/pkg/lib/kptops/pkgupdate.go b/pkg/lib/kptops/pkgupdate.go index 6cf238bf25..d8ed509e07 100644 --- a/pkg/lib/kptops/pkgupdate.go +++ b/pkg/lib/kptops/pkgupdate.go @@ -1,4 +1,4 @@ -// Copyright 2022 The kpt Authors +// Copyright 2022,2026 The kpt Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -19,6 +19,7 @@ import ( "fmt" "os" "path/filepath" + "strings" kptfilev1 "github.com/kptdev/kpt/api/kptfile/v1" "github.com/kptdev/kpt/pkg/kptfile/kptfileutil" @@ -59,11 +60,17 @@ func PkgUpdate(ctx context.Context, ref string, packageDir string, _ PkgUpdateOp if kf.Upstream == nil || kf.Upstream.Git == nil { return fmt.Errorf("package must have an upstream reference") } + if strings.HasPrefix(kf.Upstream.Git.Repo, "-") { + return fmt.Errorf("invalid git repo %q: must not start with '-'", kf.Upstream.Git.Repo) + } // originalRootKfRef := rootKf.Upstream.Git.Ref if ref != "" { kf.Upstream.Git.Ref = ref } + if strings.HasPrefix(kf.Upstream.Git.Ref, "-") { + return fmt.Errorf("invalid git ref %q: must not start with '-'", kf.Upstream.Git.Ref) + } // if u.Strategy != "" { // rootKf.Upstream.UpdateStrategy = u.Strategy // } diff --git a/pkg/lib/update/update.go b/pkg/lib/update/update.go index 138f56b1f6..4a6ca2fe89 100644 --- a/pkg/lib/update/update.go +++ b/pkg/lib/update/update.go @@ -134,10 +134,18 @@ func (u *Command) Run(ctx context.Context) error { return errors.E(op, u.Pkg.UniquePath, fmt.Errorf("package must have an upstream reference")) } + if strings.HasPrefix(rootKf.Upstream.Git.Repo, "-") { + return errors.E(op, u.Pkg.UniquePath, + fmt.Errorf("invalid git repo %q: must not start with '-'", rootKf.Upstream.Git.Repo)) + } originalRootKfRef := rootKf.Upstream.Git.Ref if u.Ref != "" { rootKf.Upstream.Git.Ref = u.Ref } + if strings.HasPrefix(rootKf.Upstream.Git.Ref, "-") { + return errors.E(op, u.Pkg.UniquePath, + fmt.Errorf("invalid git ref %q: must not start with '-'", rootKf.Upstream.Git.Ref)) + } if u.Strategy != "" { rootKf.Upstream.UpdateStrategy = u.Strategy } diff --git a/pkg/lib/util/fetch/fetch.go b/pkg/lib/util/fetch/fetch.go index 6785718c97..a7539a3603 100644 --- a/pkg/lib/util/fetch/fetch.go +++ b/pkg/lib/util/fetch/fetch.go @@ -82,9 +82,15 @@ func (c Command) validate(kf *kptfilev1.KptFile) error { if len(g.Repo) == 0 { return errors.E(op, errors.MissingParam, fmt.Errorf("must specify repo")) } + if strings.HasPrefix(g.Repo, "-") { + return errors.E(op, errors.InvalidParam, fmt.Errorf("invalid git repo %q: must not start with '-'", g.Repo)) + } if len(g.Ref) == 0 { return errors.E(op, errors.MissingParam, fmt.Errorf("must specify ref")) } + if strings.HasPrefix(g.Ref, "-") { + return errors.E(op, errors.InvalidParam, fmt.Errorf("invalid git ref %q: must not start with '-'", g.Ref)) + } if len(g.Directory) == 0 { return errors.E(op, errors.MissingParam, fmt.Errorf("must specify directory")) } diff --git a/pkg/lib/util/get/get.go b/pkg/lib/util/get/get.go index 26d3a879bb..f27e6f5209 100644 --- a/pkg/lib/util/get/get.go +++ b/pkg/lib/util/get/get.go @@ -218,9 +218,15 @@ func (c *Command) DefaultValues() error { if len(g.Repo) == 0 { return errors.E(op, errors.MissingParam, fmt.Errorf("must specify repo")) } + if strings.HasPrefix(g.Repo, "-") { + return errors.E(op, errors.InvalidParam, fmt.Errorf("invalid git repo %q: must not start with '-'", g.Repo)) + } if len(g.Ref) == 0 { return errors.E(op, errors.MissingParam, fmt.Errorf("must specify ref")) } + if strings.HasPrefix(g.Ref, "-") { + return errors.E(op, errors.InvalidParam, fmt.Errorf("invalid git ref %q: must not start with '-'", g.Ref)) + } if len(c.Destination) == 0 { return errors.E(op, errors.MissingParam, fmt.Errorf("must specify destination")) } From 9be8c0814222b91d1d29ffcf81d6f8a22b2aed93 Mon Sep 17 00:00:00 2001 From: Aravindhan Ayyanathan Date: Tue, 23 Jun 2026 09:50:06 +0100 Subject: [PATCH 2/2] Address review comments Signed-off-by: Aravindhan Ayyanathan --- internal/gitutil/gitutil.go | 6 +++--- pkg/lib/update/update.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/gitutil/gitutil.go b/internal/gitutil/gitutil.go index 6ed292ab90..ca34fa44c9 100644 --- a/internal/gitutil/gitutil.go +++ b/internal/gitutil/gitutil.go @@ -361,16 +361,16 @@ func (gur *GitUpstreamRepo) cacheRepo(ctx context.Context, uri string, requiredR // Validate that refs and URI do not start with '-' to prevent them from // being interpreted as git command-line options. if strings.HasPrefix(uri, "-") { - return "", errors.E(op, errors.Git, fmt.Errorf("invalid git repo %q: must not start with '-'", uri)) + return "", errors.E(op, errors.InvalidParam, fmt.Errorf("invalid git repo %q: must not start with '-'", uri)) } for _, ref := range requiredRefs { if strings.HasPrefix(ref, "-") { - return "", errors.E(op, errors.Git, fmt.Errorf("invalid git ref %q: must not start with '-'", ref)) + return "", errors.E(op, errors.InvalidParam, fmt.Errorf("invalid git ref %q: must not start with '-'", ref)) } } for _, ref := range optionalRefs { if strings.HasPrefix(ref, "-") { - return "", errors.E(op, errors.Git, fmt.Errorf("invalid git ref %q: must not start with '-'", ref)) + return "", errors.E(op, errors.InvalidParam, fmt.Errorf("invalid git ref %q: must not start with '-'", ref)) } } diff --git a/pkg/lib/update/update.go b/pkg/lib/update/update.go index 4a6ca2fe89..a7711564bb 100644 --- a/pkg/lib/update/update.go +++ b/pkg/lib/update/update.go @@ -135,7 +135,7 @@ func (u *Command) Run(ctx context.Context) error { fmt.Errorf("package must have an upstream reference")) } if strings.HasPrefix(rootKf.Upstream.Git.Repo, "-") { - return errors.E(op, u.Pkg.UniquePath, + return errors.E(op, u.Pkg.UniquePath, errors.InvalidParam, fmt.Errorf("invalid git repo %q: must not start with '-'", rootKf.Upstream.Git.Repo)) } originalRootKfRef := rootKf.Upstream.Git.Ref @@ -143,7 +143,7 @@ func (u *Command) Run(ctx context.Context) error { rootKf.Upstream.Git.Ref = u.Ref } if strings.HasPrefix(rootKf.Upstream.Git.Ref, "-") { - return errors.E(op, u.Pkg.UniquePath, + return errors.E(op, u.Pkg.UniquePath, errors.InvalidParam, fmt.Errorf("invalid git ref %q: must not start with '-'", rootKf.Upstream.Git.Ref)) } if u.Strategy != "" {