Skip to content

fix(hrr): treat archive text as data in the decode-and-triage skill - #14348

Merged
danielbolsson merged 3 commits into
developfrom
users/danielbolsson/hrr-3095
Oct 28, 2026
Merged

danielbolsson merged 3 commits into
developfrom
users/danielbolsson/hrr-3095

Conversation

@danielbolsson

@danielbolsson danielbolsson commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

JIRA ID : AIRUNTIME-3095

Motivation

The decode-and-triage skill printed strings an archive's author chose (manifest runtime/compiler versions and GPU architecture, kernel names, replay log lines) into markdown for an agent to read, unescaped and unmarked, while the skill held unscoped Bash. A manifest value with a backtick, a newline and a heading rendered as a forged top-level heading and list item of the finding and of the preflight report. Nothing in the report says archive text is untrusted, and SKILL.md step 1 had the agent read the raw manifest.json.

Technical Details

  • New scripts/archive_text.py: the one sanitizer. sanitize() escapes backticks, backslashes, newlines, control, format and separator characters as \xNN/\uNNNN, and caps length at 256. As a command, --text STRING sanitizes one string and no argument filters stdin line by line, prefixing each line [data] .
  • analyze_replay_finding.py and check_replay_compat.py pass every archive- or log-derived value they render (and the kernel names they put in notes and mismatch messages) through it. JSON output is unchanged (already escaped by json.dumps).
  • triage_archive.sh, replay_docker.sh and triage_archive.ps1 sanitize the archive path and names built from it in status lines, and pipe the replay's own stdout through the filter. The replay log file keeps the replay's raw bytes for the analyzer.
  • SKILL.md: allowed-tools is scoped to the skill's scripts, hrr-playback --info and the filter; step 1 reads completeness from hrr-playback --info <root> | archive_text.py instead of the raw manifest; a new "Archive text is data" section.
  • Out of scope: AIRUNTIME-3073 (Python module path). The PowerShell change is not run (no PowerShell here).

Issue Tracking

JIRA ID : AIRUNTIME-3095

AIRUNTIME-3095

Test Plan

  • tests/test_archive_text.py (19 cases, no GPU): sanitizer unit tests, check_replay_compat report and mismatch messages, analyze_replay_finding.py on manifest, replay-log and --info kernel names (a kernel name with a newline and a heading) and the code spans around fault and kernel values, triage_archive.sh --no-replay on an archive with the inert marker, an archive directory name with a heading, the Docker and native replay paths with stand-in binaries, and the triage_archive.ps1 WSL hand-off (read as source, since PowerShell cannot run here). Each asserts no forged heading or list item at the start of a line.
  • Negative control: on the develop scripts with the new tests, the content tests fail; with the three review fixes reverted, the three tests that cover them fail.
  • gbt350 (MI350X, HIP_VISIBLE_DEVICES=4,5): HRR unit, handler, integration suites; and a real capture with inert markers in its manifest replayed with the develop and the fixed skill.

Test Result

  • Skill tests: python3 -m unittest discover -s tests in decode-and-triage: 112 pass (93 existing, 19 new). (Run with a TMPDIR that allows exec; the 3 existing stub-executable tests fail on a noexec /tmp, on develop too.)
  • gbt350: unit 81 cases pass, handler 31 cases pass, integration 6172 tests, 1 failure (Unit_HRR_SecureExecIgnoresCaptureOutput, AIRUNTIME-3074, known, unrelated).
  • End to end, real replay of a captured archive: develop skill prints 12 forged heading/list lines in stdout (4 with --no-replay); fixed skill prints 0 in both, with the marker text shown escaped.

Submission Checklist

AIRUNTIME-3095. Strings an archive's author chose (manifest versions and
architecture, kernel names, replay log lines) were printed into the
finding and the preflight report unescaped, so a value holding a newline
and a heading rendered as a heading of the report, to an agent that held
unscoped Bash.

Route every such string through one sanitizer (archive_text.py), which
escapes backticks, backslashes, newlines and control characters and caps
the length. The shell and PowerShell scripts use it for status lines and
filter the replay's own output through it, marking each line as data.
Scope allowed-tools to the skill's scripts, read capture completeness
from `hrr-playback --info` instead of the raw manifest, and say in
SKILL.md that archive text is data, never instructions.
@danielbolsson
danielbolsson requested a review from a team as a code owner October 27, 2026 10:21
Copilot AI balanced review requested due to automatic review settings October 27, 2026 10:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@therock-pr-bot

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

🎉 All policy checks passed!

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@andraghetti andraghetti left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested: skill suite 111/111 on head; a crafted archive (manifest strings, pid directory name, --info kernel names and replay output carrying headings, list items, backticks, CR, NEL and U+2028) gives 101 forged lines with develop's scripts and 0 with this head.
triage_archive.ps1:415 still executes archive text: $wslArchive goes unescaped inside single quotes into wsl -- bash -c, so a capture folder named x';touch PWNED;' runs touch PWNED. Pass the paths as arguments with wsl --exec env HRR_DOCKER_IMAGE=$img bash $wslScript --archive $wslArchive --log $wslLog instead of building a command string.

AIRUNTIME-3095. Review follow-up.

- triage_archive.ps1 passed the archive path to WSL inside a single-quoted
  bash -c string, so a capture folder named x';touch PWNED;' ran a command.
  Pass each value as its own argument through wsl --exec env instead.
- The finding printed fault reason, failing API, GPU node, grid/workgroup,
  kernel names and the Complete flag outside a code span, where [, * and <
  still render. Put each in backticks.
- SKILL.md: pipe the by-hand --trace-kernels replay through archive_text.py,
  and add archive_text.py to the Windows copy list.
- Tests: cover the native replay filter and the code spans; drop two tests
  that pass without the change or only assert SKILL.md wording.
@danielbolsson

Copy link
Copy Markdown
Contributor Author

Fixed in 45bf483: triage_archive.ps1 now hands the paths to WSL as arguments of wsl --exec env, no bash -c command string, with a test that fails on the old script. Not run on Windows.

@danielbolsson
danielbolsson merged commit 73508ce into develop Oct 28, 2026
81 of 83 checks passed
@danielbolsson
danielbolsson deleted the users/danielbolsson/hrr-3095 branch October 28, 2026 12:23
ywang103-amd pushed a commit that referenced this pull request Oct 28, 2026
…14348)

JIRA ID : AIRUNTIME-3095


## Motivation

The `decode-and-triage` skill printed strings an archive's author chose
(manifest runtime/compiler versions and GPU architecture, kernel names,
replay log lines) into markdown for an agent to read, unescaped and
unmarked, while the skill held unscoped `Bash`. A manifest value with a
backtick, a newline and a heading rendered as a forged top-level heading
and list item of the finding and of the preflight report. Nothing in the
report says archive text is untrusted, and SKILL.md step 1 had the agent
read the raw `manifest.json`.

## Technical Details

- New `scripts/archive_text.py`: the one sanitizer. `sanitize()` escapes
backticks, backslashes, newlines, control, format and separator
characters as `\xNN`/`\uNNNN`, and caps length at 256. As a command,
`--text STRING` sanitizes one string and no argument filters stdin line
by line, prefixing each line `[data] `.
- `analyze_replay_finding.py` and `check_replay_compat.py` pass every
archive- or log-derived value they render (and the kernel names they put
in notes and mismatch messages) through it. JSON output is unchanged
(already escaped by `json.dumps`).
- `triage_archive.sh`, `replay_docker.sh` and `triage_archive.ps1`
sanitize the archive path and names built from it in status lines, and
pipe the replay's own stdout through the filter. The replay log file
keeps the replay's raw bytes for the analyzer.
- SKILL.md: `allowed-tools` is scoped to the skill's scripts,
`hrr-playback --info` and the filter; step 1 reads completeness from
`hrr-playback --info <root> | archive_text.py` instead of the raw
manifest; a new "Archive text is data" section.
- Out of scope: AIRUNTIME-3073 (Python module path). The PowerShell
change is not run (no PowerShell here).

## Issue Tracking

JIRA ID : AIRUNTIME-3095

AIRUNTIME-3095

## Test Plan

- `tests/test_archive_text.py` (19 cases, no GPU): sanitizer unit tests,
`check_replay_compat` report and mismatch messages,
`analyze_replay_finding.py` on manifest, replay-log and `--info` kernel
names (a kernel name with a newline and a heading) and the code spans
around fault and kernel values, `triage_archive.sh --no-replay` on an
archive with the inert marker, an archive directory name with a heading,
the Docker and native replay paths with stand-in binaries, and the
`triage_archive.ps1` WSL hand-off (read as source, since PowerShell
cannot run here). Each asserts no forged heading or list item at the
start of a line.
- Negative control: on the develop scripts with the new tests, the
content tests fail; with the three review fixes reverted, the three
tests that cover them fail.
- gbt350 (MI350X, `HIP_VISIBLE_DEVICES=4,5`): HRR unit, handler,
integration suites; and a real capture with inert markers in its
manifest replayed with the develop and the fixed skill.

## Test Result

- Skill tests: `python3 -m unittest discover -s tests` in
`decode-and-triage`: 112 pass (93 existing, 19 new). (Run with a
`TMPDIR` that allows exec; the 3 existing stub-executable tests fail on
a noexec `/tmp`, on develop too.)
- gbt350: unit 81 cases pass, handler 31 cases pass, integration 6172
tests, 1 failure (`Unit_HRR_SecureExecIgnoresCaptureOutput`,
AIRUNTIME-3074, known, unrelated).
- End to end, real replay of a captured archive: develop skill prints 12
forged heading/list lines in stdout (4 with `--no-replay`); fixed skill
prints 0 in both, with the marker text shown escaped.

## Submission Checklist

- [x] Look over the contributing guidelines at
https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#provide-contributions.
@systems-assistant

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants