Exits normally when --gemfile-lock is not a lock file, should exit with error

[eliotsykes]

Description

It is a bit too easy for a developer to accidentally misconfigure the --gemfile-lock option and think their Gemfile.lock has no vulnerabilities. For example if they use Gemfile instead of their Gemfile.lock which has vulnerabilities, the bundle-audit command returns with exit code 0 and does not report it has been given an invalid lock file.

Steps To Reproduce

Steps to reproduce the bug:

# Command exits successfully, when Gemfile is accidentally given
# instead of the Gemfile.lock which has vulnerable gems:
bundle-audit check --gemfile-lock Gemfile
# => No vulnerabilities found

bundle-audit check --gemfile-lock Gemfile.lock
# => Vulnerabilities found!

# Giving any file other than the lock file, command exits successfully:
bundle-audit check --gemfile-lock README.md  
# => No vulnerabilities found

Expected Behavior

Command should exit with non-zero exit code, and an appropriate message output saying invalid lock file given.

Environment

$ bundler-audit --version
bundler-audit 0.9.2

$ bundle --version
Bundler version 2.5.22

$ ruby --version
3.3.8

[postmodern]

This appears to be an upstream bug. It appears that Bundler::LockfileParser will happily parse non-lockfiles.

Bundler::LockfileParser.new(File.read('README.md'))
=> 
#<Bundler::LockfileParser:0x00007ff582610830
 @dependencies={},
 @lockfile_path="Gemfile.lock",
 @most_specific_locked_platform=nil,
 @parse_method=nil,
 @platforms=[],
 @pos=
  #<Bundler::LockfileParser::Position:0x00007ff582cd5d00 @column=1, @line=267>,
 @sources=[],
 @specs=[]>

We could check if the file extension is .lock and raise an exception and exit with an error. We could also print a warning and not exit with an error?

[postmodern]

This however raises the question, how would the user accidentally confuse README.md for a Gemfile.lock file? I agree bundler-audit should print some kind of error message, but what scenario would cause another file to accidentally be given instead of a bundler lock file?

[eliotsykes]

I probably shouldn't have included the README.md example as its a distraction.

The Gemfile example is the more likely bug, and I was briefly bit by it, when it is accidentally used instead of Gemfile.lock:

# Command exits successfully, when Gemfile is accidentally given
# instead of the Gemfile.lock which has vulnerable gems:
bundle-audit check --gemfile-lock Gemfile
# => No vulnerabilities found

The specific situation I ran into was writing a script that wrapped this command for a dual-booting Rails app to use in CI. It needed to use Gemfile.next(.lock) or Gemfile(.lock), depending on the value of BUNDLE_GEMFILE.

My first attempt I mistakenly used $BUNDLE_GEMFILE instead of ${BUNDLE_GEMFILE}.lock:

#!/usr/bin/env bash

# DON'T DO THIS!
bundle exec bundle-audit check --update --gemfile-lock $BUNDLE_GEMFILE
# DON'T DO THIS!

The above command is using the wrong file but surprisingly exits normally with the "No vulnerabilities found" message. Thankfully I knew the lock file had vulnerabilities in it which lead to the discovery of this issue. It would be easy for someone to not realize this and leave the above buggy script as is and never discover their app has vulnerable gems.

Tweaking this script to use the lock file worked as intended:

#!/usr/bin/env bash

lock_file="${BUNDLE_GEMFILE:-Gemfile}.lock"
bundle exec bundle-audit check --update --gemfile-lock $lock_file