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

The process-event API could use stricter validation #261

Open
martialblog opened this issue Jul 25, 2024 · 2 comments
Open

The process-event API could use stricter validation #261

martialblog opened this issue Jul 25, 2024 · 2 comments

Comments

@martialblog
Copy link
Member

The process-event API could use stricter validation. Here's what I mean:

cat 2.json 
{
}

curl -u 'source-2:whatisatypename123' -d '@2.json' 'http://localhost:5680/process-event'
invalid event: tags must not be empty

Ok good so far, but what about:

cat 3.json 
{
  "tags": {
    "": ""
  }
}

curl -i -u 'source-2:whatisatypename123' -d '@3.json' 'http://localhost:5680/process-event'
event processed successfully

Resulting in:
screenshot_2024-07-25-143104

@julianbrost
Copy link
Collaborator

{
"tags": {
"": ""
}
}

That actually shows multiple problems:

  1. This should result in an "must set severity or event type" error (I thought this was actually already there, but doesn't seem to be the case then)
  2. Tag names should be non-empty.
  3. ID tag values should probably be non-empty, for extra tags, at least not at the moment1.

Footnotes

  1. At the moment, we do something like hostgroup/Server so these don't have a value at the moment so we can't require this at the moment. However, this specific aspect will probably change in the future, see Allow multiple values for tags #217. However, I'm not 100% sure if we want to require that even then or if we want to allow something like boolean flag tags (i.e. present/not present), that could be used for something like production, testing instead of env=production, env=testing.

@martialblog
Copy link
Member Author

From what I can tell, the Validate() method only checks the slice length and the upper length of the tag's keys (not values).

func (e *Event) Validate() error {
        if len(e.Tags) == 0 {
                return fmt.Errorf("invalid event: tags must not be empty")
        }

        for tag := range e.Tags {
                if len(tag) > 255 {
                        return fmt.Errorf("invalid event: tag %q is too long, at most 255 chars allowed, %d given", tag, len(tag))
                }
        }

        for tag := range e.ExtraTags {
                if len(tag) > 255 {
                        return fmt.Errorf(
                                "invalid event: extra tag %q is too long, at most 255 chars allowed, %d given", tag, len(tag),
                        )
                }
        }
...

This method also could use some unittests

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

No branches or pull requests

2 participants