launchpad-reviewers team mailing list archive

[Simone Pelosi <mp+489527@xxxxxxxxxxxxxxxxxx>]

Simone Pelosi has proposed merging ~pelpsi/launchpad-buildd:move-to-requests-basic-auth into launchpad-buildd:master.

Commit message:
Use requests Basic Auth for security

Replaces manual Base64 encoding and Authorization header construction
with requests.auth.HTTPBasicAuth, which is the idiomatic and safer
way to perform HTTP Basic Authentication in Python.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pelpsi/launchpad-buildd/+git/launchpad-buildd/+merge/489527
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pelpsi/launchpad-buildd:move-to-requests-basic-auth into launchpad-buildd:master.

diff --git a/debian/changelog b/debian/changelog
index dd23282..dd16d81 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+launchpad-buildd (257) UNRELEASED; urgency=medium
+
+  * Use requests Basic Auth for security. 
+
+ -- Simone Pelosi <simone.pelosi@xxxxxxxxxxxxx>  Wed, 23 Jul 2025 15:42:34 +0200
+
 launchpad-buildd (256) noble; urgency=medium
 
   [ Alvaro Crespo ]
diff --git a/lpbuildd/builder.py b/lpbuildd/builder.py
index 6c5c6b2..42e49bb 100644
--- a/lpbuildd/builder.py
+++ b/lpbuildd/builder.py
@@ -35,6 +35,7 @@ from lpbuildd.util import shell_escape
 devnull = open("/dev/null")
 
 
+# coverity[COV_PY_SIGMA.hardcoded_secret_pattern_low:SUPPRESS] doc example
 def _sanitizeURLs(bytes_seq):
     """A generator that deletes URL passwords from a bytes sequence.
 
@@ -501,6 +502,7 @@ class Builder:
                 else:
                     of = open(cachefile + ".tmp", "wb")
                     # Upped for great justice to 256k
+                    # coverity[COV_PY_SIGMA.weak_hash_core_python_hashlib:SUPPRESS]
                     check_sum = hashlib.sha1()
                     for chunk in iter(lambda: f.read(256 * 1024), b""):
                         of.write(chunk)
@@ -522,6 +524,7 @@ class Builder:
         tmppath = self.cachePath("storeFile.tmp")
         of = open(tmppath, "wb")
         try:
+            # coverity[COV_PY_SIGMA.weak_hash_core_python_hashlib:SUPPRESS]
             sha1 = hashlib.sha1()
             for chunk in iter(lambda: f.read(256 * 1024), b""):
                 sha1.update(chunk)
diff --git a/lpbuildd/target/tests/test_build_snap.py b/lpbuildd/target/tests/test_build_snap.py
index 9f56b08..0564a3e 100644
--- a/lpbuildd/target/tests/test_build_snap.py
+++ b/lpbuildd/target/tests/test_build_snap.py
@@ -9,6 +9,7 @@ import subprocess
 from textwrap import dedent
 
 import responses
+from requests.auth import HTTPBasicAuth
 from fixtures import FakeLogger, TempDir
 from systemfixtures import FakeFilesystem
 from testtools import TestCase
@@ -987,8 +988,13 @@ class TestBuildSnap(TestCase):
         )
         self.assertEqual(1, len(responses.calls))
         request = responses.calls[0].request
-        auth = base64.b64encode(b"username:password").decode()
-        self.assertEqual(f"Basic {auth}", request.headers["Authorization"])
+        auth = HTTPBasicAuth("username", "password")
+        self.assertEqual(
+            auth.password, request.headers["Authorization"].password
+        )
+        self.assertEqual(
+            auth.username, request.headers["Authorization"].username
+        )
         self.assertEqual("http://proxy-auth.example/tokens/1";, request.url)
         # XXX cjwatson 2023-02-07: Ideally we'd check the timeout as well,
         # but the version of responses in Ubuntu 20.04 doesn't store it
diff --git a/lpbuildd/tests/test_util.py b/lpbuildd/tests/test_util.py
index 09718b4..d035e91 100644
--- a/lpbuildd/tests/test_util.py
+++ b/lpbuildd/tests/test_util.py
@@ -5,6 +5,7 @@ import base64
 import json
 
 import responses
+from requests.auth import HTTPBasicAuth
 from testtools import TestCase
 
 from lpbuildd.util import (
@@ -77,7 +78,7 @@ class TestRevokeToken(TestCase):
 
         proxy_url = "http://username:password@proxy.example";
         revocation_endpoint = "http://proxy-auth.example/tokens/build_id";
-        token = base64.b64encode(b"username:password").decode()
+        basic_auth = HTTPBasicAuth("username", "password")
 
         responses.add(responses.DELETE, revocation_endpoint)
 
@@ -87,7 +88,14 @@ class TestRevokeToken(TestCase):
         self.assertEqual(
             "http://proxy-auth.example/tokens/build_id";, request.url
         )
-        self.assertEqual(f"Basic {token}", request.headers["Authorization"])
+        self.assertEqual(
+            basic_auth.password,
+            request.headers["Authorization"].password
+        )
+        self.assertEqual(
+            basic_auth.username,
+            request.headers["Authorization"].username
+        )
 
     @responses.activate
     def test_revoke_fetch_service_token(self):
diff --git a/lpbuildd/util.py b/lpbuildd/util.py
index f94b997..7b274ae 100644
--- a/lpbuildd/util.py
+++ b/lpbuildd/util.py
@@ -9,6 +9,7 @@ from shlex import quote
 from urllib.parse import urlparse
 
 import requests
+from requests.auth import HTTPBasicAuth
 
 
 def shell_escape(s):
@@ -96,9 +97,7 @@ def revoke_proxy_token(
     json_data = None
 
     if not use_fetch_service:
-        auth_string = f"{url.username}:{url.password}"
-        token = base64.b64encode(auth_string.encode()).decode()
-        headers = {"Authorization": f"Basic {token}"}
+        headers = {"Authorization": HTTPBasicAuth(url.username, url.password)}
     else:
         # When using the fetch service, we don't require authentication, but we
         # need to send the token we want to revoke in the payload.