diff --git a/scripts/install/configure_web_sudo.sh b/scripts/install/configure_web_sudo.sh index feccc3e7..bbcc14ed 100644 --- a/scripts/install/configure_web_sudo.sh +++ b/scripts/install/configure_web_sudo.sh @@ -100,10 +100,15 @@ TEMP_SUDOERS="/tmp/ledmatrix_web_sudoers_$$" echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH restart ledmatrix-web.service" # Optional: journalctl (non-critical — skip if not found) + # + # NOEXEC, matching first_time_install.sh. These rules end in a wildcard and + # journalctl starts a pager, so without it the caller can reach a shell: + # less runs "!command" as the user the pager belongs to, which here is + # root. NOEXEC stops the granted command executing anything of its own. if [ -n "$JOURNALCTL_PATH" ]; then - echo "$WEB_USER ALL=(ALL) NOPASSWD: $JOURNALCTL_PATH -u ledmatrix.service *" - echo "$WEB_USER ALL=(ALL) NOPASSWD: $JOURNALCTL_PATH -u ledmatrix *" - echo "$WEB_USER ALL=(ALL) NOPASSWD: $JOURNALCTL_PATH -t ledmatrix *" + echo "$WEB_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -u ledmatrix.service *" + echo "$WEB_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -u ledmatrix *" + echo "$WEB_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -t ledmatrix *" fi # Required: python3, bash diff --git a/test/test_sudoers_noexec_on_pagers.py b/test/test_sudoers_noexec_on_pagers.py index 623622d7..ca657785 100644 --- a/test/test_sudoers_noexec_on_pagers.py +++ b/test/test_sudoers_noexec_on_pagers.py @@ -30,6 +30,10 @@ ROOT = Path(__file__).resolve().parent.parent INSTALLERS = ( ROOT / "first_time_install.sh", ROOT / "scripts" / "install" / "configure_wifi_permissions.sh", + # Writes the same journalctl grants as first_time_install.sh. It was + # missing here, and because of that this suite passed while three + # ungranted wildcard rules sat in it. + ROOT / "scripts" / "install" / "configure_web_sudo.sh", ) #: Commands that will start another program of their own accord -- a pager, an @@ -44,8 +48,14 @@ def _grant_lines(): continue for line in installer.read_text(encoding="utf-8", errors="replace").splitlines(): stripped = line.strip() - if "NOPASSWD" in stripped and not stripped.startswith("#"): - lines.append(stripped) + if "NOPASSWD" not in stripped or stripped.startswith("#"): + continue + # Installers emit rules two ways: written literally into a heredoc, + # or echoed into a file. An echoed rule ends in a quote, so the + # trailing-wildcard check below would skip it and the rule would + # never be examined at all. + echoed = re.fullmatch(r"""echo\s+(['"])(.*)\1""", stripped) + lines.append(echoed.group(2) if echoed else stripped) return lines @@ -78,10 +88,12 @@ def test_journalctl_is_granted_at_all(): "no journalctl grant remains; the web interface reads logs through it") -@pytest.mark.parametrize("unit", ["ledmatrix.service", "ledmatrix"]) -def test_each_journalctl_rule_is_tagged(unit): +@pytest.mark.parametrize("selector", ["-u ledmatrix.service", "-u ledmatrix", + "-t ledmatrix"]) +def test_each_journalctl_rule_is_tagged(selector): + """Every selector, so removing one cannot pass by the others' presence.""" matching = [r for r in _grant_lines() - if "JOURNALCTL_PATH" in r and f"-u {unit} " in r] - assert matching, f"no journalctl rule for -u {unit}" + if "JOURNALCTL_PATH" in r and f"{selector} " in r] + assert matching, f"no journalctl rule for {selector}" untagged = [r for r in matching if "NOEXEC" not in r] assert not untagged, f"untagged journalctl rule(s): {untagged}"