Fix describe block extraction bug (#18)

* Add test for metadata in describe block

Signed-off-by: Pavan Kumar Bondalapati <pbondalapati@mitre.org>

* Apply cookstyle format

* Add test cases for describe block

* Handle describe block test cases

* Update test description

* Update JSON object after cookstyle format

* Overhaul

* Fix describe block extraction bug

* Add test case for multi-line describe block

* Add escaped newline after joining strings

* added test case for keywords showing up inside strings in the test code

Signed-off-by: Will Dower <wdower@mitre.org>

* fixed whitespacing in test case for keywords in strings

Signed-off-by: Will Dower <wdower@mitre.org>

* changed test controls dir name

* Rename test suite for describe block extraction

* Add comments to helper functions

* Implement enum to specify magic numbers

* Eliminate unneeded function and update some variable names

Signed-off-by: Emily Rodriguez <ecrodriguez@mm279976-pc.lan>

* Add example of percent string/literal in comment

* Transfer stack push/pop conditions to constants

* Update file name for backticks.rb

* Add test cases for delimiters in header

* Add handling for variable delimiters

* Remove dependency on getDistinctRanges()

* Update package-lock.json from npm upgrade

* Remove unnecessary index update

* Delete commented index update

* add comments and update some variable names for clarity

Signed-off-by: Emily Rodriguez <ecrodriguez@mm279976-pc.lan>

* update to include the filtering for only multiline ranges

Signed-off-by: Emily Rodriguez <ecrodriguez@mm279976-pc.lan>

---------

Signed-off-by: Pavan Kumar Bondalapati <pbondalapati@mitre.org>
Signed-off-by: Will Dower <wdower@mitre.org>
Signed-off-by: Emily Rodriguez <ecrodriguez@mm279976-pc.lan>
Co-authored-by: Pavan Kumar Bondalapati <pbondalapati@mitre.org>
Co-authored-by: Will Dower <wdower@mitre.org>
Co-authored-by: George Dias <gdias@mitre.org>
Co-authored-by: Emily Rodriguez <ecrodriguez@mm279976-pc.lan>

src/utilities/update.ts

@@ -39,69 +39,183 @@ function projectValuesOntoExistingObj(dst: Record<string, unknown>, src: Record<
   return dst
 }
 
-/*
-  Return first index found from given array that is not an empty entry (cell)
-*/
-function getIndexOfFirstLine(auditArray: string[], index: number, action: string): number {
-  let indexVal = index;
-  while (auditArray[indexVal] === '') {
-    switch (action) {
-      case '-':
-        indexVal--
-        break;
-      case '+':
-        indexVal++
-        break;
+function getRangesForLines(text: string): number[][] {  
+  /*
+    Returns an array containing two numerical indices (i.e., start and stop  
+    line numbers) for each string or multi-line comment, given raw text as 
+    an input parameter. The raw text is a string containing the entirety of an 
+    InSpec control. 
+
+    Algorithm utilizes a pair of stacks (i.e., `stack`, `rangeStack`) to keep 
+    track of string delimiters and their associated line numbers, respectively.
+
+    Combinations Handled:
+    - Single quotes (')
+    - Double quotes (")
+    - Back ticks (`)
+    - Mixed quotes ("`'")
+    - Percent strings (%; keys: q, Q, r, i, I, w, W, x; delimiters: (), {}, 
+      [], <>, most non-alphanumeric characters); (e.g., "%q()")
+    - Percent literals (%; delimiters: (), {}, [], <>, most non-
+      alphanumeric characters); (e.g., "%()")
+    - Multi-line comments (e.g., =begin\nSome comment\n=end)
+    - Variable delimiters (i.e., paranthesis: (); array: []; hash: {})
+  */
+  const stringDelimiters: {[key: string]: string} = {'(': ')', '{': '}', '[': ']', '<': '>'}
+  const variableDelimiters: {[key: string]: string} = {'(': ')', '{': '}', '[': ']'}
+  const quotes = '\'"`'
+  const strings = 'qQriIwWxs'
+  enum skipCharLength {
+    string = '('.length,
+    percentString = 'q('.length,
+    commentBegin = '=begin'.length
+  }
+
+  const stack: string[] = []
+  const rangeStack: number[][] = []
+  const ranges: number[][] = []
+
+  const lines = text.split('\n')
+  for (let i = 0; i < lines.length; i++) {
+    let j = 0
+    while (j < lines[i].length) {
+      const line = lines[i]
+      let char = line[j]
+
+      const isEmptyStack = (stack.length == 0)
+      const isNotEmptyStack = (stack.length > 0)
+
+      const isQuoteChar = quotes.includes(char)
+      const isNotEscapeChar = ((j == 0) || (j > 0 && line[j - 1] != '\\'))
+      const isPercentChar = (char == '%')
+      const isVariableDelimiterChar = Object.keys(variableDelimiters).includes(char)
+      const isStringDelimiterChar = ((j < line.length - 1) && (/^[^A-Za-z0-9]$/.test(line[j + 1])))
+      const isCommentBeginChar = ((j == 0) && (line.length >= 6) && (line.slice(0, 6) == '=begin'))
+
+      const isPercentStringKeyChar = ((j < line.length - 1) && (strings.includes(line[j + 1])))
+      const isPercentStringDelimiterChar = ((j < line.length - 2) && (/^[^A-Za-z0-9]$/.test(line[j + 2])))
+      const isPercentString = (isPercentStringKeyChar && isPercentStringDelimiterChar)
+
+      let baseCondition = (isEmptyStack && isNotEscapeChar)
+      const quotePushCondition = (baseCondition && isQuoteChar)
+      const variablePushCondition = (baseCondition && isVariableDelimiterChar)
+      const stringPushCondition = (baseCondition && isPercentChar && isStringDelimiterChar)
+      const percentStringPushCondition = (baseCondition && isPercentChar && isPercentString)
+      const commentBeginCondition = (baseCondition && isCommentBeginChar)
+
+      if (stringPushCondition) {
+        j += skipCharLength.string // j += 1
+      } else if (percentStringPushCondition) {
+        j += skipCharLength.percentString // j += 2
+      } else if (commentBeginCondition) {
+        j += skipCharLength.commentBegin // j += 6
+      }
+      char = line[j]
+
+      baseCondition = (isNotEmptyStack && isNotEscapeChar)
+      const delimiterCondition = (baseCondition && Object.keys(stringDelimiters).includes(stack[stack.length - 1]))
+      const delimiterPushCondition = (delimiterCondition && (stack[stack.length - 1] == char))
+      const delimiterPopCondition = (delimiterCondition && (stringDelimiters[stack[stack.length - 1] as string] == char))
+      const basePopCondition = (baseCondition && (stack[stack.length - 1] == char) && !Object.keys(stringDelimiters).includes(char))
+      const isCommentEndChar = ((j == 0) && (line.length >= 4) && (line.slice(0, 4) == '=end'))
+      const commentEndCondition = (baseCondition && isCommentEndChar && (stack[stack.length - 1] == '=begin'))
+
+      const popCondition = (basePopCondition || delimiterPopCondition || commentEndCondition)
+      const pushCondition = (quotePushCondition || variablePushCondition || stringPushCondition || 
+        percentStringPushCondition || delimiterPushCondition || commentBeginCondition)
+
+      if (popCondition) {
+        stack.pop()
+        rangeStack[rangeStack.length -1].push(i)
+        const range_ = rangeStack.pop() as number[]
+        if (rangeStack.length == 0) {
+          ranges.push(range_)
+        }
+      } else if (pushCondition) {
+        if (commentBeginCondition) {
+          stack.push('=begin')
+        } else {
+          stack.push(char)
+        }
+        rangeStack.push([i])
+      }
+      j++
     }
   }
+  return ranges
+}
 
-  return indexVal
+function joinMultiLineStringsFromRanges(text: string, ranges: number[][]): string[] {
+  /*
+    Returns an array of strings and joined strings at specified ranges, given 
+    raw text as an input parameter.
+  */
+  const originalLines = text.split('\n')
+  const joinedLines: string[] = []
+  let i = 0
+  while (i < originalLines.length) {
+    let foundInRanges = false
+    for (const [startIndex, stopIndex] of ranges) {
+      if (i >= startIndex && i <= stopIndex) {
+        joinedLines.push(originalLines.slice(startIndex, stopIndex + 1).join('\n'))
+        foundInRanges = true
+        i = stopIndex
+        break
+      }
+    }
+    if (!foundInRanges) {
+      joinedLines.push(originalLines[i])
+    }
+    i++
+  }
+  return joinedLines
+}
+
+function getMultiLineRanges(ranges: number[][]): number[][] {
+  /*
+    Drops ranges with the same start and stop line numbers (i.e., strings 
+    that populate a single line)
+  */
+  const multiLineRanges: number[][] = []
+  for (const [start, stop] of ranges) {
+    if (start !== stop) {
+      multiLineRanges.push([start, stop])
+    }
+  }
+  return multiLineRanges
 }
 
 /*
   This is the most likely thing to break if you are getting code formatting issues.
   Extract the existing describe blocks (what is actually run by inspec for validation)
 */
 export function getExistingDescribeFromControl(control: Control): string {
-  // Algorithm:
-  //   Locate the start and end of the control string
-  //   Update the end of the control that contains information (if empty lines are at the end of the control)
-  //   loop: until the start index is changed (loop is done from the bottom up)
-  //     Clean testing array entry line (removes any non-print characters)
-  //     if: line starts with meta-information 'tag' or 'ref'
-  //       set start index to found location
-  //       break out of the loop
-  //     end
-  //   end
-  //   Remove any empty lines after the start index (in any)
-  //   Extract the describe block from the audit control given the start and end indices
-  // Assumptions: 
-  //  1 - The meta-information 'tag' or 'ref' precedes the describe block
-  // Pros: Solves the potential issue with option 1, as the lookup for the meta-information
-  //       'tag' or 'ref' is expected to the at the beginning of the line.
   if (control.code) {
-    let existingDescribeBlock = ''
-    let indexStart = control.code.toLowerCase().indexOf('control')
-    let indexEnd = control.code.toLowerCase().trimEnd().lastIndexOf('end')
-    const auditControl = control.code.substring(indexStart, indexEnd).split('\n')
-
-    indexStart = 0
-    indexEnd = auditControl.length - 1
-    indexEnd = getIndexOfFirstLine(auditControl, indexEnd, '-')
-    let index = indexEnd
-
-    while (indexStart === 0) {
-      const line = auditControl[index].toLowerCase().trim()
-      if (line.indexOf('ref') === 0 || line.indexOf('tag') === 0) {
-        indexStart = index + 1
-      }
-      index--
-    }
+    // Join multi-line strings in InSpec control.
+    const ranges = getRangesForLines(control.code)
+    const multiLineRanges = getMultiLineRanges(ranges)
+    const lines = joinMultiLineStringsFromRanges(control.code, multiLineRanges)  // Array of lines representing the full InSpec control, with multi-line strings collapsed
 
-    indexStart = getIndexOfFirstLine(auditControl, indexStart, '+')
-    existingDescribeBlock = auditControl.slice(indexStart, indexEnd + 1).join('\n').toString()
+    // Define RegExp for lines to skip when searching for describe block.
+    const skip = ['control\\W', '  title\\W', '  desc\\W', '  impact\\W', '  tag\\W', '  ref\\W']
+    const skipRegExp = RegExp(skip.map(x => `(^${x})`).join('|'))
 
-    return existingDescribeBlock
+    // Extract describe block from InSpec control with collapsed multiline strings.
+    const describeBlock: string[] = []
+    let ignoreNewLine = true
+    for (const line of lines) {
+      const checkRegExp = ((line.trim() !== '') && !skipRegExp.test(line))
+      const checkNewLine = ((line.trim() === '') && !ignoreNewLine)
+
+      // Include '\n' if it is part of describe block, otherwise skip line.
+      if (checkRegExp || checkNewLine) {
+        describeBlock.push(line)
+        ignoreNewLine = false
+      } else {
+        ignoreNewLine = true
+      }
+    }
+    return describeBlock.slice(0, describeBlock.length - 2).join('\n') // Drop trailing ['end', '\n'] from Control block.
   } else {
     return ''
   }
@@ -199,4 +313,4 @@ export function updateProfileUsingXCCDF(from: Profile, using: string, id: 'group
     diff: updatedProfile.diff,
     markdown: markdown
   }
-}
+}

test/sample_data/controls-cookstyle/SV-230385.rb

@@ -0,0 +1,68 @@
+control 'SV-230385' do
+  title 'RHEL 8 must define default permissions for logon and non-logon shells.'
+  desc  "The umask controls the default access mode assigned to newly created
+files. A umask of 077 limits new files to mode 600 or less permissive. Although
+umask can be represented as a four-digit number, the first digit representing
+special access modes is typically ignored or required to be \"0\". This
+requirement applies to the globally configured system defaults and the local
+interactive user defaults for each account on the system."
+  desc  'rationale', ''
+  desc  'check', "
+    Verify that the umask default for installed shells is \"077\".
+
+    Check for the value of the \"UMASK\" parameter in the \"/etc/bashrc\" and
+\"/etc/csh.cshrc\" files with the following command:
+
+    Note: If the value of the \"UMASK\" parameter is set to \"000\" in either
+the \"/etc/bashrc\" or the \"/etc/csh.cshrc\" files, the Severity is raised to
+a CAT I.
+
+    # grep -i umask /etc/bashrc /etc/csh.cshrc
+
+    /etc/bashrc:          umask 077
+    /etc/bashrc:          umask 077
+    /etc/csh.cshrc:      umask 077
+    /etc/csh.cshrc:      umask 077
+
+    If the value for the \"UMASK\" parameter is not \"077\", or the \"UMASK\"
+parameter is missing or is commented out, this is a finding.
+  "
+  desc 'fix', "
+    Configure the operating system to define default permissions for all
+authenticated users in such a way that the user can only read and modify their
+own files.
+
+    Add or edit the lines for the \"UMASK\" parameter in the \"/etc/bashrc\"
+and \"etc/csh.cshrc\" files to \"077\":
+
+    UMASK 077
+  "
+  impact 0.5
+  tag severity: 'medium'
+  tag gtitle: 'SRG-OS-000480-GPOS-00227'
+  tag gid: 'V-230385'
+  tag rid: 'SV-230385r627750_rule'
+  tag stig_id: 'RHEL-08-020353'
+  tag fix_id: 'F-33029r567902_fix'
+  tag cci: ['CCI-000366']
+  tag nist: ['CM-6 b']
+
+  umask_regexp = /umask\s*(?<umask_code>\d\d\d)/
+
+  bashrc_umask = file('/etc/bashrc').content.match(umask_regexp)[:umask_code]
+  cshrc_umask = file('/etc/csh.cshrc').content.match(umask_regexp)[:umask_code]
+
+  if bashrc_umask == '000' || cshrc_umask == '000'
+    impact 0.7
+    tag severity: 'high'
+  end
+
+  describe 'umask value defined in /etc/bashrc' do
+    subject { bashrc_umask }
+    it { should cmp '077' }
+  end
+  describe 'umask value defined in /etc/csh.cshrc' do
+    subject { cshrc_umask }
+    it { should cmp '077' }
+  end
+end

test/sample_data/controls-for-describe-tests/array-in-header.rb

@@ -0,0 +1,12 @@
+control 'array-in-header' do
+  tag array: [1, 2, 3, 4, 5]
+  tag array: [1, 2,
+  3, 4, 5]
+  tag array: [1,
+  2,
+
+        3,
+  4, 5]
+
+  describe_block = nil
+end

test/sample_data/controls-for-describe-tests/back-ticks.rb

@@ -0,0 +1,15 @@
+control `back-ticks` do
+  title `Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do 
+  eiusmod tempor incididunt ut labore et dolore magna aliqua.`
+  desc `Enim lobortis scelerisque \`fermentum\` dui faucibus.`
+  desc `\`Amet dictum sit amet justo. Massa id neque aliquam vestibulum 
+  morbi blandit cursus risus. Rutrum tellus pellentesque eu tincidunt 
+  tortor aliquam nulla facilisi. Molestie nunc non blandit massa enim. 
+  At urna condimentum mattis pellentesque id nibh tortor. Amet luctus 
+  venenatis lectus magna fringilla.\``
+  impact 0.5
+  tag `back-ticks`: `back ticks`
+  tag `escape` : `\`ticks\``
+
+  describe_block = nil
+end