-
Notifications
You must be signed in to change notification settings - Fork 42
Minimalist Selection Object #821
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
Conversation
also make CVSS decision point groups conform
namespace:key:version is sufficient to uniquely identify any Decision Point
…om the other key-value pairs
…b.com/CERTCC/SSVC into 820-add-a-minimalist-selection-object # Conflicts: # data/schema/v2/Decision_Point_Value_Selection-2-0-0.schema.json # src/ssvc/selection.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a new minimal SSVC selection model, aligns version strings to semantic format across CVSS groups, and adds a v2 JSON schema for selections.
- Adds
MinimalSelectionandMinimalSelectionListPydantic models with helper functions insrc/ssvc/selection.py. - Standardizes all CVSS decision point group
versionfields to semanticX.Y.Zincollections.py. - Defines and references a new v2 JSON schema (
Decision_Point_Value_Selection-2-0-0.schema.json) and updates the current schema pointer.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/decision_points/test_cvss_helpers.py | Updated test data to use 1.0.0 instead of 1.0. |
| src/ssvc/selection.py | New module defining minimal selection classes and schema generation. |
| src/ssvc/dp_groups/cvss/collections.py | Bumped all CVSS group versions to semantic X.Y.Z. |
| src/ssvc/_mixins.py | Introduced VersionField type for enforcing semantic version patterns. |
| data/schema/v2/Decision_Point_Value_Selection-2-0-0.schema.json | Added v2 schema for selection objects with inline definitions. |
| data/schema/current/Decision_Point_Value_Selection.schema.json | Updated current schema pointer to v2. |
Comments suppressed due to low confidence (3)
src/ssvc/selection.py:90
- The new
add_selectionmethod is not covered by any tests; consider adding unit tests to verify selections are appended correctly.
def add_selection(self, selection: MinimalSelection) -> None:
src/ssvc/selection.py:100
- The
selection_from_decision_pointconversion function lacks direct test coverage; adding tests will help ensure correct mapping fromDecisionPointtoMinimalSelection.
def selection_from_decision_point(decision_point: DecisionPoint) -> MinimalSelection:
data/schema/v2/Decision_Point_Value_Selection-2-0-0.schema.json:131
- The schema does not include
additionalProperties: false, allowing arbitrary fields; consider adding it at the root to enforce strict validation.
"type": "object",
data/schema/v2/Decision_Point_Value_Selection-2-0-0.schema.json
Outdated
Show resolved
Hide resolved
|
@tschmidtb51 requesting your feedback on the new schema. |
…minimalist-selection-object
tschmidtb51
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Careful - unfinished review]
All elements should have a minLength (for Strings) etc. attribute.
I'll finish my review tomorrow with a fresh set of eyes.
| "title": "Version", | ||
| "type": "string" | ||
| }, | ||
| "values": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @sei-vsarvepalli that it is for humans nice to have a name value.
|
Still reviewing |
tschmidtb51
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my review, I came to the conclusion that it is best to use reverse FQDNs to differentiate between "language" and "xname" extensions. The latest and greatest is the new suggested ABNF - all other should follow from their or be made consistent to that...
| # --- Base namespace --- | ||
| NO_CONSECUTIVE_SEP = r"(?!.*[.-]{2,})" # no consecutive '.' or '-' | ||
|
|
||
| BASE_PATTERN = ( | ||
| rf"{NO_CONSECUTIVE_SEP}" | ||
| r"[a-z][a-z0-9]+" # starts with lowercase letter + 1+ alnum | ||
| r"(?:[.-][a-z0-9]+)*" # optional dot or dash + alnum | ||
| ) | ||
|
|
||
| BASE_NS_PATTERN = rf"(?:x_{BASE_PATTERN}|{BASE_PATTERN})" | ||
|
|
||
| # --- Extension segments --- | ||
| # A single ext-seg with at most one '#' | ||
| EXT_SEGMENT_PATTERN = ( | ||
| rf"{NO_CONSECUTIVE_SEP}" | ||
| r"[a-zA-Z][a-zA-Z0-9]*" # start with a letter | ||
| r"(?:[.-][a-zA-Z0-9]+)*" # dot or dash + alnum | ||
| r"(?:#[a-zA-Z0-9]+(?:[.-][a-zA-Z0-9]+)*)?" # optional single hash section | ||
| ) | ||
|
|
||
| # Subsequent ext-seg(s) | ||
| SUBSEQUENT_EXT = rf"{EXT_SEGMENT_PATTERN}(?:/{EXT_SEGMENT_PATTERN})*" | ||
|
|
||
|
|
||
| # --- Language extension --- | ||
| LANG_EXT = rf"(?:/{BCP_47_PATTERN}/|//)" | ||
|
|
||
| # --- Combine all parts into the full namespace pattern --- | ||
| NS_PATTERN_STR = ( | ||
| rf"^{LENGTH_CHECK_PATTERN}" | ||
| rf"{BASE_NS_PATTERN}" | ||
| rf"(?:{LANG_EXT}{SUBSEQUENT_EXT})?$" | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see comment on ABNF
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly covered in the JSON file - but the interface is hung up - I can't comment anything here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might want to add some cases after changing the regex and ABNF.
Co-authored-by: tschmidtb51 <[email protected]>
Co-authored-by: tschmidtb51 <[email protected]>
Co-authored-by: tschmidtb51 <[email protected]>
Co-authored-by: tschmidtb51 <[email protected]>
Co-authored-by: tschmidtb51 <[email protected]>
Co-authored-by: tschmidtb51 <[email protected]>
Co-authored-by: tschmidtb51 <[email protected]>
Co-authored-by: tschmidtb51 <[email protected]>
Co-authored-by: tschmidtb51 <[email protected]>
Co-authored-by: tschmidtb51 <[email protected]>
Co-authored-by: tschmidtb51 <[email protected]>
Co-authored-by: tschmidtb51 <[email protected]>
|
The ABNF and Regex commentary has gotten too complicated for me to track in the context of this PR, and absorbing all of it will require more attention than I can give it at the moment. I've merged the PR at its current state. Please consolidate the feedback into a concise issue focused on the regex constructor in As it stands, I currently regret bringing ABNF into it, as it's not something we can easily manage and generate, and am more inclined to remove it from the documentation than to revise it. |
ssvcnamespaces #753Decision_Point_Value_Selection.schema.jsonusing Pydantic #735Decision_Point_Value_Selection-1-0-1.schema.json#766Selection Object
This PR adds a
ssvc.selectionmodule which contains a few pydantic dataclasses to represent a minimal version of an SSVC selection object. TheMinimalSelectionListclass is intended to serve as the source for a new JSON schema that replacesDecision_Point_Value_Selection_1-0-1.schema.json.Our goal is to provide a terse object for data exchange purposes. To that end, the object does not represent fully-formed Decision Point objects, only the necessary fields to uniquely identify a specific decision point and its selected values. (A decision point is uniquely identified by its namespace, key, and version. Furthermore, within a given decision points, each value must have a unique value key. So the selection object only provides that level of detail. It's expected that in actual use, the receiver will dereference this selection object against a library of known decision point definitions.
Namespace Improvements
This PR also incorporates namespace improvements from #824
CoPilot Summary
This pull request introduces significant updates to the SSVC project, including the adoption of namespaces for SSVC objects, enhancements to schema definitions, and improvements to the codebase for better maintainability and functionality. The changes focus on enabling extensibility, improving schema validation, and refining documentation generation.
Namespace Implementation and Documentation:
docs/adr/0012-ssvc-namespaces.md, docs/adr/index.mdR26)._Namespacedmixin to useNamespaceStringfor stricter validation of namespace patterns (src/ssvc/_mixins.py, src/ssvc/_mixins.pyL73-R77).Schema Enhancements:
data/schema/current/Decision_Point_Value_Selection.schema.json, data/schema/current/Decision_Point_Value_Selection.schema.jsonL1-R1).Decision_Point_Value_Selectionversion 2.0.0, including new fields likeresources,references, and improved validation (data/schema/v2/Decision_Point_Value_Selection-2-0-0.schema.json, data/schema/v2/Decision_Point_Value_Selection-2-0-0.schema.jsonR1-R226).Codebase Improvements:
_Timestampedmixin to automatically set timestamps for SSVC objects and updated_Versionedto useVersionStringfor semantic version validation (src/ssvc/_mixins.py, [1] [2].idproperty forDecisionPointto provide a unified identity string (src/ssvc/decision_points/base.py, src/ssvc/decision_points/base.pyR190-R197).src/ssvc/doc_helpers.py, [1] [2].Documentation and Tools:
SelectionListschema to a file for easier integration and validation (src/ssvc/doctools.py, [1] [2].docs/adr/index.md, docs/adr/index.mdR26).These changes collectively improve the extensibility, validation, and usability of the SSVC framework while providing clear documentation and tools for developers and users.
Co-Pilot Schema Comparison
Based on review of the two schemas, here are the salient changes between version 1.0.1 and 2.0.0:
Schema Structure Changes
titlefield: "Decision Point Value Selection List"const: "2.0.0"additionalProperties: falseat the root levelProperty Changes
Removed properties:
id(vulnerability identifier) - replaced bytarget_idsrole(stakeholder role)Added properties:
target_ids: Optional array of identifiers for items being evaluated (vulnerabilities, reports, etc.)resources: Array of references providing context about decision pointsreferences: Array of references providing context about selected valuesModified properties:
timestamp: Simplified to direct string format instead of referencing external definitionselections: Now uses internalSelectiondefinition instead of external referenceSelection Object Changes
SsvcdecisionpointselectionSchemato simplifiedSelectionobjectMinimalDecisionPointValueobjects withkey,name, anddescriptionfieldsNew Schema Definitions
key,name, anddescriptionRequired Fields Changes
selections,id,timestamp,schemaVersiontimestamp,schemaVersion,selections(removedidrequirement)