From 48e00934560e94f42d5ace84a30de0d2b1dcfb47 Mon Sep 17 00:00:00 2001 From: Julien Danjou Date: Tue, 19 May 2026 17:14:39 +0200 Subject: [PATCH 1/3] refactor(ci): swap uuid for getrandom in the GHA heredoc delimiter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ci queue-info::write_github_output` formatted a unique `ghadelimiter_` to guard against a metadata payload that happens to contain its own heredoc delimiter. The actual contract is "32 unpredictable hex chars", not "a UUID per RFC 4122" — the delimiter is never parsed by anyone, only matched as a string. Pull 16 random bytes straight from `getrandom::fill` and hex-encode them. Drops `uuid` from the direct deps (it stays unreferenced and disappears from `Cargo.lock`), with `getrandom` taking its place — which `uuid` was already pulling in transitively, so the net add is zero new code shipped to the binary. The local helper is six lines. Same blast radius for a maintainer-attack story, smaller surface to read. Co-Authored-By: Claude Opus 4.7 Change-Id: Ib6599e9b6fca49281186b726a63e4641fa32596e --- Cargo.lock | 13 +------------ crates/mergify-ci/Cargo.toml | 2 +- crates/mergify-ci/src/queue_info.rs | 20 +++++++++++++++++++- 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1c13a6c9..3b7db81c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1063,6 +1063,7 @@ name = "mergify-ci" version = "0.0.0" dependencies = [ "chrono", + "getrandom 0.3.4", "mergify-core", "mergify-test-support", "serde", @@ -1072,7 +1073,6 @@ dependencies = [ "tempfile", "tokio", "url", - "uuid", "wiremock", ] @@ -2191,17 +2191,6 @@ version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" -[[package]] -name = "uuid" -version = "1.23.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ddd74a9687298c6858e9b88ec8935ec45d22e8fd5e6394fa1bd4e99a87789c76" -dependencies = [ - "getrandom 0.4.2", - "js-sys", - "wasm-bindgen", -] - [[package]] name = "uuid-simd" version = "0.8.0" diff --git a/crates/mergify-ci/Cargo.toml b/crates/mergify-ci/Cargo.toml index 27552aef..d6ff6fbc 100644 --- a/crates/mergify-ci/Cargo.toml +++ b/crates/mergify-ci/Cargo.toml @@ -12,12 +12,12 @@ publish = false [dependencies] chrono = { version = "0.4", default-features = false, features = ["clock", "serde", "std"] } mergify-core = { path = "../mergify-core" } +getrandom = "0.3" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" serde_yaml_ng = "0.10" tokio = { version = "1", default-features = false, features = ["rt"] } url = "2" -uuid = { version = "1", features = ["v4"] } [dev-dependencies] mergify-test-support = { path = "../mergify-test-support" } diff --git a/crates/mergify-ci/src/queue_info.rs b/crates/mergify-ci/src/queue_info.rs index f3308ff2..864ef85b 100644 --- a/crates/mergify-ci/src/queue_info.rs +++ b/crates/mergify-ci/src/queue_info.rs @@ -48,7 +48,7 @@ fn write_github_output(metadata: &MergeQueueMetadata) -> Result<(), CliError> { let Some(path) = env::var("GITHUB_OUTPUT").ok().filter(|s| !s.is_empty()) else { return Ok(()); }; - let delimiter = format!("ghadelimiter_{}", uuid::Uuid::new_v4()); + let delimiter = format!("ghadelimiter_{}", random_delimiter_suffix()?); let compact = serde_json::to_string(metadata) .map_err(|e| CliError::Generic(format!("failed to serialize queue metadata: {e}")))?; let mut file = OpenOptions::new() @@ -61,6 +61,24 @@ fn write_github_output(metadata: &MergeQueueMetadata) -> Result<(), CliError> { Ok(()) } +/// 16 random bytes rendered as 32 lowercase hex chars — enough +/// entropy to be unguessable inside one GitHub Actions step, which +/// is all the heredoc delimiter needs (it just has to be absent +/// from the metadata payload). `getrandom` reads from the OS RNG +/// directly; we don't need the UUID parsing/formatting plumbing +/// that `uuid` adds on top. +fn random_delimiter_suffix() -> Result { + let mut buf = [0u8; 16]; + getrandom::fill(&mut buf) + .map_err(|e| CliError::Generic(format!("OS random source unavailable: {e}")))?; + let mut hex = String::with_capacity(buf.len() * 2); + for b in buf { + use std::fmt::Write as _; + write!(hex, "{b:02x}").expect("writing to String is infallible"); + } + Ok(hex) +} + #[cfg(test)] mod tests { use mergify_core::ExitCode; From f4fe75752614683cefdb4501b13a4bed7c04ba25 Mon Sep 17 00:00:00 2001 From: Julien Danjou Date: Tue, 19 May 2026 17:15:03 +0200 Subject: [PATCH 2/3] refactor(config): standardize the workspace on serde_yaml_ng for YAML parsing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The workspace had two YAML parsers — both forks of the archived `dtolnay/serde-yaml`. `mergify-config` used `serde_norway` for `.mergify.yml`; `mergify-ci` used `serde_yaml_ng` for merge-queue metadata in PR bodies and git notes. Same job, two crates, two transitive `unsafe-libyaml*` trees in Cargo.lock. Standardize on `serde_yaml_ng` for both. The decision is grounded in concrete signal, not vibe: Metric serde_norway serde_yaml_ng ───────────────────────── ───────────── ───────────── Reverse-deps on lib.rs 229 (78 dir.) 618 (349 dir.) GitHub stars 53 109 Last commit 2025-08-04 2025-09-14 Bus factor (recent prs) 1 (solo) merges externals Maintainer statement v0.9.40 title README: explicit "I'm gonna upstream-compat maintain this" intent unsafe-libyaml backend forked ("…- canonical norway") Open since 2024-06-10 2024-05-03 License Apache-2.0 MIT (= upstream) `serde_yaml_ng` wins on every axis that matters for the "will this still be alive in two years" question: three-times the ecosystem adoption, more recent activity, accepts third-party PRs, declares the maintenance commitment in writing, and uses the canonical `unsafe-libyaml` rather than a parallel-fork backend. Functional surface is identical for both of our use shapes — `from_str` to a typed struct for ci, `from_str` to `Value` then convert to `serde_json::Value` for config validation. Migration is purely a rename at the one call site. Cargo.lock drops `serde_norway` and `unsafe-libyaml-norway`. Co-Authored-By: Claude Opus 4.7 Change-Id: If5d28d2c4259127181bace5bafb0ac02c78d8f7b --- Cargo.lock | 21 +-------------------- crates/mergify-config/Cargo.toml | 2 +- crates/mergify-config/src/validate.rs | 4 ++-- 3 files changed, 4 insertions(+), 23 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3b7db81c..9b65448c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1102,7 +1102,7 @@ dependencies = [ "mergify-test-support", "serde", "serde_json", - "serde_norway", + "serde_yaml_ng", "temp-env", "tempfile", "tokio", @@ -1825,19 +1825,6 @@ dependencies = [ "zmij", ] -[[package]] -name = "serde_norway" -version = "0.9.42" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e408f29489b5fd500fab51ff1484fc859bb655f32c671f307dcd733b72e8168c" -dependencies = [ - "indexmap", - "itoa", - "ryu", - "serde", - "unsafe-libyaml-norway", -] - [[package]] name = "serde_yaml_ng" version = "0.10.0" @@ -2155,12 +2142,6 @@ version = "0.2.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "673aac59facbab8a9007c7f6108d11f63b603f7cabff99fabf650fea5c32b861" -[[package]] -name = "unsafe-libyaml-norway" -version = "0.2.15" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b39abd59bf32521c7f2301b52d05a6a2c975b6003521cbd0c6dc1582f0a22104" - [[package]] name = "untrusted" version = "0.9.0" diff --git a/crates/mergify-config/Cargo.toml b/crates/mergify-config/Cargo.toml index e543dd5a..d7da3e78 100644 --- a/crates/mergify-config/Cargo.toml +++ b/crates/mergify-config/Cargo.toml @@ -14,7 +14,7 @@ mergify-core = { path = "../mergify-core" } jsonschema = { version = "0.46", default-features = false } serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" -serde_norway = "0.9" +serde_yaml_ng = "0.10" url = "2" [dev-dependencies] diff --git a/crates/mergify-config/src/validate.rs b/crates/mergify-config/src/validate.rs index e669ac32..1bdee53c 100644 --- a/crates/mergify-config/src/validate.rs +++ b/crates/mergify-config/src/validate.rs @@ -57,10 +57,10 @@ pub async fn run(explicit_path: Option<&Path>, output: &mut dyn Output) -> Resul fn load_yaml(path: &Path) -> Result { let text = std::fs::read_to_string(path) .map_err(|e| CliError::Configuration(format!("cannot read {}: {e}", path.display())))?; - // Parse as YAML into serde_norway::Value, then convert to JSON + // Parse as YAML into serde_yaml_ng::Value, then convert to JSON // so jsonschema can validate. The conversion is always lossless // for valid Mergify configs (mappings, sequences, scalars). - let yaml_value: serde_norway::Value = serde_norway::from_str(&text) + let yaml_value: serde_yaml_ng::Value = serde_yaml_ng::from_str(&text) .map_err(|e| CliError::Configuration(format!("Invalid YAML in {}: {e}", path.display())))?; // Mergify configs are YAML mappings at the top level; an empty // file deserializes to Null, which we treat as an empty mapping. From f58073ce7b2943214a4daaa24780969aeec84a3d Mon Sep 17 00:00:00 2001 From: Julien Danjou Date: Tue, 26 May 2026 11:25:59 +0200 Subject: [PATCH 3/3] test(ci): add live smoke test for `ci scopes` select-all path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pin the contract before porting `ci scopes` to Rust. The new test exercises the "no base provided" branch — pass `--head HEAD` without `--base` and the command must list every configured scope as touched. This is the one execution path through `ci scopes` that doesn't shell out to `git diff`, so the test stays hermetic inside the tmp dir the `cli` fixture runs in (no git init, no remote fetch, no Mergify API). The Python implementation passes today. The follow-up port lands on top and the same smoke test exercises the Rust binary, catching any wire-format or exit-code drift between the two implementations. Co-Authored-By: Claude Opus 4.7 Change-Id: I14468b7046c449104675aea0a07a273eab479316 --- func-tests/test_live_smoke.py | 46 +++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/func-tests/test_live_smoke.py b/func-tests/test_live_smoke.py index a2e5404a..7d7d07dd 100644 --- a/func-tests/test_live_smoke.py +++ b/func-tests/test_live_smoke.py @@ -432,6 +432,52 @@ def test_scopes_send( assert result.returncode == 0, f"stdout:\n{result.stdout}\nstderr:\n{result.stderr}" +def test_ci_scopes_select_all_when_no_base( + tmp_path: pathlib.Path, + cli: typing.Callable[..., typing.Any], +) -> None: + """`mergify ci scopes` with `--head HEAD` and no `--base` lists + every configured scope as touched. + + Locally evaluated — no Mergify API, no token, no git + operations: the no-base branch is the one path through the + command that doesn't shell out to `git diff`, so the test + stays hermetic in the tmp dir the `cli` fixture runs in. + Passing `--head` (without `--base`) is what forces the + "manual" `References` branch with `base=None`; the detector + fallback would otherwise pick `HEAD^..HEAD` and try to diff + against a non-existent git repo. + + Contract pinned for the upcoming Python → Rust port: both + `backend` and `frontend` from the config below must appear in + the output so the "selecting all scopes" code path stays + intact when the implementation flips. + """ + config = tmp_path / "mergify.yml" + config.write_text( + "scopes:\n" + " source:\n" + " files:\n" + " backend:\n" + " include: ['mergify_cli/**']\n" + " frontend:\n" + " include: ['web/**']\n", + encoding="utf-8", + ) + + result = cli("ci", "scopes", "--config", str(config), "--head", "HEAD") + assert result.returncode == 0, ( + f"expected 0, got {result.returncode}\n" + f"stdout:\n{result.stdout}\nstderr:\n{result.stderr}" + ) + combined = result.stdout + result.stderr + for scope in ("backend", "frontend"): + assert scope in combined, ( + f"expected scope '{scope}' in the 'select all' output\n" + f"stdout:\n{result.stdout}\nstderr:\n{result.stderr}" + ) + + def test_tests_show_no_match( live_token: str, cli: typing.Callable[..., typing.Any],