aboutsummaryrefslogtreecommitdiffhomepage
path: root/crates/shirabe
diff options
context:
space:
mode:
authornsfisis <nsfisis@gmail.com>2026-06-29 02:16:43 +0900
committernsfisis <nsfisis@gmail.com>2026-06-29 02:17:57 +0900
commit3b28fc65f5de7c6ad69bee48b2d1f52dd9b30972 (patch)
treedd2567446a1fe6e7245fd35190dc039bb46cc86b /crates/shirabe
parent1036b7e33a4360df8b99f81ef03492fee8328bd0 (diff)
downloadphp-shirabe-3b28fc65f5de7c6ad69bee48b2d1f52dd9b30972.tar.gz
php-shirabe-3b28fc65f5de7c6ad69bee48b2d1f52dd9b30972.tar.zst
php-shirabe-3b28fc65f5de7c6ad69bee48b2d1f52dd9b30972.zip
fix(advisory): pass IO as shared handle to Auditor::audit
Auditor::audit took io as &mut dyn IOInterface, forcing the audit and installer post-audit call sites to hold a borrow_mut() on the shared IO RefCell for the whole call. During advisory fetching the repositories write to their own clones of the same handle, so the borrow_mut() collided with their borrow() and panicked with 'RefCell already mutably borrowed' on 'audit --locked'. Take the Rc<RefCell<dyn IOInterface>> handle instead so writes borrow briefly and never overlap. Un-ignore the locked-audit regression test that this unblocks. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Diffstat (limited to 'crates/shirabe')
-rw-r--r--crates/shirabe/src/advisory/auditor.rs17
-rw-r--r--crates/shirabe/src/command/audit_command.rs2
-rw-r--r--crates/shirabe/src/command/diagnose_command.rs12
-rw-r--r--crates/shirabe/src/installer.rs2
-rw-r--r--crates/shirabe/tests/advisory/auditor_test.rs122
-rw-r--r--crates/shirabe/tests/command/audit_command_test.rs1
6 files changed, 95 insertions, 61 deletions
diff --git a/crates/shirabe/src/advisory/auditor.rs b/crates/shirabe/src/advisory/auditor.rs
index 662a940..402483c 100644
--- a/crates/shirabe/src/advisory/auditor.rs
+++ b/crates/shirabe/src/advisory/auditor.rs
@@ -4,6 +4,7 @@ use crate::advisory::AnySecurityAdvisory;
use crate::advisory::SecurityAdvisory;
use crate::io::ConsoleIO;
use crate::io::IOInterface;
+use crate::io::IOInterfaceImmutable;
use crate::json::JsonFile;
use crate::package::CompletePackageInterfaceHandle;
use crate::package::PackageInterfaceHandle;
@@ -17,6 +18,8 @@ use shirabe_php_shim::{
DATE_ATOM, InvalidArgumentException, PhpMixed, array_all, array_any, array_key_exists,
array_keys, array_reduce, get_class, sprintf, str_starts_with,
};
+use std::cell::RefCell;
+use std::rc::Rc;
/// Shape of the `--format=json` audit output.
#[derive(serde::Serialize)]
@@ -112,7 +115,7 @@ impl Auditor {
#[allow(clippy::too_many_arguments, reason = "to keep PHP signature")]
pub fn audit(
&self,
- io: &mut dyn IOInterface,
+ io: &Rc<RefCell<dyn IOInterface>>,
repo_set: &RepositorySet,
packages: Vec<PackageInterfaceHandle>,
format: &str,
@@ -425,13 +428,14 @@ impl Auditor {
/// @param self::FORMAT_* $format The format that will be used to output audit results.
fn output_advisories(
&self,
- io: &mut dyn IOInterface,
+ io: &Rc<RefCell<dyn IOInterface>>,
advisories: &IndexMap<String, Vec<AnySecurityAdvisory>>,
format: &str,
) -> anyhow::Result<()> {
match format {
Self::FORMAT_TABLE => {
- let io_as_console = io.as_any().downcast_ref::<ConsoleIO>();
+ let io_ref = io.borrow();
+ let io_as_console = io_ref.as_any().downcast_ref::<ConsoleIO>();
if io_as_console.is_none() {
return Err(InvalidArgumentException {
message: format!(
@@ -524,7 +528,7 @@ impl Auditor {
/// @param array<string, array<SecurityAdvisory>> $advisories
fn output_advisories_plain(
&self,
- io: &mut dyn IOInterface,
+ io: &Rc<RefCell<dyn IOInterface>>,
advisories: &IndexMap<String, Vec<AnySecurityAdvisory>>,
) -> anyhow::Result<()> {
let mut error: Vec<String> = vec![];
@@ -571,7 +575,7 @@ impl Auditor {
/// @param self::FORMAT_PLAIN|self::FORMAT_TABLE $format
fn output_abandoned_packages(
&self,
- io: &mut dyn IOInterface,
+ io: &Rc<RefCell<dyn IOInterface>>,
packages: &[CompletePackageInterfaceHandle],
format: &str,
) -> anyhow::Result<()> {
@@ -602,7 +606,8 @@ impl Auditor {
return Ok(());
}
- let io_as_console = io.as_any().downcast_ref::<ConsoleIO>();
+ let io_ref = io.borrow();
+ let io_as_console = io_ref.as_any().downcast_ref::<ConsoleIO>();
if io_as_console.is_none() {
return Err(InvalidArgumentException {
message: format!(
diff --git a/crates/shirabe/src/command/audit_command.rs b/crates/shirabe/src/command/audit_command.rs
index 24af44c..d8666bb 100644
--- a/crates/shirabe/src/command/audit_command.rs
+++ b/crates/shirabe/src/command/audit_command.rs
@@ -202,7 +202,7 @@ impl Command for AuditCommand {
let audit_format = self.get_audit_format(input, "format")?;
Ok(auditor
.audit(
- &mut *self.get_io().borrow_mut(),
+ &self.get_io(),
&repo_set,
packages,
&audit_format,
diff --git a/crates/shirabe/src/command/diagnose_command.rs b/crates/shirabe/src/command/diagnose_command.rs
index a465182..0c50d69 100644
--- a/crates/shirabe/src/command/diagnose_command.rs
+++ b/crates/shirabe/src/command/diagnose_command.rs
@@ -996,9 +996,11 @@ impl DiagnoseCommand {
)?);
repo_set.add_repository(composer_repo_as_repo)?;
- let mut io = BufferIO::new(String::new(), 0, None)?;
+ let io: std::rc::Rc<std::cell::RefCell<dyn IOInterface>> = std::rc::Rc::new(
+ std::cell::RefCell::new(BufferIO::new(String::new(), 0, None)?),
+ );
let result = match auditor.audit(
- &mut io,
+ &io,
&repo_set,
packages,
Auditor::FORMAT_TABLE,
@@ -1022,7 +1024,11 @@ impl DiagnoseCommand {
return Ok(PhpMixed::String(format!(
"<highlight>Audit found some issues:</>{}{}",
PHP_EOL,
- io.get_output()
+ io.borrow()
+ .as_any()
+ .downcast_ref::<BufferIO>()
+ .unwrap()
+ .get_output()
)));
}
diff --git a/crates/shirabe/src/installer.rs b/crates/shirabe/src/installer.rs
index f051563..1e72a94 100644
--- a/crates/shirabe/src/installer.rs
+++ b/crates/shirabe/src/installer.rs
@@ -517,7 +517,7 @@ impl Installer {
}
let audit_result = auditor.audit(
- &mut *self.io.borrow_mut(),
+ &self.io,
&repo_set,
packages,
&audit_config.audit_format,
diff --git a/crates/shirabe/tests/advisory/auditor_test.rs b/crates/shirabe/tests/advisory/auditor_test.rs
index c908f0b..fa99825 100644
--- a/crates/shirabe/tests/advisory/auditor_test.rs
+++ b/crates/shirabe/tests/advisory/auditor_test.rs
@@ -11,6 +11,7 @@ use shirabe::advisory::PartialSecurityAdvisory;
use shirabe::advisory::SecurityAdvisory;
use shirabe::downloader::TransportException;
use shirabe::io::BufferIO;
+use shirabe::io::IOInterface;
use shirabe::io::io_interface;
use shirabe::package::BasePackageHandle;
use shirabe::package::PackageInterfaceHandle;
@@ -28,6 +29,8 @@ use shirabe_php_shim::date_create;
use shirabe_semver::VersionParser;
use shirabe_semver::constraint::AnyConstraint;
use shirabe_semver::constraint::SimpleConstraint;
+use std::cell::RefCell;
+use std::rc::Rc;
fn constraint(operator: &str, version: &str) -> shirabe_semver::constraint::AnyConstraint {
SimpleConstraint::new(operator.to_string(), version.to_string(), None).into()
@@ -591,12 +594,13 @@ Found 2 abandoned packages:
for case in cases {
let repo_set = get_repo_set();
- let mut io =
- BufferIO::new(String::new(), output_interface::VERBOSITY_NORMAL, None).unwrap();
+ let io: Rc<RefCell<dyn IOInterface>> = Rc::new(RefCell::new(
+ BufferIO::new(String::new(), output_interface::VERBOSITY_NORMAL, None).unwrap(),
+ ));
let auditor = Auditor;
let result = auditor
.audit(
- &mut io,
+ &io,
&repo_set,
case.packages.clone(),
case.format,
@@ -609,7 +613,16 @@ Found 2 abandoned packages:
)
.unwrap();
assert_eq!(case.expected, result);
- assert_eq!(case.output, io.get_output().replace('\r', "").trim());
+ assert_eq!(
+ case.output,
+ io.borrow()
+ .as_any()
+ .downcast_ref::<BufferIO>()
+ .unwrap()
+ .get_output()
+ .replace('\r', "")
+ .trim()
+ );
}
}
@@ -811,23 +824,21 @@ fn test_audit_with_ignore() {
let repo_set = get_repo_set();
let (io_mock, _io_guard) = get_io_mock(io_interface::NORMAL).unwrap();
let auditor = Auditor;
- let result = {
- let mut io_guard = io_mock.borrow_mut();
- auditor
- .audit(
- &mut *io_guard,
- &repo_set,
- case.packages.clone(),
- Auditor::FORMAT_PLAIN,
- false,
- case.ignored_ids.clone(),
- Auditor::ABANDONED_FAIL,
- IndexMap::new(),
- false,
- IndexMap::new(),
- )
- .unwrap()
- };
+ let io_dyn: Rc<RefCell<dyn IOInterface>> = io_mock.clone();
+ let result = auditor
+ .audit(
+ &io_dyn,
+ &repo_set,
+ case.packages.clone(),
+ Auditor::FORMAT_PLAIN,
+ false,
+ case.ignored_ids.clone(),
+ Auditor::ABANDONED_FAIL,
+ IndexMap::new(),
+ false,
+ IndexMap::new(),
+ )
+ .unwrap();
io_mock
.borrow_mut()
.expects(case.expected_output.clone(), true)
@@ -913,11 +924,12 @@ fn test_audit_with_ignore_unreachable() {
// Without the ignoreUnreachable flag the TransportException propagates.
{
let repo_set = make_repo_set();
- let mut io =
- BufferIO::new(String::new(), output_interface::VERBOSITY_NORMAL, None).unwrap();
+ let io: Rc<RefCell<dyn IOInterface>> = Rc::new(RefCell::new(
+ BufferIO::new(String::new(), output_interface::VERBOSITY_NORMAL, None).unwrap(),
+ ));
let err = auditor
.audit(
- &mut io,
+ &io,
&repo_set,
packages.clone(),
Auditor::FORMAT_PLAIN,
@@ -935,11 +947,12 @@ fn test_audit_with_ignore_unreachable() {
// With the ignoreUnreachable flag the advisories from reachable repos are reported.
{
let repo_set = make_repo_set();
- let mut io =
- BufferIO::new(String::new(), output_interface::VERBOSITY_NORMAL, None).unwrap();
+ let io: Rc<RefCell<dyn IOInterface>> = Rc::new(RefCell::new(
+ BufferIO::new(String::new(), output_interface::VERBOSITY_NORMAL, None).unwrap(),
+ ));
let result = auditor
.audit(
- &mut io,
+ &io,
&repo_set,
packages.clone(),
Auditor::FORMAT_PLAIN,
@@ -953,7 +966,12 @@ fn test_audit_with_ignore_unreachable() {
.unwrap();
assert_eq!(Auditor::STATUS_VULNERABLE, result);
- let output = io.get_output();
+ let output = io
+ .borrow()
+ .as_any()
+ .downcast_ref::<BufferIO>()
+ .unwrap()
+ .get_output();
assert!(output.contains("The following repositories were unreachable:"));
assert!(output.contains("HTTP/1.1 404 Not Found"));
assert!(output.contains("First repo advisory"));
@@ -965,11 +983,12 @@ fn test_audit_with_ignore_unreachable() {
// With JSON format the unreachable repositories and advisories are both included.
{
let repo_set = make_repo_set();
- let mut io =
- BufferIO::new(String::new(), output_interface::VERBOSITY_NORMAL, None).unwrap();
+ let io: Rc<RefCell<dyn IOInterface>> = Rc::new(RefCell::new(
+ BufferIO::new(String::new(), output_interface::VERBOSITY_NORMAL, None).unwrap(),
+ ));
let result = auditor
.audit(
- &mut io,
+ &io,
&repo_set,
packages.clone(),
Auditor::FORMAT_JSON,
@@ -983,7 +1002,14 @@ fn test_audit_with_ignore_unreachable() {
.unwrap();
assert_eq!(Auditor::STATUS_VULNERABLE, result);
- let json: serde_json::Value = serde_json::from_str(&io.get_output()).unwrap();
+ let json: serde_json::Value = serde_json::from_str(
+ &io.borrow()
+ .as_any()
+ .downcast_ref::<BufferIO>()
+ .unwrap()
+ .get_output(),
+ )
+ .unwrap();
let unreachable = json
.get("unreachable-repositories")
.and_then(|v| v.as_array())
@@ -1056,23 +1082,21 @@ fn test_audit_with_ignore_severity() {
let repo_set = get_repo_set();
let (io_mock, _io_guard) = get_io_mock(io_interface::NORMAL).unwrap();
let auditor = Auditor;
- let result = {
- let mut io_guard = io_mock.borrow_mut();
- auditor
- .audit(
- &mut *io_guard,
- &repo_set,
- case.packages.clone(),
- Auditor::FORMAT_PLAIN,
- false,
- IndexMap::new(),
- Auditor::ABANDONED_IGNORE,
- case.ignored_severities.clone(),
- false,
- IndexMap::new(),
- )
- .unwrap()
- };
+ let io_dyn: Rc<RefCell<dyn IOInterface>> = io_mock.clone();
+ let result = auditor
+ .audit(
+ &io_dyn,
+ &repo_set,
+ case.packages.clone(),
+ Auditor::FORMAT_PLAIN,
+ false,
+ IndexMap::new(),
+ Auditor::ABANDONED_IGNORE,
+ case.ignored_severities.clone(),
+ false,
+ IndexMap::new(),
+ )
+ .unwrap();
io_mock
.borrow_mut()
.expects(case.expected_output.clone(), true)
diff --git a/crates/shirabe/tests/command/audit_command_test.rs b/crates/shirabe/tests/command/audit_command_test.rs
index bccc7c2..a1658ba 100644
--- a/crates/shirabe/tests/command/audit_command_test.rs
+++ b/crates/shirabe/tests/command/audit_command_test.rs
@@ -60,7 +60,6 @@ fn test_error_auditing_lock_file_when_it_is_missing() {
#[test]
#[serial]
-#[ignore = "'RefCell already mutably borrowed' panic at crates/shirabe/src/io/io_interface.rs:215 (write_error3) in the locked-audit advisory-fetch path; not in target file"]
fn test_audit_package_with_no_security_vulnerabilities() {
let tear_down = init_temp_composer(None, None, None, true);
let packages: Vec<PackageInterfaceHandle> = vec![get_package("dummy/pkg", "1.0.0")];