Skip to content

Conversation

@Sukuna0007Abhi
Copy link
Contributor

@Sukuna0007Abhi Sukuna0007Abhi commented Sep 25, 2025

Summary

This PR implements validation methods for TagID and Evidence types as mentioned in issue #23 and needed for veraison/corim#212.

Changes

TagID.Valid() method

  • Validates that tag identifier values are not nil
  • Ensures string values are not empty
  • Ensures UUID values are not nil UUID
  • Validates that value type is string or uuid.UUID

Evidence.Valid() method

  • Validates mandatory device-id field (not empty)
  • Validates mandatory date field (not zero)
  • Validates files if present (fs-name required)
  • Validates processes if present (process-name required)

Testing

  • Added comprehensive test coverage with 14 test cases covering all validation scenarios
  • All existing tests continue to pass
  • Added example documentation demonstrating usage patterns

Background

This implementation provides the foundational validation methods requested in issue #23. These methods also enable Evidence validation needed in veraison/corim#212 for proper data integrity checking.

Related Issues

Ready for review sir @thomas-fossati @setrofim

- Add TagID.Valid() method to validate tag identifier values
  - Checks for nil value
  - Validates empty strings
  - Validates nil UUID values
  - Ensures value type is string or uuid.UUID

- Add Evidence.Valid() method to validate evidence entries
  - Checks for empty device-id (mandatory field)
  - Checks for zero date (mandatory field)
  - Validates files if present (fs-name required)
  - Validates processes if present (process-name required)

- Add comprehensive test coverage for both methods
- Add example usage documentation

Fixes issue #212 by providing validation methods needed for
Evidence entries in CoRIM repository.

Signed-off-by: Sukuna0007Abhi <[email protected]>
Sukuna0007Abhi added a commit to Sukuna0007Abhi/corim that referenced this pull request Sep 25, 2025
## Summary

This commit implements Evidence validation in CoRIM using the newly added
Valid() methods from the SWID package, completing the work requested in
veraison#212.

## Changes

- Added Evidence validation calls using swid.Evidence.Valid() method
- Implemented proper error handling for validation failures
- Added validation at key integration points in the CoRIM workflow
- Enhanced error messages with context about which Evidence entry failed

## Dependencies

- Uses updated SWID package with Valid() methods from veraison/swid#23
  (implemented via veraison/swid#45 PR by Sukuna0007Abhi)
- Updated go.mod to use latest SWID version with replace directive

## Testing

- Added comprehensive unit tests for Evidence validation scenarios
- Added tests for both valid and invalid Evidence entries
- Verified all existing tests continue to pass
- Added integration tests for validation workflow

## Validation Points

Evidence validation is now performed at:
- CoSWIDEvidenceMap.Valid() - validates individual evidence entries
- CoSWIDEvidence.Valid() - validates evidence slice collections
- CoSWIDTriple.Valid() - validates evidence within triples
- AbbreviatedSwidTag.Valid() - validates evidence in COTS tags
- During unmarshaling of CoRIM data
- Before serialization/storage operations

## Error Handling

- Validation errors include context about failed Evidence entry
- Proper error propagation throughout the call stack
- Clear error messages for debugging and troubleshooting

## Files Modified

- coev/coswid_evidence.go: Added Valid() methods for evidence structures
- coev/coswidtriple.go: Enhanced CoSWIDTriple validation
- cots/abbreviated_swid_tag.go: Added evidence validation to SWID tags
- go.mod: Updated SWID dependency to version with Valid() methods

## Files Added

- coev/coswid_evidence_test.go: Comprehensive evidence validation tests
- cots/abbreviated_swid_evidence_test.go: SWID tag evidence validation tests

Implements veraison#212
Related: veraison/swid#23 (done via veraison/swid#45 PR)

Signed-off-by: Sukuna0007Abhi <[email protected]>
return errors.New("evidence date is zero")
}

// Validate Files if present
Copy link
Contributor

@yogeshbdeshpande yogeshbdeshpande Sep 29, 2025

Choose a reason for hiding this comment

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

The correct implementation is to invoke Method Valid() on the File Object and let File Object, in the file file.go to implement the Valid Method that can be invoked from line 71. In the Valid for file, check for validity of Mandatory (like FsName) and check for presence of optional elements in the file object. If Optional Elements are present, then please check their validity!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure sir checking it @yogeshbdeshpande

}

// Validate Processes if present
if e.Processes != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

for process, may be we are ok to check here as there is not much to check for validity of the process elements.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh ok sir

Copy link
Contributor

@yogeshbdeshpande yogeshbdeshpande left a comment

Choose a reason for hiding this comment

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

Please check the latest comments!

@Sukuna0007Abhi
Copy link
Contributor Author

Sukuna0007Abhi commented Sep 29, 2025

Ok sir @yogeshbdeshpande thanks for reviewing I am updating it

@yogeshbdeshpande
Copy link
Contributor

Thank you for all your efforts! Very much appreciate your help!

@Sukuna0007Abhi
Copy link
Contributor Author

Ready for Review sir @yogeshbdeshpande sir @thomas-fossati sir @setrofim

Hash *HashEntry `cbor:"7,keyasint,omitempty" json:"hash,omitempty" xml:"hash,attr,omitempty"`
}

// Valid validates the File receiver to ensure it has valid required and optional fields
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks very good to me! Thanks for incorporating feedback!

@Sukuna0007Abhi
Copy link
Contributor Author

Sukuna0007Abhi commented Sep 30, 2025 via email

if v == "" {
return errors.New("tag-id string value is empty")
}
case uuid.UUID:
Copy link
Contributor

Choose a reason for hiding this comment

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

you should call the Validate method on UUID..?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @yogeshbdeshpande, thank you for the review!

I investigated the UUID validation you mentioned. The github.com/google/uuid package (v1.3.0) doesn't provide a Validate() method. The available methods are:

  • String(), Version(), Variant(), etc.

UUIDs in TagID are already validated during creation via uuid.Parse() and uuid.FromBytes() in the NewTagIDFromUUID* functions. Since UUIDs can only enter a TagID through these validated paths, checking for uuid.Nil is the standard approach for this use case.

If you'd like additional validation (e.g., checking Version() or Variant()), I'm happy to add that. Could you clarify what specific validation you had in mind?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@Sukuna0007Abhi Sukuna0007Abhi Sep 30, 2025

Choose a reason for hiding this comment

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

Thanks sir, @yogeshbdeshpande Sorry, You're absolutely right! Thank you for the link.

I was looking at v1.3.0 (our current version) which doesn't have Validate, but the newer versions do have uuid.Validate(s string) as a package function.

However, since we're dealing with an already-parsed uuid.UUID (not a string), we can't use uuid.Validate() directly. But I can enhance the validation to be more thorough. Would you like me to:

  1. Update to the latest uuid version, or
  2. Add additional validation using Version() and Variant() methods?

Thanks for catching this - great learning moment!

Copy link
Contributor

Choose a reason for hiding this comment

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

I am looking into this further, will update you shortly!

Copy link
Contributor

Choose a reason for hiding this comment

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

I think, it is ok, to just check for the Variant here.

Please see:
https://github.com/veraison/corim/blob/main/comid/uuid.go#L35

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure sir @yogeshbdeshpande thank you for review and merging, thanks to all mentors also

Copy link
Contributor

@yogeshbdeshpande yogeshbdeshpande left a comment

Choose a reason for hiding this comment

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

please see more comments!

@Sukuna0007Abhi

This comment was marked as duplicate.

- Enhanced TagID.Valid() to check UUID variant compliance with RFC4122
- Fixed comment format for TagID.URI() method
- Simplified embedded field access in File.Valid()
- Added comprehensive test for UUID variant validation
- All tests pass and linting issues resolved

Signed-off-by: Sukuna0007Abhi <[email protected]>
@Sukuna0007Abhi
Copy link
Contributor Author

Hi sir @yogeshbdeshpande sir @thomas-fossati sir @setrofim ready for review

Copy link
Contributor

@yogeshbdeshpande yogeshbdeshpande left a comment

Choose a reason for hiding this comment

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

LGTM!

@Sukuna0007Abhi
Copy link
Contributor Author

Sukuna0007Abhi commented Oct 2, 2025

Thanks sir @yogeshbdeshpande I will fix that two cl errors,

@Sukuna0007Abhi
Copy link
Contributor Author

Also pls review sir @setrofim @thomas-fossati

- Replace deprecated linters (deadcode, golint, maligned, structcheck, varcheck)
  with modern equivalents (unused, revive)
- Add missing setup-go action in CI workflow to fix 'go: command not found' error
- Update actions/checkout from v1 to v4 for better compatibility

These deprecated linters have been abandoned by maintainers since v1.38-v1.49
and are replaced by actively maintained alternatives with equivalent functionality.

Signed-off-by: Sukuna0007Abhi <[email protected]>
@Sukuna0007Abhi
Copy link
Contributor Author

Sukuna0007Abhi commented Oct 2, 2025

Hi sir @yogeshbdeshpande sorry don't know how it dismissed, I just fixed the cl errors after your approval, could you please review and approve again, sorry for interruption

@yogeshbdeshpande
Copy link
Contributor

LGTM but the Linters is still failing, please check, we can work on this tomorrow!

@Sukuna0007Abhi
Copy link
Contributor Author

Sure sir @yogeshbdeshpande thanks for addressing my response

@yogeshbdeshpande
Copy link
Contributor

@Sukuna0007Abhi Please update the corresponding lint to the latest and that will fix the issue, can you please do this, so that we can merge this PR!

@Sukuna0007Abhi
Copy link
Contributor Author

Sukuna0007Abhi commented Oct 3, 2025 via email

@yogeshbdeshpande
Copy link
Contributor

@Sukuna0007Abhi : Refer to https://github.com/veraison/corim/blob/main/.github/workflows/linters.yml

But remove the mockgen part (do not copy that aspect!)

- Update linters.yml to use golangci-lint v2.5.0 (fixes typecheck panic)
- Add explicit Go 1.24 setup in linters workflow
- Update checkout action to v4 and setup-go to v5
- Configure govet to use enable: [shadow] instead of deprecated check-shadowing
- Add version: 2 to .golangci.yml for v2.x compatibility

This resolves the 'panic: unsupported version: 2' and 'undefined: cbor' errors
that were occurring with older golangci-lint versions and Go 1.24.5.

Signed-off-by: Sukuna0007Abhi <[email protected]>
- Reorder steps: setup-go first, then checkout, then install golangci-lint
- Use golangci-lint v2.5.0 (compatible with Go 1.24)
- Update to actions/setup-go@v5 and actions/checkout@v4
- Follow structure from veraison/corim repo as suggested by sir @yogeshbdeshpande
- Removed mockgen installation as instructed

Reference: https://github.com/veraison/corim/blob/main/.github/workflows/linters.yml

Signed-off-by: Sukuna0007Abhi <[email protected]>
@Sukuna0007Abhi
Copy link
Contributor Author

Sukuna0007Abhi commented Oct 3, 2025 via email

@yogeshbdeshpande
Copy link
Contributor

@Sukuna0007Abhi : Seems the new lint versions do not have gosimple, so please use in the Makefile the
arguments, similar to CoRIM like:
GOLINT_ARGS ?= run --timeout=3m -E dupl -E gocritic -E staticcheck -E lll -E prealloc

gosimple has been merged into staticcheck in golangci-lint v2.x
and is no longer available as a separate linter.

Signed-off-by: Sukuna0007Abhi <[email protected]>
@yogeshbdeshpande
Copy link
Contributor

In older versions of golangci-lint, gosimple (from staticcheck) was available as a standalone linter.

It has since been merged into staticcheck, and enabling staticcheck automatically includes all gosimple checks.

@yogeshbdeshpande
Copy link
Contributor

And try make lint on your own laptop so that you can progress this further!

@yogeshbdeshpande
Copy link
Contributor

@Sukuna0007Abhi Finally the linters pass as well:

Well Done!

@yogeshbdeshpande
Copy link
Contributor

use commit --amend
and insert your signature so that DCO can pass as well...

@Sukuna0007Abhi
Copy link
Contributor Author

Sukuna0007Abhi commented Oct 3, 2025 via email

@Sukuna0007Abhi
Copy link
Contributor Author

Sukuna0007Abhi commented Oct 3, 2025 via email

@yogeshbdeshpande
Copy link
Contributor

Thank you so much sir @yogeshbdeshpande for guiding through the end ,

On Fri, 3 Oct 2025 at 17:20, Yogesh Deshpande @.> wrote: yogeshbdeshpande left a comment (veraison/swid#45) <#45 (comment)> @Sukuna0007Abhi https://github.com/Sukuna0007Abhi Finally the linters pass as well: Well Done! — Reply to this email directly, view it on GitHub <#45 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/BLZUP7PEL2LDG4UMEPHPV3D3VZPG7AVCNFSM6AAAAACHPL4MJSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTGNRVGM4TEMJXHE . You are receiving this because you were mentioned.Message ID: @.>

No issues, just check how the github commits in CoRIM Repos have signatures in the end, you can pretty much follow the same and re-use with your signature

@yogeshbdeshpande
Copy link
Contributor

Sir @yogeshbdeshpande actually I already added signed off to all my commits, it's from another commits I think On Fri, 3 Oct 2025 at 17:20, Abhijit Das Sukuna0007Abhi < @.> wrote:

Thank you so much sir @yogeshbdeshpande for guiding through the end , On Fri, 3 Oct 2025 at 17:20, Yogesh Deshpande @.
> wrote: > yogeshbdeshpande left a comment (veraison/swid#45) > <#45 (comment)> > > @Sukuna0007Abhi https://github.com/Sukuna0007Abhi Finally the linters > pass as well: > > Well Done! > > — > Reply to this email directly, view it on GitHub > <#45 (comment)>, or > unsubscribe > https://github.com/notifications/unsubscribe-auth/BLZUP7PEL2LDG4UMEPHPV3D3VZPG7AVCNFSM6AAAAACHPL4MJSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTGNRVGM4TEMJXHE > . > You are receiving this because you were mentioned.Message ID: > @.***> >

ok fine, we can squash the commits then!

@yogeshbdeshpande
Copy link
Contributor

@Sukuna0007Abhi Giving a final review. I will create one issue, which is a simple one to remove few of lint errors.

Please feel free to progress this, if you have Bandwidth!

@Sukuna0007Abhi
Copy link
Contributor Author

Sukuna0007Abhi commented Oct 3, 2025 via email

@Sukuna0007Abhi
Copy link
Contributor Author

Sukuna0007Abhi commented Oct 3, 2025 via email

@@ -0,0 +1,83 @@
// Copyright 2020 Contributors to the Veraison project.
Copy link
Contributor

@yogeshbdeshpande yogeshbdeshpande Oct 3, 2025

Choose a reason for hiding this comment

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

Suggested change
// Copyright 2020 Contributors to the Veraison project.
// Copyright 2020-2025 Contributors to the Veraison project.

@yogeshbdeshpande
Copy link
Contributor

@Sukuna0007Abhi : For the Modified files with the Copyright header, could you please change all the Headers to how I have done in one of the example comment, as now the files have been modified.!

@Sukuna0007Abhi
Copy link
Contributor Author

Sukuna0007Abhi commented Oct 3, 2025 via email

gosimple is now included in staticcheck in golangci-lint v2.x.
Update copyright headers to original-year-2025 for all modified files.

Signed-off-by: Sukuna0007Abhi <[email protected]>
@Sukuna0007Abhi
Copy link
Contributor Author

Sukuna0007Abhi commented Oct 3, 2025 via email

Copy link
Contributor

@yogeshbdeshpande yogeshbdeshpande left a comment

Choose a reason for hiding this comment

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

LGTM!

@yogeshbdeshpande yogeshbdeshpande merged commit fd1f7f1 into veraison:main Oct 3, 2025
5 checks passed
@yogeshbdeshpande
Copy link
Contributor

@Sukuna0007Abhi Thank you for your efforts on this. Very much Appreciate !

@Sukuna0007Abhi
Copy link
Contributor Author

Sukuna0007Abhi commented Oct 3, 2025 via email

@Sukuna0007Abhi
Copy link
Contributor Author

Sukuna0007Abhi commented Oct 3, 2025 via email

setrofim pushed a commit to veraison/corim that referenced this pull request Oct 6, 2025
## Summary

This commit implements Evidence validation in CoRIM using the newly added
Valid() methods from the SWID package, completing the work requested in
#212.

## Changes

- Added Evidence validation calls using swid.Evidence.Valid() method
- Implemented proper error handling for validation failures
- Added validation at key integration points in the CoRIM workflow
- Enhanced error messages with context about which Evidence entry failed

## Dependencies

- Uses updated SWID package with Valid() methods from veraison/swid#23
  (implemented via veraison/swid#45 PR by Sukuna0007Abhi)
- Updated go.mod to use latest SWID version with replace directive

## Testing

- Added comprehensive unit tests for Evidence validation scenarios
- Added tests for both valid and invalid Evidence entries
- Verified all existing tests continue to pass
- Added integration tests for validation workflow

## Validation Points

Evidence validation is now performed at:
- CoSWIDEvidenceMap.Valid() - validates individual evidence entries
- CoSWIDEvidence.Valid() - validates evidence slice collections
- CoSWIDTriple.Valid() - validates evidence within triples
- AbbreviatedSwidTag.Valid() - validates evidence in COTS tags
- During unmarshaling of CoRIM data
- Before serialization/storage operations

## Error Handling

- Validation errors include context about failed Evidence entry
- Proper error propagation throughout the call stack
- Clear error messages for debugging and troubleshooting

## Files Modified

- coev/coswid_evidence.go: Added Valid() methods for evidence structures
- coev/coswidtriple.go: Enhanced CoSWIDTriple validation
- cots/abbreviated_swid_tag.go: Added evidence validation to SWID tags
- go.mod: Updated SWID dependency to version with Valid() methods

## Files Added

- coev/coswid_evidence_test.go: Comprehensive evidence validation tests
- cots/abbreviated_swid_evidence_test.go: SWID tag evidence validation tests

Implements #212
Related: veraison/swid#23 (done via veraison/swid#45 PR)

Signed-off-by: Sukuna0007Abhi <[email protected]>
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