From a63cbcb1f712807c359ee0db11fc68b2e17f58b6 Mon Sep 17 00:00:00 2001
From: Kevin Veen-Birkenbach <kevin@veen.world>
Date: Wed, 4 Nov 2026 23:21:53 +0100
Subject: [PATCH] fix(apparmor): leave a read-only /proc/sys alone in teardown
 and restore
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fork run 41911075483 (feature/store-i18n 2fd6c6d310) lost both workspace
jobs, debian:13 and ubuntu:26.04, in make security-apparmor-teardown:

  common.sh: line 75: /proc/sys/kernel/apparmor_restrict_unprivileged_unconfined: Read-only file system

Inside the workspace container /proc/sys is mounted read-only, and the
write that #1371 added ran under set -e. A sysctl that cannot be written
now leaves the restriction as it is with a log line instead of failing
the teardown, and only a successful write records the previous value.
Restore keeps its record when it cannot write the sysctl back.

The tests call the helpers under set -euo pipefail like teardown.sh and
use /proc/sys/kernel/ostype as a sysctl nobody can write; both new tests
fail against the previous common.sh.

| 🎯 Fix | 💥 Break | 🌱 Efficiency | ⚡ Speed | 🛡️ Security | 🏆 Score | Gate |
| --- | --- | --- | --- | --- | --- | --- |
| 95 % | 3 % | – | – | ~100 % | 92 | ✅ 90/5, efficiency and speed <= 100, security 100 % |

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 scripts/system/apparmor/common.sh             | 10 ++++++--
 .../scripts/system/apparmor/test_common.py    | 23 ++++++++++++++++++-
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/scripts/system/apparmor/common.sh b/scripts/system/apparmor/common.sh
index 914a2b9b84..c57976ee8e 100755
--- a/scripts/system/apparmor/common.sh
+++ b/scripts/system/apparmor/common.sh
@@ -70,9 +70,12 @@ apparmor_lift_unconfined_restriction() {
 		return 0
 	fi
 
+	if ! printf '0\n' 2>/dev/null >"${sysctl}"; then
+		apparmor_log "[apparmor] ${sysctl} is not writable here; the unprivileged-unconfined restriction stays ${current}"
+		return 0
+	fi
 	mkdir -p "$(dirname "${state}")"
 	printf '%s\n' "${current}" >"${state}"
-	printf '0\n' >"${sysctl}"
 	apparmor_log "[apparmor] lifted unprivileged-unconfined restriction (was ${current}); containers keep plain docker-default"
 }
 
@@ -87,7 +90,10 @@ apparmor_reinstate_unconfined_restriction() {
 		return 0
 	fi
 
-	tr -d '[:space:]' <"${state}" >"${sysctl}"
+	if ! tr -d '[:space:]' <"${state}" 2>/dev/null >"${sysctl}"; then
+		apparmor_log "[apparmor] ${sysctl} is not writable here; ${state} stays for a later restore"
+		return 0
+	fi
 	rm -f "${state}"
 	apparmor_log "[apparmor] reinstated unprivileged-unconfined restriction"
 }
diff --git a/tests/unit/python/scripts/system/apparmor/test_common.py b/tests/unit/python/scripts/system/apparmor/test_common.py
index c688d8b670..b6935fa6b7 100644
--- a/tests/unit/python/scripts/system/apparmor/test_common.py
+++ b/tests/unit/python/scripts/system/apparmor/test_common.py
@@ -15,6 +15,7 @@ from pathlib import Path
 from utils.cache.files import PROJECT_ROOT
 
 COMMON = PROJECT_ROOT / "scripts" / "system" / "apparmor" / "common.sh"
+READ_ONLY_SYSCTL = Path("/proc/sys/kernel/ostype")
 
 PROFILES = """wg-quick (enforce)
 wg-quick//ip (enforce)
@@ -51,7 +52,7 @@ def _call(function: str, sysctl: Path, state: Path) -> subprocess.CompletedProce
         [
             "bash",
             "-c",
-            f'source "{COMMON}"; {function} "$1" "$2"',
+            f'set -euo pipefail; source "{COMMON}"; {function} "$1" "$2"',
             "-",
             str(sysctl),
             str(state),
@@ -114,6 +115,26 @@ class TestUnconfinedRestriction(unittest.TestCase):
         self.assertEqual(result.returncode, 0, result.stderr)
         self.assertEqual(_value(self.sysctl), "0")
 
+    @unittest.skipUnless(READ_ONLY_SYSCTL.is_file(), "needs procfs")
+    def test_a_read_only_sysctl_leaves_the_teardown_running(self) -> None:
+        result = _call(
+            "apparmor_lift_unconfined_restriction", READ_ONLY_SYSCTL, self.state
+        )
+        self.assertEqual(result.returncode, 0, result.stderr)
+        self.assertIn("not writable", result.stdout)
+        self.assertFalse(self.state.exists())
+
+    @unittest.skipUnless(READ_ONLY_SYSCTL.is_file(), "needs procfs")
+    def test_a_read_only_sysctl_keeps_the_record_for_a_later_restore(self) -> None:
+        self.state.parent.mkdir(parents=True)
+        self.state.write_text("1\n")
+        result = _call(
+            "apparmor_reinstate_unconfined_restriction", READ_ONLY_SYSCTL, self.state
+        )
+        self.assertEqual(result.returncode, 0, result.stderr)
+        self.assertIn("not writable", result.stdout)
+        self.assertEqual(_value(self.state), "1")
+
 
 if __name__ == "__main__":
     unittest.main()