Skip to content

Two CI steps cannot fail, and no lint guards &str byte-slicing — the two gaps behind several open panic/false-summary issues #3394

Open

Description

@HetCreep

Three separate open issues (#2401/#2400, #3295/#3221, #2281) are each a specific instance of a defect class that the repository's own gates are configured not to catch. This is about the gates, not the bugs.

Provenance: AI-assisted source audit, human-directed, at v0.44.2 (700bdde). The clippy escalation behaviour was verified on a throwaway probe crate; everything else is read from the workflow and manifest files.

1. cargo audit cannot turn the build red

.github/workflows/ci.yml:88 pipes cargo audit through tee inside an if…else where both arms exit 0. Without set -o pipefail the pipeline's status is tee's, which succeeds whenever it writes. So an advisory match is reported and then discarded.

2. The unwrap/panic/expect check cannot fail either

.github/workflows/ci.yml:163 runs cargo clippy -- -W clippy::unwrap_used -W clippy::panic -W clippy::expect_used 2>&1 | tee | grep …. Same pipefail issue, and both branches only emit ::warning::. It is a report, not a gate — but it reads like a gate in the workflow list.

3. No lint guards byte-slicing, in a codebase whose job is slicing tool output

There is no clippy.toml, no [lints.clippy] in Cargo.toml (only [lints.rust]), and no crate-level #![deny]. In particular clippy::indexing_slicing and clippy::string_slice are not configured anywhere.

That is the lint that would have flagged, at review time:

Worth noting Cargo.toml:71 warnings = "deny" does work — I verified on a probe crate that it escalates warn-by-default clippy lints to hard errors (-D clippy::useless-vec implied by -D warnings, exit 101). It just cannot reach indexing_slicing/string_slice, which are allow-by-default.

4. Semgrep grandfathers every existing violation

ci.yml:198 runs semgrep with --error and --baseline-commit, so only newly-introduced findings fail. The two ERROR-severity rules in .semgrep.yml currently have pre-existing violations in the tree that are therefore permanently exempt:

  • interpreter-execution (:48-64, "it should never spawn interpreters") vs src/cmds/rust/runner.rs:111 and src/cmds/system/summary.rs:28 — note the six test-only sh sites in stream.rs all carry // nosemgrep, while these two production sites carry none.
  • ureq-outside-telemetry (:66-75, whitelisting only /src/core/telemetry.rs) vs src/core/telemetry_cmd.rs:187.

5. Releases do not run CI at all

ci.yml:4 triggers on pull_request. cd.yml:5 releases on push to develop/master. A direct push therefore ships without passing any of the above.

Also: the savings threshold is written down twice with different numbers, and neither gates

  • .claude/rules/cli-testing.md:124 — "≥60% savings is the release blocker"
  • CONTRIBUTING.md:268 — ">=20%"
  • scripts/benchmark.sh:693-694 prints WARNING: …% good (target 60%) without touching EXIT_CODE; only negative savings and empty output fail.

Suggested directions (untested — no programmer has reviewed these)

  • set -o pipefail (or ${PIPESTATUS[0]}) on the two piped steps, then confirm both jobs are in the required-status-check list.
  • Add [lints.clippy] with indexing_slicing and string_slice at warn first, to size the backlog before turning them on.
  • Either drop --baseline-commit for the two ERROR rules or add explicit // nosemgrep with a reason at the two sanctioned sites, so "no violations" means what it says.
  • Reconcile the two savings numbers and decide whether the benchmark should gate.

Happy to be told any of these are deliberate — several may be.

Activity

  1. pszymkowiak commented on Aug 4, 2026

    @pszymkowiak
    Collaborator

    wshm · Automated triage by AI

    🔍 Automated Triage

    🐛 Category bug
    🟠 Priority high
    🎯 Confidence 78%

    Summary

    CI's audit/clippy gates are non-blocking due to missing pipefail (silent pass-through), no clippy lint config for indexing/string-slicing, semgrep baseline-commit grandfathers existing violations, and releases (push to develop/master) skip CI entirely — recommend fixing pipefail, adding [lints.clippy], tightening semgrep baseline, and gating CD on CI.

    📁 Relevant files
    • .github/workflows/ci.yml
    • .github/workflows/cd.yml
    • Cargo.toml
    • .semgrep.yml
    • scripts/benchmark.sh
    • CONTRIBUTING.md
    • .claude/rules/cli-testing.md
    • src/core/utils.rs
    • src/cmds/js/prisma_cmd.rs

    Triaged automatically by wshm · This is an automated analysis, not a human review.

  2. pszymkowiak commented on Aug 4, 2026

    @pszymkowiak
    Collaborator

    wshm · Automated triage by AI

    🔍 Automated Triage

    🐛 Category bug
    🟡 Priority medium
    🎯 Confidence 72%

    Summary

    CI's cargo-audit and clippy panic-lint checks can never fail the build (missing pipefail) and semgrep's baseline-commit exempts pre-existing ERROR-severity violations, plus no clippy lint guards unsafe &str byte-slicing — fix pipefail, add indexing_slicing/string_slice lints, and reconcile semgrep/benchmark gating.

    📁 Relevant files
    • .github/workflows/ci.yml
    • .github/workflows/cd.yml
    • Cargo.toml
    • .semgrep.yml
    • scripts/benchmark.sh
    • CONTRIBUTING.md
    • .claude/rules/cli-testing.md
    • src/core/utils.rs
    • src/cmds/js/prisma_cmd.rs

    Triaged automatically by wshm · This is an automated analysis, not a human review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions