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

Adding note to sbom-validate about the output path #922

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Ninja4Code
Copy link

Added a note to the sbom-tool validate command to warn users not to output their validation file in the same folder as the drop path. The output path needs to be outside of your drop path. Otherwise on subsequent validations you'll receive a FAILURE even though nothing changed in your project dependencies. This is because the validate file gets added to your drop path and it's messed up the hash comparison because you added a file into drop path. Hopefully this makes sense.

Added a note to the sbom-tool validate command to warn users not to output their validation file in the same folder as the drop path.  The output path needs to be outside of your drop path.  Otherwise on subsequent validations you'll receive a FAILURE even though nothing changed in your project dependencies.  This is because the validate file gets added to your drop path and it's messed up the hash comparison because you added a file into drop path.  Hopefully this makes sense.
@Ninja4Code Ninja4Code requested a review from a team as a code owner February 5, 2025 19:55
@@ -132,6 +132,8 @@ This sample command provides the minimum mandatory arguments required to validat
`-o` is the output path where the tool will write the validation results. This path can be any file path on the system. In this case the tool will look for the validationOutputPath directory, create a file named output.json, and write the validation output.
`-mi` is the ManifestInfo, which provides the user's desired name and version of the manifest format.

NOTE: The output path should not be in the same drop path as specified in the generate command, otherwise the validation file will be added to the files to validate. The first validation will report SUCCESS but then other validate commands after it will fail. This is because the validation file has been added in the drop path location and it will no longer validate because it detects that your validate output file has been injected into your drop path.
Copy link
Contributor

Choose a reason for hiding this comment

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

Great callout, @Ninja4Code! This feels like something that we should treat as an error condition in a future breaking change. Here's a suggested rewording that also follows our "one space after a period" standard that we try to maintain:

Suggested change
NOTE: The output path should not be in the same drop path as specified in the generate command, otherwise the validation file will be added to the files to validate. The first validation will report SUCCESS but then other validate commands after it will fail. This is because the validation file has been added in the drop path location and it will no longer validate because it detects that your validate output file has been injected into your drop path.
NOTE: Do not specify an output path that is within the drop path, because the resulting output file will _change the set of files in the drop path_. The first validation will succeed, but subsequent validations will fail because the newly added file is not reflected in the SBOM. This may be treated as an error condition in future versions of the tool.

If you would also update the wording for the tool itself, that would be great. Instead of The path where the output json should be written. ex: Path/output.json, let's go with The path where the output json should be written. Do not specify a path within your BuildDropPath. ex: Path/output.json. in the following locations:

@Ninja4Code
Copy link
Author

Ninja4Code commented Feb 6, 2025 via email

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.

2 participants