Skip to content

Add last_affected event type. #38

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Mar 24, 2022
Merged

Add last_affected event type. #38

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3a9459d
Add `last_affected` event type.
oliverchang Mar 17, 2022
79c97a0
Update docs/schema.md
oliverchang Mar 18, 2022
39627c3
JSON validation
oliverchang Mar 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions docs/schema.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ A JSON Schema for validation is also available
"events": [ {
"introduced": string,
"fixed": string,
"last_affected": string,
"limit": string
} ]
} ],
Expand Down Expand Up @@ -443,7 +444,8 @@ The `ranges` object's `events` field is a JSON array of objects. Each object
describes a single version that either:
1. Introduces a vulnerability: `{"introduced": string}`
2. Fixes a vulnerability: `{"fixed": string}`
3. Sets an upper limit on the range being described: `{"limit": string}`
3. Describes the last known affected version: `{"last_affected": string}`
4. Sets an upper limit on the range being described: `{"limit": string}`

These `events` objects represent a "timeline" of status changes for the affected
package.
Expand All @@ -461,9 +463,14 @@ by the `affected[].ranges[].type` field.

#### Requirements

Only **a single type** (either `"introduced"`, `"fixed"`, `"limit"`) is allowed in
each event object. For instance, `{"introduced": "1.0.0", "fixed": "1.0.2"}` is
**invalid**.
Only **a single type** (either `"introduced"`, `"fixed"`, `"last_affected"`,
`"limit"`) is allowed in each event object. For instance,
`{"introduced": "1.0.0", "fixed": "1.0.2"}` is **invalid**.

Entries in the `events` array can contain either `"last_affected"` or `"fixed"`
events, but not both. It's **strongly recommended** to use `"fixed"` instead of
`"last_affected"` where possible, as it precisely identifies the version which
contains the fix.

There must be at least one `"introduced"` object in the `events` array. While
not required, it's also recommended to keep the `events` array sorted according
Expand Down Expand Up @@ -545,6 +552,8 @@ func IncludedInRanges(v, ranges)
vulnerable = true
else if evt.fixed is present && v >= evt.fixed
vulnerable = false
else if evt.last_affected is present && v > evt.last_affected
vulnerable = false

return vulnerable

Expand Down
33 changes: 33 additions & 0 deletions validation/schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,17 @@
"fixed"
]
},
{
"type": "object",
"properties": {
"last_affected": {
"type": "string"
}
},
"required": [
"last_affected"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also add validation to assert that a fixed event cannot occur alongside a last_effected event and vice versa?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, that's a good idea! I'll look at doing this.

That said, I'm wondering how this would look like for GHSA specifically when an entry wants to encode both <= and < (the patched version)? Would you just be using "fixed" here to capture the < and then database_specific to capture the <= ?

Copy link
Contributor

@chrisbloom7 chrisbloom7 Mar 18, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Currently if an advisory has a fix we replace the upper bound with < f.i.x when we rehydrate and consider it an acceptable change. It's only for advisories that don't have a fix that we need to add the rehydration hints.

Copy link
Contributor

Choose a reason for hiding this comment

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

And same thing for the other direction when transforming to OSV - if an advisory has a fix, we add the fixed event and don't add the rehydration hint.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for clarifying!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also added the JSON validation.

]
},
{
"type": "object",
"properties": {
Expand Down Expand Up @@ -162,6 +173,28 @@
"repo"
]
}
},
{
"if": {
"properties": {
"events": {
"contains": {
"required": ["last_affected"]
}
}
}
},
"then": {
"not": {
"properties": {
"events": {
"contains": {
"required": ["fixed"]
}
}
}
}
}
}
],
"required": [
Expand Down