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

reposync: check for .. in remote paths. BZ 1506205 #43

Merged
merged 2 commits into from
Jul 24, 2018

Conversation

dmnks
Copy link
Contributor

@dmnks dmnks commented Jun 22, 2018

No description provided.

For better readability (remote_path == relativepath).
@dmnks
Copy link
Contributor Author

dmnks commented Jul 20, 2018

@m-blaha Marek, could you please review my patch? Thank you!

@m-blaha m-blaha self-assigned this Jul 23, 2018
@m-blaha m-blaha merged commit a792d21 into rpm-software-management:master Jul 24, 2018
if is_subpath(pkg.remote_path, local_repo_path):
newlist.append(pkg)
continue
my.logger.warning(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A primary user of ../ paths is google compute engine's package repo. They have an odd yum repo structure where they use a ../../pool/ directory for packages similar to apt repos. We'd really like to mirror this repo into a directory of our choosing, so we hacked around it by running reposync with the --urls option and using wget to download those URLs into the correct directory structure. It'd be great if the reposync command could do the same instead of skipping these packages, I'm sure others have come across this issue.

Copy link
Contributor Author

@dmnks dmnks Jun 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's why we made it possible to enable the original (unsafe) behavior; just pass the --allow-path-traversal CLI option.

@PaulFurtado
Copy link

Unfortunately, that argument allows actually path traversal on the filesystem. We have a fixed directory on our server for packages and we want all of these packages to live somewhere under that directory.

So one option is to strip out the path traversal characters so we treat it as a relative path. For example, if the desired directory to download into was /tmp/repo and the upstream rpm path was ../../pool/name.rpm it would download to /tmp/repo/pool/name.rpm instead of /pool/name.rpm.

The other solution could be to have a flag to flatten it all to one dir so it would end up being /tmp/repo/name.rpm

@dmnks
Copy link
Contributor Author

dmnks commented Jun 19, 2019

Ah ok, gotcha.

The thing is, reposync (the original yum-based implementation in this repo) is no longer actively developed upstream. We are still fixing occasional bugs reported by our (Red Hat) customers, but we no longer backport new features into RHEL-7/CentOS7. This would definitely fall into the "features" bucket.

The community has moved to the dnf-based version of reposync which is shipped to Fedora, RHEL-8 and even RHEL-7 via the Extras channel (as a self-supported tech-preview). The use case you described makes sense and you're welcome to file an RFE against Fedora or, if you're a RH customer, a support ticket, to request such a feature, or better yet, just create a PR! :)

@dmnks
Copy link
Contributor Author

dmnks commented Jun 19, 2019

That being said, (not to sound negative :) you’re welcome to contribute a PR here and I’d be happy to merge it for you, but like I said, you’d still have to make your own build of yum-utils in order to get that into RHEL/CentOS, I’m afraid. At that point, it’d probably be easier to just maintain your own fork.

@PaulFurtado
Copy link

@dmnks you definitely don't sound negative, we patch plenty of distro packages, so that isn't unreasonable to me and we also already have hacked around this with --urls and wget.

That said, since currently stuck on 6, I didn't realize there was a dnf based version of this tool, but if course there is. I might actually switch over to that on our repo mirrors and handle it in the more modern dnf based tool.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants