-
Notifications
You must be signed in to change notification settings - Fork 181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: enforce path validation for push/attach and improve path traversal failure message for pull #988
Conversation
Can you help rename the PR following the conventional commits? Thanks |
Codecov ReportAttention: Patch coverage is
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…ttach Signed-off-by: suganyas <ssuganyatce@gmail.com>
Co-authored-by: Billy Zha <qweeah@gmail.com> Signed-off-by: suganyas <ssuganyatce@gmail.com>
Co-authored-by: Billy Zha <qweeah@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: Billy Zha <qweeah@gmail.com> Signed-off-by: suganyas <ssuganyatce@gmail.com>
Co-authored-by: Billy Zha <qweeah@gmail.com> Signed-off-by: suganyas <ssuganyatce@gmail.com>
Co-authored-by: Billy Zha <qweeah@gmail.com> Signed-off-by: suganyas <ssuganyatce@gmail.com>
Co-authored-by: Billy Zha <qweeah@gmail.com> Signed-off-by: suganyas <ssuganyatce@gmail.com>
Co-authored-by: Billy Zha <qweeah@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>
Co-authored-by: Shiwei Zhang <shizh@microsoft.com> Signed-off-by: suganyas <ssuganyatce@gmail.com>
Co-authored-by: Shiwei Zhang <shizh@microsoft.com> Signed-off-by: suganyas <ssuganyatce@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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>
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:
Screenshot showing the scenarios tested