From 755a83c30df7ee3dcaf43ab372afd336618170fb Mon Sep 17 00:00:00 2001 From: suganyas Date: Mon, 26 Jun 2023 22:14:27 +1000 Subject: [PATCH 01/16] Add changes for fixing the absolute path behaviour in oras push and attach Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 31 ++++++++++++++++++++++++++++++ cmd/oras/root/attach.go | 1 - cmd/oras/root/pull.go | 6 ++++++ cmd/oras/root/push.go | 1 - 4 files changed, 37 insertions(+), 2 deletions(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index 7fe058b83..52acecef3 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -21,6 +21,7 @@ import ( "errors" "fmt" "os" + "path/filepath" "strings" ocispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -38,6 +39,7 @@ var ( errAnnotationConflict = errors.New("`--annotation` and `--annotation-file` cannot be both specified") errAnnotationFormat = errors.New("missing key in `--annotation` flag") errAnnotationDuplication = errors.New("duplicate annotation key") + errPathValidation = errors.New("one or more files are not in the current directory.If it's intentional use --disable-path-validation flag to skip this check") ) // Packer option struct. @@ -69,6 +71,35 @@ func (opts *Packer) ExportManifest(ctx context.Context, fetcher content.Fetcher, } return os.WriteFile(opts.ManifestExportPath, manifestBytes, 0666) } +func (opts *Packer) Parse() error { + currentDir, err := os.Getwd() + var failedPaths []string + if err != nil { + return err + } + if !opts.PathValidationDisabled && len(opts.FileRefs) != 0 { + for _, path := range opts.FileRefs { + //Remove the type if specified in the path [:] format + lastIndex := strings.LastIndex(path, ":") + if lastIndex != -1 { + path = path[:lastIndex] + } + absPath, err := filepath.Abs(path) + dirPath := filepath.Dir(absPath) + if err != nil { + return err + } + if dirPath != currentDir { + failedPaths = append(failedPaths, absPath) + } + } + if len(failedPaths) > 0 { + errorMsg := fmt.Sprintf("%v: %v currentDir :%v", errPathValidation, strings.Join(failedPaths, ", "), currentDir) + return errors.New(errorMsg) + } + } + return nil +} // LoadManifestAnnotations loads the manifest annotation map. func (opts *Packer) LoadManifestAnnotations() (annotations map[string]map[string]string, err error) { diff --git a/cmd/oras/root/attach.go b/cmd/oras/root/attach.go index a1032f67b..796427bf8 100644 --- a/cmd/oras/root/attach.go +++ b/cmd/oras/root/attach.go @@ -113,7 +113,6 @@ func runAttach(ctx context.Context, opts attachOptions) error { return err } defer store.Close() - store.AllowPathTraversalOnWrite = opts.PathValidationDisabled dst, err := opts.NewTarget(opts.Common) if err != nil { diff --git a/cmd/oras/root/pull.go b/cmd/oras/root/pull.go index ec25da1c0..1f229d941 100644 --- a/cmd/oras/root/pull.go +++ b/cmd/oras/root/pull.go @@ -17,8 +17,10 @@ package root import ( "context" + "errors" "fmt" "io" + "strings" "sync" ocispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -237,6 +239,10 @@ func runPull(ctx context.Context, opts pullOptions) error { // Copy desc, err := oras.Copy(ctx, src, opts.Reference, dst, opts.Reference, copyOptions) if err != nil { + if strings.Contains(err.Error(), "path traversal disallowed") { + errorMsg := fmt.Sprintf("%v: %v ", err, "To enable path traversal use --allow-path-traversal flag") + return errors.New(errorMsg) + } return err } if pulledEmpty { diff --git a/cmd/oras/root/push.go b/cmd/oras/root/push.go index 152a2b2e1..698d3e57d 100644 --- a/cmd/oras/root/push.go +++ b/cmd/oras/root/push.go @@ -139,7 +139,6 @@ func runPush(ctx context.Context, opts pushOptions) error { return err } defer store.Close() - store.AllowPathTraversalOnWrite = opts.PathValidationDisabled if opts.manifestConfigRef != "" { path, cfgMediaType, err := fileref.Parse(opts.manifestConfigRef, oras.MediaTypeUnknownConfig) if err != nil { From caec5ff1f5693a4172ac60a2cabbfd6781115001 Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 17:05:12 +1000 Subject: [PATCH 02/16] Update cmd/oras/internal/option/packer.go Co-authored-by: Billy Zha Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index 52acecef3..23c3ce4e5 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -72,8 +72,8 @@ func (opts *Packer) ExportManifest(ctx context.Context, fetcher content.Fetcher, return os.WriteFile(opts.ManifestExportPath, manifestBytes, 0666) } func (opts *Packer) Parse() error { - currentDir, err := os.Getwd() var failedPaths []string + currentDir, err := os.Getwd() if err != nil { return err } From 007b2d95ef4136f616ea5e18285c70060d6fa861 Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 17:24:18 +1000 Subject: [PATCH 03/16] Update cmd/oras/internal/option/packer.go Co-authored-by: Billy Zha Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index 23c3ce4e5..e8bae5827 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -94,8 +94,7 @@ func (opts *Packer) Parse() error { } } if len(failedPaths) > 0 { - errorMsg := fmt.Sprintf("%v: %v currentDir :%v", errPathValidation, strings.Join(failedPaths, ", "), currentDir) - return errors.New(errorMsg) + return fmt.Errorf("%v: %v currentDir :%v", errPathValidation, strings.Join(failedPaths, ", "), currentDir) } } return nil From 8f447bd43a8df08241846dde4bf9ebde0371cfc0 Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 17:25:01 +1000 Subject: [PATCH 04/16] Update cmd/oras/root/pull.go Co-authored-by: Billy Zha Signed-off-by: suganyas --- cmd/oras/root/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/root/pull.go b/cmd/oras/root/pull.go index 1f229d941..9be7e1f8a 100644 --- a/cmd/oras/root/pull.go +++ b/cmd/oras/root/pull.go @@ -240,7 +240,7 @@ func runPull(ctx context.Context, opts pullOptions) error { desc, err := oras.Copy(ctx, src, opts.Reference, dst, opts.Reference, copyOptions) if err != nil { if strings.Contains(err.Error(), "path traversal disallowed") { - errorMsg := fmt.Sprintf("%v: %v ", err, "To enable path traversal use --allow-path-traversal flag") + errorMsg := fmt.Sprintf("%v: %w ", "to enable path traversal use --allow-path-traversal flag", err) return errors.New(errorMsg) } return err From de4bdb7c2e2d94aee8da35bc7be990011287f283 Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 20:25:36 +1000 Subject: [PATCH 05/16] Address review comments from Billy and tested code Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 12 ++++-------- cmd/oras/root/pull.go | 6 ++---- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index e8bae5827..de8c29636 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -27,6 +27,7 @@ import ( ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/spf13/pflag" "oras.land/oras-go/v2/content" + "oras.land/oras/cmd/oras/internal/fileref" ) // Pre-defined annotation keys for annotation file @@ -80,17 +81,12 @@ func (opts *Packer) Parse() error { if !opts.PathValidationDisabled && len(opts.FileRefs) != 0 { for _, path := range opts.FileRefs { //Remove the type if specified in the path [:] format - lastIndex := strings.LastIndex(path, ":") - if lastIndex != -1 { - path = path[:lastIndex] - } - absPath, err := filepath.Abs(path) - dirPath := filepath.Dir(absPath) + path, _, err = fileref.Parse(path, "") if err != nil { return err } - if dirPath != currentDir { - failedPaths = append(failedPaths, absPath) + if filepath.IsAbs(path) { + failedPaths = append(failedPaths, path) } } if len(failedPaths) > 0 { diff --git a/cmd/oras/root/pull.go b/cmd/oras/root/pull.go index 9be7e1f8a..f68188035 100644 --- a/cmd/oras/root/pull.go +++ b/cmd/oras/root/pull.go @@ -20,7 +20,6 @@ import ( "errors" "fmt" "io" - "strings" "sync" ocispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -239,9 +238,8 @@ func runPull(ctx context.Context, opts pullOptions) error { // Copy desc, err := oras.Copy(ctx, src, opts.Reference, dst, opts.Reference, copyOptions) if err != nil { - if strings.Contains(err.Error(), "path traversal disallowed") { - errorMsg := fmt.Sprintf("%v: %w ", "to enable path traversal use --allow-path-traversal flag", err) - return errors.New(errorMsg) + if errors.Is(err, file.ErrPathTraversalDisallowed) { + err = fmt.Errorf("%s: %w", "use option -T/allow-path-traversal to allow pulling outside of working directory", err) } return err } From 351a1100705409d550bf516252afa6703d52eaeb Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 22:50:20 +1000 Subject: [PATCH 06/16] Update cmd/oras/internal/option/packer.go Co-authored-by: Billy Zha Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index de8c29636..f80fbe3bc 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -40,7 +40,7 @@ var ( errAnnotationConflict = errors.New("`--annotation` and `--annotation-file` cannot be both specified") errAnnotationFormat = errors.New("missing key in `--annotation` flag") errAnnotationDuplication = errors.New("duplicate annotation key") - errPathValidation = errors.New("one or more files are not in the current directory.If it's intentional use --disable-path-validation flag to skip this check") + errPathValidation = errors.New("absolute file path detected. If it's intentional use --disable-path-validation flag to skip this check") ) // Packer option struct. From e03dec73d264c4a3260b29f49da6983ad0eb691b Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 22:50:36 +1000 Subject: [PATCH 07/16] Update cmd/oras/internal/option/packer.go Co-authored-by: Billy Zha Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index f80fbe3bc..b3e34081e 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -74,10 +74,6 @@ func (opts *Packer) ExportManifest(ctx context.Context, fetcher content.Fetcher, } func (opts *Packer) Parse() error { var failedPaths []string - currentDir, err := os.Getwd() - if err != nil { - return err - } if !opts.PathValidationDisabled && len(opts.FileRefs) != 0 { for _, path := range opts.FileRefs { //Remove the type if specified in the path [:] format From 8f03f50a08aae5c5e1618a9e0e60a984959b2140 Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 22:50:49 +1000 Subject: [PATCH 08/16] Update cmd/oras/internal/option/packer.go Co-authored-by: Billy Zha Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index b3e34081e..dfee018e3 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -86,7 +86,7 @@ func (opts *Packer) Parse() error { } } if len(failedPaths) > 0 { - return fmt.Errorf("%v: %v currentDir :%v", errPathValidation, strings.Join(failedPaths, ", "), currentDir) + return fmt.Errorf("%w: %v", errPathValidation, strings.Join(failedPaths, ", ")) } } return nil From af7c0ea10c818fc82c1e8e9d8fc1e8d8c6e52835 Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 22:50:58 +1000 Subject: [PATCH 09/16] Update cmd/oras/root/pull.go Co-authored-by: Billy Zha Signed-off-by: suganyas --- cmd/oras/root/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/root/pull.go b/cmd/oras/root/pull.go index f68188035..d4d27b9a0 100644 --- a/cmd/oras/root/pull.go +++ b/cmd/oras/root/pull.go @@ -239,7 +239,7 @@ func runPull(ctx context.Context, opts pullOptions) error { desc, err := oras.Copy(ctx, src, opts.Reference, dst, opts.Reference, copyOptions) if err != nil { if errors.Is(err, file.ErrPathTraversalDisallowed) { - err = fmt.Errorf("%s: %w", "use option -T/allow-path-traversal to allow pulling outside of working directory", err) + err = fmt.Errorf("%s: %w", "use flag -T/allow-path-traversal to allow pulling files outside of working directory", err) } return err } From 2609e2f505c2a1772cbf4f7a65a36fcc1c05a2b8 Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 22:51:09 +1000 Subject: [PATCH 10/16] Update cmd/oras/internal/option/packer.go Co-authored-by: Billy Zha Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index dfee018e3..0114fe449 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -74,7 +74,7 @@ func (opts *Packer) ExportManifest(ctx context.Context, fetcher content.Fetcher, } func (opts *Packer) Parse() error { var failedPaths []string - if !opts.PathValidationDisabled && len(opts.FileRefs) != 0 { + if !opts.PathValidationDisabled { for _, path := range opts.FileRefs { //Remove the type if specified in the path [:] format path, _, err = fileref.Parse(path, "") From d1dd5261e57e732f47ddd7ee050023b972a42d8c Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 22:55:14 +1000 Subject: [PATCH 11/16] Update cmd/oras/internal/option/packer.go Co-authored-by: Billy Zha Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index 0114fe449..aeda50b73 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -76,7 +76,7 @@ func (opts *Packer) Parse() error { var failedPaths []string if !opts.PathValidationDisabled { for _, path := range opts.FileRefs { - //Remove the type if specified in the path [:] format + // Remove the type if specified in the path [:] format path, _, err = fileref.Parse(path, "") if err != nil { return err From 0c656eeb2fc3ed628e8a9d398bed1932d26d45ce Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 22:57:41 +1000 Subject: [PATCH 12/16] Fix the build for compilation error Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index aeda50b73..2a1dfdcea 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -77,7 +77,7 @@ func (opts *Packer) Parse() error { if !opts.PathValidationDisabled { for _, path := range opts.FileRefs { // Remove the type if specified in the path [:] format - path, _, err = fileref.Parse(path, "") + path, _, err := fileref.Parse(path, "") if err != nil { return err } From a48736f2b26fe5ae2e21f9ec7e52d903d2a9f931 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Wed, 28 Jun 2023 08:46:06 +0000 Subject: [PATCH 13/16] chore: add experimental mark to filtered tag listing Signed-off-by: Billy Zha --- cmd/oras/root/repo/tags.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/oras/root/repo/tags.go b/cmd/oras/root/repo/tags.go index 21435eadf..490a0fc94 100644 --- a/cmd/oras/root/repo/tags.go +++ b/cmd/oras/root/repo/tags.go @@ -55,10 +55,10 @@ Example - Show tags of the target OCI layout folder 'layout-dir': Example - Show tags of the target OCI layout archive 'layout.tar': oras repo tags --oci-layout layout.tar -Example - Show tags associated with a particular tagged resource: +Example - [Experimental] Show tags associated with a particular tagged resource: oras repo tags localhost:5000/hello:latest -Example - Show tags associated with a digest: +Example - [Experimental] Show tags associated with a digest: oras repo tags localhost:5000/hello@sha256:c551125a624189cece9135981621f3f3144564ddabe14b523507bf74c2281d9b `, Args: cobra.ExactArgs(1), @@ -95,7 +95,7 @@ func showTags(ctx context.Context, opts showTagsOptions) error { } filter = desc.Digest.String() } - logger.Infof("[Experimental] querying tags associated to %s, it may take a while...\n", filter) + logger.Warnf("[Experimental] querying tags associated to %s, it may take a while...\n", filter) } return finder.Tags(ctx, opts.last, func(tags []string) error { for _, tag := range tags { From 8420368d774b405b7b04c5ec4279899b7d48e9ad Mon Sep 17 00:00:00 2001 From: suganyas Date: Wed, 28 Jun 2023 19:50:10 +1000 Subject: [PATCH 14/16] Update cmd/oras/internal/option/packer.go Co-authored-by: Shiwei Zhang Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index 2a1dfdcea..63f7beb10 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -40,7 +40,7 @@ var ( errAnnotationConflict = errors.New("`--annotation` and `--annotation-file` cannot be both specified") errAnnotationFormat = errors.New("missing key in `--annotation` flag") errAnnotationDuplication = errors.New("duplicate annotation key") - errPathValidation = errors.New("absolute file path detected. If it's intentional use --disable-path-validation flag to skip this check") + errPathValidation = errors.New("absolute file path detected. If it's intentional, use --disable-path-validation flag to skip this check") ) // Packer option struct. From f3549cd7dcaba1d7b89b2fe497cc0fc302efeee9 Mon Sep 17 00:00:00 2001 From: suganyas Date: Wed, 28 Jun 2023 19:50:18 +1000 Subject: [PATCH 15/16] Update cmd/oras/root/pull.go Co-authored-by: Shiwei Zhang Signed-off-by: suganyas --- cmd/oras/root/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/root/pull.go b/cmd/oras/root/pull.go index d4d27b9a0..703833d90 100644 --- a/cmd/oras/root/pull.go +++ b/cmd/oras/root/pull.go @@ -239,7 +239,7 @@ func runPull(ctx context.Context, opts pullOptions) error { desc, err := oras.Copy(ctx, src, opts.Reference, dst, opts.Reference, copyOptions) if err != nil { if errors.Is(err, file.ErrPathTraversalDisallowed) { - err = fmt.Errorf("%s: %w", "use flag -T/allow-path-traversal to allow pulling files outside of working directory", err) + err = fmt.Errorf("%s: %w", "use flag --allow-path-traversal to allow insecurely pulling files outside of working directory", err) } return err } From ae964ecc291f2e702bd1fef544e69f2ec28f6d0c Mon Sep 17 00:00:00 2001 From: suganyas Date: Wed, 28 Jun 2023 19:50:31 +1000 Subject: [PATCH 16/16] Update cmd/oras/internal/option/packer.go Co-authored-by: Shiwei Zhang Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index 63f7beb10..d32fa62de 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -73,8 +73,8 @@ func (opts *Packer) ExportManifest(ctx context.Context, fetcher content.Fetcher, return os.WriteFile(opts.ManifestExportPath, manifestBytes, 0666) } func (opts *Packer) Parse() error { - var failedPaths []string if !opts.PathValidationDisabled { + var failedPaths []string for _, path := range opts.FileRefs { // Remove the type if specified in the path [:] format path, _, err := fileref.Parse(path, "")