From 2a74ec2bd3cd52ecbf898101762092cd6978e311 Mon Sep 17 00:00:00 2001 From: Jared Knowles Date: Thu, 4 Jun 2026 13:48:21 +0000 Subject: [PATCH] test: add security regression test and beep(type='all') coverage - Add test verifying .send_webhook() and .send_desktop_notify() use system2() (not system()) to prevent command injection. This test reads the function body and asserts system2 is present and system("...") is absent, so the vulnerability cannot be reintroduced by accident. - Add test for beep(type='all') exercising both beep and notify paths. --- tests/testthat/test_notifications.R | 34 +++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/testthat/test_notifications.R b/tests/testthat/test_notifications.R index 847e51f..e6e0b67 100644 --- a/tests/testthat/test_notifications.R +++ b/tests/testthat/test_notifications.R @@ -20,3 +20,37 @@ test_that("beep() with type=notify is silent when no tools available", { test_that("beep() with type=webhook and no URL is silent", { expect_silent(beep("test", type = "webhook")) }) + +test_that("beep(type='all') exercises both beep and notify paths", { + # type="all" should trigger the terminal beep (unless quiet) and + # attempt a desktop notification. We verify by checking that the + # function returns invisibly and does not error when notify-send + # is absent (which is the case in CI). + expect_invisible(beep("all-test", type = "all")) + expect_invisible(beep("all-test", type = "all", quiet = TRUE)) +}) + +test_that("notification helpers use system2() to prevent command injection", { + # Security regression: .send_webhook and .send_desktop_notify must + # use system2() with separate args, never system() with shell + # interpolation. system2() passes arguments directly to the + # executable without going through a shell, so shell metacharacters + # in msg, status, url, or payload are treated as literal data. + + webhook_body <- deparse(body(.send_webhook)) + notify_body <- deparse(body(.send_desktop_notify)) + + # Must use system2 + expect_true(any(grepl("system2", webhook_body)), + info = ".send_webhook() must use system2() to avoid shell injection") + expect_true(any(grepl("system2", notify_body)), + info = ".send_desktop_notify() must use system2() to avoid shell injection") + + # Must NOT use system( with string interpolation (system("curl ...")) + # We look for system( followed by a string literal (the old pattern). + # system2 calls look like system2("curl", args = ...) which is fine. + expect_false(any(grepl('system\\(\\s*"', webhook_body)), + info = ".send_webhook() must not use system() with interpolated strings") + expect_false(any(grepl('system\\(\\s*"', notify_body)), + info = ".send_desktop_notify() must not use system() with interpolated strings") +})