Skip to content

Replace serde_yaml with serde-saphyr to drop unsafe-libyaml - #142

Open
dduugg wants to merge 3 commits into
mainfrom
replace-serde-yaml-with-serde-saphyr
Open

dduugg wants to merge 3 commits into
mainfrom
replace-serde-yaml-with-serde-saphyr

Conversation

@dduugg

@dduugg dduugg commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #141.

unsafe-libyaml is unmaintained (RUSTSEC-2024-0449) and reaches us only through serde_yaml, which is itself deprecated. This replaces serde_yaml with serde-saphyr 2.0.0, the serde option the advisory suggests. serde-saphyr and its parser, granit-parser, are pure Rust with #![forbid(unsafe_code)], though granit-parser depends on arraydeque, which uses unsafe. serde-saphyr is actively maintained: 32 releases in the past year, outside issues closed in a median of under a day, and kube-rs among its 400+ dependents. Both crates have a single maintainer, though, and their security policy covers only the latest release. 2.0.0 shipped about ten weeks after 1.0.0, so we should expect to take major versions promptly, which is why Dependabot now opens serde-saphyr bumps on their own.

Alternatives considered

Crate Why not
noyalib 0.0.58 (the advisory's other suggestion) 50 of the 178 existing tests fail, even through its serde_yaml compatibility feature, mostly on the empty unowned_globs: that four fixtures use. It is still 0.0.x, so every release is semver-incompatible with the last.
serde_yaml_ng, serde_norway, serde_yaml_bw Still parse through unsafe-libyaml or a fork of it.
saphyr, yaml-rust2 No serde support, so every struct would need hand-written mapping code.

What changed

All YAML parsing goes through the new src/yaml.rs:

  • Callers read the file with fs::read and parse it with from_slice_with_options, which accepts the same encodings serde_yaml did (UTF-8, with or without a BOM). from_reader would also accept UTF-16 and silently replace invalid UTF-8 after a BOM, and it parses about 2.3x slower than serde_yaml.
  • Each option that differs from serde-saphyr's defaults has a one-line reason next to it. The one that matters is duplicate_keys: LastWins. The default rejects a repeated key anywhere in a file, including keys we never read, and one such package.yml would abort generate and validate for the whole repo. These exist in the wild: this public packwerk app repeats enforce_privacy. The other options keep serde_yaml's behavior for <<, for !!binary, for .inf and .nan in values we ignore, for YAML 1.1 booleans (yes and no stay errors), and for single-line error messages.
  • Resource budgets are tighter than serde-saphyr's defaults: nesting depth 16 instead of 64, 50,000 nodes instead of 250,000, and lower caps on scalar bytes and on anchor copies. LastWins buffers mappings before serde sees them, nested ones once per level, so under the defaults a crafted 480 KB package.yml made generate peak at 2.4 GB and take 15 s, against 35 MB and 0.5 s on main. Under these budgets that file fails with a budget error, and the worst file that fits peaks at about 195 MB. The depth limit also keeps nested input from overflowing the stack. The 1,015 files read in two large private monorepos nest at most 5 deep, with at most 2,162 nodes and no anchors. The node limit is the new yaml_max_nodes config key, so a repo whose files outgrow it can raise it without waiting for a release, and a file over it fails with a hint naming the key. A 20,000-node default would cap the worst case at about 40 MB, with 9x headroom over that largest file instead of 23x.
  • serde-saphyr rejects an empty, ~ or null value in a String field or list item, which serde_yaml accepted. Two small deserialize_with helpers keep accepting them, as "". The list helper also reads an empty, ~ or null list as empty itself, rather than relying on serde-saphyr to, since serde-saphyr#233 asks it to stop.
  • The config tests and the helper in src/common_test.rs now go through Config::load_from_path, so they exercise the production path.
  • 20 new tests pin these behaviors, seven of them directly in src/yaml.rs, next to the options they cover. Every option, the depth and node limits, every deserialize_with attribute, and each of the four places that pass yaml_max_nodes on fail at least one test when removed. Eleven tests assert what serde_yaml already did (checked by running the same assertions on main). The other nine pin intended changes: a repeated owner takes the last value, a BOM before several keys now parses, a ~ or null list reads as empty, a value nested 20 deep is rejected, and so is a list of 100,000 items, unless yaml_max_nodes raises the limit. That last part is checked through generate, for-file and for-file --from-codeowners, and for the team cache, which is keyed on the limit.
  • Dependabot's cargo group now excludes serde-saphyr, so its bumps arrive as their own pull requests and never wait on unrelated breakage. granit-parser is a transitive dependency, which Dependabot version updates skip, so its fixes arrive through serde-saphyr bumps or RustSec advisories.

Behavior changes

Verified unchanged:

  • Exit codes, stdout and every CODEOWNERS file written, across 74 CLI runs in 37 small projects: valid and broken config, team and package.yml files, the empty-file cases in the last bullet, and all 13 fixtures (including docs/CODEOWNERS from the custom-path fixture).
  • 2,356 real files parse to the same values, or fail in both: the 42 fixtures, 1,134 public package.yml, team and config files from GitHub, and about 1,200 files from nine private repositories. The one exception is two public GitHub Actions workflows named package.yml that start with a UTF-8 BOM, which serde_yaml rejected and serde-saphyr accepts. For the seven private repositories with a config, generate gives byte-identical CODEOWNERS, stdout, stderr and exit codes.
  • With the final budgets: generate on two large private monorepos, which reads 1,015 YAML files, gives byte-identical CODEOWNERS, empty stdout and stderr and exit code 0, the same as main. None of a wider local corpus of 1,615 package.yml, team and config files, or the 42 fixtures, hits a budget, even at 20,000 nodes.
  • An empty, comment-only or ----only package.yml still means "no owner".

Changed: error text on every failure path, and a handful of rare inputs that are now rejected, accepted, or read differently. Apart from the error text of files that fail in both, and the two BOM files above, none of these occur in the 2,356 files.

Every change, with examples

Error text changes on every failure path. Messages no longer start with the key path, type errors no longer say what was found, and locations read line L, column C, including the line 1, column 1 that serde_yaml left out:

Input Before After
team file without github missing field `github` missing field `github` at line 1, column 1
owned_globs: 5 owned_globs: invalid type: integer `5`, expected a sequence at line 1 column 14 expected sequence start at line 1, column 14
do_not_add_to_codeowners_file: yes github.do_not_add_to_codeowners_file: invalid type: string "yes", expected a boolean at line 4 column 34 invalid boolean (strict mode expects true/false) at line 4, column 34
empty team file missing field `name` unexpected end of input at line 1, column 1
unterminated quote found unexpected end of stream at line 4 column 1, while scanning a quoted scalar at line 3 column 9 invalid indentation in multiline quoted scalar at line 4, column 1
a second document deserializing from YAML containing more than one document is not supported multiple YAML documents detected; use from_str_multiple or from_str_multiple_with_options at line 3, column 1
~: x at the top level (accepted) cannot deserialize null into string; use Option<String> at line 1, column 1

The last two messages give advice meant for Rust callers of serde-saphyr. A budget breach reads like budget breached: Depth { depth: 17 } at line 2, column 70, under the path of the file. A node-limit breach adds a hint naming yaml_max_nodes.

A missing field is also reported at a different line. serde_yaml gave the line where the mapping starts. serde-saphyr gives the line of the last key it read in that mapping, so a longer team file without github can point at an unrelated key such as ruby:.

Newly rejected. serde_yaml accepts these, and so does Psych except where noted:

  • A quoted string continued on a line indented no deeper than its own key, such as:
    metadata:
      description: "first line
      second line"
    serde-saphyr applies the YAML 1.2 rule here and has no option to relax it. Indent the continuation past the key, or use a block scalar. A continuation line that starts with a tab is rejected the same way.
  • A comment directly after a closing quote or bracket, such as name: "Payroll"# note.
  • A null key such as ~: x directly in a mapping we read: the top level, metadata, github or ruby. Nested deeper inside a value we ignore, null keys still parse.
  • A core tag that serde-saphyr cannot apply, even under keys we ignore: !!timestamp with any value (Psych's load_file rejects it too), !!int with a value outside 64 bits or in YAML 1.1 form such as 017 or 1:20, !!float 1_000.5, or a value that does not fit, such as !!int abc. In a field we read, a non-string core tag such as name: !!int 123 is rejected too, and so is !!str true for a boolean.
  • Nesting deeper than 16 levels, or more than 50,000 nodes (configurable with yaml_max_nodes), anywhere in the file. serde_yaml had no limits for values we ignore.
  • Files with at least 100 aliases and more than 10 aliases per anchor, and an alias with the colon attached, such as *a: 1. Psych 4's load_file rejects aliases by default.
  • A BOM after ---.
  • U+0085, U+2028 or U+2029 used as a line break, which YAML 1.2 no longer allows.

Newly accepted:

  • A repeated key for a field we read takes the last value, as in Psych. serde_yaml failed with duplicate field `owner` .
  • A UTF-8 BOM before --- or before the first of several keys. serde_yaml failed on these, usually with "deserializing from YAML containing more than one document is not supported".
  • ~ for a list field, read as an empty list, and a quoted "true" or an odd-case tRuE for a boolean.
  • A package.yml that is only ~, null or ..., read as no owner, as in Psych.
  • Syntax libyaml rejects but YAML 1.2 allows: tabs in more places, such as before a key under metadata:; anchor names containing / or non-ASCII characters; unknown % directives; and surrogate-pair escapes such as "😀".

Different values:

  • ~, null in any capitalization such as nUlL, or a !!null-tagged value in a string field or list item reads as "", and in owner as no owner, instead of the literal text. Psych also reads these as nil.
  • A | or > block scalar that ends the file without a trailing newline now ends in \n, so owner: | followed by TeamA at end of file reads as "TeamA\n".
  • U+0085, U+2028 and U+2029 are no longer line breaks, so name: TeamA followed by U+0085 reads as "TeamA\u{85}".

In an owner or a team name, either of the last two means the name no longer matches its team, the way a typo would. In code_ownership, which compares the names Rust reads against the ones Psych reads, for_file then returns nil for that team's files.

Cost

  • The release binary grows by about 0.8 MB after strip -x, from 3.71 MB to 4.50 MB on aarch64-apple-darwin. Unstripped, each architecture's binary grows by 0.9 to 1.0 MB, and the universal mac binary the release job uploads by 1.9 MB. Most of it is serde-saphyr's deserializer, plus annotate-snippets, which stays linked even with snippets off.
  • Parsing takes about 1.3x as long per file on a large private corpus, about 6 µs more, and up to about 1.9x on small or synthetic files. End-to-end generate on a large monorepo is unchanged within noise, because walking the tree dominates.
  • serde-saphyr needs Rust 1.89, under our 1.97.1 pin and the Rust 1.98 in code_ownership's release images.

Verification

  • cargo audit: no warnings, down from RUSTSEC-2024-0449. cargo tree -i unsafe-libyaml and cargo tree -i serde_yaml match nothing.
  • cargo fmt --all -- --check, RUSTFLAGS=-Dwarnings cargo clippy --all-targets --all-features, and cargo test: 198 passed, 0 failed, 3 ignored (178 before, plus the 20 new tests).
  • The release workflow's two-target mac build and its lipo step succeed. Its cross build succeeds for x86_64-unknown-linux-gnu, and for aarch64-unknown-linux-gnu it builds and links codeowners.
  • Cargo.lock is exactly what cargo build resolves from main's lockfile, with no cargo update: 6 packages removed, 11 added.
  • The release job's grep "\+version" check finds nothing in this diff with BSD grep or GNU grep, so merging does not cut a release.
  • rubyatscale/code_ownership's specs pass with its native extension built against this branch's first commit: 96 examples, 0 failures, 1 pending, the same as against main.
  • About 56 million fuzzed inputs, parsed as each of our structs and as IgnoredAny with this PR's options, caused no panics.

Notes

  • No version bump. The new yaml_max_nodes key is documented in the README's configuration section.
  • Add allow_unowned_globs to permit files with no owner #135 adds a config test that calls serde_yaml::from_reader. The two PRs merge without conflicts, and an earlier green CI run on whichever lands second won't catch it, so rebase or re-run CI before merging that one. The test needs switching to Config::load_from_path, and the new allow_unowned_globs field needs deserialize_with = "crate::yaml::strings_or_nulls" to match the other list fields. Merging this first means Add allow_unowned_globs to permit files with no owner #135's 0.6.0 release ships the fix.

unsafe-libyaml is unmaintained (RUSTSEC-2024-0449) and reaches us only
through serde_yaml, which is itself deprecated. serde-saphyr is a
maintained serde deserializer written in safe Rust, and it is the serde
option the advisory suggests.

All YAML parsing now goes through src/yaml.rs. Callers read the whole
file and pass the bytes to crate::yaml::from_slice, which calls
from_slice_with_options with options that keep serde_yaml's semantics
where serde-saphyr allows it: a repeated key keeps its last value
instead of failing the file, `<<` stays an ordinary key, non-finite
floats and `!!binary` are not interpreted, only true and false are
booleans, and errors stay on one line. Resource budgets stay at
serde-saphyr's defaults. Two deserialize_with helpers keep empty values
in string fields and list items working as before.

The config tests now go through Config::load_from_path, so they cover
the same path as production. New tests pin each option, the helpers,
empty package.yml files and the depth limit.

Fixes #141
@dduugg
dduugg requested a review from a team as a code owner October 20, 2026 00:07
@github-project-automation github-project-automation Bot moved this to Triage in Modularity Oct 20, 2026
With DuplicateKeyPolicy::LastWins, serde-saphyr buffers mappings before
serde sees them, and nested mappings once per level. Under the default
budget a crafted 480 KB package.yml made `generate` peak at 2.4 GB and
take 15 s, against 35 MB and 0.5 s on main. Cap depth at 16 and nodes
at 50,000, and lower the scalar-byte and anchor-copy caps. The worst
file that fits now peaks at about 195 MB. The 1,015 files read in two
large private monorepos nest at most 5 deep with at most 2,162 nodes,
and `generate` output for both is unchanged.

strings_or_nulls now reads an empty, `~` or `null` list as empty itself,
rather than relying on serde-saphyr to do so (serde-saphyr#233 asks it
to stop).

The empty-value tests now cover every deserialize_with field, so
removing any of them fails a test; before, 11 of the 15 could be
removed with the suite still passing. The depth test now nests 20
deep, between the new limit and the default, and asserts a budget
error; a new test does the same for the node limit.

Dependabot's cargo group now excludes serde-saphyr, so its bumps get
their own pull request.
A team file or package.yml over the node limit aborts the whole run,
and until now raising the limit took a new release. The new
`yaml_max_nodes` key in config/code_ownership.yml (default 50,000)
sets it for every path that reads those files: generate and validate,
for-file, and for-file --from-codeowners. The team cache is keyed on
it, so teams loaded under one limit are not reused under another.

A file over the limit now also gets a hint naming the key. The config
file itself is still read with the default, since it can't raise its
own limit and has at most a few hundred nodes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

RUSTSEC-2024-0449: unsafe-libyaml is unmaintained

1 participant