From 259e6b87a73911eca0e71a33a81381801364868a Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 1 Jul 2026 01:44:53 -0700 Subject: [PATCH] fix(teams-pipeline): reject dot-only recording display_name Path(raw).name reduces '..'/'.'/'' to themselves, so basename extraction alone still let a Graph-provided display_name of '..' or '../' escape the temp recording directory (tmp_dir / '..' resolves to the parent). Reject the dot-only basenames explicitly and fall back to the artifact id. Extends @outsourc-e's regression coverage with the dot-only cases. --- plugins/teams_pipeline/pipeline.py | 10 +- scripts/release.py | 1 + tests/plugins/test_teams_pipeline_plugin.py | 102 ++++++++++++++++++++ 3 files changed, 111 insertions(+), 2 deletions(-) diff --git a/plugins/teams_pipeline/pipeline.py b/plugins/teams_pipeline/pipeline.py index 052325953..a4c600b11 100644 --- a/plugins/teams_pipeline/pipeline.py +++ b/plugins/teams_pipeline/pipeline.py @@ -459,8 +459,14 @@ class TeamsMeetingPipeline: # display_name comes from Graph API and is ultimately set by # the meeting organizer — strip any directory components so a # crafted name like "../../etc/cron.d/evil" can't escape tmp_dir. - raw_name = recording.display_name or f"{recording.artifact_id}.mp4" - recording_name = Path(raw_name).name or f"{recording.artifact_id}.mp4" + # Path(...).name reduces "." / ".." / "" to themselves, so the + # dot-only basenames must be rejected explicitly (joining "tmp/.." + # resolves to the parent dir); fall back to the artifact id. + fallback_name = f"{recording.artifact_id}.mp4" + raw_name = recording.display_name or fallback_name + recording_name = Path(raw_name).name + if recording_name in ("", ".", ".."): + recording_name = fallback_name recording_path = Path(tmp_dir) / recording_name await download_recording_artifact( self.graph_client, diff --git a/scripts/release.py b/scripts/release.py index dec2d0efe..b0f5ac525 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -1699,6 +1699,7 @@ AUTHOR_MAP = { "35164907+MoonJuhan@users.noreply.github.com": "MoonJuhan", # PR #28288 salvage (unreadable JSONL transcripts) "codemike@naver.com": "MoonJuhan", "201563152+outsourc-e@users.noreply.github.com": "outsourc-e", # PR #28164 salvage (cron emoji ZWJ) + "eric@outsourc-e.com": "outsourc-e", # PR #28177 salvage (Teams recording path traversal) "201803425+Zyrixtrex@users.noreply.github.com": "Zyrixtrex", # PR #28275 salvage (Google OAuth timeout) "zyrixtrex@gmail.com": "Zyrixtrex", "120500656+ooovenenoso@users.noreply.github.com": "ooovenenoso", # PR #28256 salvage (tool loop recovery hints) diff --git a/tests/plugins/test_teams_pipeline_plugin.py b/tests/plugins/test_teams_pipeline_plugin.py index c16a56e4f..c91ba0976 100644 --- a/tests/plugins/test_teams_pipeline_plugin.py +++ b/tests/plugins/test_teams_pipeline_plugin.py @@ -388,6 +388,108 @@ class TestTeamsMeetingPipeline: assert teams_record is not None assert teams_record["message_id"] == "msg-1" + @pytest.mark.parametrize("crafted_name", ["..", "../", ".", ""]) + async def test_recording_dot_only_display_name_falls_back_to_artifact_id( + self, tmp_path, monkeypatch, crafted_name + ): + # Path("..").name == ".." and Path(".").name == "" — so basename + # extraction alone does not neutralize dot-only names. Joining + # tmp_dir / ".." resolves to the parent directory (an escape), so + # the pipeline must reject these and fall back to the artifact id. + from plugins.teams_pipeline import pipeline as pipeline_module + + monkeypatch.setattr(pipeline_module, "resolve_meeting_reference", _transcript_meeting_resolver) + + async def _no_transcript(client, meeting_ref): + return None, None + + async def _recordings(client, meeting_ref): + return [ + MeetingArtifact( + artifact_type="recording", + artifact_id="rec-dot", + display_name=crafted_name, + download_url="https://files.example/recording.mp4", + ) + ] + + downloaded_targets = [] + + async def _download(client, meeting_ref, recording, destination): + target = Path(destination) + downloaded_targets.append(target) + target.write_bytes(b"video-bytes") + return {"path": str(target), "size_bytes": 11, "content_type": "video/mp4"} + + async def _prepare_audio(self, recording_path): + audio_path = recording_path.with_suffix(".wav") + audio_path.write_bytes(b"audio-bytes") + return audio_path + + def _transcribe(file_path, model): + return {"success": True, "transcript": "Action: Follow up.", "provider": "local"} + + async def _summarize(**kwargs): + return pipeline_module.TeamsMeetingSummaryPayload( + meeting_ref=kwargs["resolved_meeting"], + title="Weekly Sync", + transcript_text=kwargs["transcript_text"], + summary="Fallback summary", + key_decisions=[], + action_items=["Follow up."], + risks=[], + confidence="medium", + confidence_notes="Generated from STT fallback.", + source_artifacts=kwargs["artifacts"], + ) + + class FakeNotionWriter: + async def write_summary(self, payload, config, existing_record=None): + return {"page_id": "page-1", "url": "https://notion.so/page-1"} + + async def _teams_sender(payload, config, existing_record=None): + return {"message_id": "msg-1"} + + monkeypatch.setattr(pipeline_module, "fetch_preferred_transcript_text", _no_transcript) + monkeypatch.setattr(pipeline_module, "list_recording_artifacts", _recordings) + monkeypatch.setattr(pipeline_module, "download_recording_artifact", _download) + monkeypatch.setattr(pipeline_module.TeamsMeetingPipeline, "_prepare_audio_path", _prepare_audio) + monkeypatch.setattr(pipeline_module, "enrich_meeting_with_call_record", _no_call_record) + + temp_root = tmp_path / "teams-tmp" + store = TeamsPipelineStore(tmp_path / "teams-store.json") + pipeline = TeamsMeetingPipeline( + graph_client=FakeGraphClient(), + store=store, + config={ + "tmp_dir": str(temp_root), + "notion": {"enabled": True, "database_id": "db-1"}, + "teams_delivery": {"enabled": True, "channel_id": "channel-1"}, + }, + transcribe_fn=_transcribe, + summarize_fn=_summarize, + notion_writer=FakeNotionWriter(), + teams_sender=_teams_sender, + ) + + job = await pipeline.run_notification( + { + "id": "notif-dot", + "changeType": "updated", + "resource": "communications/onlineMeetings/meeting-dot", + "resourceData": {"id": "meeting-dot"}, + } + ) + + assert job.status == "completed" + assert downloaded_targets + target = downloaded_targets[0] + # Fell back to the artifact id, not the crafted dot-only name. + assert target.name == "rec-dot.mp4" + # Stayed inside the generated temp recording directory (no escape). + assert target.resolve().parent.parent == temp_root.resolve() + assert target.resolve().parent.name.startswith("teams-recording-") + async def test_missing_transcript_and_recording_schedules_retry(self, tmp_path, monkeypatch): from plugins.teams_pipeline import pipeline as pipeline_module