Skip to content

perf+security: mimalloc, cargo-deny bans, refname validation, markdown escape, ureq Agent reuse#516

Open
BryanFRD wants to merge 4 commits into
fix/security-hardening-and-cwd-bugsfrom
feat/quick-wins-perf-security
Open

perf+security: mimalloc, cargo-deny bans, refname validation, markdown escape, ureq Agent reuse#516
BryanFRD wants to merge 4 commits into
fix/security-hardening-and-cwd-bugsfrom
feat/quick-wins-perf-security

Conversation

@BryanFRD
Copy link
Copy Markdown
Contributor

@BryanFRD BryanFRD commented May 24, 2026

Stacked on top of #505. Four quick-win items from the second audit pass.

Closes #509, #511, #512. First item of #507.

1. mimalloc allocator (first item of #507)

`#[global_allocator] static GLOBAL: mimalloc::MiMalloc = mimalloc::MiMalloc;` in `src/main.rs`. Gated behind the `cli` feature. Expected 5-15% wall-time on TagIndex::build / revwalk / regex captures. +200 KB binary.

2. cargo-deny bans (#511)

cargo-deny was already wired in CI but the bans list was empty. Added denials for `git2` / `libgit2-sys` (regression guard post-#487) and `openssl-sys` / `openssl-src` (replaced by rustls).

3. Markdown escape + refname validation (#512)

  • `format_preview_comment` now goes through `escape_md_cell` for every interpolation. Closes a markdown injection vector where a package name containing `|`, `<script>`, newline, or `](evil)` could break the table or inject content (XSS-prone on Gitea/Forgejo).
  • `src/git/validate.rs::ensure_safe_refname_fragment` rejects refnames with leading `-` (flag confusion), NUL, newline, or control chars. Called at the entry of `create_tag`, `create_or_move_tag`, `push_branch`, `push_tags`, `verify_remote_branch`, `reset_branch_to_remote`. `git tag` invocations also gained `--` separators.

4. ureq Agent reuse (#509)

`ureq::get` / `ureq::post` / `ureq::patch` / `ureq::put` create a fresh Agent + TLS handshake per call. A 50-pkg release does ~150 HTTPS round-trips against `api.github.com` — each was paying a fresh handshake.

Built one `ureq::Agent` in `build_forge()`, stored on `GitHubForge` / `GitLabForge`, reused for every HTTP call. HTTP keep-alive saves 2-8 seconds on a 50-pkg release.

Test plan

BryanFRD added 3 commits May 24, 2026 14:11
Default allocator (glibc malloc on Linux, HeapAlloc on Windows) is
suboptimal for alloc-heavy short-lived CLIs. mimalloc consistently
shaves 5-15% wall time on workloads that match ours (TagIndex::build,
revwalk + commit message decode, regex captures during conventional-
commit parsing).

Gated behind the cli feature so the wasm build doesn't pull it in.
Adds ~200 KB to the release binary; net positive on perf benches.

First item from #507.
cargo-deny already runs in CI (security job), but the bans section was
empty. Add explicit denials for:
- git2 / libgit2-sys: just migrated off in #487, prevent regression
- openssl-sys / openssl-src: vendored via libgit2's old chain, the gix
  migration moved us to rustls. Reintroducing would double binary size
  and inherit OpenSSL's CVE cadence

Closes #511.
Closes #512.

## Preview PR comment markdown escaping

format_preview_comment was interpolating pkg.name / pkg.current_version
/ pkg.next_version / pkg.bump_type directly into a markdown table —
all of which come from user-controlled .ferrflow + version files on
the PRs HEAD. A package name like foo|<script> broke the table on
github.com (rendered fine but visible) and triggered actual HTML on
custom forge installs (Gitea, Forgejo with permissive markdown).

Added escape_md_cell: encodes |, newline, <, >, backslash, backtick.
6 unit tests covering pipe-break, HTML injection, link injection,
backtick code, newline-row-break.

## Refname validation

New src/git/validate.rs::ensure_safe_refname_fragment rejects:
- empty
- leading - (flag confusion: --exec=ls etc.)
- NUL byte
- newline / carriage return
- control characters except tab

Called from create_tag, create_or_move_tag, push_branch, push_tags,
verify_remote_branch, reset_branch_to_remote.

The git tag invocations also gained a -- separator before the tag
name so an exotic value cant be re-interpreted as a flag even if the
validator misses something.

## Tests

- 514 lib + 629 bin tests pass (was 514 + 616)
- clippy -D warnings clean
- New tests: 7 in validate.rs, 6 in preview.rs::tests
Copilot AI review requested due to automatic review settings May 24, 2026 12:17
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

The bare ureq::get / ureq::post helpers create a fresh Agent (and TLS
handshake) per call. A 50-pkg release does ~150 HTTPS round-trips
against api.github.com (create_release × N, find_draft_release × N,
publish_release × N, plus comment + auto-merge for PR mode) — each
paying a fresh handshake.

Build one Agent in build_forge() and store it on GitHubForge /
GitLabForge. Subsequent calls reuse it via HTTP keep-alive. Expected
2-8 seconds saved on a 50-pkg release; smaller wins on single-pkg.

Closes #509.
@BryanFRD BryanFRD changed the title perf+security: mimalloc, cargo-deny bans, markdown escape + refname validation perf+security: mimalloc, cargo-deny bans, refname validation, markdown escape, ureq Agent reuse May 24, 2026
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