chore: Rust & CI update

[czareko]

No description provided.

[github-advanced-security]

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

• The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
• Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
• You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

[ethan-crypto]

Overall looks good, we just need to make sure to preserve the brew exit code without exiting before the Rust setup for macos.

[n13]

I've reviewed PR #103 end-to-end (workflow files, composite actions, Cargo.toml / Cargo.lock, toolchain pin, taplo config), checked it against main, and verified the green CI run. Here's my review.

TL;DR

Solid, mostly well-thought-out CI hardening. The diff is bigger than the title suggests — it bundles DRY composite actions, action version bumps, least-privilege permissions, a CodeQL workflow, a toolchain pin (stable → 1.93.0), cargo caching, and a cargo-audit policy change. CI is green and ethan-crypto's earlier brew-exit-code feedback was addressed correctly.

There are a few concrete fixes I'd want before merging — especially one unused dependency that snuck in — plus several smaller polish items.

---

What's good

• The new .github/actions/{ubuntu,macos}/action.yml composite actions kill a lot of duplication across ci.yml and create-release-tag-and-publish.yml. Big DRY win.

• Top-level permissions: contents: read in both workflows with per-job contents: write overrides on the tag/release jobs — textbook least-privilege.

• actions/cache keys correctly include rust-toolchain.toml so a toolchain bump invalidates the cache:

            path: |
              ~/.cargo/registry
              ~/.cargo/git
              target
            key: ${{ runner.os }}-cargo-build-${{ hashFiles('**/Cargo.lock', 'rust-toolchain.toml') }}
            restore-keys:

• CARGO_NET_RETRY=10 + CARGO_NET_TIMEOUT=60 is a nice transient-failure mitigation paired with the brew retry loop.

• The macOS brew exit-code fix from ethan-crypto's review is correctly applied:

          set +e
          brew install openssl cmake 2>&1 | sed '/already installed and up-to-date/d'
          brew_status=${PIPESTATUS[0]}
          set -e
          
          if [ "$brew_status" -ne 0 ]; then
            exit "$brew_status"
          fi

• CodeQL with both actions + rust (build-mode: none) is a good low-cost baseline — actions catches workflow-permission regressions, rust adds taint/quality on our code without paying the build cost.

---

Things I'd fix before merging

1. serial_test is an unused dev-dependency

Cargo.toml adds serial_test = "3.1" (and it pulls in scc, sdd, serial_test_derive in Cargo.lock), but the codebase has zero #[serial] attributes and zero use serial_test imports — I grepped both the PR branch and the workspace.

 ...
 "serial_test",          # <-- pulled into the dep graph but never used
 "sha2 0.10.9",
 ...
]

Either remove it, or include the test that motivated adding it. Right now it's dead weight on test compile time.

2. brew uninstall cmake is undocumented and surprising

        if [ "$OK" -ne 1 ]; then echo "::error::brew install protobuf llvm failed after 3 attempts"; exit 1; fi
        curl https://sh.rustup.rs -sSf | sh -s -- -y
        brew uninstall cmake
        set +e
        brew install openssl cmake 2>&1 | sed '/already installed and up-to-date/d'

Uninstall-then-reinstall of cmake looks like a workaround for the GitHub runner's pre-baked cmake conflicting with the brew formula, but there's no comment explaining why. Without one, the next person to touch this file will (correctly) wonder if it's a bug and remove it. Please add a one-line "why" comment, or drop the line if it's stale.

3. macOS composite installs rustup unconditionally

        curl https://sh.rustup.rs -sSf | sh -s -- -y

GitHub-hosted macos-latest and macos-15-intel runners ship with rustup pre-installed. rustup toolchain install (which is already there 5 lines below) honors rust-toolchain.toml on its own. The curl line just adds ~5–10s, plus a network dep on sh.rustup.rs. Drop it unless you've actually seen runners without rustup.

4. continue-on-error: true on cargo audit weakens the merge gate

      - name: Run cargo audit (informational only)
        # Only this step is non-blocking — every other step in this job
        # (checkout, caches, cargo-audit install) must fail loudly so we
        # don't silently skip the audit.
        continue-on-error: true
        run: cargo audit

The reasoning (don't let third-party RUSTSEC alerts block unrelated PRs) is fair, and the comment correctly distinguishes blocking vs. non-blocking steps. But "non-blocking + nobody reads green CI logs" = vulnerable dep can land on main unnoticed. The Cargo.toml already has explicit pins for bytes / quinn-proto / rustls-webpki advisories, which suggests the team is aware. Two alternatives that keep the gate strong:

• Keep blocking, but use a .cargo/audit.toml [advisories] ignore = ["RUSTSEC-…"] list for known/accepted IDs.
• Or run cargo audit --deny warnings and explicitly ignore unmaintained-dep warnings.

The release-workflow security-audit job stays blocking, which is good — but the asymmetry should be intentional and called out somewhere.

5. Action version inconsistency between workflows

ci.yml and codeql.yml upgraded to actions/checkout@v5 and actions/cache@v5. create-release-tag-and-publish.yml only went to v4 (and one v3 → v4 bump):

        uses: actions/cache@v4

Within a single "CI update" PR, I'd expect them to land together. Either bump release-workflow to v5 too, or add a comment explaining why it's pinned lower (e.g. one of the v5 changes affects artifact upload semantics).

6. Empty PR description

For a 9-commit, 9-file change touching CI permissions, supply-chain pinning, cargo-audit policy, and a CodeQL onboarding, a short rationale block in the PR body would help future archaeologists. Three bullets covering "(1) DRY composite actions, (2) toolchain pin to 1.93.0, (3) intentionally moved cargo-audit to non-blocking" would be enough.

---

Smaller polish (non-blocking)

• Caching gaps. fast-checks reinstalls taplo-cli every run (~20s+); examples rebuilds the full dep tree every run. Both could share the same actions/cache@v5 block as analysis/build-and-test-matrix. Easy speedup on every PR.
• Matrix asymmetry. ci.yml switched to fail-fast: false, but create-release-tag-and-publish.yml kept fail-fast: true. Probably fine for releases (fail fast = don't burn macOS minutes), but worth a deliberate decision rather than drift.
• Dropped rust: stable matrix dimension. Now that rust-toolchain.toml pins the channel, the matrix simplification is correct — but it does mean only the pinned channel is tested. If you want a "does this still build on stable?" smoke test, add a separate job (not a matrix dim). Otherwise, fine to leave as is.
• CodeQL actions pinned by tag (@v4). Strict supply-chain hygiene says pin first-party actions to commit SHAs since security-events: write is granted. Most teams accept the tag for github/codeql-action/*. Mention only because the rest of the PR is security-themed.
• Hard-pinned 1.93.0. Just remember to bump it deliberately; otherwise the cargo install cargo-audit --locked --version 0.22.1-style version juggling will start drifting.

---

Files I checked

• .github/actions/macos/action.yml (new)
• .github/actions/ubuntu/action.yml (new)
• .github/workflows/ci.yml (modified)
• .github/workflows/codeql.yml (new)
• .github/workflows/create-release-tag-and-publish.yml (modified)
• Cargo.toml / Cargo.lock (modified)
• rust-toolchain → rust-toolchain.toml (rename + pin)
• taplo.toml (modified — reorder_keys = true is what triggered the Cargo.toml reordering)

Per the user rules I haven't posted any of this to GitHub — happy to do so if you'd like, or to push fixes for the concrete items (serial_test removal, brew uninstall cmake comment, macOS rustup curl removal) on a follow-up branch. Just say the word.

[n13]

Pick and choose from AI review what to fix - maybe versioning should be consistent ... but none of these are blockers.