diff --git a/NAMESPACE b/NAMESPACE index b1937604..980f9bef 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -1,5 +1,8 @@ # Generated by roxygen2: do not edit by hand +S3method(format,rsconnect_secret) +S3method(print,rsconnect_secret) +S3method(str,rsconnect_secret) export(accountInfo) export(accountUsage) export(accounts) diff --git a/NEWS.md b/NEWS.md index ed2b8acf..65c57a38 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,9 @@ # rsconnect 0.8.30 (development version) +* `accountInfo()` and `servers()` now redacts sensitive information (secrets, + private keys, and certificates) to make it hard to accidentally reveal + such information in logs (#675). + * The logic used by `deployApp()` for determining whether you publish a new update or update an existing app has been simplified. Now `appName`, `account`, and `server` are used to find existing deployments. If none diff --git a/R/accounts.R b/R/accounts.R index 638a84ce..e2eec569 100644 --- a/R/accounts.R +++ b/R/accounts.R @@ -272,23 +272,45 @@ setAccountInfo <- function(name, token, secret, if (is.null(accountId)) stop("Unable to determine account id for account named '", name, "'") + registerCloudTokenSecret( + serverName = serverInfo$name, + accountName = name, + userId = userId, + token = token, + secret = secret, + ) + invisible() +} + +registerCloudTokenSecret <- function(serverName, + accountName, + userId, + accountId, + token, + secret) { # get the path to the config file - configFile <- accountConfigFile(name, serverInfo$name) - dir.create(dirname(configFile), recursive = TRUE, showWarnings = FALSE) + path <- accountConfigFile(accountName, serverName) + dir.create(dirname(path), recursive = TRUE, showWarnings = FALSE) # write the user info - write.dcf(list(name = name, - userId = userId, - accountId = accountId, - token = token, - secret = secret, - server = serverInfo$name), - configFile, - width = 100) + write.dcf( + list( + name = accountName, + userId = userId, + accountId = accountName, + token = token, + secret = secret, + server = serverName + ), + path, + width = 100 + ) # set restrictive permissions on it if possible if (identical(.Platform$OS.type, "unix")) - Sys.chmod(configFile, mode = "0600") + Sys.chmod(path, mode = "0600") + + path } #' @rdname accounts @@ -303,8 +325,13 @@ accountInfo <- function(name = NULL, server = NULL) { info <- as.list(accountDcf) # remove all whitespace from private key if (!is.null(info$private_key)) { - info$private_key <- gsub("[[:space:]]", "", info$private_key) + info$private_key <- secret(gsub("[[:space:]]", "", info$private_key)) } + + if (!is.null(info$secret)) { + info$secret <- secret(info$secret) + } + info } @@ -487,3 +514,25 @@ accountInfoFromHostUrl <- function(hostUrl) { return(accountInfo(name = as.character(account[1, "name"]), server = server)) } + + +secret <- function(x) { + stopifnot(is.character(x)) + structure(x, class = "rsconnect_secret") +} + +#' @export +format.rsconnect_secret <- function(x, ...) { + paste0(substr(x, 1, 6), "... (redacted)") +} + +#' @export +print.rsconnect_secret <- function(x, ...) { + print(format(x)) + invisible(x) +} + +#' @export +str.rsconnect_secret <- function(object, ...) { + cat(" ", format(object), "\n", sep = "") +} diff --git a/R/servers.R b/R/servers.R index b900dbfe..b5213999 100644 --- a/R/servers.R +++ b/R/servers.R @@ -59,10 +59,11 @@ servers <- function(local = FALSE) { info }) locals <- do.call(rbind, parsed) + locals <- as.data.frame(locals, stringsAsFactors = FALSE) if (local) { - locals + out <- locals } else { - serversList <- rbind( + out <- rbind( locals, as.data.frame(shinyappsServerInfo(), stringsAsFactors = FALSE), as.data.frame(cloudServerInfo(), stringsAsFactors = FALSE)) @@ -75,8 +76,9 @@ servers <- function(local = FALSE) { as.data.frame(cloudServerInfo("rstudio.cloud"), stringsAsFactors = FALSE) ) } - serversList } + out$certificate <- secret(out$certificate) + out } serverConfigDir <- function() { diff --git a/tests/testthat/_snaps/accounts.md b/tests/testthat/_snaps/accounts.md new file mode 100644 index 00000000..e2ec8ae1 --- /dev/null +++ b/tests/testthat/_snaps/accounts.md @@ -0,0 +1,20 @@ +# secrets are hidden from casual inspection + + Code + accountInfo("john")$private_key + Output + [1] "THISIS... (redacted)" + Code + accountInfo("susan")$secret + Output + [1] "THIS I... (redacted)" + Code + str(accountInfo("john")) + Output + List of 5 + $ username : chr "john" + $ accountId : chr "userId" + $ token : chr "token" + $ server : chr "example.com" + $ private_key: THISIS... (redacted) + diff --git a/tests/testthat/_snaps/servers.md b/tests/testthat/_snaps/servers.md new file mode 100644 index 00000000..7a3aa707 --- /dev/null +++ b/tests/testthat/_snaps/servers.md @@ -0,0 +1,10 @@ +# servers() redacts the certificate + + Code + servers() + Output + name url certificate + 1 cert_test_a https://localhost:4567/ -----B... (redacted) + 2 shinyapps.io https://api.shinyapps.io/v1 Amazon... (redacted) + 3 posit.cloud https://api.shinyapps.io/v1 Amazon... (redacted) + diff --git a/tests/testthat/test-accounts.R b/tests/testthat/test-accounts.R index ebf46a4b..ea2d6228 100644 --- a/tests/testthat/test-accounts.R +++ b/tests/testthat/test-accounts.R @@ -59,3 +59,24 @@ test_that("All hosted product names are identified as cloud", { expect_true(isCloudServer("posit.cloud")) expect_false(isCloudServer("connect.internal")) }) + +test_that("secrets are hidden from casual inspection", { + local_temp_config() + registerUserToken("example.com", "john", "userId", "token", "THIS IS A SECRET") + registerCloudTokenSecret( + "shinyapps.io", + "susan", + "userId", + "accountId", + "token", + "THIS IS A SECRET" + ) + + expect_snapshot({ + accountInfo("john")$private_key + accountInfo("susan")$secret + + str(accountInfo("john")) + }) + +}) diff --git a/tests/testthat/test-servers.R b/tests/testthat/test-servers.R new file mode 100644 index 00000000..fa762883 --- /dev/null +++ b/tests/testthat/test-servers.R @@ -0,0 +1,12 @@ +test_that("servers() redacts the certificate", { + local_temp_config() + + # add a server with a sample certificate + addServer( + url = "https://localhost:4567/", + name = "cert_test_a", + certificate = test_path("certs/sample.crt"), + quiet = TRUE + ) + expect_snapshot(servers()) +})