Skip to content

Commit

Permalink
internal/{frontend,vuln}: display affected modules
Browse files Browse the repository at this point in the history
For vulns with no detailed package data, display affected
modules and versions.

Change-Id: Ibacdcd486cbb47b17a11d331692356a0603ac6d1
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/581181
kokoro-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Tatiana Bradley <tatianabradley@google.com>
  • Loading branch information
tatianab committed Apr 24, 2024
1 parent a3b6cd3 commit 41da83f
Show file tree
Hide file tree
Showing 9 changed files with 112 additions and 63 deletions.
19 changes: 11 additions & 8 deletions internal/frontend/vulns.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,11 @@ type VulnListPage struct {
// vuln entry.
type VulnEntryPage struct {
page.BasePage
Entry *osv.Entry
AffectedPackages []*vuln.AffectedPackage
AliasLinks []link
AdvisoryLinks []link
Entry *osv.Entry
AffectedPackages []*vuln.AffectedComponent
ModulesWithNoPackages []*vuln.AffectedComponent
AliasLinks []link
AdvisoryLinks []link
}

func (s *Server) serveVuln(w http.ResponseWriter, r *http.Request, _ internal.DataSource) error {
Expand Down Expand Up @@ -120,11 +121,13 @@ func newVulnEntryPage(ctx context.Context, client *vuln.Client, id string) (*Vul
if entry == nil {
return nil, derrors.NotFound
}
pkgs, mods := vuln.AffectedComponents(entry)
return &VulnEntryPage{
Entry: entry,
AffectedPackages: vuln.AffectedPackages(entry),
AliasLinks: aliasLinks(entry),
AdvisoryLinks: advisoryLinks(entry),
Entry: entry,
AffectedPackages: pkgs,
ModulesWithNoPackages: mods,
AliasLinks: aliasLinks(entry),
AdvisoryLinks: advisoryLinks(entry),
}, nil
}

Expand Down
27 changes: 16 additions & 11 deletions internal/vuln/vulns.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,11 @@ func toVulns(entries []*osv.Entry) []Vuln {
return vulns
}

// AffectedPackage holds information about a package affected by a certain vulnerability.
type AffectedPackage struct {
PackagePath string
Versions string
// Lists of affected symbols.
// AffectedComponent holds information about a module/package affected by a certain vulnerability.
type AffectedComponent struct {
Path string
Versions string
// Lists of affected symbols (for packages).
// If both of these lists are empty, all symbols in the package are affected.
ExportedSymbols []string
UnexportedSymbols []string
Expand Down Expand Up @@ -131,9 +131,8 @@ func collectRangePairs(a osv.Affected) []pair {
return ps
}

// AffectedPackages extracts information about affected packages from the given osv.Entry.
func AffectedPackages(e *osv.Entry) []*AffectedPackage {
var affs []*AffectedPackage
// AffectedComponents extracts information about affected packages (and // modules, if there are any with no package information) from the given osv.Entry.
func AffectedComponents(e *osv.Entry) (pkgs, modsNoPkgs []*AffectedComponent) {
for _, a := range e.Affected {
pairs := collectRangePairs(a)
var vs []string
Expand All @@ -152,18 +151,24 @@ func AffectedPackages(e *osv.Entry) []*AffectedPackage {
}
vs = append(vs, s)
}
if len(a.EcosystemSpecific.Packages) == 0 {
modsNoPkgs = append(modsNoPkgs, &AffectedComponent{
Path: a.Module.Path,
Versions: strings.Join(vs, ", "),
})
}
for _, p := range a.EcosystemSpecific.Packages {
exported, unexported := affectedSymbols(p.Symbols)
affs = append(affs, &AffectedPackage{
PackagePath: p.Path,
pkgs = append(pkgs, &AffectedComponent{
Path: p.Path,
Versions: strings.Join(vs, ", "),
ExportedSymbols: exported,
UnexportedSymbols: unexported,
// TODO(hyangah): where to place GOOS/GOARCH info
})
}
}
return affs
return pkgs, modsNoPkgs
}

func affectedSymbols(in []string) (e, u []string) {
Expand Down
92 changes: 56 additions & 36 deletions internal/vuln/vulns_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ func TestCollectRangePairs(t *testing.T) {

}

func TestAffectedPackages_Versions(t *testing.T) {
func TestAffectedComponents_Versions(t *testing.T) {
for _, test := range []struct {
name string
in []osv.RangeEvent
Expand Down Expand Up @@ -231,7 +231,7 @@ func TestAffectedPackages_Versions(t *testing.T) {
}},
}},
}
out := AffectedPackages(entry)
out, _ := AffectedComponents(entry)
got := out[0].Versions
if got != test.want {
t.Errorf("got %q, want %q\n", got, test.want)
Expand All @@ -240,11 +240,12 @@ func TestAffectedPackages_Versions(t *testing.T) {
}
}

func TestAffectedPackagesPackagesSymbols(t *testing.T) {
func TestAffectedComponents(t *testing.T) {
tests := []struct {
name string
in *osv.Entry
want []*AffectedPackage
name string
in *osv.Entry
wantPkgs []*AffectedComponent
wantMods []*AffectedComponent
}{
{
name: "one symbol",
Expand All @@ -260,10 +261,11 @@ func TestAffectedPackagesPackagesSymbols(t *testing.T) {
},
}},
},
want: []*AffectedPackage{{
PackagePath: "example.com/mod/pkg",
wantPkgs: []*AffectedComponent{{
Path: "example.com/mod/pkg",
ExportedSymbols: []string{"F"},
}},
wantMods: nil,
},
{
name: "multiple symbols",
Expand All @@ -279,11 +281,12 @@ func TestAffectedPackagesPackagesSymbols(t *testing.T) {
},
}},
},
want: []*AffectedPackage{{
PackagePath: "example.com/mod/pkg",
wantPkgs: []*AffectedComponent{{
Path: "example.com/mod/pkg",
ExportedSymbols: []string{"F", "S.F"},
UnexportedSymbols: []string{"g", "S.f", "s.F", "s.f"},
}},
wantMods: nil,
},
{
name: "no symbol",
Expand All @@ -298,51 +301,68 @@ func TestAffectedPackagesPackagesSymbols(t *testing.T) {
},
}},
},
want: []*AffectedPackage{{
PackagePath: "example.com/mod/pkg",
wantPkgs: []*AffectedComponent{{
Path: "example.com/mod/pkg",
}},
wantMods: nil,
},
{
name: "multiple pkgs and modules",
in: &osv.Entry{
ID: "GO-2022-0004",
Affected: []osv.Affected{{
Module: osv.Module{Path: "example.com/mod1"},
EcosystemSpecific: osv.EcosystemSpecific{
Packages: []osv.Package{{
Path: "example.com/mod1/pkg1",
}, {
Path: "example.com/mod1/pkg2",
Symbols: []string{"F"},
}},
},
}, {
Module: osv.Module{Path: "example.com/mod2"},
EcosystemSpecific: osv.EcosystemSpecific{
Packages: []osv.Package{{
Path: "example.com/mod2/pkg3",
Symbols: []string{"g", "H"},
Affected: []osv.Affected{
{
Module: osv.Module{Path: "example.com/mod"},
Ranges: []osv.Range{{
Type: osv.RangeTypeSemver,
Events: []osv.RangeEvent{{Fixed: "1.5"}},
}},
// no packages
},
}},
{
Module: osv.Module{Path: "example.com/mod1"},
EcosystemSpecific: osv.EcosystemSpecific{
Packages: []osv.Package{{
Path: "example.com/mod1/pkg1",
}, {
Path: "example.com/mod1/pkg2",
Symbols: []string{"F"},
}},
},
}, {
Module: osv.Module{Path: "example.com/mod2"},
EcosystemSpecific: osv.EcosystemSpecific{
Packages: []osv.Package{{
Path: "example.com/mod2/pkg3",
Symbols: []string{"g", "H"},
}},
},
}},
},
want: []*AffectedPackage{{
PackagePath: "example.com/mod1/pkg1",
wantPkgs: []*AffectedComponent{{
Path: "example.com/mod1/pkg1",
}, {
PackagePath: "example.com/mod1/pkg2",
Path: "example.com/mod1/pkg2",
ExportedSymbols: []string{"F"},
}, {
PackagePath: "example.com/mod2/pkg3",
Path: "example.com/mod2/pkg3",
ExportedSymbols: []string{"H"},
UnexportedSymbols: []string{"g"},
}},
wantMods: []*AffectedComponent{{
Path: "example.com/mod",
Versions: "before v1.5",
}},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := AffectedPackages(tt.in)
if diff := cmp.Diff(tt.want, got, cmpopts.IgnoreUnexported(AffectedPackage{})); diff != "" {
t.Errorf("mismatch (-want, +got):\n%s", diff)
gotPkgs, gotMods := AffectedComponents(tt.in)
if diff := cmp.Diff(tt.wantPkgs, gotPkgs, cmpopts.IgnoreUnexported(AffectedComponent{})); diff != "" {
t.Errorf("pkgs mismatch (-want, +got):\n%s", diff)
}
if diff := cmp.Diff(tt.wantMods, gotMods, cmpopts.IgnoreUnexported(AffectedComponent{})); diff != "" {
t.Errorf("mods mismatch (-want, +got):\n%s", diff)
}
})
}
Expand Down
4 changes: 4 additions & 0 deletions static/frontend/vuln/entry/entry.css
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@
padding: 0.5rem;
}

.VulnEntryModules {
grid-template-columns: minmax(10em, 50%) 1fr;
}

/* Header */
.VulnEntryPackages-item-container:first-child {
background-color: var(--color-background-accented);
Expand Down
2 changes: 1 addition & 1 deletion static/frontend/vuln/entry/entry.min.css

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions static/frontend/vuln/entry/entry.min.css.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 22 additions & 5 deletions static/frontend/vuln/entry/entry.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@
</p>
{{end}}
<div class="VulnEntry">
{{template "affected" .AffectedPackages}}
{{with .ModulesWithNoPackages}}{{template "affected-modules" .}}{{end}}
{{with .AffectedPackages}}{{template "affected-packages" .}}{{end}}
{{template "entry" .}}
</div>
{{end}}
Expand All @@ -53,7 +54,7 @@
</div>
{{end}}

{{define "affected"}}
{{define "affected-packages"}}
<h2>Affected Packages</h2>
<ul class="VulnEntryPackages VulnEntryPackages-container">
<li class="VulnEntryPackages-item VulnEntryPackages-item-container">
Expand All @@ -63,14 +64,14 @@
</li>
{{range .}}
<li class="VulnEntryPackages-item VulnEntryPackages-item-container">
<div class="VulnEntryPackages-attr" data-name="Path"><a href="/{{.PackagePath}}">{{.PackagePath}}</a></div>
<div class="VulnEntryPackages-attr" data-name="Path"><a href="/{{.Path}}">{{.Path}}</a></div>
<div class="VulnEntryPackages-attr" data-name="Versions">{{if .Versions}}{{.Versions}}{{else}}all versions, no known fixed{{end}}</div>
<div class="VulnEntryPackages-attr VulnEntryPackages-symbols" data-name="Symbols">
{{ $vuln := . }}
{{if .ExportedSymbols}}{{ $length := len .ExportedSymbols}}
{{if lt $length 5}}<ul>{{range .ExportedSymbols}}<li><a href="/{{$vuln.PackagePath}}#{{.}}">{{.}}</a></li>{{end}}</ul>
{{if lt $length 5}}<ul>{{range .ExportedSymbols}}<li><a href="/{{$vuln.Path}}#{{.}}">{{.}}</a></li>{{end}}</ul>
{{else}}<details><summary>{{len .ExportedSymbols}} affected symbols</summary>
<ul class="VulnEntryPackages-detailsContent">{{range .ExportedSymbols}}<li><a href="/{{$vuln.PackagePath}}#{{.}}">{{.}}</a></li>{{end}}</ul></details>
<ul class="VulnEntryPackages-detailsContent">{{range .ExportedSymbols}}<li><a href="/{{$vuln.Path}}#{{.}}">{{.}}</a></li>{{end}}</ul></details>
{{end}}
{{else if .UnexportedSymbols}}
<details><summary>{{len .UnexportedSymbols}} unexported affected symbols</summary>
Expand All @@ -87,6 +88,22 @@
</ul>
{{end}}

{{define "affected-modules"}}
<h2>Affected Modules</h2>
<ul class="VulnEntryPackages VulnEntryPackages-container">
<li class="VulnEntryPackages-item VulnEntryPackages-item-container VulnEntryModules">
<div class="VulnEntryPackages-attr">Path</div>
<div class="VulnEntryPackages-attr">Versions</div>
</li>
{{range .}}
<li class="VulnEntryPackages-item VulnEntryPackages-item-container VulnEntryModules">
<div class="VulnEntryPackages-attr" data-name="Path"><a href="/{{.Path}}">{{.Path}}</a></div>
<div class="VulnEntryPackages-attr" data-name="Versions">{{if .Versions}}{{.Versions}}{{else}}all versions, no known fixed{{end}}</div>
</li>
{{end}}
</ul>
{{end}}

{{define "entry"}}
{{$e := .Entry}}
{{if .AliasLinks}}
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/screentest/testdata/ci/vuln-entry-no-packages.a.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit 41da83f

Please sign in to comment.