fix(safeoutputs): sanitize symlink paths embedded in PR description markdown

Addresses follow-up review feedback on PR #549.

The append_skipped_symlink_notice helper previously embedded raw filenames
inside an inline-code span in the PR description. Filenames may legally
contain backticks (e.g. `foo�ar`) or control characters (newlines, tabs),
which would terminate the code span and garble or break out of the blockquote.

The agent is the adversary in this code path, so the risk is display-only
(no secondary exfiltration vector), but the previous output was malformed
when adversarial filenames were involved.

CommonMark code spans do NOT honour backslash escapes — the backtick-count
rule terminates the span instead — so the naive `path.replace('`', `\\`)`
suggested in review is not actually an escape. Instead, sanitize_path_for_markdown:

  - Replaces backticks with apostrophes (visually clear, terminator-safe).
  - Collapses all ASCII control characters (newline, CR, tab, etc.) to '?'.

Display-only sanitisation: the canonical path the agent originally requested
is unchanged in the upload pipeline; only the markdown rendering of the
skipped-symlinks notice is affected.

Adds four targeted tests covering backticks, control characters, pass-through
of normal paths, and end-to-end sanitisation through append_skipped_symlink_notice.

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

Author: jamesadevine

src/safeoutputs/create_pull_request.rs

@@ -1905,12 +1905,39 @@ fn append_skipped_symlink_notice(body: &str, skipped_symlinks: &[String]) -> Str
     );
     for path in skipped_symlinks {
         notice.push_str("> - `");
-        notice.push_str(path);
+        notice.push_str(&sanitize_path_for_markdown(path));
         notice.push_str("`\n");
     }
     format!("{}{}", body, notice)
 }
 
+/// Make an arbitrary filesystem path safe to embed inside an inline-code span
+/// in a markdown PR description.
+///
+/// The agent that produced the PR is the adversary in this code path: it can
+/// plant filenames containing characters that would otherwise break out of the
+/// inline-code context and garble (or HTML-inject into) the description.
+/// CommonMark code spans do NOT honour backslash escapes (the backtick-count
+/// rule terminates the span instead), so naive `path.replace('`', "\\`")` is
+/// not actually an escape — it just inserts a stray backslash. Instead we:
+///
+/// - Replace backticks with an apostrophe (visually clear, terminator-safe).
+/// - Collapse all ASCII control characters — including newlines, carriage
+///   returns, and tabs — to a single `?` so a multi-line filename can't break
+///   the blockquote.
+///
+/// This is display-only sanitisation; the canonical path the agent actually
+/// requested is unchanged in the upload pipeline.
+fn sanitize_path_for_markdown(path: &str) -> String {
+    path.chars()
+        .map(|c| match c {
+            '`' => '\'',
+            c if c.is_control() => '?',
+            c => c,
+        })
+        .collect()
+}
+
 /// Read a file and produce an ADO push change entry.
 /// Handles both text (rawtext) and binary (base64encoded) content.
 async fn read_file_change(
@@ -2483,6 +2510,47 @@ index 0000000..abcdefg
         assert!(out.contains("`subdir/leak`"));
     }
 
+    #[test]
+    fn test_sanitize_path_for_markdown_replaces_backticks() {
+        // Backticks would otherwise terminate the inline-code span and let the
+        // adversarial filename break out of the PR description's blockquote.
+        let out = sanitize_path_for_markdown("foo`bar`baz");
+        assert!(!out.contains('`'), "all backticks must be removed: {out}");
+        assert_eq!(out, "foo'bar'baz");
+    }
+
+    #[test]
+    fn test_sanitize_path_for_markdown_collapses_control_chars() {
+        // Newlines, carriage returns, tabs, and other ASCII control characters
+        // are all replaced with '?' so a multi-line filename can't break the
+        // blockquote / list layout.
+        let out = sanitize_path_for_markdown("evil\nname\rwith\ttabs\x07bell");
+        assert_eq!(out, "evil?name?with?tabs?bell");
+    }
+
+    #[test]
+    fn test_sanitize_path_for_markdown_passthrough_normal() {
+        let out = sanitize_path_for_markdown("src/safeoutputs/create_pull_request.rs");
+        assert_eq!(out, "src/safeoutputs/create_pull_request.rs");
+    }
+
+    #[test]
+    fn test_append_skipped_symlink_notice_sanitizes_paths() {
+        // End-to-end: backticks and newlines in a filename must not break the
+        // blockquote.
+        let body = "Some PR description";
+        let skipped = vec!["evil`name\nwith\rnewlines".to_string()];
+        let out = append_skipped_symlink_notice(body, &skipped);
+        // The sanitised path appears in the listing,
+        assert!(out.contains("`evil'name?with?newlines`"));
+        // and the raw garbling characters are not anywhere in the notice.
+        let notice_only = &out[body.len()..];
+        assert!(
+            !notice_only.contains('\n') || notice_only.lines().all(|l| l.is_empty() || l.starts_with('>')),
+            "every non-empty line of the notice must remain inside the blockquote"
+        );
+    }
+
     #[test]
     fn test_validate_single_path_valid() {
         assert!(validate_single_path("src/main.rs").is_ok());