Skip to content

ci: check Gemfile.lock against ruby-advisory-db - #18

Merged
minerva-sky merged 1 commit into
codenamev:mainfrom
minerva-sky:deps/ci-advisory-check
Aug 19, 2026
Merged

ci: check Gemfile.lock against ruby-advisory-db#18
minerva-sky merged 1 commit into
codenamev:mainfrom
minerva-sky:deps/ci-advisory-check

Conversation

@minerva-sky

Copy link
Copy Markdown
Collaborator

What's broken

MAINTAINING.md:121 has told maintainers to run a security audit before every release. bundler-audit appears nowhere in the Gemfile, nowhere in Gemfile.lock, and nowhere in .github/. That step has only ever worked for someone who happened to have the gem installed globally, which means it has been a coin flip dressed as a checklist item.

You can see what the coin flip cost in #13: fourteen advisories across faraday, uri, concurrent-ruby, addressable and rexml, sitting in the lockfile, surfaced only because somebody typed bundler-audit by hand. The faraday ones were the ones that mattered, since faraday is ruby-openai's transport and therefore on the path of every LLM call this gem makes.

What this does

Adds .github/workflows/audit.yml, a separate workflow that runs bundler-audit against Gemfile.lock:

  • on PRs and pushes to main that touch Gemfile, Gemfile.lock, the gemspec, or the workflow itself
  • every Monday at 06:00 UTC, which is the only trigger that can see an advisory published against a lockfile nobody has touched
  • on demand via workflow_dispatch

Plus .bundler-audit.yml, empty, so that an advisory which genuinely can't be fixed by an upgrade has one reviewable place to be recorded rather than living in someone's shell history. And two doc lines pointing at the same command.

Decisions worth arguing with

The scanner installs outside the bundle. gem install bundler-audit -v '~> 0.9', no bundle exec. A lockfile scanner has no business inside the lockfile it scans: put it in the Gemfile and the day bundler-audit caps thor (a runtime dependency of this gem, at ~> 1.2) your test job goes red for a reason that has nothing to do with the gem.

Its own workflow file, not a job in main.yml. A workflow-level schedule: fires every job in the file, so folding this into main.yml would have meant running 610 specs every Monday and getting a red X that could mean either "an advisory landed" or "a spec broke."

Fetching the database and checking the lockfile are separate steps. bundler-audit update hard-exits on a failed clone, so a github.com blip in an unattended Monday run would otherwise be indistinguishable from a real finding. Split, the step name tells you which. The fetch gets one retry; the check gets none, because a check failure is the point.

The paths: filter is load-bearing. Without it, an advisory published on a Tuesday morning reddens every open PR, including ones that never touched a dependency, and the fix a contributor is told to make is an unrelated upgrade they didn't sign up for. With it, only dependency PRs can be blocked.

Limitations, stated up front

  1. Don't make this a required status check while the paths: filter is there. A path-filtered workflow that doesn't trigger reports no status, not a passing one, so a required Advisories / bundler-audit would leave every README PR stuck on "Expected, waiting for status" forever. There's a comment in the workflow saying so. (Not live today: branches/main/protection returns 404 and rulesets returns [], so main has no protection at all. Which also means this is a signal, not an enforced gate.)
  2. GitHub disables scheduled workflows after 60 days of repository inactivity, and only the Actions-tab "Enable workflow" control brings one back. That's exactly the dormant stretch when a stale lockfile is most likely, so the weekly sweep is weakest precisely where it's needed most.
  3. A failing scheduled run emails the workflow file's last committer, which after this merges is me, and I can't fix anything without a PR. Nobody with commit rights is automatically told.
  4. This is not consumer protection. Gemfile.lock is excluded from the packaged gem, so what's audited here is your dev and CI resolution, not what anyone installing agentic resolves. addressable and rexml from Patch 14 security advisories with conservative lockfile bumps #13 reach the lock only through webmock and rubocop and never reach a consumer at all. The runtime-transitive subtree (faraday, uri, concurrent-ruby via ruby-openai) does overlap, so it's an early warning there, and nothing stronger.

Limitations 1 through 3 all argue the same thing, which is that this is the belt and not the braces. #17 is the actual fix: Dependabot alerts are disabled on this repo, and two checkboxes in Settings buy you a signal with none of the failure modes above. If you only do one, do that one.

Verification

Run on Ruby 4.0.6 against upstream/main at 99a9167:

$ gem install bundler-audit --no-document -v '~> 0.9'   # outside the bundle, as CI does
$ bundler-audit update && bundler-audit check
No vulnerabilities found                                 # 1233 advisories, exit 0

.bundler-audit.yml parses and is honored ({"ignore" => []}). The workflow YAML parses; triggers resolve to pull_request / push / schedule / workflow_dispatch and the five steps are as written. No Ruby code is touched, so rspec and standardrb are unaffected; the CI run on this PR will exercise the new workflow against itself, since the workflow file is in its own paths: list.

Related

Panel

Four adversarial reviewers were run against the diff with instructions to refute it, twice for the three who objected. The first round killed the original design outright: it had been a job bolted into main.yml with bundler-cache: true, bundler-audit added to the Gemfile, a workflow-level cron that also ran the specs, and a second hardcoded 3.2.4. Almost none of that survived.

  • DHH (Ruby conventions, architectural purity): objected-then-cleared. Round 1: the cron fires build too, the duplicated Ruby pin, bundler-cache installing 75 gems to read one text file, CONTRIBUTING contradicting the offline-rake rationale. All fixed. Round 2: he read bundler-audit's cli.rb and showed that the --ignore CVE-... flag I'd documented is ephemeral, so a contributor would go green locally and stay red in CI, recreating the exact rot this PR fixes. Fixed by committing .bundler-audit.yml and rewriting the doc line.
  • Obie Fernandez (production edge cases): objected-then-cleared. Ten objections, of which the sharpest was that MAINTAINING.md was the real justification and I'd buried it. He was right and it now leads. Round 2 caught that my own paths: fix had armed the required-check deadlock, which is limitation 1 above and would not otherwise be in this PR. He also traced that addressable and rexml never reach a consumer, which is why limitation 4 is worded the way it is.
  • Vladimir Dementyev (performance, testability): objected-then-cleared. Round 1 got the bundle removal, the separate file, the paths scoping, timeout-minutes and permissions: contents: read; he withdrew his advisory-DB caching objection on the argument that a cached advisory database is a stale one. Round 2: the version bound ~> 0.9 existed in CI but not in either doc, gem install alone doesn't put the binary on a human's PATH, and the header comment promised a one-cause signal the file couldn't keep. All three fixed, the last by splitting fetch from check and rewriting the comment to be true.
  • The gem consumer (semver contract): cleared on the first pass, and not by taking my word for it: he executed the gemspec's own reject block over git ls-files to confirm Gemfile.lock isn't packaged, and grepped lib/ and exe/ for any runtime code reading the lockfile before concluding no saved agent or plan can be affected.

One honesty note about the protocol: round-two objections were fixed rather than sent back for a third adjudication, so no expert has signed off on the final diff verbatim. Each round-two fix was the one that expert specified, which is why I think that's defensible, but you're reviewing the argument, so you should know where it stops.

Two corrections the panel made to claims I'd have otherwise shipped: bundle audit was never a wrong spelling (bundler-audit ships both bundle-audit and bundler-audit executables, so bundler resolves it), and --version '~> 0.9' versus a bare gem install is a real divergence, not a cosmetic one.

MAINTAINING.md's release checklist has prescribed a bundler-audit run since
before this commit, but bundler-audit appears nowhere in the Gemfile, the
lockfile, or any workflow. The step only ever worked for a maintainer who
happened to have the gem installed globally, which is how 14 advisories
reached codenamev#13 without anything reporting them.

Adds a separate Advisories workflow: PRs and pushes that move Gemfile,
Gemfile.lock or the gemspec, plus a Monday sweep for advisories published
against a lockfile nobody touched. The scanner installs outside the bundle
so its resolution stays independent of the gem's, and .bundler-audit.yml
gives a deliberate exception somewhere reviewable.
@minerva-sky minerva-sky added the loop:deps Dependency / language upgrade loop label Aug 19, 2026
@minerva-sky
minerva-sky merged commit 1ae199e into codenamev:main Aug 19, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

loop:deps Dependency / language upgrade loop

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants