Are conditional expressions that have test expressions without parentheses valid Metapath?

[aj-stein-gsa]

Describe the bug

Per previous discussion with the FedRAMP developer team, metaschema-java 2.2.0 (as released with oscal-cli 2.3.1) allows conditional Metapath expressions where the test expression after the if does not have parentheses. According to my read of the spec, the EBNF specifies that behavior is non-conformant.

Who is the bug affecting

Developers using conditonal logic for Metapath constraints' test attribute.

How do we replicate this issue

Integration and/or unit tests TBD. For now, reveiw the following OSCAL-specific example in the FedRAMP developer team's notes page in the wiki with a simple constraint module with one constraint and a sample file.

The sample SSP:

<?xml version="1.0" encoding="UTF-8"?>
<system-security-plan xmlns="http://csrc.nist.gov/ns/oscal/1.0"
                      xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                      xsi:schemaLocation="http://csrc.nist.gov/ns/oscal/1.0 https://github.com/usnistgov/OSCAL/releases/download/v1.1.2/oscal_ssp_schema.xsd"
                      uuid="12345678-1234-4321-8765-123456789012">
  <metadata>
    <oscal-version>1.1.2</oscal-version>
  </metadata>
</system-security-plan>

Validation completes but with a constraint violation error with the document instance above (there are no library exceptions, evaluation completes).

<?xml version="1.0" encoding="UTF-8"?>
<metaschema-meta-constraints xmlns="http://csrc.nist.gov/ns/oscal/metaschema/1.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://csrc.nist.gov/ns/oscal/metaschema/1.0 https://raw.githubusercontent.com/metaschema-framework/metaschema/refs/heads/develop/schema/xml/metaschema-meta-constraints.xsd">
    <context>
        <metapath target="/system-security-plan/metadata"/>
        <constraints>
            <expect id="oscal-version-required" target="oscal-version" test="if . = '1.1.2' then true() else false()" level="ERROR">
                <message>A FedRAMP document MUST have a valid version. DEBUG {.}</message>
            </expect>
        </constraints>
    </context>
</metaschema-meta-constraints>

The above constraint also, when violated, has a DEBUG value of 1.1.2, meaning the constraint should not be violated.

The issue is resolved when the test expression properly uses parens.

<?xml version="1.0" encoding="UTF-8"?>
<metaschema-meta-constraints xmlns="http://csrc.nist.gov/ns/oscal/metaschema/1.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://csrc.nist.gov/ns/oscal/metaschema/1.0 https://raw.githubusercontent.com/metaschema-framework/metaschema/refs/heads/develop/schema/xml/metaschema-meta-constraints.xsd">
    <context>
        <metapath target="/system-security-plan/metadata"/>
        <constraints>
            <expect id="oscal-version-required" target="oscal-version" test="if (. = '1.1.2') then true() else false()" level="ERROR">
                <message>A FedRAMP document MUST have a valid version. DEBUG {.}</message>
            </expect>
        </constraints>
    </context>
</metaschema-meta-constraints>

Expected behavior (i.e. solution)

When usng invalid test expression syntax, the Metaschema process in metaschema-java returns an exception indicating the syntax is invalid.

Other comments

No response

[david-waltermire]

Given the expression if 'a' = '1.1.2' then true() else false(), this is how ANTLR parses the statement:

expr(ignored): if
  exprsingle(ignored): if
    orexpr(ignored): if
      andexpr(ignored): if
        comparisonexpr(ignored): if
          stringconcatexpr(ignored): if
            rangeexpr(ignored): if
              additiveexpr(ignored): if
                multiplicativeexpr(ignored): if
                  unionexpr(ignored): if
                    intersectexceptexpr(ignored): if
                      instanceofexpr(ignored): if
                        treatexpr(ignored): if
                          castableexpr(ignored): if
                            castexpr(ignored): if
                              arrowexpr(ignored): if
                                unaryexpr(ignored): if
                                  valueexpr(ignored): if
                                    simplemapexpr(ignored): if
                                      pathexpr(ignored): if
                                        relativepathexpr(ignored): if
                                          stepexpr(ignored): if
                                            axisstep: if
                                              forwardstep(ignored): if
                                                abbrevforwardstep(ignored): if
                                                  nodetest(ignored): if
                                                    nametest(ignored): if
                                                      eqname: if
                                              predicatelist: 

Given a correct statement if ('a' = '1.1.2') then true() else false(), ANTLR parses it as follows:

expr(ignored): if('a'='1.1.2')thentrue()elsefalse()
  exprsingle(ignored): if('a'='1.1.2')thentrue()elsefalse()
    ifexpr: if('a'='1.1.2')thentrue()elsefalse()
      expr(ignored): 'a'='1.1.2'
        exprsingle(ignored): 'a'='1.1.2'
          orexpr(ignored): 'a'='1.1.2'
            andexpr(ignored): 'a'='1.1.2'
              comparisonexpr: 'a'='1.1.2'
                stringconcatexpr(ignored): 'a'
                  rangeexpr(ignored): 'a'
                    additiveexpr(ignored): 'a'
                      multiplicativeexpr(ignored): 'a'
                        unionexpr(ignored): 'a'
                          intersectexceptexpr(ignored): 'a'
                            instanceofexpr(ignored): 'a'
                              treatexpr(ignored): 'a'
                                castableexpr(ignored): 'a'
                                  castexpr(ignored): 'a'
                                    arrowexpr(ignored): 'a'
                                      unaryexpr(ignored): 'a'
                                        valueexpr(ignored): 'a'
                                          simplemapexpr(ignored): 'a'
                                            pathexpr(ignored): 'a'
                                              relativepathexpr(ignored): 'a'
                                                stepexpr(ignored): 'a'
                                                  postfixexpr(ignored): 'a'
                                                    primaryexpr(ignored): 'a'
                                                      literal: 'a'
                generalcomp: =
                stringconcatexpr(ignored): '1.1.2'
                  rangeexpr(ignored): '1.1.2'
                    additiveexpr(ignored): '1.1.2'
                      multiplicativeexpr(ignored): '1.1.2'
                        unionexpr(ignored): '1.1.2'
                          intersectexceptexpr(ignored): '1.1.2'
                            instanceofexpr(ignored): '1.1.2'
                              treatexpr(ignored): '1.1.2'
                                castableexpr(ignored): '1.1.2'
                                  castexpr(ignored): '1.1.2'
                                    arrowexpr(ignored): '1.1.2'
                                      unaryexpr(ignored): '1.1.2'
                                        valueexpr(ignored): '1.1.2'
                                          simplemapexpr(ignored): '1.1.2'
                                            pathexpr(ignored): '1.1.2'
                                              relativepathexpr(ignored): '1.1.2'
                                                stepexpr(ignored): '1.1.2'
                                                  postfixexpr(ignored): '1.1.2'
                                                    primaryexpr(ignored): '1.1.2'
                                                      literal: '1.1.2'
      exprsingle(ignored): true()
        orexpr(ignored): true()
          andexpr(ignored): true()
            comparisonexpr(ignored): true()
              stringconcatexpr(ignored): true()
                rangeexpr(ignored): true()
                  additiveexpr(ignored): true()
                    multiplicativeexpr(ignored): true()
                      unionexpr(ignored): true()
                        intersectexceptexpr(ignored): true()
                          instanceofexpr(ignored): true()
                            treatexpr(ignored): true()
                              castableexpr(ignored): true()
                                castexpr(ignored): true()
                                  arrowexpr(ignored): true()
                                    unaryexpr(ignored): true()
                                      valueexpr(ignored): true()
                                        simplemapexpr(ignored): true()
                                          pathexpr(ignored): true()
                                            relativepathexpr(ignored): true()
                                              stepexpr(ignored): true()
                                                postfixexpr(ignored): true()
                                                  primaryexpr(ignored): true()
                                                    functioncall: true()
                                                      eqname: true
                                                      argumentlist: ()
      exprsingle(ignored): false()
        orexpr(ignored): false()
          andexpr(ignored): false()
            comparisonexpr(ignored): false()
              stringconcatexpr(ignored): false()
                rangeexpr(ignored): false()
                  additiveexpr(ignored): false()
                    multiplicativeexpr(ignored): false()
                      unionexpr(ignored): false()
                        intersectexceptexpr(ignored): false()
                          instanceofexpr(ignored): false()
                            treatexpr(ignored): false()
                              castableexpr(ignored): false()
                                castexpr(ignored): false()
                                  arrowexpr(ignored): false()
                                    unaryexpr(ignored): false()
                                      valueexpr(ignored): false()
                                        simplemapexpr(ignored): false()
                                          pathexpr(ignored): false()
                                            relativepathexpr(ignored): false()
                                              stepexpr(ignored): false()
                                                postfixexpr(ignored): false()
                                                  primaryexpr(ignored): false()
                                                    functioncall: false()
                                                      eqname: false
                                                      argumentlist: ()

It would be helpful to look into the how to adjust the grammar to deal with failing the error case.

Here is a quick test for doing this using metaschema-java:

Metapath10Lexer lexer = new Metapath10Lexer(CharStreams.fromString("if ('a' = '1.1.2') then true() else false()"));
    lexer.removeErrorListeners();
    lexer.addErrorListener(new FailingErrorListener());

    CommonTokenStream tokens = new CommonTokenStream(lexer);
    Metapath10 parser = new Metapath10(tokens);
    parser.removeErrorListeners();
    parser.addErrorListener(new FailingErrorListener());
    parser.setErrorHandler(new DefaultErrorStrategy() {

      @Override
      public void sync(Parser recognizer) {
        // disable
      }
    });

    ParseTree tree = ObjectUtils.notNull(parser.expr());
    try (ByteArrayOutputStream os = new ByteArrayOutputStream()) {
      try (PrintStream ps = new PrintStream(os, true, StandardCharsets.UTF_8)) {
        ParseTreePrinter printer = new ParseTreePrinter(ps);
        printer.print(tree, Metapath10.ruleNames);
        ps.flush();
      }
      System.out.println(String.format("Metapath AST:%n%s", os.toString(StandardCharsets.UTF_8)));
    }

[david-waltermire]

Turns out this is caused here:

metaschema-java/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/MetapathExpression.java

Line 136 in 964b845

ParseTree tree = ObjectUtils.notNull(parser.expr());

This is parsing the expr context, which doesn't have an EOF so it is doing a partial parse. According to:

metaschema-java/core/src/main/antlr4/Metapath10.g4

Line 14 in 964b845

expr : exprsingle ( COMMA exprsingle)* ;

It should be parsing metapath instead which has the EOF.

metaschema-java/core/src/main/antlr4/Metapath10.g4

Line 8 in 964b845

metapath : expr EOF ;

This one line change will fix the issue.