From 71694ed0804a172c9953b01ca3f1354aad77ca6f Mon Sep 17 00:00:00 2001 From: DoubleGate Date: Tue, 18 Aug 2026 00:25:53 -0400 Subject: [PATCH 1/2] docs(ci): give security.yml the real reason it uses prebuilt tooling Both jobs justified installing `cargo-audit` / `cargo-deny` as prebuilt binaries with a sentence that argues against itself: the repo pins rustc 1.96 (rust-toolchain.toml), but the current cargo-audit release needs >= 1.88 to COMPILE 1.96 satisfies `>= 1.88`. The "but" has no force, and the constraint it names does not exist. It was true when written -- the pin was 1.86, genuinely below cargo-audit's requirement -- and it survived the v1.3.0 bump to 1.96 unchanged. The DECISION was always right; only the reason rotted. That is the dangerous shape: a reader who notices the arithmetic concludes the constraint has lifted, switches to `cargo install`, and pays a multi-minute compile of a large dependency tree on every security run -- having "fixed" a comment that was protecting something real. So the comment now gives both actual reasons, in order, because only the first is unconditional: 1. SPEED. Compiling the tool on every run dwarfs downloading it. 2. TOOLCHAIN INDEPENDENCE. `rust-toolchain.toml` is a directory override, so a build-from-source install is subject to whatever rustc this repo pins. If a tool's own MSRV ever rises above that pin, the SECURITY gate is what breaks. A prebuilt binary is immune -- it only parses `Cargo.lock`, never compiles the project, and runs under any toolchain. Reason 2 is recorded as a standing property rather than deleted as history, because it is slack today and can bind again after an MSRV change in either direction -- including a deliberate MSRV *reduction*, which is the case nobody would think to check. Found while reviewing a Dependabot bump of the very action these comments sit above. Fourth instance this release train of prose asserting something its own facts contradict, after the pixel-provenance clear, the VRC7 section-version doc, and this month's `4-8x` CI timeout claim. Comments only -- no workflow behaviour changes. --- .github/workflows/security.yml | 32 ++++++++++++++++++++++++-------- CHANGELOG.md | 19 +++++++++++++++++++ 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index f1c18c8e..3511bb22 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -55,11 +55,27 @@ jobs: - uses: actions/checkout@v7 with: persist-credentials: false - # Install the PREBUILT binary, never `cargo install` (build-from-source): - # the repo pins rustc 1.96 (rust-toolchain.toml), but the current - # cargo-audit release needs >= 1.88 to COMPILE. The prebuilt binary RUNS - # fine under any toolchain — it only parses Cargo.lock, it never compiles - # the project — so the toolchain pin is irrelevant to it. + # Install the PREBUILT binary, never `cargo install` (build-from-source). + # Two reasons, and the ORDER matters because only the first is always true: + # + # 1. Speed. `cargo install cargo-audit` compiles a large dependency tree + # on every run; the prebuilt binary is a download and an extract. + # + # 2. It decouples the security gate from the project's toolchain pin. + # `rust-toolchain.toml` is a directory override, so a build-from-source + # install is subject to whatever rustc this repo pins -- and if the + # tool's own MSRV ever rises above that pin, the SECURITY gate is what + # breaks. The prebuilt binary is immune: it only parses `Cargo.lock`, + # it never compiles the project, and it runs under any toolchain. + # + # Reason 2 was an ACTIVE constraint until v1.3.0, when the pin was 1.86 and + # cargo-audit already required 1.88. It is slack today (the pin is 1.96) + # and could bind again after any MSRV change in either direction. The + # comment this replaces still asserted it as present-tense fact -- "the + # repo pins rustc 1.96 ... BUT cargo-audit needs >= 1.88 to compile" -- + # which stopped being true at the bump and reads as a non-sequitur now, + # since 1.96 satisfies >= 1.88. A reader could reasonably conclude the + # constraint had lifted and switch to `cargo install`, losing reason 1. - uses: taiki-e/install-action@v2.85.13 with: tool: cargo-audit @@ -72,9 +88,9 @@ jobs: - uses: actions/checkout@v7 with: persist-credentials: false - # Prebuilt binary for the same reason as the audit job: the latest - # cargo-deny needs rustc >= 1.88 to build from source, while the repo is - # pinned to 1.96. Policy lives in `deny.toml`. + # Prebuilt binary for the same two reasons as the audit job above: it is + # much faster than compiling cargo-deny, and it keeps the security gate + # independent of this repo's rustc pin. Policy lives in `deny.toml`. - uses: taiki-e/install-action@v2.85.13 with: tool: cargo-deny diff --git a/CHANGELOG.md b/CHANGELOG.md index e96149a6..f5751197 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,25 @@ cycle-accurate core later replaced. ### Fixed +- **The security workflow's stated reason for using prebuilt tooling was false.** + Both jobs justified installing `cargo-audit` / `cargo-deny` as prebuilt + binaries with "the repo pins rustc 1.96 … **but** cargo-audit needs >= 1.88 to + compile" — and 1.96 satisfies `>= 1.88`, so the sentence argues against itself. + It was true when written, with the pin at 1.86, and survived the v1.3.0 bump. + + The decision was always right; only the reason had rotted. The comment now + gives both real reasons in order — speed first, since compiling the tool on + every run dwarfs downloading it, and toolchain independence second, because + `rust-toolchain.toml` is a directory override, so a build-from-source install + is hostage to this repo's pin and the **security** gate is what breaks if a + tool's MSRV ever rises above it. That second reason is slack today and can + bind again after any MSRV change in either direction, which is why it is + recorded as a standing property rather than deleted as historical. + + Found while reviewing a Dependabot bump of the very action these comments sit + above — the fourth instance this release train of prose asserting something + its own facts contradict. + - **CI jobs are bounded, so a hung job can no longer block a release.** No job in `ci.yml` carried a `timeout-minutes`, which means every one inherited GitHub's **six-hour** default. On the night of the v2.3.6 cut the `lint` job — From 4323a6a86d7b0924f3f150265446a7e46e56cd51 Mon Sep 17 00:00:00 2001 From: DoubleGate Date: Tue, 18 Aug 2026 00:50:53 -0400 Subject: [PATCH 2/2] =?UTF-8?q?docs(ci):=20condense=20the=20rationale=20?= =?UTF-8?q?=E2=80=94=20both=20review=20points=20were=20fair?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two suggestions, both taken, and both were right about the same failure: I argued the case at the length of a review reply and then left it in the file. THE WORKFLOW COMMENT went from 18 lines to 9. It keeps what a reader needs -- both reasons, and the fact that reason 2 was an active constraint before the v1.3.0 pin bump and can bind again -- and drops the retelling of what the old comment said and how a reader might be misled by it. That belongs in this commit message, which is where it now lives. THE CHANGELOG ENTRY went from three paragraphs to four lines. The sharper form of the objection is the one worth recording: this project's rule is to change the CHANGELOG alongside USER-VISIBLE behaviour, and a comment carries none. A one-line note that a stale rationale was corrected is the most that belongs there; the reasoning is a commit-message concern. "The added prose is disproportionately long and reads a bit like a rant" is fair comment on a change whose entire content is a comment. --- .github/workflows/security.yml | 30 +++++++++--------------------- CHANGELOG.md | 23 +++++------------------ 2 files changed, 14 insertions(+), 39 deletions(-) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 3511bb22..2b114e48 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -55,27 +55,15 @@ jobs: - uses: actions/checkout@v7 with: persist-credentials: false - # Install the PREBUILT binary, never `cargo install` (build-from-source). - # Two reasons, and the ORDER matters because only the first is always true: - # - # 1. Speed. `cargo install cargo-audit` compiles a large dependency tree - # on every run; the prebuilt binary is a download and an extract. - # - # 2. It decouples the security gate from the project's toolchain pin. - # `rust-toolchain.toml` is a directory override, so a build-from-source - # install is subject to whatever rustc this repo pins -- and if the - # tool's own MSRV ever rises above that pin, the SECURITY gate is what - # breaks. The prebuilt binary is immune: it only parses `Cargo.lock`, - # it never compiles the project, and it runs under any toolchain. - # - # Reason 2 was an ACTIVE constraint until v1.3.0, when the pin was 1.86 and - # cargo-audit already required 1.88. It is slack today (the pin is 1.96) - # and could bind again after any MSRV change in either direction. The - # comment this replaces still asserted it as present-tense fact -- "the - # repo pins rustc 1.96 ... BUT cargo-audit needs >= 1.88 to compile" -- - # which stopped being true at the bump and reads as a non-sequitur now, - # since 1.96 satisfies >= 1.88. A reader could reasonably conclude the - # constraint had lifted and switch to `cargo install`, losing reason 1. + # Install the PREBUILT binary, never `cargo install` (build-from-source): + # 1. compiling the tool on every run dwarfs downloading it, and + # 2. `rust-toolchain.toml` is a directory override, so a build-from-source + # install is subject to this repo's rustc pin -- if a tool's own MSRV + # ever rises above that pin, the SECURITY gate is what breaks. The + # prebuilt binary only parses `Cargo.lock` and runs under any toolchain. + # Reason 2 was an ACTIVE constraint until the v1.3.0 pin bump (1.86 vs + # cargo-audit's 1.88); it is slack today and can bind again after an MSRV + # change in either direction, so it is kept rather than deleted as history. - uses: taiki-e/install-action@v2.85.13 with: tool: cargo-audit diff --git a/CHANGELOG.md b/CHANGELOG.md index f5751197..76116d97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,24 +16,11 @@ cycle-accurate core later replaced. ### Fixed -- **The security workflow's stated reason for using prebuilt tooling was false.** - Both jobs justified installing `cargo-audit` / `cargo-deny` as prebuilt - binaries with "the repo pins rustc 1.96 … **but** cargo-audit needs >= 1.88 to - compile" — and 1.96 satisfies `>= 1.88`, so the sentence argues against itself. - It was true when written, with the pin at 1.86, and survived the v1.3.0 bump. - - The decision was always right; only the reason had rotted. The comment now - gives both real reasons in order — speed first, since compiling the tool on - every run dwarfs downloading it, and toolchain independence second, because - `rust-toolchain.toml` is a directory override, so a build-from-source install - is hostage to this repo's pin and the **security** gate is what breaks if a - tool's MSRV ever rises above it. That second reason is slack today and can - bind again after any MSRV change in either direction, which is why it is - recorded as a standing property rather than deleted as historical. - - Found while reviewing a Dependabot bump of the very action these comments sit - above — the fourth instance this release train of prose asserting something - its own facts contradict. +- **Corrected a stale comment in `security.yml`.** It justified installing + `cargo-audit` / `cargo-deny` as prebuilt binaries with "the repo pins rustc + 1.96 **but** cargo-audit needs >= 1.88 to compile" — which argues against + itself, since 1.96 satisfies that. True when written at a 1.86 pin; it + survived the v1.3.0 bump. Comments only; no behaviour change. - **CI jobs are bounded, so a hung job can no longer block a release.** No job in `ci.yml` carried a `timeout-minutes`, which means every one inherited