From f86615aaa566994a2aa506b0453247d38aff35a4 Mon Sep 17 00:00:00 2001
From: Martin Pitt <mpitt@redhat.com>
Date: Thu, 14 Nov 2024 16:25:22 +0100
Subject: [PATCH] bridge: Fix crash on invalid cockpit.conf

If there is a syntax error in cockpit.conf, the bridge previously
crashed with e.g. a `configparser.MissingSectionHeaderError`.

Intercept that, log a warning instead, and ignore the config file
instead.

https://bugzilla.redhat.com/show_bug.cgi?id=2324979
---
 pkg/base1/test-dbus.js       | 14 ++++++++++++++
 src/cockpit/config.py        |  7 ++++++-
 test/verify/check-connection | 15 +++++++++++++++
 3 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/pkg/base1/test-dbus.js b/pkg/base1/test-dbus.js
index 0a75ba1d4b48..bb9f4f803fb8 100644
--- a/pkg/base1/test-dbus.js
+++ b/pkg/base1/test-dbus.js
@@ -524,6 +524,20 @@ Empty=
     assert.rejects(proxy.GetString("SomeSection", "UnknownKey"),
                    /key.*UnknownKey.*not exist/,
                    "unknown key raises an error");
+
+    // empty config
+    await cockpit.file(configDir + "/cockpit/cockpit.conf").replace("");
+    await proxy.Reload();
+    assert.rejects(proxy.GetString("SomeSection", "SomeA"),
+                   /key.*SomeSection.*not exist/,
+                   "query in empty config raises an error");
+
+    // broken config (missing section header)
+    await cockpit.file(configDir + "/cockpit/cockpit.conf").replace("SomeA = two");
+    await proxy.Reload();
+    assert.rejects(proxy.GetString("SomeSection", "SomeA"),
+                   /key.*SomeSection.*not exist/,
+                   "query in broken config raises an error");
 });
 
 QUnit.test("nonexisting address", async assert => {
diff --git a/src/cockpit/config.py b/src/cockpit/config.py
index 04d017e37e83..2c89240b03c6 100644
--- a/src/cockpit/config.py
+++ b/src/cockpit/config.py
@@ -78,7 +78,12 @@ def reload(self):
         cockpit_conf = lookup_config('cockpit.conf')
         logger.debug("cockpit.Config: loading %s", cockpit_conf)
         # this may not exist, but it's ok to not have a config file and thus leave self.config empty
-        self.config.read(cockpit_conf)
+        try:
+            self.config.read(cockpit_conf)
+        except configparser.Error as exc:
+            logger.warning("cockpit.conf is invalid: %s", exc)
+            self.config.clear()
+            return
 
 
 class Environment(bus.Object, interface='cockpit.Environment'):
diff --git a/test/verify/check-connection b/test/verify/check-connection
index 7a05826712cb..d0b502d01a23 100755
--- a/test/verify/check-connection
+++ b/test/verify/check-connection
@@ -1209,6 +1209,21 @@ UnixPath=/run/cockpit/session
         self.login_and_go(None)
         b.wait_visible("#nav-system li:contains(Hackices)")
         self.assertFalse(b.is_present("#nav-system li:contains(Services)"))
+        b.logout()
+
+        # still starts with a broken config (no section headers)
+        m.write("/etc/test-xdg/cockpit/cockpit.conf", "LoginTitle=NoNoNO\n")
+        m.execute("systemctl stop cockpit")
+        self.login_and_go(None)
+        b.wait_visible("#nav-system li:contains(Hackices)")
+        self.assertFalse(b.is_present("#nav-system li:contains(Services)"))
+        b.logout()
+        # message from cockpit-ws
+        self.allow_journal_messages(".*cockpit.conf: key=val line not in any section.*")
+        # message from bridge
+        self.allow_journal_messages(".*cockpit.conf is invalid: File contains no section headers.*")
+        self.allow_journal_messages(".*cockpit.conf.*line: 1")
+        self.allow_journal_messages(".*LoginTitle=NoNoNO.*")
 
     @testlib.nondestructive
     @testlib.skipBeiboot("no cockpit-bridge binary on host in beiboot mode")