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 Schema: "control-id" is currently optional #1053

Closed
stephenbanghart opened this issue Nov 15, 2021 · 4 comments · Fixed by #1111
Closed

Profile Schema: "control-id" is currently optional #1053

stephenbanghart opened this issue Nov 15, 2021 · 4 comments · Fixed by #1111
Assignees
Labels
bug Scope: Metaschema Issues targeted at the metaschema pipeline
Milestone

Comments

@stephenbanghart
Copy link
Contributor

"control-id" (https://pages.nist.gov/OSCAL/reference/latest/profile/xml-reference/#/profile/modify/alter/@control-id) is currently optional.

As the containing statement is inherently non-operational without this value, it should either be changed to required, or explained in the profile specification.

I'd like to hear from @butler54 as well as the NIST team (pinging @david-waltermire-nist @wendellpiez so they see this) on their thoughts between these two options.

@stephenbanghart stephenbanghart added bug Scope: Metaschema Issues targeted at the metaschema pipeline labels Nov 15, 2021
@wendellpiez
Copy link
Contributor

wendellpiez commented Nov 15, 2021

The only rationale that occurs to me for control-id being optional is in view of possible extension of the semantics of alter to address other elements (besides controls). However that is a hypothetical, arguably unlikely (given the purpose of profiles), and up to us in any case.

To preserve some wiggle room however and to avoid backward-incompatible thrashing in the XSD/JSON Schema, the requirement could be expressed as a cardinality constraint rather than as a part of the definition. (I.e. min-occurs=0 but add a constraint to require it). Depending on whether and how easily constraints can be layered (in or out), that offers more flexibility. (I am fine with either.)

The profile resolution spec IMO should state that the flag is expected and there is an error condition (either reportable or just GIGO -- at least, optionally reportable as a warning?) in profile resolution, if an alter does not resolve to a single target (irrespective of nominal validity issues in upstream).

This is similar to if I have clashing IDs and no map to resolve the clash (which can happen even if everything is schema-valid), except I am pointing to nothing instead of more than one thing. Error.

So my vote is to address this in the profile spec, irrespective of changes to schemas or new constraints.

@JustKuzya
Copy link
Contributor

JustKuzya commented Nov 16, 2021 via email

@wendellpiez
Copy link
Contributor

@JustKuzya,

That is correct, there is zero guarantee that a control with id ac-1 in a resolved profile would have any similarity to (any) ac-1 in an upstream catalog. alter can do just about anything while 'nominal control identity' implicitly remains with the ID value.

Indeed with the new mapping feature there is no guarantee that ac-1 was even identified as ac-1 and not something else.

Users of profiles should understand this! Additionally, it is an opportunity for a profile resolver to supplement resolution with warnings about modifications judged (by whatever set of rules) to be out of bounds. (For example, maybe you have a rule to say that no new ID given in a map can be the same as any old ID in the resolved source, to detect 'creativity' with this. Etc.) Like traceability, that is an option a resolver could offer over and above "bare" resolution.

HTH - interesting question - @stephenbanghart and @david-waltermire-nist may also have perspective.

@butler54
Copy link

butler54 commented Nov 17, 2021

So my thoughts - I think it should be required but I'm happy to leave this to v2. But my recommendation is the text in the documentation is updated to say that it is expected to error without it.

My thoughts on the scope of alter: There is so much flexibility already I see that as being unlikely to be exercised. The most likely functionality (IMO) that will be used is adding and removing either

  1. Props
  2. 'Guidance' like sections (e.g. corporate specific overlays). These are easily accommodated in the current functionality.

@david-waltermire david-waltermire self-assigned this Jan 13, 2022
@david-waltermire david-waltermire added this to the OSCAL 1.0.1 milestone Jan 13, 2022
aj-stein-nist added a commit that referenced this issue Jan 26, 2022
Resolves #1053.

Discussed with the team and agreed that this is in fact a bug that
requires a backward-breaking compatibility change to fix.
david-waltermire pushed a commit that referenced this issue Jan 26, 2022
Resolves #1053.

Discussed with the team and agreed that this is in fact a bug that
requires a backward-breaking compatibility change to fix.
iMichaela pushed a commit to iMichaela/OSCAL that referenced this issue Apr 7, 2022
…#1111)

Resolves usnistgov#1053.

Discussed with the team and agreed that this is in fact a bug that
requires a backward-breaking compatibility change to fix.
Rene2mt pushed a commit to Rene2mt/OSCAL that referenced this issue May 17, 2022
…#1111)

Resolves usnistgov#1053.

Discussed with the team and agreed that this is in fact a bug that
requires a backward-breaking compatibility change to fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Scope: Metaschema Issues targeted at the metaschema pipeline
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants