simp / simp/rubygem-simp-scelint

check_check_ces crashes on a non-Array 'ces' value instead of reporting it

Open
#90 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Ruby
Stars
0
Forks
3
Avg merge
3d 20h
Merged PRs (30d)
5

Description

Scelint::Lint#check_check_ces (lib/scelint.rb:546) warns that ces is not an Array and then iterates it anyway:

def check_check_ces(file, file_data)
  warnings << "#{file}: bad ces '#{file_data}'" unless file_data.is_a?(Array)

  file_data.each do |key|   # <- raises when file_data is a String
    warnings << "#{file}: bad ce '#{key}'" unless key.is_a?(String)
  end
end

The unless guard is missing a return, so a String (or any non-Enumerable) reaches #each.

Reproduction

# SIMP/compliance_profiles/bad.yaml
version: 2.0.0
checks:
  check_one:
    type: puppet-class-parameter
    settings:
      parameter: foo::bar
      value: 1
    ces: "not-an-array"
$ bundle exec exe/scelint /path/to/module
undefined method 'each' for an instance of String

Backtrace:

lib/scelint.rb:549  Scelint::Lint#check_check_ces
lib/scelint.rb:592  block in Scelint::Lint#check_checks
lib/scelint.rb:570  Scelint::Lint#check_checks

Why it escapes as a raw exception

#lint wraps its body in rescue => e and converts failures into an error entry, but #merged_data_lint has no equivalent. The crash above comes through the merged-data path, so the user gets an unhandled NoMethodError and no file name rather than a lint finding.

Suggested fix

Two independent changes, both worth making:

  1. return after the warning in check_check_ces. Worth auditing the sibling check_* methods for the same shape — check_controls, check_profile_ces, check_profile_checks, and check_oval_ids all validate a container type before iterating, and should be checked for the same missing guard.
  2. Give merged_data_lint the same rescue treatment as lint, so a malformed fragment surfaces as an error rather than a stack trace.

Context

Found while comparing scelint's coverage against ComplianceEngine.schema (now reachable after #88). The schema reports this same input cleanly as a type violation, which is a reasonable argument for running a structural schema pass ahead of the semantic checks — see simp/rubygem-simp-compliance_engine#134, #135, #136 for what needs to land upstream first.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in lib/scelint.rb at check_check_ces, then inspect check_controls, check_profile_ces, check_profile_checks, check_oval_ids, lint, and merged_data_lint. Run bundle exec exe/scelint with the bad.yaml reproduction; done means invalid containers are reported without iteration crashes and merged-data failures become named lint errors rather than raw stack traces.

Written by the indexing model from the issue text.

Assessment

Tech stack
ruby
Domain
devtools
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.