oscal_complete_schema.xsd (v1.0.4) lacks with-parent-controls attribute of include-controls element

[GaryGapinski]

Describe the bug

The include-controls definition has a type oscal-complete:oscal-profile-select-control-by-id-ASSEMBLY which has a definition which lacks the with-parent-controls attribute described here in the OSCAL Profile Resolution Specification Draft.

Who is the bug affecting

Scrutinizers of the OSCAL Profile Resolution Specification Draft who eschew the inclusion of ancestor controls during profile resolution.

What is affected by this bug

Metaschema

How do we replicate this issue

Inspect the oscal-profile-select-control-by-id-ASSEMBLY assembly in oscal_complete_schema.xsd.

Expected behavior (i.e. solution)

include-controls should have an optional with-parent-controls attribute.

Other comments

The with-child-controls attribute fortunate enough to be included is not constrained to the documented values of `'yes' and 'no'.

Acceptance Criteria

• Review the reported issue and the complete schema as produced in develop, then main to check if the schema is correctly or incorrectly missing the relevant fields.
• Open a PR to fix the elements, if missing, for the OSCAL Profile metaschema definition
• Check the specification.

[aj-stein-nist]

Moved inline to bug report description.

[wendellpiez]

Requesting clarification: why is the attribute defined in the schema not working?

OSCAL/xml/schema/oscal_complete_schema.xsd

Line 2146 in f87dcb1

<xs:attribute name="with-child-controls" type="oscal-complete:token">

This can also be tested by trying a schema against a suitable document e.g. in an editor.

[GaryGapinski]

Requesting clarification: why is the [with-child-controls]attribute defined in the schema not working?

The with-child-controls attribute is defined within the schema without constraint. The with-parent-controls is the attribute missing from the schema.

with-child-controls="yes" appears to work correctly when profile resolution occurs (but not with https://github.com/usnistgov/OSCAL/tree/main/src/utils/util/resolver-pipeline — see usnistgov/liboscal-java#232).

Using https://raw.githubusercontent.com/galtm/OSCAL/saxon11/src/utils/util/resolver-pipeline/oscal-profile-RESOLVE.xsl
with-child-controls works almost correctly: #1663 is still observed.

gapinski@flexion-mac-C02FCBVSMD6N resolver-pipeline % alias xslt='java -cp ~/saxon/saxon-he-12.0.jar net.sf.saxon.Transform'                                                                  
gapinski@flexion-mac-C02FCBVSMD6N resolver-pipeline % cat c-ce-p.xml                                                                                                                          
<?xml version="1.0" encoding="UTF-8"?>
<?xml-model href="https://github.com/usnistgov/OSCAL/raw/v1.0.4/xml/schema/oscal_complete_schema.xsd" schematypens="http://www.w3.org/2001/XMLSchema" title="OSCAL complete schema" ?>
<profile xmlns="http://csrc.nist.gov/ns/oscal/1.0" uuid="3353e8e1-3eef-42ef-8163-1f71616bafca">
    <metadata>
        <title />
        <last-modified>2023-02-18T13:06:18Z</last-modified>
        <version>2023-02-18T13:06:18Z</version>
        <oscal-version>1.0.4</oscal-version>
    </metadata>
    <import href="c-ce-c.xml">
        <include-controls with-child-controls="yes">
            <with-id>c1.1</with-id>
        </include-controls>
    </import>
</profile>
gapinski@flexion-mac-C02FCBVSMD6N resolver-pipeline % cat c-ce-c.xml 
<?xml version="1.0" encoding="UTF-8"?>
<?xml-model href="https://github.com/usnistgov/OSCAL/raw/v1.0.4/xml/schema/oscal_complete_schema.xsd" schematypens="http://www.w3.org/2001/XMLSchema" title="OSCAL complete schema" ?>
<catalog xmlns="http://csrc.nist.gov/ns/oscal/1.0" uuid="37b7acd4-ab3f-4217-99d1-2ab9e490db49">
    <metadata>
        <title />
        <last-modified>2023-02-18T13:06:18Z</last-modified>
        <version>2023-02-18T13:06:18Z</version>
        <oscal-version>1.0.4</oscal-version>
    </metadata>
    <control id="c1">
        <title />
        <control id="c1.1">
            <title />
            <control id="c1.1.1">
                <title />
            </control>
        </control>
    </control>
</catalog>
gapinski@flexion-mac-C02FCBVSMD6N resolver-pipeline % xslt -xsl:https://raw.githubusercontent.com/galtm/OSCAL/saxon11/src/utils/util/resolver-pipeline/oscal-profile-RESOLVE.xsl -s:c-ce-p.xml
<?xml version="1.0" encoding="UTF-8"?>
<catalog xmlns="http://csrc.nist.gov/ns/oscal/1.0"
          uuid="00000000-0000-4000-B000-000000000000">
   <metadata>
      <title/>
      <last-modified>2023-03-03T20:00:21.864183-05:00</last-modified>
      <version>2023-02-18T13:06:18Z</version>
      <oscal-version>1.0.4</oscal-version>
      <prop name="resolution-tool"
             value="OSCAL Profile Resolver XSLT Pipeline OPRXP"/>
      <link href="file:/Users/gapinski/Projects/github/usnistgov/OSCAL/src/utils/util/resolver-pipeline/c-ce-p.xml"
             rel="source-profile"/>
   </metadata>
   <control id="c1">
      <title/>
   </control>
   <control id="c1.1">
      <title/>
   </control>
   <control id="c1.1.1">
      <title/>
   </control>
</catalog>

with-child-controls="no" appears to work correctly when profile resolution occurs (but not with https://github.com/usnistgov/OSCAL/tree/main/src/utils/util/resolver-pipeline — see usnistgov/liboscal-java#232).

gapinski@flexion-mac-C02FCBVSMD6N resolver-pipeline % cat c-ce-p.xml 
<?xml version="1.0" encoding="UTF-8"?>
<?xml-model href="https://github.com/usnistgov/OSCAL/raw/v1.0.4/xml/schema/oscal_complete_schema.xsd" schematypens="http://www.w3.org/2001/XMLSchema" title="OSCAL complete schema" ?>
<profile xmlns="http://csrc.nist.gov/ns/oscal/1.0" uuid="3353e8e1-3eef-42ef-8163-1f71616bafca">
    <metadata>
        <title />
        <last-modified>2023-02-18T13:06:18Z</last-modified>
        <version>2023-02-18T13:06:18Z</version>
        <oscal-version>1.0.4</oscal-version>
    </metadata>
    <import href="c-ce-c.xml">
        <include-controls with-child-controls="no">
            <with-id>c1.1</with-id>
        </include-controls>
    </import>
</profile>
gapinski@flexion-mac-C02FCBVSMD6N resolver-pipeline % xslt -xsl:https://raw.githubusercontent.com/galtm/OSCAL/saxon11/src/utils/util/resolver-pipeline/oscal-profile-RESOLVE.xsl -s:c-ce-p.xml
<?xml version="1.0" encoding="UTF-8"?>
<catalog xmlns="http://csrc.nist.gov/ns/oscal/1.0"
          uuid="00000000-0000-4000-B000-000000000000">
   <metadata>
      <title/>
      <last-modified>2023-03-03T20:14:50.389889-05:00</last-modified>
      <version>2023-02-18T13:06:18Z</version>
      <oscal-version>1.0.4</oscal-version>
      <prop name="resolution-tool"
             value="OSCAL Profile Resolver XSLT Pipeline OPRXP"/>
      <link href="file:/Users/gapinski/Projects/github/usnistgov/OSCAL/src/utils/util/resolver-pipeline/c-ce-p.xml"
             rel="source-profile"/>
   </metadata>
   <control id="c1">
      <title/>
   </control>
   <control id="c1.1">
      <title/>
   </control>
</catalog>
gapinski@flexion-mac-C02FCBVSMD6N resolver-pipeline % 

[wendellpiez]

@GaryGapinski thanks for clarification: it is terrible what the brain does to prevent the eyes from seeing.

Agreed, with-parent-controls is very soft, if it ever really existed. This has to be aligned all the way up to the spec, including unit tests. The need for these going forward is growing more acute, as there is more than one profile resolver that must behave correctly.

@galtm @aj-stein-nist let's take note that the scope of definition here (semantics to be settled, documented, mocked up and tested) includes both with-parent-controls and with-child-controls in the different "merge" modes.

[wendellpiez]

Now retracing Gary's steps, I am able to confirm some kind of regression in the schema. The flag with-parent-controls is indeed invalid where the spec says it should be used.

Suggest we look at the commit history of the profile metaschema to determine if this flag was not removed at some point - if only because my test harnesses include data that is now invalid -- suggesting they were valid at some point.

If we can confirm when this flag was pulled from the schema we are in a better position to judge whether the schema is in error (this should be replaced) or maybe the change was made purposefully (and the spec should catch up).

If there is no flag for toggling this behavior we also need to determine whether controls can be included by implication (by virtue of a descendant control being included) at any rate in flat and as-is merge modes.

[aj-stein-nist]

OK this appears to have been easier to figure out than I had expected. It appears that work was done to add with-parent-controls to the specification, as was with-child-controls, but it was never added to the metaschemas at all. It was not removed, it was presumed there and never included (unless previously it was added to a branch that was not removed into develop or main) in a long-standing feature branch or on a fork for a NIST staff developer, I do not have enough time to find it.

To conduct the search I did the following on both develop and main and every mention of with-parent-controls is in the following commits, they only touch the specification.

$ git branch --show-current
develop
$ git rev-parse HEAD
e3c4432e74eae1fabaaa6ecf9d29df0915a887f2
$  git log -S with-parent-controls --oneline
539fd730a Requirement Tagging - Profile Resolution Specification (#1089)
8c662395c Publishing Generated Website Content [ci skip]
2c4354709 Profile specification (#1017)
$ git branch --show-current
main
$ git rev-parse HEAD
0dfb7123488576b016ca58806033bfc9b6be8e0b
$ git log -S with-parent-controls --oneline
539fd730a Requirement Tagging - Profile Resolution Specification (#1089)
8c662395c Publishing Generated Website Content [ci skip]
2c4354709 Profile specification (#1017)

If we can confirm when this flag was pulled from the schema we are in a better position to judge whether the schema is in error (this should be replaced) or maybe the change was made purposefully (and the spec should catch up).

So you are correct, @wendellpiez, good hunch, can you draft a PR (we will discuss where to point the branch in a little bit, rebasing will be simple with a small change like this, and if I am wrong I will help you): please add with-parent-controls?

If there is no flag for toggling this behavior we also need to determine whether controls can be included by implication (by virtue of a descendant control being included) at any rate in flat and as-is merge modes.

I will review the profile resolution spec again later today, but I had trouble understanding the current perspective on this for with-child-controls and with-parent-controls and I read it a few times. So maybe you and I can meet tomorrow to discuss it?

[aj-stein-nist]

I also checked release-1.0 branch to see if we are in the same boat there, we are.

@wendellpiez, looks like we have an immediate game plan to add with-parent-controls back. I update the order of checklist items in #1662 (comment) to reflect this. Can we sync up on the specification later today or tomorrow?

[wendellpiez]

Yup, sounds good. I will make this change over develop and if you want it somewhere else, you can let me know.

There may be some crafting of language for description.

[aj-stein-nist]

@wendellpiez, can you write up the challenges with the schema modifications in a comment to this issue before the sprint review and we discuss further then? I would like to address this work, but I assume we should have the team brief one another on blockers and things. I presume there will not be a lot of movement in the next hour.

[wendellpiez]

Quick version:

• Confirmed that the metaschema is out of alignment with the Profile Resolution spec -
  • spec mentions with-parent-controls in context of include-controls (only)
  • the Profile metaschema is missing this flag, as reported (a different flag with-child-controls is okay), hence it is not permitted by either XSD or JSON Schema
• However, the simple solution (adding an optional flag to include-controls) is not viable:
  • the same model is currently used also for exclude-controls
  • meaning refactoring is called for unless we define (undefined) semantics for exclude-controls/@with-parent-controls
  • additionally, both these models are also reused inside control regrouping (optional feature in merge phase) making more analysis/testing/due diligence necessary (the flag makes no sense there either)

In summary:

• Solution is still to add the flag where we need it in the Profile metaschema (just it's a bit complicated)
• I have some notion of how to do this
• Yet this is also an opportunity to skill up on Metaschema dev and testing / share the knowhow
• It also needs to be tested fully through to schema generation / testing and many hands there is also good

Finally - working this could also be easier with unit tests such as are behind the draft PR here: #1479

I think that covers it for now --

[aj-stein-nist]

For Sprint 65, @wendellpiez is the driver for this issue.

[aj-stein-nist]

@nikitawootten-nist you want to pair on this later today?

[aj-stein-nist]

@wendellpiez one of the main concerns I have with the comment regard alignment with the profile resolution specification is what is written:

"Exclusions work the same way as inclusions, except with the opposite effect - the indicated control(s) do not appear in the target catalog."

So I think we are going to have to, in the short-term, make the semantics of include and exclude align, and with-parent-controls reflect that. I know that seems odd, but I am not sure how to get out of that for inclusion and exclusion of controls.

I still think your point about select-control-by-id assembly in the context of merge is important, but that is part of the custom directive? I am looking at #1479, but can we talk more about that Monday or early next week? Perhaps we separate that out then as the key dichotomy line, but I need to read through your tests to understand.

Does that sound good?

[wendellpiez]

Current thinking:

• In a working branch, I proceed to work up a provisional solution to add an optional flag - but aiming primarily to 'see the bottom' not for a solution per se
• @nikitawootten-nist and @aj-stein-nist separately or together set up to do the same thing: some kind of metaschema-to-schema production they can run to produce schemas - same as CI/CD but externally.

I will do my work in oXygen (both editing and production) but you can pick whatever you like, as long as you can validate metaschema instances and produce schemas from your new or edited metaschema. (Validation does include Schematron.) My work should include at least some test data including valid and known-invalid samples.

After doing this (ask for help?) we convene, compare notes, and start to talk about the remodeling, with potential solutions.

Bonus: if we could generate schemas using oscal-cli we could test that as well.

[aj-stein-nist]

Current thinking:

* In a working branch, I proceed to work up a provisional solution to add an optional flag - but aiming primarily to 'see the bottom' not for a solution per se

* @nikitawootten-nist and @aj-stein-nist separately or together set up to do the same thing: some kind of metaschema-to-schema production they can run to produce schemas - same as CI/CD but externally.

I will do my work in oXygen (both editing and production) but you can pick whatever you like, as long as you can validate metaschema instances and produce schemas from your new or edited metaschema. (Validation does include Schematron.) My work should include at least some test data including valid and known-invalid samples.

After doing this (ask for help?) we convene, compare notes, and start to talk about the remodeling, with potential solutions.

Bonus: if we could generate schemas using oscal-cli we could test that as well.

Sounds good, where is the working branch?

[wendellpiez]

It is here: https://github.com/wendellpiez/OSCAL/tree/issue1662-adding-with-parent-controls-flag

But the only thing in it so far is a working copy of the profile metaschema (not yet being worked) -- rsn.

[wendellpiez]

New demo profile metaschema: https://github.com/wendellpiez/OSCAL/blob/issue1662-adding-with-parent-controls-flag/src/metaschema/oscal_profile1662_metaschema.xml

This is the same as the profile metaschema (sitting next to it for comparison) except with a little rearranging of definitions to achieve the exact mod wanted, namely to add a flag with-parent-controls in one (but not all) places where control inclusion (or exclusion) may also designate with-child-controls.

It also includes three XML test instances for testing validation in a generated schema (XSD).

Next up for @aj-stein-nist and @nikitawootten-nist -- either bring this stuff down and check it out, or let me know where you are stuck:

• making sense of what I put in this branch
• generating XSD for a metaschema
• validating instances against a generated schema (or any schema of interest)
• starting to examine my mods to trace the rewiring

We can also work on this together if it's useful. Let me know if this isn't the best way to support distribution of temporary WIP - I am assuming commit histories can be squeezed.

[wendellpiez]

@aj-stein-nist this refactoring attempts to thread the needle in the spec by interpolating into the Profile Resolution spec the idea that the semantics of exclusion are the same insofar as they are applicable, which includes not only that they have the opposite effect, but also that not all the capabilities are there (such as being able to exclude an entire branch of a control hierarchy by listing one of its descendants) because not all the features are there (no with-parent-controls flag).

This reflects the original intent and I do not believe it contradicts the spec, which doesn't actually say that exclusions are identical to inclusion every way except excluding. (It says they 'work the same'.)

If we must edit the Spec to clarify this, I would prefer that than to have a leaky semantic asking for trouble, unit tests and agreement somewhere down the line.

[aj-stein-nist]

So yesterday, @nikitawootten-nist, @wendellpiez, and I did a deep dive and Wendell broke down specific points on the the implications of a custom merge directive and how including with-parent-controls on include and exclude is a bad idea. After much discussion of detail and the implications of spec alignment based on the ambiguous use of the phrase "works the same," I have flipped my position again. I have come to see the light.

Yesterday, I committed to pair with Nikita to interpret what Wendell said and the approach he presented. I will try with Nikita to achieve the same thing with specifically copy his branch to check my understanding when we pair today or tomorrow, more to come.

Then we will need to update and align the spec.