feat!: updates AssessmentStep to AssessmentMethod #83

jpower432 · 2025-08-08T22:32:50Z

What Changed?

AssessmentMethod was added
The cue schema for layer4 was updated to validate the updated structure generate using the library

⚠️ BREAKING
This removes the AssessmentStep type. Similar functionality can be acheived with the MethodExecutor on AssessmentMethod.

Note: Adapted from work on AssessmentMethods done by @trumant in #19

Rationale

Adding AssessmentMethod supports a multi-method Assessment that is flexible enough to accommodate different types of procedures to complete a control requirement. This new structure also captures detailed information about each individual check. The goal of AssessmentMethod is to be self-contained, allowing for composability and ensuring the solution retains the core functionality and structure of AssessmentSteps.

Open Questions

EDITED: Added questions from initial feedback

Is Assessment Method the most clear terminology to use? Perhaps Assessment Procedure?
If we want Assessment Methods to be seen as only as "options" do they need individual pre-condition steps?

Testing

A new file under layer4/testdata was added that is populated output from a Layer 4 structure generated by the TestEvaluate unit test. This was used to validate against the updated in the cue schema.

cue vet -d "#Layer4" schemas/layer-4.cue layer4/testdata/good-evaluation.yml

Issues

Closes #73
Overlaps/Impact #23

trumant

Quick review from my phone. Will take a deeper look on Monday morning

trumant · 2025-08-08T22:36:53Z

schemas/layer-4.cue

+	"corrupted-state": bool
+	// RemediationGuide is the URL to the documentation for this evaluation
+	"remediation-guide": string
+	// Assessments is a map of pointers to Assessment objects to establish idempotency


This looks to be a slice not a map

Good catch! Found that in a couple places actually. Updated on the last commit.

trumant · 2025-08-08T22:37:49Z

schemas/layer-4.cue

+	// Methods is a slice of assessment methods that were executed during the test
+	methods: [...#AssessmentMethod]
+	// MethodsExecuted is the number of assessment methods that were executed during the test
+	"methods-executed"?: int @go(MethodExecuted)


Singular/plural mismatch

trumant · 2025-08-08T22:39:22Z

schemas/layer-4.cue

-    "target-object"?: _
-}
+	// TargetName is the name or ID of the resource or configuration that is to be changed
+	"target-name": string @go(TagertName)


Suggested change

"target-name": string @go(TagertName)

"target-name": string @go(TargetName)

schemas/layer-4.cue

trumant · 2025-08-13T11:08:31Z

schemas/layer-4.cue

+	// Methods defines the assessment methods associated with the assessment
+	methods: [...#AssessmentMethod]
+	// MethodsExecuted is the number of assessment methods that were executed during the assessment
+	"methods-executed"?: int @go(MethodsExecuted)


Why is this data summarized here when it can be computed easily enough by the data consumer?

Thanks for the feedback. You're right - the steps-counter field made sense for a standalone function, but with the new run field in the AssessmentMethod struct, it's redundant.

Moves from AssessmentStep to AssessmentMethod to allow for metadata to be used to describe the check or test Signed-off-by: Jennifer Power <[email protected]>

The layer 4 schema is not up to date with the Go code implemented. This chanes updates the schema and uses evaluation data generated from L4 unit tests to verify againt the schema. Signed-off-by: Jennifer Power <[email protected]>

Signed-off-by: Jennifer Power <[email protected]>

Some of the comments around AssessementMethod require clarification around intended usage Signed-off-by: Jennifer Power <[email protected]>

To better show the impact of the change, the testdata was updated to an example from the pvtr baseline validator Signed-off-by: Jennifer Power <[email protected]>

This commit refactors the AssessmentMethod schema to use CUE's disjunction feature to formally define conditional logic for the run and result fields. The schema was allowing a state where run was true but a result was missing. Signed-off-by: Jennifer Power <[email protected]>

Co-authored-by: Travis Truman <[email protected]> Signed-off-by: Jennifer Power <[email protected]> Signed-off-by: Jennifer Power <[email protected]>

This change normalizes descriptions between different types defined in cue for layer4 and removes Go type language from the cue schema comments Signed-off-by: Jennifer Power <[email protected]>

This change refactors the work in the previous commits to allow for top-level metadata to support different types of assessment procedures and move the AssessmentSteps under AssessmentProcedure struct. The concept of "method" is updated to describe the procedure categorically for tools that run automated procedures. Signed-off-by: Jennifer Power <[email protected]>

jpower432 · 2025-08-19T21:28:25Z

Add a commment to describe the refactor in d431b80

What Changed?

AssessmentProcedure was added
AssessmentStep functionality is kept as is and moved under the Assessment Procedure

Rationale

Results from a maintainer discussion where we decided the assessment logic should keep steps as they currently are, but adding procedure to allow for each assessment to contain multiple groups of steps.

Points of Discussion

The concept of ProcedureMethod was added here to ensure the automated control evaluation skipped over any assessment procedures that should be completed by a human. I initially added the Test method and the Observation method to describe how the results of the procedure are to be determined.
The Assessment does not halt other procedures if a procedure fails. However, the procedure will halt if a step fails.

Open Questions

Message handling - If mulitple procedures run, how should should the Assessment message aggregate them?

trumant · 2025-08-21T03:12:41Z

schemas/layer-4.cue

-    "documentation-url"?: =~"^https?://[^\\s]+$"
-    "corrupted-state"?: bool
-    "assessment-results"?: [...#AssessmentResult]
+	// Name is the name of the control being evaluated


If we expect a tool will consume lower layers and produce output conforming with ControlEvaluation, should we consider a datetime here or elsewhere?

I agree a timestamp would be valuable. The run-duration field in Assessments is useful, but I'm wondering if it would make more sense to include a start and end datetime and let the consumer calculate the duration as needed. WDYT?

trumant · 2025-08-22T12:35:13Z

schemas/layer-4.cue

+	// RunDuration is the time it took to run the assessment
+	"run-duration"?: string @go(RunDuration)


Why do the typical data consumers care how long the assessment took? Or, put another way, what value does this data provide and to whome?

Agree. This was part of the attempt to align the schema with the current struct in the Go library. I would propose start and end timestamps instead.

I proposed something to address the run-duration field here - #112

trumant · 2025-08-22T12:35:21Z

schemas/layer-4.cue

+	// RunDuration is the time it took to run the assessment
+	"run-duration"?: string @go(RunDuration)
+	// Value is the object that was returned during the assessment
+	value?: _


What is this?

this one's a bit legacy — was used to log details about the inspected value

trumant · 2025-08-22T12:38:45Z

schemas/layer-4.cue

+	// Result communicates whether the assessment has been run, and if so, the outcome(s)
+	result: #Result


The "has been run" language here confuses me.

In what circumstances or use cases do we expect to have a document conforming to this schema that does not represent an Assessment activity that occurred in the past?

Is there value in having a layer4 payload that communicates an "intended" or "future" assessment?

I think there are two use cases I was trying to capture:

Currently, steps can have the state of NotRun if a prior Assessment Step has failed and the process is halted.

Part of my intent/goal with adding the Procedures was to allow Layer4 to be used in pre-run state (essentially like a test plan) as an input to a tool to execute the tests and fill in the result information.

trumant · 2025-08-22T12:39:08Z

schemas/layer-4.cue

+	// Result communicates whether the assessment has been run, and if so, the outcome(s)
+	result: #Result
+	// Message describes the result of the assessment
+	message: string


If it's describing the result, can we put it in #Result?

I proposed something here to update the existing Result field - #113

trumant · 2025-08-22T12:40:20Z

schemas/layer-4.cue

+// ProcedureMethod describes method options that can be used to determine the results
+#ProcedureMethod: "Test" | "Observation"


I think we need to clarify the language here. In my experience testing involves observation.

What value does this provide to the intended data consumers? How do you see them using this fact?

I agree, these could be more descriptive. I wanted to allow categorization of the prodecures based on if the results were determined with automation or human judgement of behavior or state.

Perhaps:

Suggested change

// ProcedureMethod describes method options that can be used to determine the results

#ProcedureMethod: "Test" | "Observation"

#EvaluationMethod: "Automated" | "Manual"

How this would be used might depend on if we want to support Layer4 as an input. This allows categorization of the procedures so that they might be handled differently to determine the results.

trumant · 2025-08-22T12:41:52Z

schemas/layer-4.cue

+	// Steps provides the address for the assessment steps executed
+	"steps"?: [...string]


How is a consumer of this data going to interpret, analyze or otherwise make use of this particular field?

trumant · 2025-08-22T12:42:49Z

schemas/layer-4.cue

+#Result: #ResultWhenRun | "Not Run"

+// Result describes the outcome(s) of an assessment procedure when it is executed.
+#ResultWhenRun: "Passed" | "Failed" | "Needs Review" | "Not Applicable" | "Unknown"


Contingent on the decision above on "has been run", but I'd propose removing #ResultWhenRun altogether

I'm thinking a potential solution here is to capture whether a step, assessment, or procedure ran in different field like run and keep the results field focused on assessment outcomes. Perhaps this would align with your comment about adding message to results. WDYT?

jpower432 · 2025-08-22T13:38:44Z

Looks like this branch has drifted from main. I will perform a rebase and get it back into a reviewable state.

jpower432 · 2025-09-18T21:12:04Z

Superseded by #117

jpower432 requested a review from a team as a code owner August 8, 2025 22:32

trumant reviewed Aug 8, 2025

View reviewed changes

trumant reviewed Aug 11, 2025

View reviewed changes

jpower432 requested a review from trumant August 12, 2025 15:58

trumant reviewed Aug 13, 2025

View reviewed changes

jpower432 marked this pull request as draft August 14, 2025 13:07

jpower432 force-pushed the break/assessment-method branch from 04bf5a4 to fc8a208 Compare August 19, 2025 16:34

jpower432 and others added 10 commits August 19, 2025 12:37

feat!: replace AssessmentStep with AssessmentMethod

8b9c8b4

Moves from AssessmentStep to AssessmentMethod to allow for metadata to be used to describe the check or test Signed-off-by: Jennifer Power <[email protected]>

chore: updates layer4 schema to reflect code changes

42864d1

The layer 4 schema is not up to date with the Go code implemented. This chanes updates the schema and uses evaluation data generated from L4 unit tests to verify againt the schema. Signed-off-by: Jennifer Power <[email protected]>

chore: add yaml struct tags to AssessmentMethod

815fef4

Signed-off-by: Jennifer Power <[email protected]>

fix: address inconsistencies in comments and type names

2b7beee

Signed-off-by: Jennifer Power <[email protected]>

fix: refines comments in layer4 Assessment types

affd076

Signed-off-by: Jennifer Power <[email protected]>

fix: refines comments and inconsistencies in layer4 schema

34334bc

Some of the comments around AssessementMethod require clarification around intended usage Signed-off-by: Jennifer Power <[email protected]>

chore: updates testdata to reflect a concrete example

f65f6ec

To better show the impact of the change, the testdata was updated to an example from the pvtr baseline validator Signed-off-by: Jennifer Power <[email protected]>

fix: apply suggestions for layer4 schema comments

f2c2beb

Co-authored-by: Travis Truman <[email protected]> Signed-off-by: Jennifer Power <[email protected]> Signed-off-by: Jennifer Power <[email protected]>

chore: refines layer4 schema comments

9d8a610

This change normalizes descriptions between different types defined in cue for layer4 and removes Go type language from the cue schema comments Signed-off-by: Jennifer Power <[email protected]>

jpower432 force-pushed the break/assessment-method branch from fc8a208 to d51aed2 Compare August 19, 2025 16:38

jpower432 force-pushed the break/assessment-method branch from d51aed2 to d431b80 Compare August 19, 2025 21:11

github-actions bot added the feature label Aug 20, 2025

trumant reviewed Aug 21, 2025

View reviewed changes

trumant reviewed Aug 22, 2025

View reviewed changes

This was referenced Aug 22, 2025

feat!: replace run duration with start and end times #112

Merged

feat!: groups assessment outcomes into Results (for discussion) #113

Closed

jpower432 mentioned this pull request Sep 2, 2025

feat!: adds EvaluationPlan to layer 4 schema #117

Merged

jpower432 closed this Sep 18, 2025

	"target-name": string @go(TagertName)
	"target-name": string @go(TargetName)

		// RunDuration is the time it took to run the assessment
		"run-duration"?: string @go(RunDuration)

		// Result communicates whether the assessment has been run, and if so, the outcome(s)
		result: #Result

		// ProcedureMethod describes method options that can be used to determine the results
		#ProcedureMethod: "Test" \| "Observation"

	// ProcedureMethod describes method options that can be used to determine the results
	#ProcedureMethod: "Test" \| "Observation"
	#EvaluationMethod: "Automated" \| "Manual"

		// Steps provides the address for the assessment steps executed
		"steps"?: [...string]

feat!: updates AssessmentStep to AssessmentMethod #83

feat!: updates AssessmentStep to AssessmentMethod #83

Uh oh!

Conversation

jpower432 commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Changed?

Rationale

Open Questions

Testing

Issues

Uh oh!

trumant left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jpower432 commented Aug 19, 2025

What Changed?

Rationale

Points of Discussion

Open Questions

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jpower432 Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jpower432 Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jpower432 Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jpower432 Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jpower432 commented Aug 8, 2025 •

edited

Loading

jpower432 Aug 22, 2025 •

edited

Loading

jpower432 Aug 22, 2025 •

edited

Loading

jpower432 Aug 22, 2025 •

edited

Loading

jpower432 Aug 22, 2025 •

edited

Loading