Simone Pelosi has proposed merging ~pelpsi/launchpad-buildd:remove-assert-usage into launchpad-buildd:master.
Commit message: Remove assert usage and log try-catch This is to improve the security posture of the launchpad-buildd project. We fixed all the warnings from ruff security checks. The ones suppressed are intentional. Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~pelpsi/launchpad-buildd/+git/launchpad-buildd/+merge/490604 -- Your team Launchpad code reviewers is requested to review the proposed merge of ~pelpsi/launchpad-buildd:remove-assert-usage into launchpad-buildd:master.
diff --git a/debian/changelog b/debian/changelog index dd16d81..358617a 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,6 +1,8 @@ launchpad-buildd (257) UNRELEASED; urgency=medium - * Use requests Basic Auth for security. + * Use requests Basic Auth for security. + * Remove `assert` usage. + * Log `try-catch`. -- Simone Pelosi <[email protected]> Wed, 23 Jul 2025 15:42:34 +0200 diff --git a/lpbuildd/target/lxd.py b/lpbuildd/target/lxd.py index ab35590..8bdbb39 100644 --- a/lpbuildd/target/lxd.py +++ b/lpbuildd/target/lxd.py @@ -3,6 +3,7 @@ import io import json +import logging import os import platform import re @@ -389,8 +390,8 @@ class LXD(Backend): with open(self.dnsmasq_pid_file) as f: try: dnsmasq_pid = int(f.read()) - except Exception: - pass + except Exception as e: + logging.warning("Failed to read dnsmasq PID file: %s", e) else: # dnsmasq is supposed to drop privileges, but kill it as # root just in case it fails to do so for some reason. diff --git a/lpbuildd/target/vcs.py b/lpbuildd/target/vcs.py index 2f79225..e287c87 100644 --- a/lpbuildd/target/vcs.py +++ b/lpbuildd/target/vcs.py @@ -45,7 +45,10 @@ class VCSOperationMixin(StatusOperationMixin): if self.args.branch is not None: return self.args.branch else: - assert self.args.git_repository is not None + if self.args.git_repository is None: + raise ValueError( + "git_repository must be provided when branch is None" + ) description = self.args.git_repository if self.args.git_path is not None: description += " " + self.args.git_path @@ -85,7 +88,10 @@ class VCSOperationMixin(StatusOperationMixin): cmd.insert(1, "-Ossl.cert_reqs=none") else: # this handles the git case - assert self.args.git_repository is not None + if self.args.git_repository is None: + raise ValueError( + "git_repository must be provided for git operations" + ) cmd = ["git", "clone", "-n"] if quiet: cmd.append("-q") diff --git a/lpbuildd/util.py b/lpbuildd/util.py index 9231206..ce483f4 100644 --- a/lpbuildd/util.py +++ b/lpbuildd/util.py @@ -1,7 +1,6 @@ # Copyright 2015-2017 Canonical Ltd. This software is licensed under the # GNU Affero General Public License version 3 (see the file LICENSE). -import base64 import os import subprocess import sys @@ -48,7 +47,8 @@ def get_arch_bits(arch): def set_personality(args, arch, series=None): bits = get_arch_bits(arch) - assert bits in (32, 64) + if bits not in (32, 64): + raise ValueError(f"Unsupported architecture bit size: {bits}") if bits == 32: setarch_cmd = ["linux32"] else: diff --git a/pyproject.toml b/pyproject.toml index 3168b1e..2bfbbab 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,19 @@ [tool.black] line-length = 79 -target-version = ['py38'] +target-version = ["py38"] [tool.isort] line_length = 79 + +[tool.ruff.lint] +# Enable all security rules (SXXX = Bandit checks) +select = ["S"] + +# Ignore specific security checks that are intentionally allowed +ignore = [ + "S113", # request timeout - intention + "S324", # SHA1 usage - legacy + "S603", # subprocess call - intentional + "S606", # os.execv without shell - intentional + "S607", # partial executable paths - intentional +]
_______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
