Skip to content
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

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

Open
lht2017 opened this issue Apr 24, 2024 · 7 comments
Labels
stale Inactive issues or pull requests

Comments

@lht2017
Copy link

lht2017 commented Apr 24, 2024

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
Copy link
Member

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)).

// resolveWritePath resolves the path to write for the given name.
func (s *Store) resolveWritePath(name string) (string, error) {
path := s.absPath(name)
if !s.AllowPathTraversalOnWrite {
base, err := filepath.Abs(s.workingDir)
if err != nil {
return "", err
}
target, err := filepath.Abs(path)
if err != nil {
return "", err
}
rel, err := filepath.Rel(base, target)
if err != nil {
return "", ErrPathTraversalDisallowed
}
rel = filepath.ToSlash(rel)
if strings.HasPrefix(rel, "../") || rel == ".." {
return "", ErrPathTraversalDisallowed
}
}
if s.DisableOverwrite {
if _, err := os.Stat(path); err == nil {
return "", ErrOverwriteDisallowed
} else if !os.IsNotExist(err) {
return "", err
}
}
return path, nil
}

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
Copy link
Author

lht2017 commented May 10, 2024

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
Copy link
Member

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
Copy link
Member

FeynmanZhou commented May 28, 2024

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.

@lht2017
Copy link
Author

lht2017 commented Jul 15, 2024

@Wwwsylvia Thanks,by the way, how can I use oras pull a file to specified other path?

@Wwwsylvia
Copy link
Member

Wwwsylvia commented Jul 15, 2024

Hi @lht2017 , how about:

  1. Specify a relative path when pushing the target file, so that you would be able to pull it to any places specified
  2. Or pull the target file to the absolute path and then move it to the desired place?

Copy link

This issue is stale because it has been open for 60 days with no activity. Remove the stale label or comment to prevent it from being closed in 30 days.

@github-actions github-actions bot added the stale Inactive issues or pull requests label Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Inactive issues or pull requests
Projects
None yet
Development

No branches or pull requests

3 participants