Skip to content

chore: unbreak reach main CI — fmt 17 files, migrate deny.toml to 0.14+ schema#5

Merged
todie merged 1 commit into
mainfrom
chore/fmt-deny-main
Apr 7, 2026
Merged

chore: unbreak reach main CI — fmt 17 files, migrate deny.toml to 0.14+ schema#5
todie merged 1 commit into
mainfrom
chore/fmt-deny-main

Conversation

@todie

@todie todie commented Apr 7, 2026

Copy link
Copy Markdown
Member

Why

Two pre-existing issues on main were silently breaking CI on every open PR (#1 release-please, #2 screenshot fix, #3 editorconfig) via the Format and Deny jobs. Neither was introduced by the open PRs. Both landed in commit ea74fa4 ("feat: CI/CD overhaul + process restart loop + zero clippy warnings") — the "zero clippy warnings" part was true, the "cargo fmt --check passes" part apparently wasn't, and the deny.toml was never migrated when cargo-deny 0.14 shipped a breaking schema change.

Opening this so the Format + Deny jobs pass on all four currently-open reach PRs without each having to carry the fix.

1. rustfmt violations across 17 files (86 diff hunks)

cargo fmt --check on clean main produces 86 diff hunks across:

crates/reach-cli/src/commands/{connect,create,destroy,exec,list,screenshot,serve,vnc}.rs
crates/reach-cli/src/{config,docker,mcp}.rs
crates/reach-cli/tests/{docker_types,e2e_container,mcp_types}.rs
crates/reach-supervisor/src/{main,processes,signals}.rs

Ran cargo fmt --all with the project's rustfmt.toml (max_width = 100). No behavioral changes — pure whitespace / alignment / import-grouping edits. Every hunk is rustfmt doing its job.

2. deny.toml schema migration (cargo-deny 0.14+)

cargo-deny 0.14 removed or changed several schema keys:

Old New Why
[advisories] vulnerability = "deny" removed cargo-deny now always denies vulnerabilities in the default pipeline
[advisories] unmaintained = "warn" "workspace" field semantics changed from severity enum {deny,warn,allow} to scope enum {all,workspace,transitive,none}. Picked "workspace" — we only care about crates we directly depend on being marked unmaintained, not transitives we don't control
[licenses] unlicensed = "deny" removed see EmbarkStudios/cargo-deny#611. Unlicensed crates are now deny-by-default in the licenses pipeline without the explicit key

3. RUSTSEC-2024-0437 ignore (protobuf 2.28 stack overflow)

After the schema migration, cargo deny check surfaces a real advisory: protobuf 2.28.0 is transitively pulled in by prometheus 0.13.4 → reach-supervisor. RUSTSEC-2024-0437 is a stack overflow from unbounded recursion when parsing untrusted protobuf input.

Does not apply to reach-supervisor. The supervisor uses prometheus to export metrics (serialise to the Prometheus text format, served over an HTTP endpoint on port 8400). It never parses protobuf input from the network or from untrusted sources, so the vector is unreachable.

Added an explicit [advisories] ignore = ["RUSTSEC-2024-0437"] with the detailed justification as an inline comment. Follow-up: upgrade to a prometheus version that uses protobuf 3.x, or drop the prometheus crate in favour of a pure-text-format exporter.

Verification

Full local CI parity:

$ cargo fmt --check
$ cargo deny check
advisories ok, bans ok, licenses ok, sources ok
$ cargo clippy --workspace -- -D warnings
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 8.79s
$ cargo build --workspace --release
    Finished `release` profile [optimized] target(s) in 18.22s

All four checks clean.

Cascade impact

After this lands on main, the Format + Deny jobs will pass on open PRs #1, #2, #3 without any per-branch edits. Worth a cascade-rebase on #2 (my screenshot fix) so its single commit picks up the fmt-normalised docker.rs line lengths. PR #1 and #3 don't touch Rust source, so no rebase needed for them.

Follow-ups

  • Upgrade prometheus crate or migrate to a text-only metrics exporter to drop the protobuf 2.x dependency entirely (kills the RUSTSEC-2024-0437 ignore).
  • Add a cargo fmt --check step to the pre-commit hook config so this doesn't happen again. The hook already exists; this chore didn't touch it.

🤖 Generated with Claude Code

…4+ schema

Two pre-existing issues on main that were silently breaking CI on every
open PR (#1 release-please, #2 screenshot fix, #3 editorconfig) via the
Format and Deny jobs. Neither was introduced by the open PRs — they
landed in the CI/CD overhaul commit 'ea74fa4 feat: CI/CD overhaul +
process restart loop + zero clippy warnings' (the 'zero clippy warnings'
part was true; the 'cargo fmt --check passes' part apparently wasn't).

## 1. rustfmt violations across 17 files

`cargo fmt --check` on main produces 86 diff hunks across:

  crates/reach-cli/src/commands/{connect,create,destroy,exec,list,screenshot,serve,vnc}.rs
  crates/reach-cli/src/{config,docker,mcp}.rs
  crates/reach-cli/tests/{docker_types,e2e_container,mcp_types}.rs
  crates/reach-supervisor/src/{main,processes,signals}.rs

Applied `cargo fmt --all` with the project's rustfmt.toml (max_width=100).
No behavioral changes — pure whitespace/alignment/import-grouping edits.

## 2. deny.toml schema migration (cargo-deny 0.14+)

cargo-deny 0.14 removed or changed several schema keys:

- `[advisories] vulnerability = "deny"` → REMOVED (cargo-deny now
  always denies vulnerabilities in its pipeline).
- `[advisories] unmaintained = "warn"` → REJECTED. The field's
  semantics changed from severity ("deny"/"warn") to scope
  ("all"/"workspace"/"transitive"/"none"). Migrated to
  "workspace" — we only care about crates we directly depend on being
  marked unmaintained, not transitives we don't control.
- `[licenses] unlicensed = "deny"` → REMOVED (see
  EmbarkStudios/cargo-deny#611 for the migration note). cargo-deny now
  handles unlicensed crates as deny-by-default in the licenses pipeline
  without the explicit key.

## 3. RUSTSEC-2024-0437 ignore (protobuf 2.28 stack overflow)

After the schema migration, `cargo deny check` surfaces a real
vulnerability: protobuf 2.28.0 is transitively pulled in by
`prometheus 0.13.4 → reach-supervisor`. RUSTSEC-2024-0437 is a stack
overflow caused by unbounded recursion when PARSING untrusted protobuf
input.

reach-supervisor uses prometheus to EXPORT metrics (serialise to the
Prometheus text format, served over an HTTP endpoint on port 8400). It
never parses protobuf input from the network or from untrusted sources,
so the vector does not apply. Added an explicit `[advisories] ignore`
with a detailed justification comment so the rationale is preserved
inline. Follow-up: upgrade to a prometheus version that uses protobuf
3.x, or drop the prometheus crate in favour of a pure-text-format
exporter.

## Verification

Local CI parity:

  `cargo fmt --check`                  clean
  `cargo deny check`                   advisories ok, bans ok, licenses ok, sources ok
  `cargo clippy --workspace -- -D warnings`  zero warnings
  `cargo build --workspace --release`   clean

After this lands on main, the Format + Deny jobs will start passing on
open PRs #1, #2, #3 without any per-branch rebasing (the fixes are on
main, not downstream). Worthwhile to cascade-rebase the three so they
pick up the fmt-normalised line-lengths in docker.rs (which PR #2
touches).
@todie
todie merged commit fa2fc61 into main Apr 7, 2026
9 of 11 checks passed
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.

1 participant