From 7e1f814b2c11f88349de86483e147fdb905cd94b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=9B=9B=E9=87=8E?= <114060217+siye566@users.noreply.github.com> Date: Thu, 22 Oct 2026 10:03:11 +0800 Subject: [PATCH] fix(skills): validate music audio before opening output files (#6688) * fix(skills): validate music audio before opening output files * fix(skills): reject non-MP3 music responses before writing --------- Co-authored-by: Willem Jiang --- README.md | 2 + skills/public/music-generation/SKILL.md | 3 + .../music-generation/scripts/generate.py | 36 +++++- tests/skills/test_music_generation.py | 121 +++++++++++++++++- 4 files changed, 149 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 708c6d6535..2351aef6da 100644 --- a/README.md +++ b/README.md @@ -1533,6 +1533,8 @@ Skills are what make DeerFlow do *almost anything*. A standard Agent Skill is a structured capability module — a Markdown file that defines a workflow, best practices, and references to supporting resources. DeerFlow ships with built-in skills for research, report generation, slide creation, web pages, image and video generation, and more. But the real power is extensibility: add your own skills, replace the built-in ones, or combine them into compound workflows. +The [music-generation skill](skills/public/music-generation/SKILL.md#output-handling) preserves existing output when the provider returns malformed, empty or unrecognized audio. + Skills are loaded progressively — only when the task needs them, not all at once. This keeps the context window lean and makes DeerFlow work well even with token-sensitive models. When deferred skill discovery is enabled, `describe_skill` ranks installed skills by bounded, Unicode-normalized intent-term coverage across names and descriptions. Natural multi-term requests can therefore find a relevant skill without requiring one exact phrase, while exact `select:` and required-name `+prefix` lookups remain available. Ranked searches use up to 256 characters and return up to five results; exact `select:` lists are not truncated and return all requested catalog matches. diff --git a/skills/public/music-generation/SKILL.md b/skills/public/music-generation/SKILL.md index 1310196317..94c6c42971 100644 --- a/skills/public/music-generation/SKILL.md +++ b/skills/public/music-generation/SKILL.md @@ -67,6 +67,9 @@ Do NOT read the python file, just call it with the parameters. ## Output Handling - Music is saved as MP3 (typically in `/mnt/user-data/outputs/`). +- Returned audio must decode to nonempty bytes and have an ID3 tag header or MPEG frame signature. This is a header sanity check, not complete audio decoding. +- Malformed, empty or unrecognized audio is rejected before creating the output directory or opening the output file; an existing output is preserved. +- Missing audio reports "MiniMax returned no audio data"; whitespace-only hex reports "MiniMax audio decoded to empty bytes". - Share the generated file with the user using the present_files tool. - Offer to iterate on style or lyrics if adjustments are needed. diff --git a/skills/public/music-generation/scripts/generate.py b/skills/public/music-generation/scripts/generate.py index fab5586d44..34bd2803be 100644 --- a/skills/public/music-generation/scripts/generate.py +++ b/skills/public/music-generation/scripts/generate.py @@ -10,7 +10,16 @@ MINIMAX_DEFAULT_HOST = "https://api.minimaxi.com" def _check_base_resp(payload: dict) -> None: base = payload.get("base_resp") or {} if base.get("status_code", 0) != 0: - raise Exception(f"MiniMax error {base.get('status_code')}: {base.get('status_msg')}") + raise Exception( + f"MiniMax error {base.get('status_code')}: {base.get('status_msg')}" + ) + + +def _has_mp3_header(audio: bytes) -> bool: + """Check common MP3 signatures without decoding the audio stream.""" + has_id3 = len(audio) >= 10 and audio.startswith(b"ID3") + has_mpeg_frame = len(audio) >= 4 and audio[0] == 0xFF and audio[1] & 0xE0 == 0xE0 + return has_id3 or has_mpeg_frame def generate_music(prompt_file: str, output_file: str) -> str: @@ -50,7 +59,10 @@ def generate_music(prompt_file: str, output_file: str) -> str: host = os.getenv("MINIMAX_API_HOST", MINIMAX_DEFAULT_HOST).rstrip("/") response = requests.post( f"{host}/v1/music_generation", - headers={"Authorization": f"Bearer {api_key}", "Content-Type": "application/json"}, + headers={ + "Authorization": f"Bearer {api_key}", + "Content-Type": "application/json", + }, json=body, timeout=300, ) @@ -60,20 +72,32 @@ def generate_music(prompt_file: str, output_file: str) -> str: audio_hex = (payload.get("data") or {}).get("audio") if not audio_hex: raise Exception("MiniMax returned no audio data") + # Decode before opening the output: malformed responses must not erase it. + audio = bytes.fromhex(audio_hex) + if not audio: + raise ValueError("MiniMax audio decoded to empty bytes") + # The request pins MP3; reject obvious non-audio responses before writing. + if not _has_mp3_header(audio): + raise ValueError("MiniMax audio does not have an MP3 header") output_dir = os.path.dirname(output_file) if output_dir: os.makedirs(output_dir, exist_ok=True) with open(output_file, "wb") as f: - f.write(bytes.fromhex(audio_hex)) + f.write(audio) return f"Successfully generated music to {output_file}" if __name__ == "__main__": parser = argparse.ArgumentParser(description="Generate music using MiniMax API") - parser.add_argument("--prompt-file", required=True, - help="Absolute path to JSON spec file {title, prompt, lyrics?, is_instrumental?}") - parser.add_argument("--output-file", required=True, help="Output path for generated MP3") + parser.add_argument( + "--prompt-file", + required=True, + help="Absolute path to JSON spec file {title, prompt, lyrics?, is_instrumental?}", + ) + parser.add_argument( + "--output-file", required=True, help="Output path for generated MP3" + ) args = parser.parse_args() try: diff --git a/tests/skills/test_music_generation.py b/tests/skills/test_music_generation.py index 5cce4c1267..6378c75378 100644 --- a/tests/skills/test_music_generation.py +++ b/tests/skills/test_music_generation.py @@ -8,6 +8,9 @@ from skill_loader import FakeResp, load # noqa: E402 mus = load("music-generation") +# Synthetic payloads exercise signatures, not complete audio decoding. +_ID3_AUDIO = b"ID3\x04\x00\x00\x00\x00\x00\x00\xff\xfb\x90\x64synthetic MP3 bytes" + @pytest.fixture(autouse=True) def clean_env(monkeypatch): @@ -20,8 +23,13 @@ def _post_ok(captured): captured["url"] = url captured["headers"] = headers captured["json"] = json - return FakeResp({"data": {"audio": b"songbytes".hex(), "status": 2}, - "base_resp": {"status_code": 0}}) + return FakeResp( + { + "data": {"audio": _ID3_AUDIO.hex(), "status": 2}, + "base_resp": {"status_code": 0}, + } + ) + return fake_post @@ -30,11 +38,13 @@ def test_with_lyrics_payload_and_writes(monkeypatch, tmp_path): captured = {} monkeypatch.setattr(mus.requests, "post", _post_ok(captured)) spec = tmp_path / "s.json" - spec.write_text('{"title":"X","prompt":"pop, happy","lyrics":"[verse]\\nla la"}', - encoding="utf-8") + spec.write_text( + '{"title":"X","prompt":"pop, happy","lyrics":"[verse]\\nla la"}', + encoding="utf-8", + ) out = tmp_path / "o.mp3" msg = mus.generate_music(str(spec), str(out)) - assert out.read_bytes() == b"songbytes" + assert out.read_bytes() == _ID3_AUDIO assert captured["url"].endswith("/v1/music_generation") assert captured["headers"]["Authorization"] == "Bearer m" assert captured["json"]["model"] == "music-2.6-free" @@ -81,7 +91,9 @@ def test_raises_on_base_resp_error(monkeypatch, tmp_path): monkeypatch.setenv("MINIMAX_API_KEY", "m") def fake_post(url, headers=None, json=None, **kw): - return FakeResp({"base_resp": {"status_code": 1008, "status_msg": "no balance"}}) + return FakeResp( + {"base_resp": {"status_code": 1008, "status_msg": "no balance"}} + ) monkeypatch.setattr(mus.requests, "post", fake_post) spec = tmp_path / "s.json" @@ -119,7 +131,9 @@ def test_empty_prompt_raises(monkeypatch, tmp_path): monkeypatch.setattr(mus.requests, "post", fake_post) spec = tmp_path / "s.json" - spec.write_text('{"title":"X","lyrics":"[verse]\\nhi"}', encoding="utf-8") # no prompt + spec.write_text( + '{"title":"X","lyrics":"[verse]\\nhi"}', encoding="utf-8" + ) # no prompt with pytest.raises(ValueError, match="prompt"): mus.generate_music(str(spec), str(tmp_path / "o.mp3")) @@ -133,3 +147,96 @@ def test_empty_lyrics_falls_back_to_optimizer(monkeypatch, tmp_path): mus.generate_music(str(spec), str(tmp_path / "o.mp3")) assert captured["json"]["lyrics_optimizer"] is True assert "lyrics" not in captured["json"] + + +@pytest.mark.parametrize( + "audio_hex", + [ + pytest.param("not-hex", id="invalid-hex"), + pytest.param("0", id="odd-length-hex"), + pytest.param(" \t\n", id="empty-bytes"), + pytest.param(["61"], id="non-string"), + pytest.param(b"provider failure".hex(), id="html"), + pytest.param(b'{"error":"provider failed"}'.hex(), id="json"), + pytest.param(b"arbitrary bytes".hex(), id="non-audio"), + pytest.param(b"ID3".hex(), id="truncated-id3"), + pytest.param(b"\xff\xfb".hex(), id="truncated-frame"), + pytest.param(b"\xff\x00\x90\x64".hex(), id="invalid-sync"), + ], +) +@pytest.mark.parametrize("existing", [False, True]) +def test_invalid_audio_does_not_touch_output( + monkeypatch, tmp_path, audio_hex, existing +): + monkeypatch.setenv("MINIMAX_API_KEY", "m") + monkeypatch.setattr( + mus.requests, + "post", + lambda *args, **kwargs: FakeResp( + {"base_resp": {"status_code": 0}, "data": {"audio": audio_hex}} + ), + ) + spec = tmp_path / "s.json" + spec.write_text('{"prompt":"jazz"}', encoding="utf-8") + out = tmp_path / "new-directory" / "song.mp3" + if existing: + out.parent.mkdir() + out.write_bytes(b"previous song") + + with pytest.raises((ValueError, TypeError)): + mus.generate_music(str(spec), str(out)) + + if existing: + assert out.read_bytes() == b"previous song" + else: + assert not out.exists() + assert not out.parent.exists() + + +@pytest.mark.parametrize( + "audio", + [ + _ID3_AUDIO, + b"\xff\xfb\x90\x64test", + b"\xff\xf3\x80\x64test", + b"\xff\xe3\x80\x64test", + ], + ids=["id3", "mpeg-1", "mpeg-2", "mpeg-2.5"], +) +@pytest.mark.parametrize("existing", [False, True]) +def test_valid_spaced_hex_replaces_output(monkeypatch, tmp_path, audio, existing): + monkeypatch.setenv("MINIMAX_API_KEY", "m") + monkeypatch.setattr( + mus.requests, + "post", + lambda *args, **kwargs: FakeResp( + { + "base_resp": {"status_code": 0}, + "data": {"audio": " \n".join(f"{byte:02x}" for byte in audio)}, + } + ), + ) + spec = tmp_path / "s.json" + spec.write_text('{"prompt":"jazz"}', encoding="utf-8") + out = tmp_path / "output" / "song.mp3" + if existing: + out.parent.mkdir() + out.write_bytes(b"previous song") + + assert "Successfully generated music" in mus.generate_music(str(spec), str(out)) + assert out.read_bytes() == audio + + +def test_decoded_empty_audio_has_distinct_diagnostic(monkeypatch, tmp_path): + monkeypatch.setenv("MINIMAX_API_KEY", "m") + monkeypatch.setattr( + mus.requests, + "post", + lambda *args, **kwargs: FakeResp( + {"base_resp": {"status_code": 0}, "data": {"audio": " \t\n"}} + ), + ) + spec = tmp_path / "s.json" + spec.write_text('{"prompt":"jazz"}', encoding="utf-8") + with pytest.raises(ValueError, match="^MiniMax audio decoded to empty bytes$"): + mus.generate_music(str(spec), str(tmp_path / "song.mp3"))