Skip to content

scrobble test omits artist assertion, masking always-empty artist bug #423

Description

@forkwright

Finding

The test scrobble_function_calls_api_with_track_id exercises the production scrobble function but only asserts on the submitted track id and timestamp. It never inspects the artist field. The production code hardcodes artist: String::new(), so every scrobble is submitted with an empty artist, yet the test suite reports full coverage of the scrobble path.

Evidence

crates/syndesmos/src/lastfm/scrobble.rs:22 — the production scrobble builds its request with a permanently empty artist:

let params = ScrobbleParams {
    artist: String::new(),
    track: track_id.to_string(),
    album: None,
    timestamp: Timestamp::now().as_second(),
};

crates/syndesmos/src/lastfm/scrobble.rs:65 — the test calls the real function but checks only .track and .timestamp, never .artist:

async fn scrobble_function_calls_api_with_track_id() {
    let mock = MockLastfmApi::new();
    let circuit = breaker();
    let track_id = MediaId::new();
    let user_id = UserId::new();

    scrobble(&mock, track_id, user_id, &circuit).await.unwrap();

    let submitted = mock.submitted_scrobbles();
    assert_eq!(submitted.len(), 1);
    assert_eq!(submitted[0].track, track_id.to_string());
    assert!(submitted[0].timestamp > 0);
}

The sibling test submit_scrobble_via_mock_records_correct_parameters (line 42) does assert a non-empty artist, but it feeds the mock a hand-built ScrobbleParams and never invokes the production scrobble function — so it cannot catch the hardcoded empty artist either.

Why this matters

The defect ships data the test claims to guard. A scrobble with an empty artist is a malformed Last.fm submission, but more relevant under the threat model: the test gives false coverage assurance for the exact code path that exfiltrates listening activity to a third-party network service. An operator auditing what leaves the device trusts the test suite to characterize the scrobble payload; the missing assertion means the actual outbound field set is never validated, and the empty-artist defect can survive arbitrary refactors of the scrobble path undetected.

Desired correction

Add an artist assertion to scrobble_function_calls_api_with_track_id, e.g. assert!(!submitted[0].artist.is_empty(), "artist must be non-empty for a valid Last.fm scrobble"), and assert the artist matches the source media's metadata. Update the production scrobble function to populate artist from the track/media metadata instead of String::new() so the strengthened test passes.

Done when: scrobble_function_calls_api_with_track_id asserts a non-empty artist sourced from media metadata, and the production scrobble function populates the artist field from that metadata rather than an empty string.

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions