Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hide token/key from casual inspection #675

Merged
merged 6 commits into from
Feb 17, 2023
Merged

Hide token/key from casual inspection #675

merged 6 commits into from
Feb 17, 2023

Conversation

hadley
Copy link
Member

@hadley hadley commented Feb 16, 2023

This obviously isn't fool proof, but should be enough to avoid accidentally disclosing in a log file.

This obviously isn't fool proof, but should be enough to avoid accidentally disclosing in a log file.
Copy link
Contributor

@aronatkins aronatkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NEWS and accounts adjustment.

@@ -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))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does rsconnect::accounts need the same adjustment?

This adjusts responses from the package API and should be included in NEWS.md.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

accounts() just includes name and server — did you mean servers()?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably, yes!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that the certificates field in servers() is actually secret, but it is very long, so redacting it is a service to the user.

@hadley
Copy link
Member Author

hadley commented Feb 17, 2023

@toph-allen stringsAsFactors strikes yet again.

@hadley hadley merged commit 50c33f7 into main Feb 17, 2023
@hadley hadley deleted the hide-secrets branch February 17, 2023 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants