Compare commits

...

8 Commits

Author SHA1 Message Date
Brad Warren
f6f9d9c163 only run relevant tests 2021-01-04 15:30:10 -08:00
Adrien Ferrand
34010d9b5d Fix name 2021-01-04 15:25:56 -08:00
Adrien Ferrand
0dc5b52be1 Enable windows tests on Python 3.8 and package it on Python 3.8 also. 2021-01-04 15:25:56 -08:00
Adrien Ferrand
f325bd1bea Add unit tests 2021-01-04 22:33:10 +01:00
Adrien Ferrand
5ce5ca9caa Raise error with long paths on Windows 2021-01-04 22:31:23 +01:00
Adrien Ferrand
5eec69236d Merge branch 'master-upstream' into forbid-readlink 2021-01-04 21:24:00 +01:00
Adrien Ferrand
dbd4c2a7ed Use readlink 2020-11-20 16:58:35 +01:00
Adrien Ferrand
9c0133e1fe Forbid os.readlink() 2020-11-20 13:56:57 +01:00
11 changed files with 85 additions and 171 deletions

View File

@@ -1,62 +1,11 @@
jobs:
- job: docker_build
pool:
vmImage: ubuntu-18.04
strategy:
matrix:
amd64:
DOCKER_ARCH: amd64
# Do not run the heavy non-amd64 builds for test branches
${{ if not(startsWith(variables['Build.SourceBranchName'], 'test-')) }}:
arm32v6:
DOCKER_ARCH: arm32v6
arm64v8:
DOCKER_ARCH: arm64v8
steps:
- bash: set -e && tools/docker/build.sh $(dockerTag) $DOCKER_ARCH
displayName: Build the Docker images
# We don't filter for the Docker Hub organization to continue to allow
# easy testing of these scripts on forks.
- bash: |
set -e
DOCKER_IMAGES=$(docker images --filter reference='*/certbot' --filter reference='*/dns-*' --format '{{.Repository}}')
docker save --output images.tar $DOCKER_IMAGES
displayName: Save the Docker images
# If the name of the tar file or artifact changes, the deploy stage will
# also need to be updated.
- bash: set -e && mv images.tar $(Build.ArtifactStagingDirectory)
displayName: Prepare Docker artifact
- task: PublishPipelineArtifact@1
inputs:
path: $(Build.ArtifactStagingDirectory)
artifact: docker_$(DOCKER_ARCH)
displayName: Store Docker artifact
- job: docker_run
dependsOn: docker_build
pool:
vmImage: ubuntu-18.04
steps:
- task: DownloadPipelineArtifact@2
inputs:
artifact: docker_amd64
path: $(Build.SourcesDirectory)
displayName: Retrieve Docker images
- bash: set -e && docker load --input $(Build.SourcesDirectory)/images.tar
displayName: Load Docker images
- bash: |
set -ex
DOCKER_IMAGES=$(docker images --filter reference='*/certbot' --filter reference='*/dns-*' --format '{{.Repository}}:{{.Tag}}')
for DOCKER_IMAGE in ${DOCKER_IMAGES}
do docker run --rm "${DOCKER_IMAGE}" plugins --prepare
done
displayName: Run integration tests for Docker images
- job: installer_build
pool:
vmImage: vs2017-win2016
steps:
- task: UsePythonVersion@0
inputs:
versionSpec: 3.7
versionSpec: 3.8
architecture: x86
addToPath: true
- script: python windows-installer/construct.py
@@ -113,105 +62,3 @@ jobs:
set PATH=%ProgramFiles(x86)%\Certbot\bin;%PATH%
venv\Scripts\python -m pytest certbot-ci\certbot_integration_tests\certbot_tests -n 4
displayName: Run certbot integration tests
- job: snaps_build
pool:
vmImage: ubuntu-18.04
timeoutInMinutes: 0
variables:
# Do not run the heavy non-amd64 builds for test branches
${{ if not(startsWith(variables['Build.SourceBranchName'], 'test-')) }}:
ARCHS: amd64 arm64 armhf
${{ if startsWith(variables['Build.SourceBranchName'], 'test-') }}:
ARCHS: amd64
steps:
- script: |
set -e
sudo apt-get update
sudo apt-get install -y --no-install-recommends snapd
sudo snap install --classic snapcraft
displayName: Install dependencies
- task: UsePythonVersion@0
inputs:
versionSpec: 3.8
addToPath: true
- task: DownloadSecureFile@1
name: credentials
inputs:
secureFile: launchpad-credentials
- script: |
set -e
git config --global user.email "$(Build.RequestedForEmail)"
git config --global user.name "$(Build.RequestedFor)"
mkdir -p ~/.local/share/snapcraft/provider/launchpad
cp $(credentials.secureFilePath) ~/.local/share/snapcraft/provider/launchpad/credentials
python3 tools/snap/build_remote.py ALL --archs ${ARCHS} --timeout 19800
displayName: Build snaps
- script: |
set -e
mv *.snap $(Build.ArtifactStagingDirectory)
mv certbot-dns-*/*.snap $(Build.ArtifactStagingDirectory)
displayName: Prepare artifacts
- task: PublishPipelineArtifact@1
inputs:
path: $(Build.ArtifactStagingDirectory)
artifact: snaps
displayName: Store snaps artifacts
- job: snap_run
dependsOn: snaps_build
pool:
vmImage: ubuntu-18.04
steps:
- task: UsePythonVersion@0
inputs:
versionSpec: 3.8
addToPath: true
- script: |
set -e
sudo apt-get update
sudo apt-get install -y --no-install-recommends nginx-light snapd
python3 -m venv venv
venv/bin/python tools/pipstrap.py
venv/bin/python tools/pip_install.py -U tox
displayName: Install dependencies
- task: DownloadPipelineArtifact@2
inputs:
artifact: snaps
path: $(Build.SourcesDirectory)/snap
displayName: Retrieve Certbot snaps
- script: |
set -e
sudo snap install --dangerous --classic snap/certbot_*_amd64.snap
displayName: Install Certbot snap
- script: |
set -e
venv/bin/python -m tox -e integration-external,apacheconftest-external-with-pebble
displayName: Run tox
- job: snap_dns_run
dependsOn: snaps_build
pool:
vmImage: ubuntu-18.04
steps:
- script: |
set -e
sudo apt-get update
sudo apt-get install -y --no-install-recommends snapd
displayName: Install dependencies
- task: UsePythonVersion@0
inputs:
versionSpec: 3.8
addToPath: true
- task: DownloadPipelineArtifact@2
inputs:
artifact: snaps
path: $(Build.SourcesDirectory)/snap
displayName: Retrieve Certbot snaps
- script: |
set -e
python3 -m venv venv
venv/bin/python tools/pipstrap.py
venv/bin/python tools/pip_install.py -e certbot-ci
displayName: Prepare Certbot-CI
- script: |
set -e
sudo -E venv/bin/pytest certbot-ci/snap_integration_tests/dns_tests --allow-persistent-changes --snap-folder $(Build.SourcesDirectory)/snap --snap-arch amd64
displayName: Test DNS plugins snaps

View File

@@ -16,13 +16,13 @@ jobs:
IMAGE_NAME: vs2017-win2016
PYTHON_VERSION: 3.6
TOXENV: py36
windows-py37-cover:
windows-py38-cover:
IMAGE_NAME: vs2017-win2016
PYTHON_VERSION: 3.7
TOXENV: py37-cover
PYTHON_VERSION: 3.8
TOXENV: py38-cover
windows-integration-certbot:
IMAGE_NAME: vs2017-win2016
PYTHON_VERSION: 3.7
PYTHON_VERSION: 3.8
TOXENV: integration-certbot
linux-oldest-tests-1:
IMAGE_NAME: ubuntu-18.04

View File

@@ -2,5 +2,4 @@ stages:
- stage: TestAndPackage
jobs:
- template: ../jobs/standard-tests-jobs.yml
- template: ../jobs/extended-tests-jobs.yml
- template: ../jobs/packaging-jobs.yml

View File

@@ -20,6 +20,7 @@ from certbot import interfaces
from certbot import util
from certbot._internal import constants
from certbot.compat import os
from certbot.compat import filesystem
logger = logging.getLogger(__name__)
@@ -324,7 +325,7 @@ class AccountFileStorage(interfaces.AccountStorage):
if server_path in reused_servers:
next_server_path = reused_servers[server_path]
next_dir_path = link_func(next_server_path)
if os.path.islink(next_dir_path) and os.readlink(next_dir_path) == dir_path:
if os.path.islink(next_dir_path) and filesystem.readlink(next_dir_path) == dir_path:
possible_next_link = True
server_path = next_server_path
dir_path = next_dir_path
@@ -332,7 +333,7 @@ class AccountFileStorage(interfaces.AccountStorage):
# if there's not a next one up to delete, then delete me
# and whatever I link to
while os.path.islink(dir_path):
target = os.readlink(dir_path)
target = filesystem.readlink(dir_path)
os.unlink(dir_path)
dir_path = target

View File

@@ -214,7 +214,7 @@ def get_link_target(link):
"""
try:
target = os.readlink(link)
target = filesystem.readlink(link)
except OSError:
raise errors.CertStorageError(
"Expected {0} to be a symlink".format(link))
@@ -223,6 +223,7 @@ def get_link_target(link):
target = os.path.join(os.path.dirname(link), target)
return os.path.abspath(target)
def _write_live_readme_to(readme_path, is_base_dir=False):
prefix = ""
if is_base_dir:
@@ -665,7 +666,7 @@ class RenewableCert(interfaces.RenewableCert):
current_link = getattr(self, kind)
if os.path.lexists(current_link):
os.unlink(current_link)
os.symlink(os.readlink(previous_link), current_link)
os.symlink(filesystem.readlink(previous_link), current_link)
for _, link in previous_symlinks:
if os.path.exists(link):
@@ -846,7 +847,7 @@ class RenewableCert(interfaces.RenewableCert):
link = getattr(self, kind)
filename = "{0}{1}.pem".format(kind, version)
# Relative rather than absolute target directory
target_directory = os.path.dirname(os.readlink(link))
target_directory = os.path.dirname(filesystem.readlink(link))
# TODO: it could be safer to make the link first under a temporary
# filename, then unlink the old link, then rename the new link
# to the old link; this ensures that this process is able to
@@ -1121,7 +1122,7 @@ class RenewableCert(interfaces.RenewableCert):
# The behavior below keeps the prior key by creating a new
# symlink to the old key or the target of the old key symlink.
if os.path.islink(old_privkey):
old_privkey = os.readlink(old_privkey)
old_privkey = filesystem.readlink(old_privkey)
else:
old_privkey = "privkey{0}.pem".format(prior_version)
logger.debug("Writing symlink to old private key, %s.", old_privkey)

View File

@@ -4,6 +4,7 @@ from __future__ import absolute_import
import errno
import os # pylint: disable=os-module-forbidden
import stat
import sys
from acme.magic_typing import List
@@ -361,7 +362,8 @@ def realpath(file_path):
"""
original_path = file_path
if POSIX_MODE:
# Since Python 3.8, os.path.realpath also resolves symlinks on Windows.
if POSIX_MODE or sys.version_info >= (3, 8):
path = os.path.realpath(file_path)
if os.path.islink(path):
# If path returned by realpath is still a link, it means that it failed to
@@ -383,8 +385,35 @@ def realpath(file_path):
return os.path.abspath(file_path)
def readlink(link_path):
# type: (str) -> str
"""
Return a string representing the path to which the symbolic link points.
:param str link_path: The symlink path to resolve
:return: The path the symlink points to
:returns: str
:raise: ValueError if a long path (260> characters) is encountered on Windows
"""
path = os.readlink(link_path)
if POSIX_MODE or not path.startswith('\\\\?\\'):
return path
# At that point, we are on Windows and the path returned uses the long form (Python 3.8+).
# Max length of a normal path is 260 characters on Windows, including the non printable
# termination character "<NUL>". The termination character is not included in Python
# strings, giving a max length of 259 characters, + 4 characters for the extended form
# prefix, to an effective max length 263 characters on a string representing a normal path.
if len(path) < 264:
return path[4:]
raise ValueError("Long paths are not supported by Certbot on Windows.")
# On Windows is_executable run from an unprivileged shell may claim that a path is
# executable when it is excutable only if run from a privileged shell. This result
# executable when it is executable only if run from a privileged shell. This result
# is due to the fact that GetEffectiveRightsFromAcl calculate effective rights
# without taking into consideration if the target user has currently required the
# elevated privileges or not. However this is not a problem since certbot always

View File

@@ -152,3 +152,14 @@ def fstat(*unused_args, **unused_kwargs):
raise RuntimeError('Usage of os.fstat() is forbidden. '
'Use certbot.compat.filesystem functions instead '
'(eg. has_min_permissions, has_same_ownership).')
# Method os.readlink has a significant behavior change with Python 3.8+. Starting
# with this version, it will return the resolved path in its "extended-style" form
# unconditionally, which allows to use more than 259 characters, and its string
# representation is prepended with "\\?\". Problem is that it does it for any path,
# and will make equality comparison fail with paths that will use the simple form.
def readlink(*unused_args, **unused_kwargs):
"""Method os.readlink() is forbidden"""
raise RuntimeError('Usage of os.readlink() is forbidden. '
'Use certbot.compat.filesystem.realpath() instead.')

View File

@@ -99,7 +99,7 @@ class UpdateLiveSymlinksTest(BaseCertManagerTest):
for kind in ALL_FOUR:
os.chdir(os.path.dirname(self.config_files[domain][kind]))
self.assertEqual(
filesystem.realpath(os.readlink(self.config_files[domain][kind])),
filesystem.realpath(filesystem.readlink(self.config_files[domain][kind])),
filesystem.realpath(archive_paths[domain][kind]))
finally:
os.chdir(prev_dir)

View File

@@ -597,6 +597,32 @@ class IsExecutableTest(test_util.TempDirTestCase):
self.assertFalse(filesystem.is_executable("exe"))
class ReadlinkTest(unittest.TestCase):
@unittest.skipUnless(POSIX_MODE, reason='Tests specific to Linux')
@mock.patch("certbot.compat.filesystem.os.readlink")
def test_path_posix(self, mock_readlink):
mock_readlink.return_value = "/normal/path"
self.assertEqual(filesystem.readlink("dummy"), "/normal/path")
@unittest.skipIf(POSIX_MODE, reason='Tests specific to Windows')
@mock.patch("certbot.compat.filesystem.os.readlink")
def test_normal_path_windows(self, mock_readlink):
# Python <3.8
mock_readlink.return_value = "C:\\short\\path"
self.assertEqual(filesystem.readlink("dummy"), "C:\\short\\path")
# Python >=3.8 (os.readlink always returns the extended form)
mock_readlink.return_value = "\\\\?\\C:\\short\\path"
self.assertEqual(filesystem.readlink("dummy"), "C:\\short\\path")
@unittest.skipIf(POSIX_MODE, reason='Tests specific to Windows')
@mock.patch("certbot.compat.filesystem.os.readlink")
def test_extended_path_windows(self, mock_readlink):
# Following path is largely over the 260 characters permitted in the normal form.
mock_readlink.return_value = "\\\\?\\C:\\long" + 1000 * "\\path"
with self.assertRaises(ValueError):
filesystem.readlink("dummy")
@contextlib.contextmanager
def _fix_windows_runtime():
if os.name != 'nt':

View File

@@ -330,7 +330,7 @@ class RenewableCertTests(BaseRenewableCertTest):
self.test_rc._update_link_to("chain", 3000)
# However, current_version doesn't allow querying the resulting
# version (because it's a broken link).
self.assertEqual(os.path.basename(os.readlink(self.test_rc.chain)),
self.assertEqual(os.path.basename(filesystem.readlink(self.test_rc.chain)),
"chain3000.pem")
def test_version(self):
@@ -514,7 +514,7 @@ class RenewableCertTests(BaseRenewableCertTest):
# privkey.
for i in (6, 7, 8):
self.assertTrue(os.path.islink(self.test_rc.version("privkey", i)))
self.assertEqual("privkey3.pem", os.path.basename(os.readlink(
self.assertEqual("privkey3.pem", os.path.basename(filesystem.readlink(
self.test_rc.version("privkey", i))))
for kind in ALL_FOUR:

View File

@@ -9,7 +9,7 @@ import sys
import tempfile
import time
PYTHON_VERSION = (3, 7, 4)
PYTHON_VERSION = (3, 8, 6)
PYTHON_BITNESS = 32
PYWIN32_VERSION = 227 # do not forget to edit pywin32 dependency accordingly in setup.py
NSIS_VERSION = '3.04'