Describe the bug
A clear and concise description of what the bug is.
I believe there is a bug here:
|
semver, err := version.NewVersion(versionStr) |
Or at least an overabundance of caution that will lead to too many false positives. The issue is that people often declare actions like: action@v4. This means "use the latest version of action where the major version is '4'". However, the logic on the above line, assumes that action@v4 actually means action v4.0.0. This results in components being reported as vulnerable when they aren't and often have not been for a very long time. One can argue about the pros and cons of not hard pinning an action version, but it is incontrovertible that this is a very common practice to pin, especially to a major version.
I think the proper fix, is to detect this case (probably major version is enough, but someone could theoretically pin to a minor version). And if this exists, and there is a fixed version of the vulnerable component that matches down to the pinned version, then it isn't vulnerable. Probably the simplest way is to pad out the semantic version with an absurdly high value so in the above example "v4" becomes "4.9999999999.9999999999" and "v4.1" becomes "4.1.9999999999". Or you could parse out all the to values in vulnerable ranges and left match against them: regex match '^4..*' against
Expected behavior
We don't get alerts about vulnerable components when we aren't hard-pinning and there's a fixed version.
Temp Patch
FWIW, I have made the following changes locally at the above location and "fixed" the issue in my local copy, though I don't know that this is the correct fix for the project as it makes some potential assumptions about the version string and the total number of versions
// import "strings" at the top
parts := strings.Split(versionStr, ".")
switch len(parts) {
case 1:
versionStr = versionStr + ".999999999.999999999"
case 2:
versionStr = versionStr + ".999999999"
}
// next line is: semver, err := version.NewVersion(versionStr)
Describe the bug
A clear and concise description of what the bug is.
I believe there is a bug here:
poutine/opa/builtins.go
Line 78 in f8df65b
Or at least an overabundance of caution that will lead to too many false positives. The issue is that people often declare actions like:
action@v4. This means "use the latest version of action where the major version is '4'". However, the logic on the above line, assumes thataction@v4actually means action v4.0.0. This results in components being reported as vulnerable when they aren't and often have not been for a very long time. One can argue about the pros and cons of not hard pinning an action version, but it is incontrovertible that this is a very common practice to pin, especially to a major version.I think the proper fix, is to detect this case (probably major version is enough, but someone could theoretically pin to a minor version). And if this exists, and there is a fixed version of the vulnerable component that matches down to the pinned version, then it isn't vulnerable. Probably the simplest way is to pad out the semantic version with an absurdly high value so in the above example "v4" becomes "4.9999999999.9999999999" and "v4.1" becomes "4.1.9999999999". Or you could parse out all the to values in vulnerable ranges and left match against them: regex match '^4..*' against
Expected behavior
We don't get alerts about vulnerable components when we aren't hard-pinning and there's a fixed version.
Temp Patch
FWIW, I have made the following changes locally at the above location and "fixed" the issue in my local copy, though I don't know that this is the correct fix for the project as it makes some potential assumptions about the version string and the total number of versions