Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix case where res is nil in etc_group #2984

Merged
merged 1 commit into from
Apr 26, 2018

Conversation

chris-rock
Copy link
Contributor

With #2359 we missed some edge cases, where res is nil, which happens in inspec check mode.

With that improvement, https://github.com/dev-sec/cis-docker-benchmark passes inspec check again.

Going from:

$ inspec check /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark
bundler: failed to load command: inspec (/Library/Ruby/Gems/2.3.0/bin/inspec)
NoMethodError: undefined method `empty?' for nil:NilClass
  /Users/chartmann/go/src/github.com/chef/inspec/lib/resources/etc_group.rb:81:in `where'
  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/host_configuration.rb:168:in `block in load_with_context'
  /Users/chartmann/go/src/github.com/chef/inspec/lib/inspec/rule.rb:50:in `instance_eval'
  /Users/chartmann/go/src/github.com/chef/inspec/lib/inspec/rule.rb:50:in `initialize'
  /Users/chartmann/go/src/github.com/chef/inspec/lib/inspec/control_eval_context.rb:75:in `new'
  /Users/chartmann/go/src/github.com/chef/inspec/lib/inspec/control_eval_context.rb:75:in `block (2 levels) in create'
  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/host_configuration.rb:149:in `load_with_context'
  /Users/chartmann/go/src/github.com/chef/inspec/lib/inspec/profile_context.rb:158:in `instance_eval'
  /Users/chartmann/go/src/github.com/chef/inspec/lib/inspec/profile_context.rb:158:in `load_with_context'
  /Users/chartmann/go/src/github.com/chef/inspec/lib/inspec/profile_context.rb:142:in `load_control_file'
  /Users/chartmann/go/src/github.com/chef/inspec/lib/inspec/profile.rb:170:in `block in collect_tests'
  /Users/chartmann/go/src/github.com/chef/inspec/lib/inspec/profile.rb:167:in `each'
  /Users/chartmann/go/src/github.com/chef/inspec/lib/inspec/profile.rb:167:in `collect_tests'
  /Users/chartmann/go/src/github.com/chef/inspec/lib/inspec/profile.rb:473:in `load_checks_params'
  /Users/chartmann/go/src/github.com/chef/inspec/lib/inspec/profile.rb:466:in `load_params'
  /Users/chartmann/go/src/github.com/chef/inspec/lib/inspec/profile.rb:160:in `params'
  /Users/chartmann/go/src/github.com/chef/inspec/lib/inspec/profile.rb:326:in `controls_count'
  /Users/chartmann/go/src/github.com/chef/inspec/lib/inspec/profile.rb:297:in `check'
  /Users/chartmann/go/src/github.com/chef/inspec/lib/inspec/cli.rb:76:in `check'
  /Library/Ruby/Gems/2.3.0/gems/thor-0.20.0/lib/thor/command.rb:27:in `run'
  /Library/Ruby/Gems/2.3.0/gems/thor-0.20.0/lib/thor/invocation.rb:126:in `invoke_command'
  /Library/Ruby/Gems/2.3.0/gems/thor-0.20.0/lib/thor.rb:387:in `dispatch'
  /Library/Ruby/Gems/2.3.0/gems/thor-0.20.0/lib/thor/base.rb:466:in `start'
  /Users/chartmann/go/src/github.com/chef/inspec/bin/inspec:12:in `<top (required)>'
  /Library/Ruby/Gems/2.3.0/bin/inspec:22:in `load'
  /Library/Ruby/Gems/2.3.0/bin/inspec:22:in `<top (required)>'

to

inspec check /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark
Location:    /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark
Profile:     cis-docker-benchmark
Controls:    106
Timestamp:   2018-04-20T22:33:31+02:00
Valid:       true

  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/docker_daemon_configuration.rb:556: Control docker-2.24 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_images.rb:37: Control docker-4.1 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_images.rb:146: Control docker-4.6 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_images.rb:166: Control docker-4.7 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_images.rb:209: Control docker-4.9 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_runtime.rb:48: Control docker-5.1 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_runtime.rb:103: Control docker-5.3 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_runtime.rb:130: Control docker-5.4 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_runtime.rb:151: Control docker-5.5 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_runtime.rb:181: Control docker-5.6 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_runtime.rb:208: Control docker-5.7 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_runtime.rb:236: Control docker-5.8 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_runtime.rb:250: Control docker-5.9 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_runtime.rb:271: Control docker-5.10 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_runtime.rb:293: Control docker-5.11 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_runtime.rb:316: Control docker-5.12 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_runtime.rb:341: Control docker-5.13 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_runtime.rb:368: Control docker-5.14 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_runtime.rb:394: Control docker-5.15 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_runtime.rb:415: Control docker-5.16 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_runtime.rb:436: Control docker-5.17 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_runtime.rb:460: Control docker-5.18 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_runtime.rb:482: Control docker-5.19 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_runtime.rb:505: Control docker-5.20 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_runtime.rb:526: Control docker-5.21 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_runtime.rb:588: Control docker-5.24 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_runtime.rb:609: Control docker-5.25 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_runtime.rb:633: Control docker-5.26 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_runtime.rb:671: Control docker-5.28 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_runtime.rb:713: Control docker-5.30 has no tests defined
  !  /Users/chartmann/go/src/github.com/dev-sec/cis-docker-benchmark/controls/container_runtime.rb:735: Control docker-5.31 has no tests defined

Summary:     0 errors, 31 warnings

Signed-off-by: Christoph Hartmann <chris@lollyrock.com>
@chris-rock chris-rock requested a review from a team as a code owner April 20, 2018 20:39
Copy link
Contributor

@jquick jquick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @chris-rock !

@clintoncwolfe clintoncwolfe self-requested a review April 26, 2018 16:09
Copy link
Contributor

@clintoncwolfe clintoncwolfe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand we need to get that check passing to unblock other projects, so I'll approve this for now, and open an issue to improve behavior when the path is bad.

idx = fields[k.to_sym]
next if idx.nil?
res = res.select { |x| x[idx].to_s == v.to_s }
unless res.nil?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty concerning. If entries is nil, then most of the resource is broken, and this resource does little to cushion the blow. This fix will work for inspec check, but won't help people using exec; we're not exposing the issue, that the group file was not found.

inspec> describe etc_group('/no/such/file').where(group_name: 'awesome') do
inspec>   its('users') { should include 'bob' }
inspec> end

Profile: inspec-shell
Version: (not specified)

  #<Inspec::Resources::EtcGroupView:0x00007fe41a46d628>
     ×  users should include "bob"
     expected nil to include "bob", but it does not respond to `include?`

Test Summary: 0 successful, 1 failure, 0 skipped
inspec> inspec.version
=> "2.1.54"
inspec> exit
[cwolfe@lodi inspec-review]$ git log --oneline -n 1
a6039358 (HEAD -> chris-rock/etc-group-nil, origin/chris-rock/etc-group-nil) Fix case where res is nil in etc_group

@clintoncwolfe clintoncwolfe merged commit 6b0c67e into master Apr 26, 2018
@clintoncwolfe clintoncwolfe deleted the chris-rock/etc-group-nil branch April 26, 2018 16:29
@clintoncwolfe clintoncwolfe added CLI: check Type: Bug Feature not working as expected and removed bug labels Apr 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: CLI: check Type: Bug Feature not working as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants