diff --git a/CHANGELOG.md b/CHANGELOG.md index 34331fb86..72edca347 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,16 @@ - Display the analyzed song tempo (BPM) as a badge in the rehearsal workspace. - 각 합주 역할(Role)별 개인 연습 진행도를 0~100% 범위로 기록 및 시각화할 수 있는 연습 진척도(`practiceProgress`) 트래커 기능 추가. UI 컨트롤(슬라이더 및 +/- 버튼)과 한/영 다국어 지원 포함. +### Fixed + +- Enforce one canonical local-audio resource policy across native local-file/YouTube bootstrap intake, the desktop bridge, Python request preflight, temporal decoding, and stem separation so oversized, overlong, malformed, wrong-rate, or non-finite input fails before bootstrap storage or expensive analysis/model work. +- Preflight source-container duration, sample rate, and channel count from the already-open audio handle before temporal, stem, or bass-transcription decoders resample, downmix, or truncate it; successful metadata probes rewind the handle and malformed probes fail closed. +- Bound the admitted canonical decoded mono buffer to 317,520,000 bytes as well as the existing 39,690,000-sample ceiling, so decoder dtype expansion cannot stay within the sample count while exceeding the explicit in-memory audio budget. +- Fail closed on malformed known YouTube duration metadata before `download=True`; Boolean, non-numeric, non-finite, zero, negative, and non-canonical numeric-subtype duration evidence can no longer authorize a media download through Python numeric coercion or subclass semantics. +- Align YouTube download admission with that same 100 MiB encoded-byte ceiling: abort in-flight with yt-dlp `max_filesize` and a progress hook, reject announced oversize before `download=True`, delete owned `.part` / `.ytdl` / `-Frag*` siblings from that import directory on abort, reject a completed path that resolves outside the current import cache before post-download validation, cleanup, or success, and delete owned post-download artifacts that still exceed the policy. A 60 MiB import that the old 50 MB check rejected is now accepted; a file one byte over 100 MiB is not. +- Bound native stored-score PDF reads to the 25 MiB product limit before heap allocation and revalidate PDF magic on the same opened descriptor, preventing an attached score that later grows from bypassing the local resource boundary. +- Treat every zero-element NumPy layout as empty chord input, including shapes whose first dimension is non-zero, before feature extraction. + ### Changed - Consolidated Bandit, dependency audits, supplemental secret checks, and Trivy into one trusted-branch security backstop, delegated CodeQL to GitHub default setup, and removed duplicate local PR security and release-preflight runs. diff --git a/apps/desktop/core/Cargo.toml b/apps/desktop/core/Cargo.toml index b01a537dc..44f482e73 100644 --- a/apps/desktop/core/Cargo.toml +++ b/apps/desktop/core/Cargo.toml @@ -7,7 +7,7 @@ publish = false [lib] name = "bandscope_desktop_core" -path = "src/lib.rs" +path = "src/root.rs" [lints.rust] unexpected_cfgs = { level = "warn", check-cfg = ['cfg(coverage)'] } diff --git a/apps/desktop/core/src/audio_resource.rs b/apps/desktop/core/src/audio_resource.rs new file mode 100644 index 000000000..a8383067b --- /dev/null +++ b/apps/desktop/core/src/audio_resource.rs @@ -0,0 +1,325 @@ +use crate::content_sha256::StreamingSha256; +use std::io::{ErrorKind, Read, Write}; + +/// Maximum encoded local-audio file size accepted by the desktop bootstrap boundary. +pub const MAX_LOCAL_AUDIO_FILE_BYTES: u64 = 100 * 1024 * 1024; + +const LOCAL_AUDIO_READ_ERROR: &str = "Could not read the selected audio file."; +const LOCAL_AUDIO_WRITE_ERROR: &str = "Could not prepare the local project workspace."; +const LOCAL_AUDIO_TOO_LARGE_ERROR: &str = + "Choose a shorter or smaller song file to start analysis."; + +/// Immutable identity evidence for one successfully staged local-audio byte stream. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct LocalAudioCopyReceipt { + /// Exact number of bytes written successfully to the staging writer. + pub file_size_bytes: u64, + /// SHA-256 of exactly the bytes written successfully, encoded as lowercase hexadecimal. + pub content_sha256: String, +} + +/// Validate a native local-audio file length before storing bootstrap metadata. +/// +/// The caller must obtain this length from the native filesystem descriptor or +/// metadata boundary rather than from renderer-controlled JSON. The function +/// intentionally returns only bounded product messages and never includes a +/// local path or payload content. +pub fn validate_local_audio_file_size(file_size_bytes: u64) -> Result { + if file_size_bytes == 0 { + return Err(LOCAL_AUDIO_READ_ERROR.to_string()); + } + if file_size_bytes > MAX_LOCAL_AUDIO_FILE_BYTES { + return Err(LOCAL_AUDIO_TOO_LARGE_ERROR.to_string()); + } + Ok(file_size_bytes) +} + +fn read_retrying_interrupted(reader: &mut impl Read, buffer: &mut [u8]) -> Result { + loop { + match reader.read(buffer) { + Ok(read) => return Ok(read), + Err(error) if error.kind() == ErrorKind::Interrupted => continue, + Err(_) => return Err(LOCAL_AUDIO_READ_ERROR.to_string()), + } + } +} + +fn copy_bounded_local_audio_with_limit( + mut reader: R, + writer: &mut W, + max_bytes: u64, +) -> Result { + let mut copied = 0_u64; + let mut buffer = [0_u8; 64 * 1024]; + let mut content_digest = StreamingSha256::default(); + + loop { + if copied == max_bytes { + let mut overflow_probe = [0_u8; 1]; + let read = read_retrying_interrupted(&mut reader, &mut overflow_probe)?; + if read == 0 { + break; + } + return Err(LOCAL_AUDIO_TOO_LARGE_ERROR.to_string()); + } + + let remaining = (max_bytes - copied).min(buffer.len() as u64) as usize; + let read = read_retrying_interrupted(&mut reader, &mut buffer[..remaining])?; + if read == 0 { + break; + } + writer + .write_all(&buffer[..read]) + .map_err(|_| LOCAL_AUDIO_WRITE_ERROR.to_string())?; + content_digest + .update(&buffer[..read]) + .map_err(|_| LOCAL_AUDIO_READ_ERROR.to_string())?; + copied += read as u64; + } + + if copied == 0 { + return Err(LOCAL_AUDIO_READ_ERROR.to_string()); + } + let content_sha256 = content_digest + .finalize_hex() + .map_err(|_| LOCAL_AUDIO_READ_ERROR.to_string())?; + Ok(LocalAudioCopyReceipt { + file_size_bytes: copied, + content_sha256, + }) +} + +/// Copy one admitted local-audio stream into a staging writer and return native content identity. +/// +/// Security Notes: callers must pass an already-open, OS-authorized source +/// descriptor and a private app-owned staging writer. The helper writes no more +/// than the 100 MiB ceiling, hashes exactly the bytes whose writes succeeded, +/// and, after reaching the ceiling exactly, reads only one probe byte to detect +/// source growth. Source-read and destination-write failures use distinct +/// bounded product errors so storage failures are not misdiagnosed as bad media. +/// The caller must discard the staging artifact on error, synchronize it before +/// publication, and bind the returned receipt only to the artifact that was +/// actually published. +pub fn copy_bounded_local_audio_with_receipt( + reader: R, + writer: &mut W, +) -> Result { + copy_bounded_local_audio_with_limit(reader, writer, MAX_LOCAL_AUDIO_FILE_BYTES) +} + +/// Re-read a published app-owned source and prove that it matches its staging receipt. +/// +/// Security Notes: the caller must pass an already-open descriptor for the +/// synchronized, published `source.` object. This helper opens no +/// path and grants no filesystem authority. The staging receipt is native +/// evidence from the prior bounded copy, so its byte length becomes the tighter +/// publication-read ceiling: the verifier hashes at most that many bytes and +/// reads one additional probe byte to reject growth. It then requires both size +/// and digest to equal the staging receipt. Any invalid expected length, read, +/// growth, truncation, or content mismatch is reported as a bounded +/// project-workspace failure because the selected source already passed +/// admission before publication. +pub fn verify_local_audio_publication_receipt( + reader: R, + expected: &LocalAudioCopyReceipt, +) -> Result { + if expected.file_size_bytes == 0 || expected.file_size_bytes > MAX_LOCAL_AUDIO_FILE_BYTES { + return Err(LOCAL_AUDIO_WRITE_ERROR.to_string()); + } + + let mut sink = std::io::sink(); + let actual = copy_bounded_local_audio_with_limit(reader, &mut sink, expected.file_size_bytes) + .map_err(|_| LOCAL_AUDIO_WRITE_ERROR.to_string())?; + if actual != *expected { + return Err(LOCAL_AUDIO_WRITE_ERROR.to_string()); + } + Ok(actual) +} + +/// Copy one admitted local-audio stream into a staging writer and return its byte count. +/// +/// This compatibility adapter preserves the existing desktop call boundary while +/// callers migrate to `copy_bounded_local_audio_with_receipt`. It uses the same +/// bounded copy and content-hash path and discards only the returned digest. +pub fn copy_bounded_local_audio(reader: R, writer: &mut W) -> Result { + copy_bounded_local_audio_with_receipt(reader, writer).map(|receipt| receipt.file_size_bytes) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::io::{Cursor, Error}; + + struct FailingWriter; + + impl Write for FailingWriter { + fn write(&mut self, _buffer: &[u8]) -> std::io::Result { + Err(Error::new(ErrorKind::Other, "simulated destination failure")) + } + + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } + } + + struct FailingReader; + + impl Read for FailingReader { + fn read(&mut self, _buffer: &mut [u8]) -> std::io::Result { + Err(Error::new(ErrorKind::Other, "simulated source failure")) + } + } + + struct InterruptedThenReader { + bytes: Cursor>, + interrupted: bool, + } + + impl Read for InterruptedThenReader { + fn read(&mut self, buffer: &mut [u8]) -> std::io::Result { + if !self.interrupted { + self.interrupted = true; + return Err(Error::from(ErrorKind::Interrupted)); + } + self.bytes.read(buffer) + } + } + + struct CountingReader { + bytes: Cursor>, + bytes_read: usize, + } + + impl Read for CountingReader { + fn read(&mut self, buffer: &mut [u8]) -> std::io::Result { + let read = self.bytes.read(buffer)?; + self.bytes_read += read; + Ok(read) + } + } + + #[test] + fn bounded_copy_rejects_stream_growth_without_staging_bytes_past_the_limit() { + let input = Cursor::new(vec![1_u8, 2, 3, 4, 5]); + let mut staged = Vec::new(); + + let error = copy_bounded_local_audio_with_limit(input, &mut staged, 4) + .expect_err("a source that grows beyond the admitted byte limit must fail closed"); + + assert_eq!(error, LOCAL_AUDIO_TOO_LARGE_ERROR); + assert_eq!(staged, vec![1_u8, 2, 3, 4]); + } + + #[test] + fn bounded_copy_accepts_the_exact_limit_and_reports_content_identity() { + let input = Cursor::new(vec![1_u8, 2, 3, 4]); + let mut staged = Vec::new(); + + let receipt = copy_bounded_local_audio_with_limit(input, &mut staged, 4) + .expect("the exact encoded-byte limit remains admissible"); + + assert_eq!(receipt.file_size_bytes, 4); + assert_eq!( + receipt.content_sha256, + "9f64a747e1b97f131fabb6b447296c9b6f0201e79fb3c5356e6c77e89b6a806a" + ); + assert_eq!(staged, vec![1_u8, 2, 3, 4]); + } + + #[test] + fn bounded_copy_reports_destination_failure_as_workspace_failure() { + let input = Cursor::new(vec![1_u8, 2, 3, 4]); + let mut staged = FailingWriter; + + let error = copy_bounded_local_audio_with_limit(input, &mut staged, 4) + .expect_err("a staging write failure must not be reported as a source read failure"); + + assert_eq!(error, LOCAL_AUDIO_WRITE_ERROR); + } + + #[test] + fn bounded_copy_keeps_source_failure_distinct_from_workspace_failure() { + let input = FailingReader; + let mut staged = Vec::new(); + + let error = copy_bounded_local_audio_with_limit(input, &mut staged, 4) + .expect_err("a source read failure must retain the media-read diagnosis"); + + assert_eq!(error, LOCAL_AUDIO_READ_ERROR); + assert!(staged.is_empty()); + } + + #[test] + fn bounded_copy_retries_interrupted_source_reads_without_changing_identity() { + let input = InterruptedThenReader { + bytes: Cursor::new(vec![1_u8, 2, 3, 4]), + interrupted: false, + }; + let mut staged = Vec::new(); + + let receipt = copy_bounded_local_audio_with_limit(input, &mut staged, 4) + .expect("an interrupted source read should be retried"); + + assert_eq!(receipt.file_size_bytes, 4); + assert_eq!( + receipt.content_sha256, + "9f64a747e1b97f131fabb6b447296c9b6f0201e79fb3c5356e6c77e89b6a806a" + ); + assert_eq!(staged, vec![1_u8, 2, 3, 4]); + } + + #[test] + fn publication_verification_maps_read_failure_to_workspace_failure() { + let expected = LocalAudioCopyReceipt { + file_size_bytes: 4, + content_sha256: + "9f64a747e1b97f131fabb6b447296c9b6f0201e79fb3c5356e6c77e89b6a806a" + .to_string(), + }; + + let error = verify_local_audio_publication_receipt(FailingReader, &expected) + .expect_err("published artifact read failure must be a workspace failure"); + + assert_eq!(error, LOCAL_AUDIO_WRITE_ERROR); + } + + #[test] + fn publication_verification_stops_after_expected_size_plus_one_probe_byte() { + let expected = LocalAudioCopyReceipt { + file_size_bytes: 4, + content_sha256: + "9f64a747e1b97f131fabb6b447296c9b6f0201e79fb3c5356e6c77e89b6a806a" + .to_string(), + }; + let mut published = CountingReader { + bytes: Cursor::new(vec![1_u8, 2, 3, 4, 5, 6, 7, 8]), + bytes_read: 0, + }; + + let error = verify_local_audio_publication_receipt(&mut published, &expected) + .expect_err("a grown published artifact must fail without scanning unrelated tail bytes"); + + assert_eq!(error, LOCAL_AUDIO_WRITE_ERROR); + assert_eq!(published.bytes_read, 5); + } + + #[test] + fn publication_verification_rejects_impossible_expected_lengths_without_reading() { + for file_size_bytes in [0, MAX_LOCAL_AUDIO_FILE_BYTES + 1] { + let expected = LocalAudioCopyReceipt { + file_size_bytes, + content_sha256: "00".repeat(32), + }; + let mut published = CountingReader { + bytes: Cursor::new(vec![1_u8, 2, 3, 4]), + bytes_read: 0, + }; + + let error = verify_local_audio_publication_receipt(&mut published, &expected) + .expect_err("an impossible native receipt length must fail before reading"); + + assert_eq!(error, LOCAL_AUDIO_WRITE_ERROR); + assert_eq!(published.bytes_read, 0); + } + } +} diff --git a/apps/desktop/core/src/content_sha256.rs b/apps/desktop/core/src/content_sha256.rs new file mode 100644 index 000000000..dbb109a49 --- /dev/null +++ b/apps/desktop/core/src/content_sha256.rs @@ -0,0 +1,293 @@ +//! Streaming SHA-256 for local content-identity receipts. +//! +//! The operations and constants follow NIST FIPS 180-4 SHA-256. The known-answer +//! tests below are correctness checks, not CAVP validation or a FIPS 140 claim. + +use std::io::{self, ErrorKind, Read}; + +const BLOCK_BYTES: usize = 64; +const DIGEST_BYTES: usize = 32; +const INITIAL_STATE: [u32; 8] = [ + 0x6a09_e667, + 0xbb67_ae85, + 0x3c6e_f372, + 0xa54f_f53a, + 0x510e_527f, + 0x9b05_688c, + 0x1f83_d9ab, + 0x5be0_cd19, +]; +const ROUND_CONSTANTS: [u32; 64] = [ + 0x428a_2f98, 0x7137_4491, 0xb5c0_fbcf, 0xe9b5_dba5, 0x3956_c25b, 0x59f1_11f1, + 0x923f_82a4, 0xab1c_5ed5, 0xd807_aa98, 0x1283_5b01, 0x2431_85be, 0x550c_7dc3, + 0x72be_5d74, 0x80de_b1fe, 0x9bdc_06a7, 0xc19b_f174, 0xe49b_69c1, 0xefbe_4786, + 0x0fc1_9dc6, 0x240c_a1cc, 0x2de9_2c6f, 0x4a74_84aa, 0x5cb0_a9dc, 0x76f9_88da, + 0x983e_5152, 0xa831_c66d, 0xb003_27c8, 0xbf59_7fc7, 0xc6e0_0bf3, 0xd5a7_9147, + 0x06ca_6351, 0x1429_2967, 0x27b7_0a85, 0x2e1b_2138, 0x4d2c_6dfc, 0x5338_0d13, + 0x650a_7354, 0x766a_0abb, 0x81c2_c92e, 0x9272_2c85, 0xa2bf_e8a1, 0xa81a_664b, + 0xc24b_8b70, 0xc76c_51a3, 0xd192_e819, 0xd699_0624, 0xf40e_3585, 0x106a_a070, + 0x19a4_c116, 0x1e37_6c08, 0x2748_774c, 0x34b0_bcb5, 0x391c_0cb3, 0x4ed8_aa4a, + 0x5b9c_ca4f, 0x682e_6ff3, 0x748f_82ee, 0x78a5_636f, 0x84c8_7814, 0x8cc7_0208, + 0x90be_fffa, 0xa450_6ceb, 0xbef9_a3f7, 0xc671_78f2, +]; + +#[derive(Clone)] +pub(crate) struct StreamingSha256 { + words: [u32; 8], + buffer: [u8; BLOCK_BYTES], + buffer_len: usize, + message_len_bytes: u64, +} + +impl Default for StreamingSha256 { + fn default() -> Self { + Self { + words: INITIAL_STATE, + buffer: [0; BLOCK_BYTES], + buffer_len: 0, + message_len_bytes: 0, + } + } +} + +impl StreamingSha256 { + /// Add the next contiguous admitted byte slice to this digest state. + pub(crate) fn update(&mut self, mut bytes: &[u8]) -> Result<(), ()> { + self.message_len_bytes = self + .message_len_bytes + .checked_add(bytes.len() as u64) + .ok_or(())?; + + if self.buffer_len != 0 { + let copied = (BLOCK_BYTES - self.buffer_len).min(bytes.len()); + self.buffer[self.buffer_len..self.buffer_len + copied] + .copy_from_slice(&bytes[..copied]); + self.buffer_len += copied; + bytes = &bytes[copied..]; + if self.buffer_len == BLOCK_BYTES { + let block = self.buffer; + self.compress(&block); + self.buffer_len = 0; + } + } + + while bytes.len() >= BLOCK_BYTES { + let block: &[u8; BLOCK_BYTES] = bytes[..BLOCK_BYTES].try_into().map_err(|_| ())?; + self.compress(block); + bytes = &bytes[BLOCK_BYTES..]; + } + + if !bytes.is_empty() { + self.buffer[..bytes.len()].copy_from_slice(bytes); + self.buffer_len = bytes.len(); + } + Ok(()) + } + + /// Finalize the digest as canonical lowercase hexadecimal. + pub(crate) fn finalize_hex(mut self) -> Result { + let message_len_bits = self.message_len_bytes.checked_mul(8).ok_or(())?; + + self.buffer[self.buffer_len] = 0x80; + self.buffer_len += 1; + if self.buffer_len > 56 { + self.buffer[self.buffer_len..].fill(0); + let block = self.buffer; + self.compress(&block); + self.buffer = [0; BLOCK_BYTES]; + self.buffer_len = 0; + } + self.buffer[self.buffer_len..56].fill(0); + self.buffer[56..].copy_from_slice(&message_len_bits.to_be_bytes()); + let block = self.buffer; + self.compress(&block); + + let mut digest = [0_u8; DIGEST_BYTES]; + for (index, word) in self.words.into_iter().enumerate() { + digest[index * 4..index * 4 + 4].copy_from_slice(&word.to_be_bytes()); + } + + let mut encoded = String::with_capacity(DIGEST_BYTES * 2); + const HEX: &[u8; 16] = b"0123456789abcdef"; + for byte in digest { + encoded.push(HEX[(byte >> 4) as usize] as char); + encoded.push(HEX[(byte & 0x0f) as usize] as char); + } + Ok(encoded) + } + + fn compress(&mut self, block: &[u8; BLOCK_BYTES]) { + let mut schedule = [0_u32; 64]; + for (index, chunk) in block.chunks_exact(4).enumerate() { + schedule[index] = u32::from_be_bytes( + chunk + .try_into() + .expect("SHA-256 message word always contains four bytes"), + ); + } + for index in 16..64 { + let small_sigma0 = schedule[index - 15].rotate_right(7) + ^ schedule[index - 15].rotate_right(18) + ^ (schedule[index - 15] >> 3); + let small_sigma1 = schedule[index - 2].rotate_right(17) + ^ schedule[index - 2].rotate_right(19) + ^ (schedule[index - 2] >> 10); + schedule[index] = schedule[index - 16] + .wrapping_add(small_sigma0) + .wrapping_add(schedule[index - 7]) + .wrapping_add(small_sigma1); + } + + let [mut a, mut b, mut c, mut d, mut e, mut f, mut g, mut h] = self.words; + for index in 0..64 { + let big_sigma1 = e.rotate_right(6) ^ e.rotate_right(11) ^ e.rotate_right(25); + let choose = (e & f) ^ ((!e) & g); + let temporary1 = h + .wrapping_add(big_sigma1) + .wrapping_add(choose) + .wrapping_add(ROUND_CONSTANTS[index]) + .wrapping_add(schedule[index]); + let big_sigma0 = a.rotate_right(2) ^ a.rotate_right(13) ^ a.rotate_right(22); + let majority = (a & b) ^ (a & c) ^ (b & c); + let temporary2 = big_sigma0.wrapping_add(majority); + + h = g; + g = f; + f = e; + e = d.wrapping_add(temporary1); + d = c; + c = b; + b = a; + a = temporary1.wrapping_add(temporary2); + } + + self.words[0] = self.words[0].wrapping_add(a); + self.words[1] = self.words[1].wrapping_add(b); + self.words[2] = self.words[2].wrapping_add(c); + self.words[3] = self.words[3].wrapping_add(d); + self.words[4] = self.words[4].wrapping_add(e); + self.words[5] = self.words[5].wrapping_add(f); + self.words[6] = self.words[6].wrapping_add(g); + self.words[7] = self.words[7].wrapping_add(h); + } +} + +/// Hash a caller-owned byte stream as canonical lowercase SHA-256. +/// +/// Security Notes: this helper never opens a path, logs bytes, or grants filesystem +/// authority. The caller must supply an already-authorized reader and decide how +/// the resulting digest is bound to a concrete artifact. `Interrupted` reads are +/// retried; other reader failures are returned unchanged. This is content identity, +/// not an authenticity primitive or a FIPS module-validation claim. +pub fn sha256_hex_reader(mut reader: impl Read) -> io::Result { + let mut digest = StreamingSha256::default(); + let mut chunk = [0_u8; 64 * 1024]; + loop { + match reader.read(&mut chunk) { + Ok(0) => break, + Ok(read_bytes) => digest + .update(&chunk[..read_bytes]) + .map_err(|_| io::Error::new(ErrorKind::InvalidData, "SHA-256 input too large"))?, + Err(error) if error.kind() == ErrorKind::Interrupted => continue, + Err(error) => return Err(error), + } + } + digest + .finalize_hex() + .map_err(|_| io::Error::new(ErrorKind::InvalidData, "SHA-256 input too large")) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::io::{Cursor, Error}; + + fn digest_in_chunks(bytes: &[u8], chunk_size: usize) -> String { + let mut digest = StreamingSha256::default(); + for chunk in bytes.chunks(chunk_size) { + digest.update(chunk).expect("test vector length must fit SHA-256"); + } + digest + .finalize_hex() + .expect("test vector bit length must fit SHA-256") + } + + struct InterruptedShortReader { + bytes: Vec, + cursor: usize, + interrupted: bool, + } + + impl Read for InterruptedShortReader { + fn read(&mut self, output: &mut [u8]) -> io::Result { + if !self.interrupted { + self.interrupted = true; + return Err(Error::from(ErrorKind::Interrupted)); + } + if self.cursor == self.bytes.len() { + return Ok(0); + } + let copied = 7.min(output.len()).min(self.bytes.len() - self.cursor); + output[..copied].copy_from_slice(&self.bytes[self.cursor..self.cursor + copied]); + self.cursor += copied; + Ok(copied) + } + } + + struct FailingReader; + + impl Read for FailingReader { + fn read(&mut self, _output: &mut [u8]) -> io::Result { + Err(Error::new(ErrorKind::Other, "fixture read failure")) + } + } + + #[test] + fn matches_sha256_known_answer_vectors() { + for (message, expected) in [ + ( + &b""[..], + "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + ), + ( + &b"abc"[..], + "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad", + ), + ( + &b"abcdbcdecdefdefgefghfghighijhijkijkljklmklmnlmnomnopnopq"[..], + "248d6a61d20638b8e5c026930c3e6039a33ce45964ff2167f6ecedd419db06c1", + ), + ] { + assert_eq!(digest_in_chunks(message, 7), expected); + } + } + + #[test] + fn shared_reader_retries_interrupted_short_reads() { + let bytes = (0..131_111) + .map(|index| (index % 251) as u8) + .collect::>(); + let expected = sha256_hex_reader(Cursor::new(&bytes)).expect("reference hash should succeed"); + let actual = sha256_hex_reader(InterruptedShortReader { + bytes, + cursor: 0, + interrupted: false, + }) + .expect("interrupted short reads should be retried"); + assert_eq!(actual, expected); + } + + #[test] + fn shared_reader_propagates_non_interrupted_failure() { + let error = sha256_hex_reader(FailingReader).expect_err("reader failure must propagate"); + assert_eq!(error.kind(), ErrorKind::Other); + } + + #[test] + fn matches_the_million_a_vector() { + assert_eq!( + digest_in_chunks(&vec![b'a'; 1_000_000], 64 * 1024), + "cdc76e5c9914fb9281a1c7e284d73e67f1809a48a497200e046d39ccc7112cd0" + ); + } +} diff --git a/apps/desktop/core/src/publication_identity.rs b/apps/desktop/core/src/publication_identity.rs new file mode 100644 index 000000000..0d984b2a0 --- /dev/null +++ b/apps/desktop/core/src/publication_identity.rs @@ -0,0 +1,83 @@ +use crate::{ + audio_resource::{LocalAudioCopyReceipt, MAX_LOCAL_AUDIO_FILE_BYTES}, + runtime_core::{is_valid_project_id, AUDIO_EXTENSIONS}, +}; +use serde::{Deserialize, Serialize}; + +const LOCAL_AUDIO_PUBLICATION_IDENTITY_ERROR: &str = + "Could not prepare the local project workspace."; + +/// Path-free native identity for one verified app-owned local-audio publication. +/// +/// This value is suitable for Project Persistence handoff because it names only +/// a BandScope-owned artifact and carries the exact native size/digest evidence +/// produced by Resource Admission. It never contains an external or absolute +/// filesystem path. +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct LocalAudioPublicationIdentity { + /// Locally minted BandScope project id that owns the publication. + pub project_id: String, + /// Deterministic app-owned artifact name within that project. + pub artifact_name: String, + /// Canonical lowercase admitted audio extension. + pub extension: String, + /// Exact number of bytes in the verified publication. + pub file_size_bytes: u64, + /// Lowercase SHA-256 of the exact verified publication bytes. + pub content_sha256: String, +} + +fn is_lowercase_sha256(value: &str) -> bool { + value.len() == 64 + && value + .bytes() + .all(|byte| byte.is_ascii_digit() || (b'a'..=b'f').contains(&byte)) +} + +/// Build the durable path-free identity for verified local-audio publication evidence. +/// +/// Security Notes: callers must supply a project id minted under BandScope's +/// existing project-id grammar and the canonical lowercase extension that was +/// admitted by Resource Admission. The receipt must come from the verified +/// publication path, not renderer input. Invalid ids, extensions, sizes, or +/// digest encodings fail closed with the bounded project-workspace diagnosis. +pub fn build_local_audio_publication_identity( + project_id: &str, + extension: &str, + receipt: &LocalAudioCopyReceipt, +) -> Result { + if !is_valid_project_id(project_id) + || !AUDIO_EXTENSIONS.contains(&extension) + || extension.bytes().any(|byte| byte.is_ascii_uppercase()) + || receipt.file_size_bytes == 0 + || receipt.file_size_bytes > MAX_LOCAL_AUDIO_FILE_BYTES + || !is_lowercase_sha256(&receipt.content_sha256) + { + return Err(LOCAL_AUDIO_PUBLICATION_IDENTITY_ERROR.to_string()); + } + + Ok(LocalAudioPublicationIdentity { + project_id: project_id.to_string(), + artifact_name: format!("source.{extension}"), + extension: extension.to_string(), + file_size_bytes: receipt.file_size_bytes, + content_sha256: receipt.content_sha256.clone(), + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn lowercase_sha256_requires_exact_canonical_encoding() { + assert!(is_lowercase_sha256( + "9f64a747e1b97f131fabb6b447296c9b6f0201e79fb3c5356e6c77e89b6a806a" + )); + assert!(!is_lowercase_sha256(&"a".repeat(63))); + assert!(!is_lowercase_sha256(&"a".repeat(65))); + assert!(!is_lowercase_sha256(&"A".repeat(64))); + assert!(!is_lowercase_sha256(&"g".repeat(64))); + } +} diff --git a/apps/desktop/core/src/root.rs b/apps/desktop/core/src/root.rs new file mode 100644 index 000000000..56df05035 --- /dev/null +++ b/apps/desktop/core/src/root.rs @@ -0,0 +1,25 @@ +//! Pure, GUI-independent logic for the BandScope desktop application. +//! +//! The historical desktop-core implementation remains in `lib.rs` as the +//! compatibility module while bounded resource boundaries are isolated in +//! auditable modules. Public symbols are re-exported so downstream callers keep +//! the same crate-root API. + +#[path = "lib.rs"] +mod runtime_core; +mod audio_resource; +mod content_sha256; +mod publication_identity; +mod score_pdf; + +pub use audio_resource::{ + copy_bounded_local_audio, copy_bounded_local_audio_with_receipt, + validate_local_audio_file_size, verify_local_audio_publication_receipt, + LocalAudioCopyReceipt, MAX_LOCAL_AUDIO_FILE_BYTES, +}; +pub use content_sha256::sha256_hex_reader; +pub use publication_identity::{ + build_local_audio_publication_identity, LocalAudioPublicationIdentity, +}; +pub use runtime_core::*; +pub use score_pdf::read_validated_score_pdf; diff --git a/apps/desktop/core/src/score_pdf.rs b/apps/desktop/core/src/score_pdf.rs new file mode 100644 index 000000000..2b26744cc --- /dev/null +++ b/apps/desktop/core/src/score_pdf.rs @@ -0,0 +1,82 @@ +use crate::{MAX_SCORE_PDF_BYTES, PDF_MAGIC}; +use std::{fs::File, io::Read, path::Path}; + +const SCORE_READ_ERROR: &str = "Could not read the score PDF."; +const SCORE_TOO_LARGE_ERROR: &str = "Score PDF is too large (exceeds 25MB limit)."; +const SCORE_INVALID_PDF_ERROR: &str = "Stored score is not a valid PDF."; + +fn read_validated_pdf_stream( + reader: &mut impl Read, + expected_len: u64, +) -> Result, String> { + if expected_len > MAX_SCORE_PDF_BYTES { + return Err(SCORE_TOO_LARGE_ERROR.to_string()); + } + + // MAX_SCORE_PDF_BYTES is 25 MiB, which fits every supported Rust `usize`. + let mut bytes = vec![0_u8; expected_len as usize]; + reader + .read_exact(&mut bytes) + .map_err(|_| SCORE_READ_ERROR.to_string())?; + + let mut growth_probe = [0_u8; 1]; + if reader + .read(&mut growth_probe) + .map_err(|_| SCORE_READ_ERROR.to_string())? + != 0 + { + return Err(SCORE_TOO_LARGE_ERROR.to_string()); + } + + if !bytes.starts_with(PDF_MAGIC) { + return Err(SCORE_INVALID_PDF_ERROR.to_string()); + } + + Ok(bytes) +} + +/// Read one already-authorized stored score without allocating beyond the PDF limit. +/// +/// The caller remains responsible for path authority and containment. This helper +/// opens that resolved path once, snapshots the descriptor length, allocates only +/// that bounded size, reads exactly that many bytes, and then probes one additional +/// byte on the same descriptor. A file that was already oversized is rejected +/// before heap allocation; a file that grows after metadata inspection is rejected +/// by the one-byte probe without extending the heap buffer beyond the product cap. +/// Errors intentionally omit the local path and file content. +pub fn read_validated_score_pdf(path: &Path) -> Result, String> { + let mut file = File::open(path).map_err(|_| SCORE_READ_ERROR.to_string())?; + let metadata = file + .metadata() + .map_err(|_| SCORE_READ_ERROR.to_string())?; + if !metadata.is_file() { + return Err(SCORE_READ_ERROR.to_string()); + } + read_validated_pdf_stream(&mut file, metadata.len()) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::io::Cursor; + + #[test] + fn stream_rejects_growth_after_the_metadata_length_snapshot() { + let mut reader = Cursor::new(b"%PDF-extra".to_vec()); + + let error = read_validated_pdf_stream(&mut reader, PDF_MAGIC.len() as u64) + .expect_err("bytes beyond the metadata snapshot must fail closed"); + + assert_eq!(error, SCORE_TOO_LARGE_ERROR); + } + + #[test] + fn stream_rejects_truncation_after_the_metadata_length_snapshot() { + let mut reader = Cursor::new(PDF_MAGIC.to_vec()); + + let error = read_validated_pdf_stream(&mut reader, (PDF_MAGIC.len() + 1) as u64) + .expect_err("truncation after the metadata snapshot must fail closed"); + + assert_eq!(error, SCORE_READ_ERROR); + } +} diff --git a/apps/desktop/core/tests/audio_resource_next_action.rs b/apps/desktop/core/tests/audio_resource_next_action.rs new file mode 100644 index 000000000..9fa77d800 --- /dev/null +++ b/apps/desktop/core/tests/audio_resource_next_action.rs @@ -0,0 +1,9 @@ +use bandscope_desktop_core::{validate_local_audio_file_size, MAX_LOCAL_AUDIO_FILE_BYTES}; + +#[test] +fn oversized_local_audio_names_the_next_rehearsal_action() { + assert_eq!( + validate_local_audio_file_size(MAX_LOCAL_AUDIO_FILE_BYTES + 1), + Err("Choose a shorter or smaller song file to start analysis.".to_string()) + ); +} diff --git a/apps/desktop/core/tests/audio_resource_policy.rs b/apps/desktop/core/tests/audio_resource_policy.rs new file mode 100644 index 000000000..163e49d50 --- /dev/null +++ b/apps/desktop/core/tests/audio_resource_policy.rs @@ -0,0 +1,25 @@ +use bandscope_desktop_core::{validate_local_audio_file_size, MAX_LOCAL_AUDIO_FILE_BYTES}; + +#[test] +fn local_audio_size_policy_accepts_the_exact_native_bootstrap_ceiling() { + assert_eq!( + validate_local_audio_file_size(MAX_LOCAL_AUDIO_FILE_BYTES), + Ok(MAX_LOCAL_AUDIO_FILE_BYTES) + ); +} + +#[test] +fn local_audio_size_policy_rejects_an_empty_native_bootstrap_source() { + assert_eq!( + validate_local_audio_file_size(0), + Err("Could not read the selected audio file.".to_string()) + ); +} + +#[test] +fn local_audio_size_policy_rejects_a_native_source_above_the_canonical_ceiling() { + assert_eq!( + validate_local_audio_file_size(MAX_LOCAL_AUDIO_FILE_BYTES + 1), + Err("Choose a shorter or smaller song file to start analysis.".to_string()) + ); +} diff --git a/apps/desktop/core/tests/content_sha256_shared_kernel.rs b/apps/desktop/core/tests/content_sha256_shared_kernel.rs new file mode 100644 index 000000000..b8d9a0ebd --- /dev/null +++ b/apps/desktop/core/tests/content_sha256_shared_kernel.rs @@ -0,0 +1,10 @@ +use bandscope_desktop_core::sha256_hex_reader; +use std::io::Cursor; + +#[test] +fn shared_sha256_reader_matches_the_fips_180_4_abc_vector() { + assert_eq!( + sha256_hex_reader(Cursor::new(b"abc")).as_deref(), + Ok("ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad") + ); +} diff --git a/apps/desktop/core/tests/local_audio_content_identity.rs b/apps/desktop/core/tests/local_audio_content_identity.rs new file mode 100644 index 000000000..b43eb1bc5 --- /dev/null +++ b/apps/desktop/core/tests/local_audio_content_identity.rs @@ -0,0 +1,43 @@ +use bandscope_desktop_core::{ + copy_bounded_local_audio_with_receipt, verify_local_audio_publication_receipt, +}; +use std::io::Cursor; + +#[test] +fn local_audio_copy_receipt_hashes_exact_admitted_bytes() { + let input = vec![1_u8, 2, 3, 4]; + let mut staged = Vec::new(); + + let receipt = copy_bounded_local_audio_with_receipt(Cursor::new(&input), &mut staged) + .expect("bounded admission should return content identity for the bytes it stages"); + + assert_eq!(staged, input); + assert_eq!(receipt.file_size_bytes, 4); + assert_eq!( + receipt.content_sha256, + "9f64a747e1b97f131fabb6b447296c9b6f0201e79fb3c5356e6c77e89b6a806a" + ); +} + +#[test] +fn publication_receipt_requires_the_published_bytes_to_match_the_stage() { + let input = vec![1_u8, 2, 3, 4]; + let mut staged = Vec::new(); + let staged_receipt = copy_bounded_local_audio_with_receipt(Cursor::new(&input), &mut staged) + .expect("staging should produce native identity evidence"); + + let published_receipt = verify_local_audio_publication_receipt( + Cursor::new(&staged), + &staged_receipt, + ) + .expect("unchanged published bytes should retain the staging identity"); + + assert_eq!(published_receipt, staged_receipt); + + let mismatch = verify_local_audio_publication_receipt( + Cursor::new(vec![1_u8, 2, 3, 5]), + &staged_receipt, + ) + .expect_err("same-size mutation after staging must fail publication binding"); + assert_eq!(mismatch, "Could not prepare the local project workspace."); +} diff --git a/apps/desktop/core/tests/local_audio_publication_identity.rs b/apps/desktop/core/tests/local_audio_publication_identity.rs new file mode 100644 index 000000000..cd3b5fb63 --- /dev/null +++ b/apps/desktop/core/tests/local_audio_publication_identity.rs @@ -0,0 +1,77 @@ +use bandscope_desktop_core::{ + build_local_audio_publication_identity, LocalAudioCopyReceipt, +}; + +fn receipt() -> LocalAudioCopyReceipt { + LocalAudioCopyReceipt { + file_size_bytes: 4, + content_sha256: + "9f64a747e1b97f131fabb6b447296c9b6f0201e79fb3c5356e6c77e89b6a806a".to_string(), + } +} + +#[test] +fn publication_identity_is_path_free_and_deterministic() { + let identity = build_local_audio_publication_identity("project-1-1", "wav", &receipt()) + .expect("verified publication evidence should become a durable path-free identity"); + + assert_eq!(identity.project_id, "project-1-1"); + assert_eq!(identity.artifact_name, "source.wav"); + assert_eq!(identity.extension, "wav"); + assert_eq!(identity.file_size_bytes, 4); + assert_eq!( + identity.content_sha256, + "9f64a747e1b97f131fabb6b447296c9b6f0201e79fb3c5356e6c77e89b6a806a" + ); + + let json = serde_json::to_value(&identity).expect("publication identity should serialize"); + assert_eq!( + json, + serde_json::json!({ + "projectId": "project-1-1", + "artifactName": "source.wav", + "extension": "wav", + "fileSizeBytes": 4, + "contentSha256": "9f64a747e1b97f131fabb6b447296c9b6f0201e79fb3c5356e6c77e89b6a806a" + }) + ); + assert!(json.get("sourcePath").is_none()); + assert!(json.get("path").is_none()); +} + +#[test] +fn publication_identity_rejects_noncanonical_or_fabricated_evidence() { + for (project_id, extension, receipt) in [ + ("../project-1-1", "wav", receipt()), + ("project-1-1", "WAV", receipt()), + ("project-1-1", "exe", receipt()), + ( + "project-1-1", + "wav", + LocalAudioCopyReceipt { + file_size_bytes: 0, + content_sha256: "00".repeat(32), + }, + ), + ( + "project-1-1", + "wav", + LocalAudioCopyReceipt { + file_size_bytes: 4, + content_sha256: "AA".repeat(32), + }, + ), + ( + "project-1-1", + "wav", + LocalAudioCopyReceipt { + file_size_bytes: 4, + content_sha256: "not-a-sha256".to_string(), + }, + ), + ] { + let error = build_local_audio_publication_identity(project_id, extension, &receipt) + .expect_err("only canonical native publication evidence may cross persistence handoff"); + assert_eq!(error, "Could not prepare the local project workspace."); + } +} diff --git a/apps/desktop/core/tests/score_pdf_read.rs b/apps/desktop/core/tests/score_pdf_read.rs new file mode 100644 index 000000000..b70068931 --- /dev/null +++ b/apps/desktop/core/tests/score_pdf_read.rs @@ -0,0 +1,91 @@ +use bandscope_desktop_core::{read_validated_score_pdf, MAX_SCORE_PDF_BYTES}; +use std::io::Write; +use std::path::PathBuf; +use std::time::{SystemTime, UNIX_EPOCH}; + +fn unique_test_dir(name: &str) -> PathBuf { + let suffix = SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("system clock should be after epoch") + .as_nanos(); + std::env::temp_dir().join(format!("bandscope-{name}-{suffix}")) +} + +#[test] +fn score_pdf_read_returns_only_valid_bounded_pdf_bytes() { + let root = unique_test_dir("score-read-valid"); + std::fs::create_dir_all(&root).expect("test directory should be created"); + let path = root.join("score.pdf"); + let expected = b"%PDF-1.7\nvalidated body"; + std::fs::write(&path, expected).expect("valid PDF fixture should be written"); + + let actual = read_validated_score_pdf(&path).expect("valid stored PDF should be readable"); + + assert_eq!(actual, expected); + let _ = std::fs::remove_dir_all(root); +} + +#[test] +fn score_pdf_read_rejects_empty_short_and_wrong_magic_content() { + let root = unique_test_dir("score-read-invalid"); + std::fs::create_dir_all(&root).expect("test directory should be created"); + + for (name, content) in [ + ("empty.pdf", b"".as_slice()), + ("short.pdf", b"%PD".as_slice()), + ("wrong.pdf", b"PK\x03\x04 not a PDF".as_slice()), + ] { + let path = root.join(name); + std::fs::write(&path, content).expect("invalid PDF fixture should be written"); + let error = read_validated_score_pdf(&path).expect_err("invalid PDF must fail closed"); + assert!( + error == "Could not read the score PDF." || error == "Stored score is not a valid PDF.", + "unexpected payload-safe error: {error}" + ); + assert!(!error.contains(root.to_string_lossy().as_ref())); + } + + let _ = std::fs::remove_dir_all(root); +} + +#[test] +fn score_pdf_read_rejects_oversized_sparse_file_before_heap_allocation() { + let root = unique_test_dir("score-read-oversized"); + std::fs::create_dir_all(&root).expect("test directory should be created"); + let path = root.join("oversized.pdf"); + let mut file = std::fs::File::create(&path).expect("oversized PDF fixture should be created"); + file.write_all(b"%PDF-") + .expect("PDF magic should be written before extending sparse file"); + file.set_len(MAX_SCORE_PDF_BYTES + 1) + .expect("sparse PDF fixture should exceed the product limit"); + drop(file); + + let error = read_validated_score_pdf(&path).expect_err("oversized PDF must fail closed"); + + assert_eq!(error, "Score PDF is too large (exceeds 25MB limit)."); + let _ = std::fs::remove_dir_all(root); +} + +#[cfg(unix)] +#[test] +fn score_pdf_read_rejects_non_file_descriptor() { + let root = unique_test_dir("score-read-directory"); + std::fs::create_dir_all(&root).expect("test directory should be created"); + + let error = read_validated_score_pdf(&root).expect_err("directory must fail closed"); + + assert_eq!(error, "Could not read the score PDF."); + assert!(!error.contains(root.to_string_lossy().as_ref())); + let _ = std::fs::remove_dir_all(root); +} + +#[test] +fn score_pdf_read_rejects_missing_file_without_exposing_path() { + let root = unique_test_dir("score-read-missing"); + let path = root.join("private-score.pdf"); + + let error = read_validated_score_pdf(&path).expect_err("missing PDF must fail closed"); + + assert_eq!(error, "Could not read the score PDF."); + assert!(!error.contains("private-score.pdf")); +} diff --git a/apps/desktop/src-tauri/src/main.rs b/apps/desktop/src-tauri/src/main.rs index ed4f967bd..92e0cf784 100644 --- a/apps/desktop/src-tauri/src/main.rs +++ b/apps/desktop/src-tauri/src/main.rs @@ -14,6 +14,16 @@ use std::{ use tauri::{Emitter, Manager, Runtime}; use time::{format_description::well_known::Rfc3339, OffsetDateTime}; +/// Native-only cache of verified local-audio publication identities. +/// +/// Security Notes: entries are keyed only by BandScope-minted project ids and +/// contain the bounded path-free publication evidence emitted by Resource +/// Admission. User filesystem paths are never retained in this state. +#[derive(Default)] +struct LocalAudioPublicationIdentityState( + std::sync::Mutex>, +); + fn iso_timestamp_now() -> String { OffsetDateTime::now_utc() .format(&Rfc3339) @@ -141,7 +151,25 @@ fn app_owned_root( Ok(root) } -fn normalize_local_audio_source(path: &Path) -> Result { +/// Admit one OS-selected local audio file into a project-owned immutable source artifact. +/// +/// Security Notes: the external path is used only to canonicalize and open the +/// user-authorized source. Size is checked from that opened descriptor, bytes +/// are copied through the bounded Resource Admission helper into a private +/// same-project staging file. After the stage is synchronized, publication uses +/// a same-filesystem hard link so an existing `source.` name cannot +/// be overwritten; the private stage name is then removed. The published object +/// is required to remain a regular non-symlink filesystem entry and its opened +/// bytes must reproduce the staging size+SHA-256 receipt before bootstrap +/// authority is returned. This keeps later analysis bound to the app-owned +/// publication rather than the mutable user-selected path. Atomic no-follow +/// descriptor acquisition remains a separate platform-hardening requirement; +/// these portable checks do not claim O_NOFOLLOW-equivalent race semantics. +fn materialize_local_audio_source( + path: &Path, + project_root: &Path, + project_id: &str, +) -> Result<(LocalAudioSourcePayload, LocalAudioPublicationIdentity), String> { let canonical = path .canonicalize() .map_err(|_| "Could not read the selected audio file.".to_string())?; @@ -153,22 +181,109 @@ fn normalize_local_audio_source(path: &Path) -> Result receipt, + Err(error) => { + drop(staged); + let _ = std::fs::remove_file(&stage); + return Err(error); + } + }; + if staged.sync_all().is_err() { + drop(staged); + let _ = std::fs::remove_file(&stage); + return Err("Could not prepare the local project workspace.".to_string()); + } + drop(staged); - Ok(LocalAudioSourcePayload { - source_path: canonical.to_string_lossy().into_owned(), - file_name: file_name.to_string(), - extension, - file_size_bytes: metadata.len(), - }) + if std::fs::hard_link(&stage, &destination).is_err() { + let _ = std::fs::remove_file(&stage); + return Err("Could not prepare the local project workspace.".to_string()); + } + if std::fs::remove_file(&stage).is_err() { + let _ = std::fs::remove_file(&destination); + let _ = std::fs::remove_file(&stage); + return Err("Could not prepare the local project workspace.".to_string()); + } + + let published_path_metadata = match std::fs::symlink_metadata(&destination) { + Ok(metadata) if metadata.is_file() && !metadata.file_type().is_symlink() => metadata, + _ => { + let _ = std::fs::remove_file(&destination); + return Err("Could not prepare the local project workspace.".to_string()); + } + }; + if published_path_metadata.len() != receipt.file_size_bytes { + let _ = std::fs::remove_file(&destination); + return Err("Could not prepare the local project workspace.".to_string()); + } + let published = match std::fs::File::open(&destination) { + Ok(file) => file, + Err(_) => { + let _ = std::fs::remove_file(&destination); + return Err("Could not prepare the local project workspace.".to_string()); + } + }; + let published_descriptor_metadata = match published.metadata() { + Ok(metadata) if metadata.is_file() && metadata.len() == receipt.file_size_bytes => metadata, + _ => { + drop(published); + let _ = std::fs::remove_file(&destination); + return Err("Could not prepare the local project workspace.".to_string()); + } + }; + if published_descriptor_metadata.len() != published_path_metadata.len() + || verify_local_audio_publication_receipt(published, &receipt).is_err() + { + let _ = std::fs::remove_file(&destination); + return Err("Could not prepare the local project workspace.".to_string()); + } + let published_path_metadata = match std::fs::symlink_metadata(&destination) { + Ok(metadata) if metadata.is_file() && !metadata.file_type().is_symlink() => metadata, + _ => { + let _ = std::fs::remove_file(&destination); + return Err("Could not prepare the local project workspace.".to_string()); + } + }; + if published_path_metadata.len() != receipt.file_size_bytes { + let _ = std::fs::remove_file(&destination); + return Err("Could not prepare the local project workspace.".to_string()); + } + + let publication_identity = + build_local_audio_publication_identity(project_id, &extension, &receipt)?; + Ok(( + LocalAudioSourcePayload { + source_path: destination.to_string_lossy().into_owned(), + file_name, + extension, + file_size_bytes: receipt.file_size_bytes, + }, + publication_identity, + )) } fn parse_request_payload(payload: Value) -> Result { @@ -304,6 +419,20 @@ fn store_bootstrap_source(state: &AppState, summary: ProjectBootstrapSummaryPayl } } +/// Retain path-free publication evidence before the renderer receives bootstrap authority. +fn store_local_audio_publication_identity( + state: &LocalAudioPublicationIdentityState, + identity: LocalAudioPublicationIdentity, +) -> Result<(), String> { + let project_id = identity.project_id.clone(); + let mut identities = state + .0 + .lock() + .map_err(|_| "Could not prepare the local project workspace.".to_string())?; + identities.insert(project_id, identity); + Ok(()) +} + fn lookup_bootstrap_source( state: &AppState, project_id: &str, @@ -637,16 +766,19 @@ fn get_analysis_job_status(job_id: String, state: tauri::State<'_, AppState>) -> fn select_local_audio_source( app: tauri::AppHandle, state: tauri::State<'_, AppState>, + publication_state: tauri::State<'_, LocalAudioPublicationIdentityState>, ) -> Result { let path = FileDialog::new() .add_filter("Audio", &AUDIO_EXTENSIONS) .pick_file() .ok_or_else(|| "Choose a WAV, MP3, FLAC, or M4A file to start analysis.".to_string())?; - let source = normalize_local_audio_source(&path)?; let project_id = next_project_id(&state); let project_root = app_owned_root(&app, "projects", &project_id)?; let cache_root = app_owned_root(&app, "cache", &project_id)?; let temp_root = app_owned_root(&app, "temp", &project_id)?; + let (source, publication_identity) = + materialize_local_audio_source(&path, &project_root, &project_id)?; + store_local_audio_publication_identity(&publication_state, publication_identity)?; let summary = ProjectBootstrapSummaryPayload { project_id, @@ -712,6 +844,7 @@ async fn import_youtube_url( if parsed.get("ok").and_then(|v| v.as_bool()) == Some(true) { if let Some(metadata) = parsed.get("metadata") { let source = youtube_source_from_metadata(metadata, &cache_root)?; + validate_local_audio_file_size(source.file_size_bytes)?; let summary = ProjectBootstrapSummaryPayload { project_id, @@ -826,7 +959,9 @@ fn attach_score_pdf( /// Security Notes: no path crosses the IPC boundary. Both ids are validated /// against strict allowlist shapes, the path is rebuilt locally, and the /// canonicalize-plus-prefix guard in `resolve_existing_score_pdf` rejects any -/// escape from the app-owned scores root. +/// escape from the app-owned scores root. The resolved file is then read +/// through the bounded core helper so growth after attachment cannot trigger +/// an allocation beyond the 25 MiB product limit. #[tauri::command] fn read_score_pdf( project_id: String, @@ -838,7 +973,7 @@ fn read_score_pdf( } let scores_root = scores_root_for_project(&app, &project_id)?; let path = resolve_existing_score_pdf(&scores_root, &score_id)?; - std::fs::read(path).map_err(|_| "Could not read the score PDF.".to_string()) + read_validated_score_pdf(&path) } /// Security Notes: same id validation and traversal guard as `read_score_pdf`; @@ -868,6 +1003,7 @@ fn remove_score_pdf( fn main() { tauri::Builder::default() .manage(AppState::default()) + .manage(LocalAudioPublicationIdentityState::default()) .invoke_handler(tauri::generate_handler![ select_local_audio_source, import_youtube_url, diff --git a/apps/desktop/src-tauri/tests/local_audio_publication_contract.rs b/apps/desktop/src-tauri/tests/local_audio_publication_contract.rs new file mode 100644 index 000000000..f818d0e0b --- /dev/null +++ b/apps/desktop/src-tauri/tests/local_audio_publication_contract.rs @@ -0,0 +1,73 @@ +#[test] +fn local_audio_materializer_consumes_publication_bound_receipt() { + let source = include_str!("../src/main.rs"); + let materializer_start = source + .find("fn materialize_local_audio_source(") + .expect("desktop materializer must remain present"); + let materializer_tail = &source[materializer_start..]; + let materializer_end = materializer_tail + .find("\n}\n\nfn parse_request_payload") + .expect("materializer boundary must remain inspectable"); + let materializer = &materializer_tail[..materializer_end]; + + assert!( + materializer.contains("copy_bounded_local_audio_with_receipt"), + "production materialization must retain native size+SHA-256 staging evidence" + ); + assert!( + materializer.contains("verify_local_audio_publication_receipt"), + "production materialization must re-read the published app-owned source and bind it to the staging receipt" + ); + assert!( + !materializer.contains("copy_bounded_local_audio(source"), + "the compatibility byte-count-only adapter must not remain on the production publication path" + ); +} + +#[test] +fn local_audio_publication_must_not_overwrite_an_existing_source_name() { + let source = include_str!("../src/main.rs"); + let materializer_start = source + .find("fn materialize_local_audio_source(") + .expect("desktop materializer must remain present"); + let materializer_tail = &source[materializer_start..]; + let materializer_end = materializer_tail + .find("\n}\n\nfn parse_request_payload") + .expect("materializer boundary must remain inspectable"); + let materializer = &materializer_tail[..materializer_end]; + + assert!( + materializer.contains("std::fs::hard_link(&stage, &destination)"), + "publication must use an atomic no-clobber filesystem create instead of check-then-rename" + ); + assert!( + !materializer.contains("destination.exists()"), + "a preflight existence check is racy and must not authorize a later overwrite-capable rename" + ); + assert!( + !materializer.contains("std::fs::rename(&stage, &destination)"), + "overwrite-capable rename must not publish the immutable project source" + ); +} + +#[test] +fn local_audio_selection_retains_verified_path_free_identity_in_native_state() { + let source = include_str!("../src/main.rs"); + + assert!( + source.contains("struct LocalAudioPublicationIdentityState"), + "verified source identity must have a native-only state owner" + ); + assert!( + source.contains("build_local_audio_publication_identity(project_id, &extension, &receipt)"), + "the production materializer must derive persistence identity from the verified native receipt" + ); + assert!( + source.contains("store_local_audio_publication_identity(&publication_state, publication_identity)"), + "selection must retain native publication identity before returning bootstrap authority" + ); + assert!( + source.contains(".manage(LocalAudioPublicationIdentityState::default())"), + "the native publication identity state must be registered with the Tauri runtime" + ); +} diff --git a/apps/desktop/src/lib/analysis.audio-resource-next-action.test.ts b/apps/desktop/src/lib/analysis.audio-resource-next-action.test.ts new file mode 100644 index 000000000..8643ab84b --- /dev/null +++ b/apps/desktop/src/lib/analysis.audio-resource-next-action.test.ts @@ -0,0 +1,55 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { + MAX_LOCAL_AUDIO_FILE_BYTES, + importYoutubeUrl, + selectLocalAudioSource +} from "./analysis"; + +type TauriWindow = Window & { + __TAURI_INTERNALS__?: unknown; + __TAURI_INVOKE__?: unknown; +}; + +const tauriWindow = window as TauriWindow; +const NEXT_ACTION = "Choose a shorter or smaller song file to start analysis."; + +function oversizedBootstrap(projectId: string) { + return { + projectId, + sourceMode: "reference", + projectRoot: `/tmp/bandscope/projects/${projectId}`, + cacheRoot: `/tmp/bandscope/cache/${projectId}`, + tempRoot: `/tmp/bandscope/temp/${projectId}`, + source: { + sourcePath: `/tmp/bandscope/${projectId}/input.wav`, + fileName: "input.wav", + extension: "wav", + fileSizeBytes: MAX_LOCAL_AUDIO_FILE_BYTES + 1 + } + }; +} + +describe("audio resource rejection next action", () => { + beforeEach(() => { + delete tauriWindow.__TAURI_INTERNALS__; + delete tauriWindow.__TAURI_INVOKE__; + }); + + it("names the next action for an oversized local selection", async () => { + tauriWindow.__TAURI_INVOKE__ = vi.fn().mockResolvedValue(oversizedBootstrap("local-project")); + + await expect(selectLocalAudioSource()).resolves.toEqual({ + ok: false, + error: { code: "invalid_request", message: NEXT_ACTION } + }); + }); + + it("names the same next action for an oversized imported selection", async () => { + tauriWindow.__TAURI_INVOKE__ = vi.fn().mockResolvedValue(oversizedBootstrap("youtube-project")); + + await expect(importYoutubeUrl("https://youtu.be/4ozX4yFUC34")).resolves.toEqual({ + ok: false, + error: { code: "invalid_request", message: NEXT_ACTION } + }); + }); +}); diff --git a/apps/desktop/src/lib/analysis.resource-policy.test.ts b/apps/desktop/src/lib/analysis.resource-policy.test.ts new file mode 100644 index 000000000..33ab81804 --- /dev/null +++ b/apps/desktop/src/lib/analysis.resource-policy.test.ts @@ -0,0 +1,58 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { importYoutubeUrl, selectLocalAudioSource } from "./analysis"; + +type TauriWindow = Window & { + __TAURI_INTERNALS__?: unknown; + __TAURI_INVOKE__?: unknown; +}; + +const tauriWindow = window as TauriWindow; +const INVALID_RESOURCE_POLICY_MESSAGE = + "Selected audio file metadata violates the analysis resource policy."; + +function fractionalBootstrap(projectId: string) { + return { + projectId, + sourceMode: "reference", + projectRoot: `/tmp/bandscope/projects/${projectId}`, + cacheRoot: `/tmp/bandscope/cache/${projectId}`, + tempRoot: `/tmp/bandscope/temp/${projectId}`, + source: { + sourcePath: `/tmp/bandscope/${projectId}/input.wav`, + fileName: "input.wav", + extension: "wav", + fileSizeBytes: 1.5 + } + }; +} + +describe("analysis encoded-byte policy parity", () => { + beforeEach(() => { + delete tauriWindow.__TAURI_INTERNALS__; + delete tauriWindow.__TAURI_INVOKE__; + }); + + it("rejects fractional local-file metadata before project state", async () => { + tauriWindow.__TAURI_INVOKE__ = vi.fn().mockResolvedValue(fractionalBootstrap("local-project")); + + await expect(selectLocalAudioSource()).resolves.toEqual({ + ok: false, + error: { + code: "invalid_request", + message: INVALID_RESOURCE_POLICY_MESSAGE + } + }); + }); + + it("rejects fractional imported-file metadata before project state", async () => { + tauriWindow.__TAURI_INVOKE__ = vi.fn().mockResolvedValue(fractionalBootstrap("youtube-project")); + + await expect(importYoutubeUrl("https://youtu.be/4ozX4yFUC34")).resolves.toEqual({ + ok: false, + error: { + code: "invalid_request", + message: INVALID_RESOURCE_POLICY_MESSAGE + } + }); + }); +}); diff --git a/apps/desktop/src/lib/analysis.test.ts b/apps/desktop/src/lib/analysis.test.ts index e3347d1f5..4fb47f211 100644 --- a/apps/desktop/src/lib/analysis.test.ts +++ b/apps/desktop/src/lib/analysis.test.ts @@ -1,9 +1,11 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { createDemoAnalysisJobRequest, createDemoRehearsalSong } from "@bandscope/shared-types"; import { + MAX_LOCAL_AUDIO_FILE_BYTES, MAX_YOUTUBE_URL_LENGTH, getAnalysisJobStatus, importYoutubeUrl, + selectLocalAudioSource, startAnalysisJob } from "./analysis"; @@ -13,6 +15,7 @@ type TauriWindow = Window & { }; const tauriWindow = window as TauriWindow; +const OVERSIZED_LOCAL_AUDIO_NEXT_ACTION = "Choose a shorter or smaller song file to start analysis."; describe("analysis bridge", () => { beforeEach(() => { @@ -20,6 +23,58 @@ describe("analysis bridge", () => { delete tauriWindow.__TAURI_INVOKE__; }); + it("rejects an oversized native local-audio selection before it becomes project state", async () => { + tauriWindow.__TAURI_INVOKE__ = vi.fn().mockResolvedValue({ + projectId: "native-local-project", + sourceMode: "reference", + projectRoot: "/tmp/bandscope/projects/native-local-project", + cacheRoot: "/tmp/bandscope/cache/native-local-project", + tempRoot: "/tmp/bandscope/temp/native-local-project", + source: { + sourcePath: "/tmp/bandscope/input.wav", + fileName: "input.wav", + extension: "wav", + fileSizeBytes: MAX_LOCAL_AUDIO_FILE_BYTES + 1 + } + }); + + const selection = await selectLocalAudioSource(); + + expect(selection).toEqual({ + ok: false, + error: { + code: "invalid_request", + message: OVERSIZED_LOCAL_AUDIO_NEXT_ACTION + } + }); + }); + + it("rejects an oversized native YouTube import before it becomes project state", async () => { + tauriWindow.__TAURI_INVOKE__ = vi.fn().mockResolvedValue({ + projectId: "native-youtube-project", + sourceMode: "reference", + projectRoot: "/tmp/bandscope/projects/native-youtube-project", + cacheRoot: "/tmp/bandscope/cache/native-youtube-project", + tempRoot: "/tmp/bandscope/temp/native-youtube-project", + source: { + sourcePath: "/tmp/bandscope/temp/native-youtube-project/youtube.wav", + fileName: "youtube.wav", + extension: "wav", + fileSizeBytes: MAX_LOCAL_AUDIO_FILE_BYTES + 1 + } + }); + + const selection = await importYoutubeUrl("https://youtu.be/4ozX4yFUC34"); + + expect(selection).toEqual({ + ok: false, + error: { + code: "invalid_request", + message: OVERSIZED_LOCAL_AUDIO_NEXT_ACTION + } + }); + }); + it("imports a standard YouTube URL through the browser fallback when Tauri is absent", async () => { const selection = await importYoutubeUrl("https://www.youtube.com/watch?v=4ozX4yFUC34"); @@ -99,6 +154,37 @@ describe("analysis bridge", () => { expect(selection.ok).toBe(true); }); + it.each([ + "Could not read the selected audio file.", + "Could not prepare the local project workspace.", + "Could not prepare the local cache workspace.", + "Could not prepare the local temp workspace." + ])("preserves an approved native local-audio string error: %s", async (message) => { + tauriWindow.__TAURI_INVOKE__ = vi.fn().mockRejectedValue(message); + + await expect(selectLocalAudioSource()).resolves.toEqual({ + ok: false, + error: { + code: "invalid_request", + message + } + }); + }); + + it("redacts an unapproved native local-audio string error", async () => { + tauriWindow.__TAURI_INVOKE__ = vi + .fn() + .mockRejectedValue("Could not read /Users/example/Music/private-demo.wav"); + + await expect(selectLocalAudioSource()).resolves.toEqual({ + ok: false, + error: { + code: "invalid_request", + message: "Choose a WAV, MP3, FLAC, or M4A file to start analysis." + } + }); + }); + it("normalizes legacy analysis job status responses before returning them", async () => { const legacyResult = createDemoRehearsalSong() as unknown as { sections: Array>; diff --git a/apps/desktop/src/lib/analysis.ts b/apps/desktop/src/lib/analysis.ts index bb750b34b..33adb504b 100644 --- a/apps/desktop/src/lib/analysis.ts +++ b/apps/desktop/src/lib/analysis.ts @@ -35,8 +35,14 @@ const BROWSER_PROGRESS_STEPS = [ { progressLabel: "Saving reusable features", progressStage: "persist", progressPercent: 90 } ] as const; const UNSUPPORTED_LOCAL_AUDIO_MESSAGE = "Choose a WAV, MP3, FLAC, or M4A file to start analysis."; +const LOCAL_AUDIO_TOO_LARGE_MESSAGE = "Choose a shorter or smaller song file to start analysis."; +const LOCAL_AUDIO_POLICY_MESSAGE = + "Selected audio file metadata violates the analysis resource policy."; +const MAX_LOCAL_AUDIO_FILE_BYTES = 100 * 1024 * 1024; const SAFE_LOCAL_AUDIO_MESSAGES = new Set([ UNSUPPORTED_LOCAL_AUDIO_MESSAGE, + LOCAL_AUDIO_TOO_LARGE_MESSAGE, + LOCAL_AUDIO_POLICY_MESSAGE, "Could not read the selected audio file.", "Could not prepare the local project workspace.", "Could not prepare the local cache workspace.", @@ -45,7 +51,7 @@ const SAFE_LOCAL_AUDIO_MESSAGES = new Set([ const YOUTUBE_VIDEO_ID_PATTERN = /^[A-Za-z0-9_-]{11}$/; const MAX_YOUTUBE_URL_LENGTH = 2000; -export { MAX_YOUTUBE_URL_LENGTH }; +export { MAX_LOCAL_AUDIO_FILE_BYTES, MAX_YOUTUBE_URL_LENGTH }; /** Documented. */ export type LocalAudioSelectionResult = @@ -217,6 +223,26 @@ async function invokeAnalysis(command: string, args?: Record): return browserFallback(command, args); } +/** + * Parse a native/import bootstrap and enforce policy-v1 encoded-byte parity + * before the selection is allowed to become desktop project state. + * + * Python service and descriptor checks remain authoritative for analysis; this + * bridge check is defense in depth so local-file and imported-file intake fail + * at the same 100 MiB boundary instead of waiting for a later analysis stage. + */ +function parseBoundedAudioBootstrap(response: unknown): ProjectBootstrapSummary { + const bootstrap = parseProjectBootstrapSummary(response); + const fileSizeBytes = bootstrap.source.fileSizeBytes; + if (!Number.isSafeInteger(fileSizeBytes)) { + throw new Error(LOCAL_AUDIO_POLICY_MESSAGE); + } + if (fileSizeBytes > MAX_LOCAL_AUDIO_FILE_BYTES) { + throw new Error(LOCAL_AUDIO_TOO_LARGE_MESSAGE); + } + return bootstrap; +} + /** Documented. */ export function createDefaultAnalysisRequest(): AnalysisJobRequest { return createDemoAnalysisJobRequest(); @@ -228,7 +254,7 @@ export async function selectLocalAudioSource(): Promise` artifact and the native content identity for that publication. Project Persistence owns the later versioned project reference that consumes this evidence; it does not copy or hash user media itself. + +The hardening sequence exposed distinct defects: + +- source-read and app-owned destination-write failures were initially collapsed into one diagnosis; +- the one-byte over-limit probe was initially written into the disposable stage; +- the bounded copy returned only a byte count, so there was no native identity for the exact bytes written; +- SHA-256 existed in more than one security-sensitive implementation and initially had no reusable reader-only core port; +- a staging receipt alone did not prove that the final published object still contained the same bytes; +- publication verification initially read against the product-wide 100 MiB ceiling instead of the receipt's tighter expected length; +- the production Tauri materializer initially discarded the receipt and stayed on the byte-count-only adapter; +- publication initially used `destination.exists()` followed by overwrite-capable `rename`, creating a check-then-act clobber window; +- even after publication verification existed, Project Persistence still had no typed path-free handoff value for `projectId + artifactName + extension + fileSizeBytes + contentSha256`; +- after that type existed, the production selector still discarded the verified identity instead of retaining it in native state for the persistence owner. + +The canonical #866 branch now repairs those defects through native retention. Production local-file materialization consumes the native receipt, synchronizes the stage, publishes with a same-filesystem no-clobber hard link, removes the private stage name, verifies the published bytes, derives `LocalAudioPublicationIdentity` from that verified receipt, and retains the path-free value in native Tauri state keyed by the locally minted project id before returning bootstrap authority. The strict analysis-runtime `LocalAudioSource` wire remains unchanged. + +The remaining integration is now across the owning persistence boundary rather than the intake copy/hash path: #970 must consume this native identity when constructing durable `sourceReference`, and restart must re-admit the app-owned artifact before fresh playback authority is minted. Platform-atomic no-follow descriptor acquisition, parent-directory crash durability, YouTube durable-source policy, and decoder licensing remain separate open work. + +## Constraints and invariants + +- Local analysis remains local-first; this boundary adds no network authority. +- Renderer input never selects an arbitrary analysis or persistence path. +- The encoded-byte ceiling remains exactly 100 MiB. +- Metadata length before copying is not final evidence when the selected source can change during admission. +- Source-read failure and app-owned write/publication failure remain distinguishable without exposing paths or raw OS errors. +- `Interrupted` reads are retried. +- SHA-256 covers only byte slices whose staging writes succeeded. The one-byte growth probe is not admitted content and is not hashed into the receipt. +- SHA-256 is content-identity/correctness evidence only. This code does not claim CAVP validation, FIPS 140 validation, authenticity, or protection against an actor who can replace both artifact and stored digest. +- Reusable SHA-256 and publication-verification APIs accept caller-owned `Read` values and acquire no path authority. +- Publication verification consumes at most `expected.file_size_bytes + 1` bytes and rejects invalid expected lengths before reading. +- Publication must not overwrite an existing app-owned source name. Same-project hard-link publication fails closed when the destination exists or the filesystem cannot provide that primitive; it does not fall back to overwrite-capable rename. +- The analysis-runtime `LocalAudioSource` contract remains `sourcePath + fileName + extension + fileSizeBytes`. `contentSha256` is not injected into that strict Rust/TypeScript/Python request without a versioned contract change. +- The persistence identity is a distinct contract. It contains exactly `projectId + artifactName + extension + fileSizeBytes + contentSha256`; it contains no `path` or `sourcePath` field. +- The persistence identity accepts only an existing BandScope project-id grammar, canonical lowercase admitted extension, byte size `1..=100 MiB`, and exactly 64 lowercase hexadecimal SHA-256 characters. `artifactName` is derived as `source.` rather than accepted from renderer input. +- Verified persistence identity is retained only in native Tauri state keyed by the minted project id. The renderer does not author or supply that evidence. +- If native identity state cannot be retained, local-source selection fails closed rather than returning bootstrap authority without persistence evidence. +- Portable `symlink_metadata` / open / re-check logic narrows linked-object substitution but does not claim atomic `O_NOFOLLOW` or Windows reparse-point-equivalent semantics. +- The parent project directory is not yet explicitly synchronized after destination-link creation and stage unlink, so power-loss durability of the directory entries is not claimed. + +## Decision record + +1. Keep the external canonical path and revalidate before every analysis — rejected. Restart and persistence would still depend on mutable host authority. +2. Persist the absolute external path — rejected. It widens disclosure and violates #962's path-free direction. +3. Copy the selected file into app-owned `source.` — selected. Later analysis can use BandScope-owned authority. +4. Keep `std::io::copy` and one generic error — rejected. Explicit bounded read/write preserves the ceiling while distinguishing source and destination failures. +5. Hash later in the renderer or from the original path — rejected. Neither is authoritative for bytes actually staged into BandScope storage. +6. Add another SHA-256 implementation in persistence or Active Player — rejected. `bandscope_desktop_core::sha256_hex_reader` is the reader-only Shared Kernel. +7. Treat the staging receipt as publication truth without rereading — rejected. Same-size mutation would evade byte-count checks. +8. Re-read every published object up to 100 MiB — rejected. The native receipt gives a tighter expected length. +9. Leave the Tauri caller on `copy_bounded_local_audio -> u64` — rejected. Production publication must retain native size+digest evidence and verify the publication before bootstrap authority is returned. +10. Check `destination.exists()` and then rename the stage — rejected. On overwrite-capable rename semantics the sequence is racy. +11. Create the destination with `std::fs::hard_link(stage, destination)` and remove the private stage name — selected for the same-filesystem project root. It creates the destination without clobbering an existing name and keeps the synchronized bytes unchanged. +12. Add `contentSha256` to the existing analysis `LocalAudioSource` payload — rejected. Python admission is strict and this would mix persistence evidence with a narrower runtime request. +13. Define a separate path-free `LocalAudioPublicationIdentity` whose artifact name is derived from canonical native evidence — selected. This keeps Resource Admission as the copy/hash authority and gives #970 a serializable persistence input without absolute paths. +14. Return bootstrap authority while leaving the verified identity only in a local stack variable — rejected. The selector now retains the typed identity in native Tauri state keyed by project id before returning; #970 can adopt that native evidence without trusting renderer-authored digest/path data. + +## Implementation and exact evidence + +The cumulative hardening remains test-first where behavior changed: + +- `dbeee9c7407c72f999f584eb0eb9342ddc39fddd` adopted protected `develop@314ddeae7b775a4957594b599358c8255617eb2e` through ordinary non-force ancestry. +- RED `804a2867e877947feaffb1da6c6072e6a49049fe` and fix `0beee45b98e51ba46b571a82c6d0d93db61ea8d6` established exact-limit acceptance and one-byte-over rejection. +- `a2b1bd9e33a69be75f813f005abd37345200ce55` moved successful local-file intake to an app-owned same-project stage; `323a7fac00c4954af12b382802a9d6f8359ef4c5` exported the core port to Tauri. +- Diagnostics RED `131d6d7220985abd207559e6eb5dc122ac989cf4` and fix `ac4adfdb5df82f48aadd5e028433e3336d3ce2ae` separated source-read and destination-write failures and made the one-byte over-limit check read-only. +- Content-identity RED `dc413794fb84c736085ab77b763854ba0f58bdf1` and fix `566cd1f991296e7f3c288cb07a11c2d2effb258a` introduced `LocalAudioCopyReceipt { file_size_bytes, content_sha256 }`. +- Shared-kernel RED `373824c7bbb40f2df1bb2721316680378c104834` and fix `d1ba40683772019577fec4d8c767ff8b23294e38` exposed reader-only `sha256_hex_reader`. +- Publication RED `fdfdd7003b8a9162f846dcf22ffe66a3afd5f47e` and fix `a1c85cbfbdc7051169f097e8ad235e3bbac439d3` introduced `verify_local_audio_publication_receipt`; `20e7faaddd619c6cbd053876ca6de27b9933a4a2` exported it. +- Bounded-verification RED `6a0692ee288d3b126bd0598e07e03c88a702d567` and fix `c65a9fd312f4d67e6d1cad83b80b1213e692c8dd` changed publication verification to stop after expected bytes plus one growth probe. +- Production-integration RED `ed9fe7eba6261753dc0f68e820e2b642703fe2cd` and fix `bdf8f87d5e5c9db423537c7633e7ff4b92bec5b6` moved the Tauri materializer onto native receipt + publication verification. +- No-clobber RED `45b1f72abeded4e478775d31085244621f68c9f0` and fix `eb972e951ef090c92b595c752b18d66f11f6b96e` replaced check-then-rename with same-filesystem hard-link publication. +- Path-free handoff RED `bad908c83bfb89f545f0f2f637d96ac8fdfa3e0e` requires exact camelCase serialization of the five persistence fields, no path fields, and fail-closed rejection of invalid native evidence. +- Path-free handoff fix `87bdeea92d3bb6dc45eb666f422bd8a3d36f3872` adds `LocalAudioPublicationIdentity` and `build_local_audio_publication_identity`; export `344a9a39f32ac40b3e137c76e2cfd46243827bb5` makes the contract available from `bandscope_desktop_core` to the #970 owner. +- An earlier exploratory retention RED `cbfa967b16e94f2d84940665ce38537075a8ce41` was intentionally neutralized by `d8c57ce1d64d0bc9963219740aeaa83d9569a90b` rather than leaving a known failing head; those two commits add no production claim. +- Production native-retention RED `106ae75cad85553e56964a9844ea7a01f6ce456c` requires the materializer to derive the typed identity from the verified receipt, the selector to store it in native state, and Tauri to register that state. +- Native-retention fix `e4e2ba734bc80304a754ce2eb52e473fd9ee3631` returns `LocalAudioSourcePayload + LocalAudioPublicationIdentity` from materialization, stores the identity in `LocalAudioPublicationIdentityState` before bootstrap authority is returned, and registers the native state with the Tauri runtime. + +The SHA-256 implementation is checked against standard known-answer vectors including the empty message, `abc`, the multi-block vector, and one million `a` bytes. Those are correctness regressions, not validation-module evidence. + +## Security Notes + +The selected audio path, file metadata, and media bytes are untrusted. The OS file dialog supplies initial user authority; BandScope uses that path only to canonicalize and open the source. The project-owned artifact is the authority after successful admission. + +The production Tauri materializer synchronizes the stage, creates the destination through a no-clobber same-filesystem hard link, removes the private stage name, requires regular/non-symlink path observations, opens the publication, checks descriptor size, verifies exact receipt equality, and performs a post-verification path check. Publication mismatch or read failure is normalized to the bounded project-workspace diagnosis; source/destination paths, raw OS errors, and audio bytes are not exposed. + +`LocalAudioPublicationIdentity` does not acquire filesystem authority. It converts already verified native evidence into a deterministic, path-free value for the persistence boundary. Invalid project ids, extensions, byte counts, or digest encodings fail closed. Production local-file selection now retains that value in native Tauri state before returning the ordinary bootstrap summary, so the renderer does not need to invent a digest or persist a host path. Durable project serialization and restart re-admission remain #970 responsibilities. + +No new logging, telemetry, network transfer, or raw-media export is introduced. The SHA-256 receipt and publication identity are non-secret content identity. + +## Test and acceptance points + +- exact 100 MiB encoded-byte limit accepted; one byte over rejected; +- empty source rejected; +- source-reader and destination-writer failures remain distinct and path-safe; +- `Interrupted` reads retry without changing identity; +- failed writes cannot return a partial receipt; +- the growth probe is neither staged nor hashed; +- unchanged published bytes reproduce the staging receipt; +- same-size mutation, truncation, growth, or publication-read failure fails closed; +- grown publication stops after expected bytes plus one probe; +- production publication cannot use existence-check plus overwrite-capable rename; +- production Tauri local-file materialization consumes receipt and publication-verification ports, not the compatibility byte-count adapter; +- path-free identity serializes exactly the five persistence fields and cannot serialize `path`/`sourcePath`; +- invalid project ids, uppercase/unsupported extensions, zero/oversized byte counts, and noncanonical SHA-256 encodings are rejected; +- production local-file selection derives identity from the verified receipt and retains it in registered native Tauri state before returning bootstrap authority; +- hosted Rust/Tauri, Windows, macOS, security, SBOM, coverage/package, and independent-review evidence must be reacquired on the final exact #866 head. + +Synthetic arrays or source-text checks do not substitute for production scientific acceptance. Rights-cleared real decoded audio still has to exercise the integrated Windows/macOS intake/decode/analysis/playback path where the relevant commercial claim is made. + +## Remaining risks and follow-up + +The local-file path now has two separate native contracts: `LocalAudioCopyReceipt` proves the exact bytes staged/published, and retained `LocalAudioPublicationIdentity` represents the path-free durable evidence intended for Project Persistence. The next cross-owner step is for #970 to consume that retained identity when writing `sourceReference`; it must not reconstruct digest/path evidence from renderer JSON or re-hash the user's original media. + +After #970 persists `projectId + artifactName + extension + fileSizeBytes + contentSha256`, restart must resolve only the app-owned artifact, re-establish regular/no-link containment, bounded size/SHA-256 and applicable decode admission, reconstruct a fresh bootstrap, and only then let #1160 combine persisted `selectedPlaybackSource` intent with fresh native stem availability. Missing preferred stems fail closed to Full mix. + +When #866 enters #1160 ancestry, the private playable-stem SHA-256 implementation should be deleted in favor of `bandscope_desktop_core::sha256_hex_reader` while preserving stem identity/error tests. YouTube intake still uses its owned cache artifact and needs an explicit durable-source promotion decision. Platform-atomic no-follow acquisition and parent-directory crash durability remain Resource Admission/platform work. Issue #1129 remains the commercial decoder-dependency gate. + +## References + +National Institute of Standards and Technology. (2015). *Secure Hash Standard (SHS)* (FIPS PUB 180-4). https://doi.org/10.6028/NIST.FIPS.180-4 + +National Institute of Standards and Technology. (2023, March 7). *Decision to revise FIPS 180-4, Secure Hash Standard (SHS).* https://csrc.nist.gov/news/2023/decision-to-revise-fips-180-4 diff --git a/docs/security/app-security.md b/docs/security/app-security.md index a9983fb97..d7250bbf5 100644 --- a/docs/security/app-security.md +++ b/docs/security/app-security.md @@ -137,8 +137,11 @@ Every boundary crossing requires validation, scope restriction, minimal logging, - Cross-check extension, MIME, and actual decode behavior. - Prefer isolated worker processing for decode and analysis. - Guard against very large files, abnormal duration, and hostile metadata. +- Apply the versioned canonical local-audio resource policy consistently at request preflight and again at the opened-file/decoded-waveform boundary; request metadata is never authoritative for actual resource use. +- Before any decoder resamples, downmixes, or duration-truncates local audio, inspect source-container metadata from the already-open handle with `soundfile.info`, enforce the shared 8 kHz–192 kHz and mono/stereo source contract, reject overlong sources, and rewind the handle before `librosa.load`. +- In the Python analysis boundary, reject decoded audio that is empty, non-finite, wrong-rate, wrong-shaped, or over the accepted sample budget before beat tracking or model inference. Use the one-sample-over decode probe described in `docs/doctoring/audio-resource-policy.md` so an exact-boundary track remains accepted while excess decoded output is observable and fails closed. - Do not add arbitrary filesystem scanning just to find media files. -- When bootstrapping a project around local audio, prefer referencing the validated original file plus app-owned temp/cache/project roots over copying the file until persistence requirements justify the extra storage boundary. +- When bootstrapping a project around local audio, use the OS-selected external file only as untrusted admission input. Stage and sync admitted bytes under the app-owned project root, publish them as `source.`, then reopen and verify the published regular/non-symlink object against the bounded size and SHA-256 receipt before analysis or persistence. Do not persist an arbitrary external absolute path as authority. ### YouTube and remote URL import @@ -147,6 +150,8 @@ Every boundary crossing requires validation, scope restriction, minimal logging, - Validate scheme, host, path, and query before any fetch or handoff. - Do not widen URL intake into a generic remote downloader. - Sanitize remote metadata before display. +- Apply the same canonical 100 MiB encoded-byte ceiling during YouTube download as local-file intake. Abort with yt-dlp `max_filesize` and a progress hook, then delete owned `.part` / `.ytdl` / `-Frag*` siblings that stay inside that import directory. Do not keep a divergent post-download-only 50 MB limit that lets a large transfer fill the cache root first. +- Revalidate the filesystem-observed downloaded length before storing bootstrap state. Treat announced `filesize` / `filesize_approx` as a pre-download hint only. ### Subprocesses and native tools diff --git a/services/analysis-engine/src/bandscope_analysis/__init__.py b/services/analysis-engine/src/bandscope_analysis/__init__.py index 3867248e8..ce4beb801 100644 --- a/services/analysis-engine/src/bandscope_analysis/__init__.py +++ b/services/analysis-engine/src/bandscope_analysis/__init__.py @@ -1,6 +1,36 @@ """BandScope analysis engine package.""" -from .api import get_analysis_status +import logging +from importlib import import_module + from .health import build_health_report +_STEM_SAFE_FAILURE_LOG_MESSAGES = frozenset( + { + "Stem separation failed because the source file was missing.", + "Stem separation unavailable because Demucs or torch is not installed.", + "Stem separation rejected invalid audio source data.", + "Stem separation failed with a runtime error.", + "Stem separation failed unexpectedly.", + "Stem separation failed before analysis job completion.", + } +) + + +class _ApiDiagnosticPrivacyFilter(logging.Filter): + """Redact traceback payloads only for known stem safe-failure diagnostics.""" + + def filter(self, record: logging.LogRecord) -> bool: + """Preserve unrelated diagnostics while redacting owned safe-failure tracebacks.""" + if record.getMessage() in _STEM_SAFE_FAILURE_LOG_MESSAGES: + record.exc_info = None + record.exc_text = None + return True + + +_api_logger = logging.getLogger("bandscope_analysis.api") +_api_logger.addFilter(_ApiDiagnosticPrivacyFilter()) +_api_module = import_module(".api", __name__) +get_analysis_status = _api_module.get_analysis_status + __all__ = ["build_health_report", "get_analysis_status"] diff --git a/services/analysis-engine/src/bandscope_analysis/api.py b/services/analysis-engine/src/bandscope_analysis/api.py index b376de293..217e27a42 100644 --- a/services/analysis-engine/src/bandscope_analysis/api.py +++ b/services/analysis-engine/src/bandscope_analysis/api.py @@ -14,6 +14,7 @@ import numpy as np +from bandscope_analysis.audio_resource_policy import DEFAULT_AUDIO_RESOURCE_POLICY from bandscope_analysis.health import HealthReport, build_health_report from bandscope_analysis.roles import RoleExtractor from bandscope_analysis.sections import extract_sections @@ -306,8 +307,12 @@ def validate_analysis_job_request(payload: object) -> AnalysisJobRequest: raise ValueError("Invalid analysis job request: invalid field 'localSource.fileName'") if extension not in {"wav", "mp3", "flac", "m4a"}: raise ValueError("Invalid analysis job request: invalid field 'localSource.extension'") - if not isinstance(file_size_bytes, int) or file_size_bytes <= 0: - raise ValueError("Invalid analysis job request: invalid field 'localSource.fileSizeBytes'") + try: + file_size_bytes = DEFAULT_AUDIO_RESOURCE_POLICY.validate_encoded_file_bytes(file_size_bytes) + except ValueError as error: + raise ValueError( + "Invalid analysis job request: invalid field 'localSource.fileSizeBytes'" + ) from error normalized: AnalysisJobRequest = { "sourceKind": source_kind, diff --git a/services/analysis-engine/src/bandscope_analysis/audio_decode.py b/services/analysis-engine/src/bandscope_analysis/audio_decode.py new file mode 100644 index 000000000..e558ac8b2 --- /dev/null +++ b/services/analysis-engine/src/bandscope_analysis/audio_decode.py @@ -0,0 +1,76 @@ +"""Bounded PCM decode port for the Resource Admission & Decode context. + +The current adapter still delegates to ``librosa`` and therefore remains a +transitional boundary while #1129 removes the libsndfile-backed runtime graph. +Consumers must call this port rather than selecting decoder fallbacks +independently. + +Security Notes: +- The caller-authorized binary handle, container metadata, decoder output, and + third-party decoder exceptions are untrusted. +- Source metadata is admitted before decode and the resulting PCM is revalidated + against the same versioned policy before it can enter MIR or model work. +- Decoder details remain exception causes only; the surfaced failure is the + payload-free canonical resource-policy error. +- This port adds no path, network, subprocess, or credential authority. +""" + +from __future__ import annotations + +import warnings +from typing import BinaryIO, cast + +import librosa +import numpy as np +from numpy.typing import NDArray + +from bandscope_analysis.audio_metadata import preflight_audio_metadata +from bandscope_analysis.audio_resource_policy import ( + DEFAULT_AUDIO_RESOURCE_POLICY, + AudioResourcePolicy, + AudioResourcePolicyError, +) + +AudioMonoArray = NDArray[np.float32] + + +def _malformed_decode_error() -> AudioResourcePolicyError: + """Build the stable payload-free decoder failure.""" + return AudioResourcePolicyError("malformed_header") + + +def decode_mono_audio( + source: BinaryIO, + *, + policy: AudioResourcePolicy = DEFAULT_AUDIO_RESOURCE_POLICY, +) -> tuple[AudioMonoArray, int]: + """Admit and decode one caller-owned source to bounded mono float32 PCM.""" + preflight_audio_metadata(source, policy) + + try: + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", category=DeprecationWarning, module=r"^audioread") + warnings.filterwarnings("ignore", category=FutureWarning, module=r"^audioread") + decoded, sample_rate = librosa.load( + source, + sr=policy.target_sample_rate, + mono=True, + duration=policy.decode_probe_duration_seconds, + ) + except AudioResourcePolicyError: + raise + except Exception as error: + raise _malformed_decode_error() from error + + try: + pcm = np.ravel(np.asarray(decoded, dtype=np.float32)) + except (OverflowError, TypeError, ValueError) as error: + raise _malformed_decode_error() from error + + try: + policy.validate_decoded_audio(pcm, sample_rate) + except AudioResourcePolicyError: + raise + except Exception as error: + raise _malformed_decode_error() from error + return cast(AudioMonoArray, pcm), int(sample_rate) diff --git a/services/analysis-engine/src/bandscope_analysis/audio_metadata.py b/services/analysis-engine/src/bandscope_analysis/audio_metadata.py new file mode 100644 index 000000000..51e0a6176 --- /dev/null +++ b/services/analysis-engine/src/bandscope_analysis/audio_metadata.py @@ -0,0 +1,43 @@ +"""Bounded source-container metadata preflight for local audio decoders. + +Security Notes: +- The selected audio bytes and container headers are untrusted. +- This module reads metadata from an already-open caller-owned handle only; it + does not open paths, decode PCM, follow URLs, or allocate a waveform. +- Malformed headers, unsupported source rates/channels, and overlong sources + fail closed with the payload-free canonical policy error. +- A successful probe rewinds the handle so the downstream decoder receives the + same source from its beginning. +""" + +from __future__ import annotations + +from typing import BinaryIO + +import soundfile # type: ignore[import-untyped] # soundfile has no py.typed marker. + +from bandscope_analysis.audio_resource_policy import ( + DEFAULT_AUDIO_RESOURCE_POLICY, + AudioResourcePolicy, + AudioResourcePolicyError, +) + + +def preflight_audio_metadata( + fileobj: BinaryIO, + policy: AudioResourcePolicy = DEFAULT_AUDIO_RESOURCE_POLICY, +) -> None: + """Validate source metadata without decoding PCM and rewind the handle.""" + try: + fileobj.seek(0) + info = soundfile.info(fileobj) + fileobj.seek(0) + policy.validate_source_metadata( + frames=info.frames, + sample_rate=info.samplerate, + channels=info.channels, + ) + except AudioResourcePolicyError: + raise + except Exception as error: + raise AudioResourcePolicyError("malformed_header") from error diff --git a/services/analysis-engine/src/bandscope_analysis/audio_resource_policy.py b/services/analysis-engine/src/bandscope_analysis/audio_resource_policy.py new file mode 100644 index 000000000..5badde63b --- /dev/null +++ b/services/analysis-engine/src/bandscope_analysis/audio_resource_policy.py @@ -0,0 +1,276 @@ +"""Canonical resource admission policy for local audio analysis. + +The policy is intentionally independent of individual analyzers. Expensive +feature code consumes a decoded artifact only after encoded-file and decoded +output checks agree on the same versioned limits. This prevents temporal, +separation, chord, and register features from silently inventing incompatible +resource ceilings. + +Security Notes: +- Encoded byte counts are validated before decode/allocation work when the + opened file descriptor can provide an authoritative size. +- Decoded audio is revalidated because container metadata and decoder behavior + are untrusted; accepted artifacts are finite, mono, floating-point, at the + configured sample rate, and within configured sample and memory budgets. +- Decoders receive a one-sample-over-budget probe duration so a longer source is + rejected instead of being silently truncated to the accepted duration. +- Policy arithmetic rejects unrepresentable limits before float/sample-count + conversion so malformed configuration cannot escape the stable failure mode. +- Resource rejections expose only a stable reason and policy version; messages + remain payload-free and never include source paths or audio content. +""" + +from __future__ import annotations + +import math +import sys +from dataclasses import dataclass +from typing import Any, NoReturn, cast + +import numpy as np +from numpy.typing import NDArray + +AUDIO_RESOURCE_POLICY_VERSION = "1" +DEFAULT_TARGET_SAMPLE_RATE = 44_100 +DEFAULT_MIN_SOURCE_SAMPLE_RATE = 8_000 +DEFAULT_MAX_SOURCE_SAMPLE_RATE = 192_000 +DEFAULT_MIN_SOURCE_CHANNELS = 1 +DEFAULT_MAX_SOURCE_CHANNELS = 2 +DEFAULT_MAX_ENCODED_FILE_BYTES = 100 * 1024 * 1024 +DEFAULT_MAX_DURATION_SECONDS = 15 * 60 +DEFAULT_MAX_DECODED_AUDIO_BYTES = ( + DEFAULT_TARGET_SAMPLE_RATE * DEFAULT_MAX_DURATION_SECONDS * np.dtype(np.float64).itemsize +) +_POLICY_ERROR = "Audio input violates the audio resource policy." + + +class AudioResourcePolicyError(ValueError): + """Payload-free resource rejection with stable machine-readable provenance.""" + + def __init__(self, reason: str) -> None: + """Record a stable rejection reason and the policy version that produced it.""" + super().__init__(_POLICY_ERROR) + self.reason = reason + self.policy_version = AUDIO_RESOURCE_POLICY_VERSION + + +def _reject(reason: str) -> NoReturn: + """Fail closed without echoing untrusted resource metadata.""" + raise AudioResourcePolicyError(reason) + + +@dataclass(frozen=True) +class AudioResourcePolicy: + """Versioned limits applied before and after local audio decoding. + + Args: + max_encoded_file_bytes: Maximum non-empty encoded source size. + target_sample_rate: Required sample rate of the canonical decoded mono + artifact. + max_duration_seconds: Maximum decoded duration represented as a sample + ceiling at ``target_sample_rate``. + max_decoded_audio_bytes: Maximum in-memory byte size of the canonical + decoded mono NumPy buffer. + min_source_sample_rate: Minimum source-container sample rate accepted + before resampling. + max_source_sample_rate: Maximum source-container sample rate accepted + before resampling. + min_source_channels: Minimum source-container channel count accepted + before downmixing. + max_source_channels: Maximum source-container channel count accepted + before downmixing. + """ + + max_encoded_file_bytes: int = DEFAULT_MAX_ENCODED_FILE_BYTES + target_sample_rate: int = DEFAULT_TARGET_SAMPLE_RATE + max_duration_seconds: float = float(DEFAULT_MAX_DURATION_SECONDS) + max_decoded_audio_bytes: int = DEFAULT_MAX_DECODED_AUDIO_BYTES + min_source_sample_rate: int = DEFAULT_MIN_SOURCE_SAMPLE_RATE + max_source_sample_rate: int = DEFAULT_MAX_SOURCE_SAMPLE_RATE + min_source_channels: int = DEFAULT_MIN_SOURCE_CHANNELS + max_source_channels: int = DEFAULT_MAX_SOURCE_CHANNELS + + def __post_init__(self) -> None: + """Reject invalid policy configuration before it can weaken admission.""" + if ( + isinstance(self.max_encoded_file_bytes, bool) + or not isinstance(self.max_encoded_file_bytes, int) + or self.max_encoded_file_bytes <= 0 + or self.max_encoded_file_bytes > sys.maxsize - 1 + ): + raise ValueError(_POLICY_ERROR) + if ( + isinstance(self.target_sample_rate, bool) + or not isinstance(self.target_sample_rate, int) + or self.target_sample_rate <= 0 + or self.target_sample_rate > sys.maxsize - 1 + ): + raise ValueError(_POLICY_ERROR) + if isinstance(self.max_duration_seconds, bool) or not isinstance( + self.max_duration_seconds, int | float + ): + raise ValueError(_POLICY_ERROR) + if ( + isinstance(self.max_decoded_audio_bytes, bool) + or not isinstance(self.max_decoded_audio_bytes, int) + or self.max_decoded_audio_bytes <= 0 + or self.max_decoded_audio_bytes > sys.maxsize - 1 + ): + raise ValueError(_POLICY_ERROR) + for source_bound in ( + self.min_source_sample_rate, + self.max_source_sample_rate, + self.min_source_channels, + self.max_source_channels, + ): + if ( + isinstance(source_bound, bool) + or not isinstance(source_bound, int) + or source_bound <= 0 + or source_bound > sys.maxsize - 1 + ): + raise ValueError(_POLICY_ERROR) + if ( + self.min_source_sample_rate > self.max_source_sample_rate + or self.min_source_channels > self.max_source_channels + ): + raise ValueError(_POLICY_ERROR) + try: + duration_seconds = float(self.max_duration_seconds) + except (OverflowError, ValueError): + raise ValueError(_POLICY_ERROR) from None + if not math.isfinite(duration_seconds) or duration_seconds <= 0.0: + raise ValueError(_POLICY_ERROR) + decoded_samples = self.target_sample_rate * duration_seconds + if ( + not math.isfinite(decoded_samples) + or decoded_samples < 1.0 + or decoded_samples > sys.maxsize - 1 + ): + raise ValueError(_POLICY_ERROR) + + @property + def max_decoded_samples(self) -> int: + """Return the maximum mono sample count allowed after decoding.""" + return int(self.target_sample_rate * float(self.max_duration_seconds)) + + @property + def decode_probe_duration_seconds(self) -> float: + """Return a bounded decoder duration that includes one rejection probe sample.""" + return (self.max_decoded_samples + 1) / self.target_sample_rate + + def validate_encoded_file_bytes(self, file_size: object) -> int: + """Validate an authoritative encoded file size before decoding. + + Args: + file_size: Byte count obtained from the already-open source file. + + Returns: + The validated integer byte count. + + Raises: + AudioResourcePolicyError: If the value is not a positive integer + within policy. + """ + if isinstance(file_size, bool) or not isinstance(file_size, int) or file_size <= 0: + _reject("malformed_header") + if file_size > self.max_encoded_file_bytes: + _reject("encoded_file_too_large") + return file_size + + def validate_source_metadata( + self, + frames: object, + sample_rate: object, + channels: object, + ) -> None: + """Validate source-container metadata before any decode transformation. + + Args: + frames: Number of source frames reported by the container parser. + sample_rate: Source sample rate in Hz before resampling. + channels: Source channel count before downmixing. + + Raises: + AudioResourcePolicyError: If metadata is malformed or outside the + source bounds. + """ + if isinstance(frames, bool) or not isinstance(frames, int) or frames <= 0: + _reject("malformed_header") + if ( + isinstance(sample_rate, bool) + or not isinstance(sample_rate, int) + or sample_rate < self.min_source_sample_rate + or sample_rate > self.max_source_sample_rate + ): + _reject("sampling_rate_unsupported") + if ( + isinstance(channels, bool) + or not isinstance(channels, int) + or channels < self.min_source_channels + or channels > self.max_source_channels + ): + _reject("channel_count_unsupported") + try: + source_duration_seconds = float(frames) / float(sample_rate) + except (OverflowError, ValueError): + _reject("malformed_header") + if source_duration_seconds > float(self.max_duration_seconds): + _reject("duration_exceeded") + + def validate_decoded_audio( + self, + audio: object, + sample_rate: object, + ) -> NDArray[np.floating[Any]]: + """Revalidate the canonical decoded artifact before feature analysis. + + Args: + audio: Candidate mono NumPy array returned by the decoder. + sample_rate: Decoder-reported sample rate in Hz. + + Returns: + The original validated NumPy floating-point array without copying it. + + Raises: + AudioResourcePolicyError: If dtype, shape, sample rate, sample + count, memory use, or finiteness does not satisfy this policy. + """ + if ( + not isinstance(audio, np.ndarray) + or audio.ndim != 1 + or audio.size == 0 + or not np.issubdtype(audio.dtype, np.floating) + ): + _reject("malformed_header") + if ( + isinstance(sample_rate, bool) + or not isinstance(sample_rate, int) + or sample_rate != self.target_sample_rate + ): + _reject("sampling_rate_unsupported") + if audio.size > self.max_decoded_samples: + _reject("decoded_sample_count_exceeded") + if audio.nbytes > self.max_decoded_audio_bytes: + _reject("memory_budget_exceeded") + if not np.isfinite(audio).all(): + _reject("malformed_header") + return cast(NDArray[np.floating[Any]], audio) + + +DEFAULT_AUDIO_RESOURCE_POLICY = AudioResourcePolicy() + +__all__ = [ + "AUDIO_RESOURCE_POLICY_VERSION", + "AudioResourcePolicy", + "AudioResourcePolicyError", + "DEFAULT_AUDIO_RESOURCE_POLICY", + "DEFAULT_MAX_DECODED_AUDIO_BYTES", + "DEFAULT_MAX_DURATION_SECONDS", + "DEFAULT_MAX_ENCODED_FILE_BYTES", + "DEFAULT_MAX_SOURCE_CHANNELS", + "DEFAULT_MAX_SOURCE_SAMPLE_RATE", + "DEFAULT_MIN_SOURCE_CHANNELS", + "DEFAULT_MIN_SOURCE_SAMPLE_RATE", + "DEFAULT_TARGET_SAMPLE_RATE", +] diff --git a/services/analysis-engine/src/bandscope_analysis/chords/chord_recognizer.py b/services/analysis-engine/src/bandscope_analysis/chords/chord_recognizer.py index 8f6466924..2d414afc8 100644 --- a/services/analysis-engine/src/bandscope_analysis/chords/chord_recognizer.py +++ b/services/analysis-engine/src/bandscope_analysis/chords/chord_recognizer.py @@ -396,7 +396,7 @@ def recognize(self, y: np.ndarray, sr: int = 22050) -> list[TrackedChord]: Returns: List of TrackedChord dicts with start_time, end_time, chord, and confidence. """ - if len(y) == 0: + if y.size == 0: return [] y_harmonic = self._separate_harmonic(y) diff --git a/services/analysis-engine/src/bandscope_analysis/separation/audio_separator.py b/services/analysis-engine/src/bandscope_analysis/separation/audio_separator.py index c36e0f1fc..2507bf6cf 100644 --- a/services/analysis-engine/src/bandscope_analysis/separation/audio_separator.py +++ b/services/analysis-engine/src/bandscope_analysis/separation/audio_separator.py @@ -9,9 +9,16 @@ Security Notes: - Treats the selected audio file as untrusted input: the path is normalized and verified to be a file, and a maximum byte size is enforced before decode. -- Inference runs locally on CPU with no network access. The model weights are - loaded from the local Demucs cache or a configured bundled path; offline - weight bundling is tracked in the supplemental component inventory. +- Decoded audio is revalidated against the same versioned resource policy before + Demucs/model work so overlong, malformed, or non-finite decoder output fails + closed instead of being silently truncated or normalized. +- Empty, non-finite, or float32-overflowed model stems fail closed before they + can become successful silence or downstream rehearsal evidence. +- Inference runs locally with no network access. Accelerator outputs cross back + to CPU before NumPy conversion so configured device execution cannot fail at + the device/host boundary. The model weights are loaded from the local Demucs + cache or a configured bundled path; offline weight bundling is tracked in the + supplemental component inventory. - Does not log or persist raw audio, separated stems, or full source paths. - Fails with bounded, filename-scoped errors so callers can surface a safe failure without leaking local directory structure. @@ -23,20 +30,18 @@ import logging import os import sys -import warnings from dataclasses import dataclass from pathlib import Path from typing import Any, cast -import librosa import numpy as np -from bandscope_analysis.temporal.analyzer import ( - KNOWN_LIBROSA_NUMBA_WARNING_FILTERS, - MAX_ANALYSIS_DURATION_SECONDS, - MAX_AUDIO_FILE_BYTES, - TARGET_SR, +from bandscope_analysis.audio_decode import decode_mono_audio +from bandscope_analysis.audio_resource_policy import ( + DEFAULT_MAX_DURATION_SECONDS, + AudioResourcePolicy, ) +from bandscope_analysis.temporal.analyzer import MAX_AUDIO_FILE_BYTES, TARGET_SR from .model import AudioSeparationResult, AudioStemArray, AudioStemName, AudioStemPayload @@ -45,6 +50,7 @@ # Demucs htdemucs emits these four sources; this is the canonical stem set. _STEM_ORDER: tuple[AudioStemName, ...] = ("vocals", "bass", "drums", "other") _EMPTY_RANGE_EPS = 1e-9 +_MODEL_OUTPUT_ERROR = "Stem separation produced invalid audio." def _contains_parent_path_segment(path: Path) -> bool: @@ -63,7 +69,7 @@ class AudioSeparationConfig: target_sample_rate: int = TARGET_SR max_file_bytes: int = MAX_AUDIO_FILE_BYTES - max_duration_seconds: float = float(MAX_ANALYSIS_DURATION_SECONDS) + max_duration_seconds: float = float(DEFAULT_MAX_DURATION_SECONDS) model_name: str = "htdemucs" device: str = "cpu" # Demucs splits long audio into overlapping segments internally, bounding @@ -75,8 +81,13 @@ class AudioStemSeparator: """Split a selected local mix into canonical stems for downstream analysis.""" def __init__(self, config: AudioSeparationConfig | None = None) -> None: - """Initialize the local stem separator (model is loaded lazily).""" + """Initialize the local stem separator and its canonical resource policy.""" self.config = config or AudioSeparationConfig() + self.resource_policy = AudioResourcePolicy( + max_encoded_file_bytes=self.config.max_file_bytes, + target_sample_rate=self.config.target_sample_rate, + max_duration_seconds=self.config.max_duration_seconds, + ) self._model: Any = None def separate(self, audio_path: str | Path) -> AudioSeparationResult: @@ -119,8 +130,8 @@ def _separate_signal( """Run the Demucs model on mono audio and return canonical mono stems. This is the single boundary to the neural model; it converts the mono - signal to the stereo tensor Demucs expects, applies the model on CPU, and - downmixes each source back to a mono float array. + signal to the stereo tensor Demucs expects, applies the model on the + configured device, and downmixes each source back to a mono host array. """ model = self._load_model() sources = self._apply_model(model, audio) @@ -172,7 +183,13 @@ def _apply_model(self, model: Any, audio: AudioStemArray) -> dict[str, np.ndarra progress=False, )[0] out = out * ref_std + ref_mean - return {name: out[i].mean(0).numpy() for i, name in enumerate(model.sources)} + stems: dict[str, np.ndarray[Any, Any]] = {} + for index, name in enumerate(model.sources): + stem = out[index].mean(0) + if self.config.device != "cpu": + stem = stem.cpu() + stems[name] = stem.numpy() + return stems def _resolve_audio_file(self, audio_path: str | Path) -> Path: """Normalize and validate the selected source path.""" @@ -190,39 +207,24 @@ def _resolve_audio_file(self, audio_path: str | Path) -> Path: return path def _load_audio(self, path: Path) -> tuple[AudioStemArray, int]: - """Load bounded mono audio without logging or exposing the full source path.""" + """Load bounded mono audio through the canonical decoder authority.""" try: with path.open("rb") as fileobj: file_size = os.fstat(fileobj.fileno()).st_size - if file_size > self.config.max_file_bytes: - raise ValueError( - "Audio file is too large for stem separation: " - f"{file_size} bytes (max {self.config.max_file_bytes} bytes)" - ) - - with warnings.catch_warnings(): - warnings.filterwarnings( - "ignore", category=DeprecationWarning, module=r"^audioread" - ) - warnings.filterwarnings("ignore", category=FutureWarning, module=r"^audioread") - for category, message, module in KNOWN_LIBROSA_NUMBA_WARNING_FILTERS: - warnings.filterwarnings( - "ignore", - category=category, - message=message, - module=module, - ) - y, sr = librosa.load( - fileobj, - sr=self.config.target_sample_rate, - mono=True, - duration=self.config.max_duration_seconds, - ) + if file_size <= 0: + raise ValueError(f"Stem separation decode failed for {path.name}") + try: + self.resource_policy.validate_encoded_file_bytes(file_size) + except ValueError as error: + raise ValueError("Audio file is too large for stem separation") from error + y, sr = decode_mono_audio(fileobj, policy=self.resource_policy) except ValueError: raise except Exception as error: raise ValueError(f"Stem separation decode failed for {path.name}") from error + if y.size == 0: + raise ValueError(f"Stem separation decode failed for {path.name}") return _as_float_array(y), int(sr) def _fit_length(self, audio: AudioStemArray, target_length: int) -> AudioStemArray: @@ -235,7 +237,12 @@ def _fit_length(self, audio: AudioStemArray, target_length: int) -> AudioStemArr def _as_float_array(values: object) -> AudioStemArray: - """Convert decoder and model output to a finite one-dimensional float array.""" - array = np.ravel(np.asarray(values, dtype=np.float32)) - finite = np.nan_to_num(array, copy=False, nan=0.0, posinf=0.0, neginf=0.0) - return cast(AudioStemArray, finite) + """Convert one finite, non-empty decoder/model output into mono float32 audio.""" + try: + with np.errstate(over="ignore", invalid="ignore"): + array = np.ravel(np.asarray(values, dtype=np.float32)) + except (OverflowError, TypeError, ValueError) as error: + raise ValueError(_MODEL_OUTPUT_ERROR) from error + if array.size == 0 or not np.isfinite(array).all(): + raise ValueError(_MODEL_OUTPUT_ERROR) + return cast(AudioStemArray, array) \ No newline at end of file diff --git a/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py b/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py index 7fe5ae6f7..584ef6c99 100644 --- a/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py +++ b/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py @@ -4,7 +4,6 @@ import logging import os -import warnings from pathlib import Path from typing import Any @@ -12,18 +11,36 @@ import numpy as np from numpy.typing import NDArray +from bandscope_analysis.audio_decode import decode_mono_audio +from bandscope_analysis.audio_resource_policy import ( + DEFAULT_AUDIO_RESOURCE_POLICY, + DEFAULT_MAX_DURATION_SECONDS, + AudioResourcePolicy, +) + from .model import TemporalFeatures logger = logging.getLogger(__name__) -# Standard sample rate for BandScope analysis -TARGET_SR = 44100 -MAX_AUDIO_FILE_BYTES = 100 * 1024 * 1024 # 100 MiB -MAX_ANALYSIS_DURATION_SECONDS = 15 * 60 # 15 minutes +# Compatibility aliases retained for callers/tests while the canonical values +# are owned by AudioResourcePolicy. The decode-duration alias intentionally +# includes one rejection-probe sample so an overlong source is detected rather +# than silently truncated at the accepted rehearsal duration. +TARGET_SR = DEFAULT_AUDIO_RESOURCE_POLICY.target_sample_rate +MAX_AUDIO_FILE_BYTES = DEFAULT_AUDIO_RESOURCE_POLICY.max_encoded_file_bytes +MAX_ANALYSIS_DURATION_SECONDS = DEFAULT_AUDIO_RESOURCE_POLICY.decode_probe_duration_seconds KNOWN_LIBROSA_NUMBA_WARNING_FILTERS = ( (DeprecationWarning, r".*pkg_resources is deprecated.*", r".*librosa.*"), (FutureWarning, r".*Numba.*", r".*numba.*"), ) +_SAFE_TEMPORAL_FAILURE_MESSAGES = frozenset( + { + "Audio file is too large for temporal analysis", + "Audio input violates the audio resource policy.", + } +) +_MISSING_AUDIO_MESSAGE = "Audio source is unavailable for temporal analysis." +_GENERIC_TEMPORAL_FAILURE_MESSAGE = "Temporal analysis failed." # ponytail: assumes 4/4; upgrade to meter estimation or a madmom DBN if other meters matter. BEATS_PER_BAR = 4 @@ -56,11 +73,34 @@ def _estimate_downbeats( return [float(bt) for i, bt in enumerate(beat_times) if (i - best_phase) % beats_per_bar == 0] +def _safe_temporal_failure_message(error: Exception) -> str: + """Return an allowlisted diagnostic without relaying decoder payload text.""" + message = str(error) + if message in _SAFE_TEMPORAL_FAILURE_MESSAGES: + return message + return _GENERIC_TEMPORAL_FAILURE_MESSAGE + + class TemporalAnalyzer: - """Analyzes temporal features (BPM, beats) from audio files.""" + """Analyze bounded temporal features (BPM and beat grids) from local audio.""" + + def __init__(self, resource_policy: AudioResourcePolicy | None = None) -> None: + """Create an analyzer bound to one canonical local-audio resource policy. + + Args: + resource_policy: Explicit policy for tests or specialized callers. + The default preserves the public module-level byte ceiling while + taking sample-rate and accepted rehearsal duration from the + canonical policy layer. + """ + self.resource_policy = resource_policy or AudioResourcePolicy( + max_encoded_file_bytes=MAX_AUDIO_FILE_BYTES, + target_sample_rate=TARGET_SR, + max_duration_seconds=DEFAULT_MAX_DURATION_SECONDS, + ) def analyze(self, audio_path: str | Path) -> TemporalFeatures: - """Decode audio and extract temporal features. + """Decode bounded audio and extract temporal features. Args: audio_path: Path to the audio file. @@ -71,54 +111,23 @@ def analyze(self, audio_path: str | Path) -> TemporalFeatures: path = Path(audio_path) path_str = str(path) if not path.exists() or not path.is_file(): - raise FileNotFoundError(f"Audio file not found: {path_str}") + raise FileNotFoundError(_MISSING_AUDIO_MESSAGE) - logger.info(f"Loading and decoding audio: {path_str}") + logger.info("Loading and decoding bounded local audio.") try: with path.open("rb") as fileobj: file_size = os.fstat(fileobj.fileno()).st_size - if file_size > MAX_AUDIO_FILE_BYTES: - raise ValueError( - f"Audio file is too large for temporal analysis: {file_size} bytes " - f"(max {MAX_AUDIO_FILE_BYTES} bytes)" - ) - - with warnings.catch_warnings(): - warnings.filterwarnings( - "ignore", category=DeprecationWarning, module=r"^audioread" - ) - warnings.filterwarnings("ignore", category=FutureWarning, module=r"^audioread") - - # Keep the loader's known third-party churn quiet without hiding - # unrelated decoder warnings that tests and callers should see. - for category, message, module in KNOWN_LIBROSA_NUMBA_WARNING_FILTERS: - warnings.filterwarnings( - "ignore", - category=category, - message=message, - module=module, - ) - # Load audio, converting to mono and standardizing sample rate - y, sr = librosa.load( - fileobj, - sr=TARGET_SR, - mono=True, - duration=MAX_ANALYSIS_DURATION_SECONDS, - ) - - # Ensure it's a 1D float array for librosa - if not isinstance(y, np.ndarray): - raise ValueError("Expected numpy array from librosa.load") - - y_array: NDArray[np.floating[Any]] = y + try: + self.resource_policy.validate_encoded_file_bytes(file_size) + except ValueError as error: + raise ValueError("Audio file is too large for temporal analysis") from error + y_array, sr = decode_mono_audio(fileobj, policy=self.resource_policy) + duration = float(librosa.get_duration(y=y_array, sr=sr)) logger.info("Extracting tempo and beat tracking...") - # Use librosa's robust beat tracker tempo, beat_frames = librosa.beat.beat_track(y=y_array, sr=sr) - - # Convert frame indices to time (seconds) beat_times: NDArray[np.floating[Any]] = librosa.frames_to_time(beat_frames, sr=sr) # Place downbeats on the strongest-onset bar phase (looks at the audio, @@ -139,6 +148,6 @@ def analyze(self, audio_path: str | Path) -> TemporalFeatures: "audio_path": path_str, } - except Exception as e: - logger.error(f"Failed to analyze audio {path_str}: {e}") - raise ValueError(f"Temporal analysis failed: {e}") from e + except Exception as error: + logger.error("Temporal analysis failed (%s).", type(error).__name__) + raise ValueError(_safe_temporal_failure_message(error)) from error \ No newline at end of file diff --git a/services/analysis-engine/src/bandscope_analysis/transcription/api.py b/services/analysis-engine/src/bandscope_analysis/transcription/api.py index f2a732d31..8be079c08 100644 --- a/services/analysis-engine/src/bandscope_analysis/transcription/api.py +++ b/services/analysis-engine/src/bandscope_analysis/transcription/api.py @@ -3,13 +3,15 @@ from __future__ import annotations import io -import warnings from dataclasses import dataclass import librosa import numpy as np from numpy.typing import NDArray +from bandscope_analysis.audio_decode import decode_mono_audio +from bandscope_analysis.audio_resource_policy import AudioResourcePolicy + TARGET_SR = 22050 MAX_STEM_BYTES = 50 * 1024 * 1024 MAX_TRANSCRIPTION_DURATION_SECONDS = 120 @@ -17,6 +19,12 @@ HOP_LENGTH = 512 MIN_NOTE_DURATION_SECONDS = 0.05 MIN_SIGNAL_PEAK = 1e-5 +TRANSCRIPTION_RESOURCE_POLICY = AudioResourcePolicy( + max_encoded_file_bytes=MAX_STEM_BYTES, + target_sample_rate=TARGET_SR, + max_duration_seconds=MAX_TRANSCRIPTION_DURATION_SECONDS, + max_decoded_audio_bytes=(TARGET_SR * MAX_TRANSCRIPTION_DURATION_SECONDS + 1) * 8, +) @dataclass @@ -42,16 +50,8 @@ def transcribe_bass_stem(stem_data: bytes) -> list[NoteEvent]: if len(stem_data) > MAX_STEM_BYTES: raise ValueError("Stem data is too large for transcription.") - with warnings.catch_warnings(): - warnings.filterwarnings("ignore", category=DeprecationWarning, module=r"^audioread") - y, sr = librosa.load( - io.BytesIO(stem_data), - sr=TARGET_SR, - mono=True, - duration=MAX_TRANSCRIPTION_DURATION_SECONDS, - ) - - y_array = np.asarray(y, dtype=np.float32) + source = io.BytesIO(stem_data) + y_array, sr = decode_mono_audio(source, policy=TRANSCRIPTION_RESOURCE_POLICY) if y_array.size == 0 or float(np.max(np.abs(y_array))) < MIN_SIGNAL_PEAK: return [] @@ -171,4 +171,4 @@ def _merge_adjacent_equal_pitches(events: list[NoteEvent]) -> list[NoteEvent]: ) else: merged.append(event) - return merged + return merged \ No newline at end of file diff --git a/services/analysis-engine/src/bandscope_analysis/youtube.py b/services/analysis-engine/src/bandscope_analysis/youtube.py index c98f4e513..61c6bab0b 100644 --- a/services/analysis-engine/src/bandscope_analysis/youtube.py +++ b/services/analysis-engine/src/bandscope_analysis/youtube.py @@ -1,11 +1,35 @@ -""" -YouTube import capabilities for BandScope. +"""YouTube import capabilities for BandScope. This module provides a safe wrapper around yt-dlp to download audio from YouTube. + +Security Notes: + - URL intake remains host/path/query allowlisted before any network work. + - Encoded-byte admission uses the same canonical 100 MiB policy as local + audio. yt-dlp ``max_filesize`` and a progress hook abort in-flight + transfers so a multi-gigabyte download cannot fill the cache root before + the post-download check runs. + - Announced duration must be a finite positive non-Boolean number when + present; malformed known-duration metadata fails closed before download. + Download-result duration is revalidated before success so changed + metadata cannot bypass the same 15-minute admission boundary. + - Announced ``filesize`` / ``filesize_approx`` values over the policy + ceiling reject the import before ``download=True``. + - The completed download path must resolve beneath this import's ``out_dir`` + before post-download size checks, cleanup, or success metadata can use it. + - The opened-file size is revalidated with ``AudioResourcePolicy`` after + download; oversize artifacts are deleted. + - In-flight abort deletes owned ``tmpfilename`` / ``filename`` siblings + (``.part``, ``.ytdl``, ``-Frag*``) that stay inside this import's + ``out_dir``. Paths that escape the directory are ignored. + - Validation errors are payload-free and never include source paths, URLs, + cookies, or audio content. """ +from __future__ import annotations + import argparse import json +import math import os import re import sys @@ -14,6 +38,12 @@ import yt_dlp # type: ignore +from bandscope_analysis.audio_resource_policy import ( + DEFAULT_AUDIO_RESOURCE_POLICY, + DEFAULT_MAX_DURATION_SECONDS, + DEFAULT_MAX_ENCODED_FILE_BYTES, +) + YOUTUBE_VIDEO_ID_PATTERN = re.compile(r"^[A-Za-z0-9_-]{11}$") MAX_YOUTUBE_URL_LENGTH = 2000 SUPPORTED_AUDIO_EXTENSIONS = (".opus", ".m4a", ".mp3", ".wav", ".aac", ".flac", ".ogg") @@ -21,6 +51,22 @@ "Failed to download audio from YouTube. Please use a local audio file instead." ) YOUTUBE_IMPORT_FAILED_MESSAGE = "YouTube import failed. Please use a local audio file instead." +YOUTUBE_SIZE_EXCEEDED_MESSAGE = "Selected audio file exceeds the 100 MiB analysis limit." + + +class YoutubeResourceLimitError(Exception): + """Fail-closed YouTube admission error that never includes payload paths.""" + + def __init__(self, code: str, message: str) -> None: + """Store a payload-safe public error code and next-action message. + + Args: + code: Stable machine-readable error code. + message: User-facing instruction that omits paths and URLs. + """ + super().__init__(message) + self.code = code + self.message = message def validate_url(url: str) -> bool: @@ -72,9 +118,203 @@ def _find_downloaded_file(actual_filepath: str) -> Optional[str]: return actual_filepath +def _size_exceeded_result() -> Dict[str, Any]: + """Return the payload-safe oversize result shared by every admission path.""" + return { + "ok": False, + "error": { + "code": "size_exceeded", + "message": YOUTUBE_SIZE_EXCEEDED_MESSAGE, + }, + } + + +def _download_error_result() -> Dict[str, Any]: + """Return the payload-safe generic import failure result.""" + return { + "ok": False, + "error": {"code": "download_error", "message": YOUTUBE_IMPORT_FAILED_MESSAGE}, + } + + +def _reject_invalid_or_oversize_duration(info: dict[str, Any]) -> Dict[str, Any] | None: + """Validate announced duration before authorizing download work. + + Args: + info: Metadata dictionary from yt-dlp extraction. + + Returns: + A payload-safe failure for malformed/over-budget known duration, or + ``None`` when duration is absent or valid and within policy. + """ + duration = info.get("duration") + if duration is None: + return None + if type(duration) not in (int, float): + return _download_error_result() + duration_seconds = float(duration) + if not math.isfinite(duration_seconds) or duration_seconds <= 0.0: + return _download_error_result() + if duration_seconds > DEFAULT_MAX_DURATION_SECONDS: + return { + "ok": False, + "error": { + "code": "duration_exceeded", + "message": "Video exceeds the 15-minute limit.", + }, + } + return None + + +def _announced_size_exceeds_policy(announced: object) -> bool: + """Return whether yt-dlp metadata already reports an over-budget file. + + Args: + announced: Candidate ``filesize`` or ``filesize_approx`` value. + + Returns: + True when the value is a finite number strictly above the policy ceiling. + """ + if isinstance(announced, bool) or not isinstance(announced, int | float): + return False + if isinstance(announced, float) and not math.isfinite(announced): + return False + size_bytes: int | float = announced + return bool(size_bytes > DEFAULT_MAX_ENCODED_FILE_BYTES) + + +def _reject_announced_oversize(info: dict[str, Any]) -> Dict[str, Any] | None: + """Reject before download when extract_info already announced oversize bytes. + + Args: + info: Metadata dictionary from ``extract_info(..., download=False)``. + + Returns: + The size-exceeded result, or ``None`` when download may proceed. + """ + if _announced_size_exceeds_policy(info.get("filesize")) or _announced_size_exceeds_policy( + info.get("filesize_approx") + ): + return _size_exceeded_result() + return None + + +def _owned_file_path(path: object, out_dir: str) -> str | None: + """Return a real path only when it stays inside this import's output directory. + + Args: + path: Candidate filesystem path from yt-dlp status or sibling lookup. + out_dir: Directory passed to this import call. + + Returns: + The resolved file path, or ``None`` when the value is unsafe or foreign. + """ + if not isinstance(path, str) or path == "": + return None + try: + resolved = os.path.realpath(path) + root = os.path.realpath(out_dir) + except OSError: + return None + if resolved == root or not resolved.startswith(root + os.sep): + return None + return resolved + + +def _remove_owned_file(path: object, out_dir: str) -> None: + """Delete one owned regular file, ignoring missing-path races. + + Args: + path: Candidate path that must resolve inside ``out_dir``. + out_dir: Directory passed to this import call. + """ + owned = _owned_file_path(path, out_dir) + if owned is None: + return + try: + if os.path.isfile(owned): + os.remove(owned) + except OSError: + return + + +def _remove_download_artifacts(status: dict[str, Any], out_dir: str) -> None: + """Delete the current download's partial, fragment, and control files. + + Args: + status: yt-dlp progress-hook payload that may name ``tmpfilename`` + and ``filename``. + out_dir: Directory passed to this import call. + """ + stems: set[str] = set() + for key in ("tmpfilename", "filename"): + owned = _owned_file_path(status.get(key), out_dir) + if owned is None: + continue + _remove_owned_file(owned, out_dir) + name = os.path.basename(owned) + if name.endswith(".part"): + name = name[: -len(".part")] + stems.add(name) + if not stems: + return + try: + entries = os.listdir(out_dir) + except OSError: + return + for entry in entries: + matches_stem = any( + entry == stem or entry.startswith(f"{stem}.") or entry.startswith(f"{stem}-") + for stem in stems + ) + if matches_stem: + _remove_owned_file(os.path.join(out_dir, entry), out_dir) + + +def _abort_over_budget_download(status: dict[str, Any], out_dir: str) -> None: + """Abort an in-flight download once encoded bytes exceed the policy ceiling. + + Args: + status: yt-dlp progress-hook payload. Unknown statuses are ignored. + out_dir: Directory passed to this import call, used to delete partials. + """ + if status.get("status") not in {"downloading", "finished"}: + return + for key in ("downloaded_bytes", "total_bytes", "total_bytes_estimate"): + candidate = status.get(key) + if isinstance(candidate, bool) or not isinstance(candidate, int): + continue + if candidate > DEFAULT_MAX_ENCODED_FILE_BYTES: + _remove_download_artifacts(status, out_dir) + raise YoutubeResourceLimitError("size_exceeded", YOUTUBE_SIZE_EXCEEDED_MESSAGE) + + +def _make_abort_hook(out_dir: str) -> Any: + """Bind the in-flight abort hook to one import output directory. + + Args: + out_dir: Directory passed to this import call. + + Returns: + A yt-dlp progress hook that aborts and deletes owned partials. + """ + + def _bound_abort_over_budget_download(status: dict[str, Any]) -> None: + """Abort and delete owned partials for this import directory. + + Args: + status: yt-dlp progress-hook payload. + """ + _abort_over_budget_download(status, out_dir) + + return _bound_abort_over_budget_download + + def _handle_download_error(e: yt_dlp.utils.DownloadError) -> Dict[str, Any]: """Map yt-dlp DownloadError to the public YouTube import error response.""" msg = str(e).lower() + if "max-filesize" in msg or "100 mib" in msg: + return _size_exceeded_result() if ( "sign in" in msg or "members-only" in msg @@ -130,6 +370,8 @@ def download_youtube_audio(url: str, out_dir: str) -> Dict[str, Any]: "noplaylist": True, "postprocessors": [{"key": "FFmpegExtractAudio"}], "geo_bypass": False, + "max_filesize": DEFAULT_MAX_ENCODED_FILE_BYTES, + "progress_hooks": [_make_abort_hook(out_dir)], } try: @@ -137,21 +379,17 @@ def download_youtube_audio(url: str, out_dir: str) -> Dict[str, Any]: info = ydl.extract_info(url, download=False) if info is None: raise Exception("Failed to extract info") - duration = info.get("duration") - if duration is not None and duration > 15 * 60: - return { - "ok": False, - "error": { - "code": "duration_exceeded", - "message": "Video exceeds the 15-minute limit.", - }, - } + duration_rejection = _reject_invalid_or_oversize_duration(info) + if duration_rejection is not None: + return duration_rejection + announced_rejection = _reject_announced_oversize(info) + if announced_rejection is not None: + return announced_rejection info = ydl.extract_info(url, download=True) if info is None: raise Exception("Failed to extract info") actual_filepath = ydl.prepare_filename(info) - actual_filepath = _find_downloaded_file(actual_filepath) if actual_filepath is None: @@ -163,18 +401,24 @@ def download_youtube_audio(url: str, out_dir: str) -> Dict[str, Any]: }, } - if ( - os.path.exists(actual_filepath) - and os.path.getsize(actual_filepath) > 50 * 1024 * 1024 - ): - os.remove(actual_filepath) - return { - "ok": False, - "error": { - "code": "size_exceeded", - "message": "Downloaded file exceeds the 50MB limit.", - }, - } + owned_filepath = _owned_file_path(actual_filepath, out_dir) + if owned_filepath is None: + return _download_error_result() + actual_filepath = owned_filepath + + duration_rejection = _reject_invalid_or_oversize_duration(info) + if duration_rejection is not None: + _remove_owned_file(actual_filepath, out_dir) + return duration_rejection + + try: + DEFAULT_AUDIO_RESOURCE_POLICY.validate_encoded_file_bytes( + os.path.getsize(actual_filepath) + ) + except ValueError: + if os.path.exists(actual_filepath): + os.remove(actual_filepath) + return _size_exceeded_result() return { "ok": True, "metadata": { @@ -184,13 +428,12 @@ def download_youtube_audio(url: str, out_dir: str) -> Dict[str, Any]: "filepath": actual_filepath, }, } + except YoutubeResourceLimitError: + return _size_exceeded_result() except yt_dlp.utils.DownloadError as e: return _handle_download_error(e) except Exception: - return { - "ok": False, - "error": {"code": "download_error", "message": YOUTUBE_IMPORT_FAILED_MESSAGE}, - } + return _download_error_result() def main() -> None: diff --git a/services/analysis-engine/tests/test_audio_decode_port.py b/services/analysis-engine/tests/test_audio_decode_port.py new file mode 100644 index 000000000..6838584d9 --- /dev/null +++ b/services/analysis-engine/tests/test_audio_decode_port.py @@ -0,0 +1,172 @@ +"""Contract tests for the canonical local-audio decode port. + +These regressions keep resource admission, decoder failure redaction, and +decoded-output validation behind one owned boundary. +""" + +from __future__ import annotations + +import io + +import numpy as np +import pytest + +from bandscope_analysis import audio_decode +from bandscope_analysis.audio_resource_policy import ( + DEFAULT_AUDIO_RESOURCE_POLICY, + AudioResourcePolicy, + AudioResourcePolicyError, +) + + +def test_decode_mono_audio_preflights_then_validates_one_owned_decode( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Keep preflight, one decode, and decoded validation in strict order. + + The decode port must own the sequence so downstream analyzers cannot + bypass or duplicate resource admission. + """ + source = io.BytesIO(b"container") + calls: list[tuple[str, object]] = [] + decoder_output = np.array([[0.25, -0.5]], dtype=np.float64) + + def preflight(candidate: object, policy: object) -> None: + calls.append(("preflight", candidate)) + assert policy is DEFAULT_AUDIO_RESOURCE_POLICY + + def load(candidate: object, **kwargs: object) -> tuple[np.ndarray, int]: + calls.append(("decode", candidate)) + assert candidate is source + assert kwargs == { + "sr": DEFAULT_AUDIO_RESOURCE_POLICY.target_sample_rate, + "mono": True, + "duration": DEFAULT_AUDIO_RESOURCE_POLICY.decode_probe_duration_seconds, + } + return decoder_output, DEFAULT_AUDIO_RESOURCE_POLICY.target_sample_rate + + def validate(self: AudioResourcePolicy, decoded: object, sample_rate: object) -> np.ndarray: + calls.append(("validate", decoded)) + assert self is DEFAULT_AUDIO_RESOURCE_POLICY + assert isinstance(decoded, np.ndarray) + assert decoded.dtype == np.float32 + assert decoded.shape == (2,) + assert sample_rate == DEFAULT_AUDIO_RESOURCE_POLICY.target_sample_rate + return decoded + + monkeypatch.setattr(audio_decode, "preflight_audio_metadata", preflight) + monkeypatch.setattr(audio_decode.librosa, "load", load) + monkeypatch.setattr(AudioResourcePolicy, "validate_decoded_audio", validate) + + decoded, sample_rate = audio_decode.decode_mono_audio( + source, + policy=DEFAULT_AUDIO_RESOURCE_POLICY, + ) + + assert calls[0] == ("preflight", source) + assert calls[1] == ("decode", source) + assert calls[2][0] == "validate" + np.testing.assert_array_equal(decoded, np.array([0.25, -0.5], dtype=np.float32)) + assert sample_rate == DEFAULT_AUDIO_RESOURCE_POLICY.target_sample_rate + + +def test_decode_mono_audio_preserves_resource_policy_rejection( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Propagate the canonical preflight rejection without invoking a decoder. + + A rejected source must not consume additional decode resources or lose its typed policy reason. + """ + rejection = AudioResourcePolicyError("duration_exceeded") + + def reject(_source: object, _policy: object) -> None: + raise rejection + + monkeypatch.setattr(audio_decode, "preflight_audio_metadata", reject) + monkeypatch.setattr( + audio_decode.librosa, + "load", + lambda *_args, **_kwargs: pytest.fail("decoder must not run after rejected preflight"), + ) + + with pytest.raises(AudioResourcePolicyError) as caught: + audio_decode.decode_mono_audio(io.BytesIO(b"container")) + + assert caught.value is rejection + + +def test_decode_mono_audio_redacts_third_party_decoder_failure( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Map third-party decoder details to a payload-safe policy error. + + Native paths or token-shaped details may remain only in the exception + cause for local debugging, never in buyer-facing error text. + """ + secret_detail = "/Users/alice/Music/private.m4a token=secret" + monkeypatch.setattr(audio_decode, "preflight_audio_metadata", lambda *_args: None) + monkeypatch.setattr( + audio_decode.librosa, + "load", + lambda *_args, **_kwargs: (_ for _ in ()).throw(RuntimeError(secret_detail)), + ) + + with pytest.raises(AudioResourcePolicyError) as caught: + audio_decode.decode_mono_audio(io.BytesIO(b"container")) + + assert caught.value.reason == "malformed_header" + assert secret_detail not in str(caught.value) + assert isinstance(caught.value.__cause__, RuntimeError) + + +def test_decode_mono_audio_redacts_malformed_decoder_output( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Reject decoder output that cannot be normalized into bounded PCM. + + Malformed third-party values must fail at the decode boundary rather than + escaping into MIR analyzers. + """ + monkeypatch.setattr(audio_decode, "preflight_audio_metadata", lambda *_args: None) + monkeypatch.setattr( + audio_decode.librosa, + "load", + lambda *_args, **_kwargs: ([object()], DEFAULT_AUDIO_RESOURCE_POLICY.target_sample_rate), + ) + + with pytest.raises(AudioResourcePolicyError) as caught: + audio_decode.decode_mono_audio(io.BytesIO(b"container")) + + assert caught.value.reason == "malformed_header" + + +def test_decode_mono_audio_preserves_decoded_policy_rejection( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Preserve rejection identity from decoded-audio resource validation. + + The decode port must not collapse a precise post-decode budget failure + into a generic malformed-container error. + """ + rejection = AudioResourcePolicyError("decoded_sample_count_exceeded") + monkeypatch.setattr(audio_decode, "preflight_audio_metadata", lambda *_args: None) + monkeypatch.setattr( + audio_decode.librosa, + "load", + lambda *_args, **_kwargs: ( + np.array([0.1], dtype=np.float32), + DEFAULT_AUDIO_RESOURCE_POLICY.target_sample_rate, + ), + ) + + def reject_decoded( + _self: AudioResourcePolicy, _decoded: object, _sample_rate: object + ) -> np.ndarray: + raise rejection + + monkeypatch.setattr(AudioResourcePolicy, "validate_decoded_audio", reject_decoded) + + with pytest.raises(AudioResourcePolicyError) as caught: + audio_decode.decode_mono_audio(io.BytesIO(b"container")) + + assert caught.value is rejection diff --git a/services/analysis-engine/tests/test_audio_metadata.py b/services/analysis-engine/tests/test_audio_metadata.py new file mode 100644 index 000000000..78e442619 --- /dev/null +++ b/services/analysis-engine/tests/test_audio_metadata.py @@ -0,0 +1,106 @@ +"""Source-container metadata preflight regressions.""" + +from __future__ import annotations + +import io +from types import SimpleNamespace +from unittest.mock import patch + +import pytest + +from bandscope_analysis.audio_metadata import preflight_audio_metadata +from bandscope_analysis.audio_resource_policy import AudioResourcePolicyError + + +def _info(*, frames: int = 44_100, samplerate: int = 44_100, channels: int = 2) -> SimpleNamespace: + """Build the metadata subset consumed by the preflight boundary.""" + return SimpleNamespace(frames=frames, samplerate=samplerate, channels=channels) + + +@patch("bandscope_analysis.audio_metadata.soundfile.info") +def test_preflight_accepts_metadata_and_rewinds_the_caller_handle(mock_info: object) -> None: + """A successful metadata probe leaves the decoder handle at its beginning.""" + source = io.BytesIO(b"header-bytes") + + def inspect(handle: io.BytesIO) -> SimpleNamespace: + """Consume a small header before returning parsed metadata.""" + handle.read(3) + return _info() + + mock_info.side_effect = inspect # type: ignore[attr-defined] + + preflight_audio_metadata(source) + + assert source.tell() == 0 + + +@pytest.mark.parametrize( + ("info", "reason"), + [ + (_info(frames=44_100 * 901), "duration_exceeded"), + (_info(samplerate=7_999), "sampling_rate_unsupported"), + (_info(channels=3), "channel_count_unsupported"), + ], +) +@patch("bandscope_analysis.audio_metadata.soundfile.info") +def test_preflight_rejects_untrusted_source_metadata( + mock_info: object, + info: SimpleNamespace, + reason: str, +) -> None: + """Source duration, rate, and channel bounds fail before PCM decode.""" + mock_info.return_value = info # type: ignore[attr-defined] + + with pytest.raises(AudioResourcePolicyError, match="audio resource policy") as error: + preflight_audio_metadata(io.BytesIO(b"header")) + + assert error.value.reason == reason + + +@pytest.mark.parametrize( + "dependency_error", + [RuntimeError("decoder detail"), ValueError("decoder detail")], +) +def test_preflight_maps_parser_failures_to_payload_free_policy_error( + dependency_error: Exception, +) -> None: + """Container parser failures cannot masquerade as policy errors or leak decoder detail.""" + with patch( + "bandscope_analysis.audio_metadata.soundfile.info", + side_effect=dependency_error, + ): + with pytest.raises(AudioResourcePolicyError, match="audio resource policy") as error: + preflight_audio_metadata(io.BytesIO(b"bad-header")) + + assert error.value.reason == "malformed_header" + assert error.value.policy_version == "1" + assert "decoder detail" not in str(error.value) + + +@patch("bandscope_analysis.audio_metadata.soundfile.info") +def test_preflight_maps_rewind_failures_to_payload_free_policy_error(mock_info: object) -> None: + """A handle that cannot rewind after probing cannot reach a decoder.""" + + class SeekFailsAfterProbe(io.BytesIO): + """Fail only when the metadata boundary tries to rewind the handle.""" + + def __init__(self) -> None: + """Initialize the caller-owned byte handle and seek counter.""" + super().__init__(b"header") + self.seek_count = 0 + + def seek(self, *args: object, **kwargs: object) -> int: + """Reject the second seek, which is the post-probe rewind.""" + self.seek_count += 1 + if self.seek_count == 2: + raise OSError("rewind failed") + return super().seek(*args, **kwargs) + + mock_info.return_value = _info() # type: ignore[attr-defined] + + with pytest.raises(AudioResourcePolicyError, match="audio resource policy") as error: + preflight_audio_metadata(SeekFailsAfterProbe()) + + assert error.value.reason == "malformed_header" + assert error.value.policy_version == "1" + assert "rewind failed" not in str(error.value) diff --git a/services/analysis-engine/tests/test_audio_model_output_policy.py b/services/analysis-engine/tests/test_audio_model_output_policy.py new file mode 100644 index 000000000..863b5418c --- /dev/null +++ b/services/analysis-engine/tests/test_audio_model_output_policy.py @@ -0,0 +1,39 @@ +"""Regression tests for fail-closed source-separation model output.""" + +from __future__ import annotations + +import numpy as np +import pytest + +from bandscope_analysis.separation.audio_separator import _as_float_array + + +@pytest.mark.parametrize( + "values", + [ + np.array([], dtype=np.float32), + np.array([np.nan], dtype=np.float32), + np.array([np.inf], dtype=np.float32), + np.array([np.finfo(np.float64).max], dtype=np.float64), + ], +) +def test_model_output_rejects_empty_nonfinite_or_float32_overflow(values: np.ndarray) -> None: + """Malformed model stems must fail instead of becoming successful silence.""" + with pytest.raises(ValueError, match=r"^Stem separation produced invalid audio\.$"): + _as_float_array(values) + + +def test_model_output_wraps_non_numeric_conversion_errors() -> None: + """Non-numeric model output must fail with the stable payload-free error.""" + with pytest.raises(ValueError, match=r"^Stem separation produced invalid audio\.$"): + _as_float_array(object()) + + +def test_model_output_preserves_valid_finite_samples() -> None: + """Valid model samples remain finite float32 audio with their original values.""" + values = np.array([0.25, -0.5, 0.75], dtype=np.float64) + + result = _as_float_array(values) + + assert result.dtype == np.float32 + assert np.array_equal(result, values.astype(np.float32)) diff --git a/services/analysis-engine/tests/test_audio_resource_policy.py b/services/analysis-engine/tests/test_audio_resource_policy.py new file mode 100644 index 000000000..0639590cc --- /dev/null +++ b/services/analysis-engine/tests/test_audio_resource_policy.py @@ -0,0 +1,235 @@ +"""Tests for the canonical local-audio resource policy.""" + +from __future__ import annotations + +import numpy as np +import pytest + +from bandscope_analysis.audio_resource_policy import ( + AUDIO_RESOURCE_POLICY_VERSION, + DEFAULT_AUDIO_RESOURCE_POLICY, + DEFAULT_MAX_SOURCE_CHANNELS, + DEFAULT_MAX_SOURCE_SAMPLE_RATE, + DEFAULT_MIN_SOURCE_CHANNELS, + DEFAULT_MIN_SOURCE_SAMPLE_RATE, + AudioResourcePolicy, + AudioResourcePolicyError, +) + + +def test_default_policy_has_stable_version_and_rehearsal_budget() -> None: + """The default policy exposes one versioned budget shared by analyzers.""" + assert AUDIO_RESOURCE_POLICY_VERSION == "1" + assert DEFAULT_AUDIO_RESOURCE_POLICY.max_encoded_file_bytes == 100 * 1024 * 1024 + assert DEFAULT_AUDIO_RESOURCE_POLICY.target_sample_rate == 44_100 + assert DEFAULT_AUDIO_RESOURCE_POLICY.max_duration_seconds == 15 * 60 + assert DEFAULT_AUDIO_RESOURCE_POLICY.max_decoded_samples == 44_100 * 15 * 60 + assert DEFAULT_AUDIO_RESOURCE_POLICY.max_decoded_audio_bytes == 44_100 * 15 * 60 * 8 + + +def test_oversized_encoded_file_exposes_stable_policy_reason() -> None: + """Encoded-size rejection carries a stable reason and policy version for UI/provenance.""" + policy = AudioResourcePolicy(max_encoded_file_bytes=100) + + with pytest.raises(AudioResourcePolicyError) as captured: + policy.validate_encoded_file_bytes(101) + + assert captured.value.reason == "encoded_file_too_large" + assert captured.value.policy_version == AUDIO_RESOURCE_POLICY_VERSION + assert "audio resource policy" in str(captured.value).lower() + + +def test_source_metadata_exposes_stable_policy_reasons() -> None: + """Container admission distinguishes duration, rate, and channel rejection reasons.""" + policy = AudioResourcePolicy(max_duration_seconds=1.0) + + with pytest.raises(AudioResourcePolicyError) as duration_rejection: + policy.validate_source_metadata(frames=44_101, sample_rate=44_100, channels=2) + assert duration_rejection.value.reason == "duration_exceeded" + assert duration_rejection.value.policy_version == AUDIO_RESOURCE_POLICY_VERSION + + with pytest.raises(AudioResourcePolicyError) as rate_rejection: + policy.validate_source_metadata( + frames=44_100, + sample_rate=DEFAULT_MAX_SOURCE_SAMPLE_RATE + 1, + channels=2, + ) + assert rate_rejection.value.reason == "sampling_rate_unsupported" + assert rate_rejection.value.policy_version == AUDIO_RESOURCE_POLICY_VERSION + + with pytest.raises(AudioResourcePolicyError) as channel_rejection: + policy.validate_source_metadata( + frames=44_100, + sample_rate=44_100, + channels=DEFAULT_MAX_SOURCE_CHANNELS + 1, + ) + assert channel_rejection.value.reason == "channel_count_unsupported" + assert channel_rejection.value.policy_version == AUDIO_RESOURCE_POLICY_VERSION + + +def test_decoded_memory_rejection_exposes_stable_policy_reason() -> None: + """Post-decode memory rejection remains machine-readable without exposing payload data.""" + policy = AudioResourcePolicy( + target_sample_rate=8, + max_duration_seconds=1.0, + max_decoded_audio_bytes=16, + ) + audio = np.zeros(4, dtype=np.float64) + + with pytest.raises(AudioResourcePolicyError) as captured: + policy.validate_decoded_audio(audio, 8) + + assert captured.value.reason == "memory_budget_exceeded" + assert captured.value.policy_version == AUDIO_RESOURCE_POLICY_VERSION + + +@pytest.mark.parametrize("file_size", [True, -1, 0, 101]) +def test_encoded_file_size_fails_closed_outside_policy(file_size: object) -> None: + """Invalid, empty, or oversized encoded inputs are rejected before decode.""" + policy = AudioResourcePolicy(max_encoded_file_bytes=100) + + with pytest.raises(ValueError, match="audio resource policy"): + policy.validate_encoded_file_bytes(file_size) + + +def test_encoded_file_size_accepts_exact_boundary() -> None: + """A non-empty encoded file exactly at the configured ceiling is accepted.""" + policy = AudioResourcePolicy(max_encoded_file_bytes=100) + + assert policy.validate_encoded_file_bytes(100) == 100 + + +def test_source_metadata_accepts_the_published_bounds() -> None: + """Source metadata accepts the inclusive rate, channel, and duration bounds.""" + policy = AudioResourcePolicy(max_duration_seconds=15 * 60) + + policy.validate_source_metadata( + frames=DEFAULT_MAX_SOURCE_SAMPLE_RATE * 15 * 60, + sample_rate=DEFAULT_MAX_SOURCE_SAMPLE_RATE, + channels=DEFAULT_MAX_SOURCE_CHANNELS, + ) + policy.validate_source_metadata( + frames=DEFAULT_MIN_SOURCE_SAMPLE_RATE, + sample_rate=DEFAULT_MIN_SOURCE_SAMPLE_RATE, + channels=DEFAULT_MIN_SOURCE_CHANNELS, + ) + + +@pytest.mark.parametrize( + ("frames", "sample_rate", "channels"), + [ + (DEFAULT_MAX_SOURCE_SAMPLE_RATE * (15 * 60 + 1), 44_100, 2), + (44_100, DEFAULT_MIN_SOURCE_SAMPLE_RATE - 1, 2), + (44_100, DEFAULT_MAX_SOURCE_SAMPLE_RATE + 1, 2), + (44_100, 44_100, DEFAULT_MAX_SOURCE_CHANNELS + 1), + (44_100, 44_100, DEFAULT_MIN_SOURCE_CHANNELS - 1), + (0, 44_100, 2), + (44_100, True, 2), + (44_100, 44_100, True), + (10**400, 44_100, 2), + ], +) +def test_source_metadata_fails_closed_before_decode( + frames: object, + sample_rate: object, + channels: object, +) -> None: + """Overlong and malformed source metadata cannot reach a decoder.""" + with pytest.raises(ValueError, match="audio resource policy"): + DEFAULT_AUDIO_RESOURCE_POLICY.validate_source_metadata(frames, sample_rate, channels) + + +@pytest.mark.parametrize( + ("audio", "sample_rate"), + [ + (np.zeros(8_001, dtype=np.float32), 8_000), + (np.zeros((2, 4_000), dtype=np.float32), 8_000), + (np.array([0.0, np.nan], dtype=np.float32), 8_000), + (np.array(["not-a-sample"], dtype=object), 8_000), + (np.zeros(10, dtype=np.int16), 8_000), + (np.zeros(10, dtype=np.float32), 0), + (np.zeros(10, dtype=np.float32), True), + ], +) +def test_decoded_audio_fails_closed_outside_policy( + audio: np.ndarray, + sample_rate: object, +) -> None: + """Decoded output is revalidated for type, shape, finiteness, rate, and sample budget.""" + policy = AudioResourcePolicy(target_sample_rate=8_000, max_duration_seconds=1.0) + + with pytest.raises(ValueError, match="audio resource policy"): + policy.validate_decoded_audio(audio, sample_rate) + + +def test_decoded_audio_rejects_buffer_above_memory_budget() -> None: + """A decoder cannot hide excessive memory behind an allowed sample count.""" + policy = AudioResourcePolicy( + target_sample_rate=8, + max_duration_seconds=1.0, + max_decoded_audio_bytes=16, + ) + audio = np.zeros(4, dtype=np.float64) + + with pytest.raises(ValueError, match="audio resource policy"): + policy.validate_decoded_audio(audio, 8) + + +def test_decoded_audio_accepts_exact_memory_boundary() -> None: + """A finite canonical buffer exactly at the memory ceiling is accepted.""" + policy = AudioResourcePolicy( + target_sample_rate=8, + max_duration_seconds=1.0, + max_decoded_audio_bytes=32, + ) + audio = np.zeros(8, dtype=np.float32) + + assert policy.validate_decoded_audio(audio, 8) is audio + + +def test_decoded_audio_accepts_exact_sample_boundary() -> None: + """A finite mono artifact exactly at the decoded-sample ceiling is accepted.""" + policy = AudioResourcePolicy(target_sample_rate=8_000, max_duration_seconds=1.0) + audio = np.zeros(8_000, dtype=np.float32) + + validated = policy.validate_decoded_audio(audio, 8_000) + + assert validated is audio + + +@pytest.mark.parametrize( + "kwargs", + [ + {"max_encoded_file_bytes": 0}, + {"target_sample_rate": 0}, + {"max_duration_seconds": 0.0}, + {"max_duration_seconds": float("inf")}, + {"max_decoded_audio_bytes": 0}, + {"max_decoded_audio_bytes": True}, + {"min_source_sample_rate": 0}, + {"max_source_channels": True}, + {"min_source_sample_rate": 48_000, "max_source_sample_rate": 44_100}, + {"min_source_channels": 2, "max_source_channels": 1}, + ], +) +def test_policy_configuration_itself_fails_closed(kwargs: dict[str, object]) -> None: + """Invalid policy construction cannot silently create an unbounded budget.""" + with pytest.raises(ValueError, match="audio resource policy"): + AudioResourcePolicy(**kwargs) # type: ignore[arg-type] + + +@pytest.mark.parametrize( + "kwargs", + [ + {"target_sample_rate": 10**400, "max_duration_seconds": 1.0}, + {"target_sample_rate": 1, "max_duration_seconds": 10**400}, + {"max_encoded_file_bytes": 10**400}, + {"max_decoded_audio_bytes": 10**400}, + ], +) +def test_policy_configuration_fails_closed_on_unrepresentable_limits( + kwargs: dict[str, object], +) -> None: + """Extreme integer limits cannot escape stable policy validation through overflow.""" + with pytest.raises(ValueError, match="audio resource policy"): + AudioResourcePolicy(**kwargs) # type: ignore[arg-type] diff --git a/services/analysis-engine/tests/test_audio_resource_policy_coverage_regressions.py b/services/analysis-engine/tests/test_audio_resource_policy_coverage_regressions.py new file mode 100644 index 000000000..8dfa3d688 --- /dev/null +++ b/services/analysis-engine/tests/test_audio_resource_policy_coverage_regressions.py @@ -0,0 +1,74 @@ +"""Coverage regressions for fail-closed audio resource admission branches.""" + +from __future__ import annotations + +from pathlib import Path + +import numpy as np +import pytest + +from bandscope_analysis.audio_resource_policy import AudioResourcePolicy +from bandscope_analysis.separation.audio_separator import ( + AudioSeparationConfig, + AudioStemSeparator, +) + + +def test_policy_rejects_boolean_duration_configuration() -> None: + """A Boolean duration must not be coerced into a one-second resource budget.""" + with pytest.raises(ValueError, match="audio resource policy"): + AudioResourcePolicy(max_duration_seconds=True) + + +def test_policy_rejects_less_than_one_decoded_sample_budget() -> None: + """A positive duration that represents less than one sample must fail closed.""" + with pytest.raises(ValueError, match="audio resource policy"): + AudioResourcePolicy(target_sample_rate=1, max_duration_seconds=0.5) + + +def test_separator_rejects_empty_internal_loader_result_before_model( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An unexpected empty loader result must not reach Demucs inference.""" + audio_path = tmp_path / "unexpected-empty.wav" + audio_path.write_bytes(b"not-empty") + separator = AudioStemSeparator( + AudioSeparationConfig(target_sample_rate=8_000, max_file_bytes=1_000_000) + ) + monkeypatch.setattr( + separator, + "_load_audio", + lambda _path: (np.array([], dtype=np.float32), 8_000), + ) + + def fail_if_model_runs(_audio: np.ndarray, _sample_rate: int) -> dict[str, np.ndarray]: + raise AssertionError("empty decoded audio must be rejected before model inference") + + monkeypatch.setattr(separator, "_separate_signal", fail_if_model_runs) + + with pytest.raises(ValueError, match="Stem separation decode failed"): + separator.separate(audio_path) + + +def test_separator_rejects_zero_byte_file_before_decoder( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A zero-byte selected source must fail before the decoder is invoked.""" + audio_path = tmp_path / "empty.wav" + audio_path.write_bytes(b"") + separator = AudioStemSeparator( + AudioSeparationConfig(target_sample_rate=8_000, max_file_bytes=1_000_000) + ) + + def fail_if_decoder_runs(*_args: object, **_kwargs: object) -> tuple[np.ndarray, int]: + raise AssertionError("zero-byte input must be rejected before decoder invocation") + + monkeypatch.setattr( + "bandscope_analysis.audio_decode.librosa.load", + fail_if_decoder_runs, + ) + + with pytest.raises(ValueError, match="Stem separation decode failed"): + separator.separate(audio_path) diff --git a/services/analysis-engine/tests/test_audio_resource_policy_integration.py b/services/analysis-engine/tests/test_audio_resource_policy_integration.py new file mode 100644 index 000000000..72ce8d0bd --- /dev/null +++ b/services/analysis-engine/tests/test_audio_resource_policy_integration.py @@ -0,0 +1,241 @@ +"""Cross-boundary regressions for canonical local-audio resource admission.""" + +from __future__ import annotations + +from types import SimpleNamespace +from unittest.mock import Mock + +import numpy as np +import pytest + +from bandscope_analysis.api import validate_analysis_job_request +from bandscope_analysis.audio_resource_policy import ( + DEFAULT_AUDIO_RESOURCE_POLICY, + AudioResourcePolicy, +) +from bandscope_analysis.separation.audio_separator import ( + AudioSeparationConfig, + AudioStemSeparator, +) +from bandscope_analysis.temporal.analyzer import TemporalAnalyzer + + +def _local_request(file_size_bytes: object) -> dict[str, object]: + """Build one local-audio request whose only variable is encoded byte metadata.""" + return { + "sourceKind": "local_audio", + "projectId": "policy-project", + "sourceLabel": "rehearsal.wav", + "roleFocus": [], + "localSource": { + "sourcePath": "/tmp/rehearsal.wav", + "fileName": "rehearsal.wav", + "extension": "wav", + "fileSizeBytes": file_size_bytes, + }, + } + + +@pytest.mark.parametrize( + "file_size_bytes", + [True, DEFAULT_AUDIO_RESOURCE_POLICY.max_encoded_file_bytes + 1], +) +def test_request_preflight_rejects_metadata_outside_canonical_policy( + file_size_bytes: object, +) -> None: + """Reject impossible/oversized metadata before orchestration starts expensive work.""" + with pytest.raises(ValueError, match="localSource.fileSizeBytes"): + validate_analysis_job_request(_local_request(file_size_bytes)) + + +def test_request_preflight_accepts_exact_encoded_byte_boundary() -> None: + """The service API accepts the same exact encoded-byte ceiling as the policy.""" + request = validate_analysis_job_request( + _local_request(DEFAULT_AUDIO_RESOURCE_POLICY.max_encoded_file_bytes) + ) + + assert ( + request["localSource"]["fileSizeBytes"] + == DEFAULT_AUDIO_RESOURCE_POLICY.max_encoded_file_bytes + ) + + +def test_temporal_decoder_probes_one_sample_past_duration_limit_and_fails_closed( + tmp_path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Temporal decode detects a one-sample-overlong source instead of silently truncating it.""" + import librosa + + policy = AudioResourcePolicy( + max_encoded_file_bytes=100, + target_sample_rate=8, + max_duration_seconds=1.0, + ) + source = tmp_path / "overlong.wav" + source.write_bytes(b"bounded") + monkeypatch.setattr( + "bandscope_analysis.audio_decode.preflight_audio_metadata", + lambda *_args, **_kwargs: None, + ) + captured: dict[str, object] = {} + + def fake_load(fileobj: object, **kwargs: object) -> tuple[np.ndarray, int]: + captured.update(kwargs) + return np.zeros(policy.max_decoded_samples + 1, dtype=np.float32), policy.target_sample_rate + + monkeypatch.setattr(librosa, "load", fake_load) + monkeypatch.setattr( + librosa.beat, + "beat_track", + lambda **_: (_ for _ in ()).throw( + AssertionError("analysis must not run after policy rejection") + ), + ) + + with pytest.raises(ValueError, match="audio resource policy"): + TemporalAnalyzer(resource_policy=policy).analyze(source) + + assert captured["duration"] == pytest.approx( + (policy.max_decoded_samples + 1) / policy.target_sample_rate + ) + assert captured["sr"] == policy.target_sample_rate + assert captured["mono"] is True + + +@pytest.mark.parametrize( + "metadata", + [ + SimpleNamespace(frames=44_100 * 901, samplerate=44_100, channels=2), + SimpleNamespace(frames=44_100, samplerate=7_999, channels=2), + SimpleNamespace(frames=44_100, samplerate=44_100, channels=3), + ], +) +def test_temporal_rejects_source_metadata_before_librosa_decode( + tmp_path, + monkeypatch: pytest.MonkeyPatch, + metadata: SimpleNamespace, +) -> None: + """Temporal analysis must inspect source metadata before resampling or truncation.""" + import librosa + + source = tmp_path / "source-metadata.wav" + source.write_bytes(b"bounded") + monkeypatch.setattr( + "bandscope_analysis.audio_metadata.soundfile.info", + lambda _fileobj: metadata, + ) + load_mock = Mock(side_effect=AssertionError("source metadata must be checked first")) + monkeypatch.setattr(librosa, "load", load_mock) + + with pytest.raises(ValueError, match="audio resource policy"): + TemporalAnalyzer().analyze(source) + + load_mock.assert_not_called() + + +def test_stem_decoder_probes_one_sample_past_duration_limit_and_fails_closed( + tmp_path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Stem separation consumes the same decoded-sample ceiling as temporal analysis.""" + import librosa + + config = AudioSeparationConfig( + target_sample_rate=8, + max_file_bytes=100, + max_duration_seconds=1.0, + ) + source = tmp_path / "overlong.wav" + source.write_bytes(b"bounded") + monkeypatch.setattr( + "bandscope_analysis.audio_decode.preflight_audio_metadata", + lambda *_args, **_kwargs: None, + ) + captured: dict[str, object] = {} + + def fake_load(fileobj: object, **kwargs: object) -> tuple[np.ndarray, int]: + captured.update(kwargs) + return np.zeros(9, dtype=np.float32), 8 + + monkeypatch.setattr(librosa, "load", fake_load) + monkeypatch.setattr( + AudioStemSeparator, + "_separate_signal", + lambda *_: (_ for _ in ()).throw( + AssertionError("model must not run after policy rejection") + ), + ) + + with pytest.raises(ValueError, match="audio resource policy"): + AudioStemSeparator(config).separate(source) + + assert captured["duration"] == pytest.approx(9 / 8) + assert captured["sr"] == 8 + assert captured["mono"] is True + + +@pytest.mark.parametrize( + "metadata", + [ + SimpleNamespace(frames=44_100 * 901, samplerate=44_100, channels=2), + SimpleNamespace(frames=44_100, samplerate=7_999, channels=2), + SimpleNamespace(frames=44_100, samplerate=44_100, channels=3), + ], +) +def test_stem_decoder_rejects_source_metadata_before_librosa_decode( + tmp_path, + monkeypatch: pytest.MonkeyPatch, + metadata: SimpleNamespace, +) -> None: + """Stem separation must inspect source metadata before mono conversion or model work.""" + import librosa + + source = tmp_path / "source-metadata.wav" + source.write_bytes(b"bounded") + monkeypatch.setattr( + "bandscope_analysis.audio_metadata.soundfile.info", + lambda _fileobj: metadata, + ) + load_mock = Mock(side_effect=AssertionError("source metadata must be checked first")) + monkeypatch.setattr(librosa, "load", load_mock) + + separator = AudioStemSeparator(AudioSeparationConfig(max_file_bytes=100)) + with pytest.raises(ValueError, match="audio resource policy"): + separator.separate(source) + + load_mock.assert_not_called() + + +def test_stem_decoder_rejects_nonfinite_decoded_output_before_model( + tmp_path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Decoder NaN/Inf values fail closed instead of being normalized into model input.""" + import librosa + + source = tmp_path / "nonfinite.wav" + source.write_bytes(b"bounded") + monkeypatch.setattr( + "bandscope_analysis.audio_decode.preflight_audio_metadata", + lambda *_args, **_kwargs: None, + ) + monkeypatch.setattr( + librosa, + "load", + lambda *args, **kwargs: (np.array([0.0, np.nan], dtype=np.float32), 8), + ) + monkeypatch.setattr( + AudioStemSeparator, + "_separate_signal", + lambda *_: (_ for _ in ()).throw(AssertionError("model must not receive non-finite audio")), + ) + + with pytest.raises(ValueError, match="audio resource policy"): + AudioStemSeparator( + AudioSeparationConfig( + target_sample_rate=8, + max_file_bytes=100, + max_duration_seconds=1.0, + ) + ).separate(source) diff --git a/services/analysis-engine/tests/test_audio_separator_device_boundary.py b/services/analysis-engine/tests/test_audio_separator_device_boundary.py new file mode 100644 index 000000000..e8545876b --- /dev/null +++ b/services/analysis-engine/tests/test_audio_separator_device_boundary.py @@ -0,0 +1,109 @@ +"""Device-boundary regressions for local Demucs separation.""" + +from __future__ import annotations + +import sys +from types import ModuleType + +import numpy as np +import pytest + +from bandscope_analysis.separation.audio_separator import AudioSeparationConfig, AudioStemSeparator + + +class _FakeModel: + """Expose the canonical Demucs source order used by production.""" + + sources = ["drums", "bass", "other", "vocals"] + + +class _DeviceTensor: + """Minimal tensor that refuses NumPy conversion until moved to CPU.""" + + def __init__(self, array: np.ndarray, *, on_cpu: bool) -> None: + self.array = np.asarray(array, dtype=np.float32) + self.on_cpu = on_cpu + + def float(self) -> "_DeviceTensor": + return _DeviceTensor(self.array.astype(np.float32), on_cpu=self.on_cpu) + + def mean(self, axis: int | None = None) -> float | "_DeviceTensor": + value = self.array.mean(axis=axis) + if axis is None: + return float(value) + return _DeviceTensor(np.asarray(value, dtype=np.float32), on_cpu=self.on_cpu) + + def std(self) -> float: + return float(self.array.std()) + + def cpu(self) -> "_DeviceTensor": + return _DeviceTensor(self.array, on_cpu=True) + + def numpy(self) -> np.ndarray: + if not self.on_cpu: + raise RuntimeError("can't convert cuda tensor to numpy") + return self.array + + def __getitem__(self, key: object) -> "_DeviceTensor": + return _DeviceTensor(self.array[key], on_cpu=self.on_cpu) + + def __add__(self, value: float) -> "_DeviceTensor": + return _DeviceTensor(self.array + value, on_cpu=self.on_cpu) + + def __sub__(self, value: float) -> "_DeviceTensor": + return _DeviceTensor(self.array - value, on_cpu=self.on_cpu) + + def __mul__(self, value: float) -> "_DeviceTensor": + return _DeviceTensor(self.array * value, on_cpu=self.on_cpu) + + def __truediv__(self, value: float) -> "_DeviceTensor": + return _DeviceTensor(self.array / value, on_cpu=self.on_cpu) + + +class _NoGrad: + def __enter__(self) -> None: + return None + + def __exit__(self, *args: object) -> None: + return None + + +def test_apply_model_moves_device_output_to_cpu_before_numpy( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """GPU-selected separation must cross the device boundary before NumPy conversion.""" + calls: dict[str, object] = {} + fake_torch = ModuleType("torch") + fake_torch.from_numpy = lambda array: _DeviceTensor(array, on_cpu=True) # type: ignore[attr-defined] + fake_torch.no_grad = _NoGrad # type: ignore[attr-defined] + + def fake_apply_model( + model: _FakeModel, + batch: _DeviceTensor, + *, + device: str, + split: bool, + overlap: float, + progress: bool, + ) -> _DeviceTensor: + calls.update(device=device, split=split, overlap=overlap, progress=progress) + source_values = np.arange(len(model.sources), dtype=np.float32).reshape(-1, 1, 1) + separated = np.broadcast_to(source_values, (len(model.sources), 2, 4)).copy() + return _DeviceTensor(separated[None], on_cpu=False) + + demucs_module = ModuleType("demucs") + apply_module = ModuleType("demucs.apply") + apply_module.apply_model = fake_apply_model # type: ignore[attr-defined] + demucs_module.apply = apply_module # type: ignore[attr-defined] + monkeypatch.setitem(sys.modules, "torch", fake_torch) + monkeypatch.setitem(sys.modules, "demucs", demucs_module) + monkeypatch.setitem(sys.modules, "demucs.apply", apply_module) + + audio = np.array([0.0, 1.0, -1.0, 0.5], dtype=np.float32) + separator = AudioStemSeparator(AudioSeparationConfig(device="cuda", overlap=0.375)) + + result = separator._apply_model(_FakeModel(), audio) + + assert calls == {"device": "cuda", "split": True, "overlap": 0.375, "progress": False} + assert set(result) == set(_FakeModel.sources) + assert all(stem.shape == (4,) for stem in result.values()) diff --git a/services/analysis-engine/tests/test_chord_recognizer.py b/services/analysis-engine/tests/test_chord_recognizer.py index 20a6dcf78..88ff6684c 100644 --- a/services/analysis-engine/tests/test_chord_recognizer.py +++ b/services/analysis-engine/tests/test_chord_recognizer.py @@ -3,6 +3,7 @@ from unittest.mock import patch import numpy as np +import pytest from bandscope_analysis.chords.chord_recognizer import ( ChordRecognizer, @@ -20,6 +21,14 @@ def test_chord_recognizer_empty_audio() -> None: assert result == [] +@pytest.mark.parametrize("shape", [(0, 2), (2, 0)]) +def test_chord_recognizer_empty_layouts(shape: tuple[int, int]) -> None: + """Every zero-element NumPy layout must short-circuit recognition.""" + recognizer = ChordRecognizer() + + assert recognizer.recognize(np.empty(shape), sr=22050) == [] + + def test_chord_recognizer_unvoiced_audio() -> None: """Test chord recognition with noise.""" recognizer = ChordRecognizer() diff --git a/services/analysis-engine/tests/test_separation.py b/services/analysis-engine/tests/test_separation.py index f8e098521..49ac97637 100644 --- a/services/analysis-engine/tests/test_separation.py +++ b/services/analysis-engine/tests/test_separation.py @@ -466,7 +466,11 @@ def test_audio_stem_separator_rejects_empty_decoder_output( audio_path = tmp_path / "empty.wav" audio_path.write_bytes(b"placeholder") monkeypatch.setattr( - "bandscope_analysis.separation.audio_separator.librosa.load", + "bandscope_analysis.audio_decode.preflight_audio_metadata", + lambda *_args, **_kwargs: None, + ) + monkeypatch.setattr( + "bandscope_analysis.audio_decode.librosa.load", lambda *args, **kwargs: (np.array([], dtype=np.float32), 8_000), ) separator = AudioStemSeparator(AudioSeparationConfig(target_sample_rate=8_000)) @@ -481,12 +485,16 @@ def test_audio_stem_separator_redacts_decoder_exceptions( """Ensure decoder failures are surfaced without full local paths.""" audio_path = tmp_path / "broken.wav" audio_path.write_bytes(b"placeholder") + monkeypatch.setattr( + "bandscope_analysis.audio_decode.preflight_audio_metadata", + lambda *_args, **_kwargs: None, + ) def fail_decode(*args, **kwargs): raise RuntimeError(f"decoder failed under {tmp_path}") monkeypatch.setattr( - "bandscope_analysis.separation.audio_separator.librosa.load", + "bandscope_analysis.audio_decode.librosa.load", fail_decode, ) separator = AudioStemSeparator(AudioSeparationConfig(target_sample_rate=8_000)) diff --git a/services/analysis-engine/tests/test_stem_separation_logging_privacy.py b/services/analysis-engine/tests/test_stem_separation_logging_privacy.py new file mode 100644 index 000000000..8d7d2d7b1 --- /dev/null +++ b/services/analysis-engine/tests/test_stem_separation_logging_privacy.py @@ -0,0 +1,124 @@ +"""Regression tests for stem-separation logging privacy.""" + +import logging + +import pytest + +import bandscope_analysis.api as analysis_api + + +class _ResultQueue: + """Capture the worker result without starting a multiprocessing queue.""" + + def __init__(self) -> None: + self.items: list[tuple[object, object]] = [] + + def put(self, item: tuple[object, object]) -> None: + """Record one result emitted by the worker.""" + self.items.append(item) + + +class _FailingSeparator: + """Raise dependency-controlled sensitive text from the separator boundary.""" + + def separate(self, source_path: str) -> dict[str, object]: + """Simulate a dependency failure after receiving an authorized source path.""" + raise RuntimeError( + f"decoder failed for {source_path} /Users/Alice/private-song.wav token=super-secret" + ) + + +def _local_audio_request() -> dict[str, object]: + """Return a valid local-audio request without cache or temporary-path authority.""" + return { + "sourceKind": "local_audio", + "projectId": "privacy-regression", + "sourceLabel": "private-song.wav", + "roleFocus": ["bass-guitar"], + "localSource": { + "sourcePath": "/private/customer/Alice/session.wav", + "fileName": "private-song.wav", + "extension": "wav", + "fileSizeBytes": 1024, + }, + } + + +def _assert_payload_free_log(caplog: pytest.LogCaptureFixture) -> None: + """Require routine logs to omit dependency payloads and exception tracebacks.""" + assert "/private/customer/Alice/session.wav" not in caplog.text + assert "/Users/Alice/private-song.wav" not in caplog.text + assert "private-song.wav token=super-secret" not in caplog.text + assert "super-secret" not in caplog.text + assert all(record.exc_info is None for record in caplog.records) + + +def test_stem_worker_failure_log_omits_dependency_payload_and_traceback( + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """Routine worker diagnostics must not retain dependency payloads or tracebacks.""" + result_queue = _ResultQueue() + source_path = "/private/customer/Alice/session.wav" + + monkeypatch.setattr(analysis_api, "AudioStemSeparator", _FailingSeparator) + caplog.set_level(logging.ERROR, logger=analysis_api.__name__) + + analysis_api._stem_separation_worker(source_path, result_queue) + + assert result_queue.items == [ + ("runtime_error", "Runtime error occurred during stem separation.") + ] + assert "Stem separation failed with a runtime error." in caplog.text + _assert_payload_free_log(caplog) + + +def test_analysis_job_stem_failure_log_omits_dependency_payload_and_traceback( + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """Parent orchestration failure logs must keep dependency details out of routine logs.""" + sensitive_detail = ( + "decode failed for /private/customer/Alice/session.wav " + "/Users/Alice/private-song.wav token=super-secret" + ) + + def fail_features(_request: analysis_api.AnalysisJobRequest) -> None: + raise ValueError(sensitive_detail) + + monkeypatch.setattr(analysis_api, "_build_local_audio_features", fail_features) + caplog.set_level(logging.ERROR, logger=analysis_api.__name__) + + updates = analysis_api.run_analysis_job_updates( + "job-privacy", + _local_audio_request(), + "2026-08-20T00:00:00Z", + ) + + assert updates[-1]["state"] == "failed" + assert updates[-1]["error"] == { + "code": "engine_unavailable", + "message": "Stem separation failed", + } + assert "Stem separation failed before analysis job completion." in caplog.text + _assert_payload_free_log(caplog) + + +def test_api_logger_preserves_unrelated_exception_traceback( + caplog: pytest.LogCaptureFixture, +) -> None: + """Privacy redaction must not erase traceback evidence from unrelated API diagnostics.""" + caplog.set_level(logging.ERROR, logger=analysis_api.__name__) + + try: + raise RuntimeError("non-sensitive diagnostic sentinel") + except RuntimeError: + analysis_api.logger.exception("Unrelated analysis API diagnostic.") + + records = [ + record + for record in caplog.records + if record.getMessage() == "Unrelated analysis API diagnostic." + ] + assert len(records) == 1 + assert records[0].exc_info is not None diff --git a/services/analysis-engine/tests/test_temporal.py b/services/analysis-engine/tests/test_temporal.py index 6ce90ae1c..16c7f7034 100644 --- a/services/analysis-engine/tests/test_temporal.py +++ b/services/analysis-engine/tests/test_temporal.py @@ -46,9 +46,9 @@ def test_temporal_analyzer_basic(dummy_audio_file: Path) -> None: def test_temporal_analyzer_file_not_found() -> None: - """Test that analyzer raises appropriate error for missing files.""" + """Test that analyzer raises a payload-safe error for missing files.""" analyzer = TemporalAnalyzer() - with pytest.raises(FileNotFoundError, match="Audio file not found"): + with pytest.raises(FileNotFoundError, match="Audio source is unavailable"): analyzer.analyze("nonexistent_file.wav") @@ -62,7 +62,7 @@ def test_temporal_analyzer_missing_file_does_not_call_decoder( monkeypatch.setattr(librosa, "load", load_mock) analyzer = TemporalAnalyzer() - with pytest.raises(FileNotFoundError, match="Audio file not found"): + with pytest.raises(FileNotFoundError, match="Audio source is unavailable"): analyzer.analyze("nonexistent_file.wav") load_mock.assert_not_called() @@ -77,7 +77,7 @@ def test_temporal_analyzer_directory_does_not_call_decoder( load_mock = Mock(side_effect=AssertionError("librosa.load should not be called")) monkeypatch.setattr(librosa, "load", load_mock) - with pytest.raises(FileNotFoundError, match="Audio file not found"): + with pytest.raises(FileNotFoundError, match="Audio source is unavailable"): TemporalAnalyzer().analyze(tmp_path) load_mock.assert_not_called() @@ -94,7 +94,7 @@ def fake_load(*args, **kwargs): monkeypatch.setattr(librosa, "load", fake_load) test_wav = tmp_path / "test.wav" - test_wav.write_bytes(b"dummy") + sf.write(test_wav, np.zeros(4_000, dtype=np.float32), 44_100) with pytest.raises(ValueError, match="Expected numpy array"): TemporalAnalyzer().analyze(test_wav) @@ -104,7 +104,7 @@ def test_temporal_analyzer_exception_handling( monkeypatch: pytest.MonkeyPatch, tmp_path: Path, ) -> None: - """Ensure temporal analyzer catches general exceptions and raises ValueError.""" + """Ensure arbitrary decoder exception payloads are not relayed to callers.""" import librosa from bandscope_analysis.temporal.analyzer import TemporalAnalyzer @@ -115,10 +115,11 @@ def fake_load(*args: object, **kwargs: object) -> tuple[np.ndarray, int]: monkeypatch.setattr(librosa, "load", fake_load) test_wav = tmp_path / "test.wav" - test_wav.write_bytes(b"dummy") + sf.write(test_wav, np.zeros(4_000, dtype=np.float32), 44_100) - with pytest.raises(ValueError, match="Temporal analysis failed: Mocked general error"): + with pytest.raises(ValueError, match=r"^Temporal analysis failed\.$") as exc_info: TemporalAnalyzer().analyze(test_wav) + assert "Mocked general error" not in str(exc_info.value) def test_temporal_analyzer_rejects_oversized_file(monkeypatch, tmp_path: Path) -> None: @@ -128,7 +129,7 @@ def test_temporal_analyzer_rejects_oversized_file(monkeypatch, tmp_path: Path) - from bandscope_analysis.temporal import analyzer as analyzer_module test_wav = tmp_path / "large.wav" - test_wav.write_bytes(b"1234") + sf.write(test_wav, np.zeros(4_000, dtype=np.float32), 44_100) monkeypatch.setattr(analyzer_module, "MAX_AUDIO_FILE_BYTES", 1) @@ -147,7 +148,7 @@ def test_temporal_analyzer_uses_duration_limit(monkeypatch, tmp_path: Path) -> N import librosa test_wav = tmp_path / "bounded.wav" - test_wav.write_bytes(b"1234") + sf.write(test_wav, np.zeros(4_000, dtype=np.float32), 44_100) captured_kwargs: dict[str, object] = {} def fake_load(path, **kwargs): @@ -178,7 +179,7 @@ def test_temporal_analyzer_does_not_suppress_unrelated_loader_warnings( import librosa test_wav = tmp_path / "test.wav" - test_wav.write_bytes(b"dummy") + sf.write(test_wav, np.zeros(4_000, dtype=np.float32), 44_100) def fake_load(*args: object, **kwargs: object) -> tuple[np.ndarray, int]: warnings.warn("unrelated downstream warning", FutureWarning, stacklevel=2) diff --git a/services/analysis-engine/tests/test_temporal_error_privacy.py b/services/analysis-engine/tests/test_temporal_error_privacy.py new file mode 100644 index 000000000..ea0c6519f --- /dev/null +++ b/services/analysis-engine/tests/test_temporal_error_privacy.py @@ -0,0 +1,57 @@ +"""Privacy regressions for temporal-analysis failure diagnostics.""" + +from __future__ import annotations + +import logging +from pathlib import Path + +import numpy as np +import pytest +import soundfile as sf + +from bandscope_analysis.temporal import TemporalAnalyzer + + +def test_missing_temporal_source_does_not_disclose_local_path(tmp_path: Path) -> None: + """Missing-file failures must not echo an absolute customer path to callers.""" + sensitive_path = tmp_path / "private-customer-session" / "unreleased-song.wav" + + with pytest.raises(FileNotFoundError) as exc_info: + TemporalAnalyzer().analyze(sensitive_path) + + message = str(exc_info.value) + assert message == "Audio source is unavailable for temporal analysis." + assert str(sensitive_path) not in message + assert "unreleased-song.wav" not in message + + +def test_decoder_failure_redacts_source_path_and_decoder_payload( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + caplog: pytest.LogCaptureFixture, +) -> None: + """Decoder diagnostics must remain useful without logging customer path/payload data.""" + import librosa + + sensitive_path = tmp_path / "private-customer-session" / "unreleased-song.wav" + sensitive_path.parent.mkdir() + sf.write(sensitive_path, np.zeros(4_000, dtype=np.float32), 44_100) + decoder_payload = "decoder exposed /private/customer/token-shaped-audio-name.wav" + + def fail_decode(*args: object, **kwargs: object) -> tuple[object, int]: + raise RuntimeError(decoder_payload) + + monkeypatch.setattr(librosa, "load", fail_decode) + caplog.set_level(logging.INFO, logger="bandscope_analysis.temporal.analyzer") + + with pytest.raises(ValueError) as exc_info: + TemporalAnalyzer().analyze(sensitive_path) + + message = str(exc_info.value) + assert message == "Temporal analysis failed." + assert str(sensitive_path) not in message + assert decoder_payload not in message + assert str(sensitive_path) not in caplog.text + assert "unreleased-song.wav" not in caplog.text + assert decoder_payload not in caplog.text + assert "RuntimeError" in caplog.text diff --git a/services/analysis-engine/tests/test_transcription.py b/services/analysis-engine/tests/test_transcription.py index f9b55af93..80eb126ff 100644 --- a/services/analysis-engine/tests/test_transcription.py +++ b/services/analysis-engine/tests/test_transcription.py @@ -4,8 +4,11 @@ import io from dataclasses import dataclass +from types import SimpleNamespace +from unittest.mock import Mock import numpy as np +import pytest import soundfile as sf from bandscope_analysis.transcription import api as transcription_api @@ -62,6 +65,32 @@ def test_transcribe_bass_stem_rejects_oversized_input(monkeypatch) -> None: transcribe_bass_stem(b"abc") +@pytest.mark.parametrize( + "metadata", + [ + SimpleNamespace(frames=22050 * 121, samplerate=22050, channels=2), + SimpleNamespace(frames=22050, samplerate=7_999, channels=2), + SimpleNamespace(frames=22050, samplerate=22050, channels=3), + ], +) +def test_transcribe_bass_stem_rejects_source_metadata_before_decode( + monkeypatch: pytest.MonkeyPatch, + metadata: SimpleNamespace, +) -> None: + """Bass transcription must validate source duration, rate, and channels before librosa.""" + monkeypatch.setattr( + "bandscope_analysis.audio_metadata.soundfile.info", + lambda _fileobj: metadata, + ) + load_mock = Mock(side_effect=AssertionError("source metadata must be checked first")) + monkeypatch.setattr(transcription_api.librosa, "load", load_mock) + + with pytest.raises(ValueError, match="audio resource policy"): + transcribe_bass_stem(b"not-a-real-wav") + + load_mock.assert_not_called() + + def test_transcribe_bass_stem_wraps_pitch_tracking_parameter_errors(monkeypatch) -> None: """Return a stable ValueError when pYIN rejects decoded audio parameters.""" stem_data = _render_bass_sequence([ExpectedNote("E2", 0.0, 0.45)]) diff --git a/services/analysis-engine/tests/test_youtube.py b/services/analysis-engine/tests/test_youtube.py index 5531ac9d5..0ae449aa9 100644 --- a/services/analysis-engine/tests/test_youtube.py +++ b/services/analysis-engine/tests/test_youtube.py @@ -2,12 +2,22 @@ import importlib import sys +from pathlib import Path from unittest.mock import MagicMock, patch import pytest import yt_dlp # type: ignore -from bandscope_analysis.youtube import MAX_YOUTUBE_URL_LENGTH, download_youtube_audio, validate_url +from bandscope_analysis.audio_resource_policy import DEFAULT_MAX_ENCODED_FILE_BYTES +from bandscope_analysis.youtube import ( + MAX_YOUTUBE_URL_LENGTH, + YOUTUBE_SIZE_EXCEEDED_MESSAGE, + _owned_file_path, + _remove_download_artifacts, + _remove_owned_file, + download_youtube_audio, + validate_url, +) def test_validate_url() -> None: @@ -89,20 +99,23 @@ def test_download_youtube_audio_success( "id": "abc123DEF45", "title": "Test Video", "duration": 60, + "filesize": True, + "filesize_approx": float("nan"), } + out_dir = str(Path("/tmp").resolve()) mock_ydl.extract_info.return_value = mock_info - mock_ydl.prepare_filename.return_value = "/tmp/abc123DEF45.webm" + mock_ydl.prepare_filename.return_value = f"{out_dir}/abc123DEF45.webm" mock_exists.return_value = True mock_getsize.return_value = 10 * 1024 * 1024 input_url = "https://youtube.com/watch?v=abc123DEF45" - result = download_youtube_audio(input_url, "/tmp") + result = download_youtube_audio(input_url, out_dir) assert result["ok"] is True assert result["metadata"]["id"] == "abc123DEF45" assert result["metadata"]["title"] == "Test Video" assert result["metadata"]["duration"] == 60 - assert result["metadata"]["filepath"] == "/tmp/abc123DEF45.webm" + assert result["metadata"]["filepath"] == f"{out_dir}/abc123DEF45.webm" # Assert that YoutubeDL was initialized with the correct options mock_ydl_class.assert_called_once() @@ -114,6 +127,8 @@ def test_download_youtube_audio_success( assert called_opts["noplaylist"] is True assert called_opts["geo_bypass"] is False assert called_opts["postprocessors"] == [{"key": "FFmpegExtractAudio"}] + assert called_opts["max_filesize"] == DEFAULT_MAX_ENCODED_FILE_BYTES + assert called_opts["progress_hooks"] assert "%(id)s.%(ext)s" in called_opts["outtmpl"] # Verify extract_info was called twice correctly: once for metadata, once for download @@ -145,21 +160,22 @@ def test_download_youtube_audio_converted_extension( "title": "Test Video", "duration": 60, } + out_dir = str(Path("/tmp").resolve()) mock_ydl.extract_info.return_value = mock_info - mock_ydl.prepare_filename.return_value = "/tmp/abc123DEF45.webm" + mock_ydl.prepare_filename.return_value = f"{out_dir}/abc123DEF45.webm" # os.path.exists returns False for .webm, but True for the converted .opus. def exists_side_effect(path: str) -> bool: """Mock exists function to simulate converted extension file presence.""" - return path == "/tmp/abc123DEF45.opus" + return path == f"{out_dir}/abc123DEF45.opus" mock_exists.side_effect = exists_side_effect mock_getsize.return_value = 10 * 1024 * 1024 - result = download_youtube_audio("https://youtube.com/watch?v=abc123DEF45", "/tmp") + result = download_youtube_audio("https://youtube.com/watch?v=abc123DEF45", out_dir) assert result["ok"] is True - assert result["metadata"]["filepath"] == "/tmp/abc123DEF45.opus" + assert result["metadata"]["filepath"] == f"{out_dir}/abc123DEF45.opus" @patch("bandscope_analysis.youtube.os.path.exists") @@ -273,6 +289,50 @@ def test_download_youtube_audio_duration_exceeded(mock_ydl_class: MagicMock) -> assert result["error"]["code"] == "duration_exceeded" +@patch("bandscope_analysis.youtube.os.path.getsize") +@patch("bandscope_analysis.youtube.os.path.exists") +@patch("bandscope_analysis.youtube.yt_dlp.YoutubeDL") +def test_download_youtube_audio_accepts_size_between_legacy_and_canonical_ceiling( + mock_ydl_class: MagicMock, + mock_exists: MagicMock, + mock_getsize: MagicMock, +) -> None: + """A 60 MiB download that the old 50 MB check rejected is now accepted.""" + mock_ydl = MagicMock() + mock_ydl_class.return_value.__enter__.return_value = mock_ydl + out_dir = str(Path("/tmp").resolve()) + mock_ydl.extract_info.return_value = {"id": "abc123DEF45", "duration": 10 * 60} + mock_ydl.prepare_filename.return_value = f"{out_dir}/abc123DEF45.m4a" + mock_exists.return_value = True + mock_getsize.return_value = 60 * 1024 * 1024 + + result = download_youtube_audio("https://youtube.com/watch?v=abc123DEF45", out_dir) + + assert result["ok"] is True + assert result["metadata"]["filepath"] == f"{out_dir}/abc123DEF45.m4a" + + +@patch("bandscope_analysis.youtube.os.path.getsize") +@patch("bandscope_analysis.youtube.os.path.exists") +@patch("bandscope_analysis.youtube.yt_dlp.YoutubeDL") +def test_download_youtube_audio_accepts_exact_policy_ceiling( + mock_ydl_class: MagicMock, + mock_exists: MagicMock, + mock_getsize: MagicMock, +) -> None: + """An encoded YouTube file exactly at the 100 MiB ceiling is accepted.""" + mock_ydl = MagicMock() + mock_ydl_class.return_value.__enter__.return_value = mock_ydl + mock_ydl.extract_info.return_value = {"id": "abc123DEF45", "duration": 10 * 60} + mock_ydl.prepare_filename.return_value = "/tmp/abc123DEF45.m4a" + mock_exists.return_value = True + mock_getsize.return_value = DEFAULT_MAX_ENCODED_FILE_BYTES + + result = download_youtube_audio("https://youtube.com/watch?v=abc123DEF45", "/tmp") + + assert result["ok"] is True + + @patch("bandscope_analysis.youtube.os.path.getsize") @patch("bandscope_analysis.youtube.os.path.exists") @patch("bandscope_analysis.youtube.os.remove") @@ -283,18 +343,315 @@ def test_download_youtube_audio_size_exceeded( mock_exists: MagicMock, mock_getsize: MagicMock, ) -> None: - """Test download fails if size exceeds 50MB.""" + """Post-download files one byte over the canonical 100 MiB ceiling are deleted.""" + mock_ydl = MagicMock() + mock_ydl_class.return_value.__enter__.return_value = mock_ydl + out_dir = str(Path("/tmp").resolve()) + mock_ydl.extract_info.return_value = {"id": "abc123DEF45", "duration": 10 * 60} + mock_ydl.prepare_filename.return_value = f"{out_dir}/abc123DEF45.m4a" + mock_exists.return_value = True + mock_getsize.return_value = DEFAULT_MAX_ENCODED_FILE_BYTES + 1 + + result = download_youtube_audio("https://youtube.com/watch?v=abc123DEF45", out_dir) + assert result["ok"] is False + assert result["error"]["code"] == "size_exceeded" + assert result["error"]["message"] == YOUTUBE_SIZE_EXCEEDED_MESSAGE + mock_remove.assert_called_with(f"{out_dir}/abc123DEF45.m4a") + + +@patch("bandscope_analysis.youtube.os.path.getsize") +@patch("bandscope_analysis.youtube.os.path.exists") +@patch("bandscope_analysis.youtube.os.remove") +@patch("bandscope_analysis.youtube.yt_dlp.YoutubeDL") +def test_download_youtube_audio_oversize_skips_remove_when_file_already_gone( + mock_ydl_class: MagicMock, + mock_remove: MagicMock, + mock_exists: MagicMock, + mock_getsize: MagicMock, +) -> None: + """A vanished oversize artifact still fails closed without a remove race.""" mock_ydl = MagicMock() mock_ydl_class.return_value.__enter__.return_value = mock_ydl mock_ydl.extract_info.return_value = {"id": "abc123DEF45", "duration": 10 * 60} mock_ydl.prepare_filename.return_value = "/tmp/abc123DEF45.m4a" + mock_exists.side_effect = [True, False] + mock_getsize.return_value = DEFAULT_MAX_ENCODED_FILE_BYTES + 1 + + result = download_youtube_audio("https://youtube.com/watch?v=abc123DEF45", "/tmp") + + assert result["ok"] is False + assert result["error"]["code"] == "size_exceeded" + mock_remove.assert_not_called() + + +@patch("bandscope_analysis.youtube.yt_dlp.YoutubeDL") +def test_download_youtube_audio_rejects_announced_filesize_before_download( + mock_ydl_class: MagicMock, +) -> None: + """Announced filesize over the policy ceiling must not start the download.""" + mock_ydl = MagicMock() + mock_ydl_class.return_value.__enter__.return_value = mock_ydl + mock_ydl.extract_info.return_value = { + "id": "abc123DEF45", + "duration": 60, + "filesize": DEFAULT_MAX_ENCODED_FILE_BYTES + 1, + } + + result = download_youtube_audio("https://youtube.com/watch?v=abc123DEF45", "/tmp") + + assert result["ok"] is False + assert result["error"]["code"] == "size_exceeded" + assert result["error"]["message"] == YOUTUBE_SIZE_EXCEEDED_MESSAGE + mock_ydl.extract_info.assert_called_once_with( + "https://youtube.com/watch?v=abc123DEF45", + download=False, + ) + + +@pytest.mark.parametrize( + "info", + [ + { + "id": "abc123DEF45", + "duration": 60, + "filesize_approx": DEFAULT_MAX_ENCODED_FILE_BYTES + 1, + }, + { + "id": "abc123DEF45", + "duration": 60, + "filesize_approx": float(DEFAULT_MAX_ENCODED_FILE_BYTES) + 0.5, + }, + ], +) +@patch("bandscope_analysis.youtube.yt_dlp.YoutubeDL") +def test_download_youtube_audio_rejects_announced_approximate_oversize( + mock_ydl_class: MagicMock, + info: dict[str, object], +) -> None: + """Approximate oversize metadata rejects the import before download starts.""" + mock_ydl = MagicMock() + mock_ydl_class.return_value.__enter__.return_value = mock_ydl + mock_ydl.extract_info.return_value = info + + result = download_youtube_audio("https://youtube.com/watch?v=abc123DEF45", "/tmp") + + assert result["ok"] is False + assert result["error"]["code"] == "size_exceeded" + mock_ydl.extract_info.assert_called_once() + + +@patch("bandscope_analysis.youtube.yt_dlp.YoutubeDL") +def test_download_youtube_audio_progress_hook_aborts_over_budget( + mock_ydl_class: MagicMock, +) -> None: + """In-flight progress that crosses the encoded-byte ceiling fails closed.""" + mock_ydl = MagicMock() + mock_ydl_class.return_value.__enter__.return_value = mock_ydl + + def extract_info(_url: str, download: bool = False) -> dict[str, object]: + """Invoke the registered progress hook when the download starts.""" + if download: + hook = mock_ydl_class.call_args[0][0]["progress_hooks"][0] + hook( + { + "status": "downloading", + "downloaded_bytes": DEFAULT_MAX_ENCODED_FILE_BYTES + 1, + } + ) + return {"id": "abc123DEF45", "duration": 60} + + mock_ydl.extract_info.side_effect = extract_info + + result = download_youtube_audio("https://youtube.com/watch?v=abc123DEF45", "/tmp") + + assert result["ok"] is False + assert result["error"]["code"] == "size_exceeded" + assert result["error"]["message"] == YOUTUBE_SIZE_EXCEEDED_MESSAGE + + +@patch("bandscope_analysis.youtube.yt_dlp.YoutubeDL") +def test_download_youtube_audio_progress_hook_deletes_partial_artifacts( + mock_ydl_class: MagicMock, + tmp_path: Path, +) -> None: + """In-flight abort must delete written partials so they cannot fill the cache.""" + out_dir = tmp_path / "import-cache" + out_dir.mkdir() + outsider = tmp_path / "unrelated-youtube-partial.part" + partial = out_dir / "abc123DEF45.m4a.part" + fragment = out_dir / "abc123DEF45.m4a-Frag1" + control = out_dir / "abc123DEF45.m4a.ytdl" + keep = out_dir / "keep-me.txt" + partial.write_bytes(b"partial-cache-bytes") + fragment.write_bytes(b"hls-fragment-bytes") + control.write_bytes(b"ytdl-control-bytes") + keep.write_bytes(b"unrelated-cache-note") + outsider.write_bytes(b"must-not-delete") + mock_ydl = MagicMock() + mock_ydl_class.return_value.__enter__.return_value = mock_ydl + + def extract_info(_url: str, download: bool = False) -> dict[str, object]: + """Abort after yt-dlp has already written the current block to disk.""" + if download: + hook = mock_ydl_class.call_args[0][0]["progress_hooks"][0] + hook( + { + "status": "downloading", + "downloaded_bytes": DEFAULT_MAX_ENCODED_FILE_BYTES + 1, + "tmpfilename": str(partial), + "filename": str(out_dir / "abc123DEF45.m4a"), + } + ) + return {"id": "abc123DEF45", "duration": 60} + + mock_ydl.extract_info.side_effect = extract_info + + result = download_youtube_audio( + "https://youtube.com/watch?v=abc123DEF45", + str(out_dir), + ) + + assert result["ok"] is False + assert result["error"]["code"] == "size_exceeded" + assert result["error"]["message"] == YOUTUBE_SIZE_EXCEEDED_MESSAGE + assert not partial.exists() + assert not fragment.exists() + assert not control.exists() + assert keep.exists() + assert outsider.exists() + + +def test_owned_file_path_rejects_empty_foreign_and_unresolvable_paths( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Abort cleanup must not follow empty, escaped, or unresolvable paths.""" + out_dir = tmp_path / "import-cache" + out_dir.mkdir() + escaped = tmp_path / "outside.part" + escaped.write_bytes(b"keep") + + assert _owned_file_path(None, str(out_dir)) is None + assert _owned_file_path("", str(out_dir)) is None + assert _owned_file_path(str(out_dir), str(out_dir)) is None + assert _owned_file_path(str(escaped), str(out_dir)) is None + + def boom(_path: str) -> str: + """Simulate a filesystem error while resolving a candidate path.""" + raise OSError("realpath failed") + + monkeypatch.setattr("bandscope_analysis.youtube.os.path.realpath", boom) + assert _owned_file_path(str(out_dir / "clip.part"), str(out_dir)) is None + + +def test_remove_owned_file_ignores_missing_directories_and_remove_races( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Owned cleanup skips non-files and treats remove races as already gone.""" + out_dir = tmp_path / "import-cache" + nested = out_dir / "nested-dir" + nested.mkdir(parents=True) + _remove_owned_file(None, str(out_dir)) + _remove_owned_file(str(nested), str(out_dir)) + assert nested.is_dir() + + target = out_dir / "clip.part" + target.write_bytes(b"partial") + + def boom(_path: str) -> None: + """Simulate a disappearing file during abort cleanup.""" + raise OSError("remove failed") + + monkeypatch.setattr("bandscope_analysis.youtube.os.remove", boom) + _remove_owned_file(str(target), str(out_dir)) + assert target.exists() + + +def test_remove_download_artifacts_skips_empty_status_and_unlistable_cache( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Artifact sweep no-ops when yt-dlp omitted paths or the cache vanished.""" + out_dir = tmp_path / "import-cache" + out_dir.mkdir() + leftover = out_dir / "other-file.txt" + leftover.write_bytes(b"keep") + _remove_download_artifacts({"tmpfilename": None, "filename": 12}, str(out_dir)) + assert leftover.exists() + + partial = out_dir / "abc123DEF45.m4a.part" + partial.write_bytes(b"partial") + + def boom(_path: str) -> list[str]: + """Simulate the import cache disappearing after the first delete.""" + raise OSError("listdir failed") + + monkeypatch.setattr("bandscope_analysis.youtube.os.listdir", boom) + _remove_download_artifacts({"tmpfilename": str(partial)}, str(out_dir)) + assert leftover.exists() + + +@patch("bandscope_analysis.youtube.os.path.getsize") +@patch("bandscope_analysis.youtube.os.path.exists") +@patch("bandscope_analysis.youtube.yt_dlp.YoutubeDL") +def test_download_youtube_audio_progress_hook_ignores_non_budget_updates( + mock_ydl_class: MagicMock, + mock_exists: MagicMock, + mock_getsize: MagicMock, +) -> None: + """Unknown statuses and non-integer byte fields do not abort a valid download.""" + mock_ydl = MagicMock() + mock_ydl_class.return_value.__enter__.return_value = mock_ydl + mock_ydl.extract_info.return_value = {"id": "abc123DEF45", "duration": 60} + mock_ydl.prepare_filename.return_value = "/tmp/abc123DEF45.m4a" mock_exists.return_value = True - mock_getsize.return_value = 51 * 1024 * 1024 + mock_getsize.return_value = 10 * 1024 * 1024 result = download_youtube_audio("https://youtube.com/watch?v=abc123DEF45", "/tmp") + hook = mock_ydl_class.call_args[0][0]["progress_hooks"][0] + hook({"status": "error"}) + hook({"status": "downloading", "downloaded_bytes": True}) + hook({"status": "downloading", "downloaded_bytes": 12.5}) + hook({"status": "downloading", "downloaded_bytes": 10}) + hook({"status": "finished", "total_bytes": DEFAULT_MAX_ENCODED_FILE_BYTES}) + + assert result["ok"] is True + + +@patch("bandscope_analysis.youtube.yt_dlp.YoutubeDL") +def test_download_youtube_audio_maps_max_filesize_download_error( + mock_ydl_class: MagicMock, +) -> None: + """yt-dlp max-filesize aborts become the payload-safe size-exceeded result.""" + mock_ydl = MagicMock() + mock_ydl_class.return_value.__enter__.return_value = mock_ydl + mock_ydl.extract_info.side_effect = yt_dlp.utils.DownloadError( + "File is larger than max-filesize" + ) + + result = download_youtube_audio("https://youtube.com/watch?v=abc123DEF45", "/tmp") + assert result["ok"] is False assert result["error"]["code"] == "size_exceeded" - mock_remove.assert_called_with("/tmp/abc123DEF45.m4a") + assert result["error"]["message"] == YOUTUBE_SIZE_EXCEEDED_MESSAGE + assert "max-filesize" not in result["error"]["message"] + + +@patch("bandscope_analysis.youtube.yt_dlp.YoutubeDL") +def test_download_youtube_audio_maps_mib_limit_download_error( + mock_ydl_class: MagicMock, +) -> None: + """Download errors that mention the 100 MiB ceiling stay payload-safe.""" + mock_ydl = MagicMock() + mock_ydl_class.return_value.__enter__.return_value = mock_ydl + mock_ydl.extract_info.side_effect = yt_dlp.utils.DownloadError(YOUTUBE_SIZE_EXCEEDED_MESSAGE) + + result = download_youtube_audio("https://youtube.com/watch?v=abc123DEF45", "/tmp") + + assert result["ok"] is False + assert result["error"]["code"] == "size_exceeded" + assert result["error"]["message"] == YOUTUBE_SIZE_EXCEEDED_MESSAGE def test_main_block(monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]) -> None: @@ -326,38 +683,35 @@ def test_main_block(monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixtu def test_module_execution( - monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + tmp_path: Path, ) -> None: - """Test the if __name__ == '__main__' block using runpy.""" + """Test module execution against a real owned output path without network I/O.""" import runpy import bandscope_analysis.youtube + downloaded_path = tmp_path / "abc123DEF45.m4a" + downloaded_path.write_bytes(b"test-audio") test_args = [ "youtube.py", "--url", "https://youtube.com/watch?v=abc123DEF45", "--out-dir", - "/tmp", + str(tmp_path), ] monkeypatch.setattr(sys, "argv", test_args) - # Mock yt_dlp so runpy doesn't actually download + # Mock only the downloader/network boundary. Real filesystem semantics are + # required so the completed-path ownership check remains exercised. mock_yt_dlp = MagicMock() mock_ydl = MagicMock() mock_yt_dlp.YoutubeDL.return_value.__enter__.return_value = mock_ydl mock_ydl.extract_info.return_value = {"id": "abc123DEF45"} - mock_ydl.prepare_filename.return_value = "/tmp/abc123DEF45.m4a" + mock_ydl.prepare_filename.return_value = str(downloaded_path) monkeypatch.setitem(sys.modules, "yt_dlp", mock_yt_dlp) - # Mock os to ensure runpy uses our mocked filesystem methods - mock_os = MagicMock() - # Keep some essential attributes - mock_os.path = MagicMock() - mock_os.path.exists.return_value = True - mock_os.path.getsize.return_value = 10 * 1024 * 1024 - monkeypatch.setitem(sys.modules, "os", mock_os) - with patch.object(sys, "exit") as mock_exit: runpy.run_path(bandscope_analysis.youtube.__file__, run_name="__main__") mock_exit.assert_called_with(0) diff --git a/services/analysis-engine/tests/test_youtube_downloaded_duration_revalidation.py b/services/analysis-engine/tests/test_youtube_downloaded_duration_revalidation.py new file mode 100644 index 000000000..6780203f6 --- /dev/null +++ b/services/analysis-engine/tests/test_youtube_downloaded_duration_revalidation.py @@ -0,0 +1,40 @@ +"""Post-download YouTube duration revalidation regressions.""" + +from pathlib import Path +from unittest.mock import MagicMock, patch + +from bandscope_analysis.youtube import download_youtube_audio + + +@patch("bandscope_analysis.youtube.os.path.exists") +@patch("bandscope_analysis.youtube.os.path.isfile") +@patch("bandscope_analysis.youtube.os.remove") +@patch("bandscope_analysis.youtube.yt_dlp.YoutubeDL") +def test_youtube_revalidates_downloaded_duration_before_returning_success( + mock_ydl_class: MagicMock, + mock_remove: MagicMock, + mock_isfile: MagicMock, + mock_exists: MagicMock, +) -> None: + """Changed download metadata must not bypass the 15-minute admission limit.""" + mock_ydl = MagicMock() + mock_ydl_class.return_value.__enter__.return_value = mock_ydl + out_dir = str(Path("/tmp").resolve()) + mock_ydl.extract_info.side_effect = [ + {"id": "abc123DEF45", "duration": 60}, + {"id": "abc123DEF45", "title": "Changed metadata", "duration": 16 * 60}, + ] + mock_ydl.prepare_filename.return_value = f"{out_dir}/abc123DEF45.m4a" + mock_exists.return_value = True + mock_isfile.return_value = True + + result = download_youtube_audio("https://youtube.com/watch?v=abc123DEF45", out_dir) + + assert result == { + "ok": False, + "error": { + "code": "duration_exceeded", + "message": "Video exceeds the 15-minute limit.", + }, + } + mock_remove.assert_called_once_with(f"{out_dir}/abc123DEF45.m4a") diff --git a/services/analysis-engine/tests/test_youtube_duration_contract.py b/services/analysis-engine/tests/test_youtube_duration_contract.py new file mode 100644 index 000000000..0cb168787 --- /dev/null +++ b/services/analysis-engine/tests/test_youtube_duration_contract.py @@ -0,0 +1,54 @@ +"""Fail-closed YouTube duration metadata admission contract.""" + +from __future__ import annotations + +from unittest.mock import MagicMock, patch + +import pytest + +from bandscope_analysis.youtube import download_youtube_audio + + +class _NonCanonicalFloat(float): + """Numeric subtype that must not cross the untrusted metadata boundary.""" + + +@pytest.mark.parametrize( + "duration", + [ + True, + 0, + -1, + float("nan"), + float("inf"), + "60", + object(), + _NonCanonicalFloat(60.0), + ], +) +@patch("bandscope_analysis.youtube.yt_dlp.YoutubeDL") +def test_youtube_rejects_malformed_announced_duration_before_download( + mock_ydl_class: MagicMock, + duration: object, +) -> None: + """Malformed known-duration metadata must not authorize a media download.""" + mock_ydl = MagicMock() + mock_ydl_class.return_value.__enter__.return_value = mock_ydl + mock_ydl.extract_info.return_value = { + "id": "abc123DEF45", + "duration": duration, + } + + result = download_youtube_audio("https://youtube.com/watch?v=abc123DEF45", "/tmp") + + assert result == { + "ok": False, + "error": { + "code": "download_error", + "message": "YouTube import failed. Please use a local audio file instead.", + }, + } + mock_ydl.extract_info.assert_called_once_with( + "https://youtube.com/watch?v=abc123DEF45", + download=False, + ) diff --git a/services/analysis-engine/tests/test_youtube_post_download_path_authority.py b/services/analysis-engine/tests/test_youtube_post_download_path_authority.py new file mode 100644 index 000000000..75b23c5eb --- /dev/null +++ b/services/analysis-engine/tests/test_youtube_post_download_path_authority.py @@ -0,0 +1,74 @@ +"""Regression coverage for post-download YouTube path authority. + +The downloader owns only artifacts that resolve beneath the per-import output +directory. Metadata returned by yt-dlp must not turn an arbitrary filesystem path +into a successful import or deletion target. +""" + +from pathlib import Path +from unittest.mock import MagicMock, patch + +from bandscope_analysis.audio_resource_policy import DEFAULT_MAX_ENCODED_FILE_BYTES +from bandscope_analysis.youtube import YOUTUBE_IMPORT_FAILED_MESSAGE, download_youtube_audio + + +def _configure_download(mock_ydl_class: MagicMock, filepath: Path) -> None: + """Configure yt-dlp to report one completed download at ``filepath``.""" + mock_ydl = MagicMock() + mock_ydl_class.return_value.__enter__.return_value = mock_ydl + mock_ydl.extract_info.return_value = { + "id": "abc123DEF45", + "title": "Authority regression", + "duration": 60, + } + mock_ydl.prepare_filename.return_value = str(filepath) + + +@patch("bandscope_analysis.youtube.yt_dlp.YoutubeDL") +def test_download_rejects_foreign_completed_path( + mock_ydl_class: MagicMock, + tmp_path: Path, +) -> None: + """A completed path outside this import directory must never become success metadata.""" + out_dir = tmp_path / "import-cache" + out_dir.mkdir() + foreign = tmp_path / "foreign.m4a" + foreign.write_bytes(b"not-owned-by-this-import") + _configure_download(mock_ydl_class, foreign) + + result = download_youtube_audio( + "https://youtube.com/watch?v=abc123DEF45", + str(out_dir), + ) + + assert result == { + "ok": False, + "error": {"code": "download_error", "message": YOUTUBE_IMPORT_FAILED_MESSAGE}, + } + assert foreign.read_bytes() == b"not-owned-by-this-import" + + +@patch("bandscope_analysis.youtube.yt_dlp.YoutubeDL") +def test_oversize_foreign_completed_path_is_not_deleted( + mock_ydl_class: MagicMock, + tmp_path: Path, +) -> None: + """Oversize rejection must not delete a path outside this import's authority.""" + out_dir = tmp_path / "import-cache" + out_dir.mkdir() + foreign = tmp_path / "foreign-oversize.m4a" + with foreign.open("wb") as handle: + handle.truncate(DEFAULT_MAX_ENCODED_FILE_BYTES + 1) + _configure_download(mock_ydl_class, foreign) + + result = download_youtube_audio( + "https://youtube.com/watch?v=abc123DEF45", + str(out_dir), + ) + + assert result == { + "ok": False, + "error": {"code": "download_error", "message": YOUTUBE_IMPORT_FAILED_MESSAGE}, + } + assert foreign.exists() + assert foreign.stat().st_size == DEFAULT_MAX_ENCODED_FILE_BYTES + 1