fix: enforce path validation for push/attach and improve path traversal failure message for pull by suganyas · Pull Request #988 · oras-project/oras

suganyas · 2023-06-26T12:30:49Z

What this PR does / why we need it:
I just tried to push an artifact file from linux environment from a absolute path or from different directory. The path is implicitly taken by the oras cli or oras sdk when I pushed it. Like the file was in a directory /home/vts/1/a.exe. and I pushed from /home/test. I am ok if the push fails stating me that the file is not in the current directory and for security reasons you have to be in same working directory. But the push passes and pull fails . So disallow is a much better option. I am sure when I used ORAS python SDK it did fail with error stating as here https://github.com/oras-project/oras-py/blob/209c9b98043a00d1b04789cc2967ca7021dc5b2e/oras/provider.py#L651 . The CLI should have same behaviour as SDK. Then push and pull are coherent. It can be a bad experience when push is ok and pull fails and when different people do it can be hard for people to understand why it fails and also cross platform can fail if it is not intentional

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #
#980
#983
#978
Please check the following list:

Does the affected code have corresponding tests, e.g. unit test, E2E test?
Does this change require a documentation update?
Does this introduce breaking changes that would require an announcement or bumping the major version?
Do all new files have an appropriate license header?

Screenshot showing the scenarios tested

qweeah · 2023-06-27T02:37:26Z

Can you help rename the PR following the conventional commits? Thanks

codecov-commenter · 2023-06-27T02:38:46Z

Codecov Report

Attention: Patch coverage is 36.84211% with 12 lines in your changes missing coverage. Please review.

Project coverage is 80.94%. Comparing base (bf33bb7) to head (ae964ec).
Report is 330 commits behind head on main.

Files with missing lines	Patch %	Lines
cmd/oras/internal/option/packer.go	43.75%	6 Missing and 3 partials ⚠️
cmd/oras/root/pull.go	0.00%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #988      +/-   ##
==========================================
- Coverage   81.26%   80.94%   -0.33%     
==========================================
  Files          53       53              
  Lines        2776     2792      +16     
==========================================
+ Hits         2256     2260       +4     
- Misses        352      360       +8     
- Partials      168      172       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

qweeah

LGTM

…ttach Signed-off-by: suganyas <ssuganyatce@gmail.com>

Co-authored-by: Billy Zha <qweeah@gmail.com> Signed-off-by: suganyas <ssuganyatce@gmail.com>

Signed-off-by: suganyas <ssuganyatce@gmail.com>

Co-authored-by: Billy Zha <qweeah@gmail.com> Signed-off-by: suganyas <ssuganyatce@gmail.com>

Signed-off-by: suganyas <ssuganyatce@gmail.com>

Co-authored-by: Shiwei Zhang <shizh@microsoft.com> Signed-off-by: suganyas <ssuganyatce@gmail.com>

shizhMSFT

LGTM

qweeah

LGTM

…al failure message for pull (oras-project#988) Signed-off-by: suganyas <ssuganyatce@gmail.com> Co-authored-by: Billy Zha <qweeah@gmail.com> Co-authored-by: Shiwei Zhang <shizh@microsoft.com>

suganyas requested review from SteveLasker, TerryHowe, qweeah, sajayantony and shizhMSFT as code owners June 26, 2023 12:30

suganyas force-pushed the main branch from 58d20d4 to 6ba71ef Compare June 26, 2023 12:56

suganyas changed the title ~~Fixes for default behaviour handling abs path in push and attach and improvise pull failure message~~ Fixes for default behaviour handling abs path in oras push and attach and improvise oras pull failure message for path traversal Jun 26, 2023

suganyas changed the title ~~Fixes for default behaviour handling abs path in oras push and attach and improvise oras pull failure message for path traversal~~ fix: default behaviour handling abs path in oras push and attach and improvise oras pull failure message for path traversal Jun 27, 2023

qweeah requested changes Jun 27, 2023

View reviewed changes

suganyas mentioned this pull request Jun 27, 2023

improve path traversal error msg of oras pull #978

Closed

suganyas force-pushed the main branch from 791f162 to 10901c0 Compare June 27, 2023 10:28

qweeah requested changes Jun 27, 2023

View reviewed changes

Comment thread cmd/oras/root/pull.go Outdated

Comment thread cmd/oras/internal/option/packer.go Outdated

Comment thread cmd/oras/internal/option/packer.go Outdated

Comment thread cmd/oras/internal/option/packer.go Outdated

Comment thread cmd/oras/internal/option/packer.go Outdated

qweeah requested changes Jun 27, 2023

View reviewed changes

Comment thread cmd/oras/internal/option/packer.go Outdated

suganyas force-pushed the main branch from 3abf169 to e0d769f Compare June 27, 2023 12:58

qweeah mentioned this pull request Jun 27, 2023

test(e2e): add test for pushing via absolute path #991

Merged

qweeah changed the title ~~fix: default behaviour handling abs path in oras push and attach and improvise oras pull failure message for path traversal~~ fix: enforce path validation for push/attach and improve path traversal failure message for pull Jun 28, 2023

qweeah approved these changes Jun 28, 2023

View reviewed changes

suganyas and others added 11 commits June 28, 2023 08:39

Add changes for fixing the absolute path behaviour in oras push and a…

755a83c

…ttach Signed-off-by: suganyas <ssuganyatce@gmail.com>

Update cmd/oras/internal/option/packer.go

caec5ff

Co-authored-by: Billy Zha <qweeah@gmail.com> Signed-off-by: suganyas <ssuganyatce@gmail.com>

Update cmd/oras/internal/option/packer.go

007b2d9

Co-authored-by: Billy Zha <qweeah@gmail.com> Signed-off-by: suganyas <ssuganyatce@gmail.com>

Update cmd/oras/root/pull.go

8f447bd

Co-authored-by: Billy Zha <qweeah@gmail.com> Signed-off-by: suganyas <ssuganyatce@gmail.com>

Address review comments from Billy and tested code

de4bdb7

Signed-off-by: suganyas <ssuganyatce@gmail.com>

Update cmd/oras/internal/option/packer.go

351a110

Co-authored-by: Billy Zha <qweeah@gmail.com> Signed-off-by: suganyas <ssuganyatce@gmail.com>

Update cmd/oras/internal/option/packer.go

e03dec7

Co-authored-by: Billy Zha <qweeah@gmail.com> Signed-off-by: suganyas <ssuganyatce@gmail.com>

Update cmd/oras/internal/option/packer.go

8f03f50

Co-authored-by: Billy Zha <qweeah@gmail.com> Signed-off-by: suganyas <ssuganyatce@gmail.com>

Update cmd/oras/root/pull.go

af7c0ea

Co-authored-by: Billy Zha <qweeah@gmail.com> Signed-off-by: suganyas <ssuganyatce@gmail.com>

Update cmd/oras/internal/option/packer.go

2609e2f

Co-authored-by: Billy Zha <qweeah@gmail.com> Signed-off-by: suganyas <ssuganyatce@gmail.com>

Update cmd/oras/internal/option/packer.go

d1dd526

Co-authored-by: Billy Zha <qweeah@gmail.com> Signed-off-by: suganyas <ssuganyatce@gmail.com>

Fix the build for compilation error

0c656ee

Signed-off-by: suganyas <ssuganyatce@gmail.com>

qweeah force-pushed the main branch from e0d769f to 0c656ee Compare June 28, 2023 00:40

shizhMSFT added this to the v1.1.0 milestone Jun 28, 2023

shizhMSFT reviewed Jun 28, 2023

View reviewed changes

Comment thread cmd/oras/internal/option/packer.go Outdated

Comment thread cmd/oras/root/pull.go Outdated

Comment thread cmd/oras/internal/option/packer.go Outdated

suganyas and others added 3 commits June 28, 2023 19:50

Update cmd/oras/internal/option/packer.go

8420368

Co-authored-by: Shiwei Zhang <shizh@microsoft.com> Signed-off-by: suganyas <ssuganyatce@gmail.com>

Update cmd/oras/root/pull.go

f3549cd

Co-authored-by: Shiwei Zhang <shizh@microsoft.com> Signed-off-by: suganyas <ssuganyatce@gmail.com>

Update cmd/oras/internal/option/packer.go

ae964ec

Co-authored-by: Shiwei Zhang <shizh@microsoft.com> Signed-off-by: suganyas <ssuganyatce@gmail.com>

shizhMSFT approved these changes Jun 28, 2023

View reviewed changes

qweeah approved these changes Jun 28, 2023

View reviewed changes

qweeah merged commit 99c55ce into oras-project:main Jun 28, 2023

This was referenced Jun 29, 2023

--disable-path-validation not working for oras push and oras attach #983

Closed

Vote for releasing v1.1.0-rc.1 #1002

Closed

qweeah mentioned this pull request Jul 10, 2023

bump: tag and release ORAS CLI v1.1.0-rc.1 #1016

Merged

ZhongRuoyu mentioned this pull request Aug 7, 2023

oras 1.0.1 Homebrew/homebrew-core#138923

Merged

njelich added a commit to njelich/publish-feature that referenced this pull request Jan 26, 2024

fix for oras-project/oras#988

d12be7d

Conversation

suganyas commented Jun 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qweeah commented Jun 27, 2023

Uh oh!

codecov-commenter commented Jun 27, 2023 • edited by codecov Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qweeah left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shizhMSFT left a comment

Choose a reason for hiding this comment

Uh oh!

qweeah left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

suganyas commented Jun 26, 2023 •

edited

Loading

codecov-commenter commented Jun 27, 2023 •

edited by codecov Bot

Loading