launchpad-reviewers team mailing list archive

[Jürgen Gmach <mp+427221@xxxxxxxxxxxxxxxxxx>]

Jürgen Gmach has proposed merging ~jugmac00/launchpad:redact-secrets into launchpad:master.

Commit message:
buildd-manager: redact secrets

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jugmac00/launchpad/+git/launchpad/+merge/427221
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~jugmac00/launchpad:redact-secrets into launchpad:master.

diff --git a/lib/lp/buildmaster/model/buildfarmjobbehaviour.py b/lib/lp/buildmaster/model/buildfarmjobbehaviour.py
index 6c894c0..42bb426 100644
--- a/lib/lp/buildmaster/model/buildfarmjobbehaviour.py
+++ b/lib/lp/buildmaster/model/buildfarmjobbehaviour.py
@@ -119,6 +119,10 @@ class BuildFarmJobBehaviourBase:
         """The default behaviour is a no-op."""
         pass
 
+    def _redact_xmlrpc_arguments(self, args):
+        # we do not want to have secrets in logs
+        return sanitise_urls(repr(args))
+
     @defer.inlineCallbacks
     def dispatchBuildToWorker(self, logger):
         """See `IBuildFarmJobBehaviour`."""
@@ -171,7 +175,7 @@ class BuildFarmJobBehaviourBase:
                 cookie,
                 self.build.title,
                 self._builder.url,
-                sanitise_urls(repr(combined_args)),
+                self._redact_xmlrpc_arguments(combined_args),
             )
         )
 
diff --git a/lib/lp/code/model/cibuildbehaviour.py b/lib/lp/code/model/cibuildbehaviour.py
index 7398b9c..a1a985b 100644
--- a/lib/lp/code/model/cibuildbehaviour.py
+++ b/lib/lp/code/model/cibuildbehaviour.py
@@ -109,6 +109,13 @@ class CIBuildBehaviour(BuilderProxyMixin, BuildFarmJobBehaviourBase):
 
     ALLOWED_STATUS_NOTIFICATIONS = []
 
+    def _redact_xmlrpc_arguments(self, args):
+        # we do not want to have secrets in logs
+        if args["args"].get("secrets"):
+            for key in args["args"]["secrets"].keys():
+                args["args"]["secrets"][key] = "<redacted>"
+        return super()._redact_xmlrpc_arguments(args)
+
     def getLogFileName(self):
         return "buildlog_ci_%s_%s_%s.txt" % (
             self.build.git_repository.name,
diff --git a/lib/lp/code/model/tests/test_cibuildbehaviour.py b/lib/lp/code/model/tests/test_cibuildbehaviour.py
index d69505a..02ec5fd 100644
--- a/lib/lp/code/model/tests/test_cibuildbehaviour.py
+++ b/lib/lp/code/model/tests/test_cibuildbehaviour.py
@@ -470,6 +470,48 @@ class TestAsyncCIBuildBehaviour(StatsMixin, TestCIBuildBehaviourBase):
         )
 
     @defer.inlineCallbacks
+    def test_extraBuildArgs_secrets_get_redacted(self):
+        self.pushConfig(
+            "cibuild.soss",
+            secrets=json.dumps(
+                {
+                    "soss_read_auth": "%(read_auth)s",
+                    "more_secrets": "confidential",
+                }
+            ),
+        )
+        self.pushConfig("builddmaster", builder_proxy_host=None)
+
+        package = self.factory.makeDistributionSourcePackage(
+            distribution=self.factory.makeDistribution(name="soss")
+        )
+        git_repository = self.factory.makeGitRepository(target=package)
+        job = self.makeJob(
+            stages=[[("test", 0)]], git_repository=git_repository
+        )
+
+        builder = MockBuilder()
+        builder.processor = job.build.processor
+        worker = OkWorker()
+        job.setBuilder(builder, worker)
+        chroot_lfa = self.factory.makeLibraryFileAlias(db_only=True)
+        job.build.distro_arch_series.addOrUpdateChroot(
+            chroot_lfa, image_type=BuildBaseImageType.CHROOT
+        )
+        lxd_lfa = self.factory.makeLibraryFileAlias(db_only=True)
+        job.build.distro_arch_series.addOrUpdateChroot(
+            lxd_lfa, image_type=BuildBaseImageType.LXD
+        )
+        yield job.dispatchBuildToWorker(DevNullLogger())
+
+        self.assertEqual(
+            "<redacted>", worker.call_log[1][5]["secrets"]["soss_read_auth"]
+        )
+        self.assertEqual(
+            "<redacted>", worker.call_log[1][5]["secrets"]["more_secrets"]
+        )
+
+    @defer.inlineCallbacks
     def test_extraBuildArgs_archive_trusted_keys(self):
         # If the archive has a signing key, extraBuildArgs sends it.
         yield self.useFixture(InProcessKeyServerFixture()).start()