From 6d4ae9db9a78c05d09acbd1609ddf3d07324e41e Mon Sep 17 00:00:00 2001 From: Garrick Aden-Buie Date: Fri, 8 Sep 2023 09:51:16 -0400 Subject: [PATCH 01/10] fix(deployApp): Don't reveal envvar value in logs --- R/deployApp.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/deployApp.R b/R/deployApp.R index b8ad2517..6c227428 100644 --- a/R/deployApp.R +++ b/R/deployApp.R @@ -432,7 +432,7 @@ deployApp <- function(appDir = getwd(), taskComplete(quiet, "Visibility updated") } if (length(target$envVars) > 0) { - taskStart(quiet, "Updating environment variables {envVars}...") + taskStart(quiet, "Updating {length(envVars)} environment variable{?s}: {.field {names(envVars)}}...") client$setEnvVars(application$guid, target$envVars) taskComplete(quiet, "Environment variables updated") } From 523f1e518b640b2cca03a64a01d43fdd6fd8c437 Mon Sep 17 00:00:00 2001 From: Garrick Aden-Buie Date: Fri, 8 Sep 2023 09:54:23 -0400 Subject: [PATCH 02/10] fix(deployApp): Fixes an off-by-one error when checking `envVars` --- R/deployApp.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/deployApp.R b/R/deployApp.R index 6c227428..aff66e8c 100644 --- a/R/deployApp.R +++ b/R/deployApp.R @@ -360,7 +360,7 @@ deployApp <- function(appDir = getwd(), stop("Posit Connect does not support deploying without uploading. ", "Specify upload=TRUE to upload and re-deploy your application.") } - if (!isConnectServer(target$server) && length(envVars) > 1) { + if (!isConnectServer(target$server) && length(envVars) > 0) { cli::cli_abort("{.arg envVars} only supported for Posit Connect servers") } From f3dac5b38fdf05defe8b65bfd2c2d58eaa488f9b Mon Sep 17 00:00:00 2001 From: Garrick Aden-Buie Date: Fri, 8 Sep 2023 13:38:48 -0400 Subject: [PATCH 03/10] chore: Check `client` directly to know if it supports `setEnvVars` --- R/deployApp.R | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/R/deployApp.R b/R/deployApp.R index aff66e8c..4d979379 100644 --- a/R/deployApp.R +++ b/R/deployApp.R @@ -360,12 +360,14 @@ deployApp <- function(appDir = getwd(), stop("Posit Connect does not support deploying without uploading. ", "Specify upload=TRUE to upload and re-deploy your application.") } - if (!isConnectServer(target$server) && length(envVars) > 0) { - cli::cli_abort("{.arg envVars} only supported for Posit Connect servers") - } accountDetails <- accountInfo(target$account, target$server) client <- clientForAccount(accountDetails) + + if (length(envVars) > 0 && !"setEnvVars" %in% names(client)) { + cli::cli_abort("{target$server} does not support setting {.arg envVars}") + } + if (verbose) { showCookies(serverInfo(accountDetails$server)$url) } From 28531f1e34dcc5f65eaa60ecd3985965b80d87c1 Mon Sep 17 00:00:00 2001 From: Garrick Aden-Buie Date: Sat, 9 Sep 2023 07:30:58 -0400 Subject: [PATCH 04/10] Revert "fix(deployApp): Don't reveal envvar value in logs" This reverts commit 6d4ae9db9a78c05d09acbd1609ddf3d07324e41e. --- R/deployApp.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/deployApp.R b/R/deployApp.R index 4d979379..c4438d99 100644 --- a/R/deployApp.R +++ b/R/deployApp.R @@ -434,7 +434,7 @@ deployApp <- function(appDir = getwd(), taskComplete(quiet, "Visibility updated") } if (length(target$envVars) > 0) { - taskStart(quiet, "Updating {length(envVars)} environment variable{?s}: {.field {names(envVars)}}...") + taskStart(quiet, "Updating environment variables {envVars}...") client$setEnvVars(application$guid, target$envVars) taskComplete(quiet, "Environment variables updated") } From 0c9bfbfc4429931df6f129c5d25f7cd760956574 Mon Sep 17 00:00:00 2001 From: Garrick Aden-Buie Date: Sat, 9 Sep 2023 07:32:45 -0400 Subject: [PATCH 05/10] docs(deployApp): Add advice for setting env vars --- R/deployApp.R | 7 ++++--- man/deployApp.Rd | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/R/deployApp.R b/R/deployApp.R index c4438d99..12504248 100644 --- a/R/deployApp.R +++ b/R/deployApp.R @@ -50,9 +50,10 @@ #' on second and subsequent deploys, the title will be unchanged. #' @param envVars A character vector giving the names of environment variables #' whose values should be synchronised with the server (currently supported by -#' Connect only). The values of the environment variables are sent over an -#' encrypted connection and are not stored in the bundle, making this a safe -#' way to send private data to Connect. +#' Connect only). The values of the environment variables should be set in the +#' current session with [Sys.setenv()] or via an `.Renviron` file. Values are +#' sent over an encrypted connection and are not stored in the bundle, making +#' this a safe way to send private data to Connect. #' #' The names (not values) are stored in the deployment record so that future #' deployments will automatically update their values. Other environment diff --git a/man/deployApp.Rd b/man/deployApp.Rd index abd104fd..5acc0658 100644 --- a/man/deployApp.Rd +++ b/man/deployApp.Rd @@ -69,9 +69,10 @@ on second and subsequent deploys, the title will be unchanged.} \item{envVars}{A character vector giving the names of environment variables whose values should be synchronised with the server (currently supported by -Connect only). The values of the environment variables are sent over an -encrypted connection and are not stored in the bundle, making this a safe -way to send private data to Connect. +Connect only). The values of the environment variables should be set in the +current session with \code{\link[=Sys.setenv]{Sys.setenv()}} or via an \code{.Renviron} file. Values are +sent over an encrypted connection and are not stored in the bundle, making +this a safe way to send private data to Connect. The names (not values) are stored in the deployment record so that future deployments will automatically update their values. Other environment From 8a74325d1f6984a7321669b2ec9b2596e70fc6ec Mon Sep 17 00:00:00 2001 From: Garrick Aden-Buie Date: Sat, 9 Sep 2023 07:50:37 -0400 Subject: [PATCH 06/10] feat(deployApp): Throw if `envVars` is a named vector --- R/deployApp.R | 9 +++++++++ tests/testthat/_snaps/deployApp.md | 10 ++++++++++ tests/testthat/test-deployApp.R | 8 ++++++++ 3 files changed, 27 insertions(+) diff --git a/R/deployApp.R b/R/deployApp.R index 12504248..4511d75b 100644 --- a/R/deployApp.R +++ b/R/deployApp.R @@ -259,6 +259,15 @@ deployApp <- function(appDir = getwd(), } } + check_character(envVars, allow_null = TRUE) + if (!is.null(envVars) && any(nzchar(names2(envVars)))) { + cli::cli_abort(c( + "{.arg envVars} must be a character vector containing only environment variable {.strong names}.", + "i" = "Set environment variables with `Sys.setenv() or an `.Renviron` file.", + "i" = "Use `unname()`` to remove the names from the vector passed to {.arg envVars}." + )) + } + if (!is.null(appSourceDoc)) { # Used by IDE so can't deprecate recordDir <- appSourceDoc diff --git a/tests/testthat/_snaps/deployApp.md b/tests/testthat/_snaps/deployApp.md index 9f0b1a3c..343705fc 100644 --- a/tests/testthat/_snaps/deployApp.md +++ b/tests/testthat/_snaps/deployApp.md @@ -79,3 +79,13 @@ 2: Delete existing deployment & create a new app Selection: 2 +# deployApp() errors if envVars is given a named vector + + Code + deployApp(local_temp_app(), envVars = c(FLAG = "true")) + Condition + Error in `deployApp()`: + ! `envVars` must be a character vector containing only environment variable names. + i Set environment variables with `Sys.setenv() or an `.Renviron` file. + i Use `unname()`` to remove the names from the vector passed to `envVars`. + diff --git a/tests/testthat/test-deployApp.R b/tests/testthat/test-deployApp.R index 187046af..68956703 100644 --- a/tests/testthat/test-deployApp.R +++ b/tests/testthat/test-deployApp.R @@ -100,3 +100,11 @@ test_that("applicationDeleted() errors or prompts as needed", { expect_snapshot(. <- applicationDeleted(client, target, app)) expect_length(dir(app, recursive = TRUE), 0) }) + +# envvars ----------------------------------------------------------------- + +test_that("deployApp() errors if envVars is given a named vector", { + expect_snapshot(error = TRUE, { + deployApp(local_temp_app(), envVars = c("FLAG" = "true")) + }) +}) From 4a883300366a99c0fdcb38933aba7defe3137b3b Mon Sep 17 00:00:00 2001 From: Garrick Aden-Buie Date: Sat, 9 Sep 2023 14:03:42 -0400 Subject: [PATCH 07/10] Update deployApp.R Co-authored-by: Hadley Wickham --- R/deployApp.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/deployApp.R b/R/deployApp.R index 4511d75b..5b9e964f 100644 --- a/R/deployApp.R +++ b/R/deployApp.R @@ -260,7 +260,7 @@ deployApp <- function(appDir = getwd(), } check_character(envVars, allow_null = TRUE) - if (!is.null(envVars) && any(nzchar(names2(envVars)))) { + if (!is.null(envVars) && !is.null(names(envVars))) { cli::cli_abort(c( "{.arg envVars} must be a character vector containing only environment variable {.strong names}.", "i" = "Set environment variables with `Sys.setenv() or an `.Renviron` file.", From dfe66d51dc2639efe0480d1cb2153eae73d98770 Mon Sep 17 00:00:00 2001 From: Garrick Aden-Buie Date: Sat, 9 Sep 2023 14:04:22 -0400 Subject: [PATCH 08/10] Update deployApp.R Co-authored-by: Hadley Wickham --- R/deployApp.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/deployApp.R b/R/deployApp.R index 5b9e964f..a8cc3bfe 100644 --- a/R/deployApp.R +++ b/R/deployApp.R @@ -264,7 +264,7 @@ deployApp <- function(appDir = getwd(), cli::cli_abort(c( "{.arg envVars} must be a character vector containing only environment variable {.strong names}.", "i" = "Set environment variables with `Sys.setenv() or an `.Renviron` file.", - "i" = "Use `unname()`` to remove the names from the vector passed to {.arg envVars}." + "i" = "Use {.fn unname} to remove the names from the vector passed to {.arg envVars}." )) } From f1227e4ddc560cb872c267832a4fdcefaa52cc08 Mon Sep 17 00:00:00 2001 From: Garrick Aden-Buie Date: Sat, 9 Sep 2023 14:16:16 -0400 Subject: [PATCH 09/10] docs(deployApp): Recommend Renviron or {keyring} for `envVars` --- R/deployApp.R | 10 ++++++---- man/deployApp.Rd | 10 ++++++---- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/R/deployApp.R b/R/deployApp.R index a8cc3bfe..d7b0edb4 100644 --- a/R/deployApp.R +++ b/R/deployApp.R @@ -50,10 +50,12 @@ #' on second and subsequent deploys, the title will be unchanged. #' @param envVars A character vector giving the names of environment variables #' whose values should be synchronised with the server (currently supported by -#' Connect only). The values of the environment variables should be set in the -#' current session with [Sys.setenv()] or via an `.Renviron` file. Values are -#' sent over an encrypted connection and are not stored in the bundle, making -#' this a safe way to send private data to Connect. +#' Connect only). The values of sensitive environment variables should be set +#' in the current session via an `.Renviron` file or with the help of a +#' credential store like +#' [keyring](https://r-lib.github.io/keyring/index.html). Values are sent over +#' an encrypted connection and are not stored in the bundle, making this a +#' safe way to send private data to Connect. #' #' The names (not values) are stored in the deployment record so that future #' deployments will automatically update their values. Other environment diff --git a/man/deployApp.Rd b/man/deployApp.Rd index 5acc0658..899a8993 100644 --- a/man/deployApp.Rd +++ b/man/deployApp.Rd @@ -69,10 +69,12 @@ on second and subsequent deploys, the title will be unchanged.} \item{envVars}{A character vector giving the names of environment variables whose values should be synchronised with the server (currently supported by -Connect only). The values of the environment variables should be set in the -current session with \code{\link[=Sys.setenv]{Sys.setenv()}} or via an \code{.Renviron} file. Values are -sent over an encrypted connection and are not stored in the bundle, making -this a safe way to send private data to Connect. +Connect only). The values of sensitive environment variables should be set +in the current session via an \code{.Renviron} file or with the help of a +credential store like +\href{https://r-lib.github.io/keyring/index.html}{keyring}. Values are sent over +an encrypted connection and are not stored in the bundle, making this a +safe way to send private data to Connect. The names (not values) are stored in the deployment record so that future deployments will automatically update their values. Other environment From dd1233783c703cd06fc2338aab45d75ed85ba5bf Mon Sep 17 00:00:00 2001 From: Garrick Aden-Buie Date: Sat, 9 Sep 2023 15:38:08 -0400 Subject: [PATCH 10/10] tests: Update snapshots --- tests/testthat/_snaps/deployApp.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testthat/_snaps/deployApp.md b/tests/testthat/_snaps/deployApp.md index 343705fc..4c69da93 100644 --- a/tests/testthat/_snaps/deployApp.md +++ b/tests/testthat/_snaps/deployApp.md @@ -87,5 +87,5 @@ Error in `deployApp()`: ! `envVars` must be a character vector containing only environment variable names. i Set environment variables with `Sys.setenv() or an `.Renviron` file. - i Use `unname()`` to remove the names from the vector passed to `envVars`. + i Use `unname()` to remove the names from the vector passed to `envVars`.