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.fix/verification-admin-route-recovery
parent
ac18a8658b
commit
259e6b87a7
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue