diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 9169f48..59d763f 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -12,6 +12,10 @@ may be the same file: a symbolic link at any of their paths, or two existing paths that are hard links to one file, is refused before anything is written. So is a path that exists but is not a regular file (a directory, say) or that cannot be looked up, such as a derived name over the filesystem's length limit. +Each output is also opened for writing before anything is written (an absent +one is created and removed again), so a read-only copy or sidecar, or a +directory the caller cannot create files in, is refused too, instead of +failing after the report has been replaced. These checks run before the writes, so a link swapped in between a check and its write is not caught, and a crash can leave a partial file: the planned generation-directory publish is meant to close both. diff --git a/src/main.rs b/src/main.rs index 1f1ee91..df68732 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1443,6 +1443,31 @@ fn kyaml_copy_path(out: &Path, sidecar: Option<&Path>) -> Result { } } } + // Lookup alone does not show that a path can be written: a read-only + // copy or sidecar, or a directory the caller may not create files in, + // would fail only after the report had been replaced. Open each output + // for writing now, without truncating it; an absent one is created and + // removed again. The kernel decides, so ACLs and ownership count too. + for (&(role, candidate), &existed) in outputs.iter().zip(&exists) { + let probe = if existed { + fs::OpenOptions::new().write(true).open(candidate).map(drop) + } else { + fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(candidate) + .and_then(|file| { + drop(file); + fs::remove_file(candidate) + }) + }; + probe.map_err(|err| { + anyhow::anyhow!( + "cannot write the {role} {}: {err}; nothing was written", + candidate.display() + ) + })?; + } Ok(path) } diff --git a/tests/coverage_php_tests.rs b/tests/coverage_php_tests.rs index 14ba67a..91032b0 100644 --- a/tests/coverage_php_tests.rs +++ b/tests/coverage_php_tests.rs @@ -505,3 +505,61 @@ fn kyaml_output_format_refuses_a_directory_at_an_output_path() { "the report was written before the copy failed" ); } + +/// Root ignores permission bits, so the permission tests below only mean +/// something when run unprivileged. They return early otherwise. +#[cfg(unix)] +fn writes_ignore_permissions(dir: &std::path::Path) -> bool { + use std::os::unix::fs::PermissionsExt; + let probe = dir.join("privilege-probe"); + std::fs::write(&probe, "").unwrap(); + std::fs::set_permissions(&probe, std::fs::Permissions::from_mode(0o444)).unwrap(); + let privileged = std::fs::OpenOptions::new().write(true).open(&probe).is_ok(); + std::fs::remove_file(&probe).unwrap(); + privileged +} + +#[cfg(unix)] +#[test] +fn kyaml_output_format_refuses_a_read_only_copy_before_writing() { + use std::os::unix::fs::PermissionsExt; + let (dir, project) = project_dir(); + if writes_ignore_permissions(dir.path()) { + eprintln!("skipped: running with privileges that ignore permission bits"); + return; + } + let out = dir.path().join("report.json"); + std::fs::write(&out, "old report\n").unwrap(); + let copy = dir.path().join("report.kyaml"); + std::fs::write(©, "old copy\n").unwrap(); + std::fs::set_permissions(©, std::fs::Permissions::from_mode(0o444)).unwrap(); + let output = assail_kyaml(&project, &out); + assert!(!output.status.success()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("nothing was written"), "{stderr}"); + assert_eq!(std::fs::read_to_string(&out).unwrap(), "old report\n"); + assert_eq!(std::fs::read_to_string(©).unwrap(), "old copy\n"); +} + +#[cfg(unix)] +#[test] +fn kyaml_output_format_refuses_a_directory_it_cannot_create_in() { + use std::os::unix::fs::PermissionsExt; + let (dir, project) = project_dir(); + if writes_ignore_permissions(dir.path()) { + eprintln!("skipped: running with privileges that ignore permission bits"); + return; + } + let reports = dir.path().join("reports"); + std::fs::create_dir(&reports).unwrap(); + let out = reports.join("report.json"); + std::fs::write(&out, "old report\n").unwrap(); + std::fs::set_permissions(&reports, std::fs::Permissions::from_mode(0o555)).unwrap(); + let output = assail_kyaml_attested(&project, &out); + std::fs::set_permissions(&reports, std::fs::Permissions::from_mode(0o755)).unwrap(); + assert!(!output.status.success()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("nothing was written"), "{stderr}"); + assert_eq!(std::fs::read_to_string(&out).unwrap(), "old report\n"); + assert!(!reports.join("report.kyaml").exists()); +}