oras-project / oras-go

ORAS Go library
https://oras.land
Apache License 2.0
168 stars 90 forks source link

exec oras pull, and set allowpathTraversalonwrite True, and get --output to other directory, but is pull to orginal path #750

Open lht2017 opened 2 months ago

lht2017 commented 2 months ago

when see code in content/file.go in the method resolveWritePath, when allowpathTraversalonwrite is True, then return the original path to store the pull files is some bug? or it should like this bellow:

image

Wwwsylvia commented 2 months ago

Hi @lht2017 , thanks for opening this issue.

The purpose of the resolveWritePath function is to resolve and validate the target write path of the file name (name). If the target path (path) is outside of the working directory, it returns ErrPathTraversalDisallowed. Otherwise, it returns the absolute path (path) of the file name (name). That is, when AllowPathTraversalOnWrite is true, this function returns the absolute path obtained at line 606 (path := s.absPath(name)).

https://github.com/oras-project/oras-go/blob/9b6f32158776a699b23edb3db86d053623619b60/content/file/file.go#L604-L633

If you have observed any unexpected behavior, could you please provide additional details? Specifically:

  1. What actions did you take, and what were your expectations?
  2. What was your environment?
  3. How can we reproduce the issue?
  4. Are there any relevant error logs?

(We hope to have an issue template, but it is still working in progress.)

lht2017 commented 2 months ago

Thanks for replay! @Wwwsylvia 1 What actions did you take, and what were your expectations? first I user oras push a file , like /usr/local/tmp/a.txt to a remote registry ,

then use oras command to pull files from the remote registry, and use param --output & allowpathtraversalonwrite true , to other directory like /pull/data/a.txt , but the file a.txt not write in /pull/data/a.txt , actually it always write in the push path, like /usr/local/tmp/a.txt

2 What was your environment? linux el7

3 How can we reproduce the issue? use step 1

4 with debug I think oras-go/content/file/file.go the code at line 606 (path := s.absPath(name))

the name got is /usr/local/tmp/a.txt s.workingDir got is /pull/data/a.txt

may the params reverse ?

Wwwsylvia commented 1 month ago

Hi @lht2017 , this behavior is by design. The --output parameter is only applicable to relative paths, and the --allow-path-traversal means to allow storing files out of the output directory. That is to say, if one specified an absolute path on push, for example /usr/local/tmp/a.txt,

$ oras push $myrepo:$mytag /usr/local/tmp/a.txt --disable-path-validation

they would always get the file pulled to the same location usr/local/tmp/a.txt, no matter where --output points to.

oras pull $myrepo:$mytag --output /pull/data/ --allow-path-traversal

We just realized that this design could cause user misunderstanding. Do you have any suggestion to help us improve the user experience? Maybe we can update the documentation for the --output parameter to call out that it does not work for absolute paths?

FeynmanZhou commented 1 month ago

Hi @lht2017 , this behavior is by design. The --output parameter is only applicable to relative paths, and the --allow-path-traversal means to allow storing files out of the output directory. That is to say, if one specified an absolute path on push, for example /usr/local/tmp/a.txt,

$ oras push $myrepo:$mytag /usr/local/tmp/a.txt --disable-path-validation

they would always get the file pulled to the same location usr/local/tmp/a.txt, no matter where --output points to.

oras pull $myrepo:$mytag --output /pull/data/ --allow-path-traversal

We just realized that this design could cause user misunderstanding. Do you have any suggestion to help us improve the user experience? Maybe we can update the documentation for the --output parameter to call out that it does not work for absolute paths?

@qweeah We may consider adding the design philosophy of pull/push to this ORAS doc to clarify the current behaviors. Users won't be surprised if they are aware of this philosophy in advance.