Restrict tag for DomainHash to be alphanumeric#2277
Conversation
|
👋 vreff, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
|
There was a problem hiding this comment.
Pull request overview
This PR tightens teeattestation.DomainHash by restricting the domain tag to non-empty ASCII alphanumeric characters and propagating validation failures to callers via an error return, updating dependent tests accordingly.
Changes:
- Updated
DomainHashto return([]byte, error)and reject non-alphanumeric/empty tags. - Adjusted Nitro validation tests to use alphanumeric tags and handle the new error return.
- Updated
DomainHashunit tests to handle the new signature and added an invalid-tag test.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/teeattestation/nitro/validate_test.go | Updates Nitro attestation validation tests to accommodate DomainHash’s new error return and tag restrictions. |
| pkg/teeattestation/hash.go | Adds tag validation to DomainHash and changes its signature to return an error on invalid tags. |
| pkg/teeattestation/hash_test.go | Updates tests for new DomainHash signature and introduces invalid-tag coverage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
pkg/teeattestation/hash.go:21
- The tag validation rejects both non-ASCII characters and ASCII non-alphanumeric characters (e.g. '-'), but the docstring currently only mentions “non-ASCII alphanumeric characters”. Also, returning a fresh formatted error makes it harder for callers to reliably detect this validation failure. Consider updating the docstring to match the constraint and returning a sentinel error value (so callers can use errors.Is without string-matching).
// tag and data, so distinct (tag, data) pairs can never share a pre-image. [CL112-14]
// It returns an error if tag is empty or contains non-ASCII alphanumeric characters.
func DomainHash(tag string, data []byte) ([]byte, error) {
if tag == "" || !isAlphanumeric(tag) {
return nil, fmt.Errorf("invalid tag: must be non-empty and contain only alphanumeric characters")
| // tag and data, so distinct (tag, data) pairs can never share a pre-image. [CL112-14] | ||
| func DomainHash(tag string, data []byte) []byte { | ||
| // It returns an error if tag is empty or contains non-ASCII alphanumeric characters. | ||
| func DomainHash(tag string, data []byte) ([]byte, error) { |
No description provided.