Skip to content

fix: correct XPath descendant axis (//) in catalog assessment-method constraints (#1951)#2212

Open
nancysangani wants to merge 13 commits intousnistgov:developfrom
nancysangani:fix/xpath-typo-catalog-assessment-method
Open

fix: correct XPath descendant axis (//) in catalog assessment-method constraints (#1951)#2212
nancysangani wants to merge 13 commits intousnistgov:developfrom
nancysangani:fix/xpath-typo-catalog-assessment-method

Conversation

@nancysangani
Copy link
Copy Markdown
Contributor

Fixes #1951 — Correct XPath descendant axis (//) in catalog assessment-method constraints

Three constraint target expressions in oscal_catalog_metaschema.xml used / (child axis) instead of // (descendant axis), causing constraints to silently skip nested part and prop elements under assessment-method parts. An additional fourth instance was identified and fixed as well.

File changed: src/metaschema/oscal_catalog_metaschema.xml

Before After
assessment-method')]/part[ assessment-method')]//part[
assessment-method')]/prop[ assessment-method')]//prop[ (3 instances)

nancysangani and others added 10 commits March 15, 2026 23:34
Bumps [actions/checkout](https://github.com/actions/checkout) from 6.0.1 to 6.0.2.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@8e8c483...de0fac2)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-version: 6.0.2
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [actions/setup-node](https://github.com/actions/setup-node) from 6.2.0 to 6.3.0.
- [Release notes](https://github.com/actions/setup-node/releases)
- [Commits](actions/setup-node@6044e13...53b8394)

---
updated-dependencies:
- dependency-name: actions/setup-node
  dependency-version: 6.3.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps com.xmlcalabash:xmlcalabash from 3.0.31 to 3.0.42.

---
updated-dependencies:
- dependency-name: com.xmlcalabash:xmlcalabash
  dependency-version: 3.0.42
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [org.apache.maven.plugins:maven-dependency-plugin](https://github.com/apache/maven-dependency-plugin) from 3.9.0 to 3.10.0.
- [Release notes](https://github.com/apache/maven-dependency-plugin/releases)
- [Commits](apache/maven-dependency-plugin@maven-dependency-plugin-3.9.0...maven-dependency-plugin-3.10.0)

---
updated-dependencies:
- dependency-name: org.apache.maven.plugins:maven-dependency-plugin
  dependency-version: 3.10.0
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
…tion in system-implementation andimplementing abandoned PR 2107 which proposed fixing the validation constraints to properly deference and key on uri-reference values.
@nancysangani nancysangani requested a review from a team as a code owner March 22, 2026 13:10
Copilot AI review requested due to automatic review settings March 22, 2026 13:10
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes catalog metaschema constraint targeting so assessment-method-related constraints apply to nested part/prop descendants (not just direct children), aligning behavior with intended validation for assessment-method structures.

Changes:

  • Update four XPath target expressions in src/metaschema/oscal_catalog_metaschema.xml to use the descendant axis (//) under assessment-method parts.
  • Add an npm overrides entry in build/package.json to force fast-json-patch to >=3.1.1.
  • Update build/package-lock.json to resolve fast-json-patch to 3.1.1.

Reviewed changes

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

File Description
src/metaschema/oscal_catalog_metaschema.xml Corrects XPath targeting so constraints validate nested part/prop under assessment-method.
build/package.json Introduces npm override to force a newer fast-json-patch.
build/package-lock.json Locks fast-json-patch to 3.1.1 and adjusts dependency metadata accordingly.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@nancysangani
Copy link
Copy Markdown
Contributor Author

/cc @iMichaela

@nancysangani nancysangani changed the base branch from main to develop March 22, 2026 13:45
@iMichaela
Copy link
Copy Markdown
Contributor

@nancysangani - Please note, your branch needs to be rebased. Do you want me to push it or you will do it?

Copilot AI review requested due to automatic review settings March 23, 2026 14:59
@nancysangani nancysangani force-pushed the fix/xpath-typo-catalog-assessment-method branch from bbbf511 to 274108a Compare March 23, 2026 14:59
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

<enum value="method">The assessment method to use. This typically appears on
parts with the name "assessment-method".</enum>
</allowed-values>
<expect level="WARNING" id="oscal-method-part-has-method-prop" target="part[has-oscal-namespace('http://csrc.nist.gov/ns/oscal') and @name=('assessment','assessment-method')]" test="prop[has-oscal-namespace(('http://csrc.nist.gov/ns/oscal','http://csrc.nist.gov/ns/rmf')) and @name='method']"/>
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The allowed-values targets were updated to use the descendant axis (//prop[...]) under assessment-method, but the expect constraint oscal-method-part-has-method-prop still checks only a direct child prop[...]. If the method prop can occur under nested parts (the scenario this PR is addressing), this expect will raise false warnings. Consider updating the test XPath to also use a descendant search (e.g., .//prop[...]) so it aligns with the updated constraint targets.

Suggested change
<expect level="WARNING" id="oscal-method-part-has-method-prop" target="part[has-oscal-namespace('http://csrc.nist.gov/ns/oscal') and @name=('assessment','assessment-method')]" test="prop[has-oscal-namespace(('http://csrc.nist.gov/ns/oscal','http://csrc.nist.gov/ns/rmf')) and @name='method']"/>
<expect level="WARNING" id="oscal-method-part-has-method-prop" target="part[has-oscal-namespace('http://csrc.nist.gov/ns/oscal') and @name=('assessment','assessment-method')]" test=".//prop[has-oscal-namespace(('http://csrc.nist.gov/ns/oscal','http://csrc.nist.gov/ns/rmf')) and @name='method']"/>

Copilot uses AI. Check for mistakes.
@nancysangani nancysangani force-pushed the fix/xpath-typo-catalog-assessment-method branch from 274108a to 5f76eb3 Compare March 23, 2026 15:03
@nancysangani
Copy link
Copy Markdown
Contributor Author

@iMichaela Rebased onto latest develop and pushed.

Copy link
Copy Markdown
Contributor

@iMichaela iMichaela left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR aims to revert the recent update of fast-json-patch to v3.1.1 to eliminate critical vulnerability. Is there an explanation for the suggested change?

@iMichaela
Copy link
Copy Markdown
Contributor

@nancysangani - The control[...]/part[... name="assessment-method" ..]/prop[..name="method"... value="INTERVIEW | EXAMINE | TEST" ] allowed-valueconstraint should not be enforced on all childrenpropslikecontrol[...]/part[... name="assessment-method" ..]/part[..name="assessment-objective"..]/[prop[...].

I will also review the issue, to clarify it.

image

Copilot AI review requested due to automatic review settings March 24, 2026 14:38
@nancysangani
Copy link
Copy Markdown
Contributor Author

nancysangani commented Mar 24, 2026

This PR aims to revert the recent update of fast-json-patch to v3.1.1 to eliminate critical vulnerability. Is there an explanation for the suggested change?

Hi @iMichaela, apologies for the confusion! This PR is only intended to fix the XPath descendant axis bug reported in #1951. The build/package.json and build/package-lock.json changes were accidentally included from another unrelated branch. I have now cleaned up the branch — only src/metaschema/oscal_catalog_metaschema.xml is changed.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@nancysangani
Copy link
Copy Markdown
Contributor Author

@nancysangani - The control[...]/part[... name="assessment-method" ..]/prop[..name="method"... value="INTERVIEW | EXAMINE | TEST" ] allowed-valueconstraint should not be enforced on all childrenpropslikecontrol[...]/part[... name="assessment-method" ..]/part[..name="assessment-objective"..]/[prop[...].

I will also review the issue, to clarify it.

image

Thank you for the clarification! I understand the concern — using // may be too broad for the method prop constraint as it would incorrectly enforce INTERVIEW | EXAMINE | TEST values on props in nested parts like assessment-objective.

I will wait for your review of the issue before making further changes. Please let me know the correct intended behavior and I will update the PR accordingly.

@nancysangani nancysangani requested a review from iMichaela March 24, 2026 14:49
@iMichaela
Copy link
Copy Markdown
Contributor

This PR aims to revert the recent update of fast-json-patch to v3.1.1 to eliminate critical vulnerability. Is there an explanation for the suggested change?

Hi @iMichaela, apologies for the confusion! This PR is only intended to fix the XPath descendant axis bug reported in #1951. The build/package.json and build/package-lock.json changes were accidentally included from another unrelated branch. I have now cleaned up the branch — only src/metaschema/oscal_catalog_metaschema.xml is changed.

Thank you, Nancy. I will review #1951 asap and get back to you. The screenshot I inserted reveals some other issues with the declarations we currently have ( prop@name="method" is deprecated for oscal namespace but called under the next statement)

@iMichaela
Copy link
Copy Markdown
Contributor

@nancysangani - Thank you again for your contribution. This PR was not approved yet because we are still researching if the proposed updated to use the descendant axis nested for assessment-objects is useful or detrimental for the community.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

XPath bug (typo ?) in oscal_catalog_metaschema.xml

3 participants