fix(security): validate the renv version resolved from the lockfile

NEWS.md

@@ -56,6 +56,25 @@
   executes attacker-controlled commands at `docker build` time.
   This closes the post-#106 audit follow-up for all five sites
   surfaced by Copilot review.
+- Fixed a long-standing code-injection path in `dock_from_renv()`:
+  the `renv` package version resolved from the lockfile
+  (`lock$Packages$renv$Version`) was interpolated raw into the
+  generated `R -e 'remotes::install_version("renv", version = "<x>")'`
+  line without passing through `.validate_renv_version()`. A crafted
+  `renv.lock` could break out of the inner R string and execute
+  arbitrary code as root at `docker build` time. The user-supplied
+  `renv_version=` argument has been validated since the 0.3.0
+  shell-context hardening above, but the lockfile-fallback path was
+  missed; the bug itself predates 0.3.0 (it existed while the
+  vendored `{renv}` parser was in use). The validator is now applied
+  to the resolved value whatever its source. Found by an internal
+  security audit before release.
+- Tightened the `.validate_r_version()` "`X.Y.Z Patched`" branch to
+  require a single literal space (it previously used `\s`, which in R
+  also matches a newline or tab): a lockfile whose `R$Version` carried
+  an embedded newline would pass validation and then emit a two-line
+  `FROM` directive. Not exploitable for command injection, but it
+  could silently break `docker build`.
 
 ## New features
 

R/dock_from_renv.R

@@ -248,7 +248,15 @@ dock_from_renv <- function(
   # get renv version
 
   if (missing(renv_version)) {
+    # The lockfile is untrusted input: its `Packages$renv$Version` is
+    # interpolated into the generated `R -e
+    # 'remotes::install_version(...)'` line, which runs as root at
+    # `docker build` time. Validate it with the same regex applied to a
+    # user-supplied `renv_version` (which is validated at function entry).
     renv_version <- lock$Packages$renv$Version
+    if (!is.null(renv_version)) {
+      .validate_renv_version(renv_version)
+    }
   }
 
   message("renv version = ",

R/utils.R

@@ -153,7 +153,7 @@ cat_info <- function(...) {
   ok <- (
     grepl("^[0-9]+\\.[0-9]+(\\.[0-9]+)?(-[A-Za-z0-9._]+)?$", x) ||
       identical(x, "r-devel") ||
-      grepl("^[0-9]+\\.[0-9]+(\\.[0-9]+)?\\sPatched$", x)
+      grepl("^[0-9]+\\.[0-9]+(\\.[0-9]+)? Patched$", x)
   )
   if (!ok) {
     stop(

tests/testthat/test-dock_from_renv.R

@@ -243,6 +243,18 @@ test_that(".validate_r_version accepts R-devel, release-candidate and Patched lo
     dockerfiler:::.validate_r_version(""),
     "r_version"
   )
+  # The "<X.Y.Z> Patched" branch matches a single literal space only:
+  # a newline or tab between the version and "Patched" would otherwise
+  # slip through (R's `\\s` matches `\n` / `\t`) and produce a
+  # two-line FROM directive in the generated Dockerfile.
+  expect_error(
+    dockerfiler:::.validate_r_version("4.5.0\nPatched"),
+    "r_version"
+  )
+  expect_error(
+    dockerfiler:::.validate_r_version("4.5.0\tPatched"),
+    "r_version"
+  )
 })
 
 test_that(".patch_rprofile_for_ppm matches all three current PPM host shapes", {
@@ -1153,5 +1165,38 @@ test_that("dock_from_renv rejects a lockfile path whose basename contains shell
   )
 })
 
+test_that("dock_from_renv validates the renv version read from the lockfile, not only the user-supplied one", {
+  skip_if(is_rdevel, "skip on R-devel")
+  # `renv_version` is interpolated raw into the generated
+  # `R -e 'remotes::install_version("renv", version = "<x>")'` line, which
+  # runs as root at `docker build` time. `.validate_renv_version()` must be
+  # applied to the value resolved from `lock$Packages$renv$Version` (an
+  # untrusted lockfile a user may have received from a colleague, a vendored
+  # project, or a CI cache), not only to a user-supplied `renv_version=`.
+  # A crafted lockfile carrying `'1.0.0"); system("..."); ("'` would
+  # otherwise break out of the inner R string and execute arbitrary code.
+  lock <- jsonlite::read_json(
+    system.file("renv_with_1.0.0.lock", package = "dockerfiler"),
+    simplifyVector = TRUE,
+    simplifyDataFrame = FALSE,
+    simplifyMatrix = FALSE
+  )
+  lock$Packages$renv$Version <- '1.0.0"); system("touch /tmp/dockerfiler_pwned"); ("'
+  malicious_lf <- file.path(dir_build, "malicious-renv.lock")
+  jsonlite::write_json(lock, path = malicious_lf, auto_unbox = TRUE, pretty = TRUE)
+  on.exit(unlink(malicious_lf), add = TRUE)
+
+  expect_error(
+    dock_from_renv(
+      lockfile = malicious_lf,
+      FROM = "rocker/verse",
+      user = NULL,
+      sysreqs = FALSE
+      # NOTE: no `renv_version =` -> the value comes from the lockfile.
+    ),
+    "renv_version"
+  )
+})
+
 unlink(dir_build, recursive = TRUE)