false-positives when validating webhooks payloads due to email addresses #453

wolfy1339 · 2021-04-22T01:07:01Z

What happened?

When validating webhook payloads that I receive against the schemas, the email doesn't match up to the format.

The email that was tripping the validation was 41898282+github-actions[bot]@users.noreply.github.com, I assume that any web flow commit from users might not validate.

What did you expect to happen?
For their not to be any validation error, since it is a valid email address
What the problem might be

The problem probably lies upstream with ajv-formats, I haven't dug into this too much

The text was updated successfully, but these errors were encountered:

wolfy1339 · 2021-04-22T01:11:14Z

Webhook JSON payload

{
  "ref": "refs/heads/upstream",
  "before": "0000000000000000000000000000000000000000",
  "after": "9f2cb8d507ac41aabecc02ea134e9d6dd2a37a38",
  "repository": {
    "id": 325092396,
    "node_id": "MDEwOlJlcG9zaXRvcnkzMjUwOTIzOTY=",
    "name": "compare-gh-webhook-to-schema-function",
    "full_name": "wolfy1339/compare-gh-webhook-to-schema-function",
    "private": false,
    "owner": {
      "name": "wolfy1339",
      "email": "4595477+wolfy1339@users.noreply.github.com",
      "login": "wolfy1339",
      "id": 4595477,
      "node_id": "MDQ6VXNlcjQ1OTU0Nzc=",
      "avatar_url": "https://avatars.githubusercontent.com/u/4595477?v=4",
      "gravatar_id": "",
      "url": "https://api.github.com/users/wolfy1339",
      "html_url": "https://github.com/wolfy1339",
      "followers_url": "https://api.github.com/users/wolfy1339/followers",
      "following_url": "https://api.github.com/users/wolfy1339/following{/other_user}",
      "gists_url": "https://api.github.com/users/wolfy1339/gists{/gist_id}",
      "starred_url": "https://api.github.com/users/wolfy1339/starred{/owner}{/repo}",
      "subscriptions_url": "https://api.github.com/users/wolfy1339/subscriptions",
      "organizations_url": "https://api.github.com/users/wolfy1339/orgs",
      "repos_url": "https://api.github.com/users/wolfy1339/repos",
      "events_url": "https://api.github.com/users/wolfy1339/events{/privacy}",
      "received_events_url": "https://api.github.com/users/wolfy1339/received_events",
      "type": "User",
      "site_admin": false
    },
    "html_url": "https://github.com/wolfy1339/compare-gh-webhook-to-schema-function",
    "description": null,
    "fork": true,
    "url": "https://github.com/wolfy1339/compare-gh-webhook-to-schema-function",
    "forks_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/forks",
    "keys_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/keys{/key_id}",
    "collaborators_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/collaborators{/collaborator}",
    "teams_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/teams",
    "hooks_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/hooks",
    "issue_events_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/issues/events{/number}",
    "events_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/events",
    "assignees_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/assignees{/user}",
    "branches_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/branches{/branch}",
    "tags_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/tags",
    "blobs_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/git/blobs{/sha}",
    "git_tags_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/git/tags{/sha}",
    "git_refs_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/git/refs{/sha}",
    "trees_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/git/trees{/sha}",
    "statuses_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/statuses/{sha}",
    "languages_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/languages",
    "stargazers_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/stargazers",
    "contributors_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/contributors",
    "subscribers_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/subscribers",
    "subscription_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/subscription",
    "commits_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/commits{/sha}",
    "git_commits_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/git/commits{/sha}",
    "comments_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/comments{/number}",
    "issue_comment_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/issues/comments{/number}",
    "contents_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/contents/{+path}",
    "compare_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/compare/{base}...{head}",
    "merges_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/merges",
    "archive_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/{archive_format}{/ref}",
    "downloads_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/downloads",
    "issues_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/issues{/number}",
    "pulls_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/pulls{/number}",
    "milestones_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/milestones{/number}",
    "notifications_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/notifications{?since,all,participating}",
    "labels_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/labels{/name}",
    "releases_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/releases{/id}",
    "deployments_url": "https://api.github.com/repos/wolfy1339/compare-gh-webhook-to-schema-function/deployments",
    "created_at": 1609181782,
    "updated_at": "2021-04-20T17:19:22Z",
    "pushed_at": 1619037800,
    "git_url": "git://github.com/wolfy1339/compare-gh-webhook-to-schema-function.git",
    "ssh_url": "git@github.com:wolfy1339/compare-gh-webhook-to-schema-function.git",
    "clone_url": "https://github.com/wolfy1339/compare-gh-webhook-to-schema-function.git",
    "svn_url": "https://github.com/wolfy1339/compare-gh-webhook-to-schema-function",
    "homepage": null,
    "size": 1780,
    "stargazers_count": 0,
    "watchers_count": 0,
    "language": "TypeScript",
    "has_issues": false,
    "has_projects": true,
    "has_downloads": true,
    "has_wiki": true,
    "has_pages": false,
    "forks_count": 0,
    "mirror_url": null,
    "archived": false,
    "disabled": false,
    "open_issues_count": 0,
    "license": null,
    "forks": 0,
    "open_issues": 0,
    "watchers": 0,
    "default_branch": "main",
    "stargazers": 0,
    "master_branch": "main"
  },
  "pusher": {
    "name": "wolfy1339",
    "email": "4595477+wolfy1339@users.noreply.github.com"
  },
  "sender": {
    "login": "wolfy1339",
    "id": 4595477,
    "node_id": "MDQ6VXNlcjQ1OTU0Nzc=",
    "avatar_url": "https://avatars.githubusercontent.com/u/4595477?v=4",
    "gravatar_id": "",
    "url": "https://api.github.com/users/wolfy1339",
    "html_url": "https://github.com/wolfy1339",
    "followers_url": "https://api.github.com/users/wolfy1339/followers",
    "following_url": "https://api.github.com/users/wolfy1339/following{/other_user}",
    "gists_url": "https://api.github.com/users/wolfy1339/gists{/gist_id}",
    "starred_url": "https://api.github.com/users/wolfy1339/starred{/owner}{/repo}",
    "subscriptions_url": "https://api.github.com/users/wolfy1339/subscriptions",
    "organizations_url": "https://api.github.com/users/wolfy1339/orgs",
    "repos_url": "https://api.github.com/users/wolfy1339/repos",
    "events_url": "https://api.github.com/users/wolfy1339/events{/privacy}",
    "received_events_url": "https://api.github.com/users/wolfy1339/received_events",
    "type": "User",
    "site_admin": false
  },
  "installation": {
    "id": 3456996,
    "node_id": "MDIzOkludGVncmF0aW9uSW5zdGFsbGF0aW9uMzQ1Njk5Ng=="
  },
  "created": true,
  "deleted": false,
  "forced": false,
  "base_ref": null,
  "compare": "https://github.com/wolfy1339/compare-gh-webhook-to-schema-function/compare/upstream",
  "commits": [],
  "head_commit": {
    "id": "9f2cb8d507ac41aabecc02ea134e9d6dd2a37a38",
    "tree_id": "a2c0dd228b52978f856a9be24a3877f95fe2aa02",
    "distinct": true,
    "message": "feat: update `@octokit/webhooks-definitions` (#59)\n\n* feat: update `@octokit/webhooks-definitions`\r\n\r\n* feat: update `@octokit/webhooks-definitions`\r\n\r\n* feat: update `@octokit/webhooks-definitions`\r\n\r\nCo-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>",
    "timestamp": "2021-04-14T09:51:00+12:00",
    "url": "https://github.com/wolfy1339/compare-gh-webhook-to-schema-function/commit/9f2cb8d507ac41aabecc02ea134e9d6dd2a37a38",
    "author": {
      "name": "github-actions[bot]",
      "email": "41898282+github-actions[bot]@users.noreply.github.com",
      "username": "github-actions[bot]"
    },
    "committer": {
      "name": "GitHub",
      "email": "noreply@github.com",
      "username": "web-flow"
    },
    "added": [
      "src/schemas/common/discussion.d.ts",
      "src/schemas/common/discussion.schema.json",
      "src/schemas/discussion/answered.d.ts",
      "src/schemas/discussion/answered.schema.json",
      "src/schemas/discussion/category_changed.d.ts",
      "src/schemas/discussion/category_changed.schema.json",
      "src/schemas/discussion/created.d.ts",
      "src/schemas/discussion/created.schema.json",
      "src/schemas/discussion/deleted.d.ts",
      "src/schemas/discussion/deleted.schema.json",
      "src/schemas/discussion/edited.d.ts",
      "src/schemas/discussion/edited.schema.json",
      "src/schemas/discussion/index.d.ts",
      "src/schemas/discussion/locked.d.ts",
      "src/schemas/discussion/locked.schema.json",
      "src/schemas/discussion/pinned.d.ts",
      "src/schemas/discussion/pinned.schema.json",
      "src/schemas/discussion/transferred.d.ts",
      "src/schemas/discussion/transferred.schema.json",
      "src/schemas/discussion/unanswered.d.ts",
      "src/schemas/discussion/unanswered.schema.json",
      "src/schemas/discussion/unlocked.d.ts",
      "src/schemas/discussion/unlocked.schema.json",
      "src/schemas/discussion/unpinned.d.ts",
      "src/schemas/discussion/unpinned.schema.json",
      "src/schemas/discussion_comment/created.d.ts",
      "src/schemas/discussion_comment/created.schema.json",
      "src/schemas/discussion_comment/deleted.d.ts",
      "src/schemas/discussion_comment/deleted.schema.json",
      "src/schemas/discussion_comment/edited.d.ts",
      "src/schemas/discussion_comment/edited.schema.json",
      "src/schemas/discussion_comment/index.d.ts"
    ],
    "removed": [],
    "modified": [
      "package-lock.json",
      "package.json",
      "src/schemas/common/commit.d.ts",
      "src/schemas/common/commit.schema.json",
      "src/schemas/common/index.d.ts",
      "src/schemas/common/installation.d.ts",
      "src/schemas/common/installation.schema.json",
      "src/schemas/common/project-card.d.ts",
      "src/schemas/common/project-card.schema.json",
      "src/schemas/common/sponsorship-tier.d.ts",
      "src/schemas/common/sponsorship-tier.schema.json",
      "src/schemas/common/workflow-run.d.ts",
      "src/schemas/common/workflow-run.schema.json",
      "src/schemas/index.d.ts",
      "src/schemas/installation/created.d.ts",
      "src/schemas/installation/created.schema.json",
      "src/schemas/installation_repositories/added.d.ts",
      "src/schemas/installation_repositories/added.schema.json",
      "src/schemas/issue_comment/edited.d.ts",
      "src/schemas/issue_comment/edited.schema.json",
      "src/schemas/project_card/converted.d.ts",
      "src/schemas/project_card/converted.schema.json",
      "src/schemas/project_card/edited.d.ts",
      "src/schemas/project_card/edited.schema.json",
      "src/schemas/project_card/moved.d.ts",
      "src/schemas/project_card/moved.schema.json",
      "src/schemas/pull_request/synchronize.d.ts",
      "src/schemas/pull_request/synchronize.schema.json",
      "src/schemas/pull_request_review_comment/created.d.ts",
      "src/schemas/pull_request_review_comment/created.schema.json",
      "src/schemas/pull_request_review_comment/deleted.d.ts",
      "src/schemas/pull_request_review_comment/deleted.schema.json",
      "src/schemas/pull_request_review_comment/edited.d.ts",
      "src/schemas/pull_request_review_comment/edited.schema.json",
      "src/schemas/security_advisory/performed.d.ts",
      "src/schemas/security_advisory/performed.schema.json",
      "src/schemas/security_advisory/published.d.ts",
      "src/schemas/security_advisory/published.schema.json",
      "src/schemas/security_advisory/updated.d.ts",
      "src/schemas/security_advisory/updated.schema.json",
      "src/schemas/workflow_run/completed.schema.json",
      "src/schemas/workflow_run/requested.schema.json"
    ]
  }
}

AJV error:

[
    {
      "instancePath": "/head_commit/author/email",
      "schemaPath": "committer.schema.json/properties/email/oneOf/0/format",
      "keyword": "format",
      "params": {
        "format": "email"
      },
      "message": "must match format \"email\""
    },
    {
      "instancePath": "/head_commit/author/email",
      "schemaPath": "committer.schema.json/properties/email/oneOf/1/type",
      "keyword": "type",
      "params": {
        "type": "null"
      },
      "message": "must be null"
    },
    {
      "instancePath": "/head_commit/author/email",
      "schemaPath": "committer.schema.json/properties/email/oneOf",
      "keyword": "oneOf",
      "params": {
        "passingSchemas": null
      },
      "message": "must match exactly one schema in oneOf"
    }
]

wolfy1339 · 2022-07-18T00:07:43Z

I've done more research on this and the email addresses do not respect the spec RFC 5322 Section 3.4

The reason is because [ and ] aren't valid characters unless the email address is quoted.

Example invalid email:

29139614+renovate[bot]@users.noreply.github.com

Example of a fix for that email address:

"29139614+renovate[bot]"@users.noreply.github.com

Note the quotes around the part before the @

oscard0m · 2022-07-19T09:34:05Z

Who should be responsible for escaping these invalid characters? @octokit/webhooks or ajv-formats library?

timrogers · 2022-07-19T09:59:02Z

Could we just change the schema and remove format: "email" if the email addresses don't align with the email RFC? I guess that should fix this too.

timrogers · 2022-07-19T10:02:02Z

According to this page, format: "email" isn't strictly part of the OpenAPI specification - formats are user-defined.

wolfy1339 · 2022-07-19T13:27:56Z

Who should be responsible for escaping these invalid characters? @octokit/webhooks or ajv-formats library?

Neither should be escaping any data. We simply pass along the data we received.
The characters should be escaped at the source.

Could we just change the schema and remove format: "email" if the email addresses don't align with the email RFC? I guess that should fix this too.

That would indeed fix this, but then we would lose the format

According to this page, format: "email" isn't strictly part of the OpenAPI specification - formats are user-defined.

We don't use the OpenAPI specification, we use the JSON schema specification. They are different.

If we we're going to use OpenAPI, it would be OpenAPI 3.1, since it has a new webhooks field to define webhooks.
In OpenAPI 3.1, these are the defined formats: https://datatracker.ietf.org/doc/html/draft-bhutton-json-schema-validation-00#section-7.3

Here are the defined formats for JSON Schema (we use draft-07): https://json-schema.org/understanding-json-schema/reference/string.html#built-in-formats

timrogers · 2022-07-19T14:18:26Z

Ah, apologies - I assumed we were using OpenAPI here! I didn't imagine we had a different schema type, but I should have checked.

Nevertheless, I think we should probably remove the format: "email" declaration because, factually, the emails GitHub returns aren't RFC-compliant. That isn't likely to change any time soon as those email addresses are stored on many commits.

oscard0m · 2022-07-19T14:43:51Z

Is it safe to validate removing the "emails" field? I'm not sure what implications it has.

wolfy1339 · 2022-07-19T14:45:21Z

Is it safe to validate removing "emails" field? I'm not sure ehat implications it has.

We wouldn't be removing any fields from the schema, only the format. Instead of validating against a specific format, we would only validate that it is a string

oscard0m · 2022-07-19T14:50:49Z

Is it safe to validate removing "emails" field? I'm not sure ehat implications it has.

We wouldn't be removing any fields from the schema, only the format. Instead of validating against a specific format, we would only validate that it is a string

And is there any danger of not checking it has a valid email format?

octokitbot · 2022-07-21T10:19:41Z

🎉 This issue has been resolved in version 6.2.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

wolfy1339 added the Type: Bug Something isn't working as documented label Apr 22, 2021

ghost added this to Bugs in JS Apr 22, 2021

This was referenced Jul 20, 2022

fix(payload-schemas): add custom RegEx validation for 'email' #677

Closed

fix(schemas): remove 'email' format for 'email' property in committer.schema.json #678

Merged

wolfy1339 closed this as completed in #678 Jul 21, 2022

JS automation moved this from Bugs to Done Jul 21, 2022

octokitbot added the released label Jul 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

false-positives when validating webhooks payloads due to email addresses #453

false-positives when validating webhooks payloads due to email addresses #453

wolfy1339 commented Apr 22, 2021

wolfy1339 commented Apr 22, 2021

wolfy1339 commented Jul 18, 2022 •

edited

oscard0m commented Jul 19, 2022

timrogers commented Jul 19, 2022 •

edited

timrogers commented Jul 19, 2022

wolfy1339 commented Jul 19, 2022

timrogers commented Jul 19, 2022

oscard0m commented Jul 19, 2022 •

edited

wolfy1339 commented Jul 19, 2022

oscard0m commented Jul 19, 2022

octokitbot commented Jul 21, 2022

false-positives when validating webhooks payloads due to email addresses #453

false-positives when validating webhooks payloads due to email addresses #453

Comments

wolfy1339 commented Apr 22, 2021

wolfy1339 commented Apr 22, 2021

wolfy1339 commented Jul 18, 2022 • edited

oscard0m commented Jul 19, 2022

timrogers commented Jul 19, 2022 • edited

timrogers commented Jul 19, 2022

wolfy1339 commented Jul 19, 2022

timrogers commented Jul 19, 2022

oscard0m commented Jul 19, 2022 • edited

wolfy1339 commented Jul 19, 2022

oscard0m commented Jul 19, 2022

octokitbot commented Jul 21, 2022

wolfy1339 commented Jul 18, 2022 •

edited

timrogers commented Jul 19, 2022 •

edited

oscard0m commented Jul 19, 2022 •

edited