Skip to content

BUG: Stop leaking absolute media paths and SAS tokens in Attack History 'Last Message'#1865

Merged
romanlutz merged 10 commits into
microsoft:mainfrom
romanlutz:romanlutz/attack-history-media-last-message
Jun 3, 2026
Merged

BUG: Stop leaking absolute media paths and SAS tokens in Attack History 'Last Message'#1865
romanlutz merged 10 commits into
microsoft:mainfrom
romanlutz:romanlutz/attack-history-media-last-message

Conversation

@romanlutz
Copy link
Copy Markdown
Contributor

Description

The Attack History "Last Message" column (and the matching surfaces in Home and the conversation panel) was rendering the raw converted_value of the last message piece. For media outputs (TTS audio, generated images, video, binary), that raw value is the absolute on-disk path of the memory artifact — e.g. C:\Users\romanlutz\git\PyRIT\dbdata\prompt-memory-entries\audio\1780010098266691.mp3 — and for blob-backed pipelines it's the fully-qualified blob URL including the SAS token query string.

Beyond looking nonsensical in a "preview" column, this is a low-grade information disclosure: in any deployment where the GUI is exposed beyond the local user (multi-user devbox, demo, screen-share), the listing leaks the operator's home directory, install layout, and — for cloud-storage targets — short-lived SAS credentials.

This PR formats media values into a friendly, path-free label at the API serialization layer and leaves text previews untouched.

Before / after

Attack History — Last Message column

Before: raw paths and a blob URL leak into the column. The username, install layout, and (for the blob row) SAS token are all visible.

Attack History — before

After: media values render as [Image: <basename>] / [Audio: <basename>] / [Video: <basename>] / [File: <basename>]. Text rows are unchanged.

Attack History — after

Home — Recent operations card

The Home view's recent-operations list consumes the same last_message_preview field, so it picks up the same fix automatically.

Before:

Home — before

After:

Home — after

ConversationPanel's branch list also reads this field, so it gets the same treatment without a screenshot here.

Implementation

The fix is split across two commits so the architectural boundary stays clean:

1. BUG Stop leaking media file paths … (918682e)

  • pyrit/memory/sqlite_memory.py and pyrit/memory/azure_sql_memory.py: get_conversation_stats now returns both last_message_preview (raw value, capped) and last_message_data_type from PromptMemoryEntries so downstream code knows whether the value is a path or text.
  • pyrit/models/conversation_stats.py: new last_message_data_type: Optional[PromptDataType] field.
  • Unit + integration coverage for both backends.

2. refactor: move last-message preview formatting from memory to backend (f9e3a60)

  • New pyrit/backend/mappers/_preview.py with format_last_message_preview(value, data_type) — owns the GUI presentation strings ([Image: …], [Audio: …], [Video: …], [File: …]) and the basename extraction (Windows + POSIX paths, HTTP/HTTPS URLs with query-string stripping, data URIs).
  • pyrit/backend/mappers/attack_mappers.py and pyrit/backend/services/attack_service.py apply the formatter when building AttackSummary / ConversationSummary.
  • Memory layer is now strictly persistence: it caps the raw value at ConversationStats.PREVIEW_FETCH_MAX_LEN (1024 chars — enough headroom for the formatter to extract a basename from a long media path or signed blob URL) and exposes the data type. No English labels in the memory layer.
  • The storage-vs-display contract is made explicit via the public ConversationStats.PREVIEW_FETCH_MAX_LEN ClassVar (referenced by both SQL backends) and PREVIEW_MAX_LEN (the formatter's default display cap), with a docstring explaining the split.
  • Future CLI / non-GUI consumers of ConversationStats get the raw value + data type and can render however they want — no GUI coupling baked into memory.

API contract

Unchanged. AttackSummary.last_message_preview and ConversationSummary.last_message_preview still carry the friendly display string. The frontend (AttackTable.tsx, Home.tsx, ConversationPanel.tsx) is not modified — formatting moved one layer down the stack, transparently to consumers.

Edge cases covered

  • Windows paths (C:\…\image\<hash>.png) and POSIX paths (/var/lib/…/video/<hash>.mp4) — basename extracted via Path(value).name.
  • HTTPS/HTTP URLs (Azure Blob with SAS token) — query string stripped before basename extraction so signed credentials don't leak.
  • Data URIs (data:image/png;base64,...) — render as the type-only label ([Image]) rather than dumping the base64.
  • Empty media values — type-only label.
  • Long text — truncated to PREVIEW_MAX_LEN with an ellipsis, exactly as before.

Testing

uv run pytest tests/unit/memory tests/unit/backend tests/unit/models
# 1673 passed, 5 skipped

Specific coverage:

  • tests/unit/backend/test_preview.py — formatter unit tests (parametrised across all 4 media types, blob URL with SAS, data URI, empty value, text passthrough, truncation).
  • tests/unit/backend/test_mappers.pyTestAttackResultToSummary::test_media_last_message_preview_hides_absolute_path (parametrised over 4 types).
  • tests/unit/backend/test_attack_service.pytest_conversation_summary_formats_media_preview, test_list_attacks_formats_media_preview.
  • tests/unit/memory/test_sqlite_memory.py — rewritten media tests assert raw value + last_message_data_type (instead of the old pre-formatted string); new test_get_conversation_stats_preview_caps_raw_value_at_fetch_limit covers the 1024-char storage cap.
  • tests/unit/models/test_conversation_stats.py — new field, PREVIEW_FETCH_MAX_LEN ClassVar, Pydantic literal validation.

romanlutz and others added 6 commits May 29, 2026 20:56
For attacks whose final message piece is a media response (TTS / image /
video / binary), the `last_message_preview` field on `AttackSummary`
and `ConversationSummary` previously returned the raw absolute on-disk
path, e.g. `C:\Users\<name>\git\PyRIT\dbdata\prompt-memory-entries\
audio\1780010098266691.mp3` — leaking it into Attack History, the Home
recent-attacks list, and the Conversation panel branch list.

Root cause: `get_conversation_stats` in both `sqlite_memory` and
`azure_sql_memory` selected `converted_value` from
`PromptMemoryEntries` without consulting `converted_value_data_type`.
For media-path types, `converted_value` *is* the path.

Fix: also fetch `converted_value_data_type` for the last piece and run
the value through a shared `format_last_message_preview` helper. Media
types now render as `[Image: <basename>]` / `[Audio: <basename>]` /
`[Video: <basename>]` / `[File: <basename>]`, hiding the username,
install layout, and deployment topology that the absolute path
exposes. Text behavior (truncation + ellipsis) is unchanged.

Also promotes `_MEDIA_PATH_TYPES` (previously private to
`attack_mappers`) to `pyrit.models.MEDIA_PATH_DATA_TYPES` so memory
and backend layers share a single source of truth and can't drift.

No DTO/API schema changes; no frontend changes required.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
`pyrit/memory/_preview.py` produced display strings ("[Image: <name>]")
which are presentation-layer text — the memory layer should stay
data-agnostic. SQL truncation of long values still belongs in memory
(don't pull MB blobs over the wire), but English labels and basename
extraction are a GUI concern and belong next to the API mappers.

Changes:

- Add `last_message_data_type: Optional[PromptDataType]` to
  `ConversationStats`. Add `PREVIEW_FETCH_MAX_LEN` ClassVar so memory
  backends have a documented contract for the storage-fetch cap
  without importing from `pyrit/backend/`.
- Memory backends (sqlite + azure_sql) now populate the raw
  (truncated-to-1024) `last_message_preview` plus the data type. No
  formatting, no `[Image: ...]` labels.
- Move `_preview.py` to `pyrit/backend/mappers/`. The formatter's
  `max_len` defaults to `ConversationStats.PREVIEW_MAX_LEN` so callers
  don't have to plumb it through.
- `attack_mappers.attack_result_to_summary` applies the formatter when
  building `AttackSummary.last_message_preview`.
- `attack_service.get_conversations_async` applies the formatter when
  building `ConversationSummary.last_message_preview`.
- `attack_service.list_attacks_async` propagates the data type into the
  merged on-the-fly `ConversationStats` so the mapper has what it needs.

Tests:

- Move `test_preview.py` from `tests/unit/memory/` to
  `tests/unit/backend/`.
- Memory tests assert raw value + correct `last_message_data_type`
  (formatting is no longer memory's job). The 200-char truncation test
  now verifies the storage-fetch cap (`PREVIEW_FETCH_MAX_LEN`) instead
  of the obsolete 103-char SQL output.
- Add mapper tests proving media paths are formatted and never leak
  absolute paths through `AttackSummary`.
- Add `attack_service` tests proving the formatter is applied for both
  `AttackSummary` (list endpoint) and `ConversationSummary` (detail
  endpoint).
- Update `test_conversation_stats.py` to cover the new field and
  reject unknown data types.

API contract unchanged: `AttackSummary.last_message_preview` and
`ConversationSummary.last_message_preview` still carry the friendly
display string; only the layer that produces them changed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
On POSIX hosts (Linux CI, macOS), pathlib.Path treats backslashes as part of the filename, so Windows-style paths stored from a Windows host (e.g. C:\Users\...\1780.png) were passed through to the preview unchanged — defeating the SAS/absolute-path leak fix.

PureWindowsPath recognises both '/' and '\' as separators on every platform, so a single code path correctly extracts the basename regardless of which host wrote the path to memory and which host renders it.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…a-last-message' into romanlutz/attack-history-media-last-message
Comment thread pyrit/backend/mappers/_preview.py Outdated
Comment thread pyrit/backend/mappers/_preview.py Outdated
Comment thread pyrit/backend/services/attack_service.py Outdated
romanlutz and others added 3 commits June 2, 2026 15:46
Replace Optional[str] with str | None and drop the now-unused

typing.Optional import. Matches the style guide convention

(list[X], str | None) enforced across the codebase.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rs package

Addresses review feedback on PR microsoft#1865:
- Collapse adjacent pyrit.models imports in _preview.py into a single line.
- Expose format_last_message_preview via pyrit.backend.mappers package
  symbol so external consumers (attack_service, tests) no longer reach
  into the private _preview module. The _preview.py filename stays as an
  implementation-detail marker; only the package's public API is used
  from outside.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ory-media-last-message

# Conflicts:
#	pyrit/models/__init__.py
After the Pydantic refactor of Score, scorer_class_identifier is Optional[ComponentIdentifier]. Guard the access and fall back to 'Unknown' to match the convention used in Score.__str__ and score_utils.py.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@romanlutz romanlutz added this pull request to the merge queue Jun 3, 2026
Merged via the queue into microsoft:main with commit 729edfa Jun 3, 2026
47 checks passed
@romanlutz romanlutz deleted the romanlutz/attack-history-media-last-message branch June 3, 2026 02:05
romanlutz added a commit to romanlutz/PyRIT that referenced this pull request Jun 3, 2026
Brings in 1 new commit from main:

- 729edfa BUG: Stop leaking absolute media paths and SAS tokens in Attack History 'Last Message' (microsoft#1865)

One conflict in `pyrit/models/conversation_stats.py` resolved
by `git checkout --theirs` then `ruff check --fix` to apply
the PEP 604 sweep (4 violations auto-fixed in the merged code).

Verification:
- ruff check pyrit/ tests/ doc/ - clean
- ruff format --check - clean
- pytest tests/unit -n 4 - 9305 passed, 6 skipped, 0 failures

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants