Skip to content

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

Description

@silug

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.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    • Status
      In Progress

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions