feat: improve protection against SQL injection

DESCRIPTION

@@ -1,7 +1,7 @@
 Type: Package
 Package: shiny.telemetry
 Title: 'Shiny' App Usage Telemetry
-Version: 0.2.0.9002
+Version: 0.2.0.9003
 Authors@R: c(
     person("André", "Veríssimo", , "[email protected]", role = c("aut", "cre")),
     person("Kamil", "Żyła", , "[email protected]", role = "aut"),

R/auxiliary.R

@@ -4,30 +4,39 @@
 #' @param date_to date representing the last day of results. Can be NULL.
 #' @param bucket string with name of table
 #' @param timestamp_wrapper string with function to wrap up seconds in database
-#' query. This is a glue::glue() formatted string using the parameter 'seconds'.
+#' query.
+#' This is a [glue::glue_sql()] formatted string using the parameter 'seconds'.
 #'
 #' @return A string with SQL query.
 #'
 #' @noRd
 #' @examples
-#' build_query_sql("table_name")
-#' build_query_sql("table_name", Sys.Date() - 365)
-#' build_query_sql("table_name", date_to = Sys.Date() + 365)
-#' build_query_sql("table_name", Sys.Date() - 365, Sys.Date() + 365)
-#' build_query_sql("table_name", as.Date("2023-04-13"), as.Date("2000-01-01"))
+#' con <- odbc::dbConnect(RSQLite::SQLite(), ":memory:")
+#' build_query_sql("table_name", .con = con)
+#' build_query_sql("table_name", Sys.Date() - 365, .con = con)
+#' build_query_sql("table_name", date_to = Sys.Date() + 365, .con = con)
+#' build_query_sql("table_name", Sys.Date() - 365, Sys.Date() + 365, .con = con)
+#' build_query_sql("table_name", as.Date("2023-04-13"), as.Date("2000-01-01"), .con = con)
 #' build_query_sql(
 #'   "table_name", as.Date("2023-04-13"), as.Date("2000-01-01"),
-#'   timestamp_wrapper = "to_timestamp({seconds})"
+#'   timestamp_wrapper = "to_timestamp({seconds})",
+#'   .con = con
 #' )
 build_query_sql <- function(
-    bucket, date_from = NULL, date_to = NULL, timestamp_wrapper = NULL) {
+    bucket,
+    date_from = NULL,
+    date_to = NULL,
+    timestamp_wrapper = NULL,
+    .con = NULL) {
+  checkmate::assert_string(bucket)
   checkmate::assert_date(date_from, null.ok = TRUE)
   checkmate::assert_date(date_to, null.ok = TRUE)
+  checkmate::assert_string(timestamp_wrapper, null.ok = TRUE)
 
   query <- list(
     .sep = " ",
     "SELECT *",
-    "FROM {bucket}",
+    "FROM {`bucket`}",
     ifelse(!is.null(date_from) || !is.null(date_to), "WHERE", "")
   )
 
@@ -36,17 +45,15 @@ build_query_sql <- function(
     if (is.null(timestamp_wrapper)) {
       return(seconds)
     }
-    return(glue::glue(timestamp_wrapper))
+    # timestamp_wrapper is parsed here with `seconds` as a parameter
+    glue::glue_sql(timestamp_wrapper, .con = .con)
   }
 
   where <- list(.sep = " AND ")
   if (!is.null(date_from)) {
     where <- c(
       where,
-      glue::glue(
-        "time >= ",
-        "{build_timestamp(date_from)}"
-      )
+      glue::glue_sql("time >= ", "{build_timestamp(date_from)}", .con = .con)
     )
   }
 
@@ -55,12 +62,13 @@ build_query_sql <- function(
       lubridate::as_datetime()
     where <- c(
       where,
-      glue::glue("time < {build_timestamp(date_to_aux)}")
+      glue::glue_sql("time < {build_timestamp(date_to_aux)}", .con = .con)
     )
   }
 
-  query <- c(query, do.call(glue::glue, where))
-  trimws(do.call(glue::glue, query))
+  # Separate call to build WHERE clause as it has a different separator
+  query <- c(query, do.call(glue::glue_sql, c(where, .con = .con)))
+  trimws(do.call(glue::glue_sql, c(query, .con = .con)))
 }
 
 #' Process a row's detail (from DB) in JSON format to a data.frame

R/data-storage-sql-family.R

@@ -86,7 +86,7 @@ DataStorageSQLFamily <- R6::R6Class( # nolint object_name_linter
       checkmate::assert_choice(bucket, c(self$event_bucket))
 
       query <- build_query_sql(
-        bucket, date_from, date_to, private$timestamp_wrapper
+        bucket, date_from, date_to, private$timestamp_wrapper, private$db_con
       )
 
       odbc::dbGetQuery(private$db_con, query) %>%

tests/testthat/test-auxiliary_functions.R

@@ -1,37 +1,46 @@
 test_that("Build valid SQL query", {
-  build_query_sql("table_name") %>% expect_equal("SELECT * FROM table_name")
+  con <- odbc::dbConnect(RSQLite::SQLite(), ":memory:")
+
+  build_query_sql("table_name", .con = con) %>%
+    as.character() %>%
+    expect_equal("SELECT * FROM `table_name`")
 
   days_ago <- lubridate::today() - 10
   days_ago_double <- days_ago %>% lubridate::as_datetime() %>% as.double()
 
   days_future <- lubridate::today() + 15
   days_future_double <- (days_future + 1) %>% lubridate::as_datetime() %>% as.double()
 
-  build_query_sql("table_name", days_ago) %>%
+  build_query_sql("table_name", days_ago, .con = con) %>%
+    as.character() %>%
     expect_equal(glue::glue(
-      "SELECT * FROM table_name WHERE time >= {days_ago_double}"
+      "SELECT * FROM `table_name` WHERE time >= {days_ago_double}"
     ))
 
-  build_query_sql("table_name", date_to = days_future) %>%
+  build_query_sql("table_name", date_to = days_future, .con = con) %>%
+    as.character() %>%
     expect_equal(glue::glue(
-      "SELECT * FROM table_name WHERE time < {days_future_double}"
+      "SELECT * FROM `table_name` WHERE time < {days_future_double}"
     ))
 
-  build_query_sql("table_name", days_ago, days_future) %>%
+  build_query_sql("table_name", days_ago, days_future, .con = con) %>%
+    as.character() %>%
     expect_equal(glue::glue(
-      "SELECT * FROM table_name",
+      "SELECT * FROM `table_name`",
       " WHERE time >= {days_ago_double}",
       " AND time < {days_future_double}"
     ))
 
   build_query_sql(
     "table_name",
     as.Date("2023-04-13"),
-    as.Date("2000-01-01")
+    as.Date("2000-01-01"),
+    .con = con
   ) %>%
+    as.character() %>%
     expect_equal(
       glue::glue(
-        "SELECT * FROM table_name",
+        "SELECT * FROM `table_name`",
         " WHERE time >= {lubridate::as_datetime('2023-04-13') %>% as.double()}",
         " AND time < {lubridate::as_datetime('2000-01-02') %>% as.double()}"
       )