Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Profile resolver not applying alterations to parameters #1859

Open
Rene2mt opened this issue Jul 20, 2023 · 6 comments · Fixed by #1596
Open

Profile resolver not applying alterations to parameters #1859

Rene2mt opened this issue Jul 20, 2023 · 6 comments · Fixed by #1596
Labels

Comments

@Rene2mt
Copy link
Contributor

Rene2mt commented Jul 20, 2023

Describe the bug

When using the XSLT profile resolver (v1.0.4), the produced resolved profile catalogs does not apply the changes to the param specified in the profile.

Who is the bug affecting

FedRAMP and anyone using XSLT profile resolver v1.0.4

What is affected by this bug

Tooling & API

How do we replicate this issue

NOTE - the issue is when using XSLT profile resolver (version 1.0.4)

  1. Use a catalog with parameters (e.g., FedRAMP's rev resolved profile catalog)
  2. Create a profile that tries to add a prop to parameters as follows:
  3. Conduct profile resolution (e.g. through a tool like Oxygen XML or OSCAL-CLI)

Expected behavior (i.e. solution)

Notice that it works when the prop elements are added via <set-parameter param-id="parameter-ids">...</set-parameter> . This is the approach in gist https://gist.github.com/Rene2mt/de339c61034da0b81860ab5c13bc702a#file-sample-profile-xml-L54-L1868.

BUT it does not work when trying to use <alter control-id="control-id"><add position="starting" by-id="param-id">...</add></alter>. This is the approach in gist https://gist.github.com/Rene2mt/de339c61034da0b81860ab5c13bc702a#file-sample-profile-xml-L1872-L4256

This should work according to the profile resolution spec.

Other comments

No response

Revisions

No response

@Rene2mt Rene2mt added the bug label Jul 20, 2023
@github-actions github-actions bot added this to Needs Triage in Issue Triage Jul 20, 2023
@wendellpiez
Copy link
Contributor

wendellpiez commented Aug 30, 2023

Let's see to it that the alter usage is unit tested against this.

The lapse (or bug if you like) is in XSLT https://github.com/usnistgov/OSCAL/blob/develop/src/utils/resolver-pipeline/oscal-profile-resolve-modify.xsl, where a template matching param elements (

<xsl:template match="param" priority="2">
) fails to do what other control descendants do (template matching control//* (
<xsl:template match="control//*">
), namely looking for its matching alterations and acting on them. (It does what is called for from set-param which is why that is a workaround.)

Solution is to see to it that the param template does what is called for, not only wrt set-param but also alter (and unit test).

@wendellpiez
Copy link
Contributor

wendellpiez commented Aug 30, 2023

BTW if (or to the extent that) this gives rise to implementation questions such as what happens when compounding settings and alterations on a single parameter ... another reason to build unit tests ... and if this is tricky, maybe forbidding addressing parameters with alter is preferable.

galtm added a commit to galtm/OSCAL that referenced this issue Sep 14, 2023
The XSLT changes in this pull request should already fix usnistgov#1859.
This commit adds a relevant test scenario as evidence.
@galtm
Copy link
Contributor

galtm commented Sep 14, 2023

I believe #1549 fixes this issue, and I added a relevant test scenario in that pull request.

@wendellpiez
Copy link
Contributor

Fantastic! thanks!

@aj-stein-nist aj-stein-nist removed this from Needs Triage in Issue Triage Sep 20, 2023
@aj-stein-nist
Copy link
Contributor

I believe #1549 fixes this issue, and I added a relevant test scenario in that pull request.

Will mark this as blocked until we merge this into develop and #1549 is merged into the develop branch and then a release.

@aj-stein-nist aj-stein-nist linked a pull request Sep 28, 2023 that will close this issue
7 tasks
aj-stein-nist pushed a commit to galtm/OSCAL that referenced this issue Sep 28, 2023
The XSLT changes in this pull request should already fix usnistgov#1859.
This commit adds a relevant test scenario as evidence.
aj-stein-nist pushed a commit that referenced this issue Sep 28, 2023
The XSLT changes in this pull request should already fix #1859.
This commit adds a relevant test scenario as evidence.
aj-stein-nist pushed a commit that referenced this issue Sep 28, 2023
The XSLT changes in this pull request should already fix #1859.
This commit adds a relevant test scenario as evidence.
@aj-stein-nist aj-stein-nist linked a pull request Sep 28, 2023 that will close this issue
9 tasks
@iMichaela
Copy link
Contributor

This issue is probably safe to close after a sanity check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Blocked
5 participants