Skip to content

Support sftp in wrap file #14772

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions mesonbuild/wrap/wrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,29 @@ def get_data(self, urlstring: str) -> T.Tuple[str, str]:
resp = open_wrapdburl(urlstring, allow_insecure=self.allow_insecure, have_opt=self.wrap_frontend)
elif WHITELIST_SUBDOMAIN in urlstring:
raise WrapException(f'{urlstring} may be a WrapDB-impersonating URL')
elif url.scheme == 'sftp':
sftp = shutil.which('sftp')
if sftp is None:
raise WrapException('Scheme sftp is not available. Install sftp to enable it.')
with tempfile.TemporaryDirectory() as workdir, \
tempfile.NamedTemporaryFile(mode='wb', dir=self.cachedir, delete=False) as tmpfile:
args = []
if 'source_hostkey' in self.wrap.values:
with open(os.path.join(workdir, 'known_hosts'), 'w', encoding='utf-8') as f:
f.write(f'[{url.hostname}]:{url.port} {self.wrap.get("source_hostkey")}')
args += ['-o', 'UserKnownHostsFile=known_hosts']
if 'source_identityfile' in self.wrap.values:
args += ['-o', f'IdentityFile={self.wrap.get("source_identityfile")}']
Copy link
Member

Choose a reason for hiding this comment

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

Code that fiddles with the end user's crypto setup makes me especially anxious. If you set these arguments in the wrap file, then that file can only work on the machine it has been generated on (or en exact replica of it). There might also be some sort of a information leakage vulnerability here, but I don't have the skills to say so one way or another.

Because of these and similar reasons we try to stay away from access control as much as possible. Are these features absolutely necessary or can we just say "set up access control outside Meson"?

Copy link
Author

@ampleyfly ampleyfly Jul 26, 2025

Choose a reason for hiding this comment

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

The alternatives for testing the SSH part is authentication with either a key pair (which is generated in each run here), or a password (meaning we need to have a test user with a known password).
The intention was not that "source_identityfile" should be used outside of testing, as indicated by the commit message. If there's a better way to inject the necessary information into the code, for test purposes only, I'm open to suggestions.

Alternatives for the testing as a whole might be to:
a) skip SSH, just do the SFTP part (this would still require the code under test to know to connect in a different way, afaict), or
b) skip testing altogether.

EDIT: RE the crypto setup comment: In case it wasn't clear, the code does not modify the user's crypto configuration. It does, however, allow the user to specify a host public key and the path to a client identity file to use for the SFTP call. Again, this was done for testing purposes, so another way of accomplishing it would be nice.

# Older versions of the sftp client cannot handle URLs, hence the splitting of url below
if url.port:
args += ['-P', f'{url.port}']
user = f'{url.username}@' if url.username else ''
command = [sftp, '-o', 'KbdInteractiveAuthentication=no', *args, f'{user}{url.hostname}:{url.path[1:]}']
subprocess.run(command, cwd=workdir, check=True)
downloaded = os.path.join(workdir, os.path.basename(url.path))
tmpfile.close()
shutil.move(downloaded, tmpfile.name)
return self.hash_file(tmpfile.name), tmpfile.name
else:
headers = {
'User-Agent': f'mesonbuild/{coredata.version}',
Expand Down