Repository navigation
chore(cargo): replace unmaintained serde_yaml and yaml-rust-davvid - #2244
Merged
Merged
Conversation
CoverageTotal: 97.24% ⚪ 0.02 pp vs Comparing
🔇 56 ignored region(s), 0 tolerated region(s)
Touched files absent from every coverage report (11)
These files may contain no executable code or may have been omitted from the coverage run. Patch coveragePatch: 99.25% (395/398 new lines covered)
Uncovered new lines (3)
Indirect coverage changes🔴 0 lines lost coverage, 🟢 32 lines gained coverage on unchanged code. Indirect changes
|
github-merge-queue
Bot
removed this pull request from the merge queue due to a conflict with the base branch
Nov 10, 2026
newhoggy
force-pushed
the
issue-2242-replace-yaml-crates
branch
2 times, most recently
from
November 10, 2026 11:47
6c9253f to
bdf7477
Compare
Swap serde_yaml for the API-compatible serde_norway fork and yaml-rust-davvid for yaml-rust2, clearing RUSTSEC-2024-0449 (unsafe-libyaml) and RUSTSEC-2021-0153 (encoding). The 52-file migration is a mechanical rename; parsing is unchanged. yaml-rust2's literal-block emitter writes |- for multiline strings without a trailing newline and quotes ones that cannot be a literal block. It is never worse than the emitter it replaces on a probe of 13 multiline shapes; tests pin the three shapes it changes.
Add tests for tagged-value serialization, ProjectDiscovery scope and feature-context loading (valid and malformed), and extract the PR details parse into parse_pr_details so it can be tested. Replace the short-circuiting round-trip assertions in data::yaml with a single exact comparison, and mark the three serializer calls that only run after a live bridge or YouTube response as process-bound.
newhoggy
force-pushed
the
issue-2242-replace-yaml-crates
branch
from
November 10, 2026 12:16
bdf7477 to
13636c7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Replaces the two direct dependencies behind the actionable RustSec warnings in #2242:
serde_yaml(RUSTSEC-2024-0449, viaunsafe-libyaml) becomesserde_norway, andyaml-rust-davvid(RUSTSEC-2021-0153, viaencoding) becomesyaml-rust2. The 52-file migration is a mechanical rename; parsing is unchanged.Plan and candidate evaluation: #2242 (comment)
Type of Change
Related Issue
Relates to #2242. Deliberately not
Closes: three of the five advisories (pin-utils,glib,proc-macro-error) depend on upstreamhyperandtao/tray-iconreleases and still show incargo audit. The issue's third acceptance criterion allows them to be "tracked here", so closing it on merge would drop that tracking. Close it by hand if you would rather track them elsewhere.Changes Made
Core Changes:
Cargo.toml/Cargo.lock:serde_yaml→serde_norway0.9.42,yaml-rust-davvid→yaml-rust20.13. The lockfile change is only what these need (dropsunsafe-libyaml,encoding*,ahash, oldhashlink).serde_yaml→serde_norwayin 52 files undersrc/,tests/,examples/(rename only, thencargo fmt).src/data/yaml.rs: emitter import switched toyaml_rust2; three new tests pin the changed output.Why these crates:
serde_yaml_ngstill depends onunsafe-libyaml, so it fails the acceptance criterion.serde_yaml_bwandserde-saphyrare pure-Rust parsers with YAML 1.2 semantics (andserde-saphyrneeds MSRV 1.89, noValue), which would change how existing files parse, and the issue asks to keep behaviour changes out of the migration.serde_norwayis API-identical and keeps the same libyaml-port parser; its parser crate is the differently namedunsafe-libyaml-norway, which RUSTSEC-2024-0449 does not cover, but it is the same code lineage, so a future advisory against it would apply too.Documentation: ADR-0001,
ARCHITECTURE.md,CLAUDE.md,STYLE_GUIDE.md; changelog fragmentchangelog.d/2242.changed.md.Testing
cargo test --features mcpandcargo test: all pass,instasnapshots included, no.snap.newfiles, no snapshot changes. Thesrc/cli/**files touched by the rename change no flags, subcommands or help text, so no snapshot update applies.cargo clippy --all-targets -- -D warnings, with and without--features mcp;cargo fmt --check;cargo doc --no-deps --document-private-itemswith broken intra-doc links denied.cargo tree -i unsafe-libyamlandcargo tree -i encodingboth report "did not match any packages" (also with--all-features).cargo audit:unsafe-libyamlandencodingare gone; three warnings remain (pin-utils,proc-macro-error,glib).cargo deny check: advisories, bans, licenses, sources ok.scripts/changelog.py check-pr --base origin/mainpasses; the commit passesomni-dev git commit message lint../scripts/build.shor exercise the Linuxmenu-barfeature.Review Focus Areas
Output change to review:
yaml-rust2's literal-block emitter writes|-for a multiline string with no trailing newline (the old emitter wrote|, which read back with an extra newline) and quotes a string that cannot be a literal block (e.g. trailing whitespace on a line). No existing test or snapshot changed, because their assertions match on: |as a prefix; the new tests assert the exact form. On a probe of 13 multiline shapes the new emitter is never worse than the old one, and CRLF strings now round-trip.Checklist
Security Considerations
unsafe-libyamlC-port from the dependency tree.Additional Notes
Review findings skipped (pre-existing, not caused by this change):
to_yaml→from_yaml: a first line starting with spaces (" lead\nb"), two or more trailing newlines, and a trailing newline on the last key of a document. All were lossy withyaml-rust-davvidas well (probed side by side). I corrected my changelog and commit wording, which had claimed exact round-trips, but did not widen this diff into an emitter fix.docs/adrs/adr-0022.mdanddocs/plan/{project,twiddle}.mdstill nameserde_yaml; they are historical records, so I left them.Follow-ups (not filed; creating issues was outside what this run was permitted to do):
pin-utils(viahyper) andglib/proc-macro-error(Linux GTK tray stack) against upstream releases; re-runcargo auditafter eachhyperandtao/tray-iconbump; no blanketdeny.tomlignores.to_yamllossless for the multiline shapes above: fall back to a quoted scalar (or|+/an indentation indicator) where a literal block would not round-trip, with a property test over arbitrary multiline strings.Departure from the plan: the plan said the follow-up issue would carry the advisory tracking; since it could not be filed, this PR uses
Relates toinstead ofCloses.