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

Add security assessment via {oysteR} #272

Open
wants to merge 38 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
735029e
initial scaffold of oysteR integration
borgmaan Feb 2, 2023
b8950b2
update pkg_ref_cache_security to just follow simple default
borgmaan Feb 3, 2023
1816f07
extend scan to include dependencies
borgmaan Feb 3, 2023
d8bb71f
update docs
borgmaan Feb 3, 2023
ba48a9a
clean up r cmd check warnings
borgmaan Feb 3, 2023
20ce000
initial approach for excluding assessments with Suggests deps in all_…
borgmaan Feb 3, 2023
4bb0510
flip score to match others
borgmaan Feb 3, 2023
be63283
update docs
borgmaan Feb 3, 2023
1b1eebe
typo
borgmaan Feb 3, 2023
2625d0f
manually write docs to avoid issues with roxygen_assess_family
borgmaan Feb 3, 2023
1a7e7da
logic to check oysteR version
borgmaan Feb 21, 2023
c81c54a
only assess specific package security. not dependencies.
borgmaan Feb 21, 2023
0eeafc4
mechanism to auto-include assessment if Suggests package is installed
borgmaan Feb 21, 2023
07478e7
add package-level option to muffle install prompts
borgmaan Feb 22, 2023
e18db11
validate oyster install at the ref cache layer as well
borgmaan Mar 28, 2023
45429cd
explicitly cast package version for oysteR input
borgmaan Mar 28, 2023
ebbecca
generalize install checks; begin API tests for suggests
borgmaan Apr 6, 2023
e9d5e25
clean up generalized install helpers
borgmaan Apr 6, 2023
b49af45
attribute and option tests
borgmaan Apr 6, 2023
5c57b12
file move
borgmaan Apr 6, 2023
cac9499
clean up tests
borgmaan Apr 6, 2023
2037deb
fix messaging on install helpers
borgmaan Apr 6, 2023
82e9090
update suggests assessment tests
borgmaan May 4, 2023
6b73d22
scaffold other tests
borgmaan May 4, 2023
14e6a50
add mock for oysteR
borgmaan May 4, 2023
3f5f9b8
notes
borgmaan Jun 12, 2023
9efd972
Merge branch 'master' into dev/security
borgmaan Jun 14, 2023
94e9fa0
test cases for assess_security
borgmaan Jun 14, 2023
4c23d2c
Merge branch 'dev/security' of github.com:pharmaR/riskmetric into dev…
borgmaan Jun 14, 2023
19ae89a
fixtest assess
borgmaan Jun 14, 2023
297a17e
make sure security is available
borgmaan Jun 14, 2023
66d73d1
match length
borgmaan Jun 14, 2023
96a14c6
more missed security assessments in tes
borgmaan Jun 14, 2023
1ae59ef
re-build docs
borgmaan Jun 14, 2023
cac8ce0
imports to suppress warnings
borgmaan Jun 14, 2023
4c7a013
try to fix dowarnings on rcmdcheck
borgmaan Jun 14, 2023
8e98a05
resolve test errors specific to covr::code_coverage
borgmaan Jun 14, 2023
8f13cf1
update DESCRIPTION
borgmaan Jun 14, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Type: Package
Title: Risk Metrics to Evaluating R Packages
Description: Facilities for assessing R packages against a number of metrics to
help quantify their robustness.
Version: 0.2.2
Version: 0.2.2.9000
Authors@R: c(
person("R Validation Hub", role = c("aut"), email = "[email protected]"),
person("Doug", "Kelkhoff", role = c("aut"), email = "[email protected]"),
Expand All @@ -14,6 +14,7 @@ Authors@R: c(
person("Eric", "Milliman", role = c("aut")),
person("Juliane", "Manitz", role = c("aut")),
person("Mark", "Padgham", role = c("ctb")),
person("Andrew", "Borgman", role = c("aut")),
person("PSI special interest group Application and Implementation of Methodologies in Statistics", role = c("cph")))
URL: https://pharmar.github.io/riskmetric/, https://github.com/pharmaR/riskmetric
BugReports: https://github.com/pharmaR/riskmetric/issues
Expand Down Expand Up @@ -44,7 +45,8 @@ Suggests:
dplyr,
testthat,
webmockr,
jsonlite
jsonlite,
oysteR (>= 0.1.0)
RoxygenNote: 7.2.3
VignetteBuilder: knitr
Config/testthat/edition: 3
6 changes: 6 additions & 0 deletions NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ S3method(assess_remote_checks,default)
S3method(assess_remote_checks,pkg_bioc_remote)
S3method(assess_remote_checks,pkg_cran_remote)
S3method(assess_reverse_dependencies,default)
S3method(assess_security,default)
S3method(assess_size_codebase,default)
S3method(assess_size_codebase,pkg_install)
S3method(assess_size_codebase,pkg_source)
Expand All @@ -68,6 +69,7 @@ S3method(metric_score,pkg_metric_news_current)
S3method(metric_score,pkg_metric_r_cmd_check)
S3method(metric_score,pkg_metric_remote_checks)
S3method(metric_score,pkg_metric_reverse_dependencies)
S3method(metric_score,pkg_metric_security)
S3method(metric_score,pkg_metric_size_codebase)
S3method(names,pkg_ref)
S3method(pillar_shaft,list_of_pkg_metric)
Expand Down Expand Up @@ -108,6 +110,7 @@ export(assess_news_current)
export(assess_r_cmd_check)
export(assess_remote_checks)
export(assess_reverse_dependencies)
export(assess_security)
export(assess_size_codebase)
export(assessment_error_as_warning)
export(assessment_error_empty)
Expand Down Expand Up @@ -140,6 +143,7 @@ importFrom(pillar,new_pillar_shaft_simple)
importFrom(pillar,pillar_shaft)
importFrom(pillar,style_na)
importFrom(pkgload,parse_ns_file)
importFrom(stats,setNames)
importFrom(tibble,as_tibble)
importFrom(tibble,tibble)
importFrom(tools,Rd_db)
Expand All @@ -154,6 +158,8 @@ importFrom(utils,available.packages)
importFrom(utils,capture.output)
importFrom(utils,getS3method)
importFrom(utils,head)
importFrom(utils,install.packages)
importFrom(utils,menu)
importFrom(utils,packageDescription)
importFrom(utils,packageName)
importFrom(utils,packageVersion)
Expand Down
55 changes: 55 additions & 0 deletions R/assess_security.R
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the assessment should be the tibble or possibly the list of vulnerabilities found. and then metric can be binary or presence/absence of vulnerabilities.

Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
#' Assess a package for known security vulnerabilities in the OSS Index
#'
#' @param x a \code{pkg_ref} package reference object
#' @param ... additional arguments passed on to S3 methods, rarely used
#' @return a \code{pkg_metric} containing Assess for any known security vulnerabilities in the OSS Index via oysteR
#' @seealso \code{\link{metric_score.pkg_metric_security}}
#'
#' @importFrom utils install.packages menu
#' @export
assess_security <- function(x, ...) {

# checks whether a compatible version of oysteR is installed. prompts user to
# install or upgrade if needed when running interactively. an error is thrown
# in the event a valid oysteR package is not installed.
cf <- deparse(match.call()[[1]])
validate_suggests_install(pkg_name = "oysteR", calling_fn = cf)

UseMethod("assess_security")
}

attributes(assess_security)$column_name <- "security"
attributes(assess_security)$label <-
"number of vulnerabilities detected in OSS Index"

# set as a "Suggests" package to be excluded from all_assessments if the package
# is not installed
attributes(assess_security)$suggests <- !requireNamespace("oysteR", quietly = TRUE)
attributes(assess_security)$suggests_pkg <- "oysteR"


#' @export
assess_security.default <- function(x, ...) {
pkg_metric_eval(class = "pkg_metric_security", {
x$security
})
}


#' Score a package for known security vulnerabilities in the OSS Index
#'
#' Coerce the count of reported vulnerabilities to a binary indicator.
#'
#' @eval roxygen_score_family("security", dontrun = TRUE)
#' @return \code{NA} if no vulnerabilities are found, otherwise \code{0}
#'
#' @export
metric_score.pkg_metric_security <- function(x, ...) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

if assess_security == 0 wouldn't that mean no vulnerabilities were found? and thus the metric should be 1? but I also seem to remember discussing what 0 vulnerabilities found might mean and that just because none were found doesn't mean non-exist... I think I am back tracking on that line of thought and metric should either be 1 or 0, and we reserve NA for cases when the assessment cannot be performed (for some reason).

Copy link
Collaborator

Choose a reason for hiding this comment

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

If we can capture the vulnerability overivew then we could set metric to NA if the package is not found in the Sonatype database.

if (x < 1) return(NA)
else return(0)
}
attributes(metric_score.pkg_metric_security)$label <-
"A binary indicator of whether a package has OSS Index listed vulnerabilities."



3 changes: 2 additions & 1 deletion R/options.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
riskmetric.options <- list(
gitlab_api_host = "https://gitlab.com/api/v4",
github_api_host = "https://api.github.com"
github_api_host = "https://api.github.com",
skip_oysteR_install = FALSE
)
21 changes: 18 additions & 3 deletions R/pkg_assess.R
Original file line number Diff line number Diff line change
Expand Up @@ -60,19 +60,34 @@ roxygen_assess_family_catalog <- function() {
"}")
}


#' Helper to determine if assessment depends on package in Suggests
#'
#' @param assess_obj An assessment function object
#' @returns \code{TRUE} if the object does not have an attribute named
#' \code{"suggests"} and the negated attribute value if present
not_suggests <- function(assess_obj) {
sg <- attr(assess_obj, "suggests")
ifelse(is.null(sg), yes = TRUE, no = !sg)
}

#' A default list of assessments to perform for each package
#'
#' @param include_suggests Logical indicating if assessments requiring
#' dependency packages listed in \code{Suggests} be included in the list of
#' assessments?
#' @return a list of assess_* functions exported from riskmetric
#'
#' @importFrom utils packageName
#' @export
all_assessments <- function() {
all_assessments <- function(include_suggests = FALSE) {
fs <- grep("^assess_[^.]*$",
getNamespaceExports(utils::packageName()),
value = TRUE)
Map(getExportedValue, fs, ns = list(utils::packageName()))
fn_objs <- Map(getExportedValue, fs, ns = list(utils::packageName()))
if (!include_suggests) {
fn_objs <- Filter(not_suggests, fn_objs)
}
fn_objs
}

#' Get a specific set of assess_* functions for pkg_assess
Expand Down
2 changes: 1 addition & 1 deletion R/pkg_ref_cache.R
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ available_pkg_ref_fields <- function(x) {
#' @family package reference cache
#'
#' @rdname riskmetric_metadata_caching
#' @keyworks internal
#' @keywords internal
#' @noRd
pkg_ref_cache <- function(x, name, ..., .class = as.character(name)) {
UseMethod("pkg_ref_cache", structure(list(), class = .class))
Expand Down
32 changes: 32 additions & 0 deletions R/pkg_ref_cache_security.R
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think for the ref_cache we should capture the oysteR output, if ony the tibble to start, but the entire message would be best (especially the overview).

oysteR::audit("dplyr", version = "1.0.8", type = "cran")
ℹ Using cached results for 1 package

── Vulnerability overview ──

ℹ 1 package was scanned
ℹ 1 package was found in the Sonatype database
ℹ 0 packages had known vulnerabilities
ℹ A total of 0 known vulnerabilities were identified
ℹ See https://github.com/sonatype-nexus-community/oysteR/ for details.
# A tibble: 1 × 8
package version type oss_package description reference vulnerabilities no_of_vulnerabi…

1 dplyr 1.0.8 cran pkg:cran/dplyr@… "dplyr: A… https://… <list [0]> 0

Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#' Cache OSS Index results
#'
#' @inheritParams pkg_ref_cache
#' @family package reference cache
#' @return a \code{pkg_ref} object
#' @keywords internal
#' @noRd
pkg_ref_cache.security <- function(x, ...) {
validate_suggests_install(
pkg_name = "oysteR",
calling_fn = deparse(match.call()[[1]])
)
UseMethod("pkg_ref_cache.security")
}

#' Check OSS Index lists any vulnerabilities for the package
#'
#' @inheritParams pkg_ref_cache
#' @family package reference cache
#' @return a \code{pkg_ref} object
#' @keywords internal
#' @noRd
pkg_ref_cache.security.default <- function(x, ...) {
scan_results <- oysteR::audit(
pkg = x$name,
version = as.character(x$version),
type = "cran",
verbose = FALSE
)

return(sum(scan_results[["no_of_vulnerabilities"]]))
}
41 changes: 41 additions & 0 deletions R/utils_suggests.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
#' Helper function to prompt to install a package if it is not present
#'
#' @keywords internal
#'
validate_suggests_install <- function(pkg_name, calling_fn) {
# check if package is installed
if (!requireNamespace(pkg_name, quietly = TRUE)) {
# if not, prompt for install
suggests_install_helper(pkg_name, calling_fn)
}
}

#' Helper function to guide user through install of a package
#'
#' @importFrom stats setNames
#' @keywords internal
#'
suggests_install_helper <- function(pkg_name, calling_fn) {
op_nm <- sprintf("riskmetric.skip_%s_install", pkg_name)
if (interactive() & !getOption(op_nm)) {
inst_yn <- utils::menu(
choices = c("Yes", "No"),
title = sprintf(
"Running %s requires installation of the of the %s package.
Would you like to install this now?",
calling_fn, pkg_name
)
)

if (inst_yn == "1") {
utils::install.packages(pkg_name)
} else {
# if user skipped once, don't prompt again until session restarts
if (inst_yn == "2") do.call(options, as.list(setNames(TRUE, op_nm)))
stop(sprintf(
"%s not run. Please install the %s package if you wish to proceed.",
calling_fn, pkg_name
))
}
}
}
7 changes: 6 additions & 1 deletion man/all_assessments.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 22 additions & 0 deletions man/assess_security.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion man/firstS3method.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 23 additions & 0 deletions man/metric_score.pkg_metric_security.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 18 additions & 0 deletions man/not_suggests.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions man/pkg_assess.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions man/suggests_install_helper.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions man/validate_suggests_install.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading